Merge pull request #31909 from brunobat/otel-endpoint-fix

Fix OTel endpoint resolution and improve documentation
quarkusio · Mar 22, 2023 · 476cdc6 · 476cdc6
2 parents 652c0a2 + d86aaa4
commit 476cdc6
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 33 deletions.
diff --git a/docs/src/main/asciidoc/opentelemetry.adoc b/docs/src/main/asciidoc/opentelemetry.adoc
@@ -429,7 +429,7 @@ Write the SPI loader text file at `resources/META-INF/services` with name `io.op
 Then activate on the configuration:
 [source,properties]
 ----
-quarkus.opentelemetry.tracer.sampler=custom-spi-sampler
+quarkus.otel.traces.sampler=custom-spi-sampler
 ----
 
 As you can see, CDI is much simpler to work with.

diff --git a/...rc/main/java/io/quarkus/opentelemetry/deployment/exporter/otlp/OtlpExporterProcessor.java b/...rc/main/java/io/quarkus/opentelemetry/deployment/exporter/otlp/OtlpExporterProcessor.java
@@ -1,7 +1,6 @@
 package io.quarkus.opentelemetry.deployment.exporter.otlp;
 
 import static io.quarkus.opentelemetry.runtime.config.build.ExporterType.Constants.CDI_VALUE;
-import static io.quarkus.opentelemetry.runtime.config.build.ExporterType.Constants.OTLP_VALUE;
 
 import java.util.function.BooleanSupplier;
 
@@ -23,16 +22,14 @@
 public class OtlpExporterProcessor {
 
     static class OtlpExporterEnabled implements BooleanSupplier {
-        OtlpExporterBuildConfig exporBuildConfig;
+        OtlpExporterBuildConfig exportBuildConfig;
         OtelBuildConfig otelBuildConfig;
 
         public boolean getAsBoolean() {
             return otelBuildConfig.enabled &&
                     otelBuildConfig.traces.enabled.orElse(Boolean.TRUE) &&
-                    (otelBuildConfig.traces.exporter.contains(OTLP_VALUE) ||
-                            otelBuildConfig.traces.exporter.contains(CDI_VALUE))
-                    &&
-                    exporBuildConfig.enabled;
+                    otelBuildConfig.traces.exporter.contains(CDI_VALUE) &&
+                    exportBuildConfig.enabled;
         }
     }
 

diff --git a/...ry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/exporter/otlp/OtlpRecorder.java b/...ry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/exporter/otlp/OtlpRecorder.java
@@ -1,5 +1,6 @@
 package io.quarkus.opentelemetry.runtime.exporter.otlp;
 
+import static io.quarkus.opentelemetry.runtime.config.runtime.exporter.OtlpExporterRuntimeConfig.Constants.DEFAULT_GRPC_BASE_URI;
 import static io.quarkus.opentelemetry.runtime.config.runtime.exporter.OtlpExporterTracesConfig.Protocol.HTTP_PROTOBUF;
 
 import java.util.function.Consumer;
@@ -20,17 +21,22 @@
 
 @Recorder
 public class OtlpRecorder {
-    static String resolveEndpoint(final LaunchMode launchMode, final OtlpExporterRuntimeConfig runtimeConfig) {
+
+    static String resolveEndpoint(final OtlpExporterRuntimeConfig runtimeConfig) {
         String endpoint = runtimeConfig.traces.legacyEndpoint
+                .filter(OtlpRecorder::excludeDefaultEndpoint)
                 .orElse(runtimeConfig.traces.endpoint
-                        .orElse(runtimeConfig.endpoint.orElse("")));
-        if (launchMode == LaunchMode.DEVELOPMENT && endpoint.isEmpty()) {
-            // Default the endpoint for development only
-            endpoint = OtlpExporterRuntimeConfig.Constants.DEFAULT_GRPC_BASE_URI;
-        }
+                        .filter(OtlpRecorder::excludeDefaultEndpoint)
+                        .orElse(runtimeConfig.endpoint
+                                .filter(OtlpRecorder::excludeDefaultEndpoint)
+                                .orElse(DEFAULT_GRPC_BASE_URI)));
         return endpoint.trim();
     }
 
+    private static boolean excludeDefaultEndpoint(String endpoint) {
+        return !DEFAULT_GRPC_BASE_URI.equals(endpoint);
+    }
+
     public void installBatchSpanProcessorForOtlp(
             OtelRuntimeConfig otelRuntimeConfig,
             OtlpExporterRuntimeConfig exporterRuntimeConfig,
@@ -39,7 +45,7 @@ public void installBatchSpanProcessorForOtlp(
         if (otelRuntimeConfig.sdkDisabled) {
             return;
         }
-        String endpoint = resolveEndpoint(launchMode, exporterRuntimeConfig).trim();
+        String endpoint = resolveEndpoint(exporterRuntimeConfig).trim();
 
         // Only create the OtlpGrpcSpanExporter if an endpoint was set in runtime config
         if (endpoint.length() > 0) {

diff --git a/...untime/src/test/java/io/quarkus/opentelemetry/runtime/exporter/otlp/OtlpRecorderTest.java b/...untime/src/test/java/io/quarkus/opentelemetry/runtime/exporter/otlp/OtlpRecorderTest.java
@@ -1,5 +1,6 @@
 package io.quarkus.opentelemetry.runtime.exporter.otlp;
 
+import static io.quarkus.opentelemetry.runtime.config.runtime.exporter.OtlpExporterRuntimeConfig.Constants.DEFAULT_GRPC_BASE_URI;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
 import java.util.Optional;
@@ -8,50 +9,58 @@
 
 import io.quarkus.opentelemetry.runtime.config.runtime.exporter.OtlpExporterRuntimeConfig;
 import io.quarkus.opentelemetry.runtime.config.runtime.exporter.OtlpExporterTracesConfig;
-import io.quarkus.runtime.LaunchMode;
 
 class OtlpRecorderTest {
 
     @Test
     public void resolveEndpoint_legacyWins() {
         assertEquals("http://localhost:1111/",
-                OtlpRecorder.resolveEndpoint(LaunchMode.NORMAL, createOtlpExporterRuntimeConfig(
-                        "http://localhost:4317/",
+                OtlpRecorder.resolveEndpoint(createOtlpExporterRuntimeConfig(
+                        DEFAULT_GRPC_BASE_URI,
                         "http://localhost:1111/",
                         "http://localhost:2222/")));
     }
 
     @Test
-    public void resolveEndpoint_legacyTraceWins() {
+    public void resolveEndpoint_newWins() {
         assertEquals("http://localhost:2222/",
-                OtlpRecorder.resolveEndpoint(LaunchMode.NORMAL, createOtlpExporterRuntimeConfig(
-                        "http://localhost:4317/",
-                        null,
+                OtlpRecorder.resolveEndpoint(createOtlpExporterRuntimeConfig(
+                        "http://localhost:1111/",
+                        DEFAULT_GRPC_BASE_URI,
                         "http://localhost:2222/")));
     }
 
     @Test
-    public void resolveEndpoint_legacyGlobalWins() {
-        assertEquals("http://localhost:4317/",
-                OtlpRecorder.resolveEndpoint(LaunchMode.NORMAL, createOtlpExporterRuntimeConfig(
-                        "http://localhost:4317/",
-                        null,
-                        null)));
+    public void resolveEndpoint_globalWins() {
+        assertEquals("http://localhost:1111/",
+                OtlpRecorder.resolveEndpoint(createOtlpExporterRuntimeConfig(
+                        "http://localhost:1111/",
+                        DEFAULT_GRPC_BASE_URI,
+                        DEFAULT_GRPC_BASE_URI)));
     }
 
     @Test
-    public void resolveEndpoint_testIsSet() {
-        assertEquals("http://localhost:4317/",
-                OtlpRecorder.resolveEndpoint(LaunchMode.DEVELOPMENT, createOtlpExporterRuntimeConfig(
+    public void resolveEndpoint_legacyTraceWins() {
+        assertEquals("http://localhost:2222/",
+                OtlpRecorder.resolveEndpoint(createOtlpExporterRuntimeConfig(
+                        DEFAULT_GRPC_BASE_URI,
                         null,
+                        "http://localhost:2222/")));
+    }
+
+    @Test
+    public void resolveEndpoint_legacyGlobalWins() {
+        assertEquals(DEFAULT_GRPC_BASE_URI,
+                OtlpRecorder.resolveEndpoint(createOtlpExporterRuntimeConfig(
+                        DEFAULT_GRPC_BASE_URI,
                         null,
                         null)));
     }
 
     @Test
-    public void resolveEndpoint_NothingSet() {
-        assertEquals("",
-                OtlpRecorder.resolveEndpoint(LaunchMode.NORMAL, createOtlpExporterRuntimeConfig(
+    public void resolveEndpoint_testIsSet() {
+        assertEquals(DEFAULT_GRPC_BASE_URI,
+                OtlpRecorder.resolveEndpoint(createOtlpExporterRuntimeConfig(
                         null,
                         null,
                         null)));