From 872d22c0547f3e8d3435d9e3adbee1c19cee149e Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Sat, 5 Oct 2019 20:57:24 -0700 Subject: [PATCH 01/14] Adding tracecontext checker to tox Verifying that our tracecontext is compliant with the w3c tracecontext reference is valuable. Adding a tox command to verify that the TraceContext propagator adheres to the w3c spec. --- .gitignore | 1 + examples/trace/server.py | 2 ++ .../propagation/tracecontexthttptextformat.py | 4 ++++ scripts/tracecontext-integration-test.sh | 23 +++++++++++++++++++ tox.ini | 19 +++++++++++++++ 5 files changed, 49 insertions(+) create mode 100755 scripts/tracecontext-integration-test.sh diff --git a/.gitignore b/.gitignore index 679b6fd0ccf..5fd2b28df1d 100644 --- a/.gitignore +++ b/.gitignore @@ -53,3 +53,4 @@ _build/ # mypy .mypy_cache/ +target \ No newline at end of file diff --git a/examples/trace/server.py b/examples/trace/server.py index 3632540e213..45d16149800 100755 --- a/examples/trace/server.py +++ b/examples/trace/server.py @@ -14,6 +14,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import json + import flask import requests diff --git a/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py b/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py index abe778db953..14430ec867c 100644 --- a/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py +++ b/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py @@ -44,6 +44,8 @@ _DELIMITER_FORMAT_RE = re.compile(_DELIMITER_FORMAT) _MEMBER_FORMAT_RE = re.compile(_MEMBER_FORMAT) +_TRACECONTEXT_MAXIMUM_TRACESTATE_KEYS = 32 + class TraceContextHTTPTextFormat(httptextformat.HTTPTextFormat): """Extracts and injects using w3c TraceContext's headers. @@ -95,6 +97,8 @@ def extract( tracestate.update( # pylint:disable=E1101 _parse_tracestate(tracestate_header) ) + if len(tracestate) > _TRACECONTEXT_MAXIMUM_TRACESTATE_KEYS: + tracestate = trace.TraceState() span_context = trace.SpanContext( trace_id=int(trace_id, 16), diff --git a/scripts/tracecontext-integration-test.sh b/scripts/tracecontext-integration-test.sh new file mode 100755 index 00000000000..081aa73c2fd --- /dev/null +++ b/scripts/tracecontext-integration-test.sh @@ -0,0 +1,23 @@ +#!/bin/bash +set -e +# hard-coding the git tag to ensure stable builds. +TRACECONTEXT_GIT_TAG="98f210efd89c63593dce90e2bae0a1bdcb986f51" +# clone w3c tracecontext tests +mkdir -p target +rm -rf ./target/trace-context +git clone https://github.com/w3c/trace-context ./target/trace-context +cd ./target/trace-context && git checkout $TRACECONTEXT_GIT_TAG && cd - +# start example opentelemetry service, which propagates trace-context by +# default. +python ./examples/trace/server.py & +EXAMPLE_SERVER_PID=$! +# give the app server a little time to start up. Not adding some sort +# of delay would cause many of the tracecontext tests to fail being +# unable to connect. +sleep 1 +function on-shutdown { + kill $EXAMPLE_SERVER_PID +} +trap on-shutdown EXIT +cd ./target/trace-context/test +python test.py http://127.0.0.1:5000/ \ No newline at end of file diff --git a/tox.ini b/tox.ini index a6abe8e1580..5dde966bf87 100644 --- a/tox.ini +++ b/tox.ini @@ -5,6 +5,7 @@ envlist = py3{4,5,6,7,8}-test-{api,sdk,example-app,ext-wsgi,ext-http-requests,ext-jaeger,opentracing-shim} pypy3-test-{api,sdk,example-app,ext-wsgi,ext-http-requests,ext-jaeger,opentracing-shim} lint + tracecontext py37-{mypy,mypyinstalled} docs @@ -111,3 +112,21 @@ changedir = docs commands = sphinx-build -W --keep-going -b html -T . _build/html + +[testenv:tracecontext] +basepython: python3.7 +deps = + # needed for tracecontext + aiohttp~=3.6 + # needed for example trace integration + flask~=1.1 + requests~=2.7 + +commands_pre = + pip install -e {toxinidir}/opentelemetry-api + pip install -e {toxinidir}/opentelemetry-sdk + pip install -e {toxinidir}/ext/opentelemetry-ext-http-requests + pip install -e {toxinidir}/ext/opentelemetry-ext-wsgi + +commands = + {toxinidir}/scripts/tracecontext-integration-test.sh \ No newline at end of file From 922087cc28f2175fd700182e8d5476fab1a91988 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Tue, 8 Oct 2019 21:26:29 -0700 Subject: [PATCH 02/14] Migrating generate_trace/span_id to api As the tracecontext spec calls for the creation of new, valid spans in the case of recieving invalid data from headers, it is necessary to have functions that generate valid span and trace ids. --- .../tests/test_flask_example.py | 2 +- .../src/opentelemetry/trace/__init__.py | 19 +++++++++++++ .../test_tracecontexthttptextformat.py | 27 ------------------- .../src/opentelemetry/sdk/trace/__init__.py | 18 ------------- .../context/propagation/test_b3_format.py | 14 +++++----- opentelemetry-sdk/tests/trace/test_trace.py | 16 +++++------ 6 files changed, 35 insertions(+), 61 deletions(-) diff --git a/examples/opentelemetry-example-app/tests/test_flask_example.py b/examples/opentelemetry-example-app/tests/test_flask_example.py index fd0b89e98c3..441491884b1 100644 --- a/examples/opentelemetry-example-app/tests/test_flask_example.py +++ b/examples/opentelemetry-example-app/tests/test_flask_example.py @@ -20,7 +20,7 @@ from werkzeug.wrappers import BaseResponse import opentelemetry_example_app.flask_example as flask_example -from opentelemetry.sdk import trace +from opentelemetry import trace from opentelemetry.sdk.context.propagation import b3_format diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index b64ce852b03..48f62c0f03d 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -63,6 +63,7 @@ import enum import types as python_types +import random import typing from contextlib import contextmanager @@ -308,6 +309,24 @@ def format_span_id(span_id: int) -> str: return "0x{:016x}".format(span_id) +def generate_span_id(): + """Get a new random span ID. + + Returns: + A random 64-bit int for use as a span ID + """ + return random.getrandbits(64) + + +def generate_trace_id(): + """Get a new random trace ID. + + Returns: + A random 128-bit int for use as a trace ID + """ + return random.getrandbits(128) + + class SpanContext: """The state of a Span to propagate between processes. diff --git a/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py b/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py index aaf392be248..8021d80eb2e 100644 --- a/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py +++ b/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py @@ -44,33 +44,6 @@ def test_no_traceparent_header(self): span_context = FORMAT.extract(get_as_list, output) self.assertTrue(isinstance(span_context, trace.SpanContext)) - def test_from_headers_tracestate_entry_limit(self): - """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 - - 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), 33) - def test_from_headers_tracestate_duplicated_keys(self): """If a duplicate tracestate header is present, the most recent entry is used. diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 879d4e63858..c081fb9b6fa 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -299,24 +299,6 @@ def set_status(self, status: trace_api.Status) -> None: self.status = status -def generate_span_id() -> int: - """Get a new random span ID. - - Returns: - A random 64-bit int for use as a span ID - """ - return random.getrandbits(64) - - -def generate_trace_id() -> int: - """Get a new random trace ID. - - Returns: - A random 128-bit int for use as a trace ID - """ - return random.getrandbits(128) - - class Tracer(trace_api.Tracer): """See `opentelemetry.trace.Tracer`. diff --git a/opentelemetry-sdk/tests/context/propagation/test_b3_format.py b/opentelemetry-sdk/tests/context/propagation/test_b3_format.py index 09d3f88f41e..1f4c6150cd9 100644 --- a/opentelemetry-sdk/tests/context/propagation/test_b3_format.py +++ b/opentelemetry-sdk/tests/context/propagation/test_b3_format.py @@ -16,7 +16,7 @@ import opentelemetry.sdk.context.propagation.b3_format as b3_format import opentelemetry.sdk.trace as trace -import opentelemetry.trace as api_trace +import opentelemetry.trace as trace_api FORMAT = b3_format.B3Format() @@ -30,10 +30,10 @@ class TestB3Format(unittest.TestCase): @classmethod def setUpClass(cls): cls.serialized_trace_id = b3_format.format_trace_id( - trace.generate_trace_id() + trace_api.generate_trace_id() ) cls.serialized_span_id = b3_format.format_span_id( - trace.generate_span_id() + trace_api.generate_span_id() ) def test_extract_multi_header(self): @@ -163,8 +163,8 @@ def test_invalid_single_header(self): """ carrier = {FORMAT.SINGLE_HEADER_KEY: "0-1-2-3-4-5-6-7"} span_context = FORMAT.extract(get_as_list, carrier) - self.assertEqual(span_context.trace_id, api_trace.INVALID_TRACE_ID) - self.assertEqual(span_context.span_id, api_trace.INVALID_SPAN_ID) + self.assertEqual(span_context.trace_id, trace_api.INVALID_TRACE_ID) + self.assertEqual(span_context.span_id, trace_api.INVALID_SPAN_ID) def test_missing_trace_id(self): """If a trace id is missing, populate an invalid trace id.""" @@ -173,7 +173,7 @@ def test_missing_trace_id(self): FORMAT.FLAGS_KEY: "1", } span_context = FORMAT.extract(get_as_list, carrier) - self.assertEqual(span_context.trace_id, api_trace.INVALID_TRACE_ID) + self.assertEqual(span_context.trace_id, trace_api.INVALID_TRACE_ID) def test_missing_span_id(self): """If a trace id is missing, populate an invalid trace id.""" @@ -182,4 +182,4 @@ def test_missing_span_id(self): FORMAT.FLAGS_KEY: "1", } span_context = FORMAT.extract(get_as_list, carrier) - self.assertEqual(span_context.span_id, api_trace.INVALID_SPAN_ID) + self.assertEqual(span_context.span_id, trace_api.INVALID_SPAN_ID) diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 626a5499ecf..f6aa5cf7ecd 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -221,16 +221,16 @@ def test_span_members(self): tracer = trace.Tracer("test_span_members") other_context1 = trace_api.SpanContext( - trace_id=trace.generate_trace_id(), - span_id=trace.generate_span_id(), + trace_id=trace_api.generate_trace_id(), + span_id=trace_api.generate_span_id(), ) other_context2 = trace_api.SpanContext( - trace_id=trace.generate_trace_id(), - span_id=trace.generate_span_id(), + trace_id=trace_api.generate_trace_id(), + span_id=trace_api.generate_span_id(), ) other_context3 = trace_api.SpanContext( - trace_id=trace.generate_trace_id(), - span_id=trace.generate_span_id(), + trace_id=trace_api.generate_trace_id(), + span_id=trace_api.generate_span_id(), ) self.assertIsNone(tracer.get_current_span()) @@ -362,8 +362,8 @@ def test_ended_span(self): tracer = trace.Tracer("test_ended_span") other_context1 = trace_api.SpanContext( - trace_id=trace.generate_trace_id(), - span_id=trace.generate_span_id(), + trace_id=trace_api.generate_trace_id(), + span_id=trace_api.generate_span_id(), ) with tracer.start_as_current_span("root") as root: From a0b04fd01c57799dd02592b6a4564f5c4ca7973e Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Sun, 13 Oct 2019 12:32:19 -0700 Subject: [PATCH 03/14] w3c tracecontext: changing invalid span contents to return new, random ones. This fixes all the errors, leaving 6 failures. --- .../context/propagation/tracecontexthttptextformat.py | 10 +++++----- opentelemetry-api/src/opentelemetry/trace/__init__.py | 11 +++++++++-- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py b/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py index 14430ec867c..f4d977467ac 100644 --- a/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py +++ b/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py @@ -68,11 +68,11 @@ def extract( header = get_from_carrier(carrier, cls._TRACEPARENT_HEADER_NAME) if not header: - return trace.INVALID_SPAN_CONTEXT + return trace.generate_span_context() match = re.search(cls._TRACEPARENT_HEADER_FORMAT_RE, header[0]) if not match: - return trace.INVALID_SPAN_CONTEXT + return trace.generate_span_context() version = match.group(1) trace_id = match.group(2) @@ -80,13 +80,13 @@ def extract( trace_options = match.group(4) if trace_id == "0" * 32 or span_id == "0" * 16: - return trace.INVALID_SPAN_CONTEXT + return trace.generate_span_context() if version == "00": if match.group(5): - return trace.INVALID_SPAN_CONTEXT + return trace.generate_span_context() if version == "ff": - return trace.INVALID_SPAN_CONTEXT + return trace.generate_span_context() tracestate = trace.TraceState() for tracestate_header in get_from_carrier( diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 48f62c0f03d..315fcc2be82 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -309,7 +309,7 @@ def format_span_id(span_id: int) -> str: return "0x{:016x}".format(span_id) -def generate_span_id(): +def generate_span_id() -> int: """Get a new random span ID. Returns: @@ -318,7 +318,7 @@ def generate_span_id(): return random.getrandbits(64) -def generate_trace_id(): +def generate_trace_id() -> int: """Get a new random trace ID. Returns: @@ -327,6 +327,13 @@ def generate_trace_id(): return random.getrandbits(128) +def generate_span_context() -> "SpanContext": + """Generate a valid SpanContext.""" + return SpanContext( + trace_id=generate_trace_id(), span_id=generate_span_id() + ) + + class SpanContext: """The state of a Span to propagate between processes. From c7a094b2914117fc5aac3a111a12111c2e88ad2b Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Sat, 19 Oct 2019 13:39:46 -0700 Subject: [PATCH 04/14] stopgap --- .../context/propagation/tracecontexthttptextformat.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py b/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py index f4d977467ac..c6a17a9ce94 100644 --- a/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py +++ b/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py @@ -35,7 +35,7 @@ _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]" + r"[\x20-\x2b\x2d-\x3c\x3e-\x7e]{0,255}[\x21-\x2b\x2d-\x3c\x3e-\x7e]?" ) _DELIMITER_FORMAT = "[ \t]*,[ \t]*" @@ -140,12 +140,16 @@ def _parse_tracestate(string: str) -> trace.TraceState: Returns: A valid TraceState that contains values extracted from the tracestate header. + + If the format of the TraceState is illegal, all values will + be discarded and an empty tracestate will be returned. """ tracestate = trace.TraceState() for member in re.split(_DELIMITER_FORMAT_RE, string): - match = _MEMBER_FORMAT_RE.match(member) + match = _MEMBER_FORMAT_RE.fullmatch(member) if not match: - raise ValueError("illegal key-value format %r" % (member)) + # TODO: log this? + return trace.TraceState() key, _eq, value = match.groups() # typing.Dict's update is not recognized by pylint: # https://github.com/PyCQA/pylint/issues/2420 From 754a7038ccf8d1c0e9d492109da31ae9edf295b8 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Sun, 20 Oct 2019 20:33:56 -0700 Subject: [PATCH 05/14] tracecontexthttptestformat now works with integration test The tracecontexthttptextformat now adheres completely to the w3c tracecontext test suite. moving the test endpoint to a non-root, to ensure that the basic example is clear. Adding unit tests to test_tracecontexhttptextformat that were helpful. --- examples/trace/server.py | 35 ++++++- .../propagation/tracecontexthttptextformat.py | 52 +++++----- .../src/opentelemetry/trace/__init__.py | 3 +- .../test_tracecontexthttptextformat.py | 99 +++++++++++-------- .../src/opentelemetry/sdk/trace/__init__.py | 1 - scripts/tracecontext-integration-test.sh | 11 ++- 6 files changed, 130 insertions(+), 71 deletions(-) diff --git a/examples/trace/server.py b/examples/trace/server.py index 45d16149800..5b0fb8a0b29 100755 --- a/examples/trace/server.py +++ b/examples/trace/server.py @@ -46,12 +46,43 @@ @app.route("/") +<<<<<<< HEAD def hello(): with trace.tracer().start_as_current_span("parent"): +======= +def index(): + """An example which starts a span within the span created for + the request.""" + with trace.tracer().start_span("parent"): +>>>>>>> tracecontexthttptestformat now works with integration test requests.get("https://www.wikipedia.org/wiki/Rabbit") return "hello" +<<<<<<< HEAD +======= +@app.route("/verify-tracecontext", methods=["POST"]) +def verify_tracecontext(): + """Upon reception of some payload, sends a request back to the designated url. + + This route is designed to be testable with the w3c tracecontext server / client test. + """ + for action in flask.request.json: + requests.post( + url=action["url"], + data=json.dumps(action["arguments"]), + headers={ + "Accept": "application/json", + "Content-Type": "application/json; charset=utf-8", + }, + timeout=5.0, + ) + return "hello" + + +>>>>>>> tracecontexthttptestformat now works with integration test if __name__ == "__main__": - app.run(debug=True) - span_processor.shutdown() + try: + app.run(debug=True) + finally: + span_processor.shutdown() diff --git a/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py b/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py index c6a17a9ce94..959673a26fa 100644 --- a/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py +++ b/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py @@ -35,7 +35,7 @@ _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]?" + r"[\x20-\x2b\x2d-\x3c\x3e-\x7e]{1,255}[\x21-\x2b\x2d-\x3c\x3e-\x7e]?" ) _DELIMITER_FORMAT = "[ \t]*,[ \t]*" @@ -88,17 +88,10 @@ def extract( if version == "ff": return trace.generate_span_context() - tracestate = trace.TraceState() - for tracestate_header in get_from_carrier( + tracestate_headers = get_from_carrier( carrier, cls._TRACESTATE_HEADER_NAME - ): - # 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) - ) - if len(tracestate) > _TRACECONTEXT_MAXIMUM_TRACESTATE_KEYS: - tracestate = trace.TraceState() + ) + tracestate = _parse_tracestate(tracestate_headers) span_context = trace.SpanContext( trace_id=int(trace_id, 16), @@ -131,8 +124,8 @@ def inject( ) -def _parse_tracestate(string: str) -> trace.TraceState: - """Parse a w3c tracestate header into a TraceState. +def _parse_tracestate(header_list: typing.List[str]) -> trace.TraceState: + """Parse one or more w3c tracestate header into a TraceState. Args: string: the value of the tracestate header. @@ -141,19 +134,32 @@ def _parse_tracestate(string: str) -> trace.TraceState: A valid TraceState that contains values extracted from the tracestate header. - If the format of the TraceState is illegal, all values will + If the format of one headers is illegal, all values will be discarded and an empty tracestate will be returned. + + If the number of keys is beyond the maximum, all values + will be discarded and an empty tracestate will be returned. """ tracestate = trace.TraceState() - for member in re.split(_DELIMITER_FORMAT_RE, string): - match = _MEMBER_FORMAT_RE.fullmatch(member) - if not match: - # TODO: log this? - return trace.TraceState() - key, _eq, value = match.groups() - # typing.Dict's update is not recognized by pylint: - # https://github.com/PyCQA/pylint/issues/2420 - tracestate[key] = value # pylint:disable=E1137 + for header in header_list: + for member in re.split(_DELIMITER_FORMAT_RE, header): + # empty members are valid, but no need to process further. + if not member: + continue + match = _MEMBER_FORMAT_RE.fullmatch(member) + if not match: + # TODO: log this? + return trace.TraceState() + key, _eq, value = match.groups() + if key in tracestate: + # duplicate keys are not legal in + # the header, so we will remove + return trace.TraceState() + # typing.Dict's update is not recognized by pylint: + # https://github.com/PyCQA/pylint/issues/2420 + tracestate[key] = value # pylint:disable=E1137 + if len(tracestate) > _TRACECONTEXT_MAXIMUM_TRACESTATE_KEYS: + tracestate = trace.TraceState() return tracestate diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 315fcc2be82..148dd32f9df 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -364,10 +364,11 @@ def __init__( self.trace_state = trace_state def __repr__(self) -> str: - return "{}(trace_id={}, span_id={})".format( + return "{}(trace_id={}, span_id={}, trace_state={})".format( type(self).__name__, format_trace_id(self.trace_id), format_span_id(self.span_id), + repr(self.trace_state), ) def is_valid(self) -> bool: diff --git a/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py b/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py index 8021d80eb2e..a3716f636f4 100644 --- a/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py +++ b/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py @@ -22,10 +22,10 @@ def get_as_list( - dict_object: typing.Dict[str, str], key: str + dict_object: typing.Dict[str, typing.List[str]], key: str ) -> typing.List[str]: value = dict_object.get(key) - return [value] if value is not None else [] + return value if value is not None else [] class TestTraceContextFormat(unittest.TestCase): @@ -44,33 +44,6 @@ def test_no_traceparent_header(self): span_context = FORMAT.extract(get_as_list, output) self.assertTrue(isinstance(span_context, trace.SpanContext)) - 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. @@ -82,7 +55,10 @@ def test_headers_with_tracestate(self): tracestate_value = "foo=1,bar=2,baz=3" span_context = FORMAT.extract( get_as_list, - {"traceparent": traceparent_value, "tracestate": tracestate_value}, + { + "traceparent": [traceparent_value], + "tracestate": [tracestate_value], + }, ) self.assertEqual(span_context.trace_id, self.TRACE_ID) self.assertEqual(span_context.span_id, self.SPAN_ID) @@ -98,7 +74,8 @@ def test_headers_with_tracestate(self): 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. + """If the trace id is invalid, we must ignore the full traceparent header, + and return a random, valid trace. Also ignore any tracestate. @@ -115,11 +92,19 @@ def test_invalid_trace_id(self): span_context = FORMAT.extract( get_as_list, { - "traceparent": "00-00000000000000000000000000000000-1234567890123456-00", - "tracestate": "foo=1,bar=2,foo=3", + "traceparent": [ + "00-00000000000000000000000000000000-1234567890123456-00" + ], + "tracestate": ["foo=1,bar=2,foo=3"], }, ) - self.assertEqual(span_context, trace.INVALID_SPAN_CONTEXT) + self.assertNotEqual( + span_context.span_id, trace.INVALID_SPAN_CONTEXT.span_id + ) + self.assertNotEqual( + span_context.trace_id, trace.INVALID_SPAN_CONTEXT.trace_id + ) + self.assertNotEqual(span_context.span_id, "1234567890123456") def test_invalid_parent_id(self): """If the parent id is invalid, we must ignore the full traceparent header. @@ -139,11 +124,14 @@ def test_invalid_parent_id(self): span_context = FORMAT.extract( get_as_list, { - "traceparent": "00-00000000000000000000000000000000-0000000000000000-00", - "tracestate": "foo=1,bar=2,foo=3", + "traceparent": [ + "00-00000000000000000000000000000000-0000000000000000-00" + ], + "tracestate": ["foo=1,bar=2,foo=3"], }, ) - self.assertEqual(span_context, trace.INVALID_SPAN_CONTEXT) + self.assertNotEqual(span_context, trace.INVALID_SPAN_CONTEXT) + self.assertEqual(span_context.trace_state, trace.TraceState()) def test_no_send_empty_tracestate(self): """If the tracestate is empty, do not set the header. @@ -174,11 +162,14 @@ def test_format_not_supported(self): span_context = FORMAT.extract( get_as_list, { - "traceparent": "00-12345678901234567890123456789012-1234567890123456-00-residue", - "tracestate": "foo=1,bar=2,foo=3", + "traceparent": [ + "00-12345678901234567890123456789012-1234567890123456-00-residue" + ], + "tracestate": ["foo=1,bar=2,foo=3"], }, ) - self.assertEqual(span_context, trace.INVALID_SPAN_CONTEXT) + self.assertNotEqual(span_context, trace.INVALID_SPAN_CONTEXT) + self.assertNotEqual(span_context.trace_id, self.TRACE_ID) def test_propagate_invalid_context(self): """Do not propagate invalid trace context. @@ -186,3 +177,31 @@ def test_propagate_invalid_context(self): output = {} # type:typing.Dict[str, str] FORMAT.inject(trace.INVALID_SPAN_CONTEXT, dict.__setitem__, output) self.assertFalse("traceparent" in output) + + def test_tracestate_empty_header(self): + """Do not propagate invalid trace context. + """ + span_context = FORMAT.extract( + get_as_list, + { + "traceparent": [ + "00-12345678901234567890123456789012-1234567890123456-00" + ], + "tracestate": ["foo=1", ""], + }, + ) + self.assertEqual(span_context.trace_state["foo"], "1") + + def test_tracestate_header_with_trailing_comma(self): + """Do not propagate invalid trace context. + """ + span_context = FORMAT.extract( + get_as_list, + { + "traceparent": [ + "00-12345678901234567890123456789012-1234567890123456-00" + ], + "tracestate": ["foo=1,"], + }, + ) + self.assertEqual(span_context.trace_state["foo"], "1") diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index c081fb9b6fa..1cf0f7de438 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -1,5 +1,4 @@ # 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 diff --git a/scripts/tracecontext-integration-test.sh b/scripts/tracecontext-integration-test.sh index 081aa73c2fd..e9dd0ab6697 100755 --- a/scripts/tracecontext-integration-test.sh +++ b/scripts/tracecontext-integration-test.sh @@ -1,5 +1,5 @@ #!/bin/bash -set -e +# set -e # hard-coding the git tag to ensure stable builds. TRACECONTEXT_GIT_TAG="98f210efd89c63593dce90e2bae0a1bdcb986f51" # clone w3c tracecontext tests @@ -9,15 +9,18 @@ git clone https://github.com/w3c/trace-context ./target/trace-context cd ./target/trace-context && git checkout $TRACECONTEXT_GIT_TAG && cd - # start example opentelemetry service, which propagates trace-context by # default. -python ./examples/trace/server.py & +python ./examples/trace/server.py 1>&2 & EXAMPLE_SERVER_PID=$! # give the app server a little time to start up. Not adding some sort # of delay would cause many of the tracecontext tests to fail being # unable to connect. sleep 1 function on-shutdown { - kill $EXAMPLE_SERVER_PID + # send a sigint, to ensure + # it is caught as a KeyboardInterrupt in the + # example service. + kill -2 $EXAMPLE_SERVER_PID } trap on-shutdown EXIT cd ./target/trace-context/test -python test.py http://127.0.0.1:5000/ \ No newline at end of file +python test.py http://127.0.0.1:5000/verify-tracecontext \ No newline at end of file From 638e0faa26382f8a6da0e21e775ab7bbb0a7d710 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Thu, 24 Oct 2019 21:04:32 -0700 Subject: [PATCH 06/14] Addressing feedback moving the generate span / trace id methods back to API. no longer needed due to #235 moving test service to it's own module. modifying shell script to use bourne shell, using posix standard location --- .../tests/test_flask_example.py | 2 +- examples/trace/server.py | 35 +-------- .../propagation/tracecontexthttptextformat.py | 14 ++-- .../src/opentelemetry/trace/__init__.py | 29 +------ .../src/opentelemetry/sdk/trace/__init__.py | 16 ++++ .../context/propagation/test_b3_format.py | 4 +- opentelemetry-sdk/tests/trace/test_trace.py | 16 ++-- scripts/tracecontext-integration-test.sh | 12 +-- tests/w3c_tracecontext_validation_server.py | 75 +++++++++++++++++++ tox.ini | 4 +- 10 files changed, 121 insertions(+), 86 deletions(-) create mode 100644 tests/w3c_tracecontext_validation_server.py diff --git a/examples/opentelemetry-example-app/tests/test_flask_example.py b/examples/opentelemetry-example-app/tests/test_flask_example.py index 441491884b1..fd0b89e98c3 100644 --- a/examples/opentelemetry-example-app/tests/test_flask_example.py +++ b/examples/opentelemetry-example-app/tests/test_flask_example.py @@ -20,7 +20,7 @@ from werkzeug.wrappers import BaseResponse import opentelemetry_example_app.flask_example as flask_example -from opentelemetry import trace +from opentelemetry.sdk import trace from opentelemetry.sdk.context.propagation import b3_format diff --git a/examples/trace/server.py b/examples/trace/server.py index 5b0fb8a0b29..45d16149800 100755 --- a/examples/trace/server.py +++ b/examples/trace/server.py @@ -46,43 +46,12 @@ @app.route("/") -<<<<<<< HEAD def hello(): with trace.tracer().start_as_current_span("parent"): -======= -def index(): - """An example which starts a span within the span created for - the request.""" - with trace.tracer().start_span("parent"): ->>>>>>> tracecontexthttptestformat now works with integration test requests.get("https://www.wikipedia.org/wiki/Rabbit") return "hello" -<<<<<<< HEAD -======= -@app.route("/verify-tracecontext", methods=["POST"]) -def verify_tracecontext(): - """Upon reception of some payload, sends a request back to the designated url. - - This route is designed to be testable with the w3c tracecontext server / client test. - """ - for action in flask.request.json: - requests.post( - url=action["url"], - data=json.dumps(action["arguments"]), - headers={ - "Accept": "application/json", - "Content-Type": "application/json; charset=utf-8", - }, - timeout=5.0, - ) - return "hello" - - ->>>>>>> tracecontexthttptestformat now works with integration test if __name__ == "__main__": - try: - app.run(debug=True) - finally: - span_processor.shutdown() + app.run(debug=True) + span_processor.shutdown() diff --git a/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py b/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py index 959673a26fa..5decff945e1 100644 --- a/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py +++ b/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py @@ -35,11 +35,11 @@ _KEY_FORMAT = _KEY_WITHOUT_VENDOR_FORMAT + "|" + _KEY_WITH_VENDOR_FORMAT _VALUE_FORMAT = ( - r"[\x20-\x2b\x2d-\x3c\x3e-\x7e]{1,255}[\x21-\x2b\x2d-\x3c\x3e-\x7e]?" + 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) +_MEMBER_FORMAT = "({})(=)({})[ \t]*".format(_KEY_FORMAT, _VALUE_FORMAT) _DELIMITER_FORMAT_RE = re.compile(_DELIMITER_FORMAT) _MEMBER_FORMAT_RE = re.compile(_MEMBER_FORMAT) @@ -68,11 +68,11 @@ def extract( header = get_from_carrier(carrier, cls._TRACEPARENT_HEADER_NAME) if not header: - return trace.generate_span_context() + return trace.INVALID_SPAN_CONTEXT match = re.search(cls._TRACEPARENT_HEADER_FORMAT_RE, header[0]) if not match: - return trace.generate_span_context() + return trace.INVALID_SPAN_CONTEXT version = match.group(1) trace_id = match.group(2) @@ -80,13 +80,13 @@ def extract( trace_options = match.group(4) if trace_id == "0" * 32 or span_id == "0" * 16: - return trace.generate_span_context() + return trace.INVALID_SPAN_CONTEXT if version == "00": if match.group(5): - return trace.generate_span_context() + return trace.INVALID_SPAN_CONTEXT if version == "ff": - return trace.generate_span_context() + return trace.INVALID_SPAN_CONTEXT tracestate_headers = get_from_carrier( carrier, cls._TRACESTATE_HEADER_NAME diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 148dd32f9df..faca3c77e8e 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -309,31 +309,6 @@ def format_span_id(span_id: int) -> str: return "0x{:016x}".format(span_id) -def generate_span_id() -> int: - """Get a new random span ID. - - Returns: - A random 64-bit int for use as a span ID - """ - return random.getrandbits(64) - - -def generate_trace_id() -> int: - """Get a new random trace ID. - - Returns: - A random 128-bit int for use as a trace ID - """ - return random.getrandbits(128) - - -def generate_span_context() -> "SpanContext": - """Generate a valid SpanContext.""" - return SpanContext( - trace_id=generate_trace_id(), span_id=generate_span_id() - ) - - class SpanContext: """The state of a Span to propagate between processes. @@ -364,11 +339,11 @@ def __init__( self.trace_state = trace_state def __repr__(self) -> str: - return "{}(trace_id={}, span_id={}, trace_state={})".format( + return "{}(trace_id={}, span_id={}, trace_state={!r})".format( type(self).__name__, format_trace_id(self.trace_id), format_span_id(self.span_id), - repr(self.trace_state), + self.trace_state, ) def is_valid(self) -> bool: diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 1cf0f7de438..16dc7cc6e24 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -298,6 +298,22 @@ def set_status(self, status: trace_api.Status) -> None: self.status = status +def generate_span_id() -> int: + """Get a new random span ID. + Returns: + A random 64-bit int for use as a span ID + """ + return random.getrandbits(64) + + +def generate_trace_id() -> int: + """Get a new random trace ID. + Returns: + A random 128-bit int for use as a trace ID + """ + return random.getrandbits(128) + + class Tracer(trace_api.Tracer): """See `opentelemetry.trace.Tracer`. diff --git a/opentelemetry-sdk/tests/context/propagation/test_b3_format.py b/opentelemetry-sdk/tests/context/propagation/test_b3_format.py index 1f4c6150cd9..12155082692 100644 --- a/opentelemetry-sdk/tests/context/propagation/test_b3_format.py +++ b/opentelemetry-sdk/tests/context/propagation/test_b3_format.py @@ -30,10 +30,10 @@ class TestB3Format(unittest.TestCase): @classmethod def setUpClass(cls): cls.serialized_trace_id = b3_format.format_trace_id( - trace_api.generate_trace_id() + trace.generate_trace_id() ) cls.serialized_span_id = b3_format.format_span_id( - trace_api.generate_span_id() + trace.generate_span_id() ) def test_extract_multi_header(self): diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index f6aa5cf7ecd..626a5499ecf 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -221,16 +221,16 @@ def test_span_members(self): tracer = trace.Tracer("test_span_members") other_context1 = trace_api.SpanContext( - trace_id=trace_api.generate_trace_id(), - span_id=trace_api.generate_span_id(), + trace_id=trace.generate_trace_id(), + span_id=trace.generate_span_id(), ) other_context2 = trace_api.SpanContext( - trace_id=trace_api.generate_trace_id(), - span_id=trace_api.generate_span_id(), + trace_id=trace.generate_trace_id(), + span_id=trace.generate_span_id(), ) other_context3 = trace_api.SpanContext( - trace_id=trace_api.generate_trace_id(), - span_id=trace_api.generate_span_id(), + trace_id=trace.generate_trace_id(), + span_id=trace.generate_span_id(), ) self.assertIsNone(tracer.get_current_span()) @@ -362,8 +362,8 @@ def test_ended_span(self): tracer = trace.Tracer("test_ended_span") other_context1 = trace_api.SpanContext( - trace_id=trace_api.generate_trace_id(), - span_id=trace_api.generate_span_id(), + trace_id=trace.generate_trace_id(), + span_id=trace.generate_span_id(), ) with tracer.start_as_current_span("root") as root: diff --git a/scripts/tracecontext-integration-test.sh b/scripts/tracecontext-integration-test.sh index e9dd0ab6697..aba87c47914 100755 --- a/scripts/tracecontext-integration-test.sh +++ b/scripts/tracecontext-integration-test.sh @@ -1,5 +1,5 @@ -#!/bin/bash -# set -e +#!/bin/sh +set -e # hard-coding the git tag to ensure stable builds. TRACECONTEXT_GIT_TAG="98f210efd89c63593dce90e2bae0a1bdcb986f51" # clone w3c tracecontext tests @@ -9,18 +9,18 @@ git clone https://github.com/w3c/trace-context ./target/trace-context cd ./target/trace-context && git checkout $TRACECONTEXT_GIT_TAG && cd - # start example opentelemetry service, which propagates trace-context by # default. -python ./examples/trace/server.py 1>&2 & +python ./tests/w3c_tracecontext_validation_server.py 1>&2 & EXAMPLE_SERVER_PID=$! # give the app server a little time to start up. Not adding some sort # of delay would cause many of the tracecontext tests to fail being # unable to connect. sleep 1 -function on-shutdown { +function onshutdown { # send a sigint, to ensure # it is caught as a KeyboardInterrupt in the # example service. - kill -2 $EXAMPLE_SERVER_PID + kill $EXAMPLE_SERVER_PID } -trap on-shutdown EXIT +trap onshutdown EXIT cd ./target/trace-context/test python test.py http://127.0.0.1:5000/verify-tracecontext \ No newline at end of file diff --git a/tests/w3c_tracecontext_validation_server.py b/tests/w3c_tracecontext_validation_server.py new file mode 100644 index 00000000000..44b9104d172 --- /dev/null +++ b/tests/w3c_tracecontext_validation_server.py @@ -0,0 +1,75 @@ +#!/usr/bin/env python3 +# +# 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. +""" +This server is intended to be used with the W3C tracecontext validation +Service. It implements the APIs needed to be exercised by the test bed. +""" + +import json + +import flask +import requests + +from opentelemetry import trace +from opentelemetry.ext import http_requests +from opentelemetry.ext.wsgi import OpenTelemetryMiddleware +from opentelemetry.sdk.trace import Tracer +from opentelemetry.sdk.trace.export import ( + ConsoleSpanExporter, + SimpleExportSpanProcessor, +) + +# The preferred tracer implementation must be set, as the opentelemetry-api +# defines the interface with a no-op implementation. +trace.set_preferred_tracer_implementation(lambda T: Tracer()) + +# Integrations are the glue that binds the OpenTelemetry API and the +# frameworks and libraries that are used together, automatically creating +# Spans and propagating context as appropriate. +http_requests.enable(trace.tracer()) + +# SpanExporter receives the spans and send them to the target location. +span_processor = SimpleExportSpanProcessor(ConsoleSpanExporter()) +trace.tracer().add_span_processor(span_processor) + +app = flask.Flask(__name__) +app.wsgi_app = OpenTelemetryMiddleware(app.wsgi_app) + + +@app.route("/verify-tracecontext", methods=["POST"]) +def verify_tracecontext(): + """Upon reception of some payload, sends a request back to the designated url. + + This route is designed to be testable with the w3c tracecontext server / client test. + """ + for action in flask.request.json: + requests.post( + url=action["url"], + data=json.dumps(action["arguments"]), + headers={ + "Accept": "application/json", + "Content-Type": "application/json; charset=utf-8", + }, + timeout=5.0, + ) + return "hello" + + +if __name__ == "__main__": + try: + app.run(debug=True) + finally: + span_processor.shutdown() diff --git a/tox.ini b/tox.ini index 5dde966bf87..0e518281728 100644 --- a/tox.ini +++ b/tox.ini @@ -117,7 +117,7 @@ commands = basepython: python3.7 deps = # needed for tracecontext - aiohttp~=3.6 + aiohttp~=3.6 # needed for example trace integration flask~=1.1 requests~=2.7 @@ -128,5 +128,5 @@ commands_pre = pip install -e {toxinidir}/ext/opentelemetry-ext-http-requests pip install -e {toxinidir}/ext/opentelemetry-ext-wsgi -commands = +commands = {toxinidir}/scripts/tracecontext-integration-test.sh \ No newline at end of file From 0d4fd0118aae5d041da961f4391286d85bcd6547 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Sat, 26 Oct 2019 21:50:00 -0700 Subject: [PATCH 07/14] Fixing tests Ensuring resources installed to the target directory are not included in style and linting. Modifying tox invocation to include python version to ensure it's called by travis-ci. Fixing tests that are no longer valid due to previous changes ( tracecontext returning INVALID_SPAN, start_as_current_span called) --- .flake8 | 1 + .isort.cfg | 1 + .../tests/test_requests_integration.py | 4 ++-- .../propagation/tracecontexthttptextformat.py | 2 +- .../src/opentelemetry/trace/__init__.py | 2 +- .../test_tracecontexthttptextformat.py | 18 +++++------------- pyproject.toml | 2 ++ tox.ini | 2 +- 8 files changed, 14 insertions(+), 18 deletions(-) diff --git a/.flake8 b/.flake8 index 5abd0630ea0..a0924f947d6 100644 --- a/.flake8 +++ b/.flake8 @@ -10,6 +10,7 @@ exclude = .svn .tox CVS + target __pycache__ ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/gen/ ext/opentelemetry-ext-jaeger/build/* diff --git a/.isort.cfg b/.isort.cfg index 4bf64a34f1a..96011ae93de 100644 --- a/.isort.cfg +++ b/.isort.cfg @@ -12,4 +12,5 @@ line_length=79 ; ) ; docs: https://github.com/timothycrosley/isort#multi-line-output-modes multi_line_output=3 +skip=target skip_glob=ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/gen/* diff --git a/ext/opentelemetry-ext-http-requests/tests/test_requests_integration.py b/ext/opentelemetry-ext-http-requests/tests/test_requests_integration.py index 2a02e1916ab..1174de9a039 100644 --- a/ext/opentelemetry-ext-http-requests/tests/test_requests_integration.py +++ b/ext/opentelemetry-ext-http-requests/tests/test_requests_integration.py @@ -38,7 +38,7 @@ def setspanattr(key, value): self.span_attrs[key] = value self.span.set_attribute = setspanattr - self.start_span_patcher = mock.patch.object( + self.start_as_current_span_patcher = mock.patch.object( self.tracer, "start_as_current_span", autospec=True, @@ -64,7 +64,7 @@ def setspanattr(key, value): def tearDown(self): opentelemetry.ext.http_requests.disable() self.send_patcher.stop() - self.start_span_patcher.stop() + self.start_as_current_span_patcher.stop() def test_basic(self): url = "https://www.example.org/foo/bar?x=y#top" diff --git a/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py b/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py index 5decff945e1..b0f0e112010 100644 --- a/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py +++ b/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py @@ -151,7 +151,7 @@ def _parse_tracestate(header_list: typing.List[str]) -> trace.TraceState: # TODO: log this? return trace.TraceState() key, _eq, value = match.groups() - if key in tracestate: + if key in tracestate: # pylint:disable=E1135 # duplicate keys are not legal in # the header, so we will remove return trace.TraceState() diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index faca3c77e8e..f83960abb62 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -62,8 +62,8 @@ """ import enum -import types as python_types import random +import types as python_types import typing from contextlib import contextmanager diff --git a/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py b/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py index a3716f636f4..c4d3af7d8a8 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 = {} # type:typing.Dict[str, str] + output = {} # type:typing.Dict[str, typing.List[str]] span_context = FORMAT.extract(get_as_list, output) self.assertTrue(isinstance(span_context, trace.SpanContext)) @@ -98,12 +98,7 @@ def test_invalid_trace_id(self): "tracestate": ["foo=1,bar=2,foo=3"], }, ) - self.assertNotEqual( - span_context.span_id, trace.INVALID_SPAN_CONTEXT.span_id - ) - self.assertNotEqual( - span_context.trace_id, trace.INVALID_SPAN_CONTEXT.trace_id - ) + self.assertEqual(span_context, trace.INVALID_SPAN_CONTEXT) self.assertNotEqual(span_context.span_id, "1234567890123456") def test_invalid_parent_id(self): @@ -130,8 +125,7 @@ def test_invalid_parent_id(self): "tracestate": ["foo=1,bar=2,foo=3"], }, ) - self.assertNotEqual(span_context, trace.INVALID_SPAN_CONTEXT) - self.assertEqual(span_context.trace_state, trace.TraceState()) + 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. @@ -156,8 +150,7 @@ def test_format_not_supported(self): RFC 4.3 - If the version cannot be parsed, the vendor creates a new traceparent header and - deletes tracestate. + If the version cannot be parsed, return an invalid trace header. """ span_context = FORMAT.extract( get_as_list, @@ -168,8 +161,7 @@ def test_format_not_supported(self): "tracestate": ["foo=1,bar=2,foo=3"], }, ) - self.assertNotEqual(span_context, trace.INVALID_SPAN_CONTEXT) - self.assertNotEqual(span_context.trace_id, self.TRACE_ID) + self.assertEqual(span_context, trace.INVALID_SPAN_CONTEXT) def test_propagate_invalid_context(self): """Do not propagate invalid trace context. diff --git a/pyproject.toml b/pyproject.toml index eff7e2e3ec6..cb3c2eb5467 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,8 +9,10 @@ exclude = ''' | \.mypy_cache | \.tox | \.venv + | \.vscode | _build | buck-out + | target | build | dist | ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/gen # generated files diff --git a/tox.ini b/tox.ini index 0e518281728..8506047e2de 100644 --- a/tox.ini +++ b/tox.ini @@ -5,7 +5,7 @@ envlist = py3{4,5,6,7,8}-test-{api,sdk,example-app,ext-wsgi,ext-http-requests,ext-jaeger,opentracing-shim} pypy3-test-{api,sdk,example-app,ext-wsgi,ext-http-requests,ext-jaeger,opentracing-shim} lint - tracecontext + py37-tracecontext py37-{mypy,mypyinstalled} docs From d656a6357fd1aec7ed6a9f35de85186f8545ad47 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Mon, 28 Oct 2019 06:02:03 -0700 Subject: [PATCH 08/14] tracecotnext tests now run with py37-tracecontext --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 8506047e2de..8ee57ebc118 100644 --- a/tox.ini +++ b/tox.ini @@ -113,7 +113,7 @@ changedir = docs commands = sphinx-build -W --keep-going -b html -T . _build/html -[testenv:tracecontext] +[testenv:py37-tracecontext] basepython: python3.7 deps = # needed for tracecontext From ed628aa4c9edc76e0d372362132d4022d8cf33ea Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Mon, 28 Oct 2019 07:33:42 -0700 Subject: [PATCH 09/14] using bourne shell syntax for function declaration --- scripts/tracecontext-integration-test.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/tracecontext-integration-test.sh b/scripts/tracecontext-integration-test.sh index aba87c47914..7b74145b66f 100755 --- a/scripts/tracecontext-integration-test.sh +++ b/scripts/tracecontext-integration-test.sh @@ -15,7 +15,8 @@ EXAMPLE_SERVER_PID=$! # of delay would cause many of the tracecontext tests to fail being # unable to connect. sleep 1 -function onshutdown { +onshutdown() +{ # send a sigint, to ensure # it is caught as a KeyboardInterrupt in the # example service. From 2bf7d1249ef3915410e45215148795c5384f4982 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Mon, 28 Oct 2019 13:50:10 -0700 Subject: [PATCH 10/14] addressing feedback --- examples/trace/server.py | 2 -- opentelemetry-api/src/opentelemetry/trace/__init__.py | 1 - .../context/propagation/test_tracecontexthttptextformat.py | 4 +--- 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/examples/trace/server.py b/examples/trace/server.py index 45d16149800..3632540e213 100755 --- a/examples/trace/server.py +++ b/examples/trace/server.py @@ -14,8 +14,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import json - import flask import requests diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index f83960abb62..755e682f79f 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -62,7 +62,6 @@ """ import enum -import random import types as python_types import typing from contextlib import contextmanager diff --git a/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py b/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py index c4d3af7d8a8..ed952e0dbab 100644 --- a/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py +++ b/opentelemetry-api/tests/context/propagation/test_tracecontexthttptextformat.py @@ -99,7 +99,6 @@ def test_invalid_trace_id(self): }, ) self.assertEqual(span_context, trace.INVALID_SPAN_CONTEXT) - self.assertNotEqual(span_context.span_id, "1234567890123456") def test_invalid_parent_id(self): """If the parent id is invalid, we must ignore the full traceparent header. @@ -171,8 +170,7 @@ def test_propagate_invalid_context(self): self.assertFalse("traceparent" in output) def test_tracestate_empty_header(self): - """Do not propagate invalid trace context. - """ + """Test tracestate with an additional empty header (should be ignored)""" span_context = FORMAT.extract( get_as_list, { From 6194a5fc332a3083c595c27d05a8e20cb33bf82a Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Mon, 28 Oct 2019 14:15:49 -0700 Subject: [PATCH 11/14] fixing linting / broken requests test --- .../tests/test_requests_integration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/opentelemetry-ext-http-requests/tests/test_requests_integration.py b/ext/opentelemetry-ext-http-requests/tests/test_requests_integration.py index 1174de9a039..2a02e1916ab 100644 --- a/ext/opentelemetry-ext-http-requests/tests/test_requests_integration.py +++ b/ext/opentelemetry-ext-http-requests/tests/test_requests_integration.py @@ -38,7 +38,7 @@ def setspanattr(key, value): self.span_attrs[key] = value self.span.set_attribute = setspanattr - self.start_as_current_span_patcher = mock.patch.object( + self.start_span_patcher = mock.patch.object( self.tracer, "start_as_current_span", autospec=True, @@ -64,7 +64,7 @@ def setspanattr(key, value): def tearDown(self): opentelemetry.ext.http_requests.disable() self.send_patcher.stop() - self.start_as_current_span_patcher.stop() + self.start_span_patcher.stop() def test_basic(self): url = "https://www.example.org/foo/bar?x=y#top" From ad5ba02b3418d97f9eb6b34a162812d2ccf288f6 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Mon, 28 Oct 2019 14:24:11 -0700 Subject: [PATCH 12/14] addressing feedback --- .../context/propagation/tracecontexthttptextformat.py | 6 ++++-- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py b/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py index b0f0e112010..20f2601fa20 100644 --- a/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py +++ b/opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py @@ -141,6 +141,7 @@ def _parse_tracestate(header_list: typing.List[str]) -> trace.TraceState: will be discarded and an empty tracestate will be returned. """ tracestate = trace.TraceState() + value_count = 0 for header in header_list: for member in re.split(_DELIMITER_FORMAT_RE, header): # empty members are valid, but no need to process further. @@ -158,8 +159,9 @@ def _parse_tracestate(header_list: typing.List[str]) -> trace.TraceState: # typing.Dict's update is not recognized by pylint: # https://github.com/PyCQA/pylint/issues/2420 tracestate[key] = value # pylint:disable=E1137 - if len(tracestate) > _TRACECONTEXT_MAXIMUM_TRACESTATE_KEYS: - tracestate = trace.TraceState() + value_count += 1 + if value_count > _TRACECONTEXT_MAXIMUM_TRACESTATE_KEYS: + return trace.TraceState() return tracestate diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 16dc7cc6e24..0909b9b434c 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -1,4 +1,5 @@ # 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 From b5be7cc712edff752b0a4a1805273e94b3de464b Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Mon, 28 Oct 2019 18:18:01 -0700 Subject: [PATCH 13/14] Formatting, EOF newlines --- .gitignore | 2 +- scripts/tracecontext-integration-test.sh | 2 +- tests/w3c_tracecontext_validation_server.py | 6 ++++-- tox.ini | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index 5fd2b28df1d..c9acc319404 100644 --- a/.gitignore +++ b/.gitignore @@ -53,4 +53,4 @@ _build/ # mypy .mypy_cache/ -target \ No newline at end of file +target diff --git a/scripts/tracecontext-integration-test.sh b/scripts/tracecontext-integration-test.sh index 7b74145b66f..4d482ddafe5 100755 --- a/scripts/tracecontext-integration-test.sh +++ b/scripts/tracecontext-integration-test.sh @@ -24,4 +24,4 @@ onshutdown() } trap onshutdown EXIT cd ./target/trace-context/test -python test.py http://127.0.0.1:5000/verify-tracecontext \ No newline at end of file +python test.py http://127.0.0.1:5000/verify-tracecontext diff --git a/tests/w3c_tracecontext_validation_server.py b/tests/w3c_tracecontext_validation_server.py index 44b9104d172..a26141f14c8 100644 --- a/tests/w3c_tracecontext_validation_server.py +++ b/tests/w3c_tracecontext_validation_server.py @@ -51,9 +51,11 @@ @app.route("/verify-tracecontext", methods=["POST"]) def verify_tracecontext(): - """Upon reception of some payload, sends a request back to the designated url. + """Upon reception of some payload, sends a request back to the designated + url. - This route is designed to be testable with the w3c tracecontext server / client test. + This route is designed to be testable with the w3c tracecontext server / + client test. """ for action in flask.request.json: requests.post( diff --git a/tox.ini b/tox.ini index 8ee57ebc118..e30cb1a14b7 100644 --- a/tox.ini +++ b/tox.ini @@ -129,4 +129,4 @@ commands_pre = pip install -e {toxinidir}/ext/opentelemetry-ext-wsgi commands = - {toxinidir}/scripts/tracecontext-integration-test.sh \ No newline at end of file + {toxinidir}/scripts/tracecontext-integration-test.sh From 7dd47da0f0df18f3ea97528c093620cb25f7cf98 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Mon, 28 Oct 2019 17:57:25 -0700 Subject: [PATCH 14/14] Reblacken --- ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py | 2 +- .../distributedcontext/propagation/binaryformat.py | 2 +- opentelemetry-api/src/opentelemetry/propagators/__init__.py | 2 +- opentelemetry-api/src/opentelemetry/trace/__init__.py | 2 +- opentelemetry-api/src/opentelemetry/util/loader.py | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py b/ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py index a88782d6428..e5dc9654fd2 100644 --- a/ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py +++ b/ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py @@ -210,7 +210,7 @@ def test_request_attributes_with_partial_raw_uri(self): self.validate_url("http://127.0.0.1/#top") def test_request_attributes_with_partial_raw_uri_and_nonstandard_port( - self + self, ): self.environ["RAW_URI"] = "/?" del self.environ["HTTP_HOST"] diff --git a/opentelemetry-api/src/opentelemetry/distributedcontext/propagation/binaryformat.py b/opentelemetry-api/src/opentelemetry/distributedcontext/propagation/binaryformat.py index 15f8cfdf63d..d6d083c0dae 100644 --- a/opentelemetry-api/src/opentelemetry/distributedcontext/propagation/binaryformat.py +++ b/opentelemetry-api/src/opentelemetry/distributedcontext/propagation/binaryformat.py @@ -44,7 +44,7 @@ def to_bytes(context: DistributedContext) -> bytes: @staticmethod @abc.abstractmethod def from_bytes( - byte_representation: bytes + byte_representation: bytes, ) -> typing.Optional[DistributedContext]: """Return a DistributedContext that was represented by bytes. diff --git a/opentelemetry-api/src/opentelemetry/propagators/__init__.py b/opentelemetry-api/src/opentelemetry/propagators/__init__.py index 5b71e8785a9..bb75d84c3a4 100644 --- a/opentelemetry-api/src/opentelemetry/propagators/__init__.py +++ b/opentelemetry-api/src/opentelemetry/propagators/__init__.py @@ -78,7 +78,7 @@ def get_global_httptextformat() -> httptextformat.HTTPTextFormat: def set_global_httptextformat( - http_text_format: httptextformat.HTTPTextFormat + http_text_format: httptextformat.HTTPTextFormat, ) -> None: global _HTTP_TEXT_FORMAT # pylint:disable=global-statement _HTTP_TEXT_FORMAT = http_text_format diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 755e682f79f..1ac7d73d49e 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -590,7 +590,7 @@ def tracer() -> Tracer: def set_preferred_tracer_implementation( - factory: ImplementationFactory + factory: ImplementationFactory, ) -> None: """Set the factory to be used to create the tracer. diff --git a/opentelemetry-api/src/opentelemetry/util/loader.py b/opentelemetry-api/src/opentelemetry/util/loader.py index 0781ce15b79..3ae5a52fc51 100644 --- a/opentelemetry-api/src/opentelemetry/util/loader.py +++ b/opentelemetry-api/src/opentelemetry/util/loader.py @@ -173,7 +173,7 @@ def _load_impl( def set_preferred_default_implementation( - implementation_factory: _UntrustedImplFactory[_T] + implementation_factory: _UntrustedImplFactory[_T], ) -> None: """Sets a factory function that may be called for any implementation object. See the :ref:`module docs ` for more details."""