Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

Extend span APIs to support callbacks and access to spancontext #247

Open
reyang opened this issue Mar 14, 2019 · 13 comments
Open

Extend span APIs to support callbacks and access to spancontext #247

reyang opened this issue Mar 14, 2019 · 13 comments

Comments

@reyang
Copy link
Contributor

reyang commented Mar 14, 2019

This is to follow up on how to allow APM vendors to integrate with OpenCensus by having more access to span/spancontext.

  1. We will use Google doc for the design discussion.
  2. We will pilot in OpenCensus Python. An experimental development branch will be used for the work in OpenCensus Python.
  3. Once the work got validated, we will lock the design in opencensus-specs.
  4. Once the design got accepted by opencensus-specs, we will merge the Python experimental branch into master, and start the implementation in other languages.
  5. This effort should not affect/block the current OpenCensus Python release schedule.
@reyang
Copy link
Contributor Author

reyang commented Mar 14, 2019

@reyang reyang changed the title Experimental: extend span APIs to support callbacks and access to spancontext Extend span APIs to support callbacks and access to spancontext Mar 14, 2019
@bogdandrutu
Copy link
Contributor

Related to this, we also need to define the hooks and places where Tracestate can be modified. census-instrumentation/opencensus-java#1465?

@SergeyKanzhelev
Copy link
Member

Also it will help with #131. Perhaps that can be closed as dup.

@reyang
Copy link
Contributor Author

reyang commented Mar 15, 2019

Looping @falenn into the discussion.

@bputt
Copy link

bputt commented Mar 15, 2019

@reyang i'd like to be looped in as well

@discostu105
Copy link
Contributor

discostu105 commented Mar 15, 2019

I've started a document, describing use-cases and requirements: https://docs.google.com/document/d/1lu--sppvyFwcWUTnvDcxKFTm1ZtRWe4kwPZU_S86GoE/edit?usp=sharing

Feel free to contribute and describe your use-cases there.

@falenn
Copy link

falenn commented Mar 15, 2019

Before I muddy the doc, I have a dilemma:

Referring to #1465, a use case I would propose is the creation of new tracestate when acting as a client to propagate new vendoring information to downstream services. Here's my confusion: Is it inappropriate to propagate a system-id in a new tracestate? What is actually meant by vendoring and is it expected that the same vendoring information transit all requests / service exchanges for the life of the trace instance?

@falenn
Copy link

falenn commented Mar 18, 2019

Ok, to answer my own question, from https://w3c.github.io/trace-context/#tracestate-field, "The trace context is designed to allow extensibility for all distributed tracing systems. It requires them to respect context set by other systems." Vendoring, therefore is the translation of traceparent into other tracing wire / header protocols which are encapsulate in the tracestate. For example, W3C trace-context -> B3. or W3C-trace-context -> Jaeger.

My conclusion, then is that the tracestate entries will always map to the traceparent, a.k.a the tracestate entries never contain any new information that is not already in the traceparent.

If I wanted to pass system ids, then, this must still be done in metadata outside of W3C trace-context spec. Tracestate is not the place to add this. Also, if this statement is true, there is no need to directly mutate the tracestate at span building / Propagator extraction.

Now I'm confused when trying to understand the utility of tracestate. When acting as a client and propagating a trace from a W3C-based wire protocol system to a B3 wire-protocol system, x-b3-headers have to be passed, not traceparent and tracestate. We can continue to pass traceparent and tracestate as headers, but there's no guarantee that the downstream system will record these as attributes into their analysis system, nor that they will share the trace data.

If the downstream system was to share their trace data, couldn't they also add an exporter to export to our trace correlation system? When exactly is tracestate even used? Is the presence of tracestate compelling the downsteam's analysis system to share trace data with another analysis system?

Ok, sorry - answered one question to then ask another.

Can someone clarify for me the intended use of tracestate? If it should be mutable, I would write a use-case for tracestate decoration assuming its internal contents are useful only for systems that can interpret the wire protocol (perhaps I am thinking too much of Baggage). If tracestate is really for internal state and export, then its presence on the wire is not particularly useful.

@yurishkuro
Copy link

@falenn would you like to open this as a ticket in trace-context repo? If the Spec/documentation leaves you with uncertainty about tracestate, it needs to be improved there.

To answer your question, traceparent is the least common denominator representing trace context that is understood by all tracers. tracestate is a place to store additional information specific to individual tracer. For example, suppose in your own tracer you want to capture data center name where the trace is originated. There's no place to store it in the traceparent, but you're free to add it to your own entry in the tracestate.

@falenn
Copy link

falenn commented Mar 18, 2019

One more comment, sorry. It probably sounds like I derailed this conversation! I have personally constructed a custom sampler that injects probability samplers based on discrete header information, per request. For each span created, a sampler is created after evaluating the request state.

A callback after span construction for sampler injection would be useful. Also then, to follow my above comment, tracestate mutation may be required as my sampling decision is dynamic. I need to modify the TraceOptions, if not just to reflect accurate sampling decision.

@falenn
Copy link

falenn commented Mar 18, 2019

@yurishkuro Thank you with your explanation! I will open a ticket in trace-context.
Also, what do you think about a callback for tracestate mutation? Current Opencensus impl doesn't permit custom tracestate construction (a known limitation).

@binaryseed
Copy link

cc @erabug

@falenn
Copy link

falenn commented Mar 19, 2019

@yurishkuro Thanks again for working with me - just posted w3c/trace-context#263. I was confused as to the nature of tracestate, but now understand it to strictly echo traceparent with vendoring concerns. When I thought of tracestate more as baggage, I felt I needed to mutate it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants