From b1ebd77c3a588338d3462aa8847af8a5a9018788 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Wed, 23 Aug 2023 12:23:34 -0500 Subject: [PATCH 1/2] Add ConfigurableMetricReaderProvider SPI --- .../PrometheusMetricReaderProvider.java | 41 ++++++ ....internal.ConfigurableMetricReaderProvider | 1 + .../PrometheusCustomizerProviderTest.java | 134 ------------------ .../PrometheusMetricReaderProviderTest.java | 89 ++++++++++++ .../ConfigurableMetricReaderProvider.java | 44 ++++++ .../MeterProviderConfiguration.java | 2 - .../MetricExporterConfiguration.java | 68 ++++++--- .../MetricExporterConfigurationTest.java | 46 +++--- .../ConfigurableMetricExporterTest.java | 30 ++-- .../MetricExporterConfigurationTest.java | 85 ++++++----- 10 files changed, 302 insertions(+), 238 deletions(-) create mode 100644 exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProvider.java create mode 100644 exporters/prometheus/src/main/resources/META-INF/services/io.opentelemetry.sdk.autoconfigure.spi.internal.ConfigurableMetricReaderProvider delete mode 100644 exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusCustomizerProviderTest.java create mode 100644 exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java create mode 100644 sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/internal/ConfigurableMetricReaderProvider.java 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 new file mode 100644 index 00000000000..062dd1e7ccd --- /dev/null +++ b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProvider.java @@ -0,0 +1,41 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.exporter.prometheus.internal; + +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.internal.ConfigurableMetricReaderProvider; +import io.opentelemetry.sdk.metrics.export.MetricReader; + +/** + * SPI implementation for {@link PrometheusHttpServer}. + * + *

This class is internal and is hence not for public use. Its APIs are unstable and can change + * at any time. + */ +public class PrometheusMetricReaderProvider implements ConfigurableMetricReaderProvider { + + @Override + public MetricReader createMetricReader(ConfigProperties config) { + PrometheusHttpServerBuilder prometheusBuilder = PrometheusHttpServer.builder(); + + Integer port = config.getInt("otel.exporter.prometheus.port"); + if (port != null) { + prometheusBuilder.setPort(port); + } + String host = config.getString("otel.exporter.prometheus.host"); + if (host != null) { + prometheusBuilder.setHost(host); + } + return prometheusBuilder.build(); + } + + @Override + public String getName() { + return "prometheus"; + } +} diff --git a/exporters/prometheus/src/main/resources/META-INF/services/io.opentelemetry.sdk.autoconfigure.spi.internal.ConfigurableMetricReaderProvider b/exporters/prometheus/src/main/resources/META-INF/services/io.opentelemetry.sdk.autoconfigure.spi.internal.ConfigurableMetricReaderProvider new file mode 100644 index 00000000000..0cb263a5c42 --- /dev/null +++ b/exporters/prometheus/src/main/resources/META-INF/services/io.opentelemetry.sdk.autoconfigure.spi.internal.ConfigurableMetricReaderProvider @@ -0,0 +1 @@ +io.opentelemetry.exporter.prometheus.internal.PrometheusMetricReaderProvider diff --git a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusCustomizerProviderTest.java b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusCustomizerProviderTest.java deleted file mode 100644 index 69aa70072a2..00000000000 --- a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusCustomizerProviderTest.java +++ /dev/null @@ -1,134 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.exporter.prometheus.internal; - -import static org.assertj.core.api.Assertions.as; -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.when; - -import com.sun.net.httpserver.HttpServer; -import io.opentelemetry.exporter.prometheus.PrometheusHttpServer; -import io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizer; -import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; -import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties; -import io.opentelemetry.sdk.metrics.SdkMeterProvider; -import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder; -import java.io.IOException; -import java.net.ServerSocket; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; -import java.util.function.BiFunction; -import org.assertj.core.api.InstanceOfAssertFactories; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; -import org.mockito.junit.jupiter.MockitoSettings; -import org.mockito.quality.Strictness; - -@ExtendWith(MockitoExtension.class) -@MockitoSettings(strictness = Strictness.LENIENT) -class PrometheusCustomizerProviderTest { - - private static final PrometheusCustomizerProvider provider = new PrometheusCustomizerProvider(); - - private SdkMeterProviderBuilder meterProviderBuilder; - - @Mock private ConfigProperties configProperties; - - @Mock private AutoConfigurationCustomizer customizer; - - @BeforeEach - void setup() { - meterProviderBuilder = SdkMeterProvider.builder(); - doAnswer( - invocation -> { - BiFunction - meterProviderCustomizer = invocation.getArgument(0); - meterProviderBuilder = - meterProviderCustomizer.apply(meterProviderBuilder, configProperties); - return null; - }) - .when(customizer) - .addMeterProviderCustomizer(any()); - } - - @Test - void customize_PrometheusEnabled() { - when(configProperties.getList("otel.metrics.exporter")) - .thenReturn(Collections.singletonList("prometheus")); - provider.customize(customizer); - - try (SdkMeterProvider meterProvider = meterProviderBuilder.build()) { - assertThat(meterProvider) - .extracting("registeredReaders", as(InstanceOfAssertFactories.list(Object.class))) - .satisfiesExactly( - registeredReader -> - assertThat(registeredReader) - .extracting("metricReader") - .isInstanceOf(PrometheusHttpServer.class)); - } - } - - @Test - void customize_PrometheusDisabled() { - when(configProperties.getList("otel.metrics.exporter")) - .thenReturn(Collections.singletonList("foo")); - provider.customize(customizer); - - try (SdkMeterProvider meterProvider = meterProviderBuilder.build()) { - assertThat(meterProvider) - .extracting("registeredReaders", as(InstanceOfAssertFactories.list(Object.class))) - .isEmpty(); - } - } - - @Test - void configurePrometheusHttpServer_Default() { - try (PrometheusHttpServer prometheusHttpServer = - PrometheusCustomizerProvider.configurePrometheusHttpServer( - DefaultConfigProperties.createForTest(Collections.emptyMap()))) { - assertThat(prometheusHttpServer) - .extracting("server", as(InstanceOfAssertFactories.type(HttpServer.class))) - .satisfies( - server -> { - assertThat(server.getAddress().getHostName()).isEqualTo("0:0:0:0:0:0:0:0"); - assertThat(server.getAddress().getPort()).isEqualTo(9464); - }); - } - } - - @Test - void configurePrometheusHttpServer_WithConfiguration() throws IOException { - // Find a random unused port. There's a small race if another process takes it before we - // initialize. Consider adding retries to this test if it flakes, presumably it never will on - // CI since there's no prometheus there blocking the well-known port. - int port; - try (ServerSocket socket2 = new ServerSocket(0)) { - port = socket2.getLocalPort(); - } - - Map config = new HashMap<>(); - config.put("otel.exporter.prometheus.host", "localhost"); - config.put("otel.exporter.prometheus.port", String.valueOf(port)); - - try (PrometheusHttpServer prometheusHttpServer = - PrometheusCustomizerProvider.configurePrometheusHttpServer( - DefaultConfigProperties.createForTest(config))) { - assertThat(prometheusHttpServer) - .extracting("server", as(InstanceOfAssertFactories.type(HttpServer.class))) - .satisfies( - server -> { - assertThat(server.getAddress().getHostName()).isEqualTo("localhost"); - assertThat(server.getAddress().getPort()).isEqualTo(port); - }); - } - } -} 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 new file mode 100644 index 00000000000..711c82e07dd --- /dev/null +++ b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java @@ -0,0 +1,89 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.exporter.prometheus.internal; + +import static org.assertj.core.api.Assertions.as; +import static org.assertj.core.api.Assertions.assertThat; +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.internal.DefaultConfigProperties; +import io.opentelemetry.sdk.metrics.export.MetricReader; +import java.io.IOException; +import java.net.ServerSocket; +import java.util.HashMap; +import java.util.Map; +import org.assertj.core.api.InstanceOfAssertFactories; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; + +@ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) +class PrometheusMetricReaderProviderTest { + + private static final PrometheusMetricReaderProvider provider = + new PrometheusMetricReaderProvider(); + + @Mock private ConfigProperties configProperties; + + @Test + void getName() { + assertThat(provider.getName()).isEqualTo("prometheus"); + } + + @Test + void createMetricReader_Default() throws IOException { + when(configProperties.getInt(any())).thenReturn(null); + when(configProperties.getString(any())).thenReturn(null); + + try (MetricReader metricReader = provider.createMetricReader(configProperties)) { + assertThat(metricReader) + .isInstanceOf(PrometheusHttpServer.class) + .extracting("server", as(InstanceOfAssertFactories.type(HttpServer.class))) + .satisfies( + server -> { + assertThat(server.getAddress().getHostName()).isEqualTo("0:0:0:0:0:0:0:0"); + assertThat(server.getAddress().getPort()).isEqualTo(9464); + }); + } + } + + @Test + void createMetricReader_WithConfiguration() throws IOException { + // Find a random unused port. There's a small race if another process takes it before we + // initialize. Consider adding retries to this test if it flakes, presumably it never will on + // CI since there's no prometheus there blocking the well-known port. + int port; + try (ServerSocket socket2 = new ServerSocket(0)) { + port = socket2.getLocalPort(); + } + + Map config = new HashMap<>(); + config.put("otel.exporter.prometheus.host", "localhost"); + config.put("otel.exporter.prometheus.port", String.valueOf(port)); + + when(configProperties.getInt(any())).thenReturn(null); + when(configProperties.getString(any())).thenReturn(null); + + try (MetricReader metricReader = + provider.createMetricReader(DefaultConfigProperties.createForTest(config))) { + assertThat(metricReader) + .extracting("server", as(InstanceOfAssertFactories.type(HttpServer.class))) + .satisfies( + server -> { + assertThat(server.getAddress().getHostName()).isEqualTo("localhost"); + assertThat(server.getAddress().getPort()).isEqualTo(port); + }); + } + } +} diff --git a/sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/internal/ConfigurableMetricReaderProvider.java b/sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/internal/ConfigurableMetricReaderProvider.java new file mode 100644 index 00000000000..bee17df680c --- /dev/null +++ b/sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/internal/ConfigurableMetricReaderProvider.java @@ -0,0 +1,44 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.sdk.autoconfigure.spi.internal; + +import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; +import io.opentelemetry.sdk.autoconfigure.spi.metrics.ConfigurableMetricExporterProvider; +import io.opentelemetry.sdk.metrics.export.MetricExporter; +import io.opentelemetry.sdk.metrics.export.MetricReader; +import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader; + +/** + * A service provider interface (SPI) for providing additional metric readers that can be used with + * the autoconfigured SDK. If the {@code otel.metrics.exporter} property contains a value equal to + * what is returned by {@link #getName()}, the exporter returned by {@link + * #createMetricReader(ConfigProperties)} will be enabled and added to the SDK. + * + *

Where as {@link ConfigurableMetricExporterProvider} provides push-based {@link + * MetricExporter}s to be paired with {@link PeriodicMetricReader}, this SPI facilitates pull-based + * {@link MetricReader}s. + * + *

This class is internal and is hence not for public use. Its APIs are unstable and can change + * at any time. + */ +public interface ConfigurableMetricReaderProvider { + + /** + * Returns a {@link MetricReader} that can be registered to OpenTelemetry by providing the + * property value specified by {@link #getName()}. + */ + MetricReader createMetricReader(ConfigProperties config); + + /** + * Returns the name of this exporter, which can be specified with the {@code + * otel.metrics.exporter} property to enable it. The name returned should NOT be the same as any + * other exporter name, either from other implementations of this SPI or {@link + * ConfigurableMetricExporterProvider}. If the name does conflict with another exporter name, the + * resulting behavior is undefined and it is explicitly unspecified which reader / exporter will + * actually be used. + */ + String getName(); +} diff --git a/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/MeterProviderConfiguration.java b/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/MeterProviderConfiguration.java index 74727b09444..525dc54974a 100644 --- a/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/MeterProviderConfiguration.java +++ b/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/MeterProviderConfiguration.java @@ -19,7 +19,6 @@ import java.util.Collections; import java.util.List; import java.util.Locale; -import java.util.Objects; import java.util.Set; import java.util.function.BiFunction; import java.util.stream.Collectors; @@ -87,7 +86,6 @@ static List configureMetricReaders( exporterName -> MetricExporterConfiguration.configureReader( exporterName, config, spiHelper, metricExporterCustomizer, closeables)) - .filter(Objects::nonNull) .collect(Collectors.toList()); } diff --git a/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/MetricExporterConfiguration.java b/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/MetricExporterConfiguration.java index 51d54bf949a..2aa968a82bb 100644 --- a/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/MetricExporterConfiguration.java +++ b/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/MetricExporterConfiguration.java @@ -9,6 +9,7 @@ import io.opentelemetry.sdk.autoconfigure.internal.SpiHelper; 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.autoconfigure.spi.metrics.ConfigurableMetricExporterProvider; import io.opentelemetry.sdk.metrics.export.MetricExporter; import io.opentelemetry.sdk.metrics.export.MetricReader; @@ -25,15 +26,18 @@ final class MetricExporterConfiguration { private static final Duration DEFAULT_EXPORT_INTERVAL = Duration.ofMinutes(1); private static final Map EXPORTER_ARTIFACT_ID_BY_NAME; + private static final Map READER_ARTIFACT_ID_BY_NAME; static { EXPORTER_ARTIFACT_ID_BY_NAME = new HashMap<>(); EXPORTER_ARTIFACT_ID_BY_NAME.put("logging", "opentelemetry-exporter-logging"); EXPORTER_ARTIFACT_ID_BY_NAME.put("logging-otlp", "opentelemetry-exporter-logging-otlp"); EXPORTER_ARTIFACT_ID_BY_NAME.put("otlp", "opentelemetry-exporter-otlp"); + + READER_ARTIFACT_ID_BY_NAME = new HashMap<>(); + READER_ARTIFACT_ID_BY_NAME.put("prometheus", "opentelemetry-exporter-prometheus"); } - @Nullable static MetricReader configureReader( String name, ConfigProperties config, @@ -41,30 +45,30 @@ static MetricReader configureReader( BiFunction metricExporterCustomizer, List closeables) { - if (name.equals("prometheus")) { - // PrometheusHttpServer is implemented as MetricReader (not MetricExporter) and uses - // the AutoConfigurationCustomizer#addMeterProviderCustomizer SPI hook instead of - // ConfigurableMetricExporterProvider. While the prometheus SPI hook is not handled here, - // the classpath check here provides uniform exception messages. - try { - Class.forName("io.opentelemetry.exporter.prometheus.PrometheusHttpServer"); - return null; - } catch (ClassNotFoundException unused) { - throw missingExporterException("prometheus", "opentelemetry-exporter-prometheus"); - } - } - NamedSpiManager spiExportersManager = metricExporterSpiManager(config, spiHelper); - MetricExporter metricExporter = configureExporter(name, spiExportersManager); + + // If no metric exporter with name, try to load metric reader + if (metricExporter == null) { + NamedSpiManager spiMetricReadersManager = + metricReadersSpiManager(config, spiHelper); + MetricReader metricReader = configureMetricReader(name, spiMetricReadersManager); + if (metricReader != null) { + closeables.add(metricReader); + return metricReader; + } + // No exporter or reader with the name + throw new ConfigurationException("Unrecognized value for otel.metrics.exporter: " + name); + } + + // Customize metric exporter and associate it with PeriodicMetricReader closeables.add(metricExporter); MetricExporter customizedMetricExporter = metricExporterCustomizer.apply(metricExporter, config); if (customizedMetricExporter != metricExporter) { closeables.add(customizedMetricExporter); } - MetricReader reader = PeriodicMetricReader.builder(customizedMetricExporter) .setInterval(config.getDuration("otel.metric.export.interval", DEFAULT_EXPORT_INTERVAL)) @@ -73,6 +77,31 @@ static MetricReader configureReader( return reader; } + // Visible for testing + static NamedSpiManager metricReadersSpiManager( + ConfigProperties config, SpiHelper spiHelper) { + return spiHelper.loadConfigurable( + ConfigurableMetricReaderProvider.class, + ConfigurableMetricReaderProvider::getName, + ConfigurableMetricReaderProvider::createMetricReader, + config); + } + + // Visible for testing. + @Nullable + static MetricReader configureMetricReader( + String name, NamedSpiManager spiMetricReadersManager) { + MetricReader metricReader = spiMetricReadersManager.getByName(name); + if (metricReader == null) { + String artifactId = READER_ARTIFACT_ID_BY_NAME.get(name); + if (artifactId != null) { + throw missingArtifactException(name, artifactId); + } + return null; + } + return metricReader; + } + // Visible for testing static NamedSpiManager metricExporterSpiManager( ConfigProperties config, SpiHelper spiHelper) { @@ -84,20 +113,21 @@ static NamedSpiManager metricExporterSpiManager( } // Visible for testing. + @Nullable static MetricExporter configureExporter( String name, NamedSpiManager spiExportersManager) { MetricExporter metricExporter = spiExportersManager.getByName(name); if (metricExporter == null) { String artifactId = EXPORTER_ARTIFACT_ID_BY_NAME.get(name); if (artifactId != null) { - throw missingExporterException(name, artifactId); + throw missingArtifactException(name, artifactId); } - throw new ConfigurationException("Unrecognized value for otel.metrics.exporter: " + name); + return null; } return metricExporter; } - private static ConfigurationException missingExporterException( + private static ConfigurationException missingArtifactException( String exporterName, String artifactId) { return new ConfigurationException( "otel.metrics.exporter set to \"" diff --git a/sdk-extensions/autoconfigure/src/test/java/io/opentelemetry/sdk/autoconfigure/MetricExporterConfigurationTest.java b/sdk-extensions/autoconfigure/src/test/java/io/opentelemetry/sdk/autoconfigure/MetricExporterConfigurationTest.java index 729b73e59a8..fc792de3b25 100644 --- a/sdk-extensions/autoconfigure/src/test/java/io/opentelemetry/sdk/autoconfigure/MetricExporterConfigurationTest.java +++ b/sdk-extensions/autoconfigure/src/test/java/io/opentelemetry/sdk/autoconfigure/MetricExporterConfigurationTest.java @@ -6,46 +6,34 @@ package io.opentelemetry.sdk.autoconfigure; import static io.opentelemetry.sdk.autoconfigure.MetricExporterConfiguration.configureExporter; -import static io.opentelemetry.sdk.autoconfigure.MetricExporterConfiguration.configureReader; +import static io.opentelemetry.sdk.autoconfigure.MetricExporterConfiguration.configureMetricReader; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import io.opentelemetry.internal.testing.CleanupExtension; import io.opentelemetry.sdk.autoconfigure.internal.NamedSpiManager; import io.opentelemetry.sdk.autoconfigure.internal.SpiHelper; 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.metrics.export.MetricExporter; -import java.io.Closeable; -import java.util.ArrayList; +import io.opentelemetry.sdk.metrics.export.MetricReader; import java.util.Collections; -import java.util.List; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.RegisterExtension; class MetricExporterConfigurationTest { private static final ConfigProperties EMPTY = DefaultConfigProperties.createForTest(Collections.emptyMap()); - @RegisterExtension CleanupExtension cleanup = new CleanupExtension(); - private final SpiHelper spiHelper = SpiHelper.create(MetricExporterConfigurationTest.class.getClassLoader()); @Test - void configureReader_PrometheusNotOnClasspath() { - List closeables = new ArrayList<>(); + void configureExporter_UnknownExporter() { + NamedSpiManager spiExportersManager = + MetricExporterConfiguration.metricExporterSpiManager(EMPTY, spiHelper); - assertThatThrownBy( - () -> configureReader("prometheus", EMPTY, spiHelper, (a, b) -> a, new ArrayList<>())) - .isInstanceOf(ConfigurationException.class) - .hasMessage( - "otel.metrics.exporter set to \"prometheus\" but opentelemetry-exporter-prometheus" - + " not found on classpath. Make sure to add it as a dependency."); - cleanup.addCloseables(closeables); - assertThat(closeables).isEmpty(); + assertThat(configureExporter("foo", spiExportersManager)).isNull(); } @Test @@ -68,9 +56,25 @@ void configureExporter_KnownSpiExportersNotOnClasspath() { .hasMessage( "otel.metrics.exporter set to \"otlp\" but opentelemetry-exporter-otlp" + " not found on classpath. Make sure to add it as a dependency."); + } + + @Test + void configureMetricReader_UnknownExporter() { + NamedSpiManager spiMetricReadersManager = + MetricExporterConfiguration.metricReadersSpiManager(EMPTY, spiHelper); + + assertThat(configureMetricReader("foo", spiMetricReadersManager)).isNull(); + } + + @Test + void configureMetricReader_KnownSpiExportersNotOnClasspath() { + NamedSpiManager spiMetricReadersManager = + MetricExporterConfiguration.metricReadersSpiManager(EMPTY, spiHelper); - // Unrecognized exporter - assertThatThrownBy(() -> configureExporter("foo", spiExportersManager)) - .hasMessage("Unrecognized value for otel.metrics.exporter: foo"); + assertThatThrownBy(() -> configureMetricReader("prometheus", spiMetricReadersManager)) + .isInstanceOf(ConfigurationException.class) + .hasMessage( + "otel.metrics.exporter set to \"prometheus\" but opentelemetry-exporter-prometheus" + + " not found on classpath. Make sure to add it as a dependency."); } } diff --git a/sdk-extensions/autoconfigure/src/testFullConfig/java/io/opentelemetry/sdk/autoconfigure/ConfigurableMetricExporterTest.java b/sdk-extensions/autoconfigure/src/testFullConfig/java/io/opentelemetry/sdk/autoconfigure/ConfigurableMetricExporterTest.java index 961172d2085..ac020a49a57 100644 --- a/sdk-extensions/autoconfigure/src/testFullConfig/java/io/opentelemetry/sdk/autoconfigure/ConfigurableMetricExporterTest.java +++ b/sdk-extensions/autoconfigure/src/testFullConfig/java/io/opentelemetry/sdk/autoconfigure/ConfigurableMetricExporterTest.java @@ -54,27 +54,23 @@ void configureExporter_spiExporter() { @Test void configureExporter_emptyClassLoader() { - assertThatThrownBy( - () -> - MetricExporterConfiguration.configureExporter( - "testExporter", - MetricExporterConfiguration.metricExporterSpiManager( - DefaultConfigProperties.createForTest(Collections.emptyMap()), - SpiHelper.create(new URLClassLoader(new URL[] {}, null))))) - .isInstanceOf(ConfigurationException.class) - .hasMessageContaining("testExporter"); + assertThat( + MetricExporterConfiguration.configureExporter( + "testExporter", + MetricExporterConfiguration.metricExporterSpiManager( + DefaultConfigProperties.createForTest(Collections.emptyMap()), + SpiHelper.create(new URLClassLoader(new URL[] {}, null))))) + .isNull(); } @Test void configureExporter_exporterNotFound() { - assertThatThrownBy( - () -> - MetricExporterConfiguration.configureExporter( - "catExporter", - MetricExporterConfiguration.metricExporterSpiManager( - DefaultConfigProperties.createForTest(Collections.emptyMap()), spiHelper))) - .isInstanceOf(ConfigurationException.class) - .hasMessageContaining("catExporter"); + assertThat( + MetricExporterConfiguration.configureExporter( + "catExporter", + MetricExporterConfiguration.metricExporterSpiManager( + DefaultConfigProperties.createForTest(Collections.emptyMap()), spiHelper))) + .isNull(); } @Test diff --git a/sdk-extensions/autoconfigure/src/testFullConfig/java/io/opentelemetry/sdk/autoconfigure/MetricExporterConfigurationTest.java b/sdk-extensions/autoconfigure/src/testFullConfig/java/io/opentelemetry/sdk/autoconfigure/MetricExporterConfigurationTest.java index 56f10dd0dd7..7d6e7b7a0a8 100644 --- a/sdk-extensions/autoconfigure/src/testFullConfig/java/io/opentelemetry/sdk/autoconfigure/MetricExporterConfigurationTest.java +++ b/sdk-extensions/autoconfigure/src/testFullConfig/java/io/opentelemetry/sdk/autoconfigure/MetricExporterConfigurationTest.java @@ -5,7 +5,6 @@ package io.opentelemetry.sdk.autoconfigure; -import static org.assertj.core.api.Assertions.as; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -15,25 +14,23 @@ import io.opentelemetry.exporter.otlp.metrics.OtlpGrpcMetricExporter; import io.opentelemetry.exporter.prometheus.PrometheusHttpServer; import io.opentelemetry.internal.testing.CleanupExtension; -import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.autoconfigure.internal.NamedSpiManager; import io.opentelemetry.sdk.autoconfigure.internal.SpiHelper; -import io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizerProvider; 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.autoconfigure.spi.metrics.ConfigurableMetricExporterProvider; import io.opentelemetry.sdk.metrics.export.MetricExporter; import io.opentelemetry.sdk.metrics.export.MetricReader; import java.io.Closeable; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.List; -import java.util.Map; -import org.assertj.core.api.InstanceOfAssertFactories; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; class MetricExporterConfigurationTest { @@ -54,49 +51,47 @@ void configureReader_PrometheusOnClasspath() { "prometheus", EMPTY, spiHelper, (a, b) -> a, closeables); cleanup.addCloseables(closeables); - assertThat(reader).isNull(); - assertThat(closeables).isEmpty(); + assertThat(reader).isInstanceOf(PrometheusHttpServer.class); + assertThat(closeables).hasSize(1); } - /** - * Prometheus uses the {@link AutoConfigurationCustomizerProvider} SPI instead of {@link - * ConfigurableMetricExporterProvider} because it is implemented as a {@link MetricReader}. While - * {@link MetricExporterConfiguration} does not load this SPI, the test code lives here alongside - * tests of the other known SPI metric exporters. - */ - @Test - void autoConfiguredOpenTelemetrySdk_PrometheusOnClasspath() { - Map config = new HashMap<>(); - config.put("otel.traces.exporter", "none"); - config.put("otel.metrics.exporter", "prometheus"); - config.put("otel.logs.exporter", "none"); - - try (OpenTelemetrySdk sdk = - AutoConfiguredOpenTelemetrySdk.builder() - .setConfig(DefaultConfigProperties.createForTest(config)) - .build() - .getOpenTelemetrySdk()) { - assertThat(sdk.getSdkMeterProvider()) - .extracting("registeredReaders", as(InstanceOfAssertFactories.list(Object.class))) - .satisfiesExactly( - registeredReader -> - assertThat(registeredReader) - .extracting("metricReader") - .isInstanceOf(PrometheusHttpServer.class)); - } - } - - @Test - void configureExporter_KnownSpiExportersOnClasspath() { + @ParameterizedTest + @MethodSource("knownExporters") + void configureExporter_KnownSpiExportersOnClasspath( + String exporterName, Class expectedExporter) { NamedSpiManager spiExportersManager = MetricExporterConfiguration.metricExporterSpiManager(EMPTY, spiHelper); - assertThat(MetricExporterConfiguration.configureExporter("logging", spiExportersManager)) - .isInstanceOf(LoggingMetricExporter.class); - assertThat(MetricExporterConfiguration.configureExporter("logging-otlp", spiExportersManager)) - .isInstanceOf(OtlpJsonLoggingMetricExporter.class); - assertThat(MetricExporterConfiguration.configureExporter("otlp", spiExportersManager)) - .isInstanceOf(OtlpGrpcMetricExporter.class); + MetricExporter metricExporter = + MetricExporterConfiguration.configureExporter(exporterName, spiExportersManager); + cleanup.addCloseable(metricExporter); + + assertThat(metricExporter).isInstanceOf(expectedExporter); + } + + private static Stream knownExporters() { + return Stream.of( + Arguments.of("logging", LoggingMetricExporter.class), + Arguments.of("logging-otlp", OtlpJsonLoggingMetricExporter.class), + Arguments.of("otlp", OtlpGrpcMetricExporter.class)); + } + + @ParameterizedTest + @MethodSource("knownReaders") + void configureMetricReader_KnownSpiExportersOnClasspath( + String exporterName, Class expectedExporter) { + NamedSpiManager spiMetricReadersManager = + MetricExporterConfiguration.metricReadersSpiManager(EMPTY, spiHelper); + + MetricReader metricReader = + MetricExporterConfiguration.configureMetricReader(exporterName, spiMetricReadersManager); + cleanup.addCloseable(metricReader); + + assertThat(metricReader).isInstanceOf(expectedExporter); + } + + private static Stream knownReaders() { + return Stream.of(Arguments.of("prometheus", PrometheusHttpServer.class)); } @Test From fd8c40041fd5916cf6c2aee8647f0dd7b5960cdf Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Thu, 24 Aug 2023 10:27:54 -0500 Subject: [PATCH 2/2] Improve javadoc --- .../internal/ConfigurableMetricReaderProvider.java | 12 ++++++------ .../metrics/ConfigurableMetricExporterProvider.java | 12 ++++++++++-- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/internal/ConfigurableMetricReaderProvider.java b/sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/internal/ConfigurableMetricReaderProvider.java index bee17df680c..1a9feb4acb8 100644 --- a/sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/internal/ConfigurableMetricReaderProvider.java +++ b/sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/internal/ConfigurableMetricReaderProvider.java @@ -33,12 +33,12 @@ public interface ConfigurableMetricReaderProvider { MetricReader createMetricReader(ConfigProperties config); /** - * Returns the name of this exporter, which can be specified with the {@code - * otel.metrics.exporter} property to enable it. The name returned should NOT be the same as any - * other exporter name, either from other implementations of this SPI or {@link - * ConfigurableMetricExporterProvider}. If the name does conflict with another exporter name, the - * resulting behavior is undefined and it is explicitly unspecified which reader / exporter will - * actually be used. + * Returns the name of this reader, which can be specified with the {@code otel.metrics.exporter} + * property to enable it. The name returned should NOT be the same as any other reader / exporter + * name, either from other implementations of this SPI or {@link + * ConfigurableMetricExporterProvider}. If the name does conflict with another reader / exporter + * name, the resulting behavior is undefined and it is explicitly unspecified which reader / + * exporter will actually be used. */ String getName(); } diff --git a/sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/metrics/ConfigurableMetricExporterProvider.java b/sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/metrics/ConfigurableMetricExporterProvider.java index 04a6571f834..465366c4e29 100644 --- a/sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/metrics/ConfigurableMetricExporterProvider.java +++ b/sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/metrics/ConfigurableMetricExporterProvider.java @@ -6,7 +6,9 @@ package io.opentelemetry.sdk.autoconfigure.spi.metrics; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; +import io.opentelemetry.sdk.autoconfigure.spi.internal.ConfigurableMetricReaderProvider; import io.opentelemetry.sdk.metrics.export.MetricExporter; +import io.opentelemetry.sdk.metrics.export.MetricReader; /** * A service provider interface (SPI) for providing additional exporters that can be used with the @@ -14,6 +16,10 @@ * is returned by {@link #getName()}, the exporter returned by {@link * #createExporter(ConfigProperties)} will be enabled and added to the SDK. * + *

The push-based {@link MetricExporter}s supplied by this SPI are paired with a {@link + * io.opentelemetry.sdk.metrics.export.PeriodicMetricReader}. See {@link + * ConfigurableMetricReaderProvider} for providing pull-based {@link MetricReader}s. + * * @since 1.15.0 */ public interface ConfigurableMetricExporterProvider { @@ -27,8 +33,10 @@ public interface ConfigurableMetricExporterProvider { /** * Returns the name of this exporter, which can be specified with the {@code * otel.metrics.exporter} property to enable it. The name returned should NOT be the same as any - * other exporter name. If the name does conflict with another exporter name, the resulting - * behavior is undefined and it is explicitly unspecified which exporter will actually be used. + * other exporter / reader name, either from other implementations of this SPI or {@link + * ConfigurableMetricReaderProvider}. If the name does conflict with another exporter / reader + * name, the resulting behavior is undefined and it is explicitly unspecified which exporter / + * reader will actually be used. */ String getName(); }