Skip to content

Commit

Permalink
fix test case for duplicated tracestate keys
Browse files Browse the repository at this point in the history
The test_tracestate_duplicated_keys was broken, it demanded to drop
the whole tracestate when a duplicate key was found. This does not match
what the specification mandates in that situation. Instead,
implementations MAY remove the duplicates from tracestate or even leave
them in as-is.

fixes #369
  • Loading branch information
basti1302 committed Dec 14, 2021
1 parent 3e4c96a commit e3f16d7
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 8 deletions.
11 changes: 6 additions & 5 deletions test/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -549,37 +549,38 @@ def test_tracestate_multiple_headers_different_keys(self):
def test_tracestate_duplicated_keys(self):
'''
harness sends a request with an invalid tracestate header with duplicated keys
expects the tracestate to be discarded
expects the tracestate to be inherited, and the duplicated keys to be either kept as-is or one of them
to be discarded
'''
traceparent, tracestate = self.make_single_request_and_get_tracecontext([
['traceparent', '00-12345678901234567890123456789012-1234567890123456-00'],
['tracestate', 'foo=1,foo=1'],
])
self.assertEqual(traceparent.trace_id.hex(), '12345678901234567890123456789012')
self.assertRaises(KeyError, lambda: tracestate['foo'])
self.assertTrue('foo=1' in str(tracestate))

traceparent, tracestate = self.make_single_request_and_get_tracecontext([
['traceparent', '00-12345678901234567890123456789012-1234567890123456-00'],
['tracestate', 'foo=1,foo=2'],
])
self.assertEqual(traceparent.trace_id.hex(), '12345678901234567890123456789012')
self.assertRaises(KeyError, lambda: tracestate['foo'])
self.assertTrue('foo=1' in str(tracestate) or 'foo=2' in str(tracestate))

traceparent, tracestate = self.make_single_request_and_get_tracecontext([
['traceparent', '00-12345678901234567890123456789012-1234567890123456-00'],
['tracestate', 'foo=1'],
['tracestate', 'foo=1'],
])
self.assertEqual(traceparent.trace_id.hex(), '12345678901234567890123456789012')
self.assertRaises(KeyError, lambda: tracestate['foo'])
self.assertTrue('foo=1' in str(tracestate))

traceparent, tracestate = self.make_single_request_and_get_tracecontext([
['traceparent', '00-12345678901234567890123456789012-1234567890123456-00'],
['tracestate', 'foo=1'],
['tracestate', 'foo=2'],
])
self.assertEqual(traceparent.trace_id.hex(), '12345678901234567890123456789012')
self.assertRaises(KeyError, lambda: tracestate['foo'])
self.assertTrue('foo=1' in str(tracestate) or 'foo=2' in str(tracestate))

def test_tracestate_all_allowed_characters(self):
'''
Expand Down
8 changes: 5 additions & 3 deletions test/tracecontext/tracestate.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,11 @@ def from_string(self, string):
if not match:
raise ValueError('illegal key-value format {!r}'.format(member))
key, eq, value = match.groups()
if key in self._traits:
raise ValueError('conflict key {!r}'.format(key))
self._traits[key] = value
if key not in self._traits:
self._traits[key] = value
# If key is already in self._traits, the incoming tracestate header contained a duplicated key.
# According to the spec, two behaviors are valid: Either pass on the duplicated key as-is or drop
# it. We opt for dropping it.
return self

def to_string(self):
Expand Down

0 comments on commit e3f16d7

Please sign in to comment.