From 53010f8aef561cb75febb8f5af0b8e5854bd48dc Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Fri, 13 Dec 2024 14:41:16 +0100 Subject: [PATCH 01/20] Use env-entry to add tags per webapp deployment --- .../jbossmodules/ModuleInstrumentation.java | 5 +- .../LibertyServerInstrumentation.java | 4 +- ...readContextClassloaderInstrumentation.java | 5 +- .../LibertyServerInstrumentation.java | 4 +- .../servlet2/Servlet2Advice.java | 4 +- .../servlet3/Servlet3Advice.java | 4 +- .../servlet3/Servlet3Decorator.java | 4 +- .../JakartaServletInstrumentation.java | 4 +- .../latestDepTest/groovy/TomcatServer.groovy | 2 +- .../groovy/TomcatServletTest.groovy | 29 +++ .../src/test/groovy/TomcatServletTest.groovy | 22 +-- .../WebappClassLoaderInstrumentation.java | 45 ++++- .../instrumentation/wildfly-9/build.gradle | 181 ++++++++++++++++++ ...nvEntryInjectionSourceInstrumentation.java | 44 +++++ ...urceReferenceProcessorInstrumentation.java | 76 ++++++++ .../src/test/groovy/EmbeddedWildfly.groovy | 57 ++++++ .../src/test/groovy/WildFlyForkedTest.groovy | 81 ++++++++ .../test/java/test/JakartaTestServlet.java | 12 ++ .../java/test/ModulePatchInstrumentation.java | 43 +++++ .../src/test/java/test/TestServlet.java | 12 ++ .../src/test/resources/WEB-INF/web.xml | 24 +++ .../java/datadog/trace/core/CoreTracer.java | 25 ++- .../datadog/trace/core/DDSpanContext.java | 2 +- .../ClassloaderConfigurationOverrides.java | 113 +++++++++++ .../api/naming/ClassloaderServiceNames.java | 76 -------- .../api/naming/v0/MessagingNamingV0.java | 12 +- settings.gradle | 1 + 27 files changed, 773 insertions(+), 118 deletions(-) create mode 100644 dd-java-agent/instrumentation/wildfly-9/build.gradle create mode 100644 dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/EnvEntryInjectionSourceInstrumentation.java create mode 100644 dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/ResourceReferenceProcessorInstrumentation.java create mode 100644 dd-java-agent/instrumentation/wildfly-9/src/test/groovy/EmbeddedWildfly.groovy create mode 100644 dd-java-agent/instrumentation/wildfly-9/src/test/groovy/WildFlyForkedTest.groovy create mode 100644 dd-java-agent/instrumentation/wildfly-9/src/test/java/test/JakartaTestServlet.java create mode 100644 dd-java-agent/instrumentation/wildfly-9/src/test/java/test/ModulePatchInstrumentation.java create mode 100644 dd-java-agent/instrumentation/wildfly-9/src/test/java/test/TestServlet.java create mode 100644 dd-java-agent/instrumentation/wildfly-9/src/test/resources/WEB-INF/web.xml create mode 100644 internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java delete mode 100644 internal-api/src/main/java/datadog/trace/api/naming/ClassloaderServiceNames.java diff --git a/dd-java-agent/instrumentation/jboss-modules/src/main/java/datadog/trace/instrumentation/jbossmodules/ModuleInstrumentation.java b/dd-java-agent/instrumentation/jboss-modules/src/main/java/datadog/trace/instrumentation/jbossmodules/ModuleInstrumentation.java index 39c07bebdb0..300c33bb788 100644 --- a/dd-java-agent/instrumentation/jboss-modules/src/main/java/datadog/trace/instrumentation/jbossmodules/ModuleInstrumentation.java +++ b/dd-java-agent/instrumentation/jboss-modules/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,8 @@ 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.addContextualInfo( + module.getClassLoader(), new ClassloaderConfigurationOverrides.ContextualInfo(name)); } } } diff --git a/dd-java-agent/instrumentation/liberty-20/src/main/java/datadog/trace/instrumentation/liberty20/LibertyServerInstrumentation.java b/dd-java-agent/instrumentation/liberty-20/src/main/java/datadog/trace/instrumentation/liberty20/LibertyServerInstrumentation.java index 87960ac1940..c6fdef889ab 100644 --- a/dd-java-agent/instrumentation/liberty-20/src/main/java/datadog/trace/instrumentation/liberty20/LibertyServerInstrumentation.java +++ b/dd-java-agent/instrumentation/liberty-20/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/dd-java-agent/instrumentation/liberty-20/src/main/java/datadog/trace/instrumentation/liberty20/ThreadContextClassloaderInstrumentation.java b/dd-java-agent/instrumentation/liberty-20/src/main/java/datadog/trace/instrumentation/liberty20/ThreadContextClassloaderInstrumentation.java index bac66e2ef5a..64eaa219196 100644 --- a/dd-java-agent/instrumentation/liberty-20/src/main/java/datadog/trace/instrumentation/liberty20/ThreadContextClassloaderInstrumentation.java +++ b/dd-java-agent/instrumentation/liberty-20/src/main/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,8 @@ 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.addContextualInfo( + self, new ClassloaderConfigurationOverrides.ContextualInfo(name)); } } } diff --git a/dd-java-agent/instrumentation/liberty-23/src/main/java/datadog/trace/instrumentation/liberty23/LibertyServerInstrumentation.java b/dd-java-agent/instrumentation/liberty-23/src/main/java/datadog/trace/instrumentation/liberty23/LibertyServerInstrumentation.java index 9591f2d50e1..bc101255856 100644 --- a/dd-java-agent/instrumentation/liberty-23/src/main/java/datadog/trace/instrumentation/liberty23/LibertyServerInstrumentation.java +++ b/dd-java-agent/instrumentation/liberty-23/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/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java index 30feb834777..85570d6d7f8 100644 --- a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java +++ b/dd-java-agent/instrumentation/servlet/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/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java index fa6018ae060..0bf2807e9f8 100644 --- a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java +++ b/dd-java-agent/instrumentation/servlet/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/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java index 41964668659..f36907c76d9 100644 --- a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java +++ b/dd-java-agent/instrumentation/servlet/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/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaServletInstrumentation.java b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaServletInstrumentation.java index b038d43c78f..d60711b404c 100644 --- a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaServletInstrumentation.java +++ b/dd-java-agent/instrumentation/servlet/request-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 index 0e419a7b3cd..9495874d618 100644 --- 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 @@ -5,6 +5,7 @@ import org.apache.catalina.core.StandardHost import org.apache.catalina.startup.Tomcat import org.apache.tomcat.JarScanFilter import org.apache.tomcat.JarScanType +import org.apache.tomcat.util.descriptor.web.ContextEnvironment class TomcatServer implements HttpServer { def port = 0 @@ -38,7 +39,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 index 3569246fd0a..5b3f05d15db 100644 --- 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 expectedExtraServerTags(ServerEndpoint endpoint) { + super.expectedExtraServerTags(endpoint) + ["custom-tag": "custom-value"] as Map + } +} + + + 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 index cc7f79e3309..c7f80b84a67 100644 --- 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 { class TomcatServer implements HttpServer { @@ -187,7 +187,7 @@ abstract class TomcatServletTest extends AbstractServletTest 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 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/dd-java-agent/instrumentation/tomcat-classloading-9/src/main/java/datadog/trace/instrumentation/tomcat9/WebappClassLoaderInstrumentation.java b/dd-java-agent/instrumentation/tomcat-classloading-9/src/main/java/datadog/trace/instrumentation/tomcat9/WebappClassLoaderInstrumentation.java index 2980565bec8..89fd2c6de88 100644 --- a/dd-java-agent/instrumentation/tomcat-classloading-9/src/main/java/datadog/trace/instrumentation/tomcat9/WebappClassLoaderInstrumentation.java +++ b/dd-java-agent/instrumentation/tomcat-classloading-9/src/main/java/datadog/trace/instrumentation/tomcat9/WebappClassLoaderInstrumentation.java @@ -1,16 +1,19 @@ 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 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,12 +40,44 @@ 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 = new ClassloaderConfigurationOverrides.ContextualInfo(contextName); + } + if (context.getNamingResources() != null) { + final ContextEnvironment[] envs = context.getNamingResources().findEnvironments(); + if (envs != null) { + if (info == null) { + info = new ClassloaderConfigurationOverrides.ContextualInfo(null); + } + 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()) { + info.addTag(name, env.getValue()); + } + } } } + if (info != null) { + ClassloaderConfigurationOverrides.addContextualInfo(classLoader, info); + } } } } diff --git a/dd-java-agent/instrumentation/wildfly-9/build.gradle b/dd-java-agent/instrumentation/wildfly-9/build.gradle new file mode 100644 index 00000000000..48623774bf1 --- /dev/null +++ b/dd-java-agent/instrumentation/wildfly-9/build.gradle @@ -0,0 +1,181 @@ +ext { + minJavaVersionForTests = JavaVersion.VERSION_11 + latestDepTestMinJavaVersionForTests = JavaVersion.VERSION_17 + latestDepForkedTestMinJavaVersionForTests = JavaVersion.VERSION_17 +} + +repositories { + ivy { + url 'https://download.jboss.org/' + patternLayout { + artifact '/[organisation]/[revision]/[module]/[organisation]-[module]-[revision].[ext]' + metadataSources { + artifact() + } + } + } + ivy { + url 'https://github.com/wildfly' + patternLayout { + artifact '/[organisation]/releases/download/[revision]/[organisation]-[revision].[ext]' + metadataSources { + artifact() + } + } + } +} + +muzzle { + pass { + group = 'org.wildfly' + module = 'wildfly-ee' + versions = '[9.0.0.Final,)' + } +} + +apply from: "$rootDir/gradle/java.gradle" + +addTestSuiteForDir("latestDepTest", "test") +addTestSuiteExtendingForDir("latestDepForkedTest", "latestDepTest", "test") + +configurations { + wildflyTest + wildflyLatestDepTest + wildflyLatestPoll { + canBeResolved = true + } +} + +dependencies { + compileOnly group: 'org.wildfly', name: 'wildfly-ee', version: '9.0.0.Final' + + testImplementation group: 'javax.servlet', name: 'javax.servlet-api', version: '3.0.1' + testImplementation group: 'jakarta.servlet', name: 'jakarta.servlet-api', version: '6.0.0' + + testImplementation group: 'org.wildfly.core', name: 'wildfly-embedded', version: '21.1.0.Final' + testImplementation group: 'org.wildfly.core', name: 'wildfly-server', version: '21.1.0.Final' + testImplementation group: 'org.jboss.shrinkwrap', name: 'shrinkwrap-api', version: '1.2.6' + + testRuntimeOnly project(':dd-java-agent:instrumentation:servlet:request-3') + testRuntimeOnly project(':dd-java-agent:instrumentation:jboss-modules') + testRuntimeOnly project(':dd-java-agent:instrumentation:undertow:undertow-2.0') + testRuntimeOnly project(':dd-java-agent:instrumentation:undertow:undertow-2.2') + testRuntimeOnly group: 'org.jboss.shrinkwrap', name: 'shrinkwrap-spi', version: '1.2.6' + testRuntimeOnly group: 'org.jboss.shrinkwrap', name: 'shrinkwrap-impl-base', version: '1.2.6' + + wildflyTest "wildfly:servlet:21.0.0.Final@zip" + + latestDepTestImplementation group: 'org.wildfly.core', name: 'wildfly-embedded', version: '+' + latestDepTestImplementation group: 'org.wildfly.core', name: 'wildfly-server', version: '+' + wildflyLatestPoll group: 'org.wildfly', name: 'wildfly-dist', version: '+' + + configurations.wildflyLatestPoll.resolve() + def latestWildflyVersion = configurations.wildflyLatestPoll.resolvedConfiguration.getResolvedArtifacts().find { + it.name == "wildfly-dist" + }.moduleVersion.id.version + wildflyLatestDepTest "wildfly:wildfly:$latestWildflyVersion@zip" + latestDepForkedTest { + configure { + jvmArgs += ["-Dtest.jboss.home=$buildDir/wildfly-${latestWildflyVersion}"] + } + } + + + latestDepTestRuntimeOnly project(':dd-java-agent:instrumentation:servlet:request-5') +} + + +def extractWildfly(config, zipFileNamePrefix, sync) { + delete(fileTree(buildDir).include("wildfly-*/standalone/deployments/**")) + + def zipPath = config.find { + it.name.startsWith(zipFileNamePrefix) + } + if (zipPath != null) { + def zipFile = file(zipPath) + def outputDir = file("${buildDir}") + + sync.from zipTree(zipFile) + sync.into outputDir + } else { + throw new GradleException("Can't find server zip file that starts with: " + zipFileNamePrefix) + } +} + + +tasks.register("extractWildfly", Copy) { + dependsOn configurations.wildflyTest + mustRunAfter tasks.compileTestGroovy + extractWildfly(configurations.wildflyTest, "servlet", it) + + // When tests are disabled this would still be run, so disable this manually + onlyIf { !project.rootProject.hasProperty("skipTests") } +} + +tasks.register("extractLatestWildfly", Copy) { + dependsOn configurations.wildflyLatestDepTest + mustRunAfter tasks.compileLatestDepTestGroovy + mustRunAfter tasks.compileLatestDepForkedTestGroovy + mustRunAfter tasks.compileLatestDepTestJava + mustRunAfter tasks.compileLatestDepForkedTestJava + mustRunAfter tasks.compileJava + extractWildfly(configurations.wildflyLatestDepTest, "wildfly", it) + + // When tests are disabled this would still be run, so disable this manually + onlyIf { !project.rootProject.hasProperty("skipTests") } +} + +tasks.named("forkedTest").configure { + dependsOn 'extractWildfly' +} +tasks.named("latestDepForkedTest").configure { + dependsOn 'extractLatestWildfly' +} + +compileTestGroovy.configure { + javaLauncher = getJavaLauncherFor(11) +} + +[compileLatestDepTestGroovy, compileLatestDepForkedTestGroovy].each { + it.configure { + javaLauncher = getJavaLauncherFor(17) + } +} +compileTestJava.configure { + it.configure { + setJavaVersion(it, 11) + sourceCompatibility = JavaVersion.VERSION_1_8 + targetCompatibility = JavaVersion.VERSION_1_8 + } +} + +[compileLatestDepTestJava, compileLatestDepForkedTestJava].each { + it.configure { + setJavaVersion(it, 17) + sourceCompatibility = JavaVersion.VERSION_1_8 + targetCompatibility = JavaVersion.VERSION_1_8 + } +} + +processTestResources { + filesMatching('**/WEB-INF/web.xml') { + expand( + 'servletClass': 'test.TestServlet', + ) + } +} +[processLatestDepTestResources, processLatestDepForkedTestResources].each { + it.filesMatching('**/WEB-INF/web.xml') { + expand( + 'servletClass': 'test.JakartaTestServlet', + ) + } +} + +forkedTest { + configure { + jvmArgs += ["-Dtest.jboss.home=$buildDir/wildfly-servlet-21.0.0.Final"] + } +} + + diff --git a/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/EnvEntryInjectionSourceInstrumentation.java b/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/EnvEntryInjectionSourceInstrumentation.java new file mode 100644 index 00000000000..e04b95cfef6 --- /dev/null +++ b/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/EnvEntryInjectionSourceInstrumentation.java @@ -0,0 +1,44 @@ +package datadog.trace.instrumentation.wildfly; + +import static net.bytebuddy.matcher.ElementMatchers.isConstructor; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.bootstrap.InstrumentationContext; +import java.util.Collections; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import org.jboss.as.ee.component.EnvEntryInjectionSource; + +@AutoService(InstrumenterModule.class) +public class EnvEntryInjectionSourceInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForSingleType { + public EnvEntryInjectionSourceInstrumentation() { + super("wildfly", "jee-env-entry"); + } + + @Override + public String instrumentedType() { + return "org.jboss.as.ee.component.EnvEntryInjectionSource"; + } + + @Override + public Map contextStore() { + return Collections.singletonMap( + "org.jboss.as.ee.component.EnvEntryInjectionSource", Object.class.getName()); + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice(isConstructor(), getClass().getName() + "$ConstructorAdvice"); + } + + public static class ConstructorAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + public static void onExit( + @Advice.This final EnvEntryInjectionSource self, @Advice.Argument(0) final Object value) { + InstrumentationContext.get(EnvEntryInjectionSource.class, Object.class).put(self, value); + } + } +} diff --git a/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/ResourceReferenceProcessorInstrumentation.java b/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/ResourceReferenceProcessorInstrumentation.java new file mode 100644 index 00000000000..b4204290728 --- /dev/null +++ b/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/ResourceReferenceProcessorInstrumentation.java @@ -0,0 +1,76 @@ +package datadog.trace.instrumentation.wildfly; + +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.named; + +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.bootstrap.ContextStore; +import datadog.trace.bootstrap.InstrumentationContext; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import org.jboss.as.ee.component.BindingConfiguration; +import org.jboss.as.ee.component.EnvEntryInjectionSource; + +@AutoService(InstrumenterModule.class) +public class ResourceReferenceProcessorInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForSingleType { + public ResourceReferenceProcessorInstrumentation() { + super("wildfly", "jee-env-entry"); + } + + public ResourceReferenceProcessorInstrumentation( + String instrumentationName, String... additionalNames) { + super(instrumentationName, additionalNames); + } + + @Override + public String instrumentedType() { + return "org.jboss.as.ee.component.deployers.ResourceReferenceProcessor"; + } + + @Override + public Map contextStore() { + return Collections.singletonMap( + "org.jboss.as.ee.component.EnvEntryInjectionSource", Object.class.getName()); + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + isMethod().and(named("getEnvironmentEntries")), + getClass().getName() + "$GetEnvironmentEntriesAdvice"); + } + + public static class GetEnvironmentEntriesAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + public static void onExit( + @Advice.Argument(value = 1) final ClassLoader classLoader, + @Advice.Return final List configurations) { + ClassloaderConfigurationOverrides.ContextualInfo info = + ClassloaderConfigurationOverrides.getOrAddEmpty(classLoader); + ContextStore contextStore = + InstrumentationContext.get(EnvEntryInjectionSource.class, Object.class); + for (BindingConfiguration bindingConfiguration : configurations) { + if (bindingConfiguration.getSource() instanceof EnvEntryInjectionSource) { + final Object value = + contextStore.get((EnvEntryInjectionSource) bindingConfiguration.getSource()); + if (value != null + && bindingConfiguration + .getName() + .startsWith(ClassloaderConfigurationOverrides.DATADOG_TAGS_JNDI_PREFIX)) { + info.addTag( + bindingConfiguration + .getName() + .substring(ClassloaderConfigurationOverrides.DATADOG_TAGS_JNDI_PREFIX.length()), + value); + } + } + } + } + } +} diff --git a/dd-java-agent/instrumentation/wildfly-9/src/test/groovy/EmbeddedWildfly.groovy b/dd-java-agent/instrumentation/wildfly-9/src/test/groovy/EmbeddedWildfly.groovy new file mode 100644 index 00000000000..c4e9f78d56a --- /dev/null +++ b/dd-java-agent/instrumentation/wildfly-9/src/test/groovy/EmbeddedWildfly.groovy @@ -0,0 +1,57 @@ +import datadog.trace.agent.test.utils.OkHttpUtils +import datadog.trace.agent.test.utils.PortUtils +import okhttp3.HttpUrl +import okhttp3.Request +import org.jboss.shrinkwrap.api.exporter.ExplodedExporter +import org.jboss.shrinkwrap.api.spec.WebArchive +import org.wildfly.core.embedded.Configuration +import org.wildfly.core.embedded.EmbeddedProcessFactory +import org.wildfly.core.embedded.StandaloneServer +import spock.util.concurrent.PollingConditions + +import java.nio.file.Files +import java.nio.file.Path +import java.nio.file.Paths +import java.util.concurrent.TimeUnit + +class EmbeddedWildfly { + + private final StandaloneServer server + private final Path home + private final int port + + EmbeddedWildfly(String jbossHome, int httpPort) { + home = Paths.get(jbossHome) + port = httpPort + def conf = Configuration.Builder + .of(home) + .addCommandArgument("-Djboss.management.http.port=0") + .addCommandArgument("-Djboss.management.https.port=0") + .addCommandArgument("-Djboss.ajp.port=0") + .addCommandArgument("-Djboss.http.port=$httpPort") + .addCommandArgument("-Djboss.https.port=0") + .addSystemPackages("datadog", "groovy") + .build() + server = EmbeddedProcessFactory.createStandaloneServer(conf) + } + + void start() { + server.start() + PortUtils.waitForPortToOpen(port, 30, TimeUnit.SECONDS) + } + + void stop() { + server.stop() + } + + void deploy(WebArchive webArchive) { + def deploymentPath = home.resolve("standalone/deployments") + webArchive.as(ExplodedExporter).exportExploded(deploymentPath.toFile(), "test.war") + Files.createFile(deploymentPath.resolve("test.war.dodeploy")) + new PollingConditions(timeout: 30, delay: 2).eventually { + def request = new Request.Builder().url(HttpUrl.get("http://localhost:$port/test")).build() + def call = OkHttpUtils.client().newCall(request) + assert call.execute().code() == 200 + } + } +} diff --git a/dd-java-agent/instrumentation/wildfly-9/src/test/groovy/WildFlyForkedTest.groovy b/dd-java-agent/instrumentation/wildfly-9/src/test/groovy/WildFlyForkedTest.groovy new file mode 100644 index 00000000000..969d831b5be --- /dev/null +++ b/dd-java-agent/instrumentation/wildfly-9/src/test/groovy/WildFlyForkedTest.groovy @@ -0,0 +1,81 @@ +import datadog.trace.agent.test.base.WithHttpServer +import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions +import datadog.trace.agent.test.utils.OkHttpUtils +import datadog.trace.api.DDSpanTypes +import datadog.trace.bootstrap.instrumentation.api.InstrumentationTags +import datadog.trace.bootstrap.instrumentation.api.Tags +import okhttp3.HttpUrl +import okhttp3.Request +import org.jboss.shrinkwrap.api.ShrinkWrap +import org.jboss.shrinkwrap.api.spec.WebArchive +import test.JakartaTestServlet +import test.TestServlet + +class WildFlyForkedTest extends WithHttpServer implements TestingGenericHttpNamingConventions.ServerV0 { + @Override + EmbeddedWildfly startServer(int port) { + // create the archive + def war = ShrinkWrap.create(WebArchive) + war.setWebXML(getClass().getResource("/WEB-INF/web.xml")) + .addClass(TestServlet) // for wildfly 21 (EE 8) + .addClass(JakartaTestServlet) // for latestDep + + def server = new EmbeddedWildfly(System.getProperty("test.jboss.home"), port) + server.start() + server.deploy(war) + server + } + + @Override + void stopServer(EmbeddedWildfly embeddedWildfly) { + embeddedWildfly?.stop() + } + + @Override + protected void configurePreAgent() { + super.configurePreAgent() + // otherwise there are differences in setting the resource name across wildfly versions + injectSysConfig("undertow.legacy.tracing.enabled", "false") + } + + def "should have service name and tags set"() { + setup: + TEST_WRITER.clear() + when: + def request = new Request.Builder().url(HttpUrl.get(server.address()).resolve("/test/hello")).build() + def call = OkHttpUtils.client().newCall(request) + def response = call.execute() + then: + assert response.code() == 200 + assertTraces(1, { + trace(1) { + span { + serviceName "custom-service" + operationName "servlet.request" + resourceName "GET /test/hello" + spanType DDSpanTypes.HTTP_SERVER + errored false + parent() + + tags { + "$Tags.COMPONENT" "undertow-http-server" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + "$Tags.PEER_PORT" Integer + "$Tags.PEER_HOST_IPV4" { String } + "$Tags.HTTP_CLIENT_IP" { String } + "$Tags.HTTP_HOSTNAME" address.host + "$Tags.HTTP_URL" { String } + "$Tags.HTTP_METHOD" "GET" + "$Tags.HTTP_STATUS" 200 + "$Tags.HTTP_USER_AGENT" String + "$Tags.HTTP_ROUTE" { true } + "$InstrumentationTags.SERVLET_CONTEXT" "/test" + "$InstrumentationTags.SERVLET_PATH" "/hello" + "custom-metric" 1983 + defaultTags(true) + } + } + } + }) + } +} diff --git a/dd-java-agent/instrumentation/wildfly-9/src/test/java/test/JakartaTestServlet.java b/dd-java-agent/instrumentation/wildfly-9/src/test/java/test/JakartaTestServlet.java new file mode 100644 index 00000000000..2af90032412 --- /dev/null +++ b/dd-java-agent/instrumentation/wildfly-9/src/test/java/test/JakartaTestServlet.java @@ -0,0 +1,12 @@ +package test; + +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +public class JakartaTestServlet extends HttpServlet { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) { + resp.setStatus(HttpServletResponse.SC_OK); + } +} diff --git a/dd-java-agent/instrumentation/wildfly-9/src/test/java/test/ModulePatchInstrumentation.java b/dd-java-agent/instrumentation/wildfly-9/src/test/java/test/ModulePatchInstrumentation.java new file mode 100644 index 00000000000..ca2ae4a54a9 --- /dev/null +++ b/dd-java-agent/instrumentation/wildfly-9/src/test/java/test/ModulePatchInstrumentation.java @@ -0,0 +1,43 @@ +package test; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import java.net.URL; +import java.util.Collections; +import java.util.Enumeration; +import net.bytebuddy.asm.Advice; +import org.jboss.modules.Module; + +@AutoService(InstrumenterModule.class) +public class ModulePatchInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForSingleType { + + public ModulePatchInstrumentation() { + super("jboss-module-patch"); + } + + @Override + public String instrumentedType() { + return "org.jboss.modules.Module"; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice(named("getResources"), getClass().getName() + "$SystemResourcesAdvice"); + } + + public static class SystemResourcesAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + public static void onExit( + @Advice.This Module self, + @Advice.Argument(0) String name, + @Advice.Return(readOnly = false) Enumeration ret) { + if (self.getName().endsWith(".jdk")) { + ret = Collections.emptyEnumeration(); + } + } + } +} diff --git a/dd-java-agent/instrumentation/wildfly-9/src/test/java/test/TestServlet.java b/dd-java-agent/instrumentation/wildfly-9/src/test/java/test/TestServlet.java new file mode 100644 index 00000000000..551fa4baf4b --- /dev/null +++ b/dd-java-agent/instrumentation/wildfly-9/src/test/java/test/TestServlet.java @@ -0,0 +1,12 @@ +package test; + +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +public class TestServlet extends HttpServlet { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) { + resp.setStatus(HttpServletResponse.SC_OK); + } +} diff --git a/dd-java-agent/instrumentation/wildfly-9/src/test/resources/WEB-INF/web.xml b/dd-java-agent/instrumentation/wildfly-9/src/test/resources/WEB-INF/web.xml new file mode 100644 index 00000000000..bc4afc2b00a --- /dev/null +++ b/dd-java-agent/instrumentation/wildfly-9/src/test/resources/WEB-INF/web.xml @@ -0,0 +1,24 @@ + + + + + test + ${servletClass} + + + + test + /* + + + + java:comp/env/datadog/tags/service + java.lang.String + custom-service + + + java:comp/env/datadog/tags/custom-metric + java.lang.Integer + 1983 + + diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 437c397883b..efe10f3f542 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -18,6 +18,7 @@ import datadog.communication.ddagent.SharedCommunicationObjects; import datadog.communication.monitor.Monitoring; import datadog.communication.monitor.Recording; +import datadog.trace.api.ClassloaderConfigurationOverrides; import datadog.trace.api.Config; import datadog.trace.api.DDSpanId; import datadog.trace.api.DDTraceId; @@ -40,7 +41,6 @@ import datadog.trace.api.interceptor.TraceInterceptor; import datadog.trace.api.internal.TraceSegment; import datadog.trace.api.metrics.SpanMetricRegistry; -import datadog.trace.api.naming.ClassloaderServiceNames; import datadog.trace.api.naming.SpanNaming; import datadog.trace.api.remoteconfig.ServiceNameCollector; import datadog.trace.api.sampling.PrioritySampling; @@ -139,8 +139,10 @@ public static CoreTracerBuilder builder() { /** Tracer start time in nanoseconds measured up to a millisecond accuracy */ private final long startTimeNano; + /** Nanosecond ticks value at tracer start */ private final long startNanoTicks; + /** How often should traced threads check clock ticks against the wall clock */ private final long clockSyncPeriod; @@ -149,6 +151,7 @@ public static CoreTracerBuilder builder() { /** Last time (in nanosecond ticks) the clock was checked for drift */ private volatile long lastSyncTicks; + /** Nanosecond offset to counter clock drift */ private volatile long counterDrift; @@ -160,10 +163,13 @@ public static CoreTracerBuilder builder() { /** Default service name if none provided on the trace or span */ final String serviceName; + /** Writer is an charge of reporting traces and spans to the desired endpoint */ final Writer writer; + /** Sampler defines the sampling policy in order to reduce the number of traces for instance */ final Sampler initialSampler; + /** Scope manager is in charge of managing the scopes from which spans are created */ final ContinuableScopeManager scopeManager; @@ -171,10 +177,13 @@ public static CoreTracerBuilder builder() { /** Initial static configuration associated with the tracer. */ final Config initialConfig; + /** Maintains dynamic configuration associated with the tracer */ private final DynamicConfig dynamicConfig; + /** A set of tags that are added only to the application's root span */ private final Map localRootSpanTags; + /** A set of tags that are added to every span */ private final Map defaultSpanTags; @@ -1589,10 +1598,16 @@ private DDSpanContext buildSpanContext() { if (serviceName == null) { serviceName = traceConfig.getPreferredServiceName(); } + ClassloaderConfigurationOverrides.ContextualInfo contextualInfo = + ClassloaderConfigurationOverrides.maybeGetContextualInfo(); + Map contextualTags = null; if (serviceName == null && parentServiceName == null) { // in this case we have a local root without service name. We can try to see if we can find // one from the thread context classloader - serviceName = ClassloaderServiceNames.maybeGetForCurrentThread(); + if (contextualInfo != null) { + serviceName = contextualInfo.getServiceName(); + contextualTags = contextualInfo.getTags(); + } } if (serviceName == null) { // it could be on the initial snapshot but may be overridden to null and service name @@ -1609,7 +1624,8 @@ private DDSpanContext buildSpanContext() { mergedTracerTags.size() + (null == tags ? 0 : tags.size()) + (null == coreTags ? 0 : coreTags.size()) - + (null == rootSpanTags ? 0 : rootSpanTags.size()); + + (null == rootSpanTags ? 0 : rootSpanTags.size()) + + (null == contextualTags ? 0 : contextualTags.size()); if (builderRequestContextDataAppSec != null) { requestContextDataAppSec = builderRequestContextDataAppSec; @@ -1655,6 +1671,9 @@ private DDSpanContext buildSpanContext() { context.setAllTags(tags); context.setAllTags(coreTags); context.setAllTags(rootSpanTags); + if (contextualTags != null) { + context.setAllTags(contextualTags); + } return context; } } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index d8092874bb9..93ff8692004 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -765,7 +765,7 @@ void setAllTags(final Map map) { } } - void unsafeSetTag(final String tag, final Object value) { + public void unsafeSetTag(final String tag, final Object value) { unsafeTags.put(tag, value); } diff --git a/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java b/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java new file mode 100644 index 00000000000..335e2c7fa79 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java @@ -0,0 +1,113 @@ +package datadog.trace.api; + +import datadog.trace.api.config.GeneralConfig; +import datadog.trace.api.env.CapturedEnvironment; +import datadog.trace.api.remoteconfig.ServiceNameCollector; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.WeakHashMap; +import java.util.function.Function; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +public class ClassloaderConfigurationOverrides { + public static final String DATADOG_TAGS_PREFIX = "datadog/tags/"; + public static final String DATADOG_TAGS_JNDI_PREFIX = "java:comp/env/" + DATADOG_TAGS_PREFIX; + private static final boolean CAN_SPLIT_SERVICE_NAME_BY_DEPLOYMENT = + Config.get().isJeeSplitByDeployment() && !Config.get().isServiceNameSetByUser(); + + private static class Lazy { + private static final ClassloaderConfigurationOverrides INSTANCE = + new ClassloaderConfigurationOverrides(); + } + + public static class ContextualInfo { + private final String serviceName; + private final Map tags = new HashMap<>(); + + public ContextualInfo(String serviceName) { + this.serviceName = serviceName; + } + + public String getServiceName() { + return serviceName; + } + + public void addTag(String name, Object value) { + tags.put(name, value); + } + + public Map getTags() { + return Collections.unmodifiableMap(tags); + } + } + + private static Function EMPTY_CONTEXTUAL_INFO_ADDER = + ignored -> new ContextualInfo(null); + + private final WeakHashMap weakCache = new WeakHashMap<>(); + private final String inferredServiceName = + CapturedEnvironment.get().getProperties().get(GeneralConfig.SERVICE_NAME); + + private ClassloaderConfigurationOverrides() {} + + public static void addContextualInfo( + @Nonnull ClassLoader classLoader, @Nonnull ContextualInfo contextualInfo) { + Lazy.INSTANCE.weakCache.put(classLoader, contextualInfo); + } + + @Nullable + public static ContextualInfo maybeGetContextualInfo(@Nonnull ClassLoader classLoader) { + return Lazy.INSTANCE.weakCache.get(classLoader); + } + + /** + * Fetches the contextual information linked to the current thread's context classloader. + * + * @return a nullable service name. + */ + @Nullable + public static ContextualInfo maybeGetContextualInfo() { + return maybeGetContextualInfo(Thread.currentThread().getContextClassLoader()); + } + + @Nonnull + public static ContextualInfo getOrAddEmpty(@Nonnull ClassLoader classLoader) { + return Lazy.INSTANCE.weakCache.computeIfAbsent(classLoader, EMPTY_CONTEXTUAL_INFO_ADDER); + } + + /** + * Enriches the provided spans according to the service name and tags linked to the current + * thread's classloader contextual information. + * + * @param span a nonnull span + */ + public static void maybeEnrichSpan(@Nonnull final AgentSpan span) { + maybeEnrichSpan(span, Thread.currentThread().getContextClassLoader()); + } + + public static void maybeEnrichSpan( + @Nonnull final AgentSpan span, @Nonnull final ClassLoader classLoader) { + final ContextualInfo contextualInfo = maybeGetContextualInfo(classLoader); + if (contextualInfo == null) { + return; + } + if (CAN_SPLIT_SERVICE_NAME_BY_DEPLOYMENT + && contextualInfo.serviceName != null + && !contextualInfo.getServiceName().isEmpty()) { + final String currentServiceName = span.getServiceName(); + if (currentServiceName == null + || currentServiceName.equals(Lazy.INSTANCE.inferredServiceName)) { + final String serviceName = contextualInfo.getServiceName(); + + span.setServiceName(serviceName); + ServiceNameCollector.get().addService(serviceName); + } + } + for (final Map.Entry entry : contextualInfo.getTags().entrySet()) { + span.setTag(entry.getKey(), entry.getValue()); + } + } +} diff --git a/internal-api/src/main/java/datadog/trace/api/naming/ClassloaderServiceNames.java b/internal-api/src/main/java/datadog/trace/api/naming/ClassloaderServiceNames.java deleted file mode 100644 index c9d73f7df1d..00000000000 --- a/internal-api/src/main/java/datadog/trace/api/naming/ClassloaderServiceNames.java +++ /dev/null @@ -1,76 +0,0 @@ -package datadog.trace.api.naming; - -import datadog.trace.api.Config; -import datadog.trace.api.config.GeneralConfig; -import datadog.trace.api.env.CapturedEnvironment; -import datadog.trace.api.remoteconfig.ServiceNameCollector; -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; -import java.util.WeakHashMap; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; - -public class ClassloaderServiceNames { - private static final boolean ENABLED = - Config.get().isJeeSplitByDeployment() && !Config.get().isServiceNameSetByUser(); - - private static class Lazy { - private static final ClassloaderServiceNames INSTANCE = new ClassloaderServiceNames(); - } - - private final WeakHashMap weakCache = new WeakHashMap<>(); - private final String inferredServiceName = - CapturedEnvironment.get().getProperties().get(GeneralConfig.SERVICE_NAME); - - private ClassloaderServiceNames() {} - - public static void addServiceName(@Nonnull ClassLoader classLoader, @Nonnull String serviceName) { - if (ENABLED) { - Lazy.INSTANCE.weakCache.put(classLoader, serviceName); - } - } - - @Nullable - public static String maybeGet(@Nonnull ClassLoader classLoader) { - if (ENABLED) { - return Lazy.INSTANCE.weakCache.get(classLoader); - } - return null; - } - - /** - * Fetches the service name linked to the current thread's context classloader. - * - * @return a nullable service name. - */ - @Nullable - public static String maybeGetForCurrentThread() { - return maybeGet(Thread.currentThread().getContextClassLoader()); - } - - /** - * Sets the service name to the provided spans according to the service name linked to the current - * thread's classloader. - * - * @param span a nonnull span - */ - public static void maybeSetToSpan(@Nonnull final AgentSpan span) { - maybeSetToSpan(span, Thread.currentThread().getContextClassLoader()); - } - - public static void maybeSetToSpan( - @Nonnull final AgentSpan span, @Nonnull final ClassLoader classLoader) { - if (!ENABLED) { - return; - } - final String currentServiceName = span.getServiceName(); - if (currentServiceName != null - && !currentServiceName.equals(Lazy.INSTANCE.inferredServiceName)) { - return; - } - final String service = maybeGet(classLoader); - if (service != null) { - span.setServiceName(service); - ServiceNameCollector.get().addService(service); - } - } -} diff --git a/internal-api/src/main/java/datadog/trace/api/naming/v0/MessagingNamingV0.java b/internal-api/src/main/java/datadog/trace/api/naming/v0/MessagingNamingV0.java index e7e227fbeb0..60447310128 100644 --- a/internal-api/src/main/java/datadog/trace/api/naming/v0/MessagingNamingV0.java +++ b/internal-api/src/main/java/datadog/trace/api/naming/v0/MessagingNamingV0.java @@ -1,7 +1,7 @@ package datadog.trace.api.naming.v0; +import datadog.trace.api.ClassloaderConfigurationOverrides; import datadog.trace.api.Config; -import datadog.trace.api.naming.ClassloaderServiceNames; import datadog.trace.api.naming.NamingSchema; import datadog.trace.api.remoteconfig.ServiceNameCollector; import java.util.function.Supplier; @@ -16,10 +16,12 @@ private static class ClassloaderDependentNamingSupplier implements Supplier Date: Thu, 2 Jan 2025 09:18:28 +0100 Subject: [PATCH 02/20] fix gradle file --- dd-java-agent/instrumentation/wildfly-9/build.gradle | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/dd-java-agent/instrumentation/wildfly-9/build.gradle b/dd-java-agent/instrumentation/wildfly-9/build.gradle index 48623774bf1..5a9d62f86aa 100644 --- a/dd-java-agent/instrumentation/wildfly-9/build.gradle +++ b/dd-java-agent/instrumentation/wildfly-9/build.gradle @@ -80,7 +80,6 @@ dependencies { } } - latestDepTestRuntimeOnly project(':dd-java-agent:instrumentation:servlet:request-5') } @@ -125,6 +124,11 @@ tasks.register("extractLatestWildfly", Copy) { onlyIf { !project.rootProject.hasProperty("skipTests") } } + +tasks.named("test").configure { + dependsOn 'extractWildfly' +} + tasks.named("forkedTest").configure { dependsOn 'extractWildfly' } @@ -132,6 +136,9 @@ tasks.named("latestDepForkedTest").configure { dependsOn 'extractLatestWildfly' } +tasks.named("latestDepTest").configure { + dependsOn 'extractLatestWildfly' +} compileTestGroovy.configure { javaLauncher = getJavaLauncherFor(11) } From 6db31393f4bbb5ce040d9ca2a4210ce6e447f3d3 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Thu, 2 Jan 2025 09:50:31 +0100 Subject: [PATCH 03/20] Migrate to hasmethodadvice --- .../wildfly/EnvEntryInjectionSourceInstrumentation.java | 2 +- .../wildfly/ResourceReferenceProcessorInstrumentation.java | 2 +- .../src/test/java/test/ModulePatchInstrumentation.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/EnvEntryInjectionSourceInstrumentation.java b/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/EnvEntryInjectionSourceInstrumentation.java index e04b95cfef6..58c6a5275ac 100644 --- a/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/EnvEntryInjectionSourceInstrumentation.java +++ b/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/EnvEntryInjectionSourceInstrumentation.java @@ -13,7 +13,7 @@ @AutoService(InstrumenterModule.class) public class EnvEntryInjectionSourceInstrumentation extends InstrumenterModule.Tracing - implements Instrumenter.ForSingleType { + implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { public EnvEntryInjectionSourceInstrumentation() { super("wildfly", "jee-env-entry"); } diff --git a/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/ResourceReferenceProcessorInstrumentation.java b/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/ResourceReferenceProcessorInstrumentation.java index b4204290728..0494e5693fb 100644 --- a/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/ResourceReferenceProcessorInstrumentation.java +++ b/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/ResourceReferenceProcessorInstrumentation.java @@ -18,7 +18,7 @@ @AutoService(InstrumenterModule.class) public class ResourceReferenceProcessorInstrumentation extends InstrumenterModule.Tracing - implements Instrumenter.ForSingleType { + implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { public ResourceReferenceProcessorInstrumentation() { super("wildfly", "jee-env-entry"); } diff --git a/dd-java-agent/instrumentation/wildfly-9/src/test/java/test/ModulePatchInstrumentation.java b/dd-java-agent/instrumentation/wildfly-9/src/test/java/test/ModulePatchInstrumentation.java index ca2ae4a54a9..305da5d8346 100644 --- a/dd-java-agent/instrumentation/wildfly-9/src/test/java/test/ModulePatchInstrumentation.java +++ b/dd-java-agent/instrumentation/wildfly-9/src/test/java/test/ModulePatchInstrumentation.java @@ -13,7 +13,7 @@ @AutoService(InstrumenterModule.class) public class ModulePatchInstrumentation extends InstrumenterModule.Tracing - implements Instrumenter.ForSingleType { + implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { public ModulePatchInstrumentation() { super("jboss-module-patch"); From fd7030a85ba587c483f30d5122684e8075c99617 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Thu, 2 Jan 2025 12:01:57 +0100 Subject: [PATCH 04/20] exclude classes from coverage --- .../src/test/java/test/ModulePatchInstrumentation.java | 6 ++++++ internal-api/build.gradle | 3 +++ 2 files changed, 9 insertions(+) diff --git a/dd-java-agent/instrumentation/wildfly-9/src/test/java/test/ModulePatchInstrumentation.java b/dd-java-agent/instrumentation/wildfly-9/src/test/java/test/ModulePatchInstrumentation.java index 305da5d8346..84095fa4762 100644 --- a/dd-java-agent/instrumentation/wildfly-9/src/test/java/test/ModulePatchInstrumentation.java +++ b/dd-java-agent/instrumentation/wildfly-9/src/test/java/test/ModulePatchInstrumentation.java @@ -11,6 +11,12 @@ import net.bytebuddy.asm.Advice; import org.jboss.modules.Module; +/** + * This instrumentation is to hack the way the jboss module classloader is loading SPI services. In + * fact, in test we have a classloader different from the one usually used when launching wildfly. + * In particular, we do not want to have SPI load services defined outside the jboss classloader + * module, otherwise this class won't be found afterwards. + */ @AutoService(InstrumenterModule.class) public class ModulePatchInstrumentation extends InstrumenterModule.Tracing implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { diff --git a/internal-api/build.gradle b/internal-api/build.gradle index dea10d40519..f061dd25350 100644 --- a/internal-api/build.gradle +++ b/internal-api/build.gradle @@ -76,6 +76,9 @@ excludedClassesCoverage += [ "datadog.trace.bootstrap.instrumentation.api.AgentTracer.NoopTraceConfig", "datadog.trace.bootstrap.instrumentation.api.AgentTracer.TracerAPI", "datadog.trace.bootstrap.instrumentation.api.Backlog", + "datadog.trace.bootstrap.instrumentation.api.ClassloaderConfigurationOverrides", + "datadog.trace.bootstrap.instrumentation.api.ClassloaderConfigurationOverrides.Lazy", + "datadog.trace.bootstrap.instrumentation.api.ClassloaderConfigurationOverrides.ContextualInfo", "datadog.trace.bootstrap.instrumentation.api.StatsPoint", "datadog.trace.bootstrap.instrumentation.api.Schema", "datadog.trace.bootstrap.instrumentation.api.ScopeSource", From f28ee0692d670e4205b537614ec1523c816cbd66 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Thu, 2 Jan 2025 14:36:19 +0100 Subject: [PATCH 05/20] codenarc --- .../tomcat-5.5/src/latestDepTest/groovy/TomcatServer.groovy | 1 - 1 file changed, 1 deletion(-) 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 index 9495874d618..0edb4a2f4a4 100644 --- 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 @@ -5,7 +5,6 @@ import org.apache.catalina.core.StandardHost import org.apache.catalina.startup.Tomcat import org.apache.tomcat.JarScanFilter import org.apache.tomcat.JarScanType -import org.apache.tomcat.util.descriptor.web.ContextEnvironment class TomcatServer implements HttpServer { def port = 0 From bb3c9cdded002adbe83ec8243fdbb44a844acd58 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Thu, 2 Jan 2025 14:47:47 +0100 Subject: [PATCH 06/20] add more repos --- dd-java-agent/instrumentation/wildfly-9/build.gradle | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dd-java-agent/instrumentation/wildfly-9/build.gradle b/dd-java-agent/instrumentation/wildfly-9/build.gradle index 5a9d62f86aa..e1f7e37985a 100644 --- a/dd-java-agent/instrumentation/wildfly-9/build.gradle +++ b/dd-java-agent/instrumentation/wildfly-9/build.gradle @@ -5,6 +5,9 @@ ext { } repositories { + maven { + url 'https://maven.repository.redhat.com/ga/' + } ivy { url 'https://download.jboss.org/' patternLayout { @@ -26,6 +29,7 @@ repositories { } muzzle { + extraRepository('redhat-ga', 'https://maven.repository.redhat.com/ga/') pass { group = 'org.wildfly' module = 'wildfly-ee' From 1238ea9be9fad682976fb283bdbdc3546d4ce777 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Thu, 2 Jan 2025 15:16:41 +0100 Subject: [PATCH 07/20] jacoco --- internal-api/build.gradle | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal-api/build.gradle b/internal-api/build.gradle index f061dd25350..d563ce7cec7 100644 --- a/internal-api/build.gradle +++ b/internal-api/build.gradle @@ -76,9 +76,6 @@ excludedClassesCoverage += [ "datadog.trace.bootstrap.instrumentation.api.AgentTracer.NoopTraceConfig", "datadog.trace.bootstrap.instrumentation.api.AgentTracer.TracerAPI", "datadog.trace.bootstrap.instrumentation.api.Backlog", - "datadog.trace.bootstrap.instrumentation.api.ClassloaderConfigurationOverrides", - "datadog.trace.bootstrap.instrumentation.api.ClassloaderConfigurationOverrides.Lazy", - "datadog.trace.bootstrap.instrumentation.api.ClassloaderConfigurationOverrides.ContextualInfo", "datadog.trace.bootstrap.instrumentation.api.StatsPoint", "datadog.trace.bootstrap.instrumentation.api.Schema", "datadog.trace.bootstrap.instrumentation.api.ScopeSource", @@ -125,6 +122,9 @@ excludedClassesCoverage += [ "datadog.trace.api.civisibility.CiVisibilityWellKnownTags", "datadog.trace.api.civisibility.InstrumentationBridge", "datadog.trace.api.civisibility.InstrumentationTestBridge", + "datadog.trace.api.ClassloaderConfigurationOverrides", + "datadog.trace.api.ClassloaderConfigurationOverrides.Lazy", + "datadog.trace.api.ClassloaderConfigurationOverrides.ContextualInfo", // POJO "datadog.trace.api.git.GitInfo", "datadog.trace.api.git.GitInfoProvider", From 362ae468edb6d5351187a92e445cfd80be5da1a3 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Thu, 2 Jan 2025 19:06:36 +0100 Subject: [PATCH 08/20] Update internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java Co-authored-by: Bruce Bujon --- .../datadog/trace/api/ClassloaderConfigurationOverrides.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java b/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java index 335e2c7fa79..a4526debd99 100644 --- a/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java +++ b/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java @@ -106,8 +106,6 @@ public static void maybeEnrichSpan( ServiceNameCollector.get().addService(serviceName); } } - for (final Map.Entry entry : contextualInfo.getTags().entrySet()) { - span.setTag(entry.getKey(), entry.getValue()); - } + contextualInfo.getTags().forEach(span::setTag); } } From 50f817858a2841e5389776c9fd9bdd8c0dcd97f8 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Fri, 3 Jan 2025 10:50:36 +0100 Subject: [PATCH 09/20] review --- .../datadog/trace/core/DDSpanContext.java | 2 +- internal-api/build.gradle | 3 -- .../ClassloaderConfigurationOverrides.java | 5 +- ...assloaderConfigurationOverridesTest.groovy | 53 +++++++++++++++++++ 4 files changed, 57 insertions(+), 6 deletions(-) create mode 100644 internal-api/src/test/groovy/datadog/trace/api/ClassloaderConfigurationOverridesTest.groovy diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index 93ff8692004..d8092874bb9 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -765,7 +765,7 @@ void setAllTags(final Map map) { } } - public void unsafeSetTag(final String tag, final Object value) { + void unsafeSetTag(final String tag, final Object value) { unsafeTags.put(tag, value); } diff --git a/internal-api/build.gradle b/internal-api/build.gradle index d563ce7cec7..dea10d40519 100644 --- a/internal-api/build.gradle +++ b/internal-api/build.gradle @@ -122,9 +122,6 @@ excludedClassesCoverage += [ "datadog.trace.api.civisibility.CiVisibilityWellKnownTags", "datadog.trace.api.civisibility.InstrumentationBridge", "datadog.trace.api.civisibility.InstrumentationTestBridge", - "datadog.trace.api.ClassloaderConfigurationOverrides", - "datadog.trace.api.ClassloaderConfigurationOverrides.Lazy", - "datadog.trace.api.ClassloaderConfigurationOverrides.ContextualInfo", // POJO "datadog.trace.api.git.GitInfo", "datadog.trace.api.git.GitInfoProvider", diff --git a/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java b/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java index a4526debd99..7a6a3613d6a 100644 --- a/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java +++ b/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java @@ -15,7 +15,8 @@ public class ClassloaderConfigurationOverrides { public static final String DATADOG_TAGS_PREFIX = "datadog/tags/"; public static final String DATADOG_TAGS_JNDI_PREFIX = "java:comp/env/" + DATADOG_TAGS_PREFIX; - private static final boolean CAN_SPLIT_SERVICE_NAME_BY_DEPLOYMENT = + // not final for testing purposes + static boolean CAN_SPLIT_SERVICE_NAME_BY_DEPLOYMENT = Config.get().isJeeSplitByDeployment() && !Config.get().isServiceNameSetByUser(); private static class Lazy { @@ -44,7 +45,7 @@ public Map getTags() { } } - private static Function EMPTY_CONTEXTUAL_INFO_ADDER = + private static final Function EMPTY_CONTEXTUAL_INFO_ADDER = ignored -> new ContextualInfo(null); private final WeakHashMap weakCache = new WeakHashMap<>(); diff --git a/internal-api/src/test/groovy/datadog/trace/api/ClassloaderConfigurationOverridesTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ClassloaderConfigurationOverridesTest.groovy new file mode 100644 index 00000000000..4a533579d0a --- /dev/null +++ b/internal-api/src/test/groovy/datadog/trace/api/ClassloaderConfigurationOverridesTest.groovy @@ -0,0 +1,53 @@ +package datadog.trace.api + +import datadog.trace.bootstrap.instrumentation.api.AgentSpan +import datadog.trace.test.util.DDSpecification +import spock.lang.Unroll + +class ClassloaderConfigurationOverridesTest extends DDSpecification { + + @Unroll + def 'service name set = #expected when split-by-deployment enabled is #splitByDeploymentEnabled and service name manually set = #spanServiceName and contextual service name is #contextualServiceName'() { + setup: + injectSysConfig("dd.service", "test") + def span = Mock(AgentSpan) + when: + ClassloaderConfigurationOverrides.CAN_SPLIT_SERVICE_NAME_BY_DEPLOYMENT = splitByDeploymentEnabled + if (splitByDeploymentEnabled) { + injectSysConfig("dd.trace.experimental.jee.split-by-deployment", "true") + } + ClassloaderConfigurationOverrides.addContextualInfo(Thread.currentThread().getContextClassLoader(), + new ClassloaderConfigurationOverrides.ContextualInfo(contextualServiceName)) + ClassloaderConfigurationOverrides.maybeEnrichSpan(span) + then: + if (splitByDeploymentEnabled && contextualServiceName != null && !contextualServiceName.isEmpty()) { + (1.._) * span.getServiceName() + } + if (expected) { + 1 * span.setServiceName(contextualServiceName) + } + + where: + splitByDeploymentEnabled | contextualServiceName | spanServiceName | expected + false | null | null | false + false | "test" | null | false + false | null | "test" | false + true | null | null | false + true | "" | null | false + true | "cont" | null | true + true | "" | "test" | false + true | null | "test" | false + true | "test" | "test" | false + true | "cont" | "test" | true + } + + def "enrich should set tags when present"() { + def span = Mock(AgentSpan) + when: + ClassloaderConfigurationOverrides.getOrAddEmpty(Thread.currentThread().getContextClassLoader()).addTag("key", "value") + ClassloaderConfigurationOverrides.maybeEnrichSpan(span) + then: + 1 * span.setTag("key", "value") + _ * span.getServiceName() + } +} From a08ae90d9333a846268d129dbc989d6d654cbcc5 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Fri, 3 Jan 2025 11:56:58 +0100 Subject: [PATCH 10/20] use our named --- .../wildfly/ResourceReferenceProcessorInstrumentation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/ResourceReferenceProcessorInstrumentation.java b/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/ResourceReferenceProcessorInstrumentation.java index 0494e5693fb..3b1c597007e 100644 --- a/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/ResourceReferenceProcessorInstrumentation.java +++ b/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/ResourceReferenceProcessorInstrumentation.java @@ -1,7 +1,7 @@ package datadog.trace.instrumentation.wildfly; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.isMethod; -import static net.bytebuddy.matcher.ElementMatchers.named; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; From 7f3a1403efab39df11d7d742a80be7c28606bf87 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Mon, 6 Jan 2025 09:15:17 +0100 Subject: [PATCH 11/20] more coverage --- .../instrumentation/wildfly-9/build.gradle | 1 + ...assloaderConfigurationOverridesTest.groovy | 27 ++++++++++++++----- .../ot-is-shaded/build.gradle | 2 +- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/dd-java-agent/instrumentation/wildfly-9/build.gradle b/dd-java-agent/instrumentation/wildfly-9/build.gradle index e1f7e37985a..c65de8eef12 100644 --- a/dd-java-agent/instrumentation/wildfly-9/build.gradle +++ b/dd-java-agent/instrumentation/wildfly-9/build.gradle @@ -34,6 +34,7 @@ muzzle { group = 'org.wildfly' module = 'wildfly-ee' versions = '[9.0.0.Final,)' + excludeDependency 'org.jboss.xnio:xnio-nio' // not related and causes issues with missing jar in maven repo } } diff --git a/internal-api/src/test/groovy/datadog/trace/api/ClassloaderConfigurationOverridesTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ClassloaderConfigurationOverridesTest.groovy index 4a533579d0a..2cb75d54e6d 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ClassloaderConfigurationOverridesTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ClassloaderConfigurationOverridesTest.groovy @@ -2,26 +2,27 @@ package datadog.trace.api import datadog.trace.bootstrap.instrumentation.api.AgentSpan import datadog.trace.test.util.DDSpecification +import spock.lang.Shared import spock.lang.Unroll class ClassloaderConfigurationOverridesTest extends DDSpecification { + @Shared + def ddService = Config.get().getServiceName() + + @Unroll def 'service name set = #expected when split-by-deployment enabled is #splitByDeploymentEnabled and service name manually set = #spanServiceName and contextual service name is #contextualServiceName'() { setup: - injectSysConfig("dd.service", "test") def span = Mock(AgentSpan) when: ClassloaderConfigurationOverrides.CAN_SPLIT_SERVICE_NAME_BY_DEPLOYMENT = splitByDeploymentEnabled - if (splitByDeploymentEnabled) { - injectSysConfig("dd.trace.experimental.jee.split-by-deployment", "true") - } ClassloaderConfigurationOverrides.addContextualInfo(Thread.currentThread().getContextClassLoader(), new ClassloaderConfigurationOverrides.ContextualInfo(contextualServiceName)) ClassloaderConfigurationOverrides.maybeEnrichSpan(span) then: if (splitByDeploymentEnabled && contextualServiceName != null && !contextualServiceName.isEmpty()) { - (1.._) * span.getServiceName() + (1.._) * span.getServiceName() >> spanServiceName } if (expected) { 1 * span.setServiceName(contextualServiceName) @@ -34,11 +35,11 @@ class ClassloaderConfigurationOverridesTest extends DDSpecification { false | null | "test" | false true | null | null | false true | "" | null | false - true | "cont" | null | true + true | "test" | null | true true | "" | "test" | false true | null | "test" | false true | "test" | "test" | false - true | "cont" | "test" | true + true | "test" | ddService | true } def "enrich should set tags when present"() { @@ -50,4 +51,16 @@ class ClassloaderConfigurationOverridesTest extends DDSpecification { 1 * span.setTag("key", "value") _ * span.getServiceName() } + + def "no enrichment when contextual info is absent"() { + def span = Mock(AgentSpan) + when: + ClassloaderConfigurationOverrides.addContextualInfo(Thread.currentThread().getContextClassLoader(), null) + then: + ClassloaderConfigurationOverrides.maybeGetContextualInfo() == null + when: + ClassloaderConfigurationOverrides.maybeEnrichSpan(span) + then: + _ + } } diff --git a/test-published-dependencies/ot-is-shaded/build.gradle b/test-published-dependencies/ot-is-shaded/build.gradle index f46b8994634..1b5451238af 100644 --- a/test-published-dependencies/ot-is-shaded/build.gradle +++ b/test-published-dependencies/ot-is-shaded/build.gradle @@ -114,7 +114,7 @@ tasks.register('checkJarSize') { doLast { // Arbitrary limit to prevent unintentional increases to the dd-trace-ot jar size // Raise or lower as required - assert jarFile.length() <= 7 * 1024 * 1024 + assert jarFile.length() <= 8 * 1024 * 1024 } } From abfbdbc4a3da1f588daf89cde3e711149234095e Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Wed, 8 Jan 2025 12:21:23 +0100 Subject: [PATCH 12/20] Update internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java Co-authored-by: Stuart McCulloch --- .../datadog/trace/api/ClassloaderConfigurationOverrides.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java b/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java index 7a6a3613d6a..7b72b369977 100644 --- a/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java +++ b/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java @@ -75,7 +75,7 @@ public static ContextualInfo maybeGetContextualInfo() { } @Nonnull - public static ContextualInfo getOrAddEmpty(@Nonnull ClassLoader classLoader) { + public static ContextualInfo maybeCreateContextualInfo(@Nonnull ClassLoader classLoader) { return Lazy.INSTANCE.weakCache.computeIfAbsent(classLoader, EMPTY_CONTEXTUAL_INFO_ADDER); } From 2928fb82ac4d3eaf3cc1de45b7338b68d3d71ffa Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Wed, 8 Jan 2025 12:27:29 +0100 Subject: [PATCH 13/20] Update internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java Co-authored-by: Stuart McCulloch --- .../datadog/trace/api/ClassloaderConfigurationOverrides.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java b/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java index 7b72b369977..c94574327e4 100644 --- a/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java +++ b/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java @@ -19,8 +19,8 @@ public class ClassloaderConfigurationOverrides { static boolean CAN_SPLIT_SERVICE_NAME_BY_DEPLOYMENT = Config.get().isJeeSplitByDeployment() && !Config.get().isServiceNameSetByUser(); - private static class Lazy { - private static final ClassloaderConfigurationOverrides INSTANCE = + static class Lazy { + static final ClassloaderConfigurationOverrides INSTANCE = new ClassloaderConfigurationOverrides(); } From 6a4a7e7a5cb212e4907f61fceba8c83cdcd2b670 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Wed, 8 Jan 2025 12:35:24 +0100 Subject: [PATCH 14/20] review --- ...sourceReferenceProcessorInstrumentation.java | 6 ++++-- .../api/ClassloaderConfigurationOverrides.java | 17 ++++++----------- ...ClassloaderConfigurationOverridesTest.groovy | 2 +- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/ResourceReferenceProcessorInstrumentation.java b/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/ResourceReferenceProcessorInstrumentation.java index 3b1c597007e..588e220f47c 100644 --- a/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/ResourceReferenceProcessorInstrumentation.java +++ b/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/ResourceReferenceProcessorInstrumentation.java @@ -51,8 +51,7 @@ public static class GetEnvironmentEntriesAdvice { public static void onExit( @Advice.Argument(value = 1) final ClassLoader classLoader, @Advice.Return final List configurations) { - ClassloaderConfigurationOverrides.ContextualInfo info = - ClassloaderConfigurationOverrides.getOrAddEmpty(classLoader); + ClassloaderConfigurationOverrides.ContextualInfo info = null; ContextStore contextStore = InstrumentationContext.get(EnvEntryInjectionSource.class, Object.class); for (BindingConfiguration bindingConfiguration : configurations) { @@ -63,6 +62,9 @@ public static void onExit( && bindingConfiguration .getName() .startsWith(ClassloaderConfigurationOverrides.DATADOG_TAGS_JNDI_PREFIX)) { + if (info == null) { + info = ClassloaderConfigurationOverrides.maybeCreateContextualInfo(classLoader); + } info.addTag( bindingConfiguration .getName() diff --git a/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java b/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java index c94574327e4..2cf4f641feb 100644 --- a/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java +++ b/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java @@ -19,7 +19,7 @@ public class ClassloaderConfigurationOverrides { static boolean CAN_SPLIT_SERVICE_NAME_BY_DEPLOYMENT = Config.get().isJeeSplitByDeployment() && !Config.get().isServiceNameSetByUser(); - static class Lazy { + static class Lazy { static final ClassloaderConfigurationOverrides INSTANCE = new ClassloaderConfigurationOverrides(); } @@ -54,13 +54,12 @@ public Map getTags() { private ClassloaderConfigurationOverrides() {} - public static void addContextualInfo( - @Nonnull ClassLoader classLoader, @Nonnull ContextualInfo contextualInfo) { + public static void addContextualInfo(ClassLoader classLoader, ContextualInfo contextualInfo) { Lazy.INSTANCE.weakCache.put(classLoader, contextualInfo); } @Nullable - public static ContextualInfo maybeGetContextualInfo(@Nonnull ClassLoader classLoader) { + public static ContextualInfo maybeGetContextualInfo(ClassLoader classLoader) { return Lazy.INSTANCE.weakCache.get(classLoader); } @@ -74,8 +73,7 @@ public static ContextualInfo maybeGetContextualInfo() { return maybeGetContextualInfo(Thread.currentThread().getContextClassLoader()); } - @Nonnull - public static ContextualInfo maybeCreateContextualInfo(@Nonnull ClassLoader classLoader) { + public static ContextualInfo maybeCreateContextualInfo(ClassLoader classLoader) { return Lazy.INSTANCE.weakCache.computeIfAbsent(classLoader, EMPTY_CONTEXTUAL_INFO_ADDER); } @@ -95,14 +93,11 @@ public static void maybeEnrichSpan( if (contextualInfo == null) { return; } - if (CAN_SPLIT_SERVICE_NAME_BY_DEPLOYMENT - && contextualInfo.serviceName != null - && !contextualInfo.getServiceName().isEmpty()) { + final String serviceName = contextualInfo.getServiceName(); + if (CAN_SPLIT_SERVICE_NAME_BY_DEPLOYMENT && serviceName != null && !serviceName.isEmpty()) { final String currentServiceName = span.getServiceName(); if (currentServiceName == null || currentServiceName.equals(Lazy.INSTANCE.inferredServiceName)) { - final String serviceName = contextualInfo.getServiceName(); - span.setServiceName(serviceName); ServiceNameCollector.get().addService(serviceName); } diff --git a/internal-api/src/test/groovy/datadog/trace/api/ClassloaderConfigurationOverridesTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ClassloaderConfigurationOverridesTest.groovy index 2cb75d54e6d..6f57193e38b 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ClassloaderConfigurationOverridesTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ClassloaderConfigurationOverridesTest.groovy @@ -45,7 +45,7 @@ class ClassloaderConfigurationOverridesTest extends DDSpecification { def "enrich should set tags when present"() { def span = Mock(AgentSpan) when: - ClassloaderConfigurationOverrides.getOrAddEmpty(Thread.currentThread().getContextClassLoader()).addTag("key", "value") + ClassloaderConfigurationOverrides.maybeCreateContextualInfo(Thread.currentThread().getContextClassLoader()).addTag("key", "value") ClassloaderConfigurationOverrides.maybeEnrichSpan(span) then: 1 * span.setTag("key", "value") From a33e5345fb38f9b62d887f9edfb0554d42673c78 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Wed, 8 Jan 2025 12:55:47 +0100 Subject: [PATCH 15/20] add jmh --- .../CoreTracerClassloaderNamingBenchmark.java | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 dd-trace-core/src/jmh/java/datadog/trace/core/CoreTracerClassloaderNamingBenchmark.java diff --git a/dd-trace-core/src/jmh/java/datadog/trace/core/CoreTracerClassloaderNamingBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/core/CoreTracerClassloaderNamingBenchmark.java new file mode 100644 index 00000000000..cc68ddf847f --- /dev/null +++ b/dd-trace-core/src/jmh/java/datadog/trace/core/CoreTracerClassloaderNamingBenchmark.java @@ -0,0 +1,62 @@ +package datadog.trace.core; + +import static java.util.concurrent.TimeUnit.MICROSECONDS; +import static java.util.concurrent.TimeUnit.SECONDS; + +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import java.util.WeakHashMap; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +@State(Scope.Benchmark) +@Warmup(iterations = 1, time = 15, timeUnit = SECONDS) +@Measurement(iterations = 3, time = 30, timeUnit = SECONDS) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(MICROSECONDS) +@Fork(value = 1) +/** + * Benchmark (simulateOverhead) Mode Cnt Score Error Units + * CoreTracerClassloaderNamingBenchmark.benchSpanCreation false avgt 3 0.747 ± 0.040 us/op + * CoreTracerClassloaderNamingBenchmark.benchSpanCreation true avgt 3 0.695 ± 0.106 us/op + */ +public class CoreTracerClassloaderNamingBenchmark { + CoreTracer tracer; + + WeakHashMap weakCache; + + @Param({"false", "true"}) + boolean simulateOverhead; + + @Setup(Level.Iteration) + public void init(Blackhole blackhole) { + tracer = + CoreTracer.builder() + .writer(new BlackholeWriter(blackhole, new TraceCounters(), 0)) + .strictTraceWrites(false) + .build(); + weakCache = new WeakHashMap<>(); + weakCache.put(Thread.currentThread().getContextClassLoader(), "test"); + } + + @Benchmark + public void benchSpanCreation(Blackhole blackhole) { + final AgentSpan span = tracer.startSpan("", ""); + if (simulateOverhead) { + // simulates an extra getContextClassLoader + a WeakHashMap.get + weakCache.get(Thread.currentThread().getContextClassLoader()); + } + span.finish(); + blackhole.consume(span); + } +} From d46040bc692bbd0021969e84d045a4b597aec9c1 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Wed, 8 Jan 2025 14:13:56 +0100 Subject: [PATCH 16/20] optimize --- .../java/datadog/trace/core/CoreTracer.java | 15 ++++--- .../ClassloaderConfigurationOverrides.java | 43 +++++++++++++++---- 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index efe10f3f542..29208628624 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -1598,14 +1598,17 @@ private DDSpanContext buildSpanContext() { if (serviceName == null) { serviceName = traceConfig.getPreferredServiceName(); } - ClassloaderConfigurationOverrides.ContextualInfo contextualInfo = - ClassloaderConfigurationOverrides.maybeGetContextualInfo(); Map contextualTags = null; - if (serviceName == null && parentServiceName == null) { - // in this case we have a local root without service name. We can try to see if we can find - // one from the thread context classloader + if (parentServiceName == null) { + // only fetch this on local root spans + final ClassloaderConfigurationOverrides.ContextualInfo contextualInfo = + ClassloaderConfigurationOverrides.maybeGetContextualInfo(); if (contextualInfo != null) { - serviceName = contextualInfo.getServiceName(); + // in this case we have a local root without service name. + // We can try to see if we can find one from the thread context classloader + if (serviceName == null) { + serviceName = contextualInfo.getServiceName(); + } contextualTags = contextualInfo.getTags(); } } diff --git a/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java b/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java index 2cf4f641feb..1db36601531 100644 --- a/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java +++ b/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java @@ -8,6 +8,8 @@ import java.util.HashMap; import java.util.Map; import java.util.WeakHashMap; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Function; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -52,15 +54,39 @@ public Map getTags() { private final String inferredServiceName = CapturedEnvironment.get().getProperties().get(GeneralConfig.SERVICE_NAME); + private static volatile boolean atLeastOneEntry; + private static final Lock lock = new ReentrantLock(); + private ClassloaderConfigurationOverrides() {} public static void addContextualInfo(ClassLoader classLoader, ContextualInfo contextualInfo) { - Lazy.INSTANCE.weakCache.put(classLoader, contextualInfo); + try { + lock.lock(); + Lazy.INSTANCE.weakCache.put(classLoader, contextualInfo); + atLeastOneEntry = true; + } finally { + lock.unlock(); + } + } + + public static ContextualInfo maybeCreateContextualInfo(ClassLoader classLoader) { + try { + lock.lock(); + final ContextualInfo ret = + Lazy.INSTANCE.weakCache.computeIfAbsent(classLoader, EMPTY_CONTEXTUAL_INFO_ADDER); + atLeastOneEntry = true; + return ret; + } finally { + lock.unlock(); + } } @Nullable public static ContextualInfo maybeGetContextualInfo(ClassLoader classLoader) { - return Lazy.INSTANCE.weakCache.get(classLoader); + if (atLeastOneEntry) { + return Lazy.INSTANCE.weakCache.get(classLoader); + } + return null; } /** @@ -70,11 +96,10 @@ public static ContextualInfo maybeGetContextualInfo(ClassLoader classLoader) { */ @Nullable public static ContextualInfo maybeGetContextualInfo() { - return maybeGetContextualInfo(Thread.currentThread().getContextClassLoader()); - } - - public static ContextualInfo maybeCreateContextualInfo(ClassLoader classLoader) { - return Lazy.INSTANCE.weakCache.computeIfAbsent(classLoader, EMPTY_CONTEXTUAL_INFO_ADDER); + if (atLeastOneEntry) { + return maybeGetContextualInfo(Thread.currentThread().getContextClassLoader()); + } + return null; } /** @@ -84,7 +109,9 @@ public static ContextualInfo maybeCreateContextualInfo(ClassLoader classLoader) * @param span a nonnull span */ public static void maybeEnrichSpan(@Nonnull final AgentSpan span) { - maybeEnrichSpan(span, Thread.currentThread().getContextClassLoader()); + if (atLeastOneEntry) { + maybeEnrichSpan(span, Thread.currentThread().getContextClassLoader()); + } } public static void maybeEnrichSpan( From b5e56a42546d59658bf395bd4ecbf7e822fafe5c Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Wed, 8 Jan 2025 15:18:01 +0100 Subject: [PATCH 17/20] widen muzzle excludes --- dd-java-agent/instrumentation/wildfly-9/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-java-agent/instrumentation/wildfly-9/build.gradle b/dd-java-agent/instrumentation/wildfly-9/build.gradle index c65de8eef12..10d93694ef7 100644 --- a/dd-java-agent/instrumentation/wildfly-9/build.gradle +++ b/dd-java-agent/instrumentation/wildfly-9/build.gradle @@ -34,7 +34,7 @@ muzzle { group = 'org.wildfly' module = 'wildfly-ee' versions = '[9.0.0.Final,)' - excludeDependency 'org.jboss.xnio:xnio-nio' // not related and causes issues with missing jar in maven repo + excludeDependency 'org.jboss.xnio:*' // not related and causes issues with missing jar in maven repo } } From ed1c9864b6960e6adf813270045bef1d39943fbe Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Wed, 8 Jan 2025 17:51:05 +0100 Subject: [PATCH 18/20] exclude lazy from branch coverage --- internal-api/build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/internal-api/build.gradle b/internal-api/build.gradle index dea10d40519..7b05c6989d7 100644 --- a/internal-api/build.gradle +++ b/internal-api/build.gradle @@ -195,6 +195,7 @@ excludedClassesCoverage += [ ] excludedClassesBranchCoverage = [ 'datadog.trace.api.ProductActivationConfig', + 'datadog.trace.api.ClassloaderConfigurationOverrides.Lazy', 'datadog.trace.api.Config', 'datadog.trace.util.stacktrace.HotSpotStackWalker', 'datadog.trace.util.stacktrace.StackWalkerFactory' From e8e82812df0098012fd1c8872c4e545a1c940a4d Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Fri, 10 Jan 2025 09:57:57 +0100 Subject: [PATCH 19/20] clean --- .../wildfly/ResourceReferenceProcessorInstrumentation.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/ResourceReferenceProcessorInstrumentation.java b/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/ResourceReferenceProcessorInstrumentation.java index 588e220f47c..f716563c242 100644 --- a/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/ResourceReferenceProcessorInstrumentation.java +++ b/dd-java-agent/instrumentation/wildfly-9/src/main/java/datadog/trace/instrumentation/wildfly/ResourceReferenceProcessorInstrumentation.java @@ -23,11 +23,6 @@ public ResourceReferenceProcessorInstrumentation() { super("wildfly", "jee-env-entry"); } - public ResourceReferenceProcessorInstrumentation( - String instrumentationName, String... additionalNames) { - super(instrumentationName, additionalNames); - } - @Override public String instrumentedType() { return "org.jboss.as.ee.component.deployers.ResourceReferenceProcessor"; From 8760af662e515b3f95e9be9ce3ba63d6b488ac30 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Fri, 10 Jan 2025 11:38:12 +0100 Subject: [PATCH 20/20] Do not set contextual service name if jee-split-by-deployment is not enabled --- .../jbossmodules/ModuleInstrumentation.java | 3 +-- ...readContextClassloaderInstrumentation.java | 3 +-- .../WebappClassLoaderInstrumentation.java | 19 +++++++++++-------- .../ClassloaderConfigurationOverrides.java | 10 ++++++++++ 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/dd-java-agent/instrumentation/jboss-modules/src/main/java/datadog/trace/instrumentation/jbossmodules/ModuleInstrumentation.java b/dd-java-agent/instrumentation/jboss-modules/src/main/java/datadog/trace/instrumentation/jbossmodules/ModuleInstrumentation.java index 300c33bb788..da0545b0fb1 100644 --- a/dd-java-agent/instrumentation/jboss-modules/src/main/java/datadog/trace/instrumentation/jbossmodules/ModuleInstrumentation.java +++ b/dd-java-agent/instrumentation/jboss-modules/src/main/java/datadog/trace/instrumentation/jbossmodules/ModuleInstrumentation.java @@ -164,8 +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()) { - ClassloaderConfigurationOverrides.addContextualInfo( - module.getClassLoader(), new ClassloaderConfigurationOverrides.ContextualInfo(name)); + ClassloaderConfigurationOverrides.withPinnedServiceName(module.getClassLoader(), name); } } } diff --git a/dd-java-agent/instrumentation/liberty-20/src/main/java/datadog/trace/instrumentation/liberty20/ThreadContextClassloaderInstrumentation.java b/dd-java-agent/instrumentation/liberty-20/src/main/java/datadog/trace/instrumentation/liberty20/ThreadContextClassloaderInstrumentation.java index 64eaa219196..9b71fda635c 100644 --- a/dd-java-agent/instrumentation/liberty-20/src/main/java/datadog/trace/instrumentation/liberty20/ThreadContextClassloaderInstrumentation.java +++ b/dd-java-agent/instrumentation/liberty-20/src/main/java/datadog/trace/instrumentation/liberty20/ThreadContextClassloaderInstrumentation.java @@ -40,8 +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()) { - ClassloaderConfigurationOverrides.addContextualInfo( - self, new ClassloaderConfigurationOverrides.ContextualInfo(name)); + ClassloaderConfigurationOverrides.withPinnedServiceName(self, name); } } } diff --git a/dd-java-agent/instrumentation/tomcat-classloading-9/src/main/java/datadog/trace/instrumentation/tomcat9/WebappClassLoaderInstrumentation.java b/dd-java-agent/instrumentation/tomcat-classloading-9/src/main/java/datadog/trace/instrumentation/tomcat9/WebappClassLoaderInstrumentation.java index 89fd2c6de88..ccd92cba35c 100644 --- a/dd-java-agent/instrumentation/tomcat-classloading-9/src/main/java/datadog/trace/instrumentation/tomcat9/WebappClassLoaderInstrumentation.java +++ b/dd-java-agent/instrumentation/tomcat-classloading-9/src/main/java/datadog/trace/instrumentation/tomcat9/WebappClassLoaderInstrumentation.java @@ -9,6 +9,8 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; 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; @@ -47,14 +49,12 @@ public static void onContextAvailable( final String contextName = context.getBaseName(); if (contextName != null && !contextName.isEmpty()) { - info = new ClassloaderConfigurationOverrides.ContextualInfo(contextName); + info = ClassloaderConfigurationOverrides.withPinnedServiceName(classLoader, contextName); } if (context.getNamingResources() != null) { final ContextEnvironment[] envs = context.getNamingResources().findEnvironments(); if (envs != null) { - if (info == null) { - info = new ClassloaderConfigurationOverrides.ContextualInfo(null); - } + final Map 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 @@ -70,14 +70,17 @@ public static void onContextAvailable( name = env.getName().substring(DATADOG_TAGS_JNDI_PREFIX.length()); } if (name != null && !name.isEmpty()) { - info.addTag(name, env.getValue()); + tags.put(name, env.getValue()); + } + } + if (!tags.isEmpty()) { + if (info == null) { + info = ClassloaderConfigurationOverrides.maybeCreateContextualInfo(classLoader); } + tags.forEach(info::addTag); } } } - if (info != null) { - ClassloaderConfigurationOverrides.addContextualInfo(classLoader, info); - } } } } diff --git a/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java b/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java index 1db36601531..cfcff9a10b0 100644 --- a/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java +++ b/internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java @@ -81,6 +81,16 @@ public static ContextualInfo maybeCreateContextualInfo(ClassLoader classLoader) } } + @Nullable + public static ContextualInfo withPinnedServiceName(ClassLoader classLoader, String serviceName) { + if (!CAN_SPLIT_SERVICE_NAME_BY_DEPLOYMENT) { + return null; + } + final ContextualInfo contextualInfo = new ContextualInfo(serviceName); + addContextualInfo(classLoader, contextualInfo); + return contextualInfo; + } + @Nullable public static ContextualInfo maybeGetContextualInfo(ClassLoader classLoader) { if (atLeastOneEntry) {