From b01a50c59e62ca1d46c08c5e1815d6d9cbafa4fe Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Thu, 26 Sep 2019 13:46:53 -0700 Subject: [PATCH 1/6] Implementing TraceContext (fixes #116) This introduces a w3c TraceContext propagator, primarily inspired by opencensus. --- .../propagation/tracecontexthttptextformat.py | 122 +++++++++- .../src/opentelemetry/trace/__init__.py | 33 ++- opentelemetry-api/tests/context/__init__.py | 0 .../tests/context/propagation/__init__.py | 0 .../test_tracecontexthttptextformat.py | 208 ++++++++++++++++++ 5 files changed, 356 insertions(+), 7 deletions(-) create mode 100644 opentelemetry-api/tests/context/__init__.py create mode 100644 opentelemetry-api/tests/context/propagation/__init__.py create mode 100644 opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py diff --git a/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py b/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py index 575644a91f2..84fab18974e 100644 --- a/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py +++ b/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. # - +import re import typing import opentelemetry.trace as trace @@ -22,18 +22,128 @@ class TraceContextHTTPTextFormat(httptextformat.HTTPTextFormat): - """TODO: extracts and injects using w3c TraceContext's headers. + """Extracts and injects using w3c TraceContext's headers. """ + _TRACEPARENT_HEADER_NAME = "traceparent" + _TRACESTATE_HEADER_NAME = "tracestate" + _TRACEPARENT_HEADER_FORMAT = ( + "^[ \t]*([0-9a-f]{2})-([0-9a-f]{32})-([0-9a-f]{16})-([0-9a-f]{2})" + + "(-.*)?[ \t]*$" + ) + _TRACEPARENT_HEADER_FORMAT_RE = re.compile(_TRACEPARENT_HEADER_FORMAT) + + @classmethod def extract( - self, _get_from_carrier: httptextformat.Getter[_T], _carrier: _T + cls, get_from_carrier: httptextformat.Getter[_T], carrier: _T ) -> trace.SpanContext: - return trace.INVALID_SPAN_CONTEXT + """Extracts a valid SpanContext from the carrier. + + If a header + """ + header = get_from_carrier(carrier, cls._TRACEPARENT_HEADER_NAME) + + if not header: + return trace.INVALID_SPAN_CONTEXT + + match = re.search(cls._TRACEPARENT_HEADER_FORMAT_RE, header[0]) + if not match: + return trace.INVALID_SPAN_CONTEXT + + version = match.group(1) + trace_id = match.group(2) + span_id = match.group(3) + trace_options = match.group(4) + if trace_id == "0" * 32 or span_id == "0" * 16: + return trace.INVALID_SPAN_CONTEXT + + if version == "00": + if match.group(5): + return trace.INVALID_SPAN_CONTEXT + if version == "ff": + return trace.INVALID_SPAN_CONTEXT + + tracestate = trace.TraceState() + for tracestate_header in get_from_carrier( + carrier, cls._TRACESTATE_HEADER_NAME + ): + tracestate.update(_parse_tracestate(tracestate_header)) + + span_context = trace.SpanContext( + trace_id=int(trace_id, 16), + span_id=int(span_id, 16), + trace_options=trace.TraceOptions(trace_options), + trace_state=tracestate, + ) + + return span_context + + @classmethod def inject( - self, + cls, context: trace.SpanContext, set_in_carrier: httptextformat.Setter[_T], carrier: _T, ) -> None: - pass + if context == trace.INVALID_SPAN_CONTEXT: + return + traceparent_string = "-".join( + [ + "00", + format(context.trace_id, "032x"), + format(context.span_id, "016x"), + format(context.trace_options, "02x"), + ] + ) + set_in_carrier( + carrier, cls._TRACEPARENT_HEADER_NAME, traceparent_string + ) + if context.trace_state: + tracestate_string = _format_tracestate(context.trace_state) + set_in_carrier( + carrier, cls._TRACESTATE_HEADER_NAME, tracestate_string + ) + + +_DELIMITER_FORMAT = "[ \t]*,[ \t]*" +_MEMBER_FORMAT = "(%s)(=)(%s)" % ( + trace.TraceState.KEY_FORMAT, + trace.TraceState.VALUE_FORMAT, +) + +_DELIMITER_FORMAT_RE = re.compile(_DELIMITER_FORMAT) +_MEMBER_FORMAT_RE = re.compile(_MEMBER_FORMAT) + + +def _parse_tracestate(string: str) -> trace.TraceState: + """Parse a w3c tracestate header into a TraceState. + + Args: + string: the value of the tracestate header. + + Returns: + A valid TraceState that contains values extracted from + the tracestate header. + """ + tracestate = trace.TraceState() + for member in re.split(_DELIMITER_FORMAT_RE, string): + match = _MEMBER_FORMAT_RE.match(member) + if not match: + raise ValueError("illegal key-value format %r" % (member)) + key, _eq, value = match.groups() + tracestate[key] = value + return tracestate + + +def _format_tracestate(tracestate: trace.TraceState) -> str: + """Parse a w3c tracestate header into a TraceState. + + Args: + tracestate: the tracestate header to write + + Returns: + A string that adheres to the w3c tracestate + header format. + """ + return ",".join(map(lambda key: key + "=" + tracestate[key], tracestate)) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index b79cdeb4df5..260a198d862 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -62,7 +62,9 @@ """ import enum +import re import typing +from collections import OrderedDict from contextlib import contextmanager from opentelemetry.util import loader, types @@ -250,7 +252,7 @@ def get_default(cls) -> "TraceOptions": DEFAULT_TRACE_OPTIONS = TraceOptions.get_default() -class TraceState(typing.Dict[str, str]): +class TraceState(OrderedDict): """A list of key-value pairs representing vendor-specific trace info. Keys and values are strings of up to 256 printable US-ASCII characters. @@ -261,10 +263,39 @@ class TraceState(typing.Dict[str, str]): https://www.w3.org/TR/trace-context/#tracestate-field """ + MAX_TRACESTATE_VALUES = 32 + KEY_WITHOUT_VENDOR_FORMAT = r"[a-z][_0-9a-z\-\*\/]{0,255}" + KEY_WITH_VENDOR_FORMAT = ( + r"[a-z][_0-9a-z\-\*\/]{0,240}@[a-z][_0-9a-z\-\*\/]{0,13}" + ) + KEY_FORMAT = KEY_WITHOUT_VENDOR_FORMAT + "|" + KEY_WITH_VENDOR_FORMAT + VALUE_FORMAT = ( + r"[\x20-\x2b\x2d-\x3c\x3e-\x7e]{0,255}[\x21-\x2b\x2d-\x3c\x3e-\x7e]" + ) + + KEY_VALIDATION_RE = re.compile("^" + KEY_FORMAT + "$") + VALUE_VALIDATION_RE = re.compile("^" + VALUE_FORMAT + "$") + @classmethod def get_default(cls) -> "TraceState": return cls() + def __setitem__(self, key: str, value: str) -> None: + # According to the w3c spec, we can only store 32 values + if len(self) >= self.MAX_TRACESTATE_VALUES: + return + # TODO: I believe the otel spec calls for no exceptions + # that interfere with execution in the API. + # if not isinstance(key, str): + # raise ValueError("key must be an instance of str") + # if not re.match(self._KEY_VALIDATION_RE, key): + # raise ValueError("illegal key provided") + # if not isinstance(value, str): + # raise ValueError("value must be an instance of str") + # if not re.match(self._VALUE_VALIDATION_RE, value): + # raise ValueError("illegal value provided") + super().__setitem__(key, value) + DEFAULT_TRACE_STATE = TraceState.get_default() diff --git a/opentelemetry-api/tests/context/__init__.py b/opentelemetry-api/tests/context/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/opentelemetry-api/tests/context/propagation/__init__.py b/opentelemetry-api/tests/context/propagation/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py b/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py new file mode 100644 index 00000000000..ec51582bcd0 --- /dev/null +++ b/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py @@ -0,0 +1,208 @@ +# Copyright 2019, OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import unittest + +from opentelemetry import trace +from opentelemetry.context.propagation import tracecontexthttptextformat + +FORMAT = tracecontexthttptextformat.TraceContextHTTPTextFormat() + + +def get_as_list(dict_object, key): + value = dict_object.get(key) + return [value] if value is not None else [] + + +class TestTraceContextFormat(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.trace_id = int("12345678901234567890123456789012", 16) + cls.span_id = int("1234567890123456", 16) + + def test_no_traceparent_header(self): + """When tracecontext headers are not present, a new SpanContext + should be created. + + RFC 4.2.2: + + If no traceparent header is received, the vendor creates a new trace-id and parent-id that represents the current request. + """ + span_context = FORMAT.extract(get_as_list, {}) + self.assertTrue(isinstance(span_context, trace.SpanContext)) + + def test_from_headers_tracestate_entry_limit(self): + """If more than 32 entries are passed, do not propagate tracestate. + + RFC 3.3.1.1 + + There can be a maximum of 32 list-members in a list. + """ + + span_context = FORMAT.extract( + get_as_list, + { + "traceparent": "00-12345678901234567890123456789012-1234567890123456-00", + "tracestate": ",".join( + [ + "a00=0,a01=1,a02=2,a03=3,a04=4,a05=5,a06=6,a07=7,a08=8,a09=9", + "b00=0,b01=1,b02=2,b03=3,b04=4,b05=5,b06=6,b07=7,b08=8,b09=9", + "c00=0,c01=1,c02=2,c03=3,c04=4,c05=5,c06=6,c07=7,c08=8,c09=9", + "d00=0,d01=1,d02=2", + ] + ), + }, + ) + self.assertEqual(len(span_context.trace_state), 32) + + def test_from_headers_tracestate_duplicated_keys(self): + """If a duplicate tracestate header is present, the most recent entry + is used. + + RFC 3.3.1.4 + + Only one entry per key is allowed because the entry represents that last position in the trace. + Hence vendors must overwrite their entry upon reentry to their tracing system. + + For example, if a vendor name is Congo and a trace started in their system and then went through + a system named Rojo and later returned to Congo, the tracestate value would not be: + + congo=congosFirstPosition,rojo=rojosFirstPosition,congo=congosSecondPosition + + Instead, the entry would be rewritten to only include the most recent position: + + congo=congosSecondPosition,rojo=rojosFirstPosition + """ + span_context = FORMAT.extract( + get_as_list, + { + "traceparent": "00-12345678901234567890123456789012-1234567890123456-00", + "tracestate": "foo=1,bar=2,foo=3", + }, + ) + self.assertEqual(span_context.trace_state, {"foo": "3", "bar": "2"}) + + def test_headers_with_tracestate(self): + """When there is a traceparent and tracestate header, data from + both should be addded to the SpanContext. + """ + traceparent_value = "00-{trace_id}-{span_id}-00".format( + trace_id=format(self.trace_id, "032x"), + span_id=format(self.span_id, "016x"), + ) + tracestate_value = "foo=1,bar=2,baz=3" + span_context = FORMAT.extract( + get_as_list, + {"traceparent": traceparent_value, "tracestate": tracestate_value}, + ) + self.assertEqual(span_context.trace_id, self.trace_id) + self.assertEqual(span_context.span_id, self.span_id) + self.assertEqual( + span_context.trace_state, {"foo": "1", "bar": "2", "baz": "3"} + ) + + output = {} + FORMAT.inject(span_context, dict.__setitem__, output) + self.assertEqual(output["traceparent"], traceparent_value) + self.assertEqual(output["tracestate"], tracestate_value) + + def test_invalid_trace_id(self): + """If the trace id is invalid, we must ignore the full traceparent header. + + Also ignore any tracestate. + + RFC 3.2.2.3 + + If the trace-id value is invalid (for example if it contains non-allowed characters or all + zeros), vendors MUST ignore the traceparent. + + RFC 3.3 + + If the vendor failed to parse traceparent, it MUST NOT attempt to parse tracestate. + Note that the opposite is not true: failure to parse tracestate MUST NOT affect the parsing of traceparent. + """ + span_context = FORMAT.extract( + get_as_list, + { + "traceparent": "00-00000000000000000000000000000000-1234567890123456-00", + "tracestate": "foo=1,bar=2,foo=3", + }, + ) + self.assertEqual(span_context, trace.INVALID_SPAN_CONTEXT) + + def test_invalid_parent_id(self): + """If the parent id is invalid, we must ignore the full traceparent header. + + Also ignore any tracestate. + + RFC 3.2.2.3 + + Vendors MUST ignore the traceparent when the parent-id is invalid (for example, + if it contains non-lowercase hex characters). + + RFC 3.3 + + If the vendor failed to parse traceparent, it MUST NOT attempt to parse tracestate. + Note that the opposite is not true: failure to parse tracestate MUST NOT affect the parsing of traceparent. + """ + span_context = FORMAT.extract( + get_as_list, + { + "traceparent": "00-00000000000000000000000000000000-0000000000000000-00", + "tracestate": "foo=1,bar=2,foo=3", + }, + ) + self.assertEqual(span_context, trace.INVALID_SPAN_CONTEXT) + + def test_no_send_empty_tracestate(self): + """If the tracestate is empty, do not set the header. + + RFC 3.3.1.1 + + Empty and whitespace-only list members are allowed. Vendors MUST accept empty + tracestate headers but SHOULD avoid sending them. + """ + output = {} + FORMAT.inject( + trace.SpanContext(self.trace_id, self.span_id), + dict.__setitem__, + output, + ) + self.assertTrue("traceparent" in output) + self.assertFalse("tracestate" in output) + + def test_format_not_supported(self): + """If the traceparent does not adhere to the supported format, discard it and + create a new tracecontext. + + RFC 4.3 + + If the version cannot be parsed, the vendor creates a new traceparent header and + deletes tracestate. + """ + span_context = FORMAT.extract( + get_as_list, + { + "traceparent": "00-12345678901234567890123456789012-1234567890123456-00-residue", + "tracestate": "foo=1,bar=2,foo=3", + }, + ) + self.assertEqual(span_context, trace.INVALID_SPAN_CONTEXT) + + def test_propagate_invalid_context(self): + """Do not propagate invalid trace context. + """ + output = {} + FORMAT.inject(trace.INVALID_SPAN_CONTEXT, dict.__setitem__, output) + self.assertFalse("traceparent" in output) From 9d462c7064bddfa32c9727799b63ff686036d343 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Fri, 27 Sep 2019 11:04:31 -0700 Subject: [PATCH 2/6] addressing feedback. --- .../propagation/tracecontexthttptextformat.py | 43 +++++++++++-------- .../src/opentelemetry/trace/__init__.py | 31 +------------ .../test_tracecontexthttptextformat.py | 7 ++- 3 files changed, 32 insertions(+), 49 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py b/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py index 84fab18974e..6902d8e4f63 100644 --- a/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py +++ b/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py @@ -20,6 +20,30 @@ _T = typing.TypeVar("_T") +# Keys and values are strings of up to 256 printable US-ASCII characters. +# Implementations should conform to the the `W3C Trace Context - Tracestate`_ +# spec, which describes additional restrictions on valid field values. +# +# .. _W3C Trace Context - Tracestate: +# https://www.w3.org/TR/trace-context/#tracestate-field + + +_KEY_WITHOUT_VENDOR_FORMAT = r"[a-z][_0-9a-z\-\*\/]{0,255}" +_KEY_WITH_VENDOR_FORMAT = ( + r"[a-z][_0-9a-z\-\*\/]{0,240}@[a-z][_0-9a-z\-\*\/]{0,13}" +) + +_KEY_FORMAT = _KEY_WITHOUT_VENDOR_FORMAT + "|" + _KEY_WITH_VENDOR_FORMAT +_VALUE_FORMAT = ( + r"[\x20-\x2b\x2d-\x3c\x3e-\x7e]{0,255}[\x21-\x2b\x2d-\x3c\x3e-\x7e]" +) + +_DELIMITER_FORMAT = "[ \t]*,[ \t]*" +_MEMBER_FORMAT = "({})(=)({})".format(_KEY_FORMAT, _VALUE_FORMAT) + +_DELIMITER_FORMAT_RE = re.compile(_DELIMITER_FORMAT) +_MEMBER_FORMAT_RE = re.compile(_MEMBER_FORMAT) + class TraceContextHTTPTextFormat(httptextformat.HTTPTextFormat): """Extracts and injects using w3c TraceContext's headers. @@ -88,13 +112,8 @@ def inject( ) -> None: if context == trace.INVALID_SPAN_CONTEXT: return - traceparent_string = "-".join( - [ - "00", - format(context.trace_id, "032x"), - format(context.span_id, "016x"), - format(context.trace_options, "02x"), - ] + traceparent_string = "00-{:032x}-{:016x}-{:02x}".format( + context.trace_id, context.span_id, context.trace_options ) set_in_carrier( carrier, cls._TRACEPARENT_HEADER_NAME, traceparent_string @@ -106,16 +125,6 @@ def inject( ) -_DELIMITER_FORMAT = "[ \t]*,[ \t]*" -_MEMBER_FORMAT = "(%s)(=)(%s)" % ( - trace.TraceState.KEY_FORMAT, - trace.TraceState.VALUE_FORMAT, -) - -_DELIMITER_FORMAT_RE = re.compile(_DELIMITER_FORMAT) -_MEMBER_FORMAT_RE = re.compile(_MEMBER_FORMAT) - - def _parse_tracestate(string: str) -> trace.TraceState: """Parse a w3c tracestate header into a TraceState. diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 260a198d862..19cefa4e562 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -252,7 +252,7 @@ def get_default(cls) -> "TraceOptions": DEFAULT_TRACE_OPTIONS = TraceOptions.get_default() -class TraceState(OrderedDict): +class TraceState(typing.Dict[str, str]): """A list of key-value pairs representing vendor-specific trace info. Keys and values are strings of up to 256 printable US-ASCII characters. @@ -263,39 +263,10 @@ class TraceState(OrderedDict): https://www.w3.org/TR/trace-context/#tracestate-field """ - MAX_TRACESTATE_VALUES = 32 - KEY_WITHOUT_VENDOR_FORMAT = r"[a-z][_0-9a-z\-\*\/]{0,255}" - KEY_WITH_VENDOR_FORMAT = ( - r"[a-z][_0-9a-z\-\*\/]{0,240}@[a-z][_0-9a-z\-\*\/]{0,13}" - ) - KEY_FORMAT = KEY_WITHOUT_VENDOR_FORMAT + "|" + KEY_WITH_VENDOR_FORMAT - VALUE_FORMAT = ( - r"[\x20-\x2b\x2d-\x3c\x3e-\x7e]{0,255}[\x21-\x2b\x2d-\x3c\x3e-\x7e]" - ) - - KEY_VALIDATION_RE = re.compile("^" + KEY_FORMAT + "$") - VALUE_VALIDATION_RE = re.compile("^" + VALUE_FORMAT + "$") - @classmethod def get_default(cls) -> "TraceState": return cls() - def __setitem__(self, key: str, value: str) -> None: - # According to the w3c spec, we can only store 32 values - if len(self) >= self.MAX_TRACESTATE_VALUES: - return - # TODO: I believe the otel spec calls for no exceptions - # that interfere with execution in the API. - # if not isinstance(key, str): - # raise ValueError("key must be an instance of str") - # if not re.match(self._KEY_VALIDATION_RE, key): - # raise ValueError("illegal key provided") - # if not isinstance(value, str): - # raise ValueError("value must be an instance of str") - # if not re.match(self._VALUE_VALIDATION_RE, value): - # raise ValueError("illegal value provided") - super().__setitem__(key, value) - DEFAULT_TRACE_STATE = TraceState.get_default() diff --git a/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py b/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py index ec51582bcd0..86ca166f2a0 100644 --- a/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py +++ b/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py @@ -43,7 +43,10 @@ def test_no_traceparent_header(self): self.assertTrue(isinstance(span_context, trace.SpanContext)) def test_from_headers_tracestate_entry_limit(self): - """If more than 32 entries are passed, do not propagate tracestate. + """If more than 33 entries are passed, allow them. + + We are explicitly choosing not to limit the list members + as outlined in RFC 3.3.1.1 RFC 3.3.1.1 @@ -64,7 +67,7 @@ def test_from_headers_tracestate_entry_limit(self): ), }, ) - self.assertEqual(len(span_context.trace_state), 32) + self.assertEqual(len(span_context.trace_state), 33) def test_from_headers_tracestate_duplicated_keys(self): """If a duplicate tracestate header is present, the most recent entry From 4d5b7167e16d4b57d7db9d5e2defd0ca1cab0a9b Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Fri, 27 Sep 2019 12:01:48 -0700 Subject: [PATCH 3/6] Addressing feedback, fixing build. --- .../context/propagation/httptextformat.py | 4 +-- .../propagation/tracecontexthttptextformat.py | 14 +++++---- .../src/opentelemetry/trace/__init__.py | 2 -- .../test_tracecontexthttptextformat.py | 30 ++++++++++--------- 4 files changed, 27 insertions(+), 23 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/context/propagation/httptextformat.py b/opentelemetry-api/src/opentelemetry/context/propagation/httptextformat.py index 35bdfbb3fe8..9b6098a9a42 100644 --- a/opentelemetry-api/src/opentelemetry/context/propagation/httptextformat.py +++ b/opentelemetry-api/src/opentelemetry/context/propagation/httptextformat.py @@ -19,8 +19,8 @@ _T = typing.TypeVar("_T") -Setter = typing.Callable[[typing.Type[_T], str, str], None] -Getter = typing.Callable[[typing.Type[_T], str], typing.List[str]] +Setter = typing.Callable[[_T, str, str], None] +Getter = typing.Callable[[_T, str], typing.List[str]] class HTTPTextFormat(abc.ABC): diff --git a/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py b/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py index 6902d8e4f63..abe778db953 100644 --- a/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py +++ b/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py @@ -62,8 +62,6 @@ def extract( cls, get_from_carrier: httptextformat.Getter[_T], carrier: _T ) -> trace.SpanContext: """Extracts a valid SpanContext from the carrier. - - If a header """ header = get_from_carrier(carrier, cls._TRACEPARENT_HEADER_NAME) @@ -92,7 +90,11 @@ def extract( for tracestate_header in get_from_carrier( carrier, cls._TRACESTATE_HEADER_NAME ): - tracestate.update(_parse_tracestate(tracestate_header)) + # typing.Dict's update is not recognized by pylint: + # https://github.com/PyCQA/pylint/issues/2420 + tracestate.update( # pylint:disable=E1101 + _parse_tracestate(tracestate_header) + ) span_context = trace.SpanContext( trace_id=int(trace_id, 16), @@ -141,7 +143,9 @@ def _parse_tracestate(string: str) -> trace.TraceState: if not match: raise ValueError("illegal key-value format %r" % (member)) key, _eq, value = match.groups() - tracestate[key] = value + # typing.Dict's update is not recognized by pylint: + # https://github.com/PyCQA/pylint/issues/2420 + tracestate[key] = value # pylint:disable=E1137 return tracestate @@ -155,4 +159,4 @@ def _format_tracestate(tracestate: trace.TraceState) -> str: A string that adheres to the w3c tracestate header format. """ - return ",".join(map(lambda key: key + "=" + tracestate[key], tracestate)) + return ",".join(key + "=" + value for key, value in tracestate.items()) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 19cefa4e562..b79cdeb4df5 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -62,9 +62,7 @@ """ import enum -import re import typing -from collections import OrderedDict from contextlib import contextmanager from opentelemetry.util import loader, types diff --git a/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py b/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py index 86ca166f2a0..581466e0e52 100644 --- a/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py +++ b/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import typing import unittest from opentelemetry import trace @@ -20,16 +21,16 @@ FORMAT = tracecontexthttptextformat.TraceContextHTTPTextFormat() -def get_as_list(dict_object, key): +def get_as_list( + dict_object: typing.Dict[str, str], key: str +) -> typing.List[str]: value = dict_object.get(key) return [value] if value is not None else [] class TestTraceContextFormat(unittest.TestCase): - @classmethod - def setUpClass(cls): - cls.trace_id = int("12345678901234567890123456789012", 16) - cls.span_id = int("1234567890123456", 16) + TRACE_ID = int("12345678901234567890123456789012", 16) # type:int + SPAN_ID = int("1234567890123456", 16) # type:int def test_no_traceparent_header(self): """When tracecontext headers are not present, a new SpanContext @@ -39,7 +40,8 @@ def test_no_traceparent_header(self): If no traceparent header is received, the vendor creates a new trace-id and parent-id that represents the current request. """ - span_context = FORMAT.extract(get_as_list, {}) + output: typing.Dict[str, str] = {} + span_context = FORMAT.extract(get_as_list, output) self.assertTrue(isinstance(span_context, trace.SpanContext)) def test_from_headers_tracestate_entry_limit(self): @@ -101,21 +103,21 @@ def test_headers_with_tracestate(self): both should be addded to the SpanContext. """ traceparent_value = "00-{trace_id}-{span_id}-00".format( - trace_id=format(self.trace_id, "032x"), - span_id=format(self.span_id, "016x"), + trace_id=format(self.TRACE_ID, "032x"), + span_id=format(self.SPAN_ID, "016x"), ) tracestate_value = "foo=1,bar=2,baz=3" span_context = FORMAT.extract( get_as_list, {"traceparent": traceparent_value, "tracestate": tracestate_value}, ) - self.assertEqual(span_context.trace_id, self.trace_id) - self.assertEqual(span_context.span_id, self.span_id) + self.assertEqual(span_context.trace_id, self.TRACE_ID) + self.assertEqual(span_context.span_id, self.SPAN_ID) self.assertEqual( span_context.trace_state, {"foo": "1", "bar": "2", "baz": "3"} ) - output = {} + output: typing.Dict[str, str] = {} FORMAT.inject(span_context, dict.__setitem__, output) self.assertEqual(output["traceparent"], traceparent_value) self.assertEqual(output["tracestate"], tracestate_value) @@ -176,9 +178,9 @@ def test_no_send_empty_tracestate(self): Empty and whitespace-only list members are allowed. Vendors MUST accept empty tracestate headers but SHOULD avoid sending them. """ - output = {} + output: typing.Dict[str, str] = {} FORMAT.inject( - trace.SpanContext(self.trace_id, self.span_id), + trace.SpanContext(self.TRACE_ID, self.SPAN_ID), dict.__setitem__, output, ) @@ -206,6 +208,6 @@ def test_format_not_supported(self): def test_propagate_invalid_context(self): """Do not propagate invalid trace context. """ - output = {} + output: typing.Dict[str, str] = {} FORMAT.inject(trace.INVALID_SPAN_CONTEXT, dict.__setitem__, output) self.assertFalse("traceparent" in output) From b2b8807c7d9fb61f6f5ad742fe1e4f5b5a28940e Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Fri, 27 Sep 2019 12:14:17 -0700 Subject: [PATCH 4/6] fixing build for python versions older than 3.7 --- .../context/propagation/test_tracecontexthttptextformat.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py b/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py index 581466e0e52..909db6e86df 100644 --- a/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py +++ b/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py @@ -40,7 +40,7 @@ def test_no_traceparent_header(self): If no traceparent header is received, the vendor creates a new trace-id and parent-id that represents the current request. """ - output: typing.Dict[str, str] = {} + output = {} # type:typing.Dict[str, str] span_context = FORMAT.extract(get_as_list, output) self.assertTrue(isinstance(span_context, trace.SpanContext)) @@ -117,7 +117,7 @@ def test_headers_with_tracestate(self): span_context.trace_state, {"foo": "1", "bar": "2", "baz": "3"} ) - output: typing.Dict[str, str] = {} + output = {} # type:typing.Dict[str, str] FORMAT.inject(span_context, dict.__setitem__, output) self.assertEqual(output["traceparent"], traceparent_value) self.assertEqual(output["tracestate"], tracestate_value) @@ -178,7 +178,7 @@ def test_no_send_empty_tracestate(self): Empty and whitespace-only list members are allowed. Vendors MUST accept empty tracestate headers but SHOULD avoid sending them. """ - output: typing.Dict[str, str] = {} + output = {} # type:typing.Dict[str, str] FORMAT.inject( trace.SpanContext(self.TRACE_ID, self.SPAN_ID), dict.__setitem__, From a9f939e60f3dfb802b5918011aa0e41016764ae1 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Fri, 27 Sep 2019 12:17:57 -0700 Subject: [PATCH 5/6] missed one output --- .../context/propagation/test_tracecontexthttptextformat.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py b/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py index 909db6e86df..fb5b8b8eff1 100644 --- a/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py +++ b/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py @@ -208,6 +208,6 @@ def test_format_not_supported(self): def test_propagate_invalid_context(self): """Do not propagate invalid trace context. """ - output: typing.Dict[str, str] = {} + output = {} # type:typing.Dict[str, str] FORMAT.inject(trace.INVALID_SPAN_CONTEXT, dict.__setitem__, output) self.assertFalse("traceparent" in output) From 21b46817078831882e1a89e5f4e19aada798bfa2 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Fri, 27 Sep 2019 12:30:12 -0700 Subject: [PATCH 6/6] making tests python3.4 compatible. insertion order as iteration order is not a guarantee in older version of python. --- .../context/propagation/test_tracecontexthttptextformat.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py b/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py index fb5b8b8eff1..aaf392be248 100644 --- a/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py +++ b/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py @@ -120,7 +120,9 @@ def test_headers_with_tracestate(self): output = {} # type:typing.Dict[str, str] FORMAT.inject(span_context, dict.__setitem__, output) self.assertEqual(output["traceparent"], traceparent_value) - self.assertEqual(output["tracestate"], tracestate_value) + for pair in ["foo=1", "bar=2", "baz=3"]: + self.assertIn(pair, output["tracestate"]) + self.assertEqual(output["tracestate"].count(","), 2) def test_invalid_trace_id(self): """If the trace id is invalid, we must ignore the full traceparent header.