Allow digit as first chr in vendor specific trace state key#511
Conversation
Codecov Report
@@ Coverage Diff @@
## master #511 +/- ##
=======================================
Coverage 89.47% 89.47%
=======================================
Files 43 43
Lines 2213 2213
Branches 250 250
=======================================
Hits 1980 1980
Misses 161 161
Partials 72 72
Continue to review full report at Codecov.
|
|
Hi @mariojonke, Shouldn't it be modified as well? Ideally a change like it should include a test to ensure we don't introduce a regression in the future. Maybe you want to create / modify one in https://github.com/open-telemetry/opentelemetry-python/blob/master/opentelemetry-api/tests/trace/propagation/test_tracecontexthttptextformat.py. |
|
i think the Regarding tests, i'll add some. |
|
@mariojonke you're right. Probably what is not aligned is the text there "Identifiers MUST begin with a lowercase letter or a digit". |
| self.assertEqual(span.get_context().trace_state["foo"], "1") | ||
|
|
||
| def test_tracestate_keys(self): | ||
| """Do not propagate invalid trace context. |
There was a problem hiding this comment.
Is this sentence the wrong one for this test?
mauriciovasquezbernal
left a comment
mauriciovasquezbernal
left a comment
There was a problem hiding this comment.
Besides the small nit on the tests docstring, LGTM!
toumorokoshi
left a comment
toumorokoshi
left a comment
There was a problem hiding this comment.
Great, thanks for the catch!
5 participants
Allows vendor specific trace state keys to start with a digit as specified in: https://github.com/w3c/trace-context/blob/master/spec/20-http_header_format.md#key