-
Notifications
You must be signed in to change notification settings - Fork 312
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
Changes from 19 commits
478efec
3936f3f
86a5795
e91cef5
2ee919f
dad95dd
3142fec
fa3f320
54a68e9
a9e59d6
3505e86
f40741a
89292f8
8c79cba
365bc7f
be57c26
1416a77
8c825ef
57e8038
44f8ed9
8eed5ba
32b8a2a
f7b9cb0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -118,6 +118,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) { | ||||
|
@@ -126,12 +132,19 @@ 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 instrumentationName = | ||||
instrumentationNames().length == 0 ? "http-server" : instrumentationNames()[0]; | ||||
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()); | ||||
|
@@ -140,16 +153,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( | ||||
|
@@ -160,6 +164,11 @@ public AgentSpan onRequest( | |||
return onRequest(span, connection, request, getExtractedSpanContext(context)); | ||||
} | ||||
|
||||
public AgentSpanContext.Extracted getExtractedSpanContext(Context context) { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. About Line 127 in 1416a77
About Spray, I could try. I'm not that familiar with Scala, but that sounds doable. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, | ||||
|
@@ -375,34 +384,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) { | ||||
|
@@ -415,7 +413,7 @@ private AgentSpanContext.Extracted callIGCallbackStart(AgentSpanContext.Extracte | |||
return tagContext; | ||||
} | ||||
|
||||
return context; | ||||
return extracted; | ||||
} | ||||
|
||||
@Override | ||||
|
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; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wow good catch 😮 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
Uh oh!
There was an error while loading. Please reload this page.