-
Notifications
You must be signed in to change notification settings - Fork 164
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
Proposal: Separate Layer for Context Propagation #66
Conversation
Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>
Co-Authored-By: Reiley Yang <reyang@microsoft.com>
Co-Authored-By: Reiley Yang <reyang@microsoft.com>
Co-Authored-By: Reiley Yang <reyang@microsoft.com>
Co-Authored-By: Reiley Yang <reyang@microsoft.com>
* Remove registry concept * Add explicit chaining
This OTEP splits the packages a bit further than we already have. But honestly, it is not a huge change. The package layout would look like the following, with the lower-level Context and Propagation packages separated from observability:
Here are what I believe to be the important changes:
I believe that many of these API details about types and objects would be language-specific. But, in general, each observability API should wrap the context object, implement its own set of propagators, and be independent from the other APIs . All of the observability APIs make use of the same local context object to store their data, but that could be an internal detail most of the time, in languages which allow for implicit context propagation. For Inject/Extract, all of the separate propagators are injected and extracted at the same call site. the Propagation package can have a registry, or another mechanism which iterates over all of the registered propagators. Since each concern knows only about its own details - the Tracing SDK only knows how to handle span data locally in the context, and what headers should be used for injecting and extracting of the span context, and how to respond to sampling. Likewise, the Correlations SDK only knows the details about how to manage the correlations data locally, and how to inject and extract a filtered subset of correlations. So, while not a huge change, this does further separate concerns. There are API details to discuss, but I would like to avoid getting into them in this OTEP. They are addressed in the prototypes, and can be clarified in the Spec. The purpose of the OTEP is just to clarify the new package layout and their relationships. The sample APIs and examples were added by request from other community members, so that they could better understad how the packages relate to each other, and help get the prototypes written.
That is not mentioned; though I agree we need a way to filter for Correlations. |
I think the overall concept of this proposal is great. Providing a universal, reliable mechanism for transmitting context throughout a system could make OpenTelemetry attractive to development teams currently not using an observability framework. |
Co-Authored-By: Mauricio Vásquez <mauricio@kinvolk.io>
@bogdandrutu @SergeyKanzhelev since this has approval, and is primarily meant to describe the high-level goals, I would like to go ahead and merge it. We can continue discussion about the API details in spec PRs, based on the prototypes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM to move this forward to specs.
* Proposal to separate context propagation from observability * cleanup description for Extract * commas Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * Update text/0000-separate-context-propagation.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * RFC proposal: A layered approach to data formats * whitespace Co-Authored-By: Reiley Yang <reyang@microsoft.com> * Capitalization Co-Authored-By: Reiley Yang <reyang@microsoft.com> * whitespace Co-Authored-By: Reiley Yang <reyang@microsoft.com> * CleanBaggage -> ClearBaggage * move function descriptions to new line * Add Optional subheader * cleanup rough edits * clean up advice on pre-existing context implementations * Better context descriptions * remove data format file * remove git diff message * improved code sytnax * stop stuttering * Update text/0000-separate-context-propagation.md Co-Authored-By: Reiley Yang <reyang@microsoft.com> * spacing * Refine propagation * Remove registry concept * Add explicit chaining * Add RFC ID number from PR * remove RFC status line * slight calrification for GetHTTPExtractor * add global propagators * Clean up motivation * Clean up explanbation intro * Clarify context types * Fix ChainHTTPInjector and ChainHTTPExtractor * typo * Reference Trace-Context, not just traceparent * Bagge context cleanup * stronger language around context access * Update text/0042-separate-context-propagation.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * clean up tradeoffs * v2.0 of this OTEP * Update OTEP number for new submission * remove image file for unused diagram * Update text/0066-separate-context-propagation.md Co-Authored-By: alrex <alrex.boten@gmail.com> * Link to Erlang prototype * whitespace * ToC * Update text/0066-separate-context-propagation.md Co-Authored-By: Tristan Sloughter <t@crashfast.com> * more context examples * typo * Update text/0066-separate-context-propagation.md Co-Authored-By: Yusuke Tsutsumi <tsutsumi.yusuke@gmail.com> * Apply suggestions from code review Co-Authored-By: Yuri Shkuro <yurishkuro@users.noreply.github.com> * Renamed aspects to "cross-cutting concerns" * injectors are a list instead of chained * clean up API representation * cleanup examples * typo * remove correlations from proposal * Update text/0066-separate-context-propagation.md Co-Authored-By: Yuri Shkuro <yurishkuro@users.noreply.github.com> * remove the term "chaining" * remove the terms upstream and downstream * improved architecturural explanation * whitespace * update link to Go prototype * Removed the Baggage API; replaced it with Correlations. * Fix the intro paragraph for Correlations * git merges ate my homework * some sentences were out of order * Clarify that correltions must be propagated * Clarify risks * removed extra header * Clarify definition of aspect-oriented programming * Fix RemoveCorrelation * Spelling * Update text/0066-separate-context-propagation.md Co-Authored-By: Sergey Kanzhelev <S.Kanzhelev@live.com> * Clarifying details * Update python prototype * Update text/0066-separate-context-propagation.md Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com> * Update text/0066-separate-context-propagation.md Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com> * Update text/0066-separate-context-propagation.md Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com> * Clarify that the APIs and example code are not meant as final. Add C# prototype * Inject returns headers, not context * Update text/0066-separate-context-propagation.md Co-Authored-By: Mauricio Vásquez <mauricio@kinvolk.io> * spelling * remove spurious go comment Co-authored-by: Christian Neumüller <christian+github@neumueller.me> Co-authored-by: Reiley Yang <reyang@microsoft.com> Co-authored-by: alrex <alrex.boten@gmail.com> Co-authored-by: Tristan Sloughter <t@crashfast.com> Co-authored-by: Yusuke Tsutsumi <tsutsumi.yusuke@gmail.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Co-authored-by: Sergey Kanzhelev <S.Kanzhelev@live.com> Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com> Co-authored-by: Mauricio Vásquez <mauricio@kinvolk.io>
* Proposal to separate context propagation from observability * cleanup description for Extract * commas Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * Update text/0000-separate-context-propagation.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * RFC proposal: A layered approach to data formats * whitespace Co-Authored-By: Reiley Yang <reyang@microsoft.com> * Capitalization Co-Authored-By: Reiley Yang <reyang@microsoft.com> * whitespace Co-Authored-By: Reiley Yang <reyang@microsoft.com> * CleanBaggage -> ClearBaggage * move function descriptions to new line * Add Optional subheader * cleanup rough edits * clean up advice on pre-existing context implementations * Better context descriptions * remove data format file * remove git diff message * improved code sytnax * stop stuttering * Update text/0000-separate-context-propagation.md Co-Authored-By: Reiley Yang <reyang@microsoft.com> * spacing * Refine propagation * Remove registry concept * Add explicit chaining * Add RFC ID number from PR * remove RFC status line * slight calrification for GetHTTPExtractor * add global propagators * Clean up motivation * Clean up explanbation intro * Clarify context types * Fix ChainHTTPInjector and ChainHTTPExtractor * typo * Reference Trace-Context, not just traceparent * Bagge context cleanup * stronger language around context access * Update text/0042-separate-context-propagation.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * clean up tradeoffs * v2.0 of this OTEP * Update OTEP number for new submission * remove image file for unused diagram * Update text/0066-separate-context-propagation.md Co-Authored-By: alrex <alrex.boten@gmail.com> * Link to Erlang prototype * whitespace * ToC * Update text/0066-separate-context-propagation.md Co-Authored-By: Tristan Sloughter <t@crashfast.com> * more context examples * typo * Update text/0066-separate-context-propagation.md Co-Authored-By: Yusuke Tsutsumi <tsutsumi.yusuke@gmail.com> * Apply suggestions from code review Co-Authored-By: Yuri Shkuro <yurishkuro@users.noreply.github.com> * Renamed aspects to "cross-cutting concerns" * injectors are a list instead of chained * clean up API representation * cleanup examples * typo * remove correlations from proposal * Update text/0066-separate-context-propagation.md Co-Authored-By: Yuri Shkuro <yurishkuro@users.noreply.github.com> * remove the term "chaining" * remove the terms upstream and downstream * improved architecturural explanation * whitespace * update link to Go prototype * Removed the Baggage API; replaced it with Correlations. * Fix the intro paragraph for Correlations * git merges ate my homework * some sentences were out of order * Clarify that correltions must be propagated * Clarify risks * removed extra header * Clarify definition of aspect-oriented programming * Fix RemoveCorrelation * Spelling * Update text/0066-separate-context-propagation.md Co-Authored-By: Sergey Kanzhelev <S.Kanzhelev@live.com> * Clarifying details * Update python prototype * Update text/0066-separate-context-propagation.md Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com> * Update text/0066-separate-context-propagation.md Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com> * Update text/0066-separate-context-propagation.md Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com> * Clarify that the APIs and example code are not meant as final. Add C# prototype * Inject returns headers, not context * Update text/0066-separate-context-propagation.md Co-Authored-By: Mauricio Vásquez <mauricio@kinvolk.io> * spelling * remove spurious go comment Co-authored-by: Christian Neumüller <christian+github@neumueller.me> Co-authored-by: Reiley Yang <reyang@microsoft.com> Co-authored-by: alrex <alrex.boten@gmail.com> Co-authored-by: Tristan Sloughter <t@crashfast.com> Co-authored-by: Yusuke Tsutsumi <tsutsumi.yusuke@gmail.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Co-authored-by: Sergey Kanzhelev <S.Kanzhelev@live.com> Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com> Co-authored-by: Mauricio Vásquez <mauricio@kinvolk.io>
* Proposal to separate context propagation from observability * cleanup description for Extract * commas Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * Update text/0000-separate-context-propagation.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * RFC proposal: A layered approach to data formats * whitespace Co-Authored-By: Reiley Yang <reyang@microsoft.com> * Capitalization Co-Authored-By: Reiley Yang <reyang@microsoft.com> * whitespace Co-Authored-By: Reiley Yang <reyang@microsoft.com> * CleanBaggage -> ClearBaggage * move function descriptions to new line * Add Optional subheader * cleanup rough edits * clean up advice on pre-existing context implementations * Better context descriptions * remove data format file * remove git diff message * improved code sytnax * stop stuttering * Update text/0000-separate-context-propagation.md Co-Authored-By: Reiley Yang <reyang@microsoft.com> * spacing * Refine propagation * Remove registry concept * Add explicit chaining * Add RFC ID number from PR * remove RFC status line * slight calrification for GetHTTPExtractor * add global propagators * Clean up motivation * Clean up explanbation intro * Clarify context types * Fix ChainHTTPInjector and ChainHTTPExtractor * typo * Reference Trace-Context, not just traceparent * Bagge context cleanup * stronger language around context access * Update text/0042-separate-context-propagation.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * clean up tradeoffs * v2.0 of this OTEP * Update OTEP number for new submission * remove image file for unused diagram * Update text/0066-separate-context-propagation.md Co-Authored-By: alrex <alrex.boten@gmail.com> * Link to Erlang prototype * whitespace * ToC * Update text/0066-separate-context-propagation.md Co-Authored-By: Tristan Sloughter <t@crashfast.com> * more context examples * typo * Update text/0066-separate-context-propagation.md Co-Authored-By: Yusuke Tsutsumi <tsutsumi.yusuke@gmail.com> * Apply suggestions from code review Co-Authored-By: Yuri Shkuro <yurishkuro@users.noreply.github.com> * Renamed aspects to "cross-cutting concerns" * injectors are a list instead of chained * clean up API representation * cleanup examples * typo * remove correlations from proposal * Update text/0066-separate-context-propagation.md Co-Authored-By: Yuri Shkuro <yurishkuro@users.noreply.github.com> * remove the term "chaining" * remove the terms upstream and downstream * improved architecturural explanation * whitespace * update link to Go prototype * Removed the Baggage API; replaced it with Correlations. * Fix the intro paragraph for Correlations * git merges ate my homework * some sentences were out of order * Clarify that correltions must be propagated * Clarify risks * removed extra header * Clarify definition of aspect-oriented programming * Fix RemoveCorrelation * Spelling * Update text/0066-separate-context-propagation.md Co-Authored-By: Sergey Kanzhelev <S.Kanzhelev@live.com> * Clarifying details * Update python prototype * Update text/0066-separate-context-propagation.md Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com> * Update text/0066-separate-context-propagation.md Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com> * Update text/0066-separate-context-propagation.md Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com> * Clarify that the APIs and example code are not meant as final. Add C# prototype * Inject returns headers, not context * Update text/0066-separate-context-propagation.md Co-Authored-By: Mauricio Vásquez <mauricio@kinvolk.io> * spelling * remove spurious go comment Co-authored-by: Christian Neumüller <christian+github@neumueller.me> Co-authored-by: Reiley Yang <reyang@microsoft.com> Co-authored-by: alrex <alrex.boten@gmail.com> Co-authored-by: Tristan Sloughter <t@crashfast.com> Co-authored-by: Yusuke Tsutsumi <tsutsumi.yusuke@gmail.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Co-authored-by: Sergey Kanzhelev <S.Kanzhelev@live.com> Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com> Co-authored-by: Mauricio Vásquez <mauricio@kinvolk.io>
This proposal provides a high level description of OpenTelemetry, which shows how context propagation, observability, and baggage could be cleanly decoupled.
This is a second draft of the OTEP originally submitted in #42. It is resubmitted as a new PR, as much has changed. Link to a readable version of the latest draft can be found here.
Changes from the first draft:
A couple of notes on terminology:
In this doc, I refer to
TagMap
asCorrelations
, andTags
asLabels
. There can be a separate discussion for what to name this, but since Tag has repeatedly come up as a contentious name for this feature, I tried to use something "similar but different" to avoid this debate. I actually feel that the term correlation contains a more accurate meaning than tag, so it may be an improvement.The goal with this spec proposal is to find a way to describe these public APIs as simple as possible, but no simpler. This description is meant to be high level, but not inaccurate. If a detail appears to be missing, it is intentional - presumably handled at the SDK layer. Possibly, it should be present. Please review this document with this in mind.