From ee61c7b22f9deda459579f318761f16446ef5139 Mon Sep 17 00:00:00 2001 From: Cyrille Le Clerc Date: Mon, 26 Aug 2024 15:48:04 +0200 Subject: [PATCH 01/11] Load OTelSDK config from environment variables and system properties. Fix #1416 --- .../maven/OpenTelemetrySdkService.java | 65 ++++++++++--- .../maven/OpenTelemetrySdkServiceTest.java | 96 ++++++++++++++----- 2 files changed, 126 insertions(+), 35 deletions(-) diff --git a/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java b/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java index af4fe9662..2d0aa1014 100644 --- a/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java +++ b/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java @@ -11,8 +11,12 @@ import io.opentelemetry.maven.semconv.MavenOtelSemanticAttributes; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; +import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; +import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties; import io.opentelemetry.sdk.common.CompletableResultCode; +import io.opentelemetry.sdk.resources.Resource; import java.io.Closeable; +import java.util.Collections; import java.util.HashMap; import java.util.Locale; import java.util.Map; @@ -36,6 +40,10 @@ public final class OpenTelemetrySdkService implements Closeable { private final OpenTelemetrySdk openTelemetrySdk; + private Resource resource; + + private ConfigProperties configProperties; + private final Tracer tracer; private final boolean mojosInstrumentationEnabled; @@ -47,26 +55,51 @@ public OpenTelemetrySdkService() { "OpenTelemetry: Initialize OpenTelemetrySdkService v{}...", MavenOtelSemanticAttributes.TELEMETRY_DISTRO_VERSION_VALUE); - // Change default of "otel.[traces,metrics,logs].exporter" from "otlp" to "none" - // The impacts are - // * If no otel exporter settings are passed, then the Maven extension will not export - // rather than exporting on OTLP GRPC to http://localhost:4317 - // * If OTEL_EXPORTER_OTLP_ENDPOINT is defined but OTEL_[TRACES,METRICS,LOGS]_EXPORTER, - // is not, then don't export - Map properties = new HashMap<>(); - properties.put("otel.traces.exporter", "none"); - properties.put("otel.metrics.exporter", "none"); - properties.put("otel.logs.exporter", "none"); - AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk = AutoConfiguredOpenTelemetrySdk.builder() .setServiceClassLoader(getClass().getClassLoader()) - .addPropertiesSupplier(() -> properties) + .addPropertiesCustomizer(configProperties -> { + // Change default of "otel.[traces,metrics,logs].exporter" from "otlp" to "none" + if (configProperties.getString("otel.exporter.otlp.endpoint") == null) { + Map properties = new HashMap<>(); + if (configProperties.getString("otel.exporter.otlp.traces.endpoint") == null) { + properties.put("otel.traces.exporter", "none"); + } + if (configProperties.getString("otel.exporter.otlp.metrics.endpoint") == null) { + properties.put("otel.metrics.exporter", "none"); + } + if (configProperties.getString("otel.exporter.otlp.logs.endpoint") == null) { + properties.put("otel.logs.exporter", "none"); + } + return properties; + } else { + return Collections.emptyMap(); + } + }) + .addPropertiesCustomizer(config -> { + // keep a reference to the computed config properties for future use in the extension + this.configProperties = config; + return Collections.emptyMap(); + }) + .addResourceCustomizer((res, configProperties) -> { + // keep a reference to the computed Resource for future use in the extension + this.resource = Resource.builder().putAll(res).build(); + return this.resource; + }) .disableShutdownHook() .build(); + if (this.resource == null) { + this.resource = Resource.empty(); + } + if (this.configProperties == null) { + this.configProperties = DefaultConfigProperties.createFromMap(Collections.emptyMap()); + } + this.openTelemetrySdk = autoConfiguredOpenTelemetrySdk.getOpenTelemetrySdk(); + logger.debug("OpenTelemetry: OpenTelemetrySdkService initialized, resource:{}", resource); + Boolean mojoSpansEnabled = getBooleanConfig("otel.instrumentation.maven.mojo.enabled"); this.mojosInstrumentationEnabled = mojoSpansEnabled == null || mojoSpansEnabled; @@ -97,6 +130,14 @@ public Tracer getTracer() { return this.tracer; } + public Resource getResource() { + return resource; + } + + public ConfigProperties getConfigProperties() { + return configProperties; + } + /** Returns the {@link ContextPropagators} for this {@link OpenTelemetry}. */ public ContextPropagators getPropagators() { return this.openTelemetrySdk.getPropagators(); diff --git a/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java b/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java index 0b7b41a31..9a9700813 100644 --- a/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java +++ b/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java @@ -5,42 +5,92 @@ package io.opentelemetry.maven; -import org.junit.jupiter.api.Disabled; +import static io.opentelemetry.api.common.AttributeKey.stringKey; +import static org.assertj.core.api.Assertions.assertThat; + +import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; +import io.opentelemetry.sdk.resources.Resource; import org.junit.jupiter.api.Test; public class OpenTelemetrySdkServiceTest { - /** Verify default `service.name` */ + /** Verify default config */ @Test - @Disabled public void testDefaultConfiguration() { - testConfiguration("maven"); + System.clearProperty("otel.exporter.otlp.endpoint"); + System.clearProperty("otel.service.name"); + System.clearProperty("otel.resource.attributes"); + try (OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService()) { + + Resource resource = openTelemetrySdkService.getResource(); + assertThat(resource.getAttribute(stringKey("service.name"))).isEqualTo("maven"); + + ConfigProperties configProperties = openTelemetrySdkService.getConfigProperties(); + assertThat(configProperties.getString("otel.exporter.otlp.endpoint")).isNull(); + assertThat(configProperties.getString("otel.traces.exporter")).isEqualTo("none"); + assertThat(configProperties.getString("otel.metrics.exporter")).isEqualTo("none"); + assertThat(configProperties.getString("otel.logs.exporter")).isEqualTo("none"); + } } - /** Verify overwritten `service.name` */ + /** Verify overwritten `service.name`,`key1` and `key2` */ @Test - @Disabled - public void testOverwrittenConfiguration() { + public void testOverwrittenResourceAttributes() { System.setProperty("otel.service.name", "my-maven"); - try { - testConfiguration("my-maven"); + System.setProperty("otel.resource.attributes", "key1=val1,key2=val2"); + + try (OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService()) { + + Resource resource = openTelemetrySdkService.getResource(); + assertThat(resource.getAttribute(stringKey("service.name"))).isEqualTo("my-maven"); + assertThat(resource.getAttribute(stringKey("key1"))).isEqualTo("val1"); + assertThat(resource.getAttribute(stringKey("key2"))).isEqualTo("val2"); + + } finally { + System.clearProperty("otel.service.name"); + System.clearProperty("otel.resource.attributes"); + } + } + + /** Verify overwritten `"otel.exporter.otlp.endpoint" */ + @Test + public void testOverwrittenExporterConfiguration_1() { + System.setProperty("otel.exporter.otlp.endpoint", "http://example.com:4318"); + + try (OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService()) { + + + ConfigProperties configProperties = openTelemetrySdkService.getConfigProperties(); + assertThat(configProperties.getString("otel.exporter.otlp.endpoint")).isEqualTo("http://example.com:4318"); + assertThat(configProperties.getString("otel.traces.exporter")).isNull(); + assertThat(configProperties.getString("otel.metrics.exporter")).isNull(); + assertThat(configProperties.getString("otel.logs.exporter")).isNull(); + } finally { - System.clearProperty("otel.service.name"); + System.clearProperty("otel.exporter.otlp.endpoint"); } } - void testConfiguration(String expectedServiceName) { - // OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService(); - // openTelemetrySdkService.initialize(); - // try { - // Resource resource = - // openTelemetrySdkService.autoConfiguredOpenTelemetrySdk.getResource(); - // assertThat(resource.getAttribute(ResourceAttributes.SERVICE_NAME)) - // .isEqualTo(expectedServiceName); - // } finally { - // openTelemetrySdkService.dispose(); - // GlobalOpenTelemetry.resetForTest(); - // GlobalEventEmitterProvider.resetForTest(); - // } + /** Verify overwritten `"otel.exporter.otlp.traces.endpoint" */ + @Test + public void testOverwrittenExporterConfiguration_2() { + System.clearProperty("otel.exporter.otlp.endpoint"); + System.setProperty("otel.exporter.otlp.traces.endpoint", "http://example.com:4318/v1/traces"); + System.setProperty("otel.exporter.otlp.traces.protocol", "http/protobuf"); + + try (OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService()) { + + ConfigProperties configProperties = openTelemetrySdkService.getConfigProperties(); + assertThat(configProperties.getString("otel.exporter.otlp.endpoint")).isNull(); + assertThat(configProperties.getString("otel.exporter.otlp.traces.endpoint")).isEqualTo("http://example.com:4318/v1/traces"); + assertThat(configProperties.getString("otel.traces.exporter")).isNull(); + assertThat(configProperties.getString("otel.metrics.exporter")).isEqualTo("none"); + assertThat(configProperties.getString("otel.logs.exporter")).isEqualTo("none"); + + } finally { + System.clearProperty("otel.exporter.otlp.endpoint"); + System.clearProperty("otel.exporter.otlp.traces.endpoint"); + System.clearProperty("otel.exporter.otlp.traces.protocol"); + } } } From ecb469b07e8e1ca1766b854c4362fa579ee12703 Mon Sep 17 00:00:00 2001 From: Cyrille Le Clerc Date: Tue, 27 Aug 2024 09:55:16 +0200 Subject: [PATCH 02/11] Load OTel SDK config from environment variables and system properties. Fix #1416 --- .../maven/OpenTelemetrySdkService.java | 19 +++++-- .../maven/OpenTelemetrySdkServiceTest.java | 56 ++++++++++++++----- 2 files changed, 56 insertions(+), 19 deletions(-) diff --git a/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java b/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java index 2d0aa1014..81d088a6c 100644 --- a/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java +++ b/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java @@ -20,6 +20,7 @@ import java.util.HashMap; import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; import javax.annotation.PreDestroy; @@ -59,16 +60,26 @@ public OpenTelemetrySdkService() { AutoConfiguredOpenTelemetrySdk.builder() .setServiceClassLoader(getClass().getClassLoader()) .addPropertiesCustomizer(configProperties -> { - // Change default of "otel.[traces,metrics,logs].exporter" from "otlp" to "none" + // The OTel SDK by default sends data to the OTLP gRPC endpoint at localhost:4317. + // Change this behavior to disable by default the OTel SDK in the Maven extension so + // that it must be explicitly enabled by the user. + // To change this default behavior, we set "otel.[traces,metrics,logs].exporter" to + // "none" if the endpoint has not been specified if (configProperties.getString("otel.exporter.otlp.endpoint") == null) { Map properties = new HashMap<>(); - if (configProperties.getString("otel.exporter.otlp.traces.endpoint") == null) { + if (Objects.equals("otlp", + configProperties.getString("otel.traces.exporter", "otlp")) + && configProperties.getString("otel.exporter.otlp.traces.endpoint") == null) { properties.put("otel.traces.exporter", "none"); } - if (configProperties.getString("otel.exporter.otlp.metrics.endpoint") == null) { + if (Objects.equals("otlp", + configProperties.getString("otel.metrics.exporter", "otlp")) + && configProperties.getString("otel.exporter.otlp.metrics.endpoint") == null) { properties.put("otel.metrics.exporter", "none"); } - if (configProperties.getString("otel.exporter.otlp.logs.endpoint") == null) { + if (Objects.equals("otlp", + configProperties.getString("otel.logs.exporter", "otlp")) + && configProperties.getString("otel.exporter.otlp.logs.endpoint") == null) { properties.put("otel.logs.exporter", "none"); } return properties; diff --git a/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java b/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java index 9a9700813..1d3fb9a1f 100644 --- a/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java +++ b/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java @@ -39,29 +39,29 @@ public void testOverwrittenResourceAttributes() { System.setProperty("otel.service.name", "my-maven"); System.setProperty("otel.resource.attributes", "key1=val1,key2=val2"); - try (OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService()) { + try (OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService()) { - Resource resource = openTelemetrySdkService.getResource(); - assertThat(resource.getAttribute(stringKey("service.name"))).isEqualTo("my-maven"); - assertThat(resource.getAttribute(stringKey("key1"))).isEqualTo("val1"); - assertThat(resource.getAttribute(stringKey("key2"))).isEqualTo("val2"); + Resource resource = openTelemetrySdkService.getResource(); + assertThat(resource.getAttribute(stringKey("service.name"))).isEqualTo("my-maven"); + assertThat(resource.getAttribute(stringKey("key1"))).isEqualTo("val1"); + assertThat(resource.getAttribute(stringKey("key2"))).isEqualTo("val2"); } finally { - System.clearProperty("otel.service.name"); - System.clearProperty("otel.resource.attributes"); + System.clearProperty("otel.service.name"); + System.clearProperty("otel.resource.attributes"); } } - /** Verify overwritten `"otel.exporter.otlp.endpoint" */ + /** Verify defining `otel.exporter.otlp.endpoint` works */ @Test public void testOverwrittenExporterConfiguration_1() { - System.setProperty("otel.exporter.otlp.endpoint", "http://example.com:4318"); + System.setProperty("otel.exporter.otlp.endpoint", "http://example.com:4317"); try (OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService()) { - ConfigProperties configProperties = openTelemetrySdkService.getConfigProperties(); - assertThat(configProperties.getString("otel.exporter.otlp.endpoint")).isEqualTo("http://example.com:4318"); + assertThat(configProperties.getString("otel.exporter.otlp.endpoint")).isEqualTo( + "http://example.com:4317"); assertThat(configProperties.getString("otel.traces.exporter")).isNull(); assertThat(configProperties.getString("otel.metrics.exporter")).isNull(); assertThat(configProperties.getString("otel.logs.exporter")).isNull(); @@ -71,18 +71,44 @@ public void testOverwrittenExporterConfiguration_1() { } } - /** Verify overwritten `"otel.exporter.otlp.traces.endpoint" */ + /** Verify defining `otel.exporter.otlp.traces.endpoint` works */ @Test public void testOverwrittenExporterConfiguration_2() { System.clearProperty("otel.exporter.otlp.endpoint"); - System.setProperty("otel.exporter.otlp.traces.endpoint", "http://example.com:4318/v1/traces"); - System.setProperty("otel.exporter.otlp.traces.protocol", "http/protobuf"); + System.clearProperty("otel.traces.exporter"); + System.setProperty("otel.exporter.otlp.traces.endpoint", "http://example.com:4317/"); + + try (OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService()) { + + ConfigProperties configProperties = openTelemetrySdkService.getConfigProperties(); + assertThat(configProperties.getString("otel.exporter.otlp.endpoint")).isNull(); + assertThat(configProperties.getString("otel.exporter.otlp.traces.endpoint")).isEqualTo( + "http://example.com:4317/"); + assertThat(configProperties.getString("otel.traces.exporter")).isNull(); + assertThat(configProperties.getString("otel.metrics.exporter")).isEqualTo("none"); + assertThat(configProperties.getString("otel.logs.exporter")).isEqualTo("none"); + + } finally { + System.clearProperty("otel.exporter.otlp.endpoint"); + System.clearProperty("otel.traces.exporter"); + System.clearProperty("otel.exporter.otlp.traces.endpoint"); + } + } + + /** Verify defining `otel.exporter.otlp.traces.endpoint` and `otel.traces.exporter` works */ + @Test + public void testOverwrittenExporterConfiguration_3() { + System.clearProperty("otel.exporter.otlp.endpoint"); + System.setProperty("otel.traces.exporter", "otlp"); + System.setProperty("otel.exporter.otlp.traces.endpoint", "http://example.com:4317/"); try (OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService()) { ConfigProperties configProperties = openTelemetrySdkService.getConfigProperties(); assertThat(configProperties.getString("otel.exporter.otlp.endpoint")).isNull(); - assertThat(configProperties.getString("otel.exporter.otlp.traces.endpoint")).isEqualTo("http://example.com:4318/v1/traces"); + assertThat( + configProperties.getString("otel.exporter.otlp.traces.endpoint")).isEqualTo( + "http://example.com:4317/"); assertThat(configProperties.getString("otel.traces.exporter")).isNull(); assertThat(configProperties.getString("otel.metrics.exporter")).isEqualTo("none"); assertThat(configProperties.getString("otel.logs.exporter")).isEqualTo("none"); From a89bd63836b42d5ac3ea9b4b9c095b220585b57b Mon Sep 17 00:00:00 2001 From: Cyrille Le Clerc Date: Sun, 15 Sep 2024 22:56:43 +0200 Subject: [PATCH 03/11] Fix code formatting --- .../maven/OpenTelemetrySdkService.java | 84 ++++++++++--------- .../maven/OpenTelemetrySdkServiceTest.java | 13 ++- 2 files changed, 52 insertions(+), 45 deletions(-) diff --git a/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java b/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java index 81d088a6c..61c168c8d 100644 --- a/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java +++ b/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java @@ -59,44 +59,52 @@ public OpenTelemetrySdkService() { AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk = AutoConfiguredOpenTelemetrySdk.builder() .setServiceClassLoader(getClass().getClassLoader()) - .addPropertiesCustomizer(configProperties -> { - // The OTel SDK by default sends data to the OTLP gRPC endpoint at localhost:4317. - // Change this behavior to disable by default the OTel SDK in the Maven extension so - // that it must be explicitly enabled by the user. - // To change this default behavior, we set "otel.[traces,metrics,logs].exporter" to - // "none" if the endpoint has not been specified - if (configProperties.getString("otel.exporter.otlp.endpoint") == null) { - Map properties = new HashMap<>(); - if (Objects.equals("otlp", - configProperties.getString("otel.traces.exporter", "otlp")) - && configProperties.getString("otel.exporter.otlp.traces.endpoint") == null) { - properties.put("otel.traces.exporter", "none"); - } - if (Objects.equals("otlp", - configProperties.getString("otel.metrics.exporter", "otlp")) - && configProperties.getString("otel.exporter.otlp.metrics.endpoint") == null) { - properties.put("otel.metrics.exporter", "none"); - } - if (Objects.equals("otlp", - configProperties.getString("otel.logs.exporter", "otlp")) - && configProperties.getString("otel.exporter.otlp.logs.endpoint") == null) { - properties.put("otel.logs.exporter", "none"); - } - return properties; - } else { - return Collections.emptyMap(); - } - }) - .addPropertiesCustomizer(config -> { - // keep a reference to the computed config properties for future use in the extension - this.configProperties = config; - return Collections.emptyMap(); - }) - .addResourceCustomizer((res, configProperties) -> { - // keep a reference to the computed Resource for future use in the extension - this.resource = Resource.builder().putAll(res).build(); - return this.resource; - }) + .addPropertiesCustomizer( + configProperties -> { + // The OTel SDK by default sends data to the OTLP gRPC endpoint at localhost:4317. + // Change this behavior to disable by default the OTel SDK in the Maven extension + // so + // that it must be explicitly enabled by the user. + // To change this default behavior, we set "otel.[traces,metrics,logs].exporter" + // to + // "none" if the endpoint has not been specified + if (configProperties.getString("otel.exporter.otlp.endpoint") == null) { + Map properties = new HashMap<>(); + if (Objects.equals( + "otlp", configProperties.getString("otel.traces.exporter", "otlp")) + && configProperties.getString("otel.exporter.otlp.traces.endpoint") + == null) { + properties.put("otel.traces.exporter", "none"); + } + if (Objects.equals( + "otlp", configProperties.getString("otel.metrics.exporter", "otlp")) + && configProperties.getString("otel.exporter.otlp.metrics.endpoint") + == null) { + properties.put("otel.metrics.exporter", "none"); + } + if (Objects.equals( + "otlp", configProperties.getString("otel.logs.exporter", "otlp")) + && configProperties.getString("otel.exporter.otlp.logs.endpoint") == null) { + properties.put("otel.logs.exporter", "none"); + } + return properties; + } else { + return Collections.emptyMap(); + } + }) + .addPropertiesCustomizer( + config -> { + // keep a reference to the computed config properties for future use in the + // extension + this.configProperties = config; + return Collections.emptyMap(); + }) + .addResourceCustomizer( + (res, configProperties) -> { + // keep a reference to the computed Resource for future use in the extension + this.resource = Resource.builder().putAll(res).build(); + return this.resource; + }) .disableShutdownHook() .build(); diff --git a/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java b/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java index 1d3fb9a1f..f948bd84c 100644 --- a/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java +++ b/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java @@ -60,8 +60,8 @@ public void testOverwrittenExporterConfiguration_1() { try (OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService()) { ConfigProperties configProperties = openTelemetrySdkService.getConfigProperties(); - assertThat(configProperties.getString("otel.exporter.otlp.endpoint")).isEqualTo( - "http://example.com:4317"); + assertThat(configProperties.getString("otel.exporter.otlp.endpoint")) + .isEqualTo("http://example.com:4317"); assertThat(configProperties.getString("otel.traces.exporter")).isNull(); assertThat(configProperties.getString("otel.metrics.exporter")).isNull(); assertThat(configProperties.getString("otel.logs.exporter")).isNull(); @@ -82,8 +82,8 @@ public void testOverwrittenExporterConfiguration_2() { ConfigProperties configProperties = openTelemetrySdkService.getConfigProperties(); assertThat(configProperties.getString("otel.exporter.otlp.endpoint")).isNull(); - assertThat(configProperties.getString("otel.exporter.otlp.traces.endpoint")).isEqualTo( - "http://example.com:4317/"); + assertThat(configProperties.getString("otel.exporter.otlp.traces.endpoint")) + .isEqualTo("http://example.com:4317/"); assertThat(configProperties.getString("otel.traces.exporter")).isNull(); assertThat(configProperties.getString("otel.metrics.exporter")).isEqualTo("none"); assertThat(configProperties.getString("otel.logs.exporter")).isEqualTo("none"); @@ -106,9 +106,8 @@ public void testOverwrittenExporterConfiguration_3() { ConfigProperties configProperties = openTelemetrySdkService.getConfigProperties(); assertThat(configProperties.getString("otel.exporter.otlp.endpoint")).isNull(); - assertThat( - configProperties.getString("otel.exporter.otlp.traces.endpoint")).isEqualTo( - "http://example.com:4317/"); + assertThat(configProperties.getString("otel.exporter.otlp.traces.endpoint")) + .isEqualTo("http://example.com:4317/"); assertThat(configProperties.getString("otel.traces.exporter")).isNull(); assertThat(configProperties.getString("otel.metrics.exporter")).isEqualTo("none"); assertThat(configProperties.getString("otel.logs.exporter")).isEqualTo("none"); From cc3490cfa48bd611bd17e002545859f6a03ecf93 Mon Sep 17 00:00:00 2001 From: Cyrille Le Clerc Date: Tue, 17 Sep 2024 10:47:47 +0200 Subject: [PATCH 04/11] Fix unit test --- .../io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java b/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java index f948bd84c..785614042 100644 --- a/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java +++ b/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java @@ -108,7 +108,7 @@ public void testOverwrittenExporterConfiguration_3() { assertThat(configProperties.getString("otel.exporter.otlp.endpoint")).isNull(); assertThat(configProperties.getString("otel.exporter.otlp.traces.endpoint")) .isEqualTo("http://example.com:4317/"); - assertThat(configProperties.getString("otel.traces.exporter")).isNull(); + assertThat(configProperties.getString("otel.traces.exporter")).isEqualTo("otlp"); assertThat(configProperties.getString("otel.metrics.exporter")).isEqualTo("none"); assertThat(configProperties.getString("otel.logs.exporter")).isEqualTo("none"); From 139c09648fc423a577be7ddf49da923a6b30f369 Mon Sep 17 00:00:00 2001 From: Cyrille Le Clerc Date: Fri, 18 Oct 2024 12:48:07 +0200 Subject: [PATCH 05/11] WIP --- .../io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java b/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java index 785614042..f031cf912 100644 --- a/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java +++ b/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java @@ -12,6 +12,10 @@ import io.opentelemetry.sdk.resources.Resource; import org.junit.jupiter.api.Test; +/** + * TODO: once otel-java-contrib requires Java 11+, use junit-pioneer's {@code @SetSystemProperty} + * and {@code @ClearSystemProperty} + */ public class OpenTelemetrySdkServiceTest { /** Verify default config */ From 6846a02ddc9d3ef5a24269c11ee99c06431ab0bf Mon Sep 17 00:00:00 2001 From: Cyrille Le Clerc Date: Mon, 28 Oct 2024 11:07:24 +0530 Subject: [PATCH 06/11] WIP --- .../maven/OpenTelemetrySdkService.java | 84 +++++++++++-------- .../maven/OpenTelemetrySdkServiceTest.java | 16 ++-- 2 files changed, 59 insertions(+), 41 deletions(-) diff --git a/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java b/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java index 61c168c8d..1d2fc5789 100644 --- a/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java +++ b/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java @@ -20,7 +20,6 @@ import java.util.HashMap; import java.util.Locale; import java.util.Map; -import java.util.Objects; import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; import javax.annotation.PreDestroy; @@ -60,38 +59,7 @@ public OpenTelemetrySdkService() { AutoConfiguredOpenTelemetrySdk.builder() .setServiceClassLoader(getClass().getClassLoader()) .addPropertiesCustomizer( - configProperties -> { - // The OTel SDK by default sends data to the OTLP gRPC endpoint at localhost:4317. - // Change this behavior to disable by default the OTel SDK in the Maven extension - // so - // that it must be explicitly enabled by the user. - // To change this default behavior, we set "otel.[traces,metrics,logs].exporter" - // to - // "none" if the endpoint has not been specified - if (configProperties.getString("otel.exporter.otlp.endpoint") == null) { - Map properties = new HashMap<>(); - if (Objects.equals( - "otlp", configProperties.getString("otel.traces.exporter", "otlp")) - && configProperties.getString("otel.exporter.otlp.traces.endpoint") - == null) { - properties.put("otel.traces.exporter", "none"); - } - if (Objects.equals( - "otlp", configProperties.getString("otel.metrics.exporter", "otlp")) - && configProperties.getString("otel.exporter.otlp.metrics.endpoint") - == null) { - properties.put("otel.metrics.exporter", "none"); - } - if (Objects.equals( - "otlp", configProperties.getString("otel.logs.exporter", "otlp")) - && configProperties.getString("otel.exporter.otlp.logs.endpoint") == null) { - properties.put("otel.logs.exporter", "none"); - } - return properties; - } else { - return Collections.emptyMap(); - } - }) + OpenTelemetrySdkService::requireExplicitConfigOfTheOtlpExporter) .addPropertiesCustomizer( config -> { // keep a reference to the computed config properties for future use in the @@ -119,12 +87,62 @@ public OpenTelemetrySdkService() { logger.debug("OpenTelemetry: OpenTelemetrySdkService initialized, resource:{}", resource); + // TODO should we replace `getBooleanConfig(name)` by `configProperties.getBoolean(name)`? Boolean mojoSpansEnabled = getBooleanConfig("otel.instrumentation.maven.mojo.enabled"); this.mojosInstrumentationEnabled = mojoSpansEnabled == null || mojoSpansEnabled; this.tracer = openTelemetrySdk.getTracer("io.opentelemetry.contrib.maven", VERSION); } + /** + * The OTel SDK by default sends data to the OTLP gRPC endpoint localhost:4317 if no exporter and + * no OTLP exporter endpoint are defined. This is not suited for a build tool for which we want + * the OTel SDK to be disabled by default. + * + *

Change the OTel SDL behavior: if none of the exporter and the OTLP exporter endpoint are + * defined, explicitly disable the exporter setting "{@code + * otel.[traces,metrics,logs].exporter=none}" + * + * @return The properties to be returned by {@link + * io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdkBuilder#addPropertiesCustomizer(java.util.function.Function)} + */ + static Map requireExplicitConfigOfTheOtlpExporter( + ConfigProperties configProperties) { + + Map properties = new HashMap<>(); + if (configProperties.getString("otel.exporter.otlp.endpoint") == null) { + for (SignalType signalType : SignalType.values()) { + boolean isExporterImplicitlyConfiguredToOtlp = + configProperties.getString("otel." + signalType.value + ".exporter") == null; + boolean isOtlpExporterEndpointSpecified = + configProperties.getString("otel.exporter.otlp." + signalType.value + ".endpoint") + != null; + + if (isExporterImplicitlyConfiguredToOtlp && !isOtlpExporterEndpointSpecified) { + logger.debug( + "OpenTelemetry: Disabling default OTLP exporter endpoint for signal {} exporter", + signalType.value); + properties.put("otel." + signalType.value + ".exporter", "none"); + } + } + } else { + logger.debug("OpenTelemetry: OTLP exporter endpoint is explicitly configured"); + } + return properties; + } + + enum SignalType { + TRACES("traces"), + METRICS("metrics"), + LOGS("logs"); + + private final String value; + + SignalType(String value) { + this.value = value; + } + } + @PreDestroy @Override public synchronized void close() { diff --git a/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java b/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java index f031cf912..8bea02578 100644 --- a/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java +++ b/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java @@ -13,8 +13,8 @@ import org.junit.jupiter.api.Test; /** - * TODO: once otel-java-contrib requires Java 11+, use junit-pioneer's {@code @SetSystemProperty} - * and {@code @ClearSystemProperty} + * Note: if otel-java-contrib bumps to Java 11+, we could use junit-pioneer's + * {@code @SetSystemProperty} and {@code @ClearSystemProperty} but no bump is planned for now. */ public class OpenTelemetrySdkServiceTest { @@ -59,13 +59,13 @@ public void testOverwrittenResourceAttributes() { /** Verify defining `otel.exporter.otlp.endpoint` works */ @Test public void testOverwrittenExporterConfiguration_1() { - System.setProperty("otel.exporter.otlp.endpoint", "http://example.com:4317"); + System.setProperty("otel.exporter.otlp.endpoint", "https://example.com:4317"); try (OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService()) { ConfigProperties configProperties = openTelemetrySdkService.getConfigProperties(); assertThat(configProperties.getString("otel.exporter.otlp.endpoint")) - .isEqualTo("http://example.com:4317"); + .isEqualTo("https://example.com:4317"); assertThat(configProperties.getString("otel.traces.exporter")).isNull(); assertThat(configProperties.getString("otel.metrics.exporter")).isNull(); assertThat(configProperties.getString("otel.logs.exporter")).isNull(); @@ -80,14 +80,14 @@ public void testOverwrittenExporterConfiguration_1() { public void testOverwrittenExporterConfiguration_2() { System.clearProperty("otel.exporter.otlp.endpoint"); System.clearProperty("otel.traces.exporter"); - System.setProperty("otel.exporter.otlp.traces.endpoint", "http://example.com:4317/"); + System.setProperty("otel.exporter.otlp.traces.endpoint", "https://example.com:4317/"); try (OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService()) { ConfigProperties configProperties = openTelemetrySdkService.getConfigProperties(); assertThat(configProperties.getString("otel.exporter.otlp.endpoint")).isNull(); assertThat(configProperties.getString("otel.exporter.otlp.traces.endpoint")) - .isEqualTo("http://example.com:4317/"); + .isEqualTo("https://example.com:4317/"); assertThat(configProperties.getString("otel.traces.exporter")).isNull(); assertThat(configProperties.getString("otel.metrics.exporter")).isEqualTo("none"); assertThat(configProperties.getString("otel.logs.exporter")).isEqualTo("none"); @@ -104,14 +104,14 @@ public void testOverwrittenExporterConfiguration_2() { public void testOverwrittenExporterConfiguration_3() { System.clearProperty("otel.exporter.otlp.endpoint"); System.setProperty("otel.traces.exporter", "otlp"); - System.setProperty("otel.exporter.otlp.traces.endpoint", "http://example.com:4317/"); + System.setProperty("otel.exporter.otlp.traces.endpoint", "https://example.com:4317/"); try (OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService()) { ConfigProperties configProperties = openTelemetrySdkService.getConfigProperties(); assertThat(configProperties.getString("otel.exporter.otlp.endpoint")).isNull(); assertThat(configProperties.getString("otel.exporter.otlp.traces.endpoint")) - .isEqualTo("http://example.com:4317/"); + .isEqualTo("https://example.com:4317/"); assertThat(configProperties.getString("otel.traces.exporter")).isEqualTo("otlp"); assertThat(configProperties.getString("otel.metrics.exporter")).isEqualTo("none"); assertThat(configProperties.getString("otel.logs.exporter")).isEqualTo("none"); From e3316a5d262be095ed34c0b0fabba03f25a38732 Mon Sep 17 00:00:00 2001 From: Cyrille Le Clerc Date: Mon, 28 Oct 2024 11:09:29 +0530 Subject: [PATCH 07/11] WIP --- .../opentelemetry/maven/OpenTelemetrySdkService.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java b/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java index 1d2fc5789..d2877fe08 100644 --- a/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java +++ b/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java @@ -55,6 +55,9 @@ public OpenTelemetrySdkService() { "OpenTelemetry: Initialize OpenTelemetrySdkService v{}...", MavenOtelSemanticAttributes.TELEMETRY_DISTRO_VERSION_VALUE); + this.resource = Resource.empty(); + this.configProperties = DefaultConfigProperties.createFromMap(Collections.emptyMap()); + AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk = AutoConfiguredOpenTelemetrySdk.builder() .setServiceClassLoader(getClass().getClassLoader()) @@ -76,13 +79,6 @@ public OpenTelemetrySdkService() { .disableShutdownHook() .build(); - if (this.resource == null) { - this.resource = Resource.empty(); - } - if (this.configProperties == null) { - this.configProperties = DefaultConfigProperties.createFromMap(Collections.emptyMap()); - } - this.openTelemetrySdk = autoConfiguredOpenTelemetrySdk.getOpenTelemetrySdk(); logger.debug("OpenTelemetry: OpenTelemetrySdkService initialized, resource:{}", resource); From 45f8efe9fa9712bb2ff2866ba16c4972cebf45b9 Mon Sep 17 00:00:00 2001 From: Cyrille Le Clerc Date: Fri, 8 Nov 2024 23:09:10 +0100 Subject: [PATCH 08/11] WIP --- .../maven/OpenTelemetrySdkService.java | 45 +++++++------------ 1 file changed, 17 insertions(+), 28 deletions(-) diff --git a/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java b/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java index d2877fe08..34e0e4b77 100644 --- a/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java +++ b/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java @@ -106,37 +106,26 @@ static Map requireExplicitConfigOfTheOtlpExporter( ConfigProperties configProperties) { Map properties = new HashMap<>(); - if (configProperties.getString("otel.exporter.otlp.endpoint") == null) { - for (SignalType signalType : SignalType.values()) { - boolean isExporterImplicitlyConfiguredToOtlp = - configProperties.getString("otel." + signalType.value + ".exporter") == null; - boolean isOtlpExporterEndpointSpecified = - configProperties.getString("otel.exporter.otlp." + signalType.value + ".endpoint") - != null; - - if (isExporterImplicitlyConfiguredToOtlp && !isOtlpExporterEndpointSpecified) { - logger.debug( - "OpenTelemetry: Disabling default OTLP exporter endpoint for signal {} exporter", - signalType.value); - properties.put("otel." + signalType.value + ".exporter", "none"); - } - } - } else { + if (configProperties.getString("otel.exporter.otlp.endpoint") != null) { logger.debug("OpenTelemetry: OTLP exporter endpoint is explicitly configured"); + return properties; } - return properties; - } - - enum SignalType { - TRACES("traces"), - METRICS("metrics"), - LOGS("logs"); - - private final String value; - - SignalType(String value) { - this.value = value; + String[] signalTypes = {"traces", "metrics", "logs"}; + for (String signalType : signalTypes) { + boolean isExporterImplicitlyConfiguredToOtlp = + configProperties.getString("otel." + signalType + ".exporter") == null; + boolean isOtlpExporterEndpointSpecified = + configProperties.getString("otel.exporter.otlp." + signalType + ".endpoint") != null; + + if (isExporterImplicitlyConfiguredToOtlp && !isOtlpExporterEndpointSpecified) { + logger.debug( + "OpenTelemetry: Disabling default OTLP exporter endpoint for signal {} exporter", + signalType); + properties.put("otel." + signalType + ".exporter", "none"); + } } + + return properties; } @PreDestroy From 4f75414127a7a6c4d7c51abf7991131616ab23cb Mon Sep 17 00:00:00 2001 From: Cyrille Le Clerc Date: Thu, 28 Nov 2024 11:40:13 +0100 Subject: [PATCH 09/11] WIP --- .../maven/AutoConfigureUtil2.java | 34 +++++++++++++ .../maven/OpenTelemetrySdkService.java | 48 +++++-------------- 2 files changed, 46 insertions(+), 36 deletions(-) create mode 100644 maven-extension/src/main/java/io/opentelemetry/maven/AutoConfigureUtil2.java diff --git a/maven-extension/src/main/java/io/opentelemetry/maven/AutoConfigureUtil2.java b/maven-extension/src/main/java/io/opentelemetry/maven/AutoConfigureUtil2.java new file mode 100644 index 000000000..87715ca78 --- /dev/null +++ b/maven-extension/src/main/java/io/opentelemetry/maven/AutoConfigureUtil2.java @@ -0,0 +1,34 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.maven; + +import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; +import io.opentelemetry.sdk.autoconfigure.internal.AutoConfigureUtil; +import io.opentelemetry.sdk.resources.Resource; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; + +public class AutoConfigureUtil2 { + + private AutoConfigureUtil2() {} + + /** + * Returns the {@link Resource} that was auto-configured. + * + * @see AutoConfigureUtil#getConfig(AutoConfiguredOpenTelemetrySdk) + */ + public static Resource getResource( + AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk) { + try { + Method method = AutoConfiguredOpenTelemetrySdk.class.getDeclaredMethod("getResource"); + method.setAccessible(true); + return (Resource) method.invoke(autoConfiguredOpenTelemetrySdk); + } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) { + throw new IllegalStateException( + "Error calling getResource on AutoConfiguredOpenTelemetrySdk", e); + } + } +} diff --git a/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java b/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java index 34e0e4b77..b5eea0dc6 100644 --- a/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java +++ b/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java @@ -11,6 +11,7 @@ import io.opentelemetry.maven.semconv.MavenOtelSemanticAttributes; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; +import io.opentelemetry.sdk.autoconfigure.internal.AutoConfigureUtil; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties; import io.opentelemetry.sdk.common.CompletableResultCode; @@ -18,10 +19,9 @@ import java.io.Closeable; import java.util.Collections; import java.util.HashMap; -import java.util.Locale; import java.util.Map; +import java.util.Optional; import java.util.concurrent.TimeUnit; -import javax.annotation.Nullable; import javax.annotation.PreDestroy; import javax.inject.Named; import javax.inject.Singleton; @@ -40,9 +40,9 @@ public final class OpenTelemetrySdkService implements Closeable { private final OpenTelemetrySdk openTelemetrySdk; - private Resource resource; + private final Resource resource; - private ConfigProperties configProperties; + private final ConfigProperties configProperties; private final Tracer tracer; @@ -55,37 +55,26 @@ public OpenTelemetrySdkService() { "OpenTelemetry: Initialize OpenTelemetrySdkService v{}...", MavenOtelSemanticAttributes.TELEMETRY_DISTRO_VERSION_VALUE); - this.resource = Resource.empty(); - this.configProperties = DefaultConfigProperties.createFromMap(Collections.emptyMap()); - AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk = AutoConfiguredOpenTelemetrySdk.builder() .setServiceClassLoader(getClass().getClassLoader()) .addPropertiesCustomizer( OpenTelemetrySdkService::requireExplicitConfigOfTheOtlpExporter) - .addPropertiesCustomizer( - config -> { - // keep a reference to the computed config properties for future use in the - // extension - this.configProperties = config; - return Collections.emptyMap(); - }) - .addResourceCustomizer( - (res, configProperties) -> { - // keep a reference to the computed Resource for future use in the extension - this.resource = Resource.builder().putAll(res).build(); - return this.resource; - }) .disableShutdownHook() .build(); this.openTelemetrySdk = autoConfiguredOpenTelemetrySdk.getOpenTelemetrySdk(); + this.configProperties = + Optional.ofNullable(AutoConfigureUtil.getConfig(autoConfiguredOpenTelemetrySdk)) + .orElseGet(() -> DefaultConfigProperties.createFromMap(Collections.emptyMap())); + this.resource = + Optional.ofNullable(AutoConfigureUtil2.getResource(autoConfiguredOpenTelemetrySdk)) + .orElseGet(Resource::getDefault); logger.debug("OpenTelemetry: OpenTelemetrySdkService initialized, resource:{}", resource); - // TODO should we replace `getBooleanConfig(name)` by `configProperties.getBoolean(name)`? - Boolean mojoSpansEnabled = getBooleanConfig("otel.instrumentation.maven.mojo.enabled"); - this.mojosInstrumentationEnabled = mojoSpansEnabled == null || mojoSpansEnabled; + this.mojosInstrumentationEnabled = + configProperties.getBoolean("otel.instrumentation.maven.mojo.enabled", true); this.tracer = openTelemetrySdk.getTracer("io.opentelemetry.contrib.maven", VERSION); } @@ -168,17 +157,4 @@ public ContextPropagators getPropagators() { public boolean isMojosInstrumentationEnabled() { return mojosInstrumentationEnabled; } - - @Nullable - private static Boolean getBooleanConfig(String name) { - String value = System.getProperty(name); - if (value != null) { - return Boolean.parseBoolean(value); - } - value = System.getenv(name.toUpperCase(Locale.ROOT).replace('.', '_')); - if (value != null) { - return Boolean.parseBoolean(value); - } - return null; - } } From f5c48258b08afa68551955979611d1de03f09bbd Mon Sep 17 00:00:00 2001 From: Cyrille Le Clerc Date: Fri, 29 Nov 2024 11:16:34 +0100 Subject: [PATCH 10/11] Better tests and javadocs --- .../maven/AutoConfigureUtil2.java | 7 +++--- .../maven/AutoConfigureUtil2Test.java | 24 +++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 maven-extension/src/test/java/io/opentelemetry/maven/AutoConfigureUtil2Test.java diff --git a/maven-extension/src/main/java/io/opentelemetry/maven/AutoConfigureUtil2.java b/maven-extension/src/main/java/io/opentelemetry/maven/AutoConfigureUtil2.java index 87715ca78..6ebcf004b 100644 --- a/maven-extension/src/main/java/io/opentelemetry/maven/AutoConfigureUtil2.java +++ b/maven-extension/src/main/java/io/opentelemetry/maven/AutoConfigureUtil2.java @@ -6,19 +6,20 @@ package io.opentelemetry.maven; import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; -import io.opentelemetry.sdk.autoconfigure.internal.AutoConfigureUtil; import io.opentelemetry.sdk.resources.Resource; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +/** Utility class to use the {@link AutoConfiguredOpenTelemetrySdk}. */ public class AutoConfigureUtil2 { private AutoConfigureUtil2() {} /** - * Returns the {@link Resource} that was auto-configured. + * Returns the {@link Resource} that was autoconfigured. * - * @see AutoConfigureUtil#getConfig(AutoConfiguredOpenTelemetrySdk) + *

Inspired by {@link + * io.opentelemetry.sdk.autoconfigure.internal.AutoConfigureUtil#getConfig(AutoConfiguredOpenTelemetrySdk)} */ public static Resource getResource( AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk) { diff --git a/maven-extension/src/test/java/io/opentelemetry/maven/AutoConfigureUtil2Test.java b/maven-extension/src/test/java/io/opentelemetry/maven/AutoConfigureUtil2Test.java new file mode 100644 index 000000000..4a72de43e --- /dev/null +++ b/maven-extension/src/test/java/io/opentelemetry/maven/AutoConfigureUtil2Test.java @@ -0,0 +1,24 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.maven; + +import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; +import java.lang.reflect.Method; +import org.junit.jupiter.api.Test; + +class AutoConfigureUtil2Test { + + /** + * Verify the reflection call works with the current version of AutoConfiguredOpenTelemetrySdk. + * + * @throws NoSuchMethodException if the method does not exist + */ + @Test + void test_getResource() throws NoSuchMethodException { + Method method = AutoConfiguredOpenTelemetrySdk.class.getDeclaredMethod("getResource"); + method.setAccessible(true); + } +} From e5cb5827b004035d40e7a22cb0023e70392c26c3 Mon Sep 17 00:00:00 2001 From: Cyrille Le Clerc Date: Fri, 6 Dec 2024 17:51:48 +0100 Subject: [PATCH 11/11] Better tests and docs --- .../maven/OpenTelemetrySdkService.java | 13 +++++-------- .../maven/OpenTelemetrySdkServiceTest.java | 15 +++++++++++++-- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java b/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java index b5eea0dc6..a357b3177 100644 --- a/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java +++ b/maven-extension/src/main/java/io/opentelemetry/maven/OpenTelemetrySdkService.java @@ -5,6 +5,7 @@ package io.opentelemetry.maven; +import com.google.common.annotations.VisibleForTesting; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.trace.Tracer; import io.opentelemetry.context.propagation.ContextPropagators; @@ -40,7 +41,7 @@ public final class OpenTelemetrySdkService implements Closeable { private final OpenTelemetrySdk openTelemetrySdk; - private final Resource resource; + @VisibleForTesting final Resource resource; private final ConfigProperties configProperties; @@ -67,10 +68,10 @@ public OpenTelemetrySdkService() { this.configProperties = Optional.ofNullable(AutoConfigureUtil.getConfig(autoConfiguredOpenTelemetrySdk)) .orElseGet(() -> DefaultConfigProperties.createFromMap(Collections.emptyMap())); - this.resource = - Optional.ofNullable(AutoConfigureUtil2.getResource(autoConfiguredOpenTelemetrySdk)) - .orElseGet(Resource::getDefault); + this.resource = AutoConfigureUtil2.getResource(autoConfiguredOpenTelemetrySdk); + // Display resource attributes in debug logs for troubleshooting when traces are not found in + // the observability backend, helping understand `service.name`, `service.namespace`, etc. logger.debug("OpenTelemetry: OpenTelemetrySdkService initialized, resource:{}", resource); this.mojosInstrumentationEnabled = @@ -141,10 +142,6 @@ public Tracer getTracer() { return this.tracer; } - public Resource getResource() { - return resource; - } - public ConfigProperties getConfigProperties() { return configProperties; } diff --git a/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java b/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java index 8bea02578..0174a6283 100644 --- a/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java +++ b/maven-extension/src/test/java/io/opentelemetry/maven/OpenTelemetrySdkServiceTest.java @@ -10,6 +10,7 @@ import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; import io.opentelemetry.sdk.resources.Resource; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Test; /** @@ -26,7 +27,7 @@ public void testDefaultConfiguration() { System.clearProperty("otel.resource.attributes"); try (OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService()) { - Resource resource = openTelemetrySdkService.getResource(); + Resource resource = openTelemetrySdkService.resource; assertThat(resource.getAttribute(stringKey("service.name"))).isEqualTo("maven"); ConfigProperties configProperties = openTelemetrySdkService.getConfigProperties(); @@ -45,7 +46,7 @@ public void testOverwrittenResourceAttributes() { try (OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService()) { - Resource resource = openTelemetrySdkService.getResource(); + Resource resource = openTelemetrySdkService.resource; assertThat(resource.getAttribute(stringKey("service.name"))).isEqualTo("my-maven"); assertThat(resource.getAttribute(stringKey("key1"))).isEqualTo("val1"); assertThat(resource.getAttribute(stringKey("key2"))).isEqualTo("val2"); @@ -122,4 +123,14 @@ public void testOverwrittenExporterConfiguration_3() { System.clearProperty("otel.exporter.otlp.traces.protocol"); } } + + @AfterAll + static void afterAll() { + System.clearProperty("otel.exporter.otlp.endpoint"); + System.clearProperty("otel.exporter.otlp.traces.endpoint"); + System.clearProperty("otel.exporter.otlp.traces.protocol"); + System.clearProperty("otel.resource.attributes"); + System.clearProperty("otel.service.name"); + System.clearProperty("otel.traces.exporter"); + } }