opentracing / specification

A place to document (and discuss) the OpenTracing specification. 🛑 This project is DEPRECATED! https://github.com/opentracing/specification/issues/163
http://opentracing.io/spec
Apache License 2.0
1.18k stars 182 forks source link

Tracer extract ambiguous handling of corrupted span context #33

Open pavolloffay opened 7 years ago

pavolloffay commented 7 years ago

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

yurishkuro commented 7 years ago

@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.
bhs commented 7 years ago

@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):

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 commented 7 years ago

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