Skip to content

Commit 63e0f86

Browse files
committed
feat(asm): Improve block check injection
Clean up local var hook and use Context.current() instead Refactor helper to use Context instead of AgentSpan and RequestBlockingAction
1 parent 00f9f7f commit 63e0f86

File tree

1 file changed

+58
-51
lines changed
  • dd-java-agent/instrumentation/jetty-common/src/main/java/datadog/trace/instrumentation/jetty9

1 file changed

+58
-51
lines changed

dd-java-agent/instrumentation/jetty-common/src/main/java/datadog/trace/instrumentation/jetty9/HandleVisitor.java

Lines changed: 58 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package datadog.trace.instrumentation.jetty9;
22

33
import static net.bytebuddy.jar.asm.Opcodes.ALOAD;
4-
import static net.bytebuddy.jar.asm.Opcodes.ASTORE;
54
import static net.bytebuddy.jar.asm.Opcodes.F_SAME;
65
import static net.bytebuddy.jar.asm.Opcodes.GOTO;
76
import static net.bytebuddy.jar.asm.Opcodes.H_INVOKESTATIC;
@@ -10,8 +9,6 @@
109
import static net.bytebuddy.jar.asm.Opcodes.INVOKEVIRTUAL;
1110

1211
import datadog.context.Context;
13-
import datadog.trace.api.gateway.Flow;
14-
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
1512
import datadog.trace.bootstrap.instrumentation.api.Java8BytecodeBridge;
1613
import datadog.trace.instrumentation.jetty.JettyBlockingHelper;
1714
import java.io.BufferedWriter;
@@ -107,9 +104,9 @@ public class HandleVisitor extends MethodVisitor {
107104
private static final int CONTEXT_VAR = 1000;
108105

109106
/** Whether the next store is supposed to store the Context variable. */
110-
private boolean lookForStore;
107+
// private boolean lookForStore;
111108
/** Whether the Context variable was stored to local index {@link #CONTEXT_VAR}. */
112-
private boolean contextStored;
109+
// private boolean contextStored;
113110
// private int contextVarIndex = -1;
114111
/** Whether the handle() method injection was successful .*/
115112
private boolean success;
@@ -155,19 +152,20 @@ DelayCertainInsMethodVisitor delayVisitorDelegate() {
155152
public void visitMethodInsn(
156153
int opcode, String owner, String name, String descriptor, boolean isInterface) {
157154
debug("visitMethodInsn");
158-
debug(">> contextStored: " + contextStored);
155+
// debug(">> contextStored: " + contextStored);
159156
debug(">> success: " + success);
160157
debug(">> opcode: " + opcode + ", owner: " + owner + ", name: " + name + ", descriptor: " + descriptor);
161-
if (!contextStored) {
162-
lookForStore =
163-
!lookForStore
164-
&& opcode == INVOKEVIRTUAL
165-
&& name.equals("startSpan")
166-
&& descriptor.endsWith("Ldatadog/context/Context;");
167-
if (lookForStore) {
168-
debug("Found store");
169-
}
170-
} else if (!success
158+
// if (!contextStored) {
159+
// lookForStore =
160+
// !lookForStore
161+
// && opcode == INVOKEVIRTUAL
162+
// && name.equals("startSpan")
163+
// && descriptor.endsWith("Ldatadog/context/Context;");
164+
// if (lookForStore) {
165+
// debug("Found store");
166+
// }
167+
// } else
168+
if (!success
171169
&& opcode == INVOKEVIRTUAL
172170
&& owner.equals("org/eclipse/jetty/server/Server")
173171
&& name.equals("handle")
@@ -267,17 +265,17 @@ public void visitMethodInsn(
267265
Label beforeRegularDispatch = new Label();
268266
Label afterRegularDispatch = new Label();
269267

270-
super.visitVarInsn(ALOAD, CONTEXT_VAR);
271-
super.visitJumpInsn(Opcodes.IFNULL, beforeRegularDispatch);
272-
super.visitVarInsn(ALOAD, CONTEXT_VAR);
273-
super.visitMethodInsn(
274-
Opcodes.INVOKEINTERFACE,
275-
"datadog/trace/bootstrap/instrumentation/api/AgentSpan",
276-
"getRequestBlockingAction",
277-
"()" + Type.getDescriptor(Flow.Action.RequestBlockingAction.class),
278-
true);
279-
super.visitJumpInsn(Opcodes.IFNONNULL, doBlockLabel);
280-
super.visitJumpInsn(GOTO, beforeRegularDispatch);
268+
// super.visitVarInsn(ALOAD, CONTEXT_VAR);
269+
// super.visitJumpInsn(Opcodes.IFNULL, beforeRegularDispatch);
270+
// super.visitVarInsn(ALOAD, CONTEXT_VAR);
271+
// super.visitMethodInsn(
272+
// Opcodes.INVOKEINTERFACE,
273+
// "datadog/trace/bootstrap/instrumentation/api/AgentSpan",
274+
// "getRequestBlockingAction",
275+
// "()" + Type.getDescriptor(Flow.Action.RequestBlockingAction.class),
276+
// true);
277+
// super.visitJumpInsn(Opcodes.IFNONNULL, doBlockLabel);
278+
// super.visitJumpInsn(GOTO, beforeRegularDispatch);
281279

282280
super.visitLabel(doBlockLabel);
283281
super.visitFrame(F_SAME, 0, null, 0, null);
@@ -301,21 +299,29 @@ public void visitMethodInsn(
301299
"getResponse",
302300
"()Lorg/eclipse/jetty/server/Response;",
303301
false);
304-
super.visitVarInsn(ALOAD, CONTEXT_VAR);
302+
// super.visitVarInsn(ALOAD, CONTEXT_VAR);
303+
// super.visitMethodInsn(
304+
// Opcodes.INVOKEINTERFACE,
305+
// "datadog/trace/bootstrap/instrumentation/api/AgentSpan",
306+
// "getRequestBlockingAction",
307+
// "()" + Type.getDescriptor(Flow.Action.RequestBlockingAction.class),
308+
// true);
305309
super.visitMethodInsn(
306-
Opcodes.INVOKEINTERFACE,
307-
"datadog/trace/bootstrap/instrumentation/api/AgentSpan",
308-
"getRequestBlockingAction",
309-
"()" + Type.getDescriptor(Flow.Action.RequestBlockingAction.class),
310-
true);
311-
super.visitVarInsn(ALOAD, CONTEXT_VAR);
310+
INVOKESTATIC,
311+
Type.getInternalName(Java8BytecodeBridge.class),
312+
"getCurrentContext",
313+
"()Ldatadog/context/Context;",
314+
false
315+
);
316+
// super.visitVarInsn(ALOAD, CONTEXT_VAR);
312317

313318
// create the lambda
314319
super.visitInvokeDynamicInsn(
315320
"dispatch",
316321
"(Lorg/eclipse/jetty/server/Request;Lorg/eclipse/jetty/server/Response;"
317-
+ Type.getDescriptor(Flow.Action.RequestBlockingAction.class)
318-
+ Type.getDescriptor(AgentSpan.class)
322+
// + Type.getDescriptor(Flow.Action.RequestBlockingAction.class)
323+
// + Type.getDescriptor(AgentSpan.class)
324+
+ Type.getDescriptor(Context.class)
319325
+ ")Lorg/eclipse/jetty/server/HttpChannel$Dispatchable;",
320326
new Handle(
321327
H_INVOKESTATIC,
@@ -330,8 +336,9 @@ public void visitMethodInsn(
330336
Type.getInternalName(JettyBlockingHelper.class),
331337
"blockAndThrowOnFailure",
332338
"(Lorg/eclipse/jetty/server/Request;Lorg/eclipse/jetty/server/Response;"
333-
+ Type.getDescriptor(Flow.Action.RequestBlockingAction.class)
334-
+ Type.getDescriptor(AgentSpan.class)
339+
// + Type.getDescriptor(Flow.Action.RequestBlockingAction.class)
340+
// + Type.getDescriptor(AgentSpan.class)
341+
+ Type.getDescriptor(Context.class)
335342
+ ")V",
336343
false),
337344
Type.getType("()V")
@@ -387,19 +394,19 @@ private boolean checkDispatchMethodState(final List<Runnable> savedVisitations)
387394
return last instanceof DelayCertainInsMethodVisitor.GetFieldInsn;
388395
}
389396

390-
@Override
391-
public void visitVarInsn(int opcode, int varIndex) {
392-
if (lookForStore && opcode == ASTORE) {
393-
debug("Found context");
394-
contextStored = true;
395-
lookForStore = false;
396-
// contextVarIndex = varIndex;
397-
// Duplicate on stack and store to its own local var
398-
// super.visitInsn(DUP);
399-
// super.visitVarInsn(ASTORE, CONTEXT_VAR);
400-
}
401-
super.visitVarInsn(opcode, varIndex);
402-
}
397+
// @Override
398+
// public void visitVarInsn(int opcode, int varIndex) {
399+
// if (lookForStore && opcode == ASTORE) {
400+
// debug("Found context");
401+
// contextStored = true;
402+
// lookForStore = false;
403+
//// contextVarIndex = varIndex;
404+
// // Duplicate on stack and store to its own local var
405+
//// super.visitInsn(DUP);
406+
//// super.visitVarInsn(ASTORE, CONTEXT_VAR);
407+
// }
408+
// super.visitVarInsn(opcode, varIndex);
409+
// }
403410

404411
// @Override
405412
// public void visitMaxs(int maxStack, int maxLocals) {

0 commit comments

Comments
 (0)