Skip to content

Commit

Permalink
Prevent AppSec context from being closed more than once on partial flush
Browse files Browse the repository at this point in the history
We had some hooks in `CoreTracer.write` meant to be run whenever a root
span is finished. However, they were effectively called not just when
the root span finished, but also whenever a partial flush on a child
span was performed. This ended up calling the hooks multiple teams, and
earlier than expected.
  • Loading branch information
smola committed May 22, 2024
1 parent 08f3098 commit 4f31ab5
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 12 deletions.
33 changes: 22 additions & 11 deletions dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java
Original file line number Diff line number Diff line change
Expand Up @@ -240,9 +240,31 @@ PropagationTags.Factory getPropagationTagsFactory() {
return propagationTagsFactory;
}

/**
* Called when a root span is finished before it is serialized. This is might be called multiple times per root span.
* If a child span is part of a partial flush, this method will be called for its root even if not finished.
*/
@Override
public void onRootSpanFinished(AgentSpan root, EndpointTracker tracker) {
profilingContextIntegration.onRootSpanFinished(root, tracker);

}

/**
* Called when a root span is finished before it is serialized.
* This is guaranteed to called exactly once per root span.
*/
void onRootSpanPublished(final AgentSpan root) {
// Request context is propagated to contexts in child spans.
// Assume here that if present it will be so starting inthe top span.
RequestContext requestContext = root.getRequestContext();
if (requestContext != null) {
try {
requestContext.close();
} catch (IOException e) {
log.warn("Error closing request context data", e);
}
}
}

@Override
Expand Down Expand Up @@ -971,17 +993,6 @@ void write(final List<DDSpan> trace) {
}
if (null != rootSpan) {
onRootSpanFinished(rootSpan, rootSpan.getEndpointTracker());

// request context is propagated to contexts in child spans
// Assume here that if present it will be so starting in the top span
RequestContext requestContext = rootSpan.getRequestContext();
if (requestContext != null) {
try {
requestContext.close();
} catch (IOException e) {
log.warn("Error closing request context data", e);
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,11 @@ PublishState onPublish(final DDSpan span) {
// write method.
healthMetrics.onFinishSpan();
COMPLETED_SPAN_COUNT.incrementAndGet(this);
return decrementRefAndMaybeWrite(span == getRootSpan());
final DDSpan rootSpan = getRootSpan();
if (span == rootSpan) {
tracer.onRootSpanPublished(rootSpan);
}
return decrementRefAndMaybeWrite(span == rootSpan);
}

@Override
Expand Down

0 comments on commit 4f31ab5

Please sign in to comment.