Clarifying DistributedContext purpose #240
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The DistributedContext section in the overview did not include examples, and started by describing the data structure rather than the purpose. Describing the purpose and use cases first will help better understand the use cases.
toumorokoshi
requested review from
AloisReitbauer,
bogdandrutu,
c24t,
carlosalberto,
iredelmeier,
reyang,
SergeyKanzhelev,
songy23,
tedsuo and
yurishkuro
as code owners
yurishkuro
approved these changes
Sep 2, 2019
Oberon00
reviewed
Sep 3, 2019
specification/overview.md
Outdated
| **DistributedContext** is an abstract data type that represents collection of entries. | ||
| Each key of **DistributedContext** is associated with exactly one value. **DistributedContext** is serializable, | ||
| to facilitate propagating it not only inside the process but also across process boundaries. | ||
| The **DistributedContext** exists to store labels that describe the context of an operation an application performs. It is intended to enable context that are custom to the application or integrations in contrast to other contexts, such as `SpanContext`. For a specific operation, there should not be multiple **DistributedContext** instances. |
There was a problem hiding this comment.
Suggested change
| The **DistributedContext** exists to store labels that describe the context of an operation an application performs. It is intended to enable context that are custom to the application or integrations in contrast to other contexts, such as `SpanContext`. For a specific operation, there should not be multiple **DistributedContext** instances. | |
| The **DistributedContext** stores labels that describe the context of an operation an application performs. It is intended to enable contexts that are custom to the application or integrations in contrast to other contexts, such as `SpanContext`. For a specific operation, there should not be multiple **DistributedContext** instances. |
There was a problem hiding this comment.
apparently github only lets me add one suggestion per line to a batch. I'll add this in a second commit.
specification/overview.md
Outdated
| **DistributedContext** is an abstract data type that represents collection of entries. | ||
| Each key of **DistributedContext** is associated with exactly one value. **DistributedContext** is serializable, | ||
| to facilitate propagating it not only inside the process but also across process boundaries. | ||
| The **DistributedContext** exists to store labels that describe the context of an operation an application performs. It is intended to enable context that are custom to the application or integrations in contrast to other contexts, such as `SpanContext`. For a specific operation, there should not be multiple **DistributedContext** instances. |
There was a problem hiding this comment.
It is intended to enable contexts that are custom to the application or integrations
Is that so? I thought the Distributed Context, as specified now, was more or less what TagMap was in OpenCensus. But Ted commented in #242 that there will be a new proposal this week.
There was a problem hiding this comment.
I'm struggling for the right way to word this, but I wanted a way to help someone understand that this is a more free-form set of tags / labels that developers can use for their own stack's purposes.
For example, at Zillow we will probably use this to propagate originating service, which there is no spec around.
When I say "integrations" I was thinking about telemetry SaaS providers. I'm sure there's keys that are valuable to propagate for them (maybe also and originating service key).
Of course I may have misunderstood this. And yes maybe Ted will have a completely new proposal that changes the purpose dramatically.
carlosalberto
approved these changes
Sep 3, 2019
Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>
c24t
approved these changes
Sep 13, 2019
|
|
||
| **DistributedContext** is used to annotate telemetry with the name:value pair **Entry**. | ||
| Those values can be used to add dimension to the metric or additional contest properties to logs and traces. | ||
| **DistributedContext** is a collection of key-value `Entry` pairs, with each key of associated with exactly one value. **DistributedContext** is serializable, |
There was a problem hiding this comment.
Suggested change
| **DistributedContext** is a collection of key-value `Entry` pairs, with each key of associated with exactly one value. **DistributedContext** is serializable, | |
| **DistributedContext** is a collection of key-value `Entry` pairs, with each key associated with exactly one value. **DistributedContext** is serializable, |
| **DistributedContext** is an abstract data type that represents collection of entries. | ||
| Each key of **DistributedContext** is associated with exactly one value. **DistributedContext** is serializable, | ||
| to facilitate propagating it not only inside the process but also across process boundaries. | ||
| The **DistributedContext** exists to store labels that describe the context of an operation an application performs. It is intended to enable context that are custom to the application or integrations in contrast to other contexts, such as `SpanContext`. Only one **DistributedContext** should be associated with any particular operation. |
There was a problem hiding this comment.
Suggested change
| The **DistributedContext** exists to store labels that describe the context of an operation an application performs. It is intended to enable context that are custom to the application or integrations in contrast to other contexts, such as `SpanContext`. Only one **DistributedContext** should be associated with any particular operation. | |
| The **DistributedContext** exists to store labels that describe the context of an operation an application performs. It is intended to enable context that is custom to the application or integrations in contrast to other contexts, such as `SpanContext`. Only one **DistributedContext** should be associated with any particular operation. |
Assuming you want "context" to be singular here, I don't know about "integrations" though.
|
There are a couple proposals in flight (open-telemetry/oteps#42, open-telemetry/oteps#36), which will result in "distributed context" term going away, so maybe worth waiting before merging this PR. |
|
@yurishkuro this PR doesn't make anything worse in the current spec. Context separation is planned for v0.3 which is few weeks away so I'd suggest we will merge this PR for now. |
yurishkuro
approved these changes
Oct 3, 2019
SergeyKanzhelev
pushed a commit
to SergeyKanzhelev/opentelemetry-specification
that referenced
this pull request
Feb 18, 2020
* Clarifying DistributedContext The DistributedContext section in the overview did not include examples, and started by describing the data structure rather than the purpose. Describing the purpose and use cases first will help better understand the use cases. * Fix grammar and simplify wording Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>
TuckTuckFloof
pushed a commit
to TuckTuckFloof/opentelemetry-specification
that referenced
this pull request
Oct 15, 2020
carlosalberto
pushed a commit
to carlosalberto/opentelemetry-specification
that referenced
this pull request
Oct 31, 2024
* Clarifying DistributedContext The DistributedContext section in the overview did not include examples, and started by describing the data structure rather than the purpose. Describing the purpose and use cases first will help better understand the use cases. * Fix grammar and simplify wording Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The DistributedContext section in the overview did not include examples,
and started by describing the data structure rather than the purpose.
Describing the purpose and use cases first will help better understand
the use cases.