open-telemetry / opentelemetry-go

OpenTelemetry Go API and SDK
https://opentelemetry.io/docs/languages/go
Apache License 2.0
5.25k stars 1.06k forks source link

[OpenTracing Bridge] Support returning non-nil bridgeSpanContext in `Extract` when traceId and SpanId are not present in OTel SpanContext #3952

Open ChenX1993 opened 1 year ago

ChenX1993 commented 1 year ago

Problem Statement

Hi, really appreciate anyone who contributed to the OpenTracing Bridge.

The request is to make func (t *BridgeTracer) Extract(format interface{}, carrier interface{}) (ot.SpanContext, error) method able to conditionally return non-nil bridgeSpanContext when traceId and SpanId are not present OTel SpanContext.

Currently it returns nil when OTel SpanContext is invalid - no valid traceId and SpanId: https://github.com/open-telemetry/opentelemetry-go/blob/640a0cd8bc3844453976c2619c40bee4a1e4a07c/bridge/opentracing/bridge.go#L731-L734

The reason is to support jaeger-debug-id use case in OTel when we are migrating from deprecated jaeger-client-go.

To quickly explain what jaeger-debug-id does: it allows users to send a inbound request with such jaeger-debug-id header present to enforce sampling without a parent span. Screenshot 2023-03-31 at 6 32 57 AM

I can implement a custom propagator to extract the header in maybe TraceState in OTel SpanContext, and implement a custom sampler to detect if it is present in the TraceState to sample the span and to set the attribute.

The only blocker right now is the OpenTracing Bridge Extract method: it only returns a non-nil bridge Span Context when there OTel SpanContext is valid - traceId and SpanId are valid, so there is not way to pass the jaeger-debug-id info to the OTel tracer.Start().

Proposed Solution

Since the jaeger-debug-id is very specific to jaeger, and not general to OpenTracing. I am thinking about more generic way to solve it.

One approach I can think of is to make such change:

if !bridgeSC.otelSpanContext.IsValid() && !bridgeSC.otelSpanContext.isSampled {
    return nil, ot.ErrSpanContextNotFound
}

Then in my custom propagator, I can set the traceFlags to sampled when extracting jaeger-debug-id header.

Or

if !bridgeSC.otelSpanContext.IsValid() && bridgeSC.otelSpanContext.TraceState.Get("force.passthrough") == "" {
    return nil, ot.ErrSpanContextNotFound
}

Hope this request make sense. Thanks!

Kaushal28 commented 1 year ago

Hi, can I pick this up?

ChenX1993 commented 1 year ago

Hi @Kaushal28,

Can I take this? Because we use jaeger-debug-id heavily on our side, and I will need to do some internal testings along with the OSS code change to make sure it can really work.

I am also doing the same thing on Java: https://github.com/open-telemetry/opentelemetry-java/issues/5339