Skip to content

Commit

Permalink
Code review fulfillment
Browse files Browse the repository at this point in the history
Signed-off-by: Fabian Stäber <fabian@fstab.de>
  • Loading branch information
fstab committed Dec 15, 2023
1 parent 20a7831 commit fd073f9
Show file tree
Hide file tree
Showing 8 changed files with 180 additions and 19 deletions.
1 change: 1 addition & 0 deletions dependencyManagement/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ val DEPENDENCIES = listOf(
"com.google.guava:guava-beta-checker:1.0",
"com.sun.net.httpserver:http:20070405",
"com.tngtech.archunit:archunit-junit5:1.2.1",
"io.prometheus:prometheus-metrics-exporter-httpserver:1.1.0",
"com.uber.nullaway:nullaway:0.10.18",
"edu.berkeley.cs.jqf:jqf-fuzz:1.7", // jqf-fuzz version 1.8+ requires Java 11+
"eu.rekawek.toxiproxy:toxiproxy-java:2.1.7",
Expand Down
5 changes: 2 additions & 3 deletions exporters/prometheus/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,15 @@ dependencies {
api(project(":sdk:metrics"))

implementation(project(":sdk-extensions:autoconfigure-spi"))
implementation("io.prometheus:prometheus-metrics-exporter-httpserver:1.1.0")
implementation("io.prometheus:prometheus-metrics-exporter-httpserver")

compileOnly("com.sun.net.httpserver:http")
compileOnly("com.google.auto.value:auto-value-annotations")

annotationProcessor("com.google.auto.value:auto-value")

testImplementation(project(":sdk:testing"))
testImplementation("io.opentelemetry.proto:opentelemetry-proto")

testImplementation("com.sun.net.httpserver:http")
testImplementation("com.google.guava:guava")
testImplementation("com.linecorp.armeria:armeria")
testImplementation("com.linecorp.armeria:armeria-junit5")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
import javax.annotation.Nullable;

/** Convert OpenTelemetry {@link MetricData} to Prometheus {@link MetricSnapshots}. */
public class Otel2PrometheusConverter {
final class Otel2PrometheusConverter {

private static final Logger LOGGER = Logger.getLogger(Otel2PrometheusConverter.class.getName());
private static final ThrottlingLogger THROTTLING_LOGGER = new ThrottlingLogger(LOGGER);
Expand All @@ -74,21 +74,17 @@ public class Otel2PrometheusConverter {
private static final String OTEL_SCOPE_VERSION = "otel_scope_version";
private static final long NANOS_PER_MILLISECOND = TimeUnit.MILLISECONDS.toNanos(1);

public Otel2PrometheusConverter() {
this(true);
}

/**
* Constructor with feature flag parameter.
*
* @param otelScopeEnabled enable generation of the OpenTelemetry instrumentation scope info
* metric and labels.
*/
public Otel2PrometheusConverter(boolean otelScopeEnabled) {
Otel2PrometheusConverter(boolean otelScopeEnabled) {
this.otelScopeEnabled = otelScopeEnabled;
}

public MetricSnapshots convert(@Nullable Collection<MetricData> metricDataCollection) {
MetricSnapshots convert(@Nullable Collection<MetricData> metricDataCollection) {
if (metricDataCollection == null || metricDataCollection.isEmpty()) {
return MetricSnapshots.of();

Check warning on line 89 in exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java#L89

Added line #L89 was not covered by tests
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,9 @@ public static PrometheusHttpServerBuilder builder() {
String host,
int port,
@Nullable ExecutorService executor,
PrometheusRegistry prometheusRegistry) {
this.prometheusMetricReader = new PrometheusMetricReader(/* otelScopeEnabled= */ true);
PrometheusRegistry prometheusRegistry,
boolean otelScopeEnabled) {
this.prometheusMetricReader = new PrometheusMetricReader(otelScopeEnabled);
this.host = host;
this.prometheusRegistry = prometheusRegistry;
prometheusRegistry.register(prometheusMetricReader);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public final class PrometheusHttpServerBuilder {
private String host = DEFAULT_HOST;
private int port = DEFAULT_PORT;
private PrometheusRegistry prometheusRegistry = PrometheusRegistry.defaultRegistry;
private boolean otelScopeEnabled = true;

@Nullable private ExecutorService executor;

Expand Down Expand Up @@ -53,12 +54,18 @@ public PrometheusHttpServerBuilder setPrometheusRegistry(PrometheusRegistry prom
return this;

Check warning on line 54 in exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerBuilder.java

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerBuilder.java#L52-L54

Added lines #L52 - L54 were not covered by tests
}

/** Set if the {@code otel_scope_*} attributes are generated. Default is {@code true}. */
public PrometheusHttpServerBuilder setOtelScopeEnabled(boolean otelScopeEnabled) {
this.otelScopeEnabled = otelScopeEnabled;
return this;

Check warning on line 60 in exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerBuilder.java

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerBuilder.java#L59-L60

Added lines #L59 - L60 were not covered by tests
}

/**
* Returns a new {@link PrometheusHttpServer} with the configuration of this builder which can be
* registered with a {@link io.opentelemetry.sdk.metrics.SdkMeterProvider}.
*/
public PrometheusHttpServer build() {
return new PrometheusHttpServer(host, port, executor, prometheusRegistry);
return new PrometheusHttpServer(host, port, executor, prometheusRegistry, otelScopeEnabled);
}

PrometheusHttpServerBuilder() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ class PrometheusUnitsHelper {
// https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/c3b2997563106e11d39f66eec629fde25dce2bdd/pkg/translator/prometheus/normalize_name.go#L19-L19
static {
// Time
initUnit("a", "years", "year");
initUnit("mo", "months", "month");
initUnit("wk", "weeks", "week");
initUnit("d", "days", "day");
initUnit("h", "hours", "hour");
initUnit("min", "minutes", "minute");
Expand Down Expand Up @@ -71,17 +74,24 @@ static Unit convertUnit(String otelUnit) {
// SDK.
return null;
}
if (otelUnit.contains("{")) {
otelUnit = otelUnit.replaceAll("\\{[^}]*}", "").trim();
if (otelUnit.isEmpty() || otelUnit.equals("/")) {
return null;
}
}
if (predefinedUnits.containsKey(otelUnit)) {
return predefinedUnits.get(otelUnit);
}
if (otelUnit.contains("{")) {
return null;
}
if (otelUnit.contains("/")) {
String[] parts = otelUnit.split("/", 2);
String part1 = pluralNames.getOrDefault(parts[0], parts[0]);
String part2 = singularNames.getOrDefault(parts[1], parts[1]);
return new Unit(part1 + "_per_" + part2);
String part1 = pluralNames.getOrDefault(parts[0], parts[0]).trim();
String part2 = singularNames.getOrDefault(parts[1], parts[1]).trim();
if (part1.isEmpty()) {
return new Unit("per_" + part2);
} else {
return new Unit(part1 + "_per_" + part2);
}
}
return new Unit(otelUnit);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,50 @@ public void testExponentialLongHistogramScaleDown() throws IOException {
Assertions.assertEquals(10, buckets.getCount(2));
}

@Test
public void testInstrumentationScope() throws IOException {
SdkMeterProvider meterProvider =
SdkMeterProvider.builder()
.setClock(testClock)
.registerMetricReader(this.reader)
.setResource(
Resource.getDefault().toBuilder().put("telemetry.sdk.version", "1.x.x").build())
.build();
Meter meter1 = meterProvider.meterBuilder("scopeA").setInstrumentationVersion("1.1").build();
Meter meter2 = meterProvider.meterBuilder("scopeB").setInstrumentationVersion("1.2").build();
meter1
.counterBuilder("processing.time")
.setDescription("processing time in seconds")
.setUnit("s")
.ofDoubles()
.build()
.add(3.3, Attributes.builder().put("a", "b").build());
meter2
.counterBuilder("processing.time")
.setDescription("processing time in seconds")
.setUnit("s")
.ofDoubles()
.build()
.add(3.3, Attributes.builder().put("a", "b").build());
String expected =
""
+ "# TYPE processing_time_seconds counter\n"
+ "# UNIT processing_time_seconds seconds\n"
+ "# HELP processing_time_seconds processing time in seconds\n"
+ "processing_time_seconds_total{a=\"b\",otel_scope_name=\"scopeA\",otel_scope_version=\"1.1\"} 3.3\n"
+ "processing_time_seconds_created{a=\"b\",otel_scope_name=\"scopeA\",otel_scope_version=\"1.1\"} "
+ createdTimestamp
+ "\n"
+ "processing_time_seconds_total{a=\"b\",otel_scope_name=\"scopeB\",otel_scope_version=\"1.2\"} 3.3\n"
+ "processing_time_seconds_created{a=\"b\",otel_scope_name=\"scopeB\",otel_scope_version=\"1.2\"} "
+ createdTimestamp
+ "\n"
+ "# TYPE target info\n"
+ "target_info{service_name=\"unknown_service:java\",telemetry_sdk_language=\"java\",telemetry_sdk_name=\"opentelemetry\",telemetry_sdk_version=\"1.x.x\"} 1\n"
+ "# EOF\n";
assertEquals(expected, toOpenMetrics(reader.collect()));
}

@Test
public void testNameSuffix() throws IOException {
LongCounter unitAndTotal =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.exporter.prometheus;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;

import io.prometheus.metrics.model.snapshots.Unit;
import java.util.stream.Stream;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

class PrometheusUnitsHelperTest {

@ParameterizedTest
@MethodSource("providePrometheusOTelUnitEquivalentPairs")
public void testPrometheusUnitEquivalency(String otlpUnit, String expectedPrometheusUnit) {
Unit actualPrometheusUnit = PrometheusUnitsHelper.convertUnit(otlpUnit);
if (expectedPrometheusUnit == null) {
assertNull(actualPrometheusUnit);
} else {
assertEquals(expectedPrometheusUnit, actualPrometheusUnit.toString());
}
}

private static Stream<Arguments> providePrometheusOTelUnitEquivalentPairs() {
return Stream.of(
// Simple expansion - storage Bytes
Arguments.of("By", "bytes"),
// Simple expansion - storage KBy
Arguments.of("KBy", "kilobytes"),
// Simple expansion - storage MBy
Arguments.of("MBy", "megabytes"),
// Simple expansion - storage GBy
Arguments.of("GBy", "gigabytes"),
// Simple expansion - storage TBy
Arguments.of("TBy", "terabytes"),
// Simple expansion - storage KiBy
Arguments.of("KiBy", "kibibytes"),
// Simple expansion - storage MiBy
Arguments.of("MiBy", "mebibytes"),
// Simple expansion - storage GiBy
Arguments.of("GiBy", "gibibytes"),
// Simple expansion - storage TiBy
Arguments.of("TiBy", "tibibytes"),
// Simple expansion - Time unit d
Arguments.of("d", "days"),
// Simple expansion - Time unit h
Arguments.of("h", "hours"),
// Simple expansion - Time unit s
Arguments.of("s", "seconds"),
// Simple expansion - Time unit ms
Arguments.of("ms", "milliseconds"),
// Simple expansion - Time unit us
Arguments.of("us", "microseconds"),
// Simple expansion - Time unit ns
Arguments.of("ns", "nanoseconds"),
// Simple expansion - Time unit min
Arguments.of("min", "minutes"),
// Simple expansion - special symbol - %
Arguments.of("%", "percent"),
// Simple expansion - frequency
Arguments.of("Hz", "hertz"),
// Simple expansion - temperature
Arguments.of("Cel", "celsius"),
// Unit not found - Case sensitive
Arguments.of("S", "S"),
// Special case - 1
Arguments.of("1", null),
// Special Case - Drop metric units in {}
Arguments.of("{packets}", null),
// Special Case - Dropped metric units only in {}
Arguments.of("{packets}V", "volts"),
// Special Case - Dropped metric units with 'per' unit handling applicable
Arguments.of("{scanned}/{returned}", null),
// Special Case - Dropped metric units with 'per' unit handling applicable
Arguments.of("{objects}/s", "per_second"),
// Units expressing rate - 'per' units, both units expanded
Arguments.of("m/s", "meters_per_second"),
// Units expressing rate - per minute
Arguments.of("m/min", "meters_per_minute"),
// Units expressing rate - per day
Arguments.of("A/d", "amperes_per_day"),
// Units expressing rate - per week
Arguments.of("W/wk", "watts_per_week"),
// Units expressing rate - per month
Arguments.of("J/mo", "joules_per_month"),
// Units expressing rate - per year
Arguments.of("TBy/a", "terabytes_per_year"),
// Units expressing rate - 'per' units, both units unknown
Arguments.of("v/v", "v_per_v"),
// Units expressing rate - 'per' units, first unit unknown
Arguments.of("km/h", "km_per_hour"),
// Units expressing rate - 'per' units, 'per' unit unknown
Arguments.of("g/x", "grams_per_x"),
// Misc - unit containing known abbreviations improperly formatted
Arguments.of("watts_W", "watts_W"));
}
}

0 comments on commit fd073f9

Please sign in to comment.