This repository was archived by the owner on Oct 3, 2023. It is now read-only.
Add tracestate field to the Span proto.#74
Merged
bogdandrutu merged 1 commit intocensus-instrumentation:masterfrom Aug 21, 2018
Merged
Add tracestate field to the Span proto.#74bogdandrutu merged 1 commit intocensus-instrumentation:masterfrom
bogdandrutu merged 1 commit intocensus-instrumentation:masterfrom
Conversation
songy23
approved these changes
Aug 20, 2018
| // returns, etc. | ||
| // | ||
| // See the https://github.com/w3c/distributed-tracing for more details about this field. | ||
| map<string, string> tracestate = 15; |
Contributor
There was a problem hiding this comment.
nit: consider ordering the fields by its field number. Same for kind.
Member
There was a problem hiding this comment.
Is map ordered in proto? Not that important for storing as for propagation, but nevertheless for consistency it may be better to have a list
semistrict
approved these changes
Aug 21, 2018
Contributor
Author
|
@lmolkova I would like you or @SergeyKanzhelev to look into this and approve it. |
lmolkova
approved these changes
Aug 21, 2018
Contributor
|
all good, just a question. are we expecting this to be used only in process
(conversion) or basically anywhere (collector tier)? I'm guessing the
latter and the size constraints would limit the impact of this, but
definitely double size of small spans. one thing possibly already addressed
is to mention only populate this on spans with remote parent (server or
message consumer)
|
Contributor
|
another option is to switch it to a truncatable string, which retains
the order and also an help if you are trying to understand malformed
content (not entirely sure the use case)
…On Thu, Aug 23, 2018 at 7:47 AM Sergey Kanzhelev ***@***.***> wrote:
@SergeyKanzhelev commented on this pull request.
________________________________
In opencensus/proto/trace/trace.proto:
> + // The `tracestate` field conveys information about request position in multiple distributed
+ // tracing graphs.
+ //
+ // There can be a maximum of 32 members in the map.
+ //
+ // The key must begin with a lowercase letter, and can only contain lowercase letters 'a'-'z',
+ // digits '0'-'9', underscores '_', dashes '-', asterisks '*', and forward slashes '/'. For
+ // multi-tenant vendors scenarios '@' sign can be used to prefix vendor name. The maximum length
+ // for the key is 256 characters.
+ //
+ // The value is opaque string up to 256 characters printable ASCII RFC0020 characters (i.e., the
+ // range 0x20 to 0x7E) except ',' and '='. Note that this also excludes tabs, newlines, carriage
+ // returns, etc.
+ //
+ // See the https://github.com/w3c/distributed-tracing for more details about this field.
+ map<string, string> tracestate = 15;
Is map ordered in proto? Not that important for storing as for propagation, but nevertheless for consistency it may be better to have a list
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
No description provided.