Skip to content

Improve Instrumenter API to use Context instead of Span #9211

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
Aug 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
478efec
feat(context): Refactor HttpServerDecorator to return Context instead…
PerfectSlayer Jul 21, 2025
3936f3f
feat(context): Refactor HttpServerDecorator to return Context instead…
PerfectSlayer Jul 23, 2025
86a5795
WIP Reusing instrumentation name from instrumentation
PerfectSlayer Aug 5, 2025
e91cef5
WIP Context.with optimization for AgentSpan
PerfectSlayer Aug 5, 2025
2ee919f
wip(asm): Fix startSpan hook
PerfectSlayer Aug 4, 2025
dad95dd
MIP debug log
PerfectSlayer Aug 7, 2025
3142fec
feat(asm): Clean up DelayCertainInsMethodVisitor
PerfectSlayer Aug 7, 2025
fa3f320
WIP
PerfectSlayer Aug 7, 2025
54a68e9
WIP working Jetty 9
PerfectSlayer Aug 7, 2025
a9e59d6
feat(asm): Clean up blocker helper
PerfectSlayer Aug 7, 2025
3505e86
feat(asm): Improve block check injection
PerfectSlayer Aug 7, 2025
f40741a
feat(asm): Improve block check injection
PerfectSlayer Aug 8, 2025
89292f8
feat(asm): Improve block check injection
PerfectSlayer Aug 11, 2025
8c79cba
feat(asm): Fix injection check test
PerfectSlayer Aug 11, 2025
365bc7f
feat(asm): Restoring GOTO to avoid duplicate dispatch call when blocked
PerfectSlayer Aug 11, 2025
be57c26
feat(asm): Clean up visitors
PerfectSlayer Aug 11, 2025
1416a77
feat(core): Remove instrumentation name
PerfectSlayer Aug 11, 2025
8c825ef
feat(core): Clean up HttpServerDecorator
PerfectSlayer Aug 11, 2025
57e8038
feat(asm): Clean up visitors
PerfectSlayer Aug 11, 2025
44f8ed9
feat(play): Improve context usage in advice
PerfectSlayer Aug 12, 2025
8eed5ba
feat(bootstrap): Improve default instrumentation name for http servers
PerfectSlayer Aug 12, 2025
32b8a2a
feat: Remove Java8BytecodeBridge usage where it's not needed
PerfectSlayer Aug 12, 2025
f7b9cb0
feat(bootstrap): Removing optimization
PerfectSlayer Aug 12, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package datadog.trace.bootstrap.instrumentation.decorator;

import static datadog.trace.bootstrap.instrumentation.api.AgentSpan.fromContext;
import static java.util.concurrent.TimeUnit.MICROSECONDS;
import static java.util.concurrent.TimeUnit.SECONDS;

Expand Down Expand Up @@ -58,7 +59,8 @@ public void setUp() {
.build();
GlobalTracer.forceRegister(tracer);
decorator = new BenchmarkHttpServerDecorator();
span = decorator.startSpan(Collections.emptyMap(), (Context) null);
Context context = decorator.startSpan(Collections.emptyMap(), Context.root());
span = fromContext(context);
}

@Benchmark
Expand Down Expand Up @@ -99,7 +101,7 @@ public BenchmarkHttpServerDecorator() {

@Override
protected String[] instrumentationNames() {
return new String[0];
return new String[] {"benchmark"};
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ public abstract class HttpServerDecorator<REQUEST, CONNECTION, RESPONSE, REQUEST
Config.get().isRuleEnabled("URLAsResourceNameRule");

private static final BitSet SERVER_ERROR_STATUSES = Config.get().getHttpServerErrorStatuses();
private static final String DEFAULT_INSTRUMENTATION_NAME = "http-server";

private final boolean traceClientIpResolverEnabled =
Config.get().isTraceClientIpResolverEnabled();
Expand Down Expand Up @@ -118,6 +119,12 @@ protected AgentTracer.TracerAPI tracer() {
return AgentTracer.get();
}

/**
* Extracts context from an upstream service.
*
* @param carrier The request carrier to get the context from.
* @return The extracted context, {@code Context#root()} if no valid context to extract.
*/
public Context extract(REQUEST_CARRIER carrier) {
AgentPropagation.ContextVisitor<REQUEST_CARRIER> getter = getter();
if (null == carrier || null == getter) {
Expand All @@ -126,12 +133,22 @@ public Context extract(REQUEST_CARRIER carrier) {
return Propagators.defaultPropagator().extract(root(), carrier, getter);
}

public AgentSpan startSpan(
String instrumentationName, REQUEST_CARRIER carrier, AgentSpanContext.Extracted context) {
/**
* Starts a span.
*
* @param carrier The request carrier.
* @param context The parent context of the span to create.
* @return A new context bundling the span, child of the given parent context.
*/
public Context startSpan(REQUEST_CARRIER carrier, Context context) {
String[] instrumentationNames = instrumentationNames();
String instrumentationName =
instrumentationNames != null && instrumentationNames.length > 0
? instrumentationNames[0]
: DEFAULT_INSTRUMENTATION_NAME;
AgentSpanContext.Extracted extracted = callIGCallbackStart(context);
AgentSpan span =
tracer()
.startSpan(instrumentationName, spanName(), callIGCallbackStart(context))
.setMeasured(true);
tracer().startSpan(instrumentationName, spanName(), extracted).setMeasured(true);
Flow<Void> flow = callIGCallbackRequestHeaders(span, carrier);
if (flow.getAction() instanceof Flow.Action.RequestBlockingAction) {
span.setRequestBlockingAction((Flow.Action.RequestBlockingAction) flow.getAction());
Expand All @@ -140,16 +157,7 @@ public AgentSpan startSpan(
if (null != carrier && null != getter) {
tracer().getDataStreamsMonitoring().setCheckpoint(span, forHttpServer());
}
return span;
}

public AgentSpan startSpan(REQUEST_CARRIER carrier, Context context) {
return startSpan("http-server", carrier, getExtractedSpanContext(context));
}

public AgentSpanContext.Extracted getExtractedSpanContext(Context context) {
AgentSpan extractedSpan = AgentSpan.fromContext(context);
return extractedSpan == null ? null : (AgentSpanContext.Extracted) extractedSpan.context();
return context.with(span);
}

public AgentSpan onRequest(
Expand All @@ -160,6 +168,11 @@ public AgentSpan onRequest(
return onRequest(span, connection, request, getExtractedSpanContext(context));
}

public AgentSpanContext.Extracted getExtractedSpanContext(Context context) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this something that we can set as a private method now? It seems like the only two instrumentations that use this still are SprayHttpServerRunSealedRouteAdvice and TomcatServerInstrumentation. (Albeit modifications would have to be made to those functionality)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About TomcatServerInstrumentation, it feels I would be picking your leftover (git blame this line 😬 ):

// TODO: Migrate setting DD_EXTRACTED_CONTEXT_ATTRIBUTE from AgentSpanContext.Extracted to

About Spray, I could try. I'm not that familiar with Scala, but that sounds doable.
What about doing it in a follow up PR? I expect to have even more refactoring coming to make gateway inferred span happen.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow up on #9358

AgentSpan extractedSpan = AgentSpan.fromContext(context);
return extractedSpan == null ? null : (AgentSpanContext.Extracted) extractedSpan.context();
}

public AgentSpan onRequest(
final AgentSpan span,
final CONNECTION connection,
Expand Down Expand Up @@ -375,34 +388,23 @@ public AgentSpan onResponse(final AgentSpan span, final RESPONSE response) {
return span;
}

// @Override
// public Span onError(final Span span, final Throwable throwable) {
// assert span != null;
// // FIXME
// final Object status = span.getTag("http.status");
// if (status == null || status.equals(200)) {
// // Ensure status set correctly
// span.setTag("http.status", 500);
// }
// return super.onError(span, throwable);
// }

private AgentSpanContext.Extracted callIGCallbackStart(AgentSpanContext.Extracted context) {
private AgentSpanContext.Extracted callIGCallbackStart(final Context context) {
AgentSpanContext.Extracted extracted = getExtractedSpanContext(context);
AgentTracer.TracerAPI tracer = tracer();
Supplier<Flow<Object>> startedCbAppSec =
tracer.getCallbackProvider(RequestContextSlot.APPSEC).getCallback(EVENTS.requestStarted());
Supplier<Flow<Object>> startedCbIast =
tracer.getCallbackProvider(RequestContextSlot.IAST).getCallback(EVENTS.requestStarted());

if (startedCbAppSec == null && startedCbIast == null) {
return context;
return extracted;
}

TagContext tagContext = null;
if (context == null) {
if (extracted == null) {
tagContext = new TagContext();
} else if (context instanceof TagContext) {
tagContext = (TagContext) context;
} else if (extracted instanceof TagContext) {
tagContext = (TagContext) extracted;
}

if (tagContext != null) {
Expand All @@ -415,7 +417,7 @@ private AgentSpanContext.Extracted callIGCallbackStart(AgentSpanContext.Extracte
return tagContext;
}

return context;
return extracted;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import datadog.trace.core.datastreams.DataStreamsMonitoring
import java.util.function.Function
import java.util.function.Supplier

import static datadog.context.Context.root
import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_SERVER_DECODED_RESOURCE_PRESERVE_SPACES
import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_SERVER_RAW_QUERY_STRING
import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_SERVER_RAW_RESOURCE
Expand Down Expand Up @@ -497,7 +498,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest {
def decorator = newDecorator(mTracer, null)

when:
decorator.startSpan("test", headers, null)
decorator.startSpan(headers, root())

then:
1 * mSpan.setMeasured(true) >> mSpan
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package datadog.trace.instrumentation.akkahttp;

import static datadog.trace.bootstrap.instrumentation.api.AgentSpan.fromContext;
import static datadog.trace.instrumentation.akkahttp.AkkaHttpServerDecorator.DECORATE;

import akka.http.scaladsl.model.HttpRequest;
Expand All @@ -10,12 +11,13 @@

public class DatadogWrapperHelper {
public static ContextScope createSpan(final HttpRequest request) {
final Context context = DECORATE.extract(request);
final AgentSpan span = DECORATE.startSpan(request, context);
final Context parentContext = DECORATE.extract(request);
final Context context = DECORATE.startSpan(request, parentContext);
final AgentSpan span = fromContext(context);
DECORATE.afterStart(span);
DECORATE.onRequest(span, request, request, context);
DECORATE.onRequest(span, request, request, parentContext);

return context.with(span).attach();
return context.attach();
}

public static void finishSpan(final AgentSpan span, final HttpResponse response) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package datadog.trace.instrumentation.azurefunctions;
package datadog.trace.instrumentation.azure.functions;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow good catch 😮

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I need to review the spec to find out how this even work 🤷


import com.microsoft.azure.functions.HttpRequestMessage;
import com.microsoft.azure.functions.HttpResponseMessage;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package datadog.trace.instrumentation.azurefunctions;
package datadog.trace.instrumentation.azure.functions;

import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.declaresMethod;
import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.isAnnotatedWith;
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static datadog.trace.bootstrap.instrumentation.api.Java8BytecodeBridge.spanFromContext;
import static datadog.trace.bootstrap.instrumentation.api.AgentSpan.fromContext;
import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR;
import static datadog.trace.instrumentation.azurefunctions.AzureFunctionsDecorator.DECORATE;
import static datadog.trace.instrumentation.azure.functions.AzureFunctionsDecorator.DECORATE;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
Expand Down Expand Up @@ -65,23 +65,24 @@ public void methodAdvice(MethodTransformer transformer) {
public static class AzureFunctionsAdvice {
@Advice.OnMethodEnter(suppress = Throwable.class)
public static ContextScope methodEnter(
@Advice.Argument(0) final HttpRequestMessage request,
@Advice.Argument(1) final ExecutionContext context) {
final Context extractedContext = DECORATE.extract(request);
final AgentSpan span = DECORATE.startSpan(request, extractedContext);
DECORATE.afterStart(span, context.getFunctionName());
DECORATE.onRequest(span, request, request, extractedContext);
@Advice.Argument(0) final HttpRequestMessage<?> request,
@Advice.Argument(1) final ExecutionContext executionContext) {
final Context parentContext = DECORATE.extract(request);
final Context context = DECORATE.startSpan(request, parentContext);
final AgentSpan span = fromContext(context);
DECORATE.afterStart(span, executionContext.getFunctionName());
DECORATE.onRequest(span, request, request, parentContext);
HTTP_RESOURCE_DECORATOR.withRoute(
span, request.getHttpMethod().name(), request.getUri().getPath());
return extractedContext.with(span).attach();
return context.attach();
}

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void methodExit(
@Advice.Enter final ContextScope scope,
@Advice.Return final HttpResponseMessage response,
@Advice.Thrown final Throwable throwable) {
final AgentSpan span = spanFromContext(scope.context());
final AgentSpan span = fromContext(scope.context());
DECORATE.onError(span, throwable);
DECORATE.onResponse(span, response);
DECORATE.beforeFinish(span);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package datadog.trace.instrumentation.azurefunctions;
package datadog.trace.instrumentation.azure.functions;

import com.microsoft.azure.functions.HttpRequestMessage;
import datadog.trace.bootstrap.instrumentation.api.AgentPropagation;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.agent.tooling.InstrumenterModule;
import datadog.trace.api.CorrelationIdentifier;
import datadog.trace.api.GlobalTracer;
import datadog.trace.api.gateway.Flow;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import net.bytebuddy.asm.Advice;
Expand Down Expand Up @@ -74,15 +73,17 @@ public static class HandleAdvice {
}

final Context parentContext = DECORATE.extract(request);
final AgentSpan span = DECORATE.startSpan(request, parentContext);
final Context context = DECORATE.startSpan(request, parentContext);
final AgentSpan span = spanFromContext(context);
DECORATE.afterStart(span);
DECORATE.onRequest(span, request, request, parentContext);

scope = parentContext.with(span).attach();
scope = context.attach();

request.setAttribute(DD_SPAN_ATTRIBUTE, span);
request.setAttribute(CorrelationIdentifier.getTraceIdKey(), GlobalTracer.get().getTraceId());
request.setAttribute(CorrelationIdentifier.getSpanIdKey(), GlobalTracer.get().getSpanId());
request.setAttribute(
CorrelationIdentifier.getTraceIdKey(), CorrelationIdentifier.getTraceId());
request.setAttribute(CorrelationIdentifier.getSpanIdKey(), CorrelationIdentifier.getSpanId());

Flow.Action.RequestBlockingAction rba = span.getRequestBlockingAction();
if (rba != null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package datadog.trace.instrumentation.grizzlyhttp232;

import static datadog.trace.bootstrap.instrumentation.api.Java8BytecodeBridge.spanFromContext;

import datadog.appsec.api.blocking.BlockingContentType;
import datadog.context.Context;
import datadog.context.ContextScope;
Expand Down Expand Up @@ -112,13 +114,14 @@ public static NextAction onHttpCodecFilterExit(
}
HttpRequestPacket httpRequest = (HttpRequestPacket) httpHeader;
HttpResponsePacket httpResponse = httpRequest.getResponse();
Context context = DECORATE.extract(httpRequest);
AgentSpan span = DECORATE.startSpan(httpRequest, context);
ContextScope scope = context.with(span).attach();
Context parentContext = DECORATE.extract(httpRequest);
Context context = DECORATE.startSpan(httpRequest, parentContext);
ContextScope scope = context.attach();
AgentSpan span = spanFromContext(context);
DECORATE.afterStart(span);
ctx.getAttributes().setAttribute(DD_SPAN_ATTRIBUTE, span);
ctx.getAttributes().setAttribute(DD_RESPONSE_ATTRIBUTE, httpResponse);
DECORATE.onRequest(span, httpRequest, httpRequest, context);
DECORATE.onRequest(span, httpRequest, httpRequest, parentContext);

Flow.Action.RequestBlockingAction rba = span.getRequestBlockingAction();
if (rba != null && thiz instanceof HttpServerFilter) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package datadog.trace.instrumentation.jetty11;

import static datadog.trace.bootstrap.instrumentation.api.AgentSpan.fromContext;
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan;
import static datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator.DD_SPAN_ATTRIBUTE;
import static datadog.trace.instrumentation.jetty11.JettyDecorator.DECORATE;

import datadog.context.Context;
import datadog.context.ContextScope;
import datadog.trace.api.CorrelationIdentifier;
import datadog.trace.api.GlobalTracer;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import net.bytebuddy.asm.Advice;
import org.eclipse.jetty.server.HttpChannel;
Expand All @@ -26,16 +26,17 @@ public static ContextScope onEnter(
return activateSpan((AgentSpan) existingSpan);
}

final Context context = DECORATE.extract(req);
span = DECORATE.startSpan(req, context);
final ContextScope scope = context.with(span).attach();
final Context parentContext = DECORATE.extract(req);
final Context context = DECORATE.startSpan(req, parentContext);
final ContextScope scope = context.attach();
span = fromContext(context);
span.setMeasured(true);
DECORATE.afterStart(span);
DECORATE.onRequest(span, req, req, context);
DECORATE.onRequest(span, req, req, parentContext);

req.setAttribute(DD_SPAN_ATTRIBUTE, span);
req.setAttribute(CorrelationIdentifier.getTraceIdKey(), GlobalTracer.get().getTraceId());
req.setAttribute(CorrelationIdentifier.getSpanIdKey(), GlobalTracer.get().getSpanId());
req.setAttribute(CorrelationIdentifier.getTraceIdKey(), CorrelationIdentifier.getTraceId());
req.setAttribute(CorrelationIdentifier.getSpanIdKey(), CorrelationIdentifier.getSpanId());
return scope;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class HttpChannelAppSecTest extends AgentTestRunner {

@Override
void visitMethodInsn(int opcode, String owner, String name, String descriptor, boolean isInterface) {
if ('datadog/trace/bootstrap/instrumentation/api/AgentSpan' == owner && 'getRequestBlockingAction' == name) {
if ('datadog/trace/instrumentation/jetty/JettyBlockingHelper' == owner && 'hasRequestBlockingAction' == name) {
blockApplied = true
}
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface)
Expand Down
Loading