dd-trace-dotnet icon indicating copy to clipboard operation
dd-trace-dotnet copied to clipboard

OpenTracing expose a way to create a parent span context

Open tylerohlsen opened this issue 3 years ago • 2 comments

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 an ISpanContext, but privately the SpanBuilder requires that ISpanContext to be of type OpenTracingSpanContext (reference) which is internal.

tylerohlsen avatar Jan 04 '22 15:01 tylerohlsen

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:

  • from the signature of SpanBuilder.AsChildOf(OpenTracing.ISpanContext), it's not clear to the caller the we expect the type to be OpenTracingSpanContext
  • if you pass any other OpenTracing.ISpanContext implementation, we fail silently
  • there's no way to create your own OpenTracingSpanContext 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:

  • Add a public API to create a "stand-alone" 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);
  • Remove the current restriction that the parent must be of type 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?

lucaspimentel avatar Jan 12 '22 22:01 lucaspimentel

IMO, both of those changes would be beneficial.

tylerohlsen avatar Jan 14 '22 15:01 tylerohlsen