Skip to content

Commit 43c841c

Browse files
authored
Avoid these anonymous inner classes as they keep the enclosing instrumenter class alive (DataDog#3860)
1 parent 5c832f2 commit 43c841c

File tree

5 files changed

+164
-176
lines changed

5 files changed

+164
-176
lines changed

dd-java-agent/instrumentation/jetty-11/src/main/java/datadog/trace/instrumentation/jetty11/JettyServerInstrumentation.java

+3-12
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
55
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.namedOneOf;
66
import static datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.ExcludeType.RUNNABLE;
7+
import static net.bytebuddy.matcher.ElementMatchers.isDeclaredBy;
8+
import static net.bytebuddy.matcher.ElementMatchers.not;
79
import static net.bytebuddy.matcher.ElementMatchers.takesNoArguments;
810

911
import com.google.auto.service.AutoService;
@@ -14,8 +16,6 @@
1416
import java.util.Collection;
1517
import java.util.Collections;
1618
import java.util.Map;
17-
import net.bytebuddy.description.method.MethodDescription;
18-
import net.bytebuddy.matcher.ElementMatcher;
1919

2020
@AutoService(Instrumenter.class)
2121
public final class JettyServerInstrumentation extends Instrumenter.Tracing
@@ -54,16 +54,7 @@ public void adviceTransformations(AdviceTransformation transformation) {
5454
// In 9.0.3 the handle logic was extracted out to "handle"
5555
// but we still want to instrument run in case handle is missing
5656
// (without the risk of double instrumenting).
57-
named("run")
58-
.and(
59-
new ElementMatcher.Junction.ForNonNullValues<MethodDescription>() {
60-
@Override
61-
protected boolean doMatch(MethodDescription target) {
62-
// TODO this could probably be made into a nicer matcher.
63-
return !declaresMethod(named("handle"))
64-
.matches(target.getDeclaringType().asErasure());
65-
}
66-
}))),
57+
named("run").and(isDeclaredBy(not(declaresMethod(named("handle"))))))),
6758
packageName + ".JettyServerAdvice$HandleAdvice");
6859
transformation.applyAdvice(
6960
// name changed to recycle in 9.3.0

dd-java-agent/instrumentation/jetty-9/src/main/java/datadog/trace/instrumentation/jetty9/JettyServerInstrumentation.java

+3-12
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
import static datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator.DD_SPAN_ATTRIBUTE;
88
import static datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.ExcludeType.RUNNABLE;
99
import static datadog.trace.instrumentation.jetty9.JettyDecorator.DECORATE;
10+
import static net.bytebuddy.matcher.ElementMatchers.isDeclaredBy;
11+
import static net.bytebuddy.matcher.ElementMatchers.not;
1012
import static net.bytebuddy.matcher.ElementMatchers.takesNoArguments;
1113

1214
import com.google.auto.service.AutoService;
@@ -22,8 +24,6 @@
2224
import java.util.Collections;
2325
import java.util.Map;
2426
import net.bytebuddy.asm.Advice;
25-
import net.bytebuddy.description.method.MethodDescription;
26-
import net.bytebuddy.matcher.ElementMatcher;
2727
import org.eclipse.jetty.server.HttpChannel;
2828
import org.eclipse.jetty.server.Request;
2929

@@ -61,16 +61,7 @@ public void adviceTransformations(AdviceTransformation transformation) {
6161
// In 9.0.3 the handle logic was extracted out to "handle"
6262
// but we still want to instrument run in case handle is missing
6363
// (without the risk of double instrumenting).
64-
named("run")
65-
.and(
66-
new ElementMatcher.Junction.ForNonNullValues<MethodDescription>() {
67-
@Override
68-
protected boolean doMatch(MethodDescription target) {
69-
// TODO this could probably be made into a nicer matcher.
70-
return !declaresMethod(named("handle"))
71-
.matches(target.getDeclaringType().asErasure());
72-
}
73-
}))),
64+
named("run").and(isDeclaredBy(not(declaresMethod(named("handle"))))))),
7465
JettyServerInstrumentation.class.getName() + "$HandleAdvice");
7566
transformation.applyAdvice(
7667
// name changed to recycle in 9.3.0

dd-java-agent/instrumentation/resteasy-appsec/src/main/java/datadog/trace/instrumentation/resteasy/DecodedFormParametersInstrumentation.java

+13-11
Original file line numberDiff line numberDiff line change
@@ -66,18 +66,20 @@ public void adviceTransformations(AdviceTransformation transformation) {
6666

6767
@Override
6868
public ReferenceProvider runtimeMuzzleReferences() {
69-
return new ReferenceProvider() {
70-
@Override
71-
public Iterable<Reference> buildReferences(TypePool typePool) {
72-
List<Reference> references = new ArrayList<>();
73-
references.add(BASE_HTTP_REQUEST_DECODED_PARAMETERS);
74-
references.add(HTTP_SERVLET_INPUT_MESSAGE_DECODED_PARAMETERS);
75-
if (typePool.describe(NETTY_HTTP_REQUEST_CLASS_NAME).isResolved()) {
76-
references.add(NETTY_HTTP_REQUEST_DECODED_PARAMETERS);
77-
}
78-
return references;
69+
return new CustomReferenceProvider();
70+
}
71+
72+
static class CustomReferenceProvider implements ReferenceProvider {
73+
@Override
74+
public Iterable<Reference> buildReferences(TypePool typePool) {
75+
List<Reference> references = new ArrayList<>();
76+
references.add(BASE_HTTP_REQUEST_DECODED_PARAMETERS);
77+
references.add(HTTP_SERVLET_INPUT_MESSAGE_DECODED_PARAMETERS);
78+
if (typePool.describe(NETTY_HTTP_REQUEST_CLASS_NAME).isResolved()) {
79+
references.add(NETTY_HTTP_REQUEST_DECODED_PARAMETERS);
7980
}
80-
};
81+
return references;
82+
}
8183
}
8284

8385
public static class GetDecodedFormParametersAdvice {

dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/ServletRequestBodyInstrumentation.java

+64-62
Original file line numberDiff line numberDiff line change
@@ -100,75 +100,77 @@ public String toString() {
100100

101101
@Override
102102
public AdviceTransformer transformer() {
103-
// transformer possible adding extra 3 methods to ServletInputStreamWrapper
104-
return new AdviceTransformer() {
105-
private final Map<ClassLoader, Boolean> injectedClassLoaders =
106-
Collections.synchronizedMap(new WeakHashMap<ClassLoader, Boolean>());
103+
return new CustomTransformer();
104+
}
107105

108-
@Override
109-
public DynamicType.Builder<?> transform(
110-
DynamicType.Builder<?> builder,
111-
TypeDescription typeDescription,
112-
ClassLoader classLoader,
113-
JavaModule module) {
114-
if (classLoader == null) {
115-
classLoader = BOOTSTRAP_CLASSLOADER_PLACEHOLDER;
116-
}
106+
// transformer possible adding extra 3 methods to ServletInputStreamWrapper
107+
static class CustomTransformer implements AdviceTransformer {
108+
private static final String STREAM_WRAPPER_TYPE =
109+
"datadog.trace.instrumentation.servlet.http.ServletInputStreamWrapper";
110+
private final Map<ClassLoader, Boolean> injectedClassLoaders =
111+
Collections.synchronizedMap(new WeakHashMap<ClassLoader, Boolean>());
117112

118-
if (injectedClassLoaders.containsKey(classLoader)) {
119-
return builder;
120-
}
121-
injectedClassLoaders.put(classLoader, Boolean.TRUE);
113+
@Override
114+
public DynamicType.Builder<?> transform(
115+
DynamicType.Builder<?> builder,
116+
TypeDescription typeDescription,
117+
ClassLoader classLoader,
118+
JavaModule module) {
119+
if (classLoader == null) {
120+
classLoader = BOOTSTRAP_CLASSLOADER_PLACEHOLDER;
121+
}
122122

123-
TypePool.Resolution readListenerRes;
124-
TypePool typePoolUserCl;
125-
if (classLoader != BOOTSTRAP_CLASSLOADER_PLACEHOLDER) {
126-
typePoolUserCl = TypePool.Default.of(classLoader);
127-
} else {
128-
typePoolUserCl = TypePool.Default.ofBootLoader();
129-
}
130-
readListenerRes = typePoolUserCl.describe("javax.servlet.ReadListener");
131-
if (!readListenerRes.isResolved()) {
132-
// likely servlet < 3.1
133-
// inject original
134-
return new HelperInjector(
135-
"servlet-request-body", new String[] {packageName + ".ServletInputStreamWrapper"})
136-
.transform(builder, typeDescription, classLoader, module);
137-
}
123+
if (injectedClassLoaders.containsKey(classLoader)) {
124+
return builder;
125+
}
126+
injectedClassLoaders.put(classLoader, Boolean.TRUE);
138127

139-
// else at the very least servlet 3.1+ classes are available
140-
// modify ServletInputStreamWrapper before injecting it. This should be harmless even if
141-
// servlet 3.1 is on the
142-
// classpath without the implementation supporting it
143-
ClassFileLocator compoundLocator =
144-
new ClassFileLocator.Compound(
145-
ClassFileLocator.ForClassLoader.of(getClass().getClassLoader()),
146-
ClassFileLocator.ForClassLoader.of(classLoader));
128+
TypePool.Resolution readListenerRes;
129+
TypePool typePoolUserCl;
130+
if (classLoader != BOOTSTRAP_CLASSLOADER_PLACEHOLDER) {
131+
typePoolUserCl = TypePool.Default.of(classLoader);
132+
} else {
133+
typePoolUserCl = TypePool.Default.ofBootLoader();
134+
}
135+
readListenerRes = typePoolUserCl.describe("javax.servlet.ReadListener");
136+
if (!readListenerRes.isResolved()) {
137+
// likely servlet < 3.1
138+
// inject original
139+
return new HelperInjector("servlet-request-body", STREAM_WRAPPER_TYPE)
140+
.transform(builder, typeDescription, classLoader, module);
141+
}
147142

148-
TypePool.Resolution origWrapperRes =
149-
TypePool.Default.of(compoundLocator)
150-
.describe(packageName + ".ServletInputStreamWrapper");
151-
if (!origWrapperRes.isResolved()) {
152-
throw new RuntimeException("Could not load original ServletInputStreamWrapper");
153-
}
154-
TypeDescription origWrapperType = origWrapperRes.resolve();
143+
// else at the very least servlet 3.1+ classes are available
144+
// modify ServletInputStreamWrapper before injecting it. This should be harmless even if
145+
// servlet 3.1 is on the
146+
// classpath without the implementation supporting it
147+
ClassFileLocator compoundLocator =
148+
new ClassFileLocator.Compound(
149+
ClassFileLocator.ForClassLoader.of(getClass().getClassLoader()),
150+
ClassFileLocator.ForClassLoader.of(classLoader));
155151

156-
DynamicType.Unloaded<?> unloaded =
157-
new ByteBuddy()
158-
.rebase(origWrapperType, compoundLocator)
159-
.method(named("isFinished").and(takesNoArguments()))
160-
.intercept(MethodDelegation.toField("is"))
161-
.method(named("isReady").and(takesNoArguments()))
162-
.intercept(MethodDelegation.toField("is"))
163-
.method(named("setReadListener").and(takesArguments(readListenerRes.resolve())))
164-
.intercept(MethodDelegation.toField("is"))
165-
.make();
166-
return new HelperInjector(
167-
"servlet-request-body",
168-
Collections.singletonMap(origWrapperType.getName(), unloaded.getBytes()))
169-
.transform(builder, typeDescription, classLoader, module);
152+
TypePool.Resolution origWrapperRes =
153+
TypePool.Default.of(compoundLocator).describe(STREAM_WRAPPER_TYPE);
154+
if (!origWrapperRes.isResolved()) {
155+
throw new RuntimeException("Could not load original ServletInputStreamWrapper");
170156
}
171-
};
157+
TypeDescription origWrapperType = origWrapperRes.resolve();
158+
159+
DynamicType.Unloaded<?> unloaded =
160+
new ByteBuddy()
161+
.rebase(origWrapperType, compoundLocator)
162+
.method(named("isFinished").and(takesNoArguments()))
163+
.intercept(MethodDelegation.toField("is"))
164+
.method(named("isReady").and(takesNoArguments()))
165+
.intercept(MethodDelegation.toField("is"))
166+
.method(named("setReadListener").and(takesArguments(readListenerRes.resolve())))
167+
.intercept(MethodDelegation.toField("is"))
168+
.make();
169+
return new HelperInjector(
170+
"servlet-request-body",
171+
Collections.singletonMap(origWrapperType.getName(), unloaded.getBytes()))
172+
.transform(builder, typeDescription, classLoader, module);
173+
}
172174
}
173175

174176
@SuppressWarnings("Duplicates")

0 commit comments

Comments
 (0)