Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Service naming: split by jee deployment #8064

Merged
merged 14 commits into from
Dec 12, 2024
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
package datadog.trace.instrumentation.jbossmodules;

import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static datadog.trace.instrumentation.jbossmodules.ModuleNameHelper.extractDeploymentName;
import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;

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.bootstrap.AgentClassLoading;
import java.io.IOException;
import java.io.InputStream;
Expand All @@ -33,7 +36,8 @@ public String[] helperClassNames() {
return new String[] {
"org.jboss.modules.ModuleLinkageHelper",
"org.jboss.modules.ModuleLinkageHelper$1",
"org.jboss.modules.ModuleLinkageHelper$2"
"org.jboss.modules.ModuleLinkageHelper$2",
packageName + ".ModuleNameHelper",
};
}

Expand All @@ -60,6 +64,7 @@ public void methodAdvice(MethodTransformer transformer) {
.and(takesArgument(0, String.class))
.and(takesArgument(1, boolean.class)))),
ModuleInstrumentation.class.getName() + "$WidenLoadClassAdvice");
transformer.applyAdvice(isConstructor(), getClass().getName() + "$CaptureModuleNameAdvice");
}

/**
Expand Down Expand Up @@ -154,4 +159,14 @@ public static void onExit(
}
}
}

public static class CaptureModuleNameAdvice {
@Advice.OnMethodExit(suppress = Throwable.class)
public static void afterConstruct(@Advice.This final Module module) {
final String name = extractDeploymentName(module.getClassLoader());
amarziali marked this conversation as resolved.
Show resolved Hide resolved
if (name != null && !name.isEmpty()) {
ClassloaderServiceNames.addServiceName(module.getClassLoader(), name);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package datadog.trace.instrumentation.jbossmodules;

import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.annotation.Nonnull;
import org.jboss.modules.ModuleClassLoader;

public class ModuleNameHelper {
private ModuleNameHelper() {}

private static final Pattern SUBDEPLOYMENT_MATCH =
Pattern.compile("deployment(?>.+\\.ear)?\\.(.+)\\.[j|w]ar");

public static String extractDeploymentName(@Nonnull final ModuleClassLoader classLoader) {
final Matcher matcher =
SUBDEPLOYMENT_MATCH.matcher(classLoader.getModule().getIdentifier().getName());
if (matcher.matches()) {
return matcher.group(1);
}
return null;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package datadog.trace.instrumentation.liberty20;

import com.ibm.ws.classloading.internal.ThreadContextClassLoader;

public class BundleNameHelper {
private BundleNameHelper() {}

public static String extractDeploymentName(final ThreadContextClassLoader classLoader) {
final String id = classLoader.getKey();
// id is something like <type>:name#somethingelse
final int head = id.indexOf(':');
if (head < 0) {
return null;
}
final int tail = id.lastIndexOf('#', head);
if (tail < 0) {
return null;
}
final String name = id.substring(head + 1, tail);
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,14 @@
import com.google.auto.service.AutoService;
import com.ibm.ws.webcontainer.srt.SRTServletRequest;
import com.ibm.ws.webcontainer.srt.SRTServletResponse;
import com.ibm.ws.webcontainer.webapp.WebApp;
import com.ibm.wsspi.webcontainer.webapp.IWebAppDispatcherContext;
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.agent.tooling.InstrumenterModule;
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;
Expand Down Expand Up @@ -104,7 +107,16 @@ public static class HandleRequestAdvice {
request.setAttribute(DD_EXTRACTED_CONTEXT_ATTRIBUTE, extractedContext);
final AgentSpan span = DECORATE.startSpan(request, extractedContext);
scope = activateSpan(span, true);

final IWebAppDispatcherContext dispatcherContext = request.getWebAppDispatcherContext();
if (dispatcherContext != null) {
final WebApp webapp = dispatcherContext.getWebApp();
if (webapp != null) {
final ClassLoader cl = webapp.getClassLoader();
if (cl != null) {
ClassloaderServiceNames.maybeSetToSpan(span, cl);
}
}
}
amarziali marked this conversation as resolved.
Show resolved Hide resolved
DECORATE.afterStart(span);
DECORATE.onRequest(span, request, request, extractedContext);
request.setAttribute(DD_SPAN_ATTRIBUTE, span);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package datadog.trace.instrumentation.liberty20;

import static datadog.trace.instrumentation.liberty20.BundleNameHelper.extractDeploymentName;
import static net.bytebuddy.matcher.ElementMatchers.isConstructor;

import com.google.auto.service.AutoService;
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 net.bytebuddy.asm.Advice;

@AutoService(InstrumenterModule.class)
public class ThreadContextClassloaderInstrumentation extends InstrumenterModule.Tracing
implements Instrumenter.ForSingleType {

public ThreadContextClassloaderInstrumentation() {
super("liberty", "liberty-classloading");
}

@Override
public String instrumentedType() {
return "com.ibm.ws.classloading.internal.ThreadContextClassLoader";
}

@Override
public String[] helperClassNames() {
return new String[] {
packageName + ".BundleNameHelper",
};
}

@Override
public void methodAdvice(MethodTransformer transformer) {
transformer.applyAdvice(
isConstructor(), getClass().getName() + "$ThreadContextClassloaderAdvice");
}

public static class ThreadContextClassloaderAdvice {
@Advice.OnMethodExit(suppress = Throwable.class)
public static void afterConstruct(@Advice.This ThreadContextClassLoader self) {
final String name = extractDeploymentName(self);
amarziali marked this conversation as resolved.
Show resolved Hide resolved
if (name != null && !name.isEmpty()) {
ClassloaderServiceNames.addServiceName(self, name);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,20 @@ class Liberty20AsyncForkedTest extends Liberty20Test implements TestingGenericHt
}
}

@IgnoreIf({
// failing because org.apache.xalan.transformer.TransformerImpl is
// instrumented while on the the global ignores list
System.getProperty('java.vm.name') == 'IBM J9 VM' &&
System.getProperty('java.specification.version') == '1.8' })
class LibertyServletClassloaderNamingForkedTest extends Liberty20V0ForkedTest {
@Override
protected void configurePreAgent() {
super.configurePreAgent()
// will not set the service name according to the servlet context value
injectSysConfig("trace.experimental.jee.split-by-deployment", "true")
}
}

@IgnoreIf({
// failing because org.apache.xalan.transformer.TransformerImpl is
// instrumented while on the the global ignores list
Expand Down
1 change: 1 addition & 0 deletions dd-java-agent/instrumentation/liberty-23/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ dependencies {
testImplementation testFixtures(project(':dd-java-agent:appsec'))
testRuntimeOnly project(':dd-java-agent:instrumentation:osgi-4.3')
testRuntimeOnly files({ tasks.filterLogbackClassic.filteredLogbackDir })
testRuntimeOnly project(':dd-java-agent:instrumentation:liberty-20')
testRuntimeOnly project(':dd-java-agent:instrumentation:servlet:request-5')
testRuntimeOnly files({ tasks.shadowJar.archiveFile })

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,14 @@
import com.google.auto.service.AutoService;
import com.ibm.ws.webcontainer.srt.SRTServletRequest;
import com.ibm.ws.webcontainer.srt.SRTServletResponse;
import com.ibm.ws.webcontainer.webapp.WebApp;
import com.ibm.wsspi.webcontainer.webapp.IWebAppDispatcherContext;
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.agent.tooling.InstrumenterModule;
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;
Expand Down Expand Up @@ -106,7 +109,16 @@ public static class HandleRequestAdvice {
request.setAttribute(DD_EXTRACTED_CONTEXT_ATTRIBUTE, extractedContext);
final AgentSpan span = DECORATE.startSpan(request, extractedContext);
scope = activateSpan(span, true);

final IWebAppDispatcherContext dispatcherContext = request.getWebAppDispatcherContext();
if (dispatcherContext != null) {
final WebApp webapp = dispatcherContext.getWebApp();
if (webapp != null) {
final ClassLoader cl = webapp.getClassLoader();
if (cl != null) {
ClassloaderServiceNames.maybeSetToSpan(span, cl);
}
}
}
amarziali marked this conversation as resolved.
Show resolved Hide resolved
DECORATE.afterStart(span);
DECORATE.onRequest(span, request, request, extractedContext);
request.setAttribute(DD_SPAN_ATTRIBUTE, span);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,3 +170,17 @@ class Liberty23V0ForkedTest extends Liberty23Test implements TestingGenericHttpN
System.getProperty('java.specification.version') == '1.8' })
class Liberty23V1ForkedTest extends Liberty23Test implements TestingGenericHttpNamingConventions.ServerV1 {
}

@IgnoreIf({
// failing because org.apache.xalan.transformer.TransformerImpl is
// instrumented while on the the global ignores list
System.getProperty('java.vm.name') == 'IBM J9 VM' &&
System.getProperty('java.specification.version') == '1.8' })
class LibertyServletClassloaderNamingForkedTest extends Liberty23V0ForkedTest {
@Override
protected void configurePreAgent() {
super.configurePreAgent()
// will not set the service name according to the servlet context value
injectSysConfig("trace.experimental.jee.split-by-deployment", "true")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
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;
Expand Down Expand Up @@ -39,6 +40,8 @@ public static boolean onEnter(
Object spanAttr = request.getAttribute(DD_SPAN_ATTRIBUTE);
final boolean hasServletTrace = spanAttr instanceof AgentSpan;
if (hasServletTrace) {
final AgentSpan span = (AgentSpan) spanAttr;
ClassloaderServiceNames.maybeSetToSpan(span, Thread.currentThread());
// Tracing might already be applied by the FilterChain or a parent request (forward/include).
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
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.instrumentation.servlet.ServletBlockingHelper;
Expand Down Expand Up @@ -60,6 +61,8 @@ public static boolean onEnter(
Object spanAttrValue = request.getAttribute(DD_SPAN_ATTRIBUTE);
final boolean hasServletTrace = spanAttrValue instanceof AgentSpan;
if (hasServletTrace) {
final AgentSpan span = (AgentSpan) spanAttrValue;
ClassloaderServiceNames.maybeSetToSpan(span, Thread.currentThread());
// Tracing might already be applied by other instrumentation,
// the FilterChain or a parent request (forward/include).
return false;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package datadog.trace.instrumentation.servlet3;

import datadog.trace.api.naming.ClassloaderServiceNames;
import datadog.trace.bootstrap.instrumentation.api.AgentPropagation;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter;
Expand Down Expand Up @@ -82,6 +83,10 @@ public AgentSpan onRequest(
final HttpServletRequest request,
AgentSpan.Context.Extracted context) {
assert span != null;
final String eeService = ClassloaderServiceNames.maybeGetForThread(Thread.currentThread());
if (eeService != null) {
span.setServiceName(eeService);
amarziali marked this conversation as resolved.
Show resolved Hide resolved
}
if (request != null) {
String contextPath = request.getContextPath();
String servletPath = request.getServletPath();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.hasSuperType;
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator.DD_SPAN_ATTRIBUTE;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
Expand All @@ -12,6 +13,7 @@
import datadog.trace.agent.tooling.InstrumenterModule;
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;
Expand Down Expand Up @@ -51,30 +53,33 @@ public void methodAdvice(MethodTransformer transformer) {

public static class ExtractPrincipalAdvice {
@Advice.OnMethodEnter(suppress = Throwable.class)
public static boolean before(@Advice.Argument(0) final ServletRequest request) {
public static AgentSpan before(@Advice.Argument(0) final ServletRequest request) {
if (!(request instanceof HttpServletRequest)) {
return false;
return null;
}
return CallDepthThreadLocalMap.incrementCallDepth(HttpServletRequest.class) == 0;
Object span = request.getAttribute(DD_SPAN_ATTRIBUTE);
if (span instanceof AgentSpan
&& CallDepthThreadLocalMap.incrementCallDepth(HttpServletRequest.class) == 0) {
final AgentSpan agentSpan = (AgentSpan) span;
ClassloaderServiceNames.maybeSetToSpan(agentSpan, Thread.currentThread());
return agentSpan;
}
return null;
}

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void after(
@Advice.Enter boolean advice, @Advice.Argument(0) final ServletRequest request) {
if (advice) {
CallDepthThreadLocalMap.reset(HttpServletRequest.class);
final HttpServletRequest httpServletRequest =
(HttpServletRequest) request; // at this point the cast should be safe
if (Config.get().isServletPrincipalEnabled()
&& httpServletRequest.getUserPrincipal() != null) {
Object span =
request.getAttribute(
"datadog.span"); // hardcode to avoid injecting HttpServiceDecorator just for this
if (span instanceof AgentSpan) {
((AgentSpan) span)
.setTag(DDTags.USER_NAME, httpServletRequest.getUserPrincipal().getName());
}
}
@Advice.Enter final AgentSpan span, @Advice.Argument(0) final ServletRequest request) {
if (span == null) {
return;
}

CallDepthThreadLocalMap.reset(HttpServletRequest.class);
final HttpServletRequest httpServletRequest =
(HttpServletRequest) request; // at this point the cast should be safe
if (Config.get().isServletPrincipalEnabled()
&& httpServletRequest.getUserPrincipal() != null) {
span.setTag(DDTags.USER_NAME, httpServletRequest.getUserPrincipal().getName());
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions dd-java-agent/instrumentation/tomcat-5.5/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,15 @@ dependencies {

latest10TestImplementation(project(':dd-java-agent:instrumentation:tomcat-appsec-6'))
latest10TestImplementation(project(':dd-java-agent:instrumentation:tomcat-appsec-7'))
latest10TestImplementation(project(':dd-java-agent:instrumentation:tomcat-classloading-9'))

latestDepTestImplementation group: 'org.apache.tomcat.embed', name: 'tomcat-embed-core', version: '11.+'
latestDepTestImplementation group: 'org.apache.tomcat', name: 'jakartaee-migration', version: '1.+'
latestDepTestImplementation(testFixtures(project(":dd-java-agent:appsec")))

latestDepTestRuntimeOnly(project(':dd-java-agent:instrumentation:tomcat-appsec-6'))
latestDepTestRuntimeOnly(project(':dd-java-agent:instrumentation:tomcat-appsec-7'))
latestDepTestRuntimeOnly(project(':dd-java-agent:instrumentation:tomcat-classloading-9'))
}

// Exclude all the dependencies from test for latestDepTest since the names are completely different.
Expand Down
Loading
Loading