Clarify return value of `tracer.extract()`

[yurishkuro]

Problem

Currently the extract() function's return value is documented as follows (from Javascript API):

     * @return {SpanContext}
     *         The extracted SpanContext, or null if no such SpanContext could
     *         be found in `carrier`

Given that headers is the only way the tracer is able to peek into inbound request, there are scenarios where headers contain other useful tracing information besides the actual span context. Here are some specific examples:

1. In Jaeger libraries we support a special header jaeger-debug-id which allows passing a correlation ID with the inbound request (such as one done via curl) as well as instructing the tracer to stamp the tracer as "debug" (or "forced sampled"). We use a special form of SpanContext that does not contain trace or span ID (since there are none), but does contain the value of this special header.
2. Jaeger libraries also support jaeger-baggage header that can be sent without having an actual span context, again useful mostly in the context of manual requests with curl.
3. A similar situation would occur with W3C Trace-Context spec where the sender may provide a trace ID that should be captured by the receiving tracer, but the tracer may not want to actually respect that as its internal trace ID (e.g. to avoid DoS attack) yet capture it as a correlation id.

All these examples require extract() method to return something that is not a real SpanContext instance.

Proposal

change the wording to "MAY return null if there is the carrier contains no span context".

Questions to address

• Should there be some isValid() method exposed by the SpanContext? A similar issue was raised in the past in the Java API where some people were strongly against allowing extract() to ever return a null value.

[wu-sheng]

Should there be some isValid() method exposed by the SpanContext?

SkyWalking has this method in core : https://github.com/apache/incubator-skywalking/blob/master/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/ContextManager.java#L144

A similar issue was raised in the past in the Java API where some people were strongly against allowing extract() to ever return a null value.

SkyWalking's ContextCarrier will never be NULL

[pavolloffay]

Should there be some isValid() method exposed by the SpanContext?

Should the consumer be aware of this detail? Should user know whether the context is valid or whether it's only a "debug" context? Isn't context with just jaeger-debug-id a valid context type in Jaeger?

[objectiser]

The current null return value is currently only useful to enable the code to determine if there is an active (sampled) trace, which it could use to decide whether to create a child span.

If this can be handled in a different way, i.e. SpanContext.isSampled(), then the extract could always return a SpanContext?

[pavolloffay]

The current null return value is currently only useful to enable the code to determine if there is an active (sampled) trace, which it could use to decide whether to create a child span.

A valid context is also returned if the span is not sampled.

[objectiser]

Good point - but in terms of current situation, I think the only usecase of returning null is to allow code to determine whether a trace is underway (sampled or not), and therefore whether it wants to participate in that trace. So if we want to always return a SpanContext, then we may just need a mechanism to allow the code to make the same decision.

[tedsuo]

I would expect a tracer to always return a valid span context in the situations you describe.

I only see the null case as relevant when you ask a tracer to extract from an invalid carrier. Instrumentation should not have to check null or IsSampled as a guard against adding a SpanContext as a parent.

[yurishkuro]

Proposal: change the wording to "MAY return null if there is the carrier contains no span context".