Open tylerohlsen opened 2 years ago
Thanks for bringing this to our attention, Tyler. I had not looked at this API in a long time, so I had to take some time to review it. And I agree, it's not great:
SpanBuilder.AsChildOf(OpenTracing.ISpanContext)
, it's not clear to the caller the we expect the type to be OpenTracingSpanContext
OpenTracing.ISpanContext
implementation, we fail silentlyOpenTracingSpanContext
because it is internal
There is one very convoluted way to get an OpenTracingSpanContext
instance that you can use now without changes to the tracer:
public OpenTracing.ISpanContext CreateSpanContext(OpenTracing.ITracer tracer, ulong traceId, ulong parentSpanId)
{
var contextValues = new Dictionary<string, string>
{
{ Datadog.Trace.HttpHeaderNames.TraceId, traceId.ToString(CultureInfo.InvariantCulture) },
{ Datadog.Trace.HttpHeaderNames.ParentId, parentSpanId.ToString(CultureInfo.InvariantCulture) }
};
var textMap = new OpenTracing.Propagation.TextMapExtractAdapter(contextValues);
return tracer.Extract(OpenTracing.Propagation.BuiltinFormats.TextMap, textMap);
}
// usage
OpenTracing.ITracer tracer = ...;
ulong traceId = ...;
ulong parentSpanId = ...;
OpenTracing.ISpanContext spanContext = CreateSpanContext(tracer, traceId, parentSpanId);
OpenTracing.ISpanBuilder builder = tracer.BuildSpan(...).AsChildOf(spanContext);
I can think of two changes we can make to improve this situation. We could implement either one or both:
OpenTracingSpanContext
with the provided traceId and spanId without having to use the workaround above. For example, we could make OpenTracingSpanContext
public and add a public constructor like:
public OpenTracingSpanContext(ulong traceId, ulong spanId);
OpenTracingSpanContext
and allow any user-defined implementation of OpenTracing.ISpanContext
. One concern I have is that we would need to convert the string
traceId and spanId to ulong
, and those conversions can fail. Maybe we can throw an exception in SpanBuilder.AsChildOf()
if that happens?IMO, both of those changes would be beneficial.
I have my own way of triggering a downstream call to another process that I want to correlate with the upstream process trace. I don't see a way with the OpenTracing API (v2.0.1) for me to create a new parent context.
SpanBuilder.AsChildOf
does allow me to pass in anISpanContext
, but privately theSpanBuilder
requires thatISpanContext
to be of typeOpenTracingSpanContext
(reference) which is internal.