-
Notifications
You must be signed in to change notification settings - Fork 438
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
TraceState implementation as per spec #551
Changes from 12 commits
925c6a3
7f4ecfa
8f7ba83
0751130
24b95b7
c067b23
ba4ded2
70a2a26
34e9759
e6c1204
2b1be89
0d23d00
1a54cdc
47977cd
f3da416
cc41067
1c27314
228f2a7
9b60b48
ce24183
65b81e5
a03fe1b
7413c85
6acf4d6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -16,6 +16,14 @@ | |||||
|
||||||
#include <cstdint> | ||||||
#include <cstring> | ||||||
#include <string> | ||||||
|
||||||
#include <regex> | ||||||
#if (__GNUC__ == 4 && (__GNUC_MINOR__ == 8)) | ||||||
# define HAVE_WORKING_REGEX 0 | ||||||
#else | ||||||
# define HAVE_WORKING_REGEX 1 | ||||||
#endif | ||||||
|
||||||
#include "opentelemetry/nostd/span.h" | ||||||
#include "opentelemetry/nostd/string_view.h" | ||||||
|
@@ -36,9 +44,11 @@ namespace trace | |||||
class TraceState | ||||||
{ | ||||||
public: | ||||||
static constexpr int kKeyMaxSize = 256; | ||||||
static constexpr int kValueMaxSize = 256; | ||||||
static constexpr int kMaxKeyValuePairs = 32; | ||||||
static constexpr int kKeyMaxSize = 256; | ||||||
static constexpr int kValueMaxSize = 256; | ||||||
static constexpr int kMaxKeyValuePairs = 32; | ||||||
static constexpr auto kKeyValueSeparator = '='; | ||||||
static constexpr auto kMembersSeparator = ','; | ||||||
|
||||||
// Class to store key-value pairs. | ||||||
class Entry | ||||||
|
@@ -97,37 +107,163 @@ class TraceState | |||||
} | ||||||
}; | ||||||
|
||||||
// An empty TraceState. | ||||||
TraceState() noexcept : entries_(new Entry[kMaxKeyValuePairs]), num_entries_(0) {} | ||||||
/** | ||||||
* Returns a newly created Tracestate parsed from the header provided. | ||||||
* @param header Encoding of the tracestate header defined by | ||||||
* the W3C Trace Context specification https://www.w3.org/TR/trace-context/ | ||||||
* @return Tracestate A new Tracestate instance or DEFAULT | ||||||
*/ | ||||||
static TraceState FromHeader(const nostd::string_view &header) | ||||||
lalitb marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
{ | ||||||
TraceState ts; | ||||||
|
||||||
// Returns false if no such key, otherwise returns true and populates the value parameter with the | ||||||
// associated value. | ||||||
bool Get(nostd::string_view key, nostd::string_view &value) const noexcept | ||||||
std::size_t begin{0}; | ||||||
std::size_t end{0}; | ||||||
bool invalid_header = false; | ||||||
while (begin < header.size() && ts.num_entries_ < kMaxKeyValuePairs) | ||||||
{ | ||||||
// find list-member | ||||||
end = header.find(kMembersSeparator, begin); | ||||||
if (end == std::string::npos) | ||||||
{ | ||||||
// last list member. `end` points to end of it. | ||||||
end = header.size() - 1; | ||||||
} | ||||||
else | ||||||
{ | ||||||
// `end` points to end of current list member | ||||||
end = end - 1; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made it
and process next list member. I have added test case to validate this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the first char is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, you are right and bazel asan fails because of that. I am fixing it now :) Thanks for pointing out the issue. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, the easy solution was to add special conditional check for |
||||||
} | ||||||
auto list_member = TrimString(header, begin, end); // OWS handling | ||||||
if (list_member.size() == 0) | ||||||
{ | ||||||
// empty list member, move to next in list | ||||||
begin = end + 2; // begin points to start of next member | ||||||
continue; | ||||||
} | ||||||
auto key_end_pos = list_member.find(kKeyValueSeparator); | ||||||
if (key_end_pos == std::string::npos) | ||||||
{ | ||||||
// Error: invalid list member, return empty TraceState | ||||||
ts.entries_.reset(nullptr); | ||||||
ts.num_entries_ = 0; | ||||||
break; | ||||||
} | ||||||
auto key = list_member.substr(0, key_end_pos); | ||||||
auto value = list_member.substr(key_end_pos + 1); | ||||||
if (!IsValidKey(key) || !IsValidValue(value)) | ||||||
{ | ||||||
// invalid header. return empty TraceState | ||||||
ts.entries_.reset(nullptr); | ||||||
ts.num_entries_ = 0; | ||||||
break; | ||||||
} | ||||||
Entry entry(key, value); | ||||||
(ts.entries_.get())[ts.num_entries_] = entry; | ||||||
ts.num_entries_++; | ||||||
|
||||||
begin = end + 2; | ||||||
} | ||||||
return ts; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Creates a w3c tracestate header from TraceState object | ||||||
*/ | ||||||
std::string ToHeader() | ||||||
{ | ||||||
for (const auto &entry : Entries()) | ||||||
std::string header_s; | ||||||
size_t count = num_entries_; | ||||||
while (count) | ||||||
{ | ||||||
auto entry = (entries_.get())[num_entries_ - count]; | ||||||
auto kv = std::string(entry.GetKey()) + kKeyValueSeparator + std::string(entry.GetValue()); | ||||||
if (--count) | ||||||
{ | ||||||
// append "," if not last member | ||||||
kv += ","; | ||||||
} | ||||||
header_s = header_s + kv; | ||||||
} | ||||||
lalitb marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
return header_s; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Returns `value` associated with `key` passed as argument | ||||||
* Returns empty string if key is invalid or not found | ||||||
*/ | ||||||
std::string Get(nostd::string_view key) const noexcept | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should preserve the ability to distinguish between an key not being specified and a key given but having an empty value. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As per w3c specs ( https://www.w3.org/TR/trace-context/#value) the value can't be empty string ( should have at-least one non-blank character ). This is validated before storing from header. But I don't have strong preference on this, we can switch back to earlier prototype if that's more clear. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have made comments consistent with method prototype. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right. Your method signature is fine. |
||||||
{ | ||||||
if (!IsValidKey(key)) | ||||||
{ | ||||||
return std::string(); | ||||||
} | ||||||
for (size_t i = 0; i < num_entries_; i++) | ||||||
{ | ||||||
auto entry = (entries_.get())[i]; | ||||||
if (key == entry.GetKey()) | ||||||
{ | ||||||
value = entry.GetValue(); | ||||||
return true; | ||||||
return std::string(entry.GetValue()); | ||||||
} | ||||||
} | ||||||
return false; | ||||||
return std::string(); | ||||||
} | ||||||
|
||||||
// Creates an Entry for the key-value pair and adds it to entries. Returns true if pair was added | ||||||
// successfully, false otherwise. If value is null or entries_ is full, this function is a no-op. | ||||||
bool Set(nostd::string_view key, nostd::string_view value) noexcept | ||||||
/** | ||||||
* Returns `new` TransState object with following mutations applied to the existing instance: | ||||||
* Update Key value: The updated value must be moved to beginning of List | ||||||
* Add : The new key-value pair SHOULD be added to beginning of List | ||||||
* | ||||||
* If the provided key-value pair is invalid, or results in transtate that violates the | ||||||
* tracecontext specification, empty tracestate will be returned. | ||||||
*/ | ||||||
TraceState Set(const nostd::string_view &key, const nostd::string_view &value) | ||||||
{ | ||||||
if (value.empty() || num_entries_ >= kMaxKeyValuePairs) | ||||||
TraceState ts; | ||||||
if ((!IsValidKey(key) || !IsValidValue(value)) || num_entries_ == kMaxKeyValuePairs) | ||||||
{ | ||||||
return false; | ||||||
// max size reached or invalid key/value. Returning empty tracestate | ||||||
return ts; // empty instance | ||||||
} | ||||||
|
||||||
Entry entry(key, value); | ||||||
(entries_.get())[num_entries_] = entry; | ||||||
num_entries_++; | ||||||
return true; | ||||||
// add new key-value pair at beginning | ||||||
Entry e(key, value); | ||||||
(ts.entries_.get())[ts.num_entries_++] = e; | ||||||
for (size_t i = 0; i < num_entries_; i++) | ||||||
{ | ||||||
auto entry = (entries_.get())[i]; | ||||||
auto key = entry.GetKey(); | ||||||
auto value = entry.GetValue(); | ||||||
Entry e(key, value); | ||||||
(ts.entries_.get())[ts.num_entries_++] = e; | ||||||
} | ||||||
return ts; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Returns `new` TransState object after removing the attribute with given key ( if present ) | ||||||
* @returns empty TransState object if key is invalid | ||||||
* @returns copy of original TransState object if key is not present (??) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Seems for now an empty instance is returned if the key is not present, should we return original There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for suggestion
|
||||||
*/ | ||||||
TraceState Delete(const nostd::string_view &key) | ||||||
{ | ||||||
TraceState ts; | ||||||
if (!IsValidKey(key)) | ||||||
{ | ||||||
return ts; | ||||||
} | ||||||
for (size_t i = 0; i < num_entries_; i++) | ||||||
{ | ||||||
auto entry = (entries_.get())[i]; | ||||||
if ((entries_.get())[i].GetKey() != key) | ||||||
{ | ||||||
auto key = entry.GetKey(); | ||||||
auto value = entry.GetValue(); | ||||||
Entry e(key, value); | ||||||
(ts.entries_.get())[ts.num_entries_++] = e; | ||||||
} | ||||||
} | ||||||
return ts; | ||||||
} | ||||||
|
||||||
// Returns true if there are no keys, false otherwise. | ||||||
|
@@ -139,8 +275,81 @@ class TraceState | |||||
return nostd::span<Entry>(entries_.get(), num_entries_); | ||||||
} | ||||||
|
||||||
// Returns whether key is a valid key. See https://www.w3.org/TR/trace-context/#key | ||||||
/** Returns whether key is a valid key. See https://www.w3.org/TR/trace-context/#key | ||||||
* Identifiers MUST begin with a lowercase letter or a digit, and can only contain | ||||||
* lowercase letters (a-z), digits (0-9), underscores (_), dashes (-), asterisks (*), | ||||||
* and forward slashes (/). | ||||||
* For multi-tenant vendor scenarios, an at sign (@) can be used to prefix the vendor name. | ||||||
* | ||||||
*/ | ||||||
static bool IsValidKey(nostd::string_view key) | ||||||
{ | ||||||
#if HAVE_WORKING_REGEX | ||||||
return IsValidKeyRegEx(key); | ||||||
#else | ||||||
return IsValidKeyNonRegEx(key); | ||||||
#endif | ||||||
} | ||||||
|
||||||
/** Returns whether value is a valid value. See https://www.w3.org/TR/trace-context/#value | ||||||
* The value is an opaque string containing up to 256 printable ASCII (RFC0020) | ||||||
* characters ((i.e., the range 0x20 to 0x7E) except comma , and equal =) | ||||||
*/ | ||||||
static bool IsValidValue(nostd::string_view value) | ||||||
{ | ||||||
#if HAVE_WORKING_REGEX | ||||||
return IsValidValueRegEx(value); | ||||||
#else | ||||||
return IsValidValueNonRegEx(value); | ||||||
#endif | ||||||
} | ||||||
|
||||||
private: | ||||||
// Store entries in a C-style array to avoid using std::array or std::vector. | ||||||
nostd::unique_ptr<Entry[]> entries_; | ||||||
|
||||||
// Maintain the number of entries in entries_. Must be in the range [0, kMaxKeyValuePairs]. | ||||||
size_t num_entries_; | ||||||
|
||||||
// An empty TraceState. | ||||||
TraceState() noexcept : entries_(new Entry[kMaxKeyValuePairs]), num_entries_(0) {} | ||||||
|
||||||
static nostd::string_view TrimString(nostd::string_view str, size_t left, size_t right) | ||||||
{ | ||||||
while (str[(std::size_t)right] == ' ' && left < right) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: C++ style There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. fixed it now. |
||||||
{ | ||||||
right--; | ||||||
} | ||||||
while (str[(std::size_t)left] == ' ' && left < right) | ||||||
{ | ||||||
left++; | ||||||
} | ||||||
return str.substr(left, right - left + 1); | ||||||
} | ||||||
|
||||||
static bool IsValidKeyRegEx(nostd::string_view key) | ||||||
{ | ||||||
static std::regex reg_key("^[a-z0-9][a-z0-9*_\\-/]{0,255}$"); | ||||||
static std::regex reg_key_multitenant( | ||||||
"^[a-z0-9][a-z0-9*_\\-/]{0,240}(@)[a-z0-9][a-z0-9*_\\-/]{0,13}$"); | ||||||
std::string key_s(key); | ||||||
if (std::regex_match(key_s, reg_key) || std::regex_match(key_s, reg_key_multitenant)) | ||||||
{ | ||||||
return true; | ||||||
} | ||||||
return false; | ||||||
} | ||||||
|
||||||
static bool IsValidValueRegEx(nostd::string_view value) | ||||||
{ | ||||||
// Hex 0x20 to 0x2B, 0x2D to 0x3C, 0x3E to 0x7E | ||||||
std::regex reg_value( | ||||||
lalitb marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
"^[\\x20-\\x2B\\x2D-\\x3C\\x3E-\\x7E]{0,255}[\\x21-\\x2B\\x2D-\\x3C\\x3E-\\x7E]$"); | ||||||
// Need to benchmark without regex, as a string object is created here. | ||||||
return std::regex_match(std::string(value), reg_value); | ||||||
} | ||||||
|
||||||
static bool IsValidKeyNonRegEx(nostd::string_view key) | ||||||
{ | ||||||
if (key.empty() || key.size() > kKeyMaxSize || !IsLowerCaseAlphaOrDigit(key[0])) | ||||||
{ | ||||||
|
@@ -163,8 +372,7 @@ class TraceState | |||||
return true; | ||||||
} | ||||||
|
||||||
// Returns whether value is a valid value. See https://www.w3.org/TR/trace-context/#value | ||||||
static bool IsValidValue(nostd::string_view value) | ||||||
static bool IsValidValueNonRegEx(nostd::string_view value) | ||||||
{ | ||||||
if (value.empty() || value.size() > kValueMaxSize) | ||||||
{ | ||||||
|
@@ -181,15 +389,7 @@ class TraceState | |||||
return true; | ||||||
} | ||||||
|
||||||
private: | ||||||
// Store entries in a C-style array to avoid using std::array or std::vector. | ||||||
nostd::unique_ptr<Entry[]> entries_; | ||||||
|
||||||
// Maintain the number of entries in entries_. Must be in the range [0, kMaxKeyValuePairs]. | ||||||
int num_entries_; | ||||||
|
||||||
static bool IsLowerCaseAlphaOrDigit(char c) { return isdigit(c) || islower(c); } | ||||||
}; | ||||||
|
||||||
} // namespace trace | ||||||
OPENTELEMETRY_END_NAMESPACE | ||||||
OPENTELEMETRY_END_NAMESPACE |
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.
nit: change spelling of
Tracestate
in this comment toTraceState
?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.
Thanks. changed it now :)