Skip to content

Commit 74c3fa6

Browse files
mccullsclaude
andcommitted
Codebase consistency: call scope.close() before span.finish()
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 0c91932 commit 74c3fa6

62 files changed

Lines changed: 190 additions & 118 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ docs/ Developer documentation (see below)
6363
- **Forked tests**: Use `ForkedTest` suffix when tests need a separate JVM
6464
- **Flaky tests**: Annotate with `@Flaky` — they are skipped in CI by default
6565
- **Instrumentation one-shot methods**: Never extract the return values of `triggerClasses()`, `contextStore()`, `classLoaderMatcher()`, or `methodAdvice()` into static constants. These are called once by the framework — extracting to a constant adds constant-pool bloat with no benefit.
66+
- **Scope lifecycle order**: Always decorator calls → `scope.close()``span.finish()`. Closing the scope first restores the parent active span, breaking IAST/AppSec callbacks in `beforeFinish` that rely on `AgentTracer.activeSpan()`.
6667

6768
## PR conventions
6869

dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/AkkaHttpSingleRequestInstrumentation.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,13 @@ public static void methodExit(
9898

9999
if (throwable == null) {
100100
responseFuture.onComplete(new OnCompleteHandler(span), thiz.system().dispatcher());
101+
scope.close();
101102
} else {
102103
DECORATE.onError(span, throwable);
103104
DECORATE.beforeFinish(span);
105+
scope.close();
104106
span.finish();
105107
}
106-
scope.close();
107108
}
108109
}
109110

dd-java-agent/instrumentation/akka/akka-http/akka-http-10.6/src/main/java11/datadog/trace/instrumentation/akkahttp106/SingleRequestAdvice.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,12 @@ public static void methodExit(
4444
if (throwable == null) {
4545
responseFuture.onComplete(
4646
new AkkaHttpClientHelpers.OnCompleteHandler(span), thiz.system().dispatcher());
47+
scope.close();
4748
} else {
4849
DECORATE.onError(span, throwable);
4950
DECORATE.beforeFinish(span);
51+
scope.close();
5052
span.finish();
5153
}
52-
scope.close();
5354
}
5455
}

dd-java-agent/instrumentation/apache-httpclient/apache-httpclient-5.0/src/main/java/datadog/trace/instrumentation/apachehttpclient5/ApacheHttpAsyncClientInstrumentation.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,15 +142,17 @@ public static void methodExit(
142142
if (scope == null) {
143143
return;
144144
}
145+
final AgentSpan span = scope.span();
145146
try {
146147
if (throwable != null) {
147-
final AgentSpan span = scope.span();
148148
DECORATE.onError(span, throwable);
149149
DECORATE.beforeFinish(span);
150-
span.finish();
151150
}
152151
} finally {
153152
scope.close();
153+
if (throwable != null) {
154+
span.finish();
155+
}
154156
}
155157
}
156158
}

dd-java-agent/instrumentation/armeria/armeria-grpc-0.84/src/main/java/datadog/trace/instrumentation/armeria/grpc/client/ClientCallImplInstrumentation.java

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -122,12 +122,14 @@ public static void after(
122122
@Advice.Thrown Throwable error,
123123
@Advice.Local("$$ddSpan") AgentSpan span)
124124
throws Throwable {
125+
if (null != error && null != span) {
126+
DECORATE.onError(span, error);
127+
DECORATE.beforeFinish(span);
128+
}
125129
if (null != scope) {
126130
scope.close();
127131
}
128132
if (null != error && null != span) {
129-
DECORATE.onError(span, error);
130-
DECORATE.beforeFinish(span);
131133
span.finish();
132134
throw error;
133135
}
@@ -210,8 +212,8 @@ public static void closeObserver(
210212
@Advice.Enter AgentScope scope, @Advice.Argument(0) Status status) {
211213
if (null != scope) {
212214
DECORATE.onClose(scope.span(), status);
213-
scope.span().finish();
214215
scope.close();
216+
scope.span().finish();
215217
}
216218
}
217219
}
@@ -238,15 +240,17 @@ public static void closeObserver(
238240
@Advice.Argument(0) Status status,
239241
@Advice.FieldValue("closed") boolean closed) {
240242
if (null != scope) {
243+
AgentSpan span = null;
241244
if (closed) {
242-
AgentSpan span =
243-
InstrumentationContext.get(ClientCall.class, AgentSpan.class).remove(call);
245+
span = InstrumentationContext.get(ClientCall.class, AgentSpan.class).remove(call);
244246
if (span != null) {
245247
DECORATE.onClose(span, status);
246-
span.finish();
247248
}
248249
}
249250
scope.close();
251+
if (span != null) {
252+
span.finish();
253+
}
250254
}
251255
}
252256
}
@@ -268,8 +272,8 @@ public static AgentScope before() {
268272
@Advice.OnMethodExit(onThrowable = Throwable.class)
269273
public static void after(@Advice.Enter AgentScope scope) {
270274
if (null != scope) {
271-
scope.span().finish();
272275
scope.close();
276+
scope.span().finish();
273277
}
274278
}
275279
}

dd-java-agent/instrumentation/aws-java/aws-java-lambda-handler-1.2/src/main/java/datadog/trace/instrumentation/aws/v1/lambda/LambdaHandlerInstrumentation.java

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -119,15 +119,12 @@ static void exit(
119119

120120
CallDepthThreadLocalMap.reset(RequestHandler.class);
121121

122+
final AgentSpan span = scope.span();
122123
try {
123-
final AgentSpan span = scope.span();
124-
if (throwable != null) {
125-
span.addThrowable(throwable);
126-
}
127-
String lambdaRequestId = awsContext.getAwsRequestId();
128-
129124
if (throwable == null) {
130125
AgentTracer.get().notifyAppSecEnd(span, result);
126+
} else {
127+
span.addThrowable(throwable);
131128
}
132129
// Force the resource name back to the literal placeholder marker right
133130
// before finish so that the Datadog Lambda Extension's filter
@@ -144,10 +141,11 @@ static void exit(
144141
// and the HTTP/JAX-RS instrumentation will already have written
145142
// HTTP_FRAMEWORK_ROUTE (3) by this point.
146143
span.setResourceName(INVOCATION_SPAN_NAME, ResourceNamePriorities.TAG_INTERCEPTOR);
147-
span.finish();
148-
AgentTracer.get().notifyExtensionEnd(span, result, null != throwable, lambdaRequestId);
149144
} finally {
150145
scope.close();
146+
span.finish();
147+
AgentTracer.get()
148+
.notifyExtensionEnd(span, result, null != throwable, awsContext.getAwsRequestId());
151149
}
152150
}
153151
}

dd-java-agent/instrumentation/cics-9.1/src/main/java/datadog/trace/instrumentation/cics/ECIInteractionInstrumentation.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,11 @@ public static void exit(
5050
CallDepthThreadLocalMap.decrementCallDepth(ECIInteraction.class);
5151

5252
if (null != scope) {
53-
DECORATE.onError(scope.span(), throwable);
54-
DECORATE.beforeFinish(scope.span());
55-
scope.span().finish();
53+
AgentSpan span = scope.span();
54+
DECORATE.onError(span, throwable);
55+
DECORATE.beforeFinish(span);
5656
scope.close();
57+
span.finish();
5758
}
5859
}
5960
}

dd-java-agent/instrumentation/cics-9.1/src/main/java/datadog/trace/instrumentation/cics/JavaGatewayInterfaceInstrumentation.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ public static void exit(
7171

7272
DECORATE.onError(span, throwable);
7373
DECORATE.beforeFinish(span);
74-
span.finish();
7574
scope.close();
75+
span.finish();
7676
}
7777
}
7878
}

dd-java-agent/instrumentation/cucumber-5.4/src/main/java/datadog/trace/instrumentation/cucumber/CucumberStepDecorator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public AgentScope onStepStart(StepDefinition step, Object[] arguments) {
4444
public void onStepFinish(AgentScope scope) {
4545
AgentSpan span = scope.span();
4646
beforeFinish(span);
47-
span.finish();
4847
scope.close();
48+
span.finish();
4949
}
5050
}

dd-java-agent/instrumentation/datanucleus-4.0.5/src/main/java/datadog/trace/instrumentation/datanucleus/ExecutionContextInstrumentation.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,8 @@ public static void endMethod(
9797
DECORATE.onError(span, throwable);
9898
DECORATE.beforeFinish(span);
9999

100-
span.finish();
101100
scope.close();
101+
span.finish();
102102
}
103103
}
104104

@@ -133,8 +133,8 @@ public static void endMethod(
133133
DECORATE.onOperation(span, entity);
134134
DECORATE.beforeFinish(span);
135135

136-
span.finish();
137136
scope.close();
137+
span.finish();
138138
}
139139
}
140140

@@ -164,8 +164,8 @@ public static void endMethod(
164164
DECORATE.onError(span, throwable);
165165
DECORATE.beforeFinish(span);
166166

167-
span.finish();
168167
scope.close();
168+
span.finish();
169169
}
170170
}
171171

@@ -195,8 +195,8 @@ public static void endMethod(
195195
DECORATE.onError(span, throwable);
196196
DECORATE.beforeFinish(span);
197197

198-
span.finish();
199198
scope.close();
199+
span.finish();
200200
}
201201
}
202202
}

0 commit comments

Comments
 (0)