-
Notifications
You must be signed in to change notification settings - Fork 904
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
Add a section for OTel specific values in TraceState. #1852
Add a section for OTel specific values in TraceState. #1852
Conversation
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
@yurishkuro Please review, as I've updated this PR based on your feedback ;) |
@yurishkuro @Oberon00 Updated the document to use the same notation used by the TraceContext specification. Specific changes:
|
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Hey @yurishkuro Iterated and updated over this PR. Left a small note regarding SDKs being encouraged to discard a value update if it ends up exceeding the 256 char limit.
Agreed. Having such abstraction would be a good thing to have. I'd vow for specifying them in a follow up, once the foundation here is done (more specifically on the A final note is that I feel now that this handling should be a SDK-wide part, more than a Trace one. |
Updated the PR to reflect the latest feedback:
Question: Should we really go back to |
Ping @bogdandrutu - I think we are ready to go, unless you think there's something we need to tune still. |
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Ping @Oberon00 @bogdandrutu - hopefully we can iterate on the remaining bits so this can be merged ;) |
Co-authored-by: Joshua MacDonald <jmacd@users.noreply.github.com>
…#1852) * Add a section for OTel specific values in TraceState. * Update specification/trace/api.md Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> * Update specification/trace/api.md Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> * Fix sample. * Apply feedback. * Use same notation as TraceContext. * Remove unused OWS mention. * Fix link. * Update specification/trace/sdk.md Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> * Clarify values. * Prevent larger than 256 char values. * Fix typo. * Update specification/trace/sdk.md Co-authored-by: Christian Neumüller <christian+github@neumueller.me> * Move trace state handling to its own document. * Update specification/trace/tracestate-handling.md Co-authored-by: John Watson <jkwatson@gmail.com> * Fix lint. * Apply feedback. * Fix typo. * s/SHOULD/MUST * Update specification/trace/tracestate-handling.md Co-authored-by: Joshua MacDonald <jmacd@users.noreply.github.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Co-authored-by: Christian Neumüller <christian+github@neumueller.me> Co-authored-by: John Watson <jkwatson@gmail.com> Co-authored-by: Joshua MacDonald <jmacd@users.noreply.github.com>
Related to open-telemetry/oteps#168 as we want to be able to specify OTel-specific values how in
tracestate
(in the case of the aforementioned OTEP, as we want to store probability and a random number, for sampling purposes).This is a draft for now, in order to get OTEP 168 make progress.
cc @yurishkuro