-
Notifications
You must be signed in to change notification settings - Fork 888
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
API: Should propagating formats be part of Tracer or not? #112
Comments
I feel that context propagation should be separated from Tracers. Here's why: In our Otel architectural design, we would like to have a clean separation between Observability (Tracing, metrics, etc) and context propagation (both in-process and distributed propagation). Store any data in the Propagation layer, write telemetry data with the Observability layer. This is a good design, and it helps our users understand the purpose of the different layers. We are mostly there, in terms of achieving these separation of concerns. However, one of the final places where the higher-level observability layer "reached down" into the lower-level context layer is at the injection/extraction points. This confuses the relationship between the layers. If we draw a high level diagram like this one and tell users that is our architecture, then the Tracer doing the context propagation violates that diagram. Clearly, the propagation layer should do the propagation! While I believe it will be functional to leave everything in its current place, I am concerned that this will make comprehension difficult for new users. Injection/extraction of context objects is one of the most difficult concepts for new users to grasp; I believe that it is worth it to spend some cycles in July and August to try and cleanly separate the Observability and Context Propagation layers. |
Just throwing in a vote in favor of this. Really I don't see why propagators can't have it's own global registry just like tracers. I also agree that it feels a lot more cross cutting, as it should as propagation can occur through multiple forms of telemetry. |
/cc @rghetia also proposed to have the propagating formats in a separator interface |
Typically plugins like Http would provide an option to specify which propagator to use. This would be independent of which tracing sdk is used. Different server or different endpoint on same server could use different propagator. So tying propagation interface to a tracer() is not useful. |
I think it sounds like generally there's a lot of support here. What's required to move this forward? Or should we go ahead and implement it this way in our clients? |
|
Fixes open-telemetry#112. Those formats are moved into [Propagators](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/api-propagators.md).
My 2 cents:
Regarding |
Maybe you define integrations differently here, but I think propagators can be used directly by applications too, e.g. when they have a proprietary communication mechanism that is used so seldom that writing an integration would make no sense (e.g., starting a child process and passing the trace context as a command line parameter). |
Bumping this: With named tracers, Propagators work with integrations or user apps. User apps don't need any abstraction at all: how they extract/inject context from the wire - it's up to them. Having common implementations of propagators is useful. Integrations in some cases implement protocols that don't have standard way for tracing context propagation and they also don't need an abstraction too. From my experience working with messaging services in Azure and instrumenting their SDKs - it's easier to implement my own freestyle extract and inject when I control protocol then to implement Binary/TextFormat. But in other cases integration can benefit from common propagator implementation. The reason in OTel we want it to be an abstraction is that we still want to support multiple HTTP protocols and we want to be able to reuse them in integrations. TL;DR: |
Closing as OTEP 66, which covered this, was merged. |
* RUM libraries specification * Code review comments * code review comments * Code review comments * Code review comments * web semantic attributes * ex. -> e.g. * code review * code review comment * s/serviceName/applicationName/g
Original discussion: #105 (comment).
The text was updated successfully, but these errors were encountered: