Merge pull request quarkusio#35193 from michalvavrik/feature/fix-otel…

…-svc-name-when-no-attrs OpenTelemetry service name should have higher priority than app name when no resource attributes are set
mkouba · Aug 7, 2023 · 0a84b4e · 0a84b4e
2 parents 772d5e1 + c36cb73
commit 0a84b4e
Show file tree

Hide file tree

Showing 7 changed files with 115 additions and 31 deletions.
diff --git a/...rc/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetryServiceNameAppNameTest.java b/...rc/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetryServiceNameAppNameTest.java
@@ -0,0 +1,22 @@
+package io.quarkus.opentelemetry.deployment;
+
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.asset.StringAsset;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.opentelemetry.deployment.common.TestSpanExporter;
+import io.quarkus.opentelemetry.deployment.common.TestSpanExporterProvider;
+import io.quarkus.test.QuarkusUnitTest;
+
+public class OpenTelemetryServiceNameAppNameTest extends OpenTelemetryServiceNameBaseTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest TEST = new QuarkusUnitTest().setArchiveProducer(
+            () -> ShrinkWrap.create(JavaArchive.class)
+                    .addClass(TestSpanExporter.class)
+                    .addClass(TestSpanExporterProvider.class)
+                    .addAsResource(new StringAsset("" +
+                            "quarkus.otel.bsp.schedule.delay=50\n" +
+                            "quarkus.application.name=" + SERVICE_NAME + "\n"), "application.properties"));
+}
diff --git a/...loyment/OpenTelemetryServiceNameTest.java → ...ent/OpenTelemetryServiceNameBaseTest.java b/...loyment/OpenTelemetryServiceNameTest.java → ...ent/OpenTelemetryServiceNameBaseTest.java
@@ -11,41 +11,22 @@
 import jakarta.ws.rs.GET;
 import jakarta.ws.rs.Path;
 
-import org.jboss.shrinkwrap.api.ShrinkWrap;
-import org.jboss.shrinkwrap.api.spec.JavaArchive;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.RegisterExtension;
 
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.sdk.trace.data.SpanData;
 import io.quarkus.opentelemetry.deployment.common.TestSpanExporter;
-import io.quarkus.opentelemetry.deployment.common.TestSpanExporterProvider;
-import io.quarkus.test.QuarkusUnitTest;
 import io.restassured.RestAssured;
-import io.smallrye.config.SmallRyeConfig;
 
-public class OpenTelemetryServiceNameTest {
+public abstract class OpenTelemetryServiceNameBaseTest {
 
-    private static final String SERVICE_NAME = "FrankBullitt";
+    protected static final String SERVICE_NAME = "FrankBullitt";
 
-    @RegisterExtension
-    static final QuarkusUnitTest TEST = new QuarkusUnitTest().setArchiveProducer(
-            () -> ShrinkWrap.create(JavaArchive.class)
-                    .addClass(TestSpanExporter.class)
-                    .addClass(TestSpanExporterProvider.class)
-                    .addAsResource("resource-config/application.properties", "application.properties")
-                    .addAsResource(
-                            "META-INF/services-config/io.opentelemetry.sdk.autoconfigure.spi.traces.ConfigurableSpanExporterProvider",
-                            "META-INF/services/io.opentelemetry.sdk.autoconfigure.spi.traces.ConfigurableSpanExporterProvider"))
-            .overrideRuntimeConfigKey("quarkus.otel.service.name", SERVICE_NAME);
-
-    @Inject
-    SmallRyeConfig config;
     @Inject
     TestSpanExporter spanExporter;
 
     @Test
-    void testSvcNameHasPriorityOverAppNameAndResourceAttr() {
+    void testServiceName() {
         RestAssured.when()
                 .get("/hello").then()
                 .statusCode(200)

diff --git a/...io/quarkus/opentelemetry/deployment/OpenTelemetryServiceNameCombinedResourceWinsTest.java b/...io/quarkus/opentelemetry/deployment/OpenTelemetryServiceNameCombinedResourceWinsTest.java
@@ -0,0 +1,22 @@
+package io.quarkus.opentelemetry.deployment;
+
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.opentelemetry.deployment.common.TestSpanExporter;
+import io.quarkus.opentelemetry.deployment.common.TestSpanExporterProvider;
+import io.quarkus.test.QuarkusUnitTest;
+
+public class OpenTelemetryServiceNameCombinedResourceWinsTest extends OpenTelemetryServiceNameBaseTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest TEST = new QuarkusUnitTest().setArchiveProducer(
+            () -> ShrinkWrap.create(JavaArchive.class)
+                    .addClass(TestSpanExporter.class)
+                    .addClass(TestSpanExporterProvider.class))
+            .overrideConfigKey("quarkus.application.name", "application-name-must-fail")
+            .overrideRuntimeConfigKey("quarkus.otel.bsp.schedule.delay", "50")// speed up test
+            .overrideRuntimeConfigKey("quarkus.otel.service.name", SERVICE_NAME)
+            .overrideRuntimeConfigKey("quarkus.otel.resource.attributes", "service.name=" + "attributes-must-fail");
+}
diff --git a/.../io/quarkus/opentelemetry/deployment/OpenTelemetryServiceNameCombinedServiceWinsTest.java b/.../io/quarkus/opentelemetry/deployment/OpenTelemetryServiceNameCombinedServiceWinsTest.java
@@ -0,0 +1,21 @@
+package io.quarkus.opentelemetry.deployment;
+
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.opentelemetry.deployment.common.TestSpanExporter;
+import io.quarkus.opentelemetry.deployment.common.TestSpanExporterProvider;
+import io.quarkus.test.QuarkusUnitTest;
+
+public class OpenTelemetryServiceNameCombinedServiceWinsTest extends OpenTelemetryServiceNameBaseTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest TEST = new QuarkusUnitTest().setArchiveProducer(
+            () -> ShrinkWrap.create(JavaArchive.class)
+                    .addClass(TestSpanExporter.class)
+                    .addClass(TestSpanExporterProvider.class))
+            .overrideConfigKey("quarkus.application.name", "application-name-must-fail")
+            .overrideRuntimeConfigKey("quarkus.otel.bsp.schedule.delay", "50")// speed up test
+            .overrideRuntimeConfigKey("quarkus.otel.resource.attributes", "service.name=" + SERVICE_NAME);
+}
diff --git a/...java/io/quarkus/opentelemetry/deployment/OpenTelemetryServiceNameNoResourceAttrsTest.java b/...java/io/quarkus/opentelemetry/deployment/OpenTelemetryServiceNameNoResourceAttrsTest.java
@@ -0,0 +1,20 @@
+package io.quarkus.opentelemetry.deployment;
+
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.opentelemetry.deployment.common.TestSpanExporter;
+import io.quarkus.opentelemetry.deployment.common.TestSpanExporterProvider;
+import io.quarkus.test.QuarkusUnitTest;
+
+public class OpenTelemetryServiceNameNoResourceAttrsTest extends OpenTelemetryServiceNameBaseTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest TEST = new QuarkusUnitTest().setArchiveProducer(
+            () -> ShrinkWrap.create(JavaArchive.class)
+                    .addClass(TestSpanExporter.class)
+                    .addClass(TestSpanExporterProvider.class))
+            .overrideConfigKey("quarkus.otel.service.name", SERVICE_NAME)
+            .overrideRuntimeConfigKey("quarkus.otel.bsp.schedule.delay", "50");// speed up test
+}
diff --git a/...st/java/io/quarkus/opentelemetry/deployment/OpenTelemetryServiceNameResourceAttrTest.java b/...st/java/io/quarkus/opentelemetry/deployment/OpenTelemetryServiceNameResourceAttrTest.java
@@ -0,0 +1,20 @@
+package io.quarkus.opentelemetry.deployment;
+
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.opentelemetry.deployment.common.TestSpanExporter;
+import io.quarkus.opentelemetry.deployment.common.TestSpanExporterProvider;
+import io.quarkus.test.QuarkusUnitTest;
+
+public class OpenTelemetryServiceNameResourceAttrTest extends OpenTelemetryServiceNameBaseTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest TEST = new QuarkusUnitTest().setArchiveProducer(
+            () -> ShrinkWrap.create(JavaArchive.class)
+                    .addClass(TestSpanExporter.class)
+                    .addClass(TestSpanExporterProvider.class))
+            .overrideRuntimeConfigKey("quarkus.otel.bsp.schedule.delay", "50")// speed up test
+            .overrideRuntimeConfigKey("quarkus.otel.resource.attributes", "service.name=" + SERVICE_NAME);
+}
diff --git a/...ntelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/TracerUtil.java b/...ntelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/TracerUtil.java
@@ -15,20 +15,18 @@ private TracerUtil() {
     }
 
     public static Resource mapResourceAttributes(List<String> resourceAttributes, String serviceName) {
-        if (resourceAttributes.isEmpty()) {
-            return Resource.empty();
+        final AttributesBuilder attributesBuilder = Attributes.builder();
+
+        if (!resourceAttributes.isEmpty()) {
+            OpenTelemetryUtil
+                    .convertKeyValueListToMap(resourceAttributes)
+                    .forEach(attributesBuilder::put);
         }
-        AttributesBuilder attributesBuilder = Attributes.builder();
-        var attrNameToValue = OpenTelemetryUtil.convertKeyValueListToMap(resourceAttributes);
 
-        // override both default (app name) and explicitly set resource attribute
-        // it needs to be done manually because OpenTelemetry correctly sets 'otel.service.name'
-        // to existing (incoming) resource, but customizer output replaces originally set service name
         if (serviceName != null) {
-            attrNameToValue.put(SERVICE_NAME.getKey(), serviceName);
+            attributesBuilder.put(SERVICE_NAME.getKey(), serviceName);
         }
 
-        attrNameToValue.forEach(attributesBuilder::put);
         return Resource.create(attributesBuilder.build());
     }
 }