census-instrumentation / opencensus-specs

Apache License 2.0
188 stars 50 forks source link

Propose to make clear distinction between support for a backend vs instrumentation #150

Open basvanbeek opened 6 years ago

basvanbeek commented 6 years ago

There have been a couple of PR's in the various OC instrumentation libraries to add support for 3rd party tracing propagation mechanisms to become part of core OC libraries.

See: https://github.com/census-instrumentation/opencensus-go/pull/841 https://github.com/census-instrumentation/opencensus-node/pull/74 https://github.com/census-instrumentation/opencensus-python/pull/222

I would like to suggest making a clear distinction between tracing backends and tracing instrumentation and what we mean with "officially supported" by OpenCensus.

The beauty and strength of OpenCensus is that it tries to provide a cross language and as much as possible backend agnostic (at least for Dapper like tracing systems) and consistent span model.

Best user experience comes from deployments having as much native OpenCensus instrumentation as possible, using the OpenCensus official propagation formats.

OpenCensus officially supports the following tracing backends: Zipkin, Jaeger and Stackdriver.

Let's not extend this first class support to the propagation methods typically used by instrumentation for these backends. Currently OpenCensus implements a subset of B3 for HTTP and grpc-trace-bin for gRPC, which provides a single mechanism which is chosen to be the easiest to interop with the largest amount of 3rd party tracing instrumentation. B3 is already supported by Zipkin and Jaeger so consumers already have a brown field interop solution right there!

If we'd add other propagation methods as first class citizens we weaken the story of OpenCensus' consistent instrumentation model. Especially given the current efforts into coming up with an official W3C context propagation standard it does not make sense to add additional legacy propagation methods especially for 3rd party instrumentation that hasn't been around long enough to be widespread or that already supports our current mechanisms.

If we make this distinction between backend and instrumentation clear and also explain exactly what is in the realm of official repo supported we have a chance to move forward much faster and not help with proliferation of legacy mechanisms and have consumers remain with or maybe even unintentionally add technical debt!

tcolgate commented 6 years ago

It should be stated though that whilst jaeger has support for B3, this is not the default setup and requires code changes. Most tools supporting jaeger will not currently support configuring b3 (though adding that support is not particularly hard, it may be confusing for end users).

basvanbeek commented 6 years ago

For that small group of people that want to remain in a mixed instrumentation deployment brownfield we could simply have these propagation mechanisms in contrib repo's; that's what they are for in my opinion; to support not standard deployments. Especially since we're all striving to support and drive towards the W3C proposal. I'd rather see a strongly consistent instrumentation library with very clear objectives and strong signal to consumers than adding this soon to be legacy thing and keep proliferating tech debt.

tcolgate commented 6 years ago

The relevant section of the spec: https://github.com/census-instrumentation/opencensus-specs/blob/33b9a1af01c0cfb36036471fa9c464c7e7c6494d/trace/HTTP.md

basvanbeek commented 6 years ago

What are you trying to say with that? The spec is clear on supporting the usage of various propagation methods, just like it says it supports the usage of various backends; doesn't mean we should add all of them as first class citizens inside the official repos.

My point is exactly about that; we need to be clearer about the difference between backend and instrumentation. Where my personal opinion is that while we officially support multiple backends inside the main repo's but we should refrain of supporting propagation methods for mixed mode instrumentation deployments as first class citizens inside the main repo's. Those belong in contrib for the small subset of OpenCensus users that mix instrumentations.

tcolgate commented 6 years ago

@basvanbeek I posted that because if this is to be clarified, that might be a useful place to document such a clarification.

I do not think it is realistic (or, to some degree even honest), to state compatibility with a tracer without actually support the native propagation format (it is a sad fact that, until the w3c spec comes to fruition and is supported, each tracer has owned this aspect of things).

Also, it's possibly worth considering that there are very few of these trace propagation formats in the wild, not are they particularly complicated. If you essentially mandate b3 and stackdriver to meet the spec, jaeger is possibly the only other OSS tracer with any traction. It seems odd to connsider the server worthy of first class status, but not the propagation format.

I'm also not convinced that mixed environments are, or will be, the unusual case.

basvanbeek commented 6 years ago

@tcolgate I guess we agree to disagree

Time for others to chime in.

codefromthecrypt commented 6 years ago

It is kindof odd that microsoft aren't upset, neither amazon who literally can't support B3. It is also odd that other sites like Instana also haven't complained. We have certainly had one very strong complaint by an individual in favor of uplifting a single-product header into the default set of propagation formats. I think days were spent on this topic with the same reasoning on a different issue. I would also like to hear from someone else.

semistrict commented 6 years ago

OpenCensus officially supports the following tracing backends: Zipkin, Jaeger and Stackdriver.

This is an incorrect statement. Zipkin and Jaeger exporters are in-tree to make it easier to develop and evolve the exporter interfaces and to provide a reference implementation of the exporters. The copy of the Stackdriver exporter there is scheduled for removal in the next deprecation cycle (this is the case for Go at least).

I don't see any harm of adding the Jaeger propagation format in the Jaeger exporter. I get that propagation and exporter are different things, but jaeger exporter package still seems like the place users will go looking for it when they want "Jaeger support".

When TraceContext is accepted, we can come up with a good transition plan from all of the existing propagation formats. For example, we could make all the existing formats also accept TraceContext by default and then deprecate the other propagation formats gradually. This already needs to be done for B3, Stackdriver, X-Ray and (IMO) even gRPC so I don't think there's much more of a cost to do it for Jaeger.