Use env-entry to add tags per webapp deployment (#8138)

* Use env-entry to add tags per webapp deployment * fix gradle file * Migrate to hasmethodadvice * exclude classes from coverage * codenarc * add more repos * jacoco * Update internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java Co-authored-by: Bruce Bujon <PerfectSlayer@users.noreply.github.com> * review * use our named * more coverage * Update internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java Co-authored-by: Stuart McCulloch <stuart.mcculloch@datadoghq.com> * Update internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java Co-authored-by: Stuart McCulloch <stuart.mcculloch@datadoghq.com> * review * add jmh * optimize * widen muzzle excludes * exclude lazy from branch coverage * clean * Do not set contextual service name if jee-split-by-deployment is not enabled --------- Co-authored-by: Bruce Bujon <PerfectSlayer@users.noreply.github.com> Co-authored-by: Stuart McCulloch <stuart.mcculloch@datadoghq.com>
DataDog · Jan 10, 2025 · a3a8fe6 · a3a8fe6
1 parent 22458b3
commit a3a8fe6
Show file tree

Hide file tree

Showing 30 changed files with 954 additions and 121 deletions.
diff --git a/...dules/src/main/java/datadog/trace/instrumentation/jbossmodules/ModuleInstrumentation.java b/...dules/src/main/java/datadog/trace/instrumentation/jbossmodules/ModuleInstrumentation.java
@@ -9,7 +9,7 @@
 import com.google.auto.service.AutoService;
 import datadog.trace.agent.tooling.Instrumenter;
 import datadog.trace.agent.tooling.InstrumenterModule;
-import datadog.trace.api.naming.ClassloaderServiceNames;
+import datadog.trace.api.ClassloaderConfigurationOverrides;
 import datadog.trace.bootstrap.AgentClassLoading;
 import java.io.IOException;
 import java.io.InputStream;
@@ -164,7 +164,7 @@ public static class CaptureModuleNameAdvice {
     public static void afterConstruct(@Advice.This final Module module) {
       final String name = ModuleNameHelper.extractDeploymentName(module.getClassLoader());
       if (name != null && !name.isEmpty()) {
-        ClassloaderServiceNames.addServiceName(module.getClassLoader(), name);
+        ClassloaderConfigurationOverrides.withPinnedServiceName(module.getClassLoader(), name);
       }
     }
   }

diff --git a/...0/src/main/java/datadog/trace/instrumentation/liberty20/LibertyServerInstrumentation.java b/...0/src/main/java/datadog/trace/instrumentation/liberty20/LibertyServerInstrumentation.java
@@ -16,11 +16,11 @@
 import com.ibm.wsspi.webcontainer.webapp.IWebAppDispatcherContext;
 import datadog.trace.agent.tooling.Instrumenter;
 import datadog.trace.agent.tooling.InstrumenterModule;
+import datadog.trace.api.ClassloaderConfigurationOverrides;
 import datadog.trace.api.Config;
 import datadog.trace.api.CorrelationIdentifier;
 import datadog.trace.api.GlobalTracer;
 import datadog.trace.api.gateway.Flow;
-import datadog.trace.api.naming.ClassloaderServiceNames;
 import datadog.trace.bootstrap.ActiveSubsystems;
 import datadog.trace.bootstrap.ContextStore;
 import datadog.trace.bootstrap.InstrumentationContext;
@@ -116,7 +116,7 @@ public static class HandleRequestAdvice {
           if (webapp != null) {
             final ClassLoader cl = webapp.getClassLoader();
             if (cl != null) {
-              ClassloaderServiceNames.maybeSetToSpan(span, cl);
+              ClassloaderConfigurationOverrides.maybeEnrichSpan(span, cl);
             }
           }
         }

diff --git a/...java/datadog/trace/instrumentation/liberty20/ThreadContextClassloaderInstrumentation.java b/...java/datadog/trace/instrumentation/liberty20/ThreadContextClassloaderInstrumentation.java
@@ -6,7 +6,7 @@
 import com.ibm.ws.classloading.internal.ThreadContextClassLoader;
 import datadog.trace.agent.tooling.Instrumenter;
 import datadog.trace.agent.tooling.InstrumenterModule;
-import datadog.trace.api.naming.ClassloaderServiceNames;
+import datadog.trace.api.ClassloaderConfigurationOverrides;
 import net.bytebuddy.asm.Advice;
 
 @AutoService(InstrumenterModule.class)
@@ -40,7 +40,7 @@ public static class ThreadContextClassloaderAdvice {
     public static void afterConstruct(@Advice.This ThreadContextClassLoader self) {
       final String name = BundleNameHelper.extractDeploymentName(self);
       if (name != null && !name.isEmpty()) {
-        ClassloaderServiceNames.addServiceName(self, name);
+        ClassloaderConfigurationOverrides.withPinnedServiceName(self, name);
       }
     }
   }

diff --git a/...3/src/main/java/datadog/trace/instrumentation/liberty23/LibertyServerInstrumentation.java b/...3/src/main/java/datadog/trace/instrumentation/liberty23/LibertyServerInstrumentation.java
@@ -16,11 +16,11 @@
 import com.ibm.wsspi.webcontainer.webapp.IWebAppDispatcherContext;
 import datadog.trace.agent.tooling.Instrumenter;
 import datadog.trace.agent.tooling.InstrumenterModule;
+import datadog.trace.api.ClassloaderConfigurationOverrides;
 import datadog.trace.api.Config;
 import datadog.trace.api.CorrelationIdentifier;
 import datadog.trace.api.GlobalTracer;
 import datadog.trace.api.gateway.Flow;
-import datadog.trace.api.naming.ClassloaderServiceNames;
 import datadog.trace.bootstrap.ActiveSubsystems;
 import datadog.trace.bootstrap.ContextStore;
 import datadog.trace.bootstrap.InstrumentationContext;
@@ -118,7 +118,7 @@ public static class HandleRequestAdvice {
           if (webapp != null) {
             final ClassLoader cl = webapp.getClassLoader();
             if (cl != null) {
-              ClassloaderServiceNames.maybeSetToSpan(span, cl);
+              ClassloaderConfigurationOverrides.maybeEnrichSpan(span, cl);
             }
           }
         }

diff --git a/...ervlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java b/...ervlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java
@@ -4,12 +4,12 @@
 import static datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator.DD_SPAN_ATTRIBUTE;
 import static datadog.trace.instrumentation.servlet2.Servlet2Decorator.DECORATE;
 
+import datadog.trace.api.ClassloaderConfigurationOverrides;
 import datadog.trace.api.Config;
 import datadog.trace.api.CorrelationIdentifier;
 import datadog.trace.api.DDTags;
 import datadog.trace.api.GlobalTracer;
 import datadog.trace.api.gateway.Flow;
-import datadog.trace.api.naming.ClassloaderServiceNames;
 import datadog.trace.bootstrap.InstrumentationContext;
 import datadog.trace.bootstrap.instrumentation.api.AgentScope;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
@@ -42,7 +42,7 @@ public static boolean onEnter(
     final boolean hasServletTrace = spanAttr instanceof AgentSpan;
     if (hasServletTrace) {
       final AgentSpan span = (AgentSpan) spanAttr;
-      ClassloaderServiceNames.maybeSetToSpan(span);
+      ClassloaderConfigurationOverrides.maybeEnrichSpan(span);
       // Tracing might already be applied by the FilterChain or a parent request (forward/include).
       return false;
     }

diff --git a/...ervlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java b/...ervlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java
@@ -7,12 +7,12 @@
 import static datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator.DD_SPAN_ATTRIBUTE;
 import static datadog.trace.instrumentation.servlet3.Servlet3Decorator.DECORATE;
 
+import datadog.trace.api.ClassloaderConfigurationOverrides;
 import datadog.trace.api.Config;
 import datadog.trace.api.CorrelationIdentifier;
 import datadog.trace.api.DDTags;
 import datadog.trace.api.GlobalTracer;
 import datadog.trace.api.gateway.Flow;
-import datadog.trace.api.naming.ClassloaderServiceNames;
 import datadog.trace.bootstrap.instrumentation.api.AgentScope;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
@@ -63,7 +63,7 @@ public static boolean onEnter(
     final boolean hasServletTrace = spanAttrValue instanceof AgentSpan;
     if (hasServletTrace) {
       final AgentSpan span = (AgentSpan) spanAttrValue;
-      ClassloaderServiceNames.maybeSetToSpan(span);
+      ClassloaderConfigurationOverrides.maybeEnrichSpan(span);
       // Tracing might already be applied by other instrumentation,
       // the FilterChain or a parent request (forward/include).
       return false;

diff --git a/...let/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java b/...let/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java
@@ -1,6 +1,6 @@
 package datadog.trace.instrumentation.servlet3;
 
-import datadog.trace.api.naming.ClassloaderServiceNames;
+import datadog.trace.api.ClassloaderConfigurationOverrides;
 import datadog.trace.bootstrap.instrumentation.api.AgentPropagation;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
@@ -84,7 +84,7 @@ public AgentSpan onRequest(
       final HttpServletRequest request,
       AgentSpanContext.Extracted context) {
     assert span != null;
-    ClassloaderServiceNames.maybeSetToSpan(span);
+    ClassloaderConfigurationOverrides.maybeEnrichSpan(span);
     if (request != null) {
       String contextPath = request.getContextPath();
       String servletPath = request.getServletPath();

diff --git a/...5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaServletInstrumentation.java b/...5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaServletInstrumentation.java
@@ -11,9 +11,9 @@
 import com.google.auto.service.AutoService;
 import datadog.trace.agent.tooling.Instrumenter;
 import datadog.trace.agent.tooling.InstrumenterModule;
+import datadog.trace.api.ClassloaderConfigurationOverrides;
 import datadog.trace.api.Config;
 import datadog.trace.api.DDTags;
-import datadog.trace.api.naming.ClassloaderServiceNames;
 import datadog.trace.bootstrap.CallDepthThreadLocalMap;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import jakarta.servlet.ServletRequest;
@@ -61,7 +61,7 @@ public static AgentSpan before(@Advice.Argument(0) final ServletRequest request)
       if (span instanceof AgentSpan
           && CallDepthThreadLocalMap.incrementCallDepth(HttpServletRequest.class) == 0) {
         final AgentSpan agentSpan = (AgentSpan) span;
-        ClassloaderServiceNames.maybeSetToSpan(agentSpan);
+        ClassloaderConfigurationOverrides.maybeEnrichSpan(agentSpan);
         return agentSpan;
       }
       return null;

diff --git a/dd-java-agent/instrumentation/tomcat-5.5/src/latestDepTest/groovy/TomcatServer.groovy b/dd-java-agent/instrumentation/tomcat-5.5/src/latestDepTest/groovy/TomcatServer.groovy
@@ -38,7 +38,6 @@ class TomcatServer implements HttpServer {
           return false
         }
       }
-
     setupServlets(servletContext)
 
     (server.host as StandardHost).errorReportValveClass = TomcatServletTest.ErrorHandlerValve.name

diff --git a/dd-java-agent/instrumentation/tomcat-5.5/src/latestDepTest/groovy/TomcatServletTest.groovy b/dd-java-agent/instrumentation/tomcat-5.5/src/latestDepTest/groovy/TomcatServletTest.groovy
@@ -9,6 +9,7 @@ import org.apache.catalina.connector.Request
 import org.apache.catalina.connector.Response
 import org.apache.catalina.startup.Tomcat
 import org.apache.catalina.valves.ErrorReportValve
+import org.apache.tomcat.util.descriptor.web.ContextEnvironment
 import org.apache.tomcat.util.descriptor.web.FilterDef
 import org.apache.tomcat.util.descriptor.web.FilterMap
 
@@ -213,4 +214,32 @@ class TomcatServletClassloaderNamingForkedTest extends TomcatServletTest {
   }
 }
 
+class TomcatServletEnvEntriesTagTest extends TomcatServletTest {
+  def addEntry (context, name, value) {
+    def envEntry = new ContextEnvironment()
+    envEntry.setName(name)
+    envEntry.setValue(value)
+    envEntry.setType("java.lang.String")
+    context.getNamingResources().addEnvironment(envEntry)
+  }
+  @Override
+  protected void setupServlets(Context context) {
+    super.setupServlets(context)
+    addEntry(context, "datadog/tags/custom-tag", "custom-value")
+    addEntry(context, "java:comp/env/datadog/tags/service", "custom-service")
+  }
+
+  @Override
+  String expectedServiceName() {
+    "custom-service"
+  }
+
+  @Override
+  Map<String, Serializable> expectedExtraServerTags(ServerEndpoint endpoint) {
+    super.expectedExtraServerTags(endpoint) + ["custom-tag": "custom-value"] as Map<String, Serializable>
+  }
+}
+
+
+
 
diff --git a/dd-java-agent/instrumentation/tomcat-5.5/src/test/groovy/TomcatServletTest.groovy b/dd-java-agent/instrumentation/tomcat-5.5/src/test/groovy/TomcatServletTest.groovy
@@ -1,3 +1,11 @@
+import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CUSTOM_EXCEPTION
+import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR
+import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
+import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
+import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT
+import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.TIMEOUT_ERROR
+import static org.junit.Assume.assumeTrue
+
 import com.google.common.io.Files
 import datadog.trace.agent.test.asserts.TraceAssert
 import datadog.trace.agent.test.base.HttpServer
@@ -18,14 +26,6 @@ import org.apache.coyote.http11.Http11BaseProtocol
 import javax.servlet.Servlet
 import javax.servlet.ServletException
 
-import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CUSTOM_EXCEPTION
-import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR
-import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
-import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
-import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT
-import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.TIMEOUT_ERROR
-import static org.junit.Assume.assumeTrue
-
 abstract class TomcatServletTest extends AbstractServletTest<Embedded, Context> {
 
   class TomcatServer implements HttpServer {
@@ -187,7 +187,7 @@ abstract class TomcatServletTest extends AbstractServletTest<Embedded, Context>
           childOfPrevious()
           tags {
             "component" "java-web-servlet-response"
-            if ({isDataStreamsEnabled()}) {
+            if ({ isDataStreamsEnabled() }) {
               "$DDTags.PATHWAY_HASH" { String }
             }
             defaultTags()
@@ -227,8 +227,8 @@ abstract class TomcatServletTest extends AbstractServletTest<Embedded, Context>
     if (endpoint.throwsException) {
       // Exception classes get wrapped in ServletException
       ["error.message": { endpoint == EXCEPTION ? "Servlet execution threw an exception" : it == endpoint.body },
-        "error.type": { it == ServletException.name || it == InputMismatchException.name },
-        "error.stack": String]
+        "error.type"   : { it == ServletException.name || it == InputMismatchException.name },
+        "error.stack"  : String]
     } else {
       Collections.emptyMap()
     }

diff --git a/...src/main/java/datadog/trace/instrumentation/tomcat9/WebappClassLoaderInstrumentation.java b/...src/main/java/datadog/trace/instrumentation/tomcat9/WebappClassLoaderInstrumentation.java
@@ -1,16 +1,21 @@
 package datadog.trace.instrumentation.tomcat9;
 
 import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
+import static datadog.trace.api.ClassloaderConfigurationOverrides.DATADOG_TAGS_JNDI_PREFIX;
+import static datadog.trace.api.ClassloaderConfigurationOverrides.DATADOG_TAGS_PREFIX;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 
 import com.google.auto.service.AutoService;
 import datadog.trace.agent.tooling.Instrumenter;
 import datadog.trace.agent.tooling.InstrumenterModule;
-import datadog.trace.api.naming.ClassloaderServiceNames;
+import datadog.trace.api.ClassloaderConfigurationOverrides;
+import java.util.HashMap;
+import java.util.Map;
 import net.bytebuddy.asm.Advice;
 import org.apache.catalina.Context;
 import org.apache.catalina.WebResourceRoot;
 import org.apache.catalina.loader.WebappClassLoaderBase;
+import org.apache.tomcat.util.descriptor.web.ContextEnvironment;
 
 @AutoService(InstrumenterModule.class)
 public class WebappClassLoaderInstrumentation extends InstrumenterModule.Tracing
@@ -37,10 +42,43 @@ public static void onContextAvailable(
         @Advice.Argument(0) final WebResourceRoot webResourceRoot) {
       // at this moment we have the context set in this classloader, hence its name
       final Context context = webResourceRoot.getContext();
-      if (context != null) {
-        final String contextName = context.getBaseName();
-        if (contextName != null && !contextName.isEmpty()) {
-          ClassloaderServiceNames.addServiceName(classLoader, contextName);
+      if (context == null) {
+        return;
+      }
+      ClassloaderConfigurationOverrides.ContextualInfo info = null;
+
+      final String contextName = context.getBaseName();
+      if (contextName != null && !contextName.isEmpty()) {
+        info = ClassloaderConfigurationOverrides.withPinnedServiceName(classLoader, contextName);
+      }
+      if (context.getNamingResources() != null) {
+        final ContextEnvironment[] envs = context.getNamingResources().findEnvironments();
+        if (envs != null) {
+          final Map<String, String> tags = new HashMap<>();
+          for (final ContextEnvironment env : envs) {
+            // as a limitation here we simplify a lot the logic and we do not try to resolve the
+            // typed value but we just take the string representation. It avoids implementing the
+            // logic to convert to other types
+            // (i.e. long, double) or instrument more deeply tomcat naming
+            if (env.getValue() == null || env.getValue().isEmpty()) {
+              continue;
+            }
+            String name = null;
+            if (env.getName().startsWith(DATADOG_TAGS_PREFIX)) {
+              name = env.getName().substring(DATADOG_TAGS_PREFIX.length());
+            } else if (env.getName().startsWith(DATADOG_TAGS_JNDI_PREFIX)) {
+              name = env.getName().substring(DATADOG_TAGS_JNDI_PREFIX.length());
+            }
+            if (name != null && !name.isEmpty()) {
+              tags.put(name, env.getValue());
+            }
+          }
+          if (!tags.isEmpty()) {
+            if (info == null) {
+              info = ClassloaderConfigurationOverrides.maybeCreateContextualInfo(classLoader);
+            }
+            tags.forEach(info::addTag);
+          }
         }
       }
     }
-Original file line number
+Diff line change
@@ Expand Up / @@ -38,7 +38,6 @@ class TomcatServer implements HttpServer { @@
               return false
             }
           }
         setupServlets(servletContext)
         (server.host as StandardHost).errorReportValveClass = TomcatServletTest.ErrorHandlerValve.name
@@ Expand Down @@