Skip to content
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

Update regular expression for traceStateKeyFormatWithMultiTenantVendor #1475

Closed
YuriiBarninets opened this issue Jan 19, 2021 · 2 comments · Fixed by #1581
Closed

Update regular expression for traceStateKeyFormatWithMultiTenantVendor #1475

YuriiBarninets opened this issue Jan 19, 2021 · 2 comments · Fixed by #1581
Assignees
Labels
bug Something isn't working

Comments

@YuriiBarninets
Copy link

YuriiBarninets commented Jan 19, 2021

The regular expression for tracestate key format with multi-tenant vendor does not match W3C TraceContext specification.

TraceContext specification:
https://www.w3.org/TR/trace-context-1/#key

TraceContext specification allows digits to be placed at the beginning of tracestate key for multi-tenant vendor scenarios, however the current regular expression in Go OpenTelemetry prohibits it:

traceStateKeyFormatWithMultiTenantVendor = [a-z][_0-9a-z\-\*\/]{0,240}@[a-z][_0-9a-z\-\*\/]{0,13}

As a result, the following key will be reinterpreted as the wrong one:

968a934b-980df25b@dt

@MrAlias
Copy link
Contributor

MrAlias commented Jan 21, 2021

There is some ambiguity with the start character of the key as described in this issue. I think we should update to match what is there as this issues describes, but it should be understood that things might change in the future.

@MrAlias MrAlias added the bug Something isn't working label Jan 21, 2021
@MrAlias
Copy link
Contributor

MrAlias commented Jan 21, 2021

Looking into addressing this and adding tests, we should at a minimum test for the same data set that the W3C project does: https://github.com/w3c/trace-context/blob/master/test/test_data.json

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants