[WIP] Add TraceState.#42
Conversation
g-easy
requested review from
jmacd,
maxgolov,
reyang,
rnburn and
tigrannajaryan
as code owners
|
It's not clear to me what this should look like in C++. In the Java API, it's abstract: TraceState and SpanContext |
| for (int i = 0; i < n; ++i) | ||
| { | ||
| char c = key[i]; | ||
| if (!IsNumberOrDigit(c) && c != '_' && c != '-' && c != '@' && c != '*' && c != '/') |
There was a problem hiding this comment.
Vendor name has a size limitation of 13 characters according to the spec.
| // TODO: IsValidValue | ||
|
|
||
| private: | ||
| static bool IsNumberOrDigit(char c) { return (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9'); } |
| virtual ~TraceState() = default; | ||
|
|
||
| // Returns false if no such key, otherwise returns true and populates value. | ||
| virtual bool Get(nostd::string_view key, nostd::string_view *value) const noexcept |
There was a problem hiding this comment.
Is there a reason why it has to be a raw pointer and not a reference?
| virtual bool empty() const noexcept { return true; } | ||
|
|
||
| // Returns a span of all the entries. The TraceState object must outlive the span. | ||
| virtual nostd::span<Entry *> entries() const noexcept { return {}; } |
There was a problem hiding this comment.
Does it have to contain raw pointers?
|
I am currently working on context propagation and realized that I need methods from trace states to construct default trace states as well as inserting values for new trace states. Could you implement those as well? |
|
|
||
| // TODO | ||
| // | ||
| // static SpanContext Create(TraceId traceId, SpanId spanId, TraceFlags traceFlags, TraceState |
There was a problem hiding this comment.
Are these definitions going to be up eventually? I need these for my HttpTraceConotext implementation. Thanks
| virtual ~TraceState() = default; | ||
|
|
||
| // Returns false if no such key, otherwise returns true and populates value. | ||
| virtual bool Get(nostd::string_view key, nostd::string_view *value) const noexcept |
There was a problem hiding this comment.
Do we have set functions for Trace State any where? Could you have that functionality implemented? Thank you
|
Superseded by #234. |
merge from upstream
4 participants
No description provided.