specification icon indicating copy to clipboard operation
specification copied to clipboard

Tracer extract ambiguous handling of corrupted span context

Open pavolloffay opened this issue 8 years ago • 3 comments

Hello,

API in different languages define different behavior when extracting corrupted span context from a carrier.

E.g. javadoc say to throw an exception if the context is corrupted. Comment in JS say to return null if no span context could be found but it's not talking about corrupted one.

It would be good to clarify this is the specification.

If the span context is corrupted should be an exception thrown or started new trace? This could depend on tracer configuration.

I run into this when looking at brave-ot: https://github.com/openzipkin/brave-opentracing/pull/14/files#r97917595 Also see @adriancole comments here: https://github.com/openzipkin/brave-opentracing/pull/14#issuecomment-275342413

Extract in the specification: Spec: https://github.com/opentracing/specification/blob/master/specification.md#extract-a-spancontext-from-a-carrier

Extract in APIs: Java: https://github.com/opentracing/opentracing-java/blob/master/opentracing-api/src/main/java/io/opentracing/Tracer.java#L76 JS: https://github.com/opentracing/opentracing-javascript/blob/master/src/tracer.js#L177 Python: https://github.com/opentracing/opentracing-python/blob/master/opentracing/tracer.py#L108 Go: https://github.com/opentracing/opentracing-go/blob/master/tracer.go#L112

pavolloffay avatar Jan 26 '17 12:01 pavolloffay

@pavolloffay I think there are two independent issues here.

  1. what is tracer expected to do when it finds no span context in the carrier or if the carrier is corrupted. So far the spec was not firm about it because different languages have different ways of communicating errors. I agree that the spec should be more clear about it, namely that the extract() method should explicitly communicate back to the instrumentation why it could not create the span context.
  2. what instrumentation is expected to do when the span context is not extracted. This is a harder problem since instrumentation itself is not part of the spec. I personally think that a new trace should always start when the span context cannot be extracted, because no requests should be executed by the system without propagating distributed context.

yurishkuro avatar Jan 26 '17 16:01 yurishkuro

@pavolloffay thanks for bringing this up, and especially for doing so on the spec repo... one of the frustrating things about discussing it, say, for Java on its own is that it's harder to reference cross-language consistency goals.

Strawman for the spec (i.e., without referring to specific languages):

  • Extract() must differentiate between a (successfully) extracted SpanContext, a corrupted SpanContext, and the lack of a SpanContext.
  • Extract() must not crash the process because the caller neglected to read a comment. (I.e., if it throws an exception, it should be a checked exception)

Per @yurishkuro's second point: I am enthusiastic about moving OT towards a model where the programmer basically just starts a span without knowing whether there's a parent lying around somewhere. This is (very) related to #23. It would certainly argue that a (new) trace is started, for what it's worth.

bhs avatar Jan 27 '17 00:01 bhs

btw, I have been talking with @tedsuo about this... should have a POC sketch "soon".

bhs avatar Feb 10 '17 06:02 bhs