opencensus-specs icon indicating copy to clipboard operation
opencensus-specs copied to clipboard

Apache SkyWalking asks for an extendable mechanism of SpanContext

Open wu-sheng opened this issue 7 years ago • 9 comments

Hi, I am trying to build OC exporters for SkyWalking in Java, go and C++. First, I use Java. But I am facing a block because SkyWalking requires a totally different header with Zipkin B3.

SW3 header: https://github.com/apache/incubator-skywalking/blob/master/docs/en/Skywalking-Cross-Process-Propagation-Headers-Protocol-v1.md#header-item

A simple scenario is, in Tomcat span -> HTTP client(outgoing) span request, when build the span in HTTP client, do inject I think, we need the Context from Tomcat span.

For that, we don't just need to inject/extract, we need to put exact info into the ** SpanContext**. From Java's SpanContext, it is un-extendable. I have checked with @songy23 , look like it is.

So, here, I am asking, could we discuss this mechanism? @bogdandrutu @rakyll @ramonza

wu-sheng avatar Jul 03 '18 22:07 wu-sheng

I expect to support extra key-value pair in ** SpanContext**, which only propagate in Process only. Then **Inject/Extract` could work by using that context.

wu-sheng avatar Jul 03 '18 22:07 wu-sheng

@bogdandrutu @SergeyKanzhelev @wu-sheng @songy23 @rakyll @ramonza Having vendor specific information in the context (SpanContext) is a generic requirement:

  1. put vendor-specific trace information in SpanContext and propagate within the process
  2. register callback, get notified when a child span is created so there is a chance for the callback to update vendor-specific info in SpanContext
  3. propagate vendor specific trace info across process boundaries: 3.1 (while receiving requests) deserialize from the request headers and update the SpanContext 3.2 (while sending requests) extract the info from SpanContext and update the request headers

#70 has proposed a solution for this, please refer to the "tracestate" section.

reyang avatar Jul 06 '18 20:07 reyang

@reyang agree. Hope we can have these interop points.

wu-sheng avatar Jul 06 '18 20:07 wu-sheng

fyi you can look at brave which has a design including these features and not requiring inheritance.

context has a list of "extra" which propagation plugins can add to. there is a hook "decorate" when creating or resuming a span to update this.

https://github.com/openzipkin/brave/blob/master/brave/src/main/java/brave/propagation/Propagation.java

On Sat, 7 Jul 2018, 04:59 吴晟 Wu Sheng, [email protected] wrote:

@reyang https://github.com/reyang agree. Hope we can have these interop points.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/census-instrumentation/opencensus-specs/issues/131#issuecomment-403145402, or mute the thread https://github.com/notifications/unsubscribe-auth/AAD613L-B-eNJv_2ouCUkiisyhEvanahks5uD8-bgaJpZM4VBti1 .

codefromthecrypt avatar Jul 06 '18 23:07 codefromthecrypt

@adriancole Could you provide an example to show how to use the extendable mechanism? Such as exact context key-value, notification when creating a span or inject/extract implementor.

wu-sheng avatar Jul 06 '18 23:07 wu-sheng

@adriancole https://github.com/adriancole Could you provide an example to show how to use the extendable mechanism? Such as exact context key-value, notification when creating a span or inject/extract implementor.

this is example of pushing "extra stuff" though the WIP is incomplete https://github.com/openzipkin/brave/pull/693/files#diff-76f1eb805a846fbcce8a653b259c5457R28

The callsites of the decoration hook are routine things that make a new span, Tracer.toSpan(context) and anything that materializes a span from the current context.

Currently all propagation impls have copy-on-write semantics so decoration is normalized through one path: https://github.com/openzipkin/brave/blob/9d98d569940c4e68c834a8ada4fe70d51912e2eb/brave/src/main/java/brave/internal/PropagationFields.java#L23

this is needed for routine propagation, where creation of a child context should fork the parent, but and allow mutations downwards but not upwards. We don't currently have a decoration use case besides this. Eventhough everything is handled currently with one logic, it is notable that there can be multiple hooks ex. propagate amazon fields and also trace context without them interfering with eachother or race condition. That's why it is a list of extra, not a single object.

That said, if there was a good in-process propagation object, a in-process tags library could obviate a lot of the need for abstract hooks. For example, one that can do arbitrary scoping of data which might not be propagated downstream. Meanwhile to create an api like this I think you'll need similar functionality. Would love to see other designs too as I had to wing this one for lack of ability to find other things.. it wasn't easy.

codefromthecrypt avatar Jul 07 '18 00:07 codefromthecrypt

actually I should be clear I did find one thing close, just it was a bridge too far for what I needed. For the use cases we had.. it was primarily around forking and isolation, but less about transport data format and merging https://github.com/tracingplane/tracingplane-java

codefromthecrypt avatar Jul 07 '18 00:07 codefromthecrypt

@reyang @bogdandrutu Any update about this and #70 , to allow SkyWalking to extend ** SpanContext** ?

wu-sheng avatar Aug 26 '18 13:08 wu-sheng

@wu-sheng SpanContext now has a member called Tracestate (currently implemented for Java and Python), which is a key-value-pair. Please refer to #132 and https://github.com/census-instrumentation/opencensus-python/pull/259.

reyang avatar Aug 27 '18 16:08 reyang