From 46fdadc2a409477efbb0ec2e2594e6561991667b Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 27 Jun 2024 16:29:18 +0530 Subject: [PATCH 1/8] adds config to enable default exponential histogram --- .../prometheus/PrometheusHttpServer.java | 12 ++++++++- .../PrometheusHttpServerBuilder.java | 21 +++++++++++++++- .../PrometheusMetricReaderProvider.java | 21 ++++++++++++++++ .../prometheus/PrometheusHttpServerTest.java | 3 +++ .../PrometheusMetricReaderProviderTest.java | 25 +++++++++++++++++++ 5 files changed, 80 insertions(+), 2 deletions(-) diff --git a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServer.java b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServer.java index 0a306fccaa3..78609ee7549 100644 --- a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServer.java +++ b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServer.java @@ -14,9 +14,11 @@ import io.opentelemetry.sdk.common.CompletableResultCode; import io.opentelemetry.sdk.common.export.MemoryMode; import io.opentelemetry.sdk.internal.DaemonThreadFactory; +import io.opentelemetry.sdk.metrics.Aggregation; import io.opentelemetry.sdk.metrics.InstrumentType; import io.opentelemetry.sdk.metrics.data.AggregationTemporality; import io.opentelemetry.sdk.metrics.export.CollectionRegistration; +import io.opentelemetry.sdk.metrics.export.DefaultAggregationSelector; import io.opentelemetry.sdk.metrics.export.MetricReader; import io.prometheus.metrics.exporter.httpserver.HTTPServer; import io.prometheus.metrics.model.registry.PrometheusRegistry; @@ -41,6 +43,7 @@ public final class PrometheusHttpServer implements MetricReader { private final PrometheusRegistry prometheusRegistry; private final String host; private final MemoryMode memoryMode; + private final DefaultAggregationSelector defaultAggregationSelector; /** * Returns a new {@link PrometheusHttpServer} which can be registered to an {@link @@ -65,7 +68,8 @@ public static PrometheusHttpServerBuilder builder() { boolean otelScopeEnabled, @Nullable Predicate allowedResourceAttributesFilter, MemoryMode memoryMode, - @Nullable HttpHandler defaultHandler) { + @Nullable HttpHandler defaultHandler, + DefaultAggregationSelector defaultAggregationSelector) { this.builder = builder; this.prometheusMetricReader = new PrometheusMetricReader(otelScopeEnabled, allowedResourceAttributesFilter); @@ -92,6 +96,7 @@ public static PrometheusHttpServerBuilder builder() { } catch (IOException e) { throw new UncheckedIOException("Could not create Prometheus HTTP server", e); } + this.defaultAggregationSelector = defaultAggregationSelector; } @Override @@ -99,6 +104,11 @@ public AggregationTemporality getAggregationTemporality(InstrumentType instrumen return prometheusMetricReader.getAggregationTemporality(instrumentType); } + @Override + public Aggregation getDefaultAggregation(InstrumentType instrumentType) { + return defaultAggregationSelector.getDefaultAggregation(instrumentType); + } + @Override public MemoryMode getMemoryMode() { return memoryMode; diff --git a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerBuilder.java b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerBuilder.java index 20f36fdf426..89bae976f21 100644 --- a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerBuilder.java +++ b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerBuilder.java @@ -10,6 +10,9 @@ import com.sun.net.httpserver.HttpHandler; import io.opentelemetry.sdk.common.export.MemoryMode; +import io.opentelemetry.sdk.metrics.InstrumentType; +import io.opentelemetry.sdk.metrics.export.DefaultAggregationSelector; +import io.opentelemetry.sdk.metrics.export.MetricExporter; import io.prometheus.metrics.model.registry.PrometheusRegistry; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -31,6 +34,8 @@ public final class PrometheusHttpServerBuilder { @Nullable private ExecutorService executor; private MemoryMode memoryMode = DEFAULT_MEMORY_MODE; @Nullable private HttpHandler defaultHandler; + private DefaultAggregationSelector defaultAggregationSelector = + DefaultAggregationSelector.getDefault(); PrometheusHttpServerBuilder() {} @@ -126,6 +131,19 @@ public PrometheusHttpServerBuilder setDefaultHandler(HttpHandler defaultHandler) return this; } + /** + * Set the {@link DefaultAggregationSelector} used for {@link + * MetricExporter#getDefaultAggregation(InstrumentType)}. + * + *

If unset, defaults to {@link DefaultAggregationSelector#getDefault()}. + */ + public PrometheusHttpServerBuilder setDefaultAggregationSelector( + DefaultAggregationSelector defaultAggregationSelector) { + requireNonNull(defaultAggregationSelector, "defaultAggregationSelector"); + this.defaultAggregationSelector = defaultAggregationSelector; + return this; + } + /** * Returns a new {@link PrometheusHttpServer} with the configuration of this builder which can be * registered with a {@link io.opentelemetry.sdk.metrics.SdkMeterProvider}. @@ -140,6 +158,7 @@ public PrometheusHttpServer build() { otelScopeEnabled, allowedResourceAttributesFilter, memoryMode, - defaultHandler); + defaultHandler, + defaultAggregationSelector); } } diff --git a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProvider.java b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProvider.java index 39d6c755e17..c06dc204212 100644 --- a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProvider.java +++ b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProvider.java @@ -5,12 +5,19 @@ package io.opentelemetry.exporter.prometheus.internal; +import static io.opentelemetry.sdk.metrics.Aggregation.explicitBucketHistogram; + import io.opentelemetry.exporter.internal.ExporterBuilderUtil; import io.opentelemetry.exporter.prometheus.PrometheusHttpServer; import io.opentelemetry.exporter.prometheus.PrometheusHttpServerBuilder; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; +import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException; import io.opentelemetry.sdk.autoconfigure.spi.internal.ConfigurableMetricReaderProvider; +import io.opentelemetry.sdk.metrics.Aggregation; +import io.opentelemetry.sdk.metrics.InstrumentType; +import io.opentelemetry.sdk.metrics.export.DefaultAggregationSelector; import io.opentelemetry.sdk.metrics.export.MetricReader; +import io.opentelemetry.sdk.metrics.internal.aggregator.AggregationUtil; /** * SPI implementation for {@link PrometheusHttpServer}. @@ -32,6 +39,20 @@ public MetricReader createMetricReader(ConfigProperties config) { if (host != null) { prometheusBuilder.setHost(host); } + String defaultHistogramAggregation = + config.getString("otel.exporter.prometheus.metrics.default.histogram.aggregation"); + if (defaultHistogramAggregation != null) { + if (AggregationUtil.aggregationName(Aggregation.base2ExponentialBucketHistogram()) + .equalsIgnoreCase(defaultHistogramAggregation)) { + prometheusBuilder.setDefaultAggregationSelector( + DefaultAggregationSelector.getDefault() + .with(InstrumentType.HISTOGRAM, Aggregation.base2ExponentialBucketHistogram())); + } else if (!AggregationUtil.aggregationName(explicitBucketHistogram()) + .equalsIgnoreCase(defaultHistogramAggregation)) { + throw new ConfigurationException( + "Unrecognized default histogram aggregation: " + defaultHistogramAggregation); + } + } ExporterBuilderUtil.configureExporterMemoryMode(config, prometheusBuilder::setMemoryMode); diff --git a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerTest.java b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerTest.java index 9b328493c1f..61a48ab2632 100644 --- a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerTest.java +++ b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerTest.java @@ -113,6 +113,9 @@ void invalidConfig() { assertThatThrownBy(() -> PrometheusHttpServer.builder().setHost("")) .isInstanceOf(IllegalArgumentException.class) .hasMessage("host must not be empty"); + assertThatThrownBy(() -> PrometheusHttpServer.builder().setDefaultAggregationSelector(null)) + .isInstanceOf(NullPointerException.class) + .hasMessage("defaultAggregationSelector"); } @Test diff --git a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java index 5f450ab751f..0502bcf6540 100644 --- a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java +++ b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java @@ -7,14 +7,18 @@ import static org.assertj.core.api.Assertions.as; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; import com.sun.net.httpserver.HttpServer; import io.opentelemetry.exporter.prometheus.PrometheusHttpServer; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; +import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException; import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties; import io.opentelemetry.sdk.common.export.MemoryMode; +import io.opentelemetry.sdk.metrics.Aggregation; +import io.opentelemetry.sdk.metrics.InstrumentType; import io.opentelemetry.sdk.metrics.export.MetricReader; import io.prometheus.metrics.exporter.httpserver.HTTPServer; import java.io.IOException; @@ -59,6 +63,8 @@ void createMetricReader_Default() throws IOException { assertThat(server.getAddress().getPort()).isEqualTo(9464); }); assertThat(metricReader.getMemoryMode()).isEqualTo(MemoryMode.IMMUTABLE_DATA); + assertThat(metricReader.getDefaultAggregation(InstrumentType.HISTOGRAM)) + .isEqualTo(Aggregation.defaultAggregation()); } } @@ -76,6 +82,9 @@ void createMetricReader_WithConfiguration() throws IOException { config.put("otel.exporter.prometheus.host", "localhost"); config.put("otel.exporter.prometheus.port", String.valueOf(port)); config.put("otel.java.experimental.exporter.memory_mode", "reusable_data"); + config.put( + "otel.exporter.prometheus.metrics.default.histogram.aggregation", + "BASE2_EXPONENTIAL_BUCKET_HISTOGRAM"); when(configProperties.getInt(any())).thenReturn(null); when(configProperties.getString(any())).thenReturn(null); @@ -91,6 +100,22 @@ void createMetricReader_WithConfiguration() throws IOException { assertThat(server.getAddress().getPort()).isEqualTo(port); }); assertThat(metricReader.getMemoryMode()).isEqualTo(MemoryMode.REUSABLE_DATA); + assertThat(metricReader.getDefaultAggregation(InstrumentType.HISTOGRAM)) + .isEqualTo(Aggregation.base2ExponentialBucketHistogram()); } } + + @Test + void createMetricReader_WithWrongConfiguration() throws IOException { + Map config = new HashMap<>(); + config.put("otel.exporter.prometheus.metrics.default.histogram.aggregation", "foo"); + + when(configProperties.getInt(any())).thenReturn(null); + when(configProperties.getString(any())).thenReturn(null); + + assertThatThrownBy( + () -> provider.createMetricReader(DefaultConfigProperties.createFromMap(config))) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("Unrecognized default histogram aggregation:"); + } } From 4fda8cd89247a3d9b9ecd9fffec6bcaaadd0f297 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 27 Jun 2024 18:09:18 +0530 Subject: [PATCH 2/8] adds more tests --- .../PrometheusHttpServerBuilder.java | 1 + .../internal/PrometheusComponentProvider.java | 20 +++++++++++++++++++ .../prometheus/PrometheusHttpServerTest.java | 10 +++++++++- 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerBuilder.java b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerBuilder.java index 89bae976f21..c1138a6ea31 100644 --- a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerBuilder.java +++ b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerBuilder.java @@ -46,6 +46,7 @@ public final class PrometheusHttpServerBuilder { this.otelScopeEnabled = builder.otelScopeEnabled; this.allowedResourceAttributesFilter = builder.allowedResourceAttributesFilter; this.executor = builder.executor; + this.defaultAggregationSelector = builder.defaultAggregationSelector; } /** Sets the host to bind to. If unset, defaults to {@value #DEFAULT_HOST}. */ diff --git a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusComponentProvider.java b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusComponentProvider.java index dc57d2bcbe3..04ab721ee2f 100644 --- a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusComponentProvider.java +++ b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusComponentProvider.java @@ -5,11 +5,18 @@ package io.opentelemetry.exporter.prometheus.internal; +import static io.opentelemetry.sdk.metrics.Aggregation.explicitBucketHistogram; + import io.opentelemetry.exporter.prometheus.PrometheusHttpServer; import io.opentelemetry.exporter.prometheus.PrometheusHttpServerBuilder; +import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException; import io.opentelemetry.sdk.autoconfigure.spi.internal.ComponentProvider; import io.opentelemetry.sdk.autoconfigure.spi.internal.StructuredConfigProperties; +import io.opentelemetry.sdk.metrics.Aggregation; +import io.opentelemetry.sdk.metrics.InstrumentType; +import io.opentelemetry.sdk.metrics.export.DefaultAggregationSelector; import io.opentelemetry.sdk.metrics.export.MetricReader; +import io.opentelemetry.sdk.metrics.internal.aggregator.AggregationUtil; /** * File configuration SPI implementation for {@link PrometheusHttpServer}. @@ -41,6 +48,19 @@ public MetricReader create(StructuredConfigProperties config) { if (host != null) { prometheusBuilder.setHost(host); } + String defaultHistogramAggregation = config.getString("metrics.default.histogram.aggregation"); + if (defaultHistogramAggregation != null) { + if (AggregationUtil.aggregationName(Aggregation.base2ExponentialBucketHistogram()) + .equalsIgnoreCase(defaultHistogramAggregation)) { + prometheusBuilder.setDefaultAggregationSelector( + DefaultAggregationSelector.getDefault() + .with(InstrumentType.HISTOGRAM, Aggregation.base2ExponentialBucketHistogram())); + } else if (!AggregationUtil.aggregationName(explicitBucketHistogram()) + .equalsIgnoreCase(defaultHistogramAggregation)) { + throw new ConfigurationException( + "Unrecognized default histogram aggregation: " + defaultHistogramAggregation); + } + } return prometheusBuilder.build(); } diff --git a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerTest.java b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerTest.java index 61a48ab2632..bc0122fd5f5 100644 --- a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerTest.java +++ b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerTest.java @@ -25,9 +25,12 @@ import io.opentelemetry.internal.testing.slf4j.SuppressLogger; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; import io.opentelemetry.sdk.common.export.MemoryMode; +import io.opentelemetry.sdk.metrics.Aggregation; +import io.opentelemetry.sdk.metrics.InstrumentType; import io.opentelemetry.sdk.metrics.data.AggregationTemporality; import io.opentelemetry.sdk.metrics.data.MetricData; import io.opentelemetry.sdk.metrics.export.CollectionRegistration; +import io.opentelemetry.sdk.metrics.export.DefaultAggregationSelector; import io.opentelemetry.sdk.metrics.internal.data.ImmutableDoublePointData; import io.opentelemetry.sdk.metrics.internal.data.ImmutableGaugeData; import io.opentelemetry.sdk.metrics.internal.data.ImmutableLongPointData; @@ -503,10 +506,14 @@ private static List generateTestData() { @Test void toBuilder() { + DefaultAggregationSelector defaultAggregationSelector = + DefaultAggregationSelector.getDefault() + .with(InstrumentType.HISTOGRAM, Aggregation.base2ExponentialBucketHistogram()); PrometheusHttpServerBuilder builder = PrometheusHttpServer.builder(); builder.setHost("localhost"); builder.setPort(1234); builder.setOtelScopeEnabled(false); + builder.setDefaultAggregationSelector(defaultAggregationSelector); Predicate resourceAttributesFilter = s -> false; builder.setAllowedResourceAttributesFilter(resourceAttributesFilter); @@ -527,6 +534,7 @@ void toBuilder() { .hasFieldOrPropertyWithValue("otelScopeEnabled", false) .hasFieldOrPropertyWithValue("allowedResourceAttributesFilter", resourceAttributesFilter) .hasFieldOrPropertyWithValue("executor", executor) - .hasFieldOrPropertyWithValue("prometheusRegistry", prometheusRegistry); + .hasFieldOrPropertyWithValue("prometheusRegistry", prometheusRegistry) + .hasFieldOrPropertyWithValue("defaultAggregationSelector", defaultAggregationSelector); } } From a1a8095bdb81f77f135fb69289db1a075e1d4883 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 2 Jul 2024 17:23:18 +0530 Subject: [PATCH 3/8] updates config and moves common code to ExporterBuilderUtil --- .../internal/ExporterBuilderUtil.java | 25 ++++++++++++++++++ .../otlp/internal/OtlpConfigUtil.java | 20 +++----------- .../internal/PrometheusComponentProvider.java | 22 +++------------- .../PrometheusMetricReaderProvider.java | 26 ++++--------------- .../PrometheusMetricReaderProviderTest.java | 5 ++-- 5 files changed, 40 insertions(+), 58 deletions(-) diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/ExporterBuilderUtil.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/ExporterBuilderUtil.java index b19aad8e131..a04c032b19e 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/ExporterBuilderUtil.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/ExporterBuilderUtil.java @@ -5,9 +5,15 @@ package io.opentelemetry.exporter.internal; +import static io.opentelemetry.sdk.metrics.Aggregation.explicitBucketHistogram; + import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException; import io.opentelemetry.sdk.common.export.MemoryMode; +import io.opentelemetry.sdk.metrics.Aggregation; +import io.opentelemetry.sdk.metrics.InstrumentType; +import io.opentelemetry.sdk.metrics.export.DefaultAggregationSelector; +import io.opentelemetry.sdk.metrics.internal.aggregator.AggregationUtil; import java.net.URI; import java.net.URISyntaxException; import java.util.Locale; @@ -54,5 +60,24 @@ public static void configureExporterMemoryMode( memoryModeConsumer.accept(memoryMode); } + /** + * Invoke the {@code defaultAggregationSelectorConsumer} with the configured {@link + * DefaultAggregationSelector}. + */ + public static void configureHistogramDefaultAggregation( + String defaultHistogramAggregation, + Consumer defaultAggregationSelectorConsumer) { + if (AggregationUtil.aggregationName(Aggregation.base2ExponentialBucketHistogram()) + .equalsIgnoreCase(defaultHistogramAggregation)) { + defaultAggregationSelectorConsumer.accept( + DefaultAggregationSelector.getDefault() + .with(InstrumentType.HISTOGRAM, Aggregation.base2ExponentialBucketHistogram())); + } else if (!AggregationUtil.aggregationName(explicitBucketHistogram()) + .equalsIgnoreCase(defaultHistogramAggregation)) { + throw new ConfigurationException( + "Unrecognized default histogram aggregation: " + defaultHistogramAggregation); + } + } + private ExporterBuilderUtil() {} } diff --git a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java index 2b387d06367..a2a7b1d4d36 100644 --- a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java +++ b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java @@ -5,19 +5,14 @@ package io.opentelemetry.exporter.otlp.internal; -import static io.opentelemetry.sdk.metrics.Aggregation.explicitBucketHistogram; - import io.opentelemetry.exporter.internal.ExporterBuilderUtil; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException; import io.opentelemetry.sdk.common.export.MemoryMode; import io.opentelemetry.sdk.common.export.RetryPolicy; -import io.opentelemetry.sdk.metrics.Aggregation; -import io.opentelemetry.sdk.metrics.InstrumentType; import io.opentelemetry.sdk.metrics.data.AggregationTemporality; import io.opentelemetry.sdk.metrics.export.AggregationTemporalitySelector; import io.opentelemetry.sdk.metrics.export.DefaultAggregationSelector; -import io.opentelemetry.sdk.metrics.internal.aggregator.AggregationUtil; import java.io.File; import java.io.IOException; import java.io.RandomAccessFile; @@ -197,18 +192,9 @@ public static void configureOtlpHistogramDefaultAggregation( Consumer defaultAggregationSelectorConsumer) { String defaultHistogramAggregation = config.getString("otel.exporter.otlp.metrics.default.histogram.aggregation"); - if (defaultHistogramAggregation == null) { - return; - } - if (AggregationUtil.aggregationName(Aggregation.base2ExponentialBucketHistogram()) - .equalsIgnoreCase(defaultHistogramAggregation)) { - defaultAggregationSelectorConsumer.accept( - DefaultAggregationSelector.getDefault() - .with(InstrumentType.HISTOGRAM, Aggregation.base2ExponentialBucketHistogram())); - } else if (!AggregationUtil.aggregationName(explicitBucketHistogram()) - .equalsIgnoreCase(defaultHistogramAggregation)) { - throw new ConfigurationException( - "Unrecognized default histogram aggregation: " + defaultHistogramAggregation); + if (defaultHistogramAggregation != null) { + ExporterBuilderUtil.configureHistogramDefaultAggregation( + defaultHistogramAggregation, defaultAggregationSelectorConsumer); } } diff --git a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusComponentProvider.java b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusComponentProvider.java index 04ab721ee2f..26d6a5b6950 100644 --- a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusComponentProvider.java +++ b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusComponentProvider.java @@ -5,18 +5,12 @@ package io.opentelemetry.exporter.prometheus.internal; -import static io.opentelemetry.sdk.metrics.Aggregation.explicitBucketHistogram; - +import io.opentelemetry.exporter.internal.ExporterBuilderUtil; import io.opentelemetry.exporter.prometheus.PrometheusHttpServer; import io.opentelemetry.exporter.prometheus.PrometheusHttpServerBuilder; -import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException; import io.opentelemetry.sdk.autoconfigure.spi.internal.ComponentProvider; import io.opentelemetry.sdk.autoconfigure.spi.internal.StructuredConfigProperties; -import io.opentelemetry.sdk.metrics.Aggregation; -import io.opentelemetry.sdk.metrics.InstrumentType; -import io.opentelemetry.sdk.metrics.export.DefaultAggregationSelector; import io.opentelemetry.sdk.metrics.export.MetricReader; -import io.opentelemetry.sdk.metrics.internal.aggregator.AggregationUtil; /** * File configuration SPI implementation for {@link PrometheusHttpServer}. @@ -48,18 +42,10 @@ public MetricReader create(StructuredConfigProperties config) { if (host != null) { prometheusBuilder.setHost(host); } - String defaultHistogramAggregation = config.getString("metrics.default.histogram.aggregation"); + String defaultHistogramAggregation = config.getString("default_histogram_aggregation"); if (defaultHistogramAggregation != null) { - if (AggregationUtil.aggregationName(Aggregation.base2ExponentialBucketHistogram()) - .equalsIgnoreCase(defaultHistogramAggregation)) { - prometheusBuilder.setDefaultAggregationSelector( - DefaultAggregationSelector.getDefault() - .with(InstrumentType.HISTOGRAM, Aggregation.base2ExponentialBucketHistogram())); - } else if (!AggregationUtil.aggregationName(explicitBucketHistogram()) - .equalsIgnoreCase(defaultHistogramAggregation)) { - throw new ConfigurationException( - "Unrecognized default histogram aggregation: " + defaultHistogramAggregation); - } + ExporterBuilderUtil.configureHistogramDefaultAggregation( + defaultHistogramAggregation, prometheusBuilder::setDefaultAggregationSelector); } return prometheusBuilder.build(); diff --git a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProvider.java b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProvider.java index c06dc204212..4449e08b2c7 100644 --- a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProvider.java +++ b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProvider.java @@ -5,19 +5,12 @@ package io.opentelemetry.exporter.prometheus.internal; -import static io.opentelemetry.sdk.metrics.Aggregation.explicitBucketHistogram; - import io.opentelemetry.exporter.internal.ExporterBuilderUtil; import io.opentelemetry.exporter.prometheus.PrometheusHttpServer; import io.opentelemetry.exporter.prometheus.PrometheusHttpServerBuilder; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; -import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException; import io.opentelemetry.sdk.autoconfigure.spi.internal.ConfigurableMetricReaderProvider; -import io.opentelemetry.sdk.metrics.Aggregation; -import io.opentelemetry.sdk.metrics.InstrumentType; -import io.opentelemetry.sdk.metrics.export.DefaultAggregationSelector; import io.opentelemetry.sdk.metrics.export.MetricReader; -import io.opentelemetry.sdk.metrics.internal.aggregator.AggregationUtil; /** * SPI implementation for {@link PrometheusHttpServer}. @@ -39,23 +32,14 @@ public MetricReader createMetricReader(ConfigProperties config) { if (host != null) { prometheusBuilder.setHost(host); } + ExporterBuilderUtil.configureExporterMemoryMode(config, prometheusBuilder::setMemoryMode); String defaultHistogramAggregation = - config.getString("otel.exporter.prometheus.metrics.default.histogram.aggregation"); + config.getString( + "otel.java.experimental.exporter.prometheus.metrics.default.histogram.aggregation"); if (defaultHistogramAggregation != null) { - if (AggregationUtil.aggregationName(Aggregation.base2ExponentialBucketHistogram()) - .equalsIgnoreCase(defaultHistogramAggregation)) { - prometheusBuilder.setDefaultAggregationSelector( - DefaultAggregationSelector.getDefault() - .with(InstrumentType.HISTOGRAM, Aggregation.base2ExponentialBucketHistogram())); - } else if (!AggregationUtil.aggregationName(explicitBucketHistogram()) - .equalsIgnoreCase(defaultHistogramAggregation)) { - throw new ConfigurationException( - "Unrecognized default histogram aggregation: " + defaultHistogramAggregation); - } + ExporterBuilderUtil.configureHistogramDefaultAggregation( + defaultHistogramAggregation, prometheusBuilder::setDefaultAggregationSelector); } - - ExporterBuilderUtil.configureExporterMemoryMode(config, prometheusBuilder::setMemoryMode); - return prometheusBuilder.build(); } diff --git a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java index 0502bcf6540..6fba0bc3234 100644 --- a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java +++ b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java @@ -83,7 +83,7 @@ void createMetricReader_WithConfiguration() throws IOException { config.put("otel.exporter.prometheus.port", String.valueOf(port)); config.put("otel.java.experimental.exporter.memory_mode", "reusable_data"); config.put( - "otel.exporter.prometheus.metrics.default.histogram.aggregation", + "otel.java.experimental.exporter.prometheus.metrics.default.histogram.aggregation", "BASE2_EXPONENTIAL_BUCKET_HISTOGRAM"); when(configProperties.getInt(any())).thenReturn(null); @@ -108,7 +108,8 @@ void createMetricReader_WithConfiguration() throws IOException { @Test void createMetricReader_WithWrongConfiguration() throws IOException { Map config = new HashMap<>(); - config.put("otel.exporter.prometheus.metrics.default.histogram.aggregation", "foo"); + config.put( + "otel.java.experimental.exporter.prometheus.metrics.default.histogram.aggregation", "foo"); when(configProperties.getInt(any())).thenReturn(null); when(configProperties.getString(any())).thenReturn(null); From a468f9767f038b0adb9f41da0c18e36b416bba0f Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 2 Jul 2024 18:01:14 +0530 Subject: [PATCH 4/8] minor refactoring --- .../internal/PrometheusComponentProvider.java | 1 + .../internal/PrometheusMetricReaderProvider.java | 3 +++ .../exporter/prometheus/PrometheusHttpServerTest.java | 10 +--------- .../internal/PrometheusMetricReaderProviderTest.java | 2 +- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusComponentProvider.java b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusComponentProvider.java index 26d6a5b6950..02d39046d90 100644 --- a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusComponentProvider.java +++ b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusComponentProvider.java @@ -42,6 +42,7 @@ public MetricReader create(StructuredConfigProperties config) { if (host != null) { prometheusBuilder.setHost(host); } + String defaultHistogramAggregation = config.getString("default_histogram_aggregation"); if (defaultHistogramAggregation != null) { ExporterBuilderUtil.configureHistogramDefaultAggregation( diff --git a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProvider.java b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProvider.java index 4449e08b2c7..4c60d3def19 100644 --- a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProvider.java +++ b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProvider.java @@ -32,7 +32,9 @@ public MetricReader createMetricReader(ConfigProperties config) { if (host != null) { prometheusBuilder.setHost(host); } + ExporterBuilderUtil.configureExporterMemoryMode(config, prometheusBuilder::setMemoryMode); + String defaultHistogramAggregation = config.getString( "otel.java.experimental.exporter.prometheus.metrics.default.histogram.aggregation"); @@ -40,6 +42,7 @@ public MetricReader createMetricReader(ConfigProperties config) { ExporterBuilderUtil.configureHistogramDefaultAggregation( defaultHistogramAggregation, prometheusBuilder::setDefaultAggregationSelector); } + return prometheusBuilder.build(); } diff --git a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerTest.java b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerTest.java index bc0122fd5f5..61a48ab2632 100644 --- a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerTest.java +++ b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerTest.java @@ -25,12 +25,9 @@ import io.opentelemetry.internal.testing.slf4j.SuppressLogger; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; import io.opentelemetry.sdk.common.export.MemoryMode; -import io.opentelemetry.sdk.metrics.Aggregation; -import io.opentelemetry.sdk.metrics.InstrumentType; import io.opentelemetry.sdk.metrics.data.AggregationTemporality; import io.opentelemetry.sdk.metrics.data.MetricData; import io.opentelemetry.sdk.metrics.export.CollectionRegistration; -import io.opentelemetry.sdk.metrics.export.DefaultAggregationSelector; import io.opentelemetry.sdk.metrics.internal.data.ImmutableDoublePointData; import io.opentelemetry.sdk.metrics.internal.data.ImmutableGaugeData; import io.opentelemetry.sdk.metrics.internal.data.ImmutableLongPointData; @@ -506,14 +503,10 @@ private static List generateTestData() { @Test void toBuilder() { - DefaultAggregationSelector defaultAggregationSelector = - DefaultAggregationSelector.getDefault() - .with(InstrumentType.HISTOGRAM, Aggregation.base2ExponentialBucketHistogram()); PrometheusHttpServerBuilder builder = PrometheusHttpServer.builder(); builder.setHost("localhost"); builder.setPort(1234); builder.setOtelScopeEnabled(false); - builder.setDefaultAggregationSelector(defaultAggregationSelector); Predicate resourceAttributesFilter = s -> false; builder.setAllowedResourceAttributesFilter(resourceAttributesFilter); @@ -534,7 +527,6 @@ void toBuilder() { .hasFieldOrPropertyWithValue("otelScopeEnabled", false) .hasFieldOrPropertyWithValue("allowedResourceAttributesFilter", resourceAttributesFilter) .hasFieldOrPropertyWithValue("executor", executor) - .hasFieldOrPropertyWithValue("prometheusRegistry", prometheusRegistry) - .hasFieldOrPropertyWithValue("defaultAggregationSelector", defaultAggregationSelector); + .hasFieldOrPropertyWithValue("prometheusRegistry", prometheusRegistry); } } diff --git a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java index 6fba0bc3234..a5614bf12d4 100644 --- a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java +++ b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java @@ -106,7 +106,7 @@ void createMetricReader_WithConfiguration() throws IOException { } @Test - void createMetricReader_WithWrongConfiguration() throws IOException { + void createMetricReader_WithWrongConfiguration() { Map config = new HashMap<>(); config.put( "otel.java.experimental.exporter.prometheus.metrics.default.histogram.aggregation", "foo"); From 4b9e688edcaee27777aea7673dd3768b82533cef Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 2 Jul 2024 22:37:30 +0530 Subject: [PATCH 5/8] removes config from component provider --- .../prometheus/internal/PrometheusComponentProvider.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusComponentProvider.java b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusComponentProvider.java index 02d39046d90..dc57d2bcbe3 100644 --- a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusComponentProvider.java +++ b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusComponentProvider.java @@ -5,7 +5,6 @@ package io.opentelemetry.exporter.prometheus.internal; -import io.opentelemetry.exporter.internal.ExporterBuilderUtil; import io.opentelemetry.exporter.prometheus.PrometheusHttpServer; import io.opentelemetry.exporter.prometheus.PrometheusHttpServerBuilder; import io.opentelemetry.sdk.autoconfigure.spi.internal.ComponentProvider; @@ -43,12 +42,6 @@ public MetricReader create(StructuredConfigProperties config) { prometheusBuilder.setHost(host); } - String defaultHistogramAggregation = config.getString("default_histogram_aggregation"); - if (defaultHistogramAggregation != null) { - ExporterBuilderUtil.configureHistogramDefaultAggregation( - defaultHistogramAggregation, prometheusBuilder::setDefaultAggregationSelector); - } - return prometheusBuilder.build(); } } From e0a28d67edbc4af88f6b9809d20b4529f8c5d20f Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 16 Jul 2024 19:14:55 +0530 Subject: [PATCH 6/8] adds integration test for base2ExponentialBucketHistogram --- .../prometheus/CollectorIntegrationTest.java | 52 ++++++++++++++++++- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/CollectorIntegrationTest.java b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/CollectorIntegrationTest.java index f58fdf722a2..f9896beecfe 100644 --- a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/CollectorIntegrationTest.java +++ b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/CollectorIntegrationTest.java @@ -17,18 +17,24 @@ import com.linecorp.armeria.server.grpc.protocol.AbstractUnaryGrpcService; import com.linecorp.armeria.testing.junit5.server.ServerExtension; import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceRequest; import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceResponse; import io.opentelemetry.proto.common.v1.AnyValue; import io.opentelemetry.proto.common.v1.KeyValue; import io.opentelemetry.proto.metrics.v1.AggregationTemporality; +import io.opentelemetry.proto.metrics.v1.Histogram; +import io.opentelemetry.proto.metrics.v1.HistogramDataPoint; import io.opentelemetry.proto.metrics.v1.Metric; import io.opentelemetry.proto.metrics.v1.NumberDataPoint; import io.opentelemetry.proto.metrics.v1.ResourceMetrics; import io.opentelemetry.proto.metrics.v1.ScopeMetrics; import io.opentelemetry.proto.metrics.v1.Sum; +import io.opentelemetry.sdk.metrics.Aggregation; +import io.opentelemetry.sdk.metrics.InstrumentType; import io.opentelemetry.sdk.metrics.SdkMeterProvider; +import io.opentelemetry.sdk.metrics.export.DefaultAggregationSelector; import io.opentelemetry.sdk.resources.Resource; import java.io.UncheckedIOException; import java.time.Duration; @@ -71,7 +77,14 @@ class CollectorIntegrationTest { @BeforeAll static void beforeAll() { - PrometheusHttpServer prometheusHttpServer = PrometheusHttpServer.builder().setPort(0).build(); + PrometheusHttpServer prometheusHttpServer = + PrometheusHttpServer.builder() + .setPort(0) + .setDefaultAggregationSelector( + DefaultAggregationSelector.getDefault() + .with(InstrumentType.HISTOGRAM, Aggregation.base2ExponentialBucketHistogram())) + .build(); + prometheusPort = prometheusHttpServer.getAddress().getPort(); resource = Resource.getDefault(); meterProvider = @@ -115,12 +128,16 @@ void afterEach() { @Test void endToEnd() { Meter meter = meterProvider.meterBuilder("test").setInstrumentationVersion("1.0.0").build(); - meter .counterBuilder("requests") .build() .add(3, Attributes.builder().put("animal", "bear").build()); + DoubleHistogram histogram = meter.histogramBuilder("latency").build(); + + histogram.record(3, Attributes.builder().put("animal", "bear").build()); + histogram.record(5, Attributes.builder().put("animal", "bear").build()); + await() .atMost(Duration.ofSeconds(30)) .untilAsserted(() -> assertThat(grpcServer.metricRequests.size()).isGreaterThan(0)); @@ -169,6 +186,11 @@ void endToEnd() { .orElseThrow(() -> new IllegalStateException("missing scope with name \"test\"")); assertThat(testScopeMetrics.getScope().getVersion()).isEqualTo("1.0.0"); + verifyCounterMetric(testScopeMetrics); + verifyHistogramMetric(testScopeMetrics); + } + + private static void verifyCounterMetric(ScopeMetrics testScopeMetrics) { Optional optRequestTotal = testScopeMetrics.getMetricsList().stream() .filter(metric -> metric.getName().equals("requests_total")) @@ -194,6 +216,32 @@ void endToEnd() { stringKeyValue("otel_scope_version", "1.0.0")); } + private static void verifyHistogramMetric(ScopeMetrics testScopeMetrics) { + Optional optRequestTotal = + testScopeMetrics.getMetricsList().stream() + .filter(metric -> metric.getName().equals("latency")) + .findFirst(); + assertThat(optRequestTotal).isPresent(); + Metric requestTotal = optRequestTotal.get(); + assertThat(requestTotal.getDataCase()).isEqualTo(Metric.DataCase.HISTOGRAM); + + Histogram requestHistogram = requestTotal.getHistogram(); + assertThat(requestHistogram.getAggregationTemporality()) + .isEqualTo(AggregationTemporality.AGGREGATION_TEMPORALITY_CUMULATIVE); + assertThat(requestHistogram.getDataPointsCount()).isEqualTo(1); + + HistogramDataPoint requestHistogramDataPoints = requestHistogram.getDataPoints(0); + assertThat(requestHistogramDataPoints.getCount()).isEqualTo(2); + assertThat(requestHistogramDataPoints.getBucketCountsCount()).isEqualTo(1); + assertThat(requestHistogramDataPoints.getAttributesList()) + .containsExactlyInAnyOrder( + stringKeyValue("animal", "bear"), + // Scope name and version are serialized as attributes to disambiguate metrics with the + // same name in different scopes + stringKeyValue("otel_scope_name", "test"), + stringKeyValue("otel_scope_version", "1.0.0")); + } + private static KeyValue stringKeyValue(String key, String value) { return KeyValue.newBuilder() .setKey(key) From 9cba997ea775f27ff5f296d48580850753fe9f76 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Wed, 24 Jul 2024 18:28:00 -0500 Subject: [PATCH 7/8] Test prometheus protobuf format for exponential histogram --- dependencyManagement/build.gradle.kts | 1 + exporters/prometheus/build.gradle.kts | 1 + .../prometheus/CollectorIntegrationTest.java | 52 +----------- .../prometheus/PrometheusHttpServerTest.java | 79 +++++++++++++++++++ .../PrometheusMetricReaderProviderTest.java | 3 - 5 files changed, 83 insertions(+), 53 deletions(-) diff --git a/dependencyManagement/build.gradle.kts b/dependencyManagement/build.gradle.kts index fd5ab235e77..e8cb9dc4dc9 100644 --- a/dependencyManagement/build.gradle.kts +++ b/dependencyManagement/build.gradle.kts @@ -50,6 +50,7 @@ val DEPENDENCIES = listOf( "org.mockito:mockito-junit-jupiter:${mockitoVersion}", "org.slf4j:slf4j-simple:${slf4jVersion}", "org.slf4j:jul-to-slf4j:${slf4jVersion}", + "io.prometheus:prometheus-metrics-shaded-protobuf:1.3.1", "io.prometheus:simpleclient:${prometheusClientVersion}", "io.prometheus:simpleclient_common:${prometheusClientVersion}", "io.prometheus:simpleclient_httpserver:${prometheusClientVersion}", diff --git a/exporters/prometheus/build.gradle.kts b/exporters/prometheus/build.gradle.kts index b0e9809ebcc..8fe244b3f3d 100644 --- a/exporters/prometheus/build.gradle.kts +++ b/exporters/prometheus/build.gradle.kts @@ -19,6 +19,7 @@ dependencies { testImplementation(project(":sdk:testing")) testImplementation("io.opentelemetry.proto:opentelemetry-proto") + testImplementation("io.prometheus:prometheus-metrics-shaded-protobuf") testImplementation("com.sun.net.httpserver:http") testImplementation("com.google.guava:guava") testImplementation("com.linecorp.armeria:armeria") diff --git a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/CollectorIntegrationTest.java b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/CollectorIntegrationTest.java index f9896beecfe..f58fdf722a2 100644 --- a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/CollectorIntegrationTest.java +++ b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/CollectorIntegrationTest.java @@ -17,24 +17,18 @@ import com.linecorp.armeria.server.grpc.protocol.AbstractUnaryGrpcService; import com.linecorp.armeria.testing.junit5.server.ServerExtension; import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceRequest; import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceResponse; import io.opentelemetry.proto.common.v1.AnyValue; import io.opentelemetry.proto.common.v1.KeyValue; import io.opentelemetry.proto.metrics.v1.AggregationTemporality; -import io.opentelemetry.proto.metrics.v1.Histogram; -import io.opentelemetry.proto.metrics.v1.HistogramDataPoint; import io.opentelemetry.proto.metrics.v1.Metric; import io.opentelemetry.proto.metrics.v1.NumberDataPoint; import io.opentelemetry.proto.metrics.v1.ResourceMetrics; import io.opentelemetry.proto.metrics.v1.ScopeMetrics; import io.opentelemetry.proto.metrics.v1.Sum; -import io.opentelemetry.sdk.metrics.Aggregation; -import io.opentelemetry.sdk.metrics.InstrumentType; import io.opentelemetry.sdk.metrics.SdkMeterProvider; -import io.opentelemetry.sdk.metrics.export.DefaultAggregationSelector; import io.opentelemetry.sdk.resources.Resource; import java.io.UncheckedIOException; import java.time.Duration; @@ -77,14 +71,7 @@ class CollectorIntegrationTest { @BeforeAll static void beforeAll() { - PrometheusHttpServer prometheusHttpServer = - PrometheusHttpServer.builder() - .setPort(0) - .setDefaultAggregationSelector( - DefaultAggregationSelector.getDefault() - .with(InstrumentType.HISTOGRAM, Aggregation.base2ExponentialBucketHistogram())) - .build(); - + PrometheusHttpServer prometheusHttpServer = PrometheusHttpServer.builder().setPort(0).build(); prometheusPort = prometheusHttpServer.getAddress().getPort(); resource = Resource.getDefault(); meterProvider = @@ -128,16 +115,12 @@ void afterEach() { @Test void endToEnd() { Meter meter = meterProvider.meterBuilder("test").setInstrumentationVersion("1.0.0").build(); + meter .counterBuilder("requests") .build() .add(3, Attributes.builder().put("animal", "bear").build()); - DoubleHistogram histogram = meter.histogramBuilder("latency").build(); - - histogram.record(3, Attributes.builder().put("animal", "bear").build()); - histogram.record(5, Attributes.builder().put("animal", "bear").build()); - await() .atMost(Duration.ofSeconds(30)) .untilAsserted(() -> assertThat(grpcServer.metricRequests.size()).isGreaterThan(0)); @@ -186,11 +169,6 @@ void endToEnd() { .orElseThrow(() -> new IllegalStateException("missing scope with name \"test\"")); assertThat(testScopeMetrics.getScope().getVersion()).isEqualTo("1.0.0"); - verifyCounterMetric(testScopeMetrics); - verifyHistogramMetric(testScopeMetrics); - } - - private static void verifyCounterMetric(ScopeMetrics testScopeMetrics) { Optional optRequestTotal = testScopeMetrics.getMetricsList().stream() .filter(metric -> metric.getName().equals("requests_total")) @@ -216,32 +194,6 @@ private static void verifyCounterMetric(ScopeMetrics testScopeMetrics) { stringKeyValue("otel_scope_version", "1.0.0")); } - private static void verifyHistogramMetric(ScopeMetrics testScopeMetrics) { - Optional optRequestTotal = - testScopeMetrics.getMetricsList().stream() - .filter(metric -> metric.getName().equals("latency")) - .findFirst(); - assertThat(optRequestTotal).isPresent(); - Metric requestTotal = optRequestTotal.get(); - assertThat(requestTotal.getDataCase()).isEqualTo(Metric.DataCase.HISTOGRAM); - - Histogram requestHistogram = requestTotal.getHistogram(); - assertThat(requestHistogram.getAggregationTemporality()) - .isEqualTo(AggregationTemporality.AGGREGATION_TEMPORALITY_CUMULATIVE); - assertThat(requestHistogram.getDataPointsCount()).isEqualTo(1); - - HistogramDataPoint requestHistogramDataPoints = requestHistogram.getDataPoints(0); - assertThat(requestHistogramDataPoints.getCount()).isEqualTo(2); - assertThat(requestHistogramDataPoints.getBucketCountsCount()).isEqualTo(1); - assertThat(requestHistogramDataPoints.getAttributesList()) - .containsExactlyInAnyOrder( - stringKeyValue("animal", "bear"), - // Scope name and version are serialized as attributes to disambiguate metrics with the - // same name in different scopes - stringKeyValue("otel_scope_name", "test"), - stringKeyValue("otel_scope_version", "1.0.0")); - } - private static KeyValue stringKeyValue(String key, String value) { return KeyValue.newBuilder() .setKey(key) diff --git a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerTest.java b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerTest.java index 61a48ab2632..1ee412eff03 100644 --- a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerTest.java +++ b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerTest.java @@ -16,18 +16,24 @@ import com.linecorp.armeria.client.retry.RetryRule; import com.linecorp.armeria.client.retry.RetryingClient; import com.linecorp.armeria.common.AggregatedHttpResponse; +import com.linecorp.armeria.common.HttpData; import com.linecorp.armeria.common.HttpHeaderNames; import com.linecorp.armeria.common.HttpMethod; import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.RequestHeaders; import io.github.netmikey.logunit.api.LogCapturer; import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.internal.testing.slf4j.SuppressLogger; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; import io.opentelemetry.sdk.common.export.MemoryMode; +import io.opentelemetry.sdk.metrics.Aggregation; +import io.opentelemetry.sdk.metrics.InstrumentType; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.metrics.data.AggregationTemporality; import io.opentelemetry.sdk.metrics.data.MetricData; import io.opentelemetry.sdk.metrics.export.CollectionRegistration; +import io.opentelemetry.sdk.metrics.export.DefaultAggregationSelector; import io.opentelemetry.sdk.metrics.internal.data.ImmutableDoublePointData; import io.opentelemetry.sdk.metrics.internal.data.ImmutableGaugeData; import io.opentelemetry.sdk.metrics.internal.data.ImmutableLongPointData; @@ -36,7 +42,9 @@ import io.opentelemetry.sdk.resources.Resource; import io.prometheus.metrics.exporter.httpserver.HTTPServer; import io.prometheus.metrics.exporter.httpserver.MetricsHandler; +import io.prometheus.metrics.expositionformats.generated.com_google_protobuf_3_25_3.Metrics; import io.prometheus.metrics.model.registry.PrometheusRegistry; +import io.prometheus.metrics.shaded.com_google_protobuf_3_25_3.TextFormat; import java.io.ByteArrayInputStream; import java.io.IOException; import java.net.ServerSocket; @@ -529,4 +537,75 @@ void toBuilder() { .hasFieldOrPropertyWithValue("executor", executor) .hasFieldOrPropertyWithValue("prometheusRegistry", prometheusRegistry); } + + /** + * Set the default histogram aggregation to be {@link + * Aggregation#base2ExponentialBucketHistogram()}. In order to validate that exponential + * histograms are produced, we request protobuf encoded metrics when scraping since the prometheus + * text format does not support native histograms. We parse the binary content protobuf payload to + * the protobuf java bindings, and assert against the string representation. + */ + @Test + void histogramDefaultBase2ExponentialHistogram() throws IOException { + PrometheusHttpServer prometheusServer = + PrometheusHttpServer.builder() + .setHost("localhost") + .setPort(0) + .setDefaultAggregationSelector( + DefaultAggregationSelector.getDefault() + .with(InstrumentType.HISTOGRAM, Aggregation.base2ExponentialBucketHistogram())) + .build(); + try (SdkMeterProvider meterProvider = + SdkMeterProvider.builder().registerMetricReader(prometheusServer).build()) { + DoubleHistogram histogram = meterProvider.get("meter").histogramBuilder("histogram").build(); + histogram.record(1.0); + + WebClient client = + WebClient.builder("http://localhost:" + prometheusServer.getAddress().getPort()) + .decorator(RetryingClient.newDecorator(RetryRule.failsafe())) + // Request protobuf binary encoding, which is required for the prometheus native + // histogram format + .addHeader( + "Accept", + "application/vnd.google.protobuf; proto=io.prometheus.client.MetricFamily") + .build(); + AggregatedHttpResponse response = client.get("/metrics").aggregate().join(); + assertThat(response.status()).isEqualTo(HttpStatus.OK); + assertThat(response.headers().get(HttpHeaderNames.CONTENT_TYPE)) + .isEqualTo( + "application/vnd.google.protobuf; proto=io.prometheus.client.MetricFamily; encoding=delimited"); + // Parse the data to Metrics.MetricFamily protobuf java binding and assert against the string + // representation + try (HttpData data = response.content()) { + Metrics.MetricFamily metricFamily = + Metrics.MetricFamily.parseDelimitedFrom(data.toInputStream()); + String s = TextFormat.printer().printToString(metricFamily); + assertThat(s) + .isEqualTo( + "name: \"histogram\"\n" + + "help: \"\"\n" + + "type: HISTOGRAM\n" + + "metric {\n" + + " label {\n" + + " name: \"otel_scope_name\"\n" + + " value: \"meter\"\n" + + " }\n" + + " histogram {\n" + + " sample_count: 1\n" + + " sample_sum: 1.0\n" + + " schema: 8\n" + + " zero_threshold: 0.0\n" + + " zero_count: 0\n" + + " positive_span {\n" + + " offset: 0\n" + + " length: 1\n" + + " }\n" + + " positive_delta: 1\n" + + " }\n" + + "}\n"); + } + } finally { + prometheusServer.shutdown(); + } + } } diff --git a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java index a5614bf12d4..e1fc42382e5 100644 --- a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java +++ b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java @@ -111,9 +111,6 @@ void createMetricReader_WithWrongConfiguration() { config.put( "otel.java.experimental.exporter.prometheus.metrics.default.histogram.aggregation", "foo"); - when(configProperties.getInt(any())).thenReturn(null); - when(configProperties.getString(any())).thenReturn(null); - assertThatThrownBy( () -> provider.createMetricReader(DefaultConfigProperties.createFromMap(config))) .isInstanceOf(ConfigurationException.class) From 199482f157b537f81285a8ac9f77f2e51d5cf0ea Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Thu, 8 Aug 2024 16:10:02 -0500 Subject: [PATCH 8/8] fix build --- .../internal/ExporterBuilderUtil.java | 32 +++++++++---------- .../otlp/internal/OtlpConfigUtil.java | 5 +++ 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/ExporterBuilderUtil.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/ExporterBuilderUtil.java index bf0d54a7f5f..81caace0178 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/ExporterBuilderUtil.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/ExporterBuilderUtil.java @@ -61,6 +61,22 @@ public static void configureExporterMemoryMode( memoryModeConsumer.accept(memoryMode); } + /** Invoke the {@code memoryModeConsumer} with the configured {@link MemoryMode}. */ + public static void configureExporterMemoryMode( + StructuredConfigProperties config, Consumer memoryModeConsumer) { + String memoryModeStr = config.getString("memory_mode"); + if (memoryModeStr == null) { + return; + } + MemoryMode memoryMode; + try { + memoryMode = MemoryMode.valueOf(memoryModeStr.toUpperCase(Locale.ROOT)); + } catch (IllegalArgumentException e) { + throw new ConfigurationException("Unrecognized memory_mode: " + memoryModeStr, e); + } + memoryModeConsumer.accept(memoryMode); + } + /** * Invoke the {@code defaultAggregationSelectorConsumer} with the configured {@link * DefaultAggregationSelector}. @@ -80,21 +96,5 @@ public static void configureHistogramDefaultAggregation( } } - /** Invoke the {@code memoryModeConsumer} with the configured {@link MemoryMode}. */ - public static void configureExporterMemoryMode( - StructuredConfigProperties config, Consumer memoryModeConsumer) { - String memoryModeStr = config.getString("memory_mode"); - if (memoryModeStr == null) { - return; - } - MemoryMode memoryMode; - try { - memoryMode = MemoryMode.valueOf(memoryModeStr.toUpperCase(Locale.ROOT)); - } catch (IllegalArgumentException e) { - throw new ConfigurationException("Unrecognized memory_mode: " + memoryModeStr, e); - } - memoryModeConsumer.accept(memoryMode); - } - private ExporterBuilderUtil() {} } diff --git a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java index 7649393dabf..7fbc5107fd1 100644 --- a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java +++ b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java @@ -5,15 +5,20 @@ package io.opentelemetry.exporter.otlp.internal; +import static io.opentelemetry.sdk.metrics.Aggregation.explicitBucketHistogram; + import io.opentelemetry.exporter.internal.ExporterBuilderUtil; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException; import io.opentelemetry.sdk.autoconfigure.spi.internal.StructuredConfigProperties; import io.opentelemetry.sdk.common.export.MemoryMode; import io.opentelemetry.sdk.common.export.RetryPolicy; +import io.opentelemetry.sdk.metrics.Aggregation; +import io.opentelemetry.sdk.metrics.InstrumentType; import io.opentelemetry.sdk.metrics.data.AggregationTemporality; import io.opentelemetry.sdk.metrics.export.AggregationTemporalitySelector; import io.opentelemetry.sdk.metrics.export.DefaultAggregationSelector; +import io.opentelemetry.sdk.metrics.internal.aggregator.AggregationUtil; import java.io.File; import java.io.IOException; import java.io.RandomAccessFile;