address review comments, add more resource attributes

Author: laurit

instrumentation/jdbc/build.gradle.kts

@@ -13,6 +13,7 @@ muzzle {
 
 dependencies {
   compileOnly("io.opentelemetry:opentelemetry-api")
+  compileOnly("io.opentelemetry.semconv:opentelemetry-semconv-incubating")
   compileOnly(project(":custom"))
 
   testInstrumentation("io.opentelemetry.javaagent.instrumentation:opentelemetry-javaagent-jdbc")

instrumentation/jdbc/src/main/java/com/splunk/opentelemetry/instrumentation/jdbc/PropagatorInitializer.java

@@ -16,59 +16,33 @@
 
 package com.splunk.opentelemetry.instrumentation.jdbc;
 
+import static com.splunk.opentelemetry.instrumentation.jdbc.SqlCommenterInitializer.defaultPropagator;
+import static com.splunk.opentelemetry.instrumentation.jdbc.SqlCommenterInitializer.traceContextPropagator;
 import static io.opentelemetry.sdk.autoconfigure.AutoConfigureUtil.getResource;
 import static io.opentelemetry.semconv.ServiceAttributes.SERVICE_NAME;
+import static io.opentelemetry.semconv.incubating.DeploymentIncubatingAttributes.DEPLOYMENT_ENVIRONMENT_NAME;
+import static io.opentelemetry.semconv.incubating.ServiceIncubatingAttributes.SERVICE_NAMESPACE;
 
 import com.google.auto.service.AutoService;
 import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator;
-import io.opentelemetry.context.Context;
-import io.opentelemetry.context.propagation.TextMapGetter;
 import io.opentelemetry.context.propagation.TextMapPropagator;
-import io.opentelemetry.context.propagation.TextMapSetter;
 import io.opentelemetry.javaagent.extension.AgentListener;
 import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
 import io.opentelemetry.sdk.resources.Resource;
-import java.util.Collection;
-import java.util.Collections;
 
 @AutoService(AgentListener.class)
 public class PropagatorInitializer implements AgentListener {
-  // propagates service.name attribute
-  static TextMapPropagator defaultPropagator = TextMapPropagator.noop();
-  // propagates service.name attribute and traceparent
-  static TextMapPropagator traceContextPropagator = W3CTraceContextPropagator.getInstance();
 
   @Override
   public void afterAgent(AutoConfiguredOpenTelemetrySdk sdk) {
     Resource resource = getResource(sdk);
     String serviceName = resource.getAttribute(SERVICE_NAME);
-    if (!"unknown_service:java".equals(serviceName)) {
-      defaultPropagator = new ServiceAttributePropagator(serviceName);
-      traceContextPropagator =
-          TextMapPropagator.composite(defaultPropagator, traceContextPropagator);
-    }
-  }
-
-  private static class ServiceAttributePropagator implements TextMapPropagator {
-    private final String serviceName;
-
-    ServiceAttributePropagator(String serviceName) {
-      this.serviceName = serviceName;
-    }
-
-    @Override
-    public <C> void inject(Context context, C carrier, TextMapSetter<C> setter) {
-      setter.set(carrier, SERVICE_NAME.getKey(), serviceName);
-    }
-
-    @Override
-    public <C> Context extract(Context context, C carrier, TextMapGetter<C> getter) {
-      return context;
-    }
+    String serviceNamespace = resource.getAttribute(SERVICE_NAMESPACE);
+    String deploymentEnvironment = resource.getAttribute(DEPLOYMENT_ENVIRONMENT_NAME);
 
-    @Override
-    public Collection<String> fields() {
-      return Collections.emptyList();
-    }
+    defaultPropagator =
+        new ServiceAttributePropagator(serviceName, serviceNamespace, deploymentEnvironment);
+    traceContextPropagator =
+        TextMapPropagator.composite(defaultPropagator, W3CTraceContextPropagator.getInstance());
   }
 }

instrumentation/jdbc/src/main/java/com/splunk/opentelemetry/instrumentation/jdbc/ServiceAttributePropagator.java

@@ -0,0 +1,65 @@
+/*
+ * Copyright Splunk Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.splunk.opentelemetry.instrumentation.jdbc;
+
+import static io.opentelemetry.semconv.ServiceAttributes.SERVICE_NAME;
+import static io.opentelemetry.semconv.incubating.DeploymentIncubatingAttributes.DEPLOYMENT_ENVIRONMENT_NAME;
+import static io.opentelemetry.semconv.incubating.ServiceIncubatingAttributes.SERVICE_NAMESPACE;
+
+import io.opentelemetry.context.Context;
+import io.opentelemetry.context.propagation.TextMapGetter;
+import io.opentelemetry.context.propagation.TextMapPropagator;
+import io.opentelemetry.context.propagation.TextMapSetter;
+import java.util.Collection;
+import java.util.Collections;
+
+class ServiceAttributePropagator implements TextMapPropagator {
+  private final String serviceName;
+  private final String serviceNamespace;
+  private final String deploymentEnvironment;
+
+  ServiceAttributePropagator(
+      String serviceName, String serviceNamespace, String deploymentEnvironment) {
+    this.serviceName = serviceName;
+    this.serviceNamespace = serviceNamespace;
+    this.deploymentEnvironment = deploymentEnvironment;
+  }
+
+  @Override
+  public <C> void inject(Context context, C carrier, TextMapSetter<C> setter) {
+    setIfNotNull(setter, carrier, SERVICE_NAME.getKey(), serviceName);
+    setIfNotNull(setter, carrier, SERVICE_NAMESPACE.getKey(), serviceNamespace);
+    setIfNotNull(setter, carrier, DEPLOYMENT_ENVIRONMENT_NAME.getKey(), deploymentEnvironment);
+  }
+
+  private static <C> void setIfNotNull(
+      TextMapSetter<C> setter, C carrier, String key, String value) {
+    if (value != null) {
+      setter.set(carrier, key, value);
+    }
+  }
+
+  @Override
+  public <C> Context extract(Context context, C carrier, TextMapGetter<C> getter) {
+    return context;
+  }
+
+  @Override
+  public Collection<String> fields() {
+    return Collections.emptyList();
+  }
+}

instrumentation/jdbc/src/main/java/com/splunk/opentelemetry/instrumentation/jdbc/SqlCommenterInitializer.java

@@ -17,12 +17,18 @@
 package com.splunk.opentelemetry.instrumentation.jdbc;
 
 import com.google.auto.service.AutoService;
+import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator;
+import io.opentelemetry.context.propagation.TextMapPropagator;
 import io.opentelemetry.instrumentation.api.incubator.semconv.db.internal.SqlCommenterBuilder;
 import io.opentelemetry.javaagent.bootstrap.internal.AgentInstrumentationConfig;
 import io.opentelemetry.javaagent.bootstrap.internal.sqlcommenter.SqlCommenterCustomizer;
 
 @AutoService(SqlCommenterCustomizer.class)
 public class SqlCommenterInitializer implements SqlCommenterCustomizer {
+  // propagates service.name and other static attribute
+  static TextMapPropagator defaultPropagator = TextMapPropagator.noop();
+  // propagates static attributes and traceparent
+  static TextMapPropagator traceContextPropagator = W3CTraceContextPropagator.getInstance();
 
   @Override
   public void customize(SqlCommenterBuilder sqlCommenterBuilder) {
@@ -34,12 +40,12 @@ public void customize(SqlCommenterBuilder sqlCommenterBuilder) {
           // for postgres we add traceparent to comments, oracle and mssql use other means to
           // propagate context and other databases are currently unsupported
           if (connection.getClass().getName().startsWith("org.postgresql.jdbc")) {
-            return PropagatorInitializer.traceContextPropagator;
+            return traceContextPropagator;
           }
 
           // note that besides jdbc this applies to r2dbc and other data access apis that upstream
           // has sqlcommenter support for
-          return PropagatorInitializer.defaultPropagator;
+          return defaultPropagator;
         });
   }
 }

instrumentation/jdbc/src/test/java/com/splunk/opentelemetry/instrumentation/jdbc/ServiceAttributePropagatorTest.java

@@ -0,0 +1,47 @@
+/*
+ * Copyright Splunk Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.splunk.opentelemetry.instrumentation.jdbc;
+
+import static io.opentelemetry.semconv.ServiceAttributes.SERVICE_NAME;
+import static io.opentelemetry.semconv.incubating.DeploymentIncubatingAttributes.DEPLOYMENT_ENVIRONMENT_NAME;
+import static io.opentelemetry.semconv.incubating.ServiceIncubatingAttributes.SERVICE_NAMESPACE;
+import static java.util.Map.entry;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import io.opentelemetry.context.Context;
+import io.opentelemetry.context.propagation.TextMapPropagator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import org.junit.jupiter.api.Test;
+
+class ServiceAttributePropagatorTest {
+
+  @Test
+  void testPropagation() {
+    TextMapPropagator propagator =
+        new ServiceAttributePropagator(
+            "service-name", "service-namespace", "deployment-environment");
+    Map<String, String> carrier = new LinkedHashMap<>();
+    propagator.inject(Context.root(), carrier, (map, key, value) -> map.put(key, value));
+
+    assertThat(carrier)
+        .containsExactly(
+            entry(SERVICE_NAME.getKey(), "service-name"),
+            entry(SERVICE_NAMESPACE.getKey(), "service-namespace"),
+            entry(DEPLOYMENT_ENVIRONMENT_NAME.getKey(), "deployment-environment"));
+  }
+}

instrumentation/jdbc/src/test/java/com/splunk/opentelemetry/instrumentation/jdbc/postgresql/PostgreSqlServerTest.java

@@ -30,7 +30,6 @@
 import java.util.ArrayList;
 import java.util.List;
 import org.junit.jupiter.api.AfterAll;
-import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -73,7 +72,6 @@ static void cleanup() {
   }
 
   @BeforeEach
-  @AfterEach
   void cleanupTest() {
     executedSql.clear();
   }