Skip to content

Commit 294320a

Browse files
committed
Add exception handling and supression for advice.
Also centralize classloader matching.
1 parent e96c084 commit 294320a

File tree

23 files changed

+213
-136
lines changed

23 files changed

+213
-136
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,5 @@ Thumbs.db
4848
/bin
4949
/out
5050
*/out
51+
dd-java-agent/integrations/*/out
5152
dd-trace-examples/*/out

dd-java-agent/integrations/aws-sdk/src/main/java/dd/inst/aws/AWSClientInstrumentation.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package dd.inst.aws;
22

3-
import static dd.trace.ClassLoaderHasClassMatcher.classLoaderHasClasses;
3+
import static dd.trace.ClassLoaderMatcher.classLoaderHasClasses;
4+
import static dd.trace.ExceptionHandlers.defaultExceptionHandler;
45
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
56
import static net.bytebuddy.matcher.ElementMatchers.named;
67
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
@@ -32,13 +33,14 @@ public AgentBuilder instrument(final AgentBuilder agentBuilder) {
3233
new AgentBuilder.Transformer.ForAdvice()
3334
.advice(
3435
named("build").and(takesArguments(0)).and(isPublic()),
35-
AWSClientAdvice.class.getName()))
36+
AWSClientAdvice.class.getName())
37+
.withExceptionHandler(defaultExceptionHandler()))
3638
.asDecorator();
3739
}
3840

3941
public static class AWSClientAdvice {
4042

41-
@Advice.OnMethodExit(onThrowable = Throwable.class)
43+
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
4244
public static void addHandler(@Advice.This final AwsClientBuilder builder) {
4345

4446
final RequestHandler2 handler = new TracingRequestHandler(GlobalTracer.get());

dd-java-agent/integrations/helpers/helpers.gradle

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,10 @@ dependencies {
2222
compile group: 'io.opentracing.contrib', name: 'opentracing-cassandra-driver', version: '0.0.2'
2323
compile group: 'io.opentracing.contrib', name: 'opentracing-apache-httpclient', version: '0.0.2'
2424

25-
compileOnly group: 'javax.servlet', name: 'javax.servlet-api', version: '3.0.1'
26-
// compileOnly group: 'javax.servlet', name: 'servlet-api', version: '2.3'
2725
compileOnly group: 'org.mongodb', name: 'mongo-java-driver', version: '3.4.2'
2826
compileOnly group: 'org.mongodb', name: 'mongodb-driver-async', version: '3.4.2'
2927
compileOnly group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0'
3028
compileOnly group: 'javax.jms', name: 'javax.jms-api', version: '2.0.1'
31-
compileOnly group: 'com.amazonaws', name: 'aws-java-sdk-core', version: '1.11.119'
3229
compileOnly group: 'com.datastax.cassandra', name: 'cassandra-driver-core', version: '3.2.0'
3330
compileOnly group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.5.3'
3431
}

dd-java-agent/integrations/servlet-2/servlet-2.gradle

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ apply from: "${rootDir}/gradle/java.gradle"
1414

1515
dependencies {
1616
compileOnly group: 'javax.servlet', name: 'servlet-api', version: '2.3'
17+
compileOnly group: 'io.opentracing.contrib', name: 'opentracing-web-servlet-filter', version: '0.0.9'
1718

1819
compile project(':dd-trace')
1920
compile project(':dd-java-agent:tooling')
2021

2122
compile deps.bytebuddy
2223
compile deps.opentracing
2324

24-
compile group: 'io.opentracing.contrib', name: 'opentracing-web-servlet-filter', version: '0.0.9'
2525
compile group: 'com.google.auto.service', name: 'auto-service', version: '1.0-rc3'
2626
}

dd-java-agent/integrations/servlet-2/src/main/java/dd/inst/servlet2/HttpServlet2Instrumentation.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package dd.inst.servlet2;
22

3-
import static dd.trace.ClassLoaderHasClassMatcher.classLoaderHasClasses;
3+
import static dd.trace.ClassLoaderMatcher.classLoaderHasClasses;
4+
import static dd.trace.ExceptionHandlers.defaultExceptionHandler;
45
import static net.bytebuddy.matcher.ElementMatchers.isProtected;
56
import static net.bytebuddy.matcher.ElementMatchers.named;
67
import static net.bytebuddy.matcher.ElementMatchers.not;
@@ -41,13 +42,14 @@ public AgentBuilder instrument(final AgentBuilder agentBuilder) {
4142
.and(takesArgument(0, named("javax.servlet.http.HttpServletRequest")))
4243
.and(takesArgument(1, named("javax.servlet.http.HttpServletResponse")))
4344
.and(isProtected()),
44-
HttpServlet2Advice.class.getName()))
45+
HttpServlet2Advice.class.getName())
46+
.withExceptionHandler(defaultExceptionHandler()))
4547
.asDecorator();
4648
}
4749

4850
public static class HttpServlet2Advice {
4951

50-
@Advice.OnMethodEnter
52+
@Advice.OnMethodEnter(suppress = Throwable.class)
5153
public static ActiveSpan startSpan(@Advice.Argument(0) final HttpServletRequest req) {
5254

5355
final SpanContext extractedContext =
@@ -65,7 +67,7 @@ public static ActiveSpan startSpan(@Advice.Argument(0) final HttpServletRequest
6567
return span;
6668
}
6769

68-
@Advice.OnMethodExit(onThrowable = Throwable.class)
70+
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
6971
public static void stopSpan(
7072
@Advice.Argument(0) final HttpServletRequest req,
7173
@Advice.Argument(1) final HttpServletResponse resp,

dd-java-agent/integrations/servlet-3/servlet-3.gradle

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,13 @@ apply from: "${rootDir}/gradle/java.gradle"
1515

1616
dependencies {
1717
compileOnly group: 'javax.servlet', name: 'javax.servlet-api', version: '3.0.1'
18+
compileOnly group: 'io.opentracing.contrib', name: 'opentracing-web-servlet-filter', version: '0.0.9'
1819

1920
compile project(':dd-trace')
2021
compile project(':dd-java-agent:tooling')
2122

2223
compile deps.bytebuddy
2324
compile deps.opentracing
2425

25-
compile group: 'io.opentracing.contrib', name: 'opentracing-web-servlet-filter', version: '0.0.9'
2626
compile group: 'com.google.auto.service', name: 'auto-service', version: '1.0-rc3'
2727
}

dd-java-agent/integrations/servlet-3/src/main/java/dd/inst/servlet3/HttpServlet3Instrumentation.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package dd.inst.servlet3;
22

3-
import static dd.trace.ClassLoaderHasClassMatcher.classLoaderHasClasses;
3+
import static dd.trace.ClassLoaderMatcher.classLoaderHasClasses;
4+
import static dd.trace.ExceptionHandlers.defaultExceptionHandler;
45
import static net.bytebuddy.matcher.ElementMatchers.isProtected;
56
import static net.bytebuddy.matcher.ElementMatchers.named;
67
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
@@ -41,13 +42,14 @@ public AgentBuilder instrument(final AgentBuilder agentBuilder) {
4142
.and(takesArgument(0, named("javax.servlet.http.HttpServletRequest")))
4243
.and(takesArgument(1, named("javax.servlet.http.HttpServletResponse")))
4344
.and(isProtected()),
44-
HttpServlet3Advice.class.getName()))
45+
HttpServlet3Advice.class.getName())
46+
.withExceptionHandler(defaultExceptionHandler()))
4547
.asDecorator();
4648
}
4749

4850
public static class HttpServlet3Advice {
4951

50-
@Advice.OnMethodEnter
52+
@Advice.OnMethodEnter(suppress = Throwable.class)
5153
public static ActiveSpan startSpan(@Advice.Argument(0) final HttpServletRequest req) {
5254

5355
final SpanContext extractedContext =
@@ -65,7 +67,7 @@ public static ActiveSpan startSpan(@Advice.Argument(0) final HttpServletRequest
6567
return span;
6668
}
6769

68-
@Advice.OnMethodExit(onThrowable = Throwable.class)
70+
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
6971
public static void stopSpan(
7072
@Advice.Argument(0) final HttpServletRequest req,
7173
@Advice.Argument(1) final HttpServletResponse resp,

dd-java-agent/integrations/spring-web/src/main/java/dd/inst/springweb/SpringWebInstrumentation.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package dd.inst.springweb;
22

3-
import static dd.trace.ClassLoaderHasClassWithFieldMatcher.classLoaderHasClassWithField;
3+
import static dd.trace.ClassLoaderMatcher.classLoaderHasClassWithField;
4+
import static dd.trace.ExceptionHandlers.defaultExceptionHandler;
45
import static net.bytebuddy.matcher.ElementMatchers.hasSuperType;
56
import static net.bytebuddy.matcher.ElementMatchers.isInterface;
67
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
@@ -43,12 +44,14 @@ public AgentBuilder instrument(final AgentBuilder agentBuilder) {
4344
.and(isPublic())
4445
.and(nameStartsWith("handle"))
4546
.and(takesArgument(0, named("javax.servlet.http.HttpServletRequest"))),
46-
SpringWebAdvice.class.getName()));
47+
SpringWebAdvice.class.getName())
48+
.withExceptionHandler(defaultExceptionHandler()))
49+
.asDecorator();
4750
}
4851

4952
public static class SpringWebAdvice {
5053

51-
@Advice.OnMethodEnter
54+
@Advice.OnMethodEnter(suppress = Throwable.class)
5255
public static void nameResource(@Advice.Argument(0) final HttpServletRequest request) {
5356
final ActiveSpan span = GlobalTracer.get().activeSpan();
5457
if (span != null) {

dd-java-agent/src/main/java/com/datadoghq/agent/ClassLoaderIntegrationInjector.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import lombok.extern.slf4j.Slf4j;
1212

1313
@Slf4j
14+
// TODO: consider augmenting with com.sun.xml.internal.bind.v2.runtime.reflect.opt.Injector
1415
public class ClassLoaderIntegrationInjector {
1516
private final Map<ZipEntry, byte[]> entries;
1617
private final Map<ClassLoader, Method> invocationPoints = Maps.newConcurrentMap();

dd-java-agent/src/main/java/com/datadoghq/agent/InstrumentationRulesManager.java

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,21 @@
11
package com.datadoghq.agent;
22

33
import com.google.common.collect.Maps;
4-
import com.google.common.collect.Sets;
4+
import io.opentracing.Tracer;
5+
import io.opentracing.contrib.tracerresolver.TracerResolver;
6+
import io.opentracing.util.GlobalTracer;
57
import java.io.ByteArrayOutputStream;
68
import java.io.IOException;
79
import java.io.InputStream;
810
import java.io.PrintWriter;
911
import java.io.StringWriter;
1012
import java.util.ArrayList;
13+
import java.util.Collections;
1114
import java.util.HashSet;
1215
import java.util.List;
1316
import java.util.Map;
1417
import java.util.Set;
18+
import java.util.WeakHashMap;
1519
import java.util.regex.Matcher;
1620
import java.util.regex.Pattern;
1721
import java.util.zip.ZipEntry;
@@ -29,13 +33,16 @@ public class InstrumentationRulesManager {
2933
private static final String INTEGRATION_RULES = "integration-rules.btm";
3034
private static final String HELPERS_NAME = "/helpers.jar.zip";
3135

36+
private static final Object SYNC = new Object();
37+
3238
private final Retransformer transformer;
3339
private final TracingAgentConfig config;
3440
private final AgentRulesManager agentRulesManager;
3541
private final ClassLoaderIntegrationInjector injector;
3642
private final InstrumentationChecker checker = new InstrumentationChecker();
3743

38-
private final Set<ClassLoader> initializedClassloaders = Sets.newConcurrentHashSet();
44+
private final Set<ClassLoader> initializedClassloaders =
45+
Collections.newSetFromMap(new WeakHashMap<ClassLoader, Boolean>());
3946

4047
public InstrumentationRulesManager(
4148
final Retransformer trans,
@@ -77,6 +84,9 @@ public static void registerClassLoad() {
7784
}
7885

7986
public static void registerClassLoad(final Object obj) {
87+
if (AgentRulesManager.INSTANCE == null) {
88+
return;
89+
}
8090
final ClassLoader cl;
8191
if (obj instanceof ClassLoader) {
8292
cl = (ClassLoader) obj;
@@ -122,6 +132,8 @@ public void initialize(final ClassLoader classLoader) {
122132
} catch (final Exception e) {
123133
log.warn("Error uninstalling scripts", e);
124134
}
135+
136+
initTracer();
125137
}
126138

127139
/**
@@ -152,4 +164,20 @@ private void uninstallScripts(final List<String> installedScripts, final List<St
152164
log.info("Uninstall rule scripts: {}", rulesToRemove.toString());
153165
}
154166
}
167+
168+
private void initTracer() {
169+
synchronized (SYNC) {
170+
if (!GlobalTracer.isRegistered()) {
171+
// Try to obtain a tracer using the TracerResolver
172+
final Tracer resolved = TracerResolver.resolveTracer();
173+
if (resolved != null) {
174+
try {
175+
GlobalTracer.register(resolved);
176+
} catch (final RuntimeException re) {
177+
log.warn("Failed to register tracer '" + resolved + "'", re);
178+
}
179+
}
180+
}
181+
}
182+
}
155183
}

0 commit comments

Comments
 (0)