From ee8191564dda2a0fc832097f05b138166cd304ce Mon Sep 17 00:00:00 2001 From: Prajilesh N Date: Sat, 3 Oct 2020 01:24:26 +0530 Subject: [PATCH 01/11] converted textmap propagator getter to a class and added keys method --- .../exporter/datadog/propagator.py | 10 ++-- .../tests/test_datadog_format.py | 15 ++++-- .../instrumentation/asgi/__init__.py | 40 ++++++++++------ .../instrumentation/celery/__init__.py | 17 +++++-- .../instrumentation/grpc/_server.py | 17 +++++-- .../opentracing_shim/__init__.py | 46 +++++++++++++++++-- .../instrumentation/tornado/__init__.py | 15 ++++-- .../instrumentation/wsgi/__init__.py | 36 ++++++++++----- .../baggage/propagation/__init__.py | 4 +- .../src/opentelemetry/propagators/__init__.py | 8 ++-- .../opentelemetry/propagators/composite.py | 2 +- .../trace/propagation/textmap.py | 41 ++++++++++++++++- .../trace/propagation/tracecontext.py | 6 +-- .../tests/baggage/test_baggage_propagation.py | 15 ++++-- .../propagators/test_global_httptextformat.py | 17 +++++-- .../test_tracecontexthttptextformat.py | 17 +++++-- opentelemetry-sdk/CHANGELOG.md | 2 + .../sdk/trace/propagation/b3_format.py | 12 ++--- .../tests/trace/propagation/test_b3_format.py | 20 +++++--- .../src/opentelemetry/test/mock_textmap.py | 8 ++-- 20 files changed, 254 insertions(+), 94 deletions(-) diff --git a/exporter/opentelemetry-exporter-datadog/src/opentelemetry/exporter/datadog/propagator.py b/exporter/opentelemetry-exporter-datadog/src/opentelemetry/exporter/datadog/propagator.py index d2e60476e68..df73b4f1664 100644 --- a/exporter/opentelemetry-exporter-datadog/src/opentelemetry/exporter/datadog/propagator.py +++ b/exporter/opentelemetry-exporter-datadog/src/opentelemetry/exporter/datadog/propagator.py @@ -39,24 +39,24 @@ class DatadogFormat(TextMapPropagator): def extract( self, - get_from_carrier: Getter[TextMapPropagatorT], + get_from_carrier: Getter, carrier: TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: trace_id = extract_first_element( - get_from_carrier(carrier, self.TRACE_ID_KEY) + get_from_carrier.get(carrier, self.TRACE_ID_KEY) ) span_id = extract_first_element( - get_from_carrier(carrier, self.PARENT_ID_KEY) + get_from_carrier.get(carrier, self.PARENT_ID_KEY) ) sampled = extract_first_element( - get_from_carrier(carrier, self.SAMPLING_PRIORITY_KEY) + get_from_carrier.get(carrier, self.SAMPLING_PRIORITY_KEY) ) origin = extract_first_element( - get_from_carrier(carrier, self.ORIGIN_KEY) + get_from_carrier.get(carrier, self.ORIGIN_KEY) ) trace_flags = trace.TraceFlags() diff --git a/exporter/opentelemetry-exporter-datadog/tests/test_datadog_format.py b/exporter/opentelemetry-exporter-datadog/tests/test_datadog_format.py index 994cac2d602..6ff865ba815 100644 --- a/exporter/opentelemetry-exporter-datadog/tests/test_datadog_format.py +++ b/exporter/opentelemetry-exporter-datadog/tests/test_datadog_format.py @@ -22,9 +22,18 @@ FORMAT = propagator.DatadogFormat() -def get_as_list(dict_object, key): - value = dict_object.get(key) - return [value] if value is not None else [] +class Getter: + @staticmethod + def get(dict_object, key): + value = dict_object.get(key) + return [value] if value is not None else [] + + @staticmethod + def keys(dict_object): + return dict_object.keys() + + +get_as_list = Getter() class TestDatadogFormat(unittest.TestCase): diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index 879662ddcfb..89920a2903b 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -32,18 +32,32 @@ from opentelemetry.trace.status import Status, StatusCanonicalCode -def get_header_from_scope(scope: dict, header_name: str) -> typing.List[str]: - """Retrieve a HTTP header value from the ASGI scope. +class Getter: + @staticmethod + def get(scope: dict, header_name: str) -> typing.List[str]: + """Retrieve a HTTP header value from the ASGI scope. - Returns: - A list with a single string with the header value if it exists, else an empty list. - """ - headers = scope.get("headers") - return [ - value.decode("utf8") - for (key, value) in headers - if key.decode("utf8") == header_name - ] + Returns: + A list with a single string with the header value if it exists, else an empty list. + """ + headers = scope.get("headers") + return [ + value.decode("utf8") + for (key, value) in headers + if key.decode("utf8") == header_name + ] + + @staticmethod + def keys(scope: dict) -> typing.List[str]: + """Retrieve all the HTTP header keys for an ASGI scope.. + + Returns: + A list with all the keys in scope. + """ + return scope.keys() + + +get_header_from_scope = Getter() def collect_request_attributes(scope): @@ -72,10 +86,10 @@ def collect_request_attributes(scope): http_method = scope.get("method") if http_method: result["http.method"] = http_method - http_host_value = ",".join(get_header_from_scope(scope, "host")) + http_host_value = ",".join(get_header_from_scope.get(scope, "host")) if http_host_value: result["http.server_name"] = http_host_value - http_user_agent = get_header_from_scope(scope, "user-agent") + http_user_agent = get_header_from_scope.get(scope, "user-agent") if len(http_user_agent) > 0: result["http.user_agent"] = http_user_agent[0] diff --git a/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py b/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py index 4768e93d18e..bb3780e4944 100644 --- a/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py @@ -118,6 +118,7 @@ def _trace_prerun(self, *args, **kwargs): return request = task.request + carrier_extractor = Getter() tracectx = propagators.extract(carrier_extractor, request) or {} parent = get_current_span(tracectx) @@ -248,8 +249,14 @@ def _trace_retry(*args, **kwargs): span.set_attribute(_TASK_RETRY_REASON_KEY, str(reason)) -def carrier_extractor(carrier, key): - value = getattr(carrier, key, []) - if isinstance(value, str) or not isinstance(value, Iterable): - value = (value,) - return value +class Getter: + @staticmethod + def get(carrier, key): + value = getattr(carrier, key, []) + if isinstance(value, str) or not isinstance(value, Iterable): + value = (value,) + return value + + @staticmethod + def keys(carrier): + return carrier.keys() diff --git a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py index cb0e997d367..8a6b939f6f2 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py @@ -110,6 +110,18 @@ def _check_error_code(span, servicer_context, rpc_info): rpc_info.error = servicer_context.code +class Getter: + @staticmethod + def get(metadata, key) -> List[str]: + md_dict = {md.key: md.value for md in metadata} + return [md_dict[key]] if key in md_dict else [] + + @staticmethod + def keys(metadata) -> List[str]: + md_dict = {md.key: md.value for md in metadata} + return md_dict.keys() + + class OpenTelemetryServerInterceptor( grpcext.UnaryServerInterceptor, grpcext.StreamServerInterceptor ): @@ -121,11 +133,8 @@ def __init__(self, tracer): def _set_remote_context(self, servicer_context): metadata = servicer_context.invocation_metadata() if metadata: - md_dict = {md.key: md.value for md in metadata} - - def get_from_grpc_metadata(metadata, key) -> List[str]: - return [md_dict[key]] if key in md_dict else [] + get_from_grpc_metadata = Getter() # Update the context with the traceparent from the RPC metadata. ctx = propagators.extract(get_from_grpc_metadata, metadata) token = attach(ctx) diff --git a/instrumentation/opentelemetry-instrumentation-opentracing-shim/src/opentelemetry/instrumentation/opentracing_shim/__init__.py b/instrumentation/opentelemetry-instrumentation-opentracing-shim/src/opentelemetry/instrumentation/opentracing_shim/__init__.py index 6bb22130d8e..10ab5caf809 100644 --- a/instrumentation/opentelemetry-instrumentation-opentracing-shim/src/opentelemetry/instrumentation/opentracing_shim/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-opentracing-shim/src/opentelemetry/instrumentation/opentracing_shim/__init__.py @@ -503,6 +503,47 @@ def tracer(self) -> "TracerShim": return self._tracer +class Getter: + """This class provides an interface that enables extracting propagated + fields from a carrier + + """ + + @staticmethod + def get(carrier, key): + """Function that can retrieve zero + or more values from the carrier. In the case that + the value does not exist, returns an empty list. + + Args: + carrier: and object which contains values that are + used to construct a Context. This object + must be paired with an appropriate get_from_carrier + which understands how to extract a value from it. + key: key of a field in carrier. + Returns: + first value of the propagation key or an empty list if the key doesn't exist. + """ + + value = carrier.get(key) + return [value] if value is not None else [] + + @staticmethod + def keys(carrier): + """Function that can retrieve all the keys in a carrier object. + + Args: + carrier: and object which contains values that are + used to construct a Context. This object + must be paired with an appropriate get_from_carrier + which understands how to extract a value from it. + Returns: + list of keys from the carrier. + """ + + return carrier.keys() + + class TracerShim(Tracer): """Wraps a :class:`opentelemetry.trace.Tracer` object. @@ -706,10 +747,7 @@ def extract(self, format: object, carrier: object): if format not in self._supported_formats: raise UnsupportedFormatException - def get_as_list(dict_object, key): - value = dict_object.get(key) - return [value] if value is not None else [] - + get_as_list = Getter() propagator = propagators.get_global_textmap() ctx = propagator.extract(get_as_list, carrier) span = get_current_span(ctx) diff --git a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py index 6379d841a03..ad6d2d2baf8 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py @@ -174,11 +174,15 @@ def _log_exception(tracer, func, handler, args, kwargs): return func(*args, **kwargs) -def _get_header_from_request_headers( - headers: dict, header_name: str -) -> typing.List[str]: - header = headers.get(header_name) - return [header] if header else [] +class Getter: + @staticmethod + def get(headers: dict, header_name: str) -> typing.List[str]: + header = headers.get(header_name) + return [header] if header else [] + + @staticmethod + def keys(headers) -> typing.List[str]: + return headers.keys() def _get_attributes_from_request(request): @@ -206,6 +210,7 @@ def _get_operation_name(handler, request): def _start_span(tracer, handler, start_time) -> _TraceContext: + _get_header_from_request_headers = Getter() token = context.attach( propagators.extract( _get_header_from_request_headers, handler.request.headers, diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index 56c8b755c5c..9b0e54ad372 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -66,19 +66,31 @@ def hello(): _HTTP_VERSION_PREFIX = "HTTP/" -def get_header_from_environ( - environ: dict, header_name: str -) -> typing.List[str]: - """Retrieve a HTTP header value from the PEP3333-conforming WSGI environ. +class Getter: + @staticmethod + def get(environ: dict, header_name: str) -> typing.List[str]: + """Retrieve a HTTP header value from the PEP3333-conforming WSGI environ. - Returns: - A list with a single string with the header value if it exists, else an empty list. - """ - environ_key = "HTTP_" + header_name.upper().replace("-", "_") - value = environ.get(environ_key) - if value is not None: - return [value] - return [] + Returns: + A list with a single string with the header value if it exists, else an empty list. + """ + environ_key = "HTTP_" + header_name.upper().replace("-", "_") + value = environ.get(environ_key) + if value is not None: + return [value] + return [] + + @staticmethod + def keys(environ: dict) -> typing.List[str]: + """Retrieve all the HTTP header keys for an PEP3333-conforming WSGI environ. + + Returns: + A list with all the keys in environ. + """ + return environ.keys() + + +get_header_from_environ = Getter() def setifnotnone(dic, key, value): diff --git a/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py b/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py index d0920e68590..7d589cb0c64 100644 --- a/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py +++ b/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py @@ -29,7 +29,7 @@ class BaggagePropagator(textmap.TextMapPropagator): def extract( self, - get_from_carrier: textmap.Getter[textmap.TextMapPropagatorT], + get_from_carrier: textmap.Getter, carrier: textmap.TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: @@ -43,7 +43,7 @@ def extract( context = get_current() header = _extract_first_element( - get_from_carrier(carrier, self._BAGGAGE_HEADER_NAME) + get_from_carrier.get(carrier, self._BAGGAGE_HEADER_NAME) ) if not header or len(header) > self.MAX_HEADER_LENGTH: diff --git a/opentelemetry-api/src/opentelemetry/propagators/__init__.py b/opentelemetry-api/src/opentelemetry/propagators/__init__.py index c274b19f8a0..1b3f3e84b94 100644 --- a/opentelemetry-api/src/opentelemetry/propagators/__init__.py +++ b/opentelemetry-api/src/opentelemetry/propagators/__init__.py @@ -82,16 +82,16 @@ def example_route(): def extract( - get_from_carrier: textmap.Getter[textmap.TextMapPropagatorT], + get_from_carrier: textmap.Getter, carrier: textmap.TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: """ Uses the configured propagator to extract a Context from the carrier. Args: - get_from_carrier: a function that can retrieve zero - or more values from the carrier. In the case that - the value does not exist, return an empty list. + get_from_carrier: an object which contains a get function that can retrieve zero + or more values from the carrier and a keys function that can get all the keys + from carrier. carrier: and object which contains values that are used to construct a Context. This object must be paired with an appropriate get_from_carrier diff --git a/opentelemetry-api/src/opentelemetry/propagators/composite.py b/opentelemetry-api/src/opentelemetry/propagators/composite.py index 3499d2ea08a..c307479021e 100644 --- a/opentelemetry-api/src/opentelemetry/propagators/composite.py +++ b/opentelemetry-api/src/opentelemetry/propagators/composite.py @@ -35,7 +35,7 @@ def __init__( def extract( self, - get_from_carrier: textmap.Getter[textmap.TextMapPropagatorT], + get_from_carrier: textmap.Getter, carrier: textmap.TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: diff --git a/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py b/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py index 6f9ed897e11..af9b6689651 100644 --- a/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py +++ b/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py @@ -20,7 +20,44 @@ TextMapPropagatorT = typing.TypeVar("TextMapPropagatorT") Setter = typing.Callable[[TextMapPropagatorT, str, str], None] -Getter = typing.Callable[[TextMapPropagatorT, str], typing.List[str]] + + +class Getter(abc.ABC): + """This class provides an interface that enables extracting propagated + fields from a carrier + + """ + + @staticmethod + @abc.abstractmethod + def get(carrier: TextMapPropagatorT, key: str) -> typing.List[str]: + """Function that can retrieve zero + or more values from the carrier. In the case that + the value does not exist, returns an empty list. + + Args: + carrier: and object which contains values that are + used to construct a Context. This object + must be paired with an appropriate get_from_carrier + which understands how to extract a value from it. + key: key of a field in carrier. + Returns: + first value of the propagation key or an empty list if the key doesn't exist. + """ + + @staticmethod + @abc.abstractmethod + def keys(carrier: TextMapPropagatorT) -> typing.List[str]: + """Function that can retrieve all the keys in a carrier object. + + Args: + carrier: and object which contains values that are + used to construct a Context. This object + must be paired with an appropriate get_from_carrier + which understands how to extract a value from it. + Returns: + list of keys from the carrier. + """ class TextMapPropagator(abc.ABC): @@ -35,7 +72,7 @@ class TextMapPropagator(abc.ABC): @abc.abstractmethod def extract( self, - get_from_carrier: Getter[TextMapPropagatorT], + get_from_carrier: Getter, carrier: TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: diff --git a/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py b/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py index 8627b9a65cb..33385311f74 100644 --- a/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py +++ b/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py @@ -60,7 +60,7 @@ class TraceContextTextMapPropagator(textmap.TextMapPropagator): def extract( self, - get_from_carrier: textmap.Getter[textmap.TextMapPropagatorT], + get_from_carrier: textmap.Getter, carrier: textmap.TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: @@ -68,7 +68,7 @@ def extract( See `opentelemetry.trace.propagation.textmap.TextMapPropagator.extract` """ - header = get_from_carrier(carrier, self._TRACEPARENT_HEADER_NAME) + header = get_from_carrier.get(carrier, self._TRACEPARENT_HEADER_NAME) if not header: return trace.set_span_in_context(trace.INVALID_SPAN, context) @@ -91,7 +91,7 @@ def extract( if version == "ff": return trace.set_span_in_context(trace.INVALID_SPAN, context) - tracestate_headers = get_from_carrier( + tracestate_headers = get_from_carrier.get( carrier, self._TRACESTATE_HEADER_NAME ) tracestate = _parse_tracestate(tracestate_headers) diff --git a/opentelemetry-api/tests/baggage/test_baggage_propagation.py b/opentelemetry-api/tests/baggage/test_baggage_propagation.py index d5c16ead5d4..d69ce2e25f5 100644 --- a/opentelemetry-api/tests/baggage/test_baggage_propagation.py +++ b/opentelemetry-api/tests/baggage/test_baggage_propagation.py @@ -20,10 +20,17 @@ from opentelemetry.context import get_current -def get_as_list( - dict_object: typing.Dict[str, typing.List[str]], key: str -) -> typing.List[str]: - return dict_object.get(key, []) +class Getter: + @staticmethod + def get(dict_object, key): + return dict_object.get(key, []) + + @staticmethod + def keys(dict_object): + return dict_object.keys() + + +get_as_list = Getter() class TestBaggagePropagation(unittest.TestCase): diff --git a/opentelemetry-api/tests/propagators/test_global_httptextformat.py b/opentelemetry-api/tests/propagators/test_global_httptextformat.py index 9a97b281297..259ef3a5508 100644 --- a/opentelemetry-api/tests/propagators/test_global_httptextformat.py +++ b/opentelemetry-api/tests/propagators/test_global_httptextformat.py @@ -20,11 +20,18 @@ from opentelemetry.trace import get_current_span, set_span_in_context -def get_as_list( - 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 [] +class Getter: + @staticmethod + def get(dict_object, key): + value = dict_object.get(key) + return value if value is not None else [] + + @staticmethod + def keys(dict_object): + return dict_object.keys() + + +get_as_list = Getter() class TestDefaultGlobalPropagator(unittest.TestCase): diff --git a/opentelemetry-api/tests/trace/propagation/test_tracecontexthttptextformat.py b/opentelemetry-api/tests/trace/propagation/test_tracecontexthttptextformat.py index 8abe4193873..839d0567761 100644 --- a/opentelemetry-api/tests/trace/propagation/test_tracecontexthttptextformat.py +++ b/opentelemetry-api/tests/trace/propagation/test_tracecontexthttptextformat.py @@ -21,11 +21,18 @@ FORMAT = tracecontext.TraceContextTextMapPropagator() -def get_as_list( - 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 [] +class Getter: + @staticmethod + def get(dict_object, key): + value = dict_object.get(key) + return value if value is not None else [] + + @staticmethod + def keys(dict_object): + return dict_object.keys() + + +get_as_list = Getter() class TestTraceContextFormat(unittest.TestCase): diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index 71864282885..3b592ad9d3d 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -10,6 +10,8 @@ ([#1105](https://github.com/open-telemetry/opentelemetry-python/pull/1120)) - Allow for Custom Trace and Span IDs Generation - `IdsGenerator` for TracerProvider ([#1153](https://github.com/open-telemetry/opentelemetry-python/pull/1153)) +- Add keys method to TextMap propagator Getter + ([#1084](https://github.com/open-telemetry/opentelemetry-python/issues/1084)) ## Version 0.13b0 diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/propagation/b3_format.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/propagation/b3_format.py index 813b6e85600..d6c880f9d54 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/propagation/b3_format.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/propagation/b3_format.py @@ -43,7 +43,7 @@ class B3Format(TextMapPropagator): def extract( self, - get_from_carrier: Getter[TextMapPropagatorT], + get_from_carrier: Getter, carrier: TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: @@ -53,7 +53,7 @@ def extract( flags = None single_header = _extract_first_element( - get_from_carrier(carrier, self.SINGLE_HEADER_KEY) + get_from_carrier.get(carrier, self.SINGLE_HEADER_KEY) ) if single_header: # The b3 spec calls for the sampling state to be @@ -75,25 +75,25 @@ def extract( else: trace_id = ( _extract_first_element( - get_from_carrier(carrier, self.TRACE_ID_KEY) + get_from_carrier.get(carrier, self.TRACE_ID_KEY) ) or trace_id ) span_id = ( _extract_first_element( - get_from_carrier(carrier, self.SPAN_ID_KEY) + get_from_carrier.get(carrier, self.SPAN_ID_KEY) ) or span_id ) sampled = ( _extract_first_element( - get_from_carrier(carrier, self.SAMPLED_KEY) + get_from_carrier.get(carrier, self.SAMPLED_KEY) ) or sampled ) flags = ( _extract_first_element( - get_from_carrier(carrier, self.FLAGS_KEY) + get_from_carrier.get(carrier, self.FLAGS_KEY) ) or flags ) diff --git a/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py b/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py index 07b3010087a..176651aa20a 100644 --- a/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py +++ b/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py @@ -23,9 +23,18 @@ FORMAT = b3_format.B3Format() -def get_as_list(dict_object, key): - value = dict_object.get(key) - return [value] if value is not None else [] +class Getter: + @staticmethod + def get(dict_object, key): + value = dict_object.get(key) + return [value] if value is not None else [] + + @staticmethod + def keys(dict_object): + return dict_object.keys() + + +get_as_list = Getter() def get_child_parent_new_carrier(old_carrier): @@ -321,11 +330,8 @@ def test_inject_empty_context(): def test_default_span(): """Make sure propagator does not crash when working with DefaultSpan""" - def getter(carrier, key): - return carrier.get(key, None) - def setter(carrier, key, value): carrier[key] = value - ctx = FORMAT.extract(getter, {}) + ctx = FORMAT.extract(get_as_list, {}) FORMAT.inject(setter, {}, ctx) diff --git a/tests/util/src/opentelemetry/test/mock_textmap.py b/tests/util/src/opentelemetry/test/mock_textmap.py index 92c0f21f0ec..90260c86883 100644 --- a/tests/util/src/opentelemetry/test/mock_textmap.py +++ b/tests/util/src/opentelemetry/test/mock_textmap.py @@ -33,7 +33,7 @@ class NOOPTextMapPropagator(TextMapPropagator): def extract( self, - get_from_carrier: Getter[TextMapPropagatorT], + get_from_carrier: Getter, carrier: TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: @@ -56,12 +56,12 @@ class MockTextMapPropagator(TextMapPropagator): def extract( self, - get_from_carrier: Getter[TextMapPropagatorT], + get_from_carrier: Getter, carrier: TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: - trace_id_list = get_from_carrier(carrier, self.TRACE_ID_KEY) - span_id_list = get_from_carrier(carrier, self.SPAN_ID_KEY) + trace_id_list = get_from_carrier.get(carrier, self.TRACE_ID_KEY) + span_id_list = get_from_carrier.get(carrier, self.SPAN_ID_KEY) if not trace_id_list or not span_id_list: return trace.set_span_in_context(trace.INVALID_SPAN) From 16ac2742b1366a6e171a140bd766069a7955a168 Mon Sep 17 00:00:00 2001 From: Prajilesh N Date: Fri, 9 Oct 2020 01:17:26 +0530 Subject: [PATCH 02/11] updated the Getter class with default get and keys method --- .../tests/test_datadog_format.py | 25 +++------ .../instrumentation/asgi/__init__.py | 44 ++++++---------- .../instrumentation/celery/__init__.py | 21 +++----- .../instrumentation/grpc/_server.py | 21 +++----- .../opentracing_shim/__init__.py | 51 +++---------------- .../instrumentation/tornado/__init__.py | 21 +++----- .../instrumentation/wsgi/__init__.py | 40 ++++++--------- opentelemetry-api/CHANGELOG.md | 2 + .../trace/propagation/textmap.py | 24 ++++++--- .../tests/baggage/test_baggage_propagation.py | 21 +++----- .../propagators/test_global_httptextformat.py | 19 +++---- .../test_tracecontexthttptextformat.py | 33 ++++++------ opentelemetry-sdk/CHANGELOG.md | 2 +- .../tests/trace/propagation/test_b3_format.py | 28 ++++------ 14 files changed, 128 insertions(+), 224 deletions(-) diff --git a/exporter/opentelemetry-exporter-datadog/tests/test_datadog_format.py b/exporter/opentelemetry-exporter-datadog/tests/test_datadog_format.py index 6ff865ba815..91563f4aaa7 100644 --- a/exporter/opentelemetry-exporter-datadog/tests/test_datadog_format.py +++ b/exporter/opentelemetry-exporter-datadog/tests/test_datadog_format.py @@ -18,22 +18,11 @@ from opentelemetry.exporter.datadog import constants, propagator from opentelemetry.sdk import trace from opentelemetry.trace import get_current_span, set_span_in_context +from opentelemetry.trace.propagation.textmap import Getter FORMAT = propagator.DatadogFormat() - -class Getter: - @staticmethod - def get(dict_object, key): - value = dict_object.get(key) - return [value] if value is not None else [] - - @staticmethod - def keys(dict_object): - return dict_object.keys() - - -get_as_list = Getter() +getter = Getter() class TestDatadogFormat(unittest.TestCase): @@ -54,7 +43,7 @@ def test_malformed_headers(self): malformed_parent_id_key = FORMAT.PARENT_ID_KEY + "-x" context = get_current_span( FORMAT.extract( - get_as_list, + getter, { malformed_trace_id_key: self.serialized_trace_id, malformed_parent_id_key: self.serialized_parent_id, @@ -72,7 +61,7 @@ def test_missing_trace_id(self): FORMAT.PARENT_ID_KEY: self.serialized_parent_id, } - ctx = FORMAT.extract(get_as_list, carrier) + ctx = FORMAT.extract(getter, carrier) span_context = get_current_span(ctx).get_context() self.assertEqual(span_context.trace_id, trace_api.INVALID_TRACE_ID) @@ -82,7 +71,7 @@ def test_missing_parent_id(self): FORMAT.TRACE_ID_KEY: self.serialized_trace_id, } - ctx = FORMAT.extract(get_as_list, carrier) + ctx = FORMAT.extract(getter, carrier) span_context = get_current_span(ctx).get_context() self.assertEqual(span_context.span_id, trace_api.INVALID_SPAN_ID) @@ -90,7 +79,7 @@ def test_context_propagation(self): """Test the propagation of Datadog headers.""" parent_context = get_current_span( FORMAT.extract( - get_as_list, + getter, { FORMAT.TRACE_ID_KEY: self.serialized_trace_id, FORMAT.PARENT_ID_KEY: self.serialized_parent_id, @@ -147,7 +136,7 @@ def test_sampling_priority_auto_reject(self): """Test sampling priority rejected.""" parent_context = get_current_span( FORMAT.extract( - get_as_list, + getter, { FORMAT.TRACE_ID_KEY: self.serialized_trace_id, FORMAT.PARENT_ID_KEY: self.serialized_parent_id, diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index 89920a2903b..2ca33245cbb 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -29,35 +29,25 @@ from opentelemetry import context, propagators, trace from opentelemetry.instrumentation.asgi.version import __version__ # noqa from opentelemetry.instrumentation.utils import http_status_to_canonical_code +from opentelemetry.trace.propagation.textmap import Getter from opentelemetry.trace.status import Status, StatusCanonicalCode -class Getter: - @staticmethod - def get(scope: dict, header_name: str) -> typing.List[str]: - """Retrieve a HTTP header value from the ASGI scope. +def get_header_from_scope(scope: dict, header_name: str) -> typing.List[str]: + """Retrieve a HTTP header value from the ASGI scope. - Returns: - A list with a single string with the header value if it exists, else an empty list. - """ - headers = scope.get("headers") - return [ - value.decode("utf8") - for (key, value) in headers - if key.decode("utf8") == header_name - ] - - @staticmethod - def keys(scope: dict) -> typing.List[str]: - """Retrieve all the HTTP header keys for an ASGI scope.. - - Returns: - A list with all the keys in scope. - """ - return scope.keys() + Returns: + A list with a single string with the header value if it exists, else an empty list. + """ + headers = scope.get("headers") + return [ + value.decode("utf8") + for (key, value) in headers + if key.decode("utf8") == header_name + ] -get_header_from_scope = Getter() +getter = Getter(get_header_from_scope) def collect_request_attributes(scope): @@ -86,10 +76,10 @@ def collect_request_attributes(scope): http_method = scope.get("method") if http_method: result["http.method"] = http_method - http_host_value = ",".join(get_header_from_scope.get(scope, "host")) + http_host_value = ",".join(getter.get(scope, "host")) if http_host_value: result["http.server_name"] = http_host_value - http_user_agent = get_header_from_scope.get(scope, "user-agent") + http_user_agent = getter.get(scope, "user-agent") if len(http_user_agent) > 0: result["http.user_agent"] = http_user_agent[0] @@ -168,9 +158,7 @@ async def __call__(self, scope, receive, send): if scope["type"] not in ("http", "websocket"): return await self.app(scope, receive, send) - token = context.attach( - propagators.extract(get_header_from_scope, scope) - ) + token = context.attach(propagators.extract(getter, scope)) span_name, additional_attributes = self.span_details_callback(scope) try: diff --git a/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py b/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py index bb3780e4944..88a06f11030 100644 --- a/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py @@ -68,6 +68,7 @@ def add(x, y): from opentelemetry.instrumentation.celery.version import __version__ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.trace.propagation import get_current_span +from opentelemetry.trace.propagation.textmap import Getter from opentelemetry.trace.status import Status, StatusCanonicalCode logger = logging.getLogger(__name__) @@ -118,8 +119,8 @@ def _trace_prerun(self, *args, **kwargs): return request = task.request - carrier_extractor = Getter() - tracectx = propagators.extract(carrier_extractor, request) or {} + getter = Getter(carrier_extractor) + tracectx = propagators.extract(getter, request) or {} parent = get_current_span(tracectx) logger.debug("prerun signal start task_id=%s", task_id) @@ -249,14 +250,8 @@ def _trace_retry(*args, **kwargs): span.set_attribute(_TASK_RETRY_REASON_KEY, str(reason)) -class Getter: - @staticmethod - def get(carrier, key): - value = getattr(carrier, key, []) - if isinstance(value, str) or not isinstance(value, Iterable): - value = (value,) - return value - - @staticmethod - def keys(carrier): - return carrier.keys() +def carrier_extractor(carrier, key): + value = getattr(carrier, key, []) + if isinstance(value, str) or not isinstance(value, Iterable): + value = (value,) + return value diff --git a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py index 8a6b939f6f2..dae7bfeb97d 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py @@ -30,6 +30,7 @@ from opentelemetry import propagators, trace from opentelemetry.context import attach, detach +from opentelemetry.trace.propagation.textmap import Getter from . import grpcext from ._utilities import RpcInfo @@ -110,18 +111,6 @@ def _check_error_code(span, servicer_context, rpc_info): rpc_info.error = servicer_context.code -class Getter: - @staticmethod - def get(metadata, key) -> List[str]: - md_dict = {md.key: md.value for md in metadata} - return [md_dict[key]] if key in md_dict else [] - - @staticmethod - def keys(metadata) -> List[str]: - md_dict = {md.key: md.value for md in metadata} - return md_dict.keys() - - class OpenTelemetryServerInterceptor( grpcext.UnaryServerInterceptor, grpcext.StreamServerInterceptor ): @@ -133,10 +122,14 @@ def __init__(self, tracer): def _set_remote_context(self, servicer_context): metadata = servicer_context.invocation_metadata() if metadata: + md_dict = {md.key: md.value for md in metadata} + + def get_from_grpc_metadata(metadata, key) -> List[str]: + return [md_dict[key]] if key in md_dict else [] - get_from_grpc_metadata = Getter() + getter = Getter(get_from_grpc_metadata) # Update the context with the traceparent from the RPC metadata. - ctx = propagators.extract(get_from_grpc_metadata, metadata) + ctx = propagators.extract(getter, metadata) token = attach(ctx) try: yield diff --git a/instrumentation/opentelemetry-instrumentation-opentracing-shim/src/opentelemetry/instrumentation/opentracing_shim/__init__.py b/instrumentation/opentelemetry-instrumentation-opentracing-shim/src/opentelemetry/instrumentation/opentracing_shim/__init__.py index 10ab5caf809..29a9d30400a 100644 --- a/instrumentation/opentelemetry-instrumentation-opentracing-shim/src/opentelemetry/instrumentation/opentracing_shim/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-opentracing-shim/src/opentelemetry/instrumentation/opentracing_shim/__init__.py @@ -112,6 +112,7 @@ get_current_span, set_span_in_context, ) +from opentelemetry.trace.propagation.textmap import Getter from opentelemetry.util.types import Attributes ValueT = TypeVar("ValueT", int, float, bool, str) @@ -503,47 +504,6 @@ def tracer(self) -> "TracerShim": return self._tracer -class Getter: - """This class provides an interface that enables extracting propagated - fields from a carrier - - """ - - @staticmethod - def get(carrier, key): - """Function that can retrieve zero - or more values from the carrier. In the case that - the value does not exist, returns an empty list. - - Args: - carrier: and object which contains values that are - used to construct a Context. This object - must be paired with an appropriate get_from_carrier - which understands how to extract a value from it. - key: key of a field in carrier. - Returns: - first value of the propagation key or an empty list if the key doesn't exist. - """ - - value = carrier.get(key) - return [value] if value is not None else [] - - @staticmethod - def keys(carrier): - """Function that can retrieve all the keys in a carrier object. - - Args: - carrier: and object which contains values that are - used to construct a Context. This object - must be paired with an appropriate get_from_carrier - which understands how to extract a value from it. - Returns: - list of keys from the carrier. - """ - - return carrier.keys() - - class TracerShim(Tracer): """Wraps a :class:`opentelemetry.trace.Tracer` object. @@ -747,9 +707,14 @@ def extract(self, format: object, carrier: object): if format not in self._supported_formats: raise UnsupportedFormatException - get_as_list = Getter() + def get_as_list(dict_object, key): + value = dict_object.get(key) + return [value] if value is not None else [] + + getter = Getter(get_as_list) + propagator = propagators.get_global_textmap() - ctx = propagator.extract(get_as_list, carrier) + ctx = propagator.extract(getter, carrier) span = get_current_span(ctx) if span is not None: otel_context = span.get_context() diff --git a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py index ad6d2d2baf8..bc12e971b1a 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py @@ -53,6 +53,7 @@ def get(self): http_status_to_canonical_code, unwrap, ) +from opentelemetry.trace.propagation.textmap import Getter from opentelemetry.trace.status import Status from opentelemetry.util import ExcludeList, time_ns @@ -174,15 +175,11 @@ def _log_exception(tracer, func, handler, args, kwargs): return func(*args, **kwargs) -class Getter: - @staticmethod - def get(headers: dict, header_name: str) -> typing.List[str]: - header = headers.get(header_name) - return [header] if header else [] - - @staticmethod - def keys(headers) -> typing.List[str]: - return headers.keys() +def _get_header_from_request_headers( + headers: dict, header_name: str +) -> typing.List[str]: + header = headers.get(header_name) + return [header] if header else [] def _get_attributes_from_request(request): @@ -210,11 +207,9 @@ def _get_operation_name(handler, request): def _start_span(tracer, handler, start_time) -> _TraceContext: - _get_header_from_request_headers = Getter() + getter = Getter(_get_header_from_request_headers) token = context.attach( - propagators.extract( - _get_header_from_request_headers, handler.request.headers, - ) + propagators.extract(getter, handler.request.headers,) ) span = tracer.start_span( _get_operation_name(handler, handler.request), diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index 9b0e54ad372..d03c9e9f076 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -61,36 +61,28 @@ def hello(): from opentelemetry import context, propagators, trace from opentelemetry.instrumentation.utils import http_status_to_canonical_code from opentelemetry.instrumentation.wsgi.version import __version__ +from opentelemetry.trace.propagation.textmap import Getter from opentelemetry.trace.status import Status, StatusCanonicalCode _HTTP_VERSION_PREFIX = "HTTP/" -class Getter: - @staticmethod - def get(environ: dict, header_name: str) -> typing.List[str]: - """Retrieve a HTTP header value from the PEP3333-conforming WSGI environ. - - Returns: - A list with a single string with the header value if it exists, else an empty list. - """ - environ_key = "HTTP_" + header_name.upper().replace("-", "_") - value = environ.get(environ_key) - if value is not None: - return [value] - return [] - - @staticmethod - def keys(environ: dict) -> typing.List[str]: - """Retrieve all the HTTP header keys for an PEP3333-conforming WSGI environ. +def get_header_from_environ( + environ: dict, header_name: str +) -> typing.List[str]: + """Retrieve a HTTP header value from the PEP3333-conforming WSGI environ. - Returns: - A list with all the keys in environ. - """ - return environ.keys() + Returns: + A list with a single string with the header value if it exists, else an empty list. + """ + environ_key = "HTTP_" + header_name.upper().replace("-", "_") + value = environ.get(environ_key) + if value is not None: + return [value] + return [] -get_header_from_environ = Getter() +getter = Getter(get_header_from_environ) def setifnotnone(dic, key, value): @@ -207,9 +199,7 @@ def __call__(self, environ, start_response): start_response: The WSGI start_response callable. """ - token = context.attach( - propagators.extract(get_header_from_environ, environ) - ) + token = context.attach(propagators.extract(getter, environ)) span_name = self.name_callback(environ) span = self.tracer.start_span( diff --git a/opentelemetry-api/CHANGELOG.md b/opentelemetry-api/CHANGELOG.md index 434bae64042..0638d4b2710 100644 --- a/opentelemetry-api/CHANGELOG.md +++ b/opentelemetry-api/CHANGELOG.md @@ -10,6 +10,8 @@ ([#1153](https://github.com/open-telemetry/opentelemetry-python/pull/1153)) - Update baggage propagation header ([#1194](https://github.com/open-telemetry/opentelemetry-python/pull/1194)) +- Add keys method to TextMap propagator Getter + ([#1196](https://github.com/open-telemetry/opentelemetry-python/issues/1196)) ## Version 0.13b0 diff --git a/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py b/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py index af9b6689651..5b3b314195d 100644 --- a/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py +++ b/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py @@ -22,15 +22,23 @@ Setter = typing.Callable[[TextMapPropagatorT, str, str], None] -class Getter(abc.ABC): - """This class provides an interface that enables extracting propagated +class Getter: + """This class implements a Getter that enables extracting propagated fields from a carrier """ - @staticmethod - @abc.abstractmethod - def get(carrier: TextMapPropagatorT, key: str) -> typing.List[str]: + def __init__( + self, + get=lambda carrier, key: [carrier.get(key)] + if carrier.get(key) + else [], + keys=lambda carrier: list(carrier.keys()), + ): + self._get = get + self._keys = keys + + def get(self, carrier: TextMapPropagatorT, key: str) -> typing.List[str]: """Function that can retrieve zero or more values from the carrier. In the case that the value does not exist, returns an empty list. @@ -44,10 +52,9 @@ def get(carrier: TextMapPropagatorT, key: str) -> typing.List[str]: Returns: first value of the propagation key or an empty list if the key doesn't exist. """ + return self._get(carrier, key) - @staticmethod - @abc.abstractmethod - def keys(carrier: TextMapPropagatorT) -> typing.List[str]: + def keys(self, carrier: TextMapPropagatorT) -> typing.List[str]: """Function that can retrieve all the keys in a carrier object. Args: @@ -58,6 +65,7 @@ def keys(carrier: TextMapPropagatorT) -> typing.List[str]: Returns: list of keys from the carrier. """ + return self._keys(carrier) class TextMapPropagator(abc.ABC): diff --git a/opentelemetry-api/tests/baggage/test_baggage_propagation.py b/opentelemetry-api/tests/baggage/test_baggage_propagation.py index d69ce2e25f5..3fa221ecfef 100644 --- a/opentelemetry-api/tests/baggage/test_baggage_propagation.py +++ b/opentelemetry-api/tests/baggage/test_baggage_propagation.py @@ -18,19 +18,16 @@ from opentelemetry import baggage from opentelemetry.baggage.propagation import BaggagePropagator from opentelemetry.context import get_current +from opentelemetry.trace.propagation.textmap import Getter -class Getter: - @staticmethod - def get(dict_object, key): - return dict_object.get(key, []) +def get_as_list( + dict_object: typing.Dict[str, typing.List[str]], key: str +) -> typing.List[str]: + return dict_object.get(key, []) - @staticmethod - def keys(dict_object): - return dict_object.keys() - -get_as_list = Getter() +getter = Getter(get_as_list) class TestBaggagePropagation(unittest.TestCase): @@ -40,7 +37,7 @@ def setUp(self): def _extract(self, header_value): """Test helper""" header = {"baggage": [header_value]} - return baggage.get_all(self.propagator.extract(get_as_list, header)) + return baggage.get_all(self.propagator.extract(getter, header)) def _inject(self, values): """Test helper""" @@ -52,9 +49,7 @@ def _inject(self, values): return output.get("baggage") def test_no_context_header(self): - baggage_entries = baggage.get_all( - self.propagator.extract(get_as_list, {}) - ) + baggage_entries = baggage.get_all(self.propagator.extract(getter, {})) self.assertEqual(baggage_entries, {}) def test_empty_context_header(self): diff --git a/opentelemetry-api/tests/propagators/test_global_httptextformat.py b/opentelemetry-api/tests/propagators/test_global_httptextformat.py index 259ef3a5508..bb77582f1c9 100644 --- a/opentelemetry-api/tests/propagators/test_global_httptextformat.py +++ b/opentelemetry-api/tests/propagators/test_global_httptextformat.py @@ -18,20 +18,17 @@ from opentelemetry import baggage, trace from opentelemetry.propagators import extract, inject from opentelemetry.trace import get_current_span, set_span_in_context +from opentelemetry.trace.propagation.textmap import Getter -class Getter: - @staticmethod - def get(dict_object, key): - value = dict_object.get(key) - return value if value is not None else [] +def get_as_list( + 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 [] - @staticmethod - def keys(dict_object): - return dict_object.keys() - -get_as_list = Getter() +getter = Getter(get_as_list) class TestDefaultGlobalPropagator(unittest.TestCase): @@ -51,7 +48,7 @@ def test_propagation(self): "traceparent": [traceparent_value], "tracestate": [tracestate_value], } - ctx = extract(get_as_list, headers) + ctx = extract(getter, headers) baggage_entries = baggage.get_all(context=ctx) expected = {"key1": "val1", "key2": "val2"} self.assertEqual(baggage_entries, expected) diff --git a/opentelemetry-api/tests/trace/propagation/test_tracecontexthttptextformat.py b/opentelemetry-api/tests/trace/propagation/test_tracecontexthttptextformat.py index 839d0567761..fbe65195f6d 100644 --- a/opentelemetry-api/tests/trace/propagation/test_tracecontexthttptextformat.py +++ b/opentelemetry-api/tests/trace/propagation/test_tracecontexthttptextformat.py @@ -17,22 +17,19 @@ from opentelemetry import trace from opentelemetry.trace.propagation import tracecontext +from opentelemetry.trace.propagation.textmap import Getter FORMAT = tracecontext.TraceContextTextMapPropagator() -class Getter: - @staticmethod - def get(dict_object, key): - value = dict_object.get(key) - return value if value is not None else [] +def get_as_list( + 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 [] - @staticmethod - def keys(dict_object): - return dict_object.keys() - -get_as_list = Getter() +getter = Getter(get_as_list) class TestTraceContextFormat(unittest.TestCase): @@ -49,7 +46,7 @@ def test_no_traceparent_header(self): trace-id and parent-id that represents the current request. """ output = {} # type:typing.Dict[str, typing.List[str]] - span = trace.get_current_span(FORMAT.extract(get_as_list, output)) + span = trace.get_current_span(FORMAT.extract(getter, output)) self.assertIsInstance(span.get_context(), trace.SpanContext) def test_headers_with_tracestate(self): @@ -63,7 +60,7 @@ def test_headers_with_tracestate(self): tracestate_value = "foo=1,bar=2,baz=3" span_context = trace.get_current_span( FORMAT.extract( - get_as_list, + getter, { "traceparent": [traceparent_value], "tracestate": [tracestate_value], @@ -107,7 +104,7 @@ def test_invalid_trace_id(self): """ span = trace.get_current_span( FORMAT.extract( - get_as_list, + getter, { "traceparent": [ "00-00000000000000000000000000000000-1234567890123456-00" @@ -138,7 +135,7 @@ def test_invalid_parent_id(self): """ span = trace.get_current_span( FORMAT.extract( - get_as_list, + getter, { "traceparent": [ "00-00000000000000000000000000000000-0000000000000000-00" @@ -176,7 +173,7 @@ def test_format_not_supported(self): """ span = trace.get_current_span( FORMAT.extract( - get_as_list, + getter, { "traceparent": [ "00-12345678901234567890123456789012-" @@ -200,7 +197,7 @@ def test_tracestate_empty_header(self): """ span = trace.get_current_span( FORMAT.extract( - get_as_list, + getter, { "traceparent": [ "00-12345678901234567890123456789012-1234567890123456-00" @@ -216,7 +213,7 @@ def test_tracestate_header_with_trailing_comma(self): """ span = trace.get_current_span( FORMAT.extract( - get_as_list, + getter, { "traceparent": [ "00-12345678901234567890123456789012-1234567890123456-00" @@ -240,7 +237,7 @@ def test_tracestate_keys(self): ) span = trace.get_current_span( FORMAT.extract( - get_as_list, + getter, { "traceparent": [ "00-12345678901234567890123456789012-1234567890123456-00" diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index 3b592ad9d3d..672b28c6155 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -11,7 +11,7 @@ - Allow for Custom Trace and Span IDs Generation - `IdsGenerator` for TracerProvider ([#1153](https://github.com/open-telemetry/opentelemetry-python/pull/1153)) - Add keys method to TextMap propagator Getter - ([#1084](https://github.com/open-telemetry/opentelemetry-python/issues/1084)) + ([#1196](https://github.com/open-telemetry/opentelemetry-python/issues/1196)) ## Version 0.13b0 diff --git a/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py b/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py index 176651aa20a..752c0ca0fae 100644 --- a/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py +++ b/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py @@ -19,27 +19,17 @@ import opentelemetry.sdk.trace.propagation.b3_format as b3_format import opentelemetry.trace as trace_api from opentelemetry.context import get_current +from opentelemetry.trace.propagation.textmap import Getter FORMAT = b3_format.B3Format() -class Getter: - @staticmethod - def get(dict_object, key): - value = dict_object.get(key) - return [value] if value is not None else [] - - @staticmethod - def keys(dict_object): - return dict_object.keys() - - -get_as_list = Getter() +getter = Getter() def get_child_parent_new_carrier(old_carrier): - ctx = FORMAT.extract(get_as_list, old_carrier) + ctx = FORMAT.extract(getter, old_carrier) parent_context = trace_api.get_current_span(ctx).get_context() parent = trace.Span("parent", parent_context) @@ -240,7 +230,7 @@ def test_invalid_single_header(self): invalid SpanContext. """ carrier = {FORMAT.SINGLE_HEADER_KEY: "0-1-2-3-4-5-6-7"} - ctx = FORMAT.extract(get_as_list, carrier) + ctx = FORMAT.extract(getter, carrier) span_context = trace_api.get_current_span(ctx).get_context() self.assertEqual(span_context.trace_id, trace_api.INVALID_TRACE_ID) self.assertEqual(span_context.span_id, trace_api.INVALID_SPAN_ID) @@ -252,7 +242,7 @@ def test_missing_trace_id(self): FORMAT.FLAGS_KEY: "1", } - ctx = FORMAT.extract(get_as_list, carrier) + ctx = FORMAT.extract(getter, carrier) span_context = trace_api.get_current_span(ctx).get_context() self.assertEqual(span_context.trace_id, trace_api.INVALID_TRACE_ID) @@ -276,7 +266,7 @@ def test_invalid_trace_id( FORMAT.FLAGS_KEY: "1", } - ctx = FORMAT.extract(get_as_list, carrier) + ctx = FORMAT.extract(getter, carrier) span_context = trace_api.get_current_span(ctx).get_context() self.assertEqual(span_context.trace_id, 1) @@ -302,7 +292,7 @@ def test_invalid_span_id( FORMAT.FLAGS_KEY: "1", } - ctx = FORMAT.extract(get_as_list, carrier) + ctx = FORMAT.extract(getter, carrier) span_context = trace_api.get_current_span(ctx).get_context() self.assertEqual(span_context.trace_id, 1) @@ -315,7 +305,7 @@ def test_missing_span_id(self): FORMAT.FLAGS_KEY: "1", } - ctx = FORMAT.extract(get_as_list, carrier) + ctx = FORMAT.extract(getter, carrier) span_context = trace_api.get_current_span(ctx).get_context() self.assertEqual(span_context.span_id, trace_api.INVALID_SPAN_ID) @@ -333,5 +323,5 @@ def test_default_span(): def setter(carrier, key, value): carrier[key] = value - ctx = FORMAT.extract(get_as_list, {}) + ctx = FORMAT.extract(getter, {}) FORMAT.inject(setter, {}, ctx) From 1e417a8a2da7a2d9b21ec471a9839d0b4d52c560 Mon Sep 17 00:00:00 2001 From: Prajilesh N Date: Fri, 9 Oct 2020 01:42:46 +0530 Subject: [PATCH 03/11] changing the getter function with getter objects in instrumentation --- .../instrumentation/django/middleware.py | 4 ++-- .../instrumentation/falcon/__init__.py | 4 +--- .../instrumentation/flask/__init__.py | 4 +--- .../instrumentation/pyramid/callbacks.py | 4 +--- .../trace/propagation/textmap.py | 20 +++++++++++++++---- 5 files changed, 21 insertions(+), 15 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py index 11991413eb8..7b6249fa465 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py @@ -21,7 +21,7 @@ from opentelemetry.instrumentation.wsgi import ( add_response_attributes, collect_request_attributes, - get_header_from_environ, + getter, ) from opentelemetry.propagators import extract from opentelemetry.trace import SpanKind, get_tracer @@ -98,7 +98,7 @@ def process_request(self, request): environ = request.META - token = attach(extract(get_header_from_environ, environ)) + token = attach(extract(getter, environ)) tracer = get_tracer(__name__, __version__) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index bfcd45a8b58..3a1323680b9 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -114,9 +114,7 @@ def __call__(self, env, start_response): start_time = time_ns() - token = context.attach( - propagators.extract(otel_wsgi.get_header_from_environ, env) - ) + token = context.attach(propagators.extract(otel_wsgi.getter, env)) attributes = otel_wsgi.collect_request_attributes(env) span = self._tracer.start_span( otel_wsgi.get_default_span_name(env), diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index 90082dd850e..76457875304 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -113,9 +113,7 @@ def _before_request(): span_name = flask.request.endpoint or otel_wsgi.get_default_span_name( environ ) - token = context.attach( - propagators.extract(otel_wsgi.get_header_from_environ, environ) - ) + token = context.attach(propagators.extract(otel_wsgi.getter, environ)) tracer = trace.get_tracer(__name__, __version__) diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py index ada239b8e31..a854c55e2e1 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py @@ -69,9 +69,7 @@ def _before_traversal(event): start_time = environ.get(_ENVIRON_STARTTIME_KEY) - token = context.attach( - propagators.extract(otel_wsgi.get_header_from_environ, environ) - ) + token = context.attach(propagators.extract(otel_wsgi.getter, environ)) tracer = trace.get_tracer(__name__, __version__) if request.matched_route: diff --git a/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py b/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py index 5b3b314195d..60ca5971190 100644 --- a/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py +++ b/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py @@ -22,6 +22,20 @@ Setter = typing.Callable[[TextMapPropagatorT, str, str], None] +def default_get(carrier: TextMapPropagatorT, key: str) -> typing.List[str]: + return [carrier.get(key)] if carrier.get(key) else [] + + +def default_keys(carrier: TextMapPropagatorT) -> typing.List[str]: + return list(carrier.keys()) + + +GetterGetFunction = typing.Callable[ + [TextMapPropagatorT, str], typing.List[str] +] +GetterKeysFunction = typing.Callable[[TextMapPropagatorT], typing.List[str]] + + class Getter: """This class implements a Getter that enables extracting propagated fields from a carrier @@ -30,10 +44,8 @@ class Getter: def __init__( self, - get=lambda carrier, key: [carrier.get(key)] - if carrier.get(key) - else [], - keys=lambda carrier: list(carrier.keys()), + get: GetterGetFunction[TextMapPropagatorT] = default_get, + keys: GetterKeysFunction[TextMapPropagatorT] = default_keys, ): self._get = get self._keys = keys From cbbc5ef3a7bb3fe7b4cc72a47094d6d3a14f339f Mon Sep 17 00:00:00 2001 From: Prajilesh N Date: Mon, 19 Oct 2020 00:51:26 +0530 Subject: [PATCH 04/11] updating type names --- .../src/opentelemetry/trace/propagation/textmap.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py b/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py index 60ca5971190..b04210e7943 100644 --- a/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py +++ b/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py @@ -21,6 +21,9 @@ Setter = typing.Callable[[TextMapPropagatorT, str, str], None] +GetterGet = typing.Callable[[TextMapPropagatorT, str], typing.List[str]] +GetterKeys = typing.Callable[[TextMapPropagatorT], typing.List[str]] + def default_get(carrier: TextMapPropagatorT, key: str) -> typing.List[str]: return [carrier.get(key)] if carrier.get(key) else [] @@ -30,12 +33,6 @@ def default_keys(carrier: TextMapPropagatorT) -> typing.List[str]: return list(carrier.keys()) -GetterGetFunction = typing.Callable[ - [TextMapPropagatorT, str], typing.List[str] -] -GetterKeysFunction = typing.Callable[[TextMapPropagatorT], typing.List[str]] - - class Getter: """This class implements a Getter that enables extracting propagated fields from a carrier @@ -44,8 +41,8 @@ class Getter: def __init__( self, - get: GetterGetFunction[TextMapPropagatorT] = default_get, - keys: GetterKeysFunction[TextMapPropagatorT] = default_keys, + get: GetterGet[TextMapPropagatorT] = default_get, + keys: GetterKeys[TextMapPropagatorT] = default_keys, ): self._get = get self._keys = keys From a79753343a0ef1ed8eea60f33fb781dce82e7793 Mon Sep 17 00:00:00 2001 From: Prajilesh N Date: Fri, 23 Oct 2020 16:42:16 +0530 Subject: [PATCH 05/11] using DictGetter and HelperGetter for Getter Implementation of textmap --- .../instrumentation/asgi/__init__.py | 4 +- .../instrumentation/celery/__init__.py | 4 +- .../instrumentation/grpc/_server.py | 4 +- .../opentracing_shim/__init__.py | 4 +- .../instrumentation/tornado/__init__.py | 4 +- .../instrumentation/wsgi/__init__.py | 4 +- .../baggage/propagation/__init__.py | 2 +- .../src/opentelemetry/propagators/__init__.py | 2 +- .../opentelemetry/propagators/composite.py | 2 +- .../trace/propagation/textmap.py | 67 ++++++++++--------- .../trace/propagation/tracecontext.py | 5 +- .../tests/baggage/test_baggage_propagation.py | 4 +- .../propagators/test_global_httptextformat.py | 4 +- .../test_tracecontexthttptextformat.py | 4 +- .../sdk/trace/propagation/b3_format.py | 2 +- .../tests/trace/propagation/test_b3_format.py | 8 ++- .../src/opentelemetry/test/mock_textmap.py | 4 +- 17 files changed, 68 insertions(+), 60 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index 2ca33245cbb..081d7048d6f 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -29,7 +29,7 @@ from opentelemetry import context, propagators, trace from opentelemetry.instrumentation.asgi.version import __version__ # noqa from opentelemetry.instrumentation.utils import http_status_to_canonical_code -from opentelemetry.trace.propagation.textmap import Getter +from opentelemetry.trace.propagation.textmap import DictGetter, HelperGetter from opentelemetry.trace.status import Status, StatusCanonicalCode @@ -47,7 +47,7 @@ def get_header_from_scope(scope: dict, header_name: str) -> typing.List[str]: ] -getter = Getter(get_header_from_scope) +getter = HelperGetter(get_header_from_scope, DictGetter.keys) def collect_request_attributes(scope): diff --git a/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py b/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py index 905b816ac6e..4b71a9e0a37 100644 --- a/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py @@ -68,7 +68,7 @@ def add(x, y): from opentelemetry.instrumentation.celery.version import __version__ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.trace.propagation import get_current_span -from opentelemetry.trace.propagation.textmap import Getter +from opentelemetry.trace.propagation.textmap import DictGetter, HelperGetter from opentelemetry.trace.status import Status, StatusCanonicalCode logger = logging.getLogger(__name__) @@ -119,7 +119,7 @@ def _trace_prerun(self, *args, **kwargs): return request = task.request - getter = Getter(carrier_extractor) + getter = HelperGetter(carrier_extractor, DictGetter.keys) tracectx = propagators.extract(getter, request) or None logger.debug("prerun signal start task_id=%s", task_id) diff --git a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py index dae7bfeb97d..dc8b20ea2ed 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py @@ -30,7 +30,7 @@ from opentelemetry import propagators, trace from opentelemetry.context import attach, detach -from opentelemetry.trace.propagation.textmap import Getter +from opentelemetry.trace.propagation.textmap import DictGetter, HelperGetter from . import grpcext from ._utilities import RpcInfo @@ -127,7 +127,7 @@ def _set_remote_context(self, servicer_context): def get_from_grpc_metadata(metadata, key) -> List[str]: return [md_dict[key]] if key in md_dict else [] - getter = Getter(get_from_grpc_metadata) + getter = HelperGetter(get_from_grpc_metadata, DictGetter.keys) # Update the context with the traceparent from the RPC metadata. ctx = propagators.extract(getter, metadata) token = attach(ctx) diff --git a/instrumentation/opentelemetry-instrumentation-opentracing-shim/src/opentelemetry/instrumentation/opentracing_shim/__init__.py b/instrumentation/opentelemetry-instrumentation-opentracing-shim/src/opentelemetry/instrumentation/opentracing_shim/__init__.py index 6f6e86b6d6e..8eb1b8cbb1d 100644 --- a/instrumentation/opentelemetry-instrumentation-opentracing-shim/src/opentelemetry/instrumentation/opentracing_shim/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-opentracing-shim/src/opentelemetry/instrumentation/opentracing_shim/__init__.py @@ -112,7 +112,7 @@ get_current_span, set_span_in_context, ) -from opentelemetry.trace.propagation.textmap import Getter +from opentelemetry.trace.propagation.textmap import DictGetter, HelperGetter from opentelemetry.util.types import Attributes ValueT = TypeVar("ValueT", int, float, bool, str) @@ -715,7 +715,7 @@ def get_as_list(dict_object, key): value = dict_object.get(key) return [value] if value is not None else [] - getter = Getter(get_as_list) + getter = HelperGetter(get_as_list, DictGetter.keys) propagator = propagators.get_global_textmap() ctx = propagator.extract(getter, carrier) diff --git a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py index 9bc6747fc31..0204ff43508 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py @@ -54,7 +54,7 @@ def get(self): http_status_to_canonical_code, unwrap, ) -from opentelemetry.trace.propagation.textmap import Getter +from opentelemetry.trace.propagation.textmap import DictGetter, HelperGetter from opentelemetry.trace.status import Status from opentelemetry.util import ExcludeList, time_ns @@ -218,7 +218,7 @@ def _get_operation_name(handler, request): def _start_span(tracer, handler, start_time) -> _TraceContext: - getter = Getter(_get_header_from_request_headers) + getter = HelperGetter(_get_header_from_request_headers, DictGetter.keys) token = context.attach( propagators.extract(getter, handler.request.headers,) ) diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index d03c9e9f076..69737e7245c 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -61,7 +61,7 @@ def hello(): from opentelemetry import context, propagators, trace from opentelemetry.instrumentation.utils import http_status_to_canonical_code from opentelemetry.instrumentation.wsgi.version import __version__ -from opentelemetry.trace.propagation.textmap import Getter +from opentelemetry.trace.propagation.textmap import DictGetter, HelperGetter from opentelemetry.trace.status import Status, StatusCanonicalCode _HTTP_VERSION_PREFIX = "HTTP/" @@ -82,7 +82,7 @@ def get_header_from_environ( return [] -getter = Getter(get_header_from_environ) +getter = HelperGetter(get_header_from_environ, DictGetter.keys) def setifnotnone(dic, key, value): diff --git a/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py b/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py index 7d589cb0c64..bde29282541 100644 --- a/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py +++ b/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py @@ -29,7 +29,7 @@ class BaggagePropagator(textmap.TextMapPropagator): def extract( self, - get_from_carrier: textmap.Getter, + get_from_carrier: textmap.Getter[textmap.TextMapPropagatorT], carrier: textmap.TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: diff --git a/opentelemetry-api/src/opentelemetry/propagators/__init__.py b/opentelemetry-api/src/opentelemetry/propagators/__init__.py index c325caba7eb..946aefcfaad 100644 --- a/opentelemetry-api/src/opentelemetry/propagators/__init__.py +++ b/opentelemetry-api/src/opentelemetry/propagators/__init__.py @@ -82,7 +82,7 @@ def example_route(): def extract( - get_from_carrier: textmap.Getter, + get_from_carrier: textmap.Getter[textmap.TextMapPropagatorT], carrier: textmap.TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: diff --git a/opentelemetry-api/src/opentelemetry/propagators/composite.py b/opentelemetry-api/src/opentelemetry/propagators/composite.py index c307479021e..3499d2ea08a 100644 --- a/opentelemetry-api/src/opentelemetry/propagators/composite.py +++ b/opentelemetry-api/src/opentelemetry/propagators/composite.py @@ -35,7 +35,7 @@ def __init__( def extract( self, - get_from_carrier: textmap.Getter, + get_from_carrier: textmap.Getter[textmap.TextMapPropagatorT], carrier: textmap.TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: diff --git a/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py b/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py index b04210e7943..182435ec073 100644 --- a/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py +++ b/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py @@ -21,47 +21,25 @@ Setter = typing.Callable[[TextMapPropagatorT, str, str], None] -GetterGet = typing.Callable[[TextMapPropagatorT, str], typing.List[str]] -GetterKeys = typing.Callable[[TextMapPropagatorT], typing.List[str]] - -def default_get(carrier: TextMapPropagatorT, key: str) -> typing.List[str]: - return [carrier.get(key)] if carrier.get(key) else [] - - -def default_keys(carrier: TextMapPropagatorT) -> typing.List[str]: - return list(carrier.keys()) - - -class Getter: +class Getter(typing.Generic[TextMapPropagatorT]): """This class implements a Getter that enables extracting propagated fields from a carrier """ - def __init__( - self, - get: GetterGet[TextMapPropagatorT] = default_get, - keys: GetterKeys[TextMapPropagatorT] = default_keys, - ): - self._get = get - self._keys = keys - def get(self, carrier: TextMapPropagatorT, key: str) -> typing.List[str]: """Function that can retrieve zero or more values from the carrier. In the case that the value does not exist, returns an empty list. - Args: - carrier: and object which contains values that are - used to construct a Context. This object - must be paired with an appropriate get_from_carrier - which understands how to extract a value from it. - key: key of a field in carrier. - Returns: - first value of the propagation key or an empty list if the key doesn't exist. + Args: carrier: and object which contains values that are used to + construct a Context. This object must be paired with an appropriate + get_from_carrier which understands how to extract a value from it. + key: key of a field in carrier. Returns: first value of the + propagation key or an empty list if the key doesn't exist. """ - return self._get(carrier, key) + raise NotImplementedError() def keys(self, carrier: TextMapPropagatorT) -> typing.List[str]: """Function that can retrieve all the keys in a carrier object. @@ -74,6 +52,35 @@ def keys(self, carrier: TextMapPropagatorT) -> typing.List[str]: Returns: list of keys from the carrier. """ + raise NotImplementedError() + + +class DictGetter(Getter[typing.Dict[str, str]]): + def get( + self, carrier: typing.Dict[str, str], key: str + ) -> typing.List[str]: + val = carrier.get(key, None) + if val: + return [val] + return [] + + def keys(self, carrier: typing.Dict[str, str]) -> typing.List[str]: + return list(carrier.keys()) + + +class HelperGetter(Getter[TextMapPropagatorT]): + def __init__( + self, + get: typing.Callable[[TextMapPropagatorT, str], typing.List[str]], + keys: typing.Callable[[TextMapPropagatorT], typing.List[str]], + ): + self._get = get + self._keys = keys + + def get(self, carrier: TextMapPropagatorT, key: str) -> typing.List[str]: + return self._get(carrier, key) + + def keys(self, carrier: TextMapPropagatorT) -> typing.List[str]: return self._keys(carrier) @@ -89,7 +96,7 @@ class TextMapPropagator(abc.ABC): @abc.abstractmethod def extract( self, - get_from_carrier: Getter, + get_from_carrier: Getter[TextMapPropagatorT], carrier: TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: diff --git a/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py b/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py index 33609b80a12..87e4162b1bd 100644 --- a/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py +++ b/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py @@ -18,14 +18,13 @@ import opentelemetry.trace as trace from opentelemetry.context.context import Context from opentelemetry.trace.propagation import textmap - # Keys and values are strings of up to 256 printable US-ASCII characters. # Implementations should conform to 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 - +from opentelemetry.trace.propagation.textmap import TextMapPropagatorT _KEY_WITHOUT_VENDOR_FORMAT = r"[a-z][_0-9a-z\-\*\/]{0,255}" _KEY_WITH_VENDOR_FORMAT = ( @@ -60,7 +59,7 @@ class TraceContextTextMapPropagator(textmap.TextMapPropagator): def extract( self, - get_from_carrier: textmap.Getter, + get_from_carrier: textmap.Getter[textmap.TextMapPropagatorT], carrier: textmap.TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: diff --git a/opentelemetry-api/tests/baggage/test_baggage_propagation.py b/opentelemetry-api/tests/baggage/test_baggage_propagation.py index 3fa221ecfef..7562d59c2a5 100644 --- a/opentelemetry-api/tests/baggage/test_baggage_propagation.py +++ b/opentelemetry-api/tests/baggage/test_baggage_propagation.py @@ -18,7 +18,7 @@ from opentelemetry import baggage from opentelemetry.baggage.propagation import BaggagePropagator from opentelemetry.context import get_current -from opentelemetry.trace.propagation.textmap import Getter +from opentelemetry.trace.propagation.textmap import DictGetter, HelperGetter def get_as_list( @@ -27,7 +27,7 @@ def get_as_list( return dict_object.get(key, []) -getter = Getter(get_as_list) +getter = HelperGetter(get_as_list, DictGetter.keys) class TestBaggagePropagation(unittest.TestCase): diff --git a/opentelemetry-api/tests/propagators/test_global_httptextformat.py b/opentelemetry-api/tests/propagators/test_global_httptextformat.py index aec0a8d8e5e..2ce11f3c6b0 100644 --- a/opentelemetry-api/tests/propagators/test_global_httptextformat.py +++ b/opentelemetry-api/tests/propagators/test_global_httptextformat.py @@ -18,7 +18,7 @@ from opentelemetry import baggage, trace from opentelemetry.propagators import extract, inject from opentelemetry.trace import get_current_span, set_span_in_context -from opentelemetry.trace.propagation.textmap import Getter +from opentelemetry.trace.propagation.textmap import DictGetter, HelperGetter def get_as_list( @@ -28,7 +28,7 @@ def get_as_list( return value if value is not None else [] -getter = Getter(get_as_list) +getter = HelperGetter(get_as_list, DictGetter.keys) class TestDefaultGlobalPropagator(unittest.TestCase): diff --git a/opentelemetry-api/tests/trace/propagation/test_tracecontexthttptextformat.py b/opentelemetry-api/tests/trace/propagation/test_tracecontexthttptextformat.py index f947b438878..006afd1d3e9 100644 --- a/opentelemetry-api/tests/trace/propagation/test_tracecontexthttptextformat.py +++ b/opentelemetry-api/tests/trace/propagation/test_tracecontexthttptextformat.py @@ -17,7 +17,7 @@ from opentelemetry import trace from opentelemetry.trace.propagation import tracecontext -from opentelemetry.trace.propagation.textmap import Getter +from opentelemetry.trace.propagation.textmap import DictGetter, HelperGetter FORMAT = tracecontext.TraceContextTextMapPropagator() @@ -29,7 +29,7 @@ def get_as_list( return value if value is not None else [] -getter = Getter(get_as_list) +getter = HelperGetter(get_as_list, DictGetter.keys) class TestTraceContextFormat(unittest.TestCase): diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/propagation/b3_format.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/propagation/b3_format.py index d676017d1f9..0a9d9caba19 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/propagation/b3_format.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/propagation/b3_format.py @@ -43,7 +43,7 @@ class B3Format(TextMapPropagator): def extract( self, - get_from_carrier: Getter, + get_from_carrier: Getter[TextMapPropagatorT], carrier: TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: diff --git a/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py b/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py index 917f6d4665f..e89d7cf9c91 100644 --- a/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py +++ b/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py @@ -19,12 +19,12 @@ import opentelemetry.sdk.trace.propagation.b3_format as b3_format import opentelemetry.trace as trace_api from opentelemetry.context import get_current -from opentelemetry.trace.propagation.textmap import Getter +from opentelemetry.trace.propagation.textmap import DictGetter, HelperGetter FORMAT = b3_format.B3Format() -getter = Getter() +getter = DictGetter() def get_child_parent_new_carrier(old_carrier): @@ -326,5 +326,7 @@ def default_span_getter(carrier, key): def setter(carrier, key, value): carrier[key] = value - ctx = FORMAT.extract(Getter(default_span_getter), {}) + ctx = FORMAT.extract( + HelperGetter(default_span_getter, DictGetter.keys), {} + ) FORMAT.inject(setter, {}, ctx) diff --git a/tests/util/src/opentelemetry/test/mock_textmap.py b/tests/util/src/opentelemetry/test/mock_textmap.py index e9459ff71dd..83498123ded 100644 --- a/tests/util/src/opentelemetry/test/mock_textmap.py +++ b/tests/util/src/opentelemetry/test/mock_textmap.py @@ -33,7 +33,7 @@ class NOOPTextMapPropagator(TextMapPropagator): def extract( self, - get_from_carrier: Getter, + get_from_carrier: Getter[TextMapPropagatorT], carrier: TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: @@ -56,7 +56,7 @@ class MockTextMapPropagator(TextMapPropagator): def extract( self, - get_from_carrier: Getter, + get_from_carrier: Getter[TextMapPropagatorT], carrier: TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: From 2d18ab90f130e51a931e95b33abbc2b8da991405 Mon Sep 17 00:00:00 2001 From: Prajilesh N Date: Fri, 23 Oct 2020 16:52:38 +0530 Subject: [PATCH 06/11] updated tests + fixed linting issues --- .../src/opentelemetry/exporter/datadog/propagator.py | 2 +- .../tests/test_datadog_format.py | 4 ++-- .../src/opentelemetry/instrumentation/flask/__init__.py | 4 +--- .../src/opentelemetry/trace/propagation/tracecontext.py | 2 +- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/exporter/opentelemetry-exporter-datadog/src/opentelemetry/exporter/datadog/propagator.py b/exporter/opentelemetry-exporter-datadog/src/opentelemetry/exporter/datadog/propagator.py index 52ff6d56307..afc61bba401 100644 --- a/exporter/opentelemetry-exporter-datadog/src/opentelemetry/exporter/datadog/propagator.py +++ b/exporter/opentelemetry-exporter-datadog/src/opentelemetry/exporter/datadog/propagator.py @@ -39,7 +39,7 @@ class DatadogFormat(TextMapPropagator): def extract( self, - get_from_carrier: Getter, + get_from_carrier: Getter[TextMapPropagatorT], carrier: TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: diff --git a/exporter/opentelemetry-exporter-datadog/tests/test_datadog_format.py b/exporter/opentelemetry-exporter-datadog/tests/test_datadog_format.py index 04ed3c5cab3..0f92916efba 100644 --- a/exporter/opentelemetry-exporter-datadog/tests/test_datadog_format.py +++ b/exporter/opentelemetry-exporter-datadog/tests/test_datadog_format.py @@ -18,11 +18,11 @@ from opentelemetry.exporter.datadog import constants, propagator from opentelemetry.sdk import trace from opentelemetry.trace import get_current_span, set_span_in_context -from opentelemetry.trace.propagation.textmap import Getter +from opentelemetry.trace.propagation.textmap import DictGetter FORMAT = propagator.DatadogFormat() -getter = Getter() +getter = DictGetter() class TestDatadogFormat(unittest.TestCase): diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index 2f0530b3685..f0d68d2d5f1 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -117,9 +117,7 @@ def _before_request(): pass if span_name is None: span_name = otel_wsgi.get_default_span_name(environ) - token = context.attach( - propagators.extract(otel_wsgi.getter, environ) - ) + token = context.attach(propagators.extract(otel_wsgi.getter, environ)) tracer = trace.get_tracer(__name__, __version__) diff --git a/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py b/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py index 87e4162b1bd..1d4a8cb62b4 100644 --- a/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py +++ b/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py @@ -18,13 +18,13 @@ import opentelemetry.trace as trace from opentelemetry.context.context import Context from opentelemetry.trace.propagation import textmap + # Keys and values are strings of up to 256 printable US-ASCII characters. # Implementations should conform to 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 -from opentelemetry.trace.propagation.textmap import TextMapPropagatorT _KEY_WITHOUT_VENDOR_FORMAT = r"[a-z][_0-9a-z\-\*\/]{0,255}" _KEY_WITH_VENDOR_FORMAT = ( From 3a78314dac3ae94ebc6cdf1518b272e5629c7eed Mon Sep 17 00:00:00 2001 From: Prajilesh N Date: Sat, 24 Oct 2020 00:43:26 +0530 Subject: [PATCH 07/11] updated DictGetter implementation of TextMap --- .../exporter/datadog/propagator.py | 12 +++---- .../tests/test_datadog_format.py | 12 +++---- .../instrumentation/asgi/__init__.py | 31 ++++++++--------- .../instrumentation/celery/__init__.py | 27 ++++++++------- .../instrumentation/django/middleware.py | 4 +-- .../instrumentation/falcon/__init__.py | 4 ++- .../instrumentation/flask/__init__.py | 4 ++- .../instrumentation/grpc/_server.py | 12 ++----- .../opentracing_shim/__init__.py | 11 ++----- .../instrumentation/pyramid/callbacks.py | 4 ++- .../instrumentation/tornado/__init__.py | 14 +++----- .../instrumentation/wsgi/__init__.py | 33 ++++++++++--------- .../baggage/propagation/__init__.py | 4 +-- .../src/opentelemetry/propagators/__init__.py | 8 ++--- .../opentelemetry/propagators/composite.py | 4 +-- .../trace/propagation/textmap.py | 27 +++++++-------- .../trace/propagation/tracecontext.py | 8 ++--- .../tests/baggage/test_baggage_propagation.py | 17 ++++------ .../propagators/test_global_httptextformat.py | 14 ++------ .../test_tracecontexthttptextformat.py | 28 ++++++---------- .../sdk/trace/propagation/b3_format.py | 20 ++++------- .../tests/trace/propagation/test_b3_format.py | 18 +++++----- .../src/opentelemetry/test/mock_textmap.py | 8 ++--- 23 files changed, 143 insertions(+), 181 deletions(-) diff --git a/exporter/opentelemetry-exporter-datadog/src/opentelemetry/exporter/datadog/propagator.py b/exporter/opentelemetry-exporter-datadog/src/opentelemetry/exporter/datadog/propagator.py index afc61bba401..ab1468c54ab 100644 --- a/exporter/opentelemetry-exporter-datadog/src/opentelemetry/exporter/datadog/propagator.py +++ b/exporter/opentelemetry-exporter-datadog/src/opentelemetry/exporter/datadog/propagator.py @@ -39,25 +39,23 @@ class DatadogFormat(TextMapPropagator): def extract( self, - get_from_carrier: Getter[TextMapPropagatorT], + getter: Getter[TextMapPropagatorT], carrier: TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: trace_id = extract_first_element( - get_from_carrier.get(carrier, self.TRACE_ID_KEY) + getter.get(carrier, self.TRACE_ID_KEY) ) span_id = extract_first_element( - get_from_carrier.get(carrier, self.PARENT_ID_KEY) + getter.get(carrier, self.PARENT_ID_KEY) ) sampled = extract_first_element( - get_from_carrier.get(carrier, self.SAMPLING_PRIORITY_KEY) + getter.get(carrier, self.SAMPLING_PRIORITY_KEY) ) - origin = extract_first_element( - get_from_carrier.get(carrier, self.ORIGIN_KEY) - ) + origin = extract_first_element(getter.get(carrier, self.ORIGIN_KEY)) trace_flags = trace.TraceFlags() if sampled and int(sampled) in ( diff --git a/exporter/opentelemetry-exporter-datadog/tests/test_datadog_format.py b/exporter/opentelemetry-exporter-datadog/tests/test_datadog_format.py index 0f92916efba..bb41fef49b1 100644 --- a/exporter/opentelemetry-exporter-datadog/tests/test_datadog_format.py +++ b/exporter/opentelemetry-exporter-datadog/tests/test_datadog_format.py @@ -22,7 +22,7 @@ FORMAT = propagator.DatadogFormat() -getter = DictGetter() +carrier_getter = DictGetter() class TestDatadogFormat(unittest.TestCase): @@ -43,7 +43,7 @@ def test_malformed_headers(self): malformed_parent_id_key = FORMAT.PARENT_ID_KEY + "-x" context = get_current_span( FORMAT.extract( - getter, + carrier_getter, { malformed_trace_id_key: self.serialized_trace_id, malformed_parent_id_key: self.serialized_parent_id, @@ -61,7 +61,7 @@ def test_missing_trace_id(self): FORMAT.PARENT_ID_KEY: self.serialized_parent_id, } - ctx = FORMAT.extract(getter, carrier) + ctx = FORMAT.extract(carrier_getter, carrier) span_context = get_current_span(ctx).get_span_context() self.assertEqual(span_context.trace_id, trace_api.INVALID_TRACE_ID) @@ -71,7 +71,7 @@ def test_missing_parent_id(self): FORMAT.TRACE_ID_KEY: self.serialized_trace_id, } - ctx = FORMAT.extract(getter, carrier) + ctx = FORMAT.extract(carrier_getter, carrier) span_context = get_current_span(ctx).get_span_context() self.assertEqual(span_context.span_id, trace_api.INVALID_SPAN_ID) @@ -79,7 +79,7 @@ def test_context_propagation(self): """Test the propagation of Datadog headers.""" parent_span_context = get_current_span( FORMAT.extract( - getter, + carrier_getter, { FORMAT.TRACE_ID_KEY: self.serialized_trace_id, FORMAT.PARENT_ID_KEY: self.serialized_parent_id, @@ -136,7 +136,7 @@ def test_sampling_priority_auto_reject(self): """Test sampling priority rejected.""" parent_span_context = get_current_span( FORMAT.extract( - getter, + carrier_getter, { FORMAT.TRACE_ID_KEY: self.serialized_trace_id, FORMAT.PARENT_ID_KEY: self.serialized_parent_id, diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index 081d7048d6f..9975047af06 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -29,25 +29,22 @@ from opentelemetry import context, propagators, trace from opentelemetry.instrumentation.asgi.version import __version__ # noqa from opentelemetry.instrumentation.utils import http_status_to_canonical_code -from opentelemetry.trace.propagation.textmap import DictGetter, HelperGetter +from opentelemetry.trace.propagation.textmap import DictGetter from opentelemetry.trace.status import Status, StatusCanonicalCode -def get_header_from_scope(scope: dict, header_name: str) -> typing.List[str]: - """Retrieve a HTTP header value from the ASGI scope. - - Returns: - A list with a single string with the header value if it exists, else an empty list. - """ - headers = scope.get("headers") - return [ - value.decode("utf8") - for (key, value) in headers - if key.decode("utf8") == header_name - ] +# pylint:disable=arguments-differ +class CarrierGetter(DictGetter): + def get(self, scope: dict, header_name: str) -> typing.List[str]: + headers = scope.get("headers") + return [ + value.decode("utf8") + for (key, value) in headers + if key.decode("utf8") == header_name + ] -getter = HelperGetter(get_header_from_scope, DictGetter.keys) +carrier_getter = CarrierGetter() def collect_request_attributes(scope): @@ -76,10 +73,10 @@ def collect_request_attributes(scope): http_method = scope.get("method") if http_method: result["http.method"] = http_method - http_host_value = ",".join(getter.get(scope, "host")) + http_host_value = ",".join(carrier_getter.get(scope, "host")) if http_host_value: result["http.server_name"] = http_host_value - http_user_agent = getter.get(scope, "user-agent") + http_user_agent = carrier_getter.get(scope, "user-agent") if len(http_user_agent) > 0: result["http.user_agent"] = http_user_agent[0] @@ -158,7 +155,7 @@ async def __call__(self, scope, receive, send): if scope["type"] not in ("http", "websocket"): return await self.app(scope, receive, send) - token = context.attach(propagators.extract(getter, scope)) + token = context.attach(propagators.extract(carrier_getter, scope)) span_name, additional_attributes = self.span_details_callback(scope) try: diff --git a/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py b/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py index 4b71a9e0a37..c856619ebdf 100644 --- a/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py @@ -67,8 +67,7 @@ def add(x, y): from opentelemetry.instrumentation.celery import utils from opentelemetry.instrumentation.celery.version import __version__ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor -from opentelemetry.trace.propagation import get_current_span -from opentelemetry.trace.propagation.textmap import DictGetter, HelperGetter +from opentelemetry.trace.propagation.textmap import DictGetter from opentelemetry.trace.status import Status, StatusCanonicalCode logger = logging.getLogger(__name__) @@ -85,6 +84,20 @@ def add(x, y): _MESSAGE_ID_ATTRIBUTE_NAME = "messaging.message_id" +class CarrierGetter(DictGetter): + def get(self, carrier, key): + value = getattr(carrier, key, []) + if isinstance(value, str) or not isinstance(value, Iterable): + value = (value,) + return value + + def keys(self, carrier): + return [] + + +carrier_getter = CarrierGetter() + + class CeleryInstrumentor(BaseInstrumentor): def _instrument(self, **kwargs): tracer_provider = kwargs.get("tracer_provider") @@ -119,8 +132,7 @@ def _trace_prerun(self, *args, **kwargs): return request = task.request - getter = HelperGetter(carrier_extractor, DictGetter.keys) - tracectx = propagators.extract(getter, request) or None + tracectx = propagators.extract(carrier_getter, request) or None logger.debug("prerun signal start task_id=%s", task_id) @@ -249,10 +261,3 @@ def _trace_retry(*args, **kwargs): # Use `str(reason)` instead of `reason.message` in case we get # something that isn't an `Exception` span.set_attribute(_TASK_RETRY_REASON_KEY, str(reason)) - - -def carrier_extractor(carrier, key): - value = getattr(carrier, key, []) - if isinstance(value, str) or not isinstance(value, Iterable): - value = (value,) - return value diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py index bd79ef45847..a395de99e63 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py @@ -23,8 +23,8 @@ from opentelemetry.instrumentation.utils import extract_attributes_from_object from opentelemetry.instrumentation.wsgi import ( add_response_attributes, + carrier_getter, collect_request_attributes, - getter, ) from opentelemetry.propagators import extract from opentelemetry.trace import SpanKind, get_tracer @@ -125,7 +125,7 @@ def process_request(self, request): environ = request.META - token = attach(extract(getter, environ)) + token = attach(extract(carrier_getter, environ)) tracer = get_tracer(__name__, __version__) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index f39c53e5a93..8c817192262 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -114,7 +114,9 @@ def __call__(self, env, start_response): start_time = time_ns() - token = context.attach(propagators.extract(otel_wsgi.getter, env)) + token = context.attach( + propagators.extract(otel_wsgi.carrier_getter, env) + ) span = self._tracer.start_span( otel_wsgi.get_default_span_name(env), kind=trace.SpanKind.SERVER, diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index f0d68d2d5f1..1235b09a307 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -117,7 +117,9 @@ def _before_request(): pass if span_name is None: span_name = otel_wsgi.get_default_span_name(environ) - token = context.attach(propagators.extract(otel_wsgi.getter, environ)) + token = context.attach( + propagators.extract(otel_wsgi.carrier_getter, environ) + ) tracer = trace.get_tracer(__name__, __version__) diff --git a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py index dc8b20ea2ed..60f10559ace 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py @@ -24,13 +24,12 @@ """ from contextlib import contextmanager -from typing import List import grpc from opentelemetry import propagators, trace from opentelemetry.context import attach, detach -from opentelemetry.trace.propagation.textmap import DictGetter, HelperGetter +from opentelemetry.trace.propagation.textmap import DictGetter from . import grpcext from ._utilities import RpcInfo @@ -116,6 +115,7 @@ class OpenTelemetryServerInterceptor( ): def __init__(self, tracer): self._tracer = tracer + self._carrier_getter = DictGetter() @contextmanager # pylint:disable=no-self-use @@ -123,13 +123,7 @@ def _set_remote_context(self, servicer_context): metadata = servicer_context.invocation_metadata() if metadata: md_dict = {md.key: md.value for md in metadata} - - def get_from_grpc_metadata(metadata, key) -> List[str]: - return [md_dict[key]] if key in md_dict else [] - - getter = HelperGetter(get_from_grpc_metadata, DictGetter.keys) - # Update the context with the traceparent from the RPC metadata. - ctx = propagators.extract(getter, metadata) + ctx = propagators.extract(self._carrier_getter, md_dict) token = attach(ctx) try: yield diff --git a/instrumentation/opentelemetry-instrumentation-opentracing-shim/src/opentelemetry/instrumentation/opentracing_shim/__init__.py b/instrumentation/opentelemetry-instrumentation-opentracing-shim/src/opentelemetry/instrumentation/opentracing_shim/__init__.py index 8eb1b8cbb1d..00a0b8d0cb8 100644 --- a/instrumentation/opentelemetry-instrumentation-opentracing-shim/src/opentelemetry/instrumentation/opentracing_shim/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-opentracing-shim/src/opentelemetry/instrumentation/opentracing_shim/__init__.py @@ -112,7 +112,7 @@ get_current_span, set_span_in_context, ) -from opentelemetry.trace.propagation.textmap import DictGetter, HelperGetter +from opentelemetry.trace.propagation.textmap import DictGetter from opentelemetry.util.types import Attributes ValueT = TypeVar("ValueT", int, float, bool, str) @@ -528,6 +528,7 @@ def __init__(self, tracer: OtelTracer): Format.TEXT_MAP, Format.HTTP_HEADERS, ) + self._carrier_getter = DictGetter() def unwrap(self): """Returns the :class:`opentelemetry.trace.Tracer` object that is @@ -711,14 +712,8 @@ def extract(self, format: object, carrier: object): if format not in self._supported_formats: raise UnsupportedFormatException - def get_as_list(dict_object, key): - value = dict_object.get(key) - return [value] if value is not None else [] - - getter = HelperGetter(get_as_list, DictGetter.keys) - propagator = propagators.get_global_textmap() - ctx = propagator.extract(getter, carrier) + ctx = propagator.extract(self._carrier_getter, carrier) span = get_current_span(ctx) if span is not None: otel_context = span.get_span_context() diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py index a854c55e2e1..e7110bd2b55 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py @@ -69,7 +69,9 @@ def _before_traversal(event): start_time = environ.get(_ENVIRON_STARTTIME_KEY) - token = context.attach(propagators.extract(otel_wsgi.getter, environ)) + token = context.attach( + propagators.extract(otel_wsgi.carrier_getter, environ) + ) tracer = trace.get_tracer(__name__, __version__) if request.matched_route: diff --git a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py index 0204ff43508..7acc3cce09d 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py @@ -54,7 +54,7 @@ def get(self): http_status_to_canonical_code, unwrap, ) -from opentelemetry.trace.propagation.textmap import DictGetter, HelperGetter +from opentelemetry.trace.propagation.textmap import DictGetter from opentelemetry.trace.status import Status from opentelemetry.util import ExcludeList, time_ns @@ -85,6 +85,8 @@ def get_traced_request_attrs(): _excluded_urls = get_excluded_urls() _traced_attrs = get_traced_request_attrs() +carrier_getter = DictGetter() + class TornadoInstrumentor(BaseInstrumentor): patched_handlers = [] @@ -186,13 +188,6 @@ def _log_exception(tracer, func, handler, args, kwargs): return func(*args, **kwargs) -def _get_header_from_request_headers( - headers: dict, header_name: str -) -> typing.List[str]: - header = headers.get(header_name) - return [header] if header else [] - - def _get_attributes_from_request(request): attrs = { "component": "tornado", @@ -218,9 +213,8 @@ def _get_operation_name(handler, request): def _start_span(tracer, handler, start_time) -> _TraceContext: - getter = HelperGetter(_get_header_from_request_headers, DictGetter.keys) token = context.attach( - propagators.extract(getter, handler.request.headers,) + propagators.extract(carrier_getter, handler.request.headers,) ) span = tracer.start_span( diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index 69737e7245c..7db6afda1e3 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -61,28 +61,31 @@ def hello(): from opentelemetry import context, propagators, trace from opentelemetry.instrumentation.utils import http_status_to_canonical_code from opentelemetry.instrumentation.wsgi.version import __version__ -from opentelemetry.trace.propagation.textmap import DictGetter, HelperGetter +from opentelemetry.trace.propagation.textmap import DictGetter from opentelemetry.trace.status import Status, StatusCanonicalCode _HTTP_VERSION_PREFIX = "HTTP/" -def get_header_from_environ( - environ: dict, header_name: str -) -> typing.List[str]: - """Retrieve a HTTP header value from the PEP3333-conforming WSGI environ. +# pylint:disable=arguments-differ +class CarrierGetter(DictGetter): + def get(self, environ: dict, header_name: str) -> typing.List[str]: + """Retrieve a HTTP header value from the PEP3333-conforming WSGI environ. - Returns: - A list with a single string with the header value if it exists, else an empty list. - """ - environ_key = "HTTP_" + header_name.upper().replace("-", "_") - value = environ.get(environ_key) - if value is not None: - return [value] - return [] + Returns: + A list with a single string with the header value if it exists, else an empty list. + """ + environ_key = "HTTP_" + header_name.upper().replace("-", "_") + value = environ.get(environ_key) + if value is not None: + return [value] + return [] + + def keys(self, carrier): + return [] -getter = HelperGetter(get_header_from_environ, DictGetter.keys) +carrier_getter = CarrierGetter() def setifnotnone(dic, key, value): @@ -199,7 +202,7 @@ def __call__(self, environ, start_response): start_response: The WSGI start_response callable. """ - token = context.attach(propagators.extract(getter, environ)) + token = context.attach(propagators.extract(carrier_getter, environ)) span_name = self.name_callback(environ) span = self.tracer.start_span( diff --git a/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py b/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py index bde29282541..70e73a3b23b 100644 --- a/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py +++ b/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py @@ -29,7 +29,7 @@ class BaggagePropagator(textmap.TextMapPropagator): def extract( self, - get_from_carrier: textmap.Getter[textmap.TextMapPropagatorT], + getter: textmap.Getter[textmap.TextMapPropagatorT], carrier: textmap.TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: @@ -43,7 +43,7 @@ def extract( context = get_current() header = _extract_first_element( - get_from_carrier.get(carrier, self._BAGGAGE_HEADER_NAME) + getter.get(carrier, self._BAGGAGE_HEADER_NAME) ) if not header or len(header) > self.MAX_HEADER_LENGTH: diff --git a/opentelemetry-api/src/opentelemetry/propagators/__init__.py b/opentelemetry-api/src/opentelemetry/propagators/__init__.py index 946aefcfaad..fb2863ac8ad 100644 --- a/opentelemetry-api/src/opentelemetry/propagators/__init__.py +++ b/opentelemetry-api/src/opentelemetry/propagators/__init__.py @@ -82,24 +82,24 @@ def example_route(): def extract( - get_from_carrier: textmap.Getter[textmap.TextMapPropagatorT], + getter: textmap.Getter[textmap.TextMapPropagatorT], carrier: textmap.TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: """ Uses the configured propagator to extract a Context from the carrier. Args: - get_from_carrier: an object which contains a get function that can retrieve zero + getter: an object which contains a get function that can retrieve zero or more values from the carrier and a keys function that can get all the keys from carrier. carrier: and object which contains values that are used to construct a Context. This object - must be paired with an appropriate get_from_carrier + must be paired with an appropriate getter which understands how to extract a value from it. context: an optional Context to use. Defaults to current context if not set. """ - return get_global_textmap().extract(get_from_carrier, carrier, context) + return get_global_textmap().extract(getter, carrier, context) def inject( diff --git a/opentelemetry-api/src/opentelemetry/propagators/composite.py b/opentelemetry-api/src/opentelemetry/propagators/composite.py index 3499d2ea08a..441098ec1f0 100644 --- a/opentelemetry-api/src/opentelemetry/propagators/composite.py +++ b/opentelemetry-api/src/opentelemetry/propagators/composite.py @@ -35,7 +35,7 @@ def __init__( def extract( self, - get_from_carrier: textmap.Getter[textmap.TextMapPropagatorT], + getter: textmap.Getter[textmap.TextMapPropagatorT], carrier: textmap.TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: @@ -47,7 +47,7 @@ def extract( See `opentelemetry.trace.propagation.textmap.TextMapPropagator.extract` """ for propagator in self._propagators: - context = propagator.extract(get_from_carrier, carrier, context) + context = propagator.extract(getter, carrier, context) return context # type: ignore def inject( diff --git a/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py b/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py index 182435ec073..0960bc73bad 100644 --- a/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py +++ b/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py @@ -18,6 +18,7 @@ from opentelemetry.context.context import Context TextMapPropagatorT = typing.TypeVar("TextMapPropagatorT") +CarrierValT = typing.Union[typing.List[str], str] Setter = typing.Callable[[TextMapPropagatorT, str, str], None] @@ -35,7 +36,7 @@ def get(self, carrier: TextMapPropagatorT, key: str) -> typing.List[str]: Args: carrier: and object which contains values that are used to construct a Context. This object must be paired with an appropriate - get_from_carrier which understands how to extract a value from it. + getter which understands how to extract a value from it. key: key of a field in carrier. Returns: first value of the propagation key or an empty list if the key doesn't exist. """ @@ -47,7 +48,7 @@ def keys(self, carrier: TextMapPropagatorT) -> typing.List[str]: Args: carrier: and object which contains values that are used to construct a Context. This object - must be paired with an appropriate get_from_carrier + must be paired with an appropriate getter which understands how to extract a value from it. Returns: list of keys from the carrier. @@ -55,20 +56,20 @@ def keys(self, carrier: TextMapPropagatorT) -> typing.List[str]: raise NotImplementedError() -class DictGetter(Getter[typing.Dict[str, str]]): +class DictGetter(Getter[typing.Dict[str, CarrierValT]]): def get( - self, carrier: typing.Dict[str, str], key: str + self, carrier: typing.Dict[str, CarrierValT], key: str ) -> typing.List[str]: val = carrier.get(key, None) - if val: - return [val] - return [] + if not val: + return [] + return val if isinstance(val, typing.List) else [val] - def keys(self, carrier: typing.Dict[str, str]) -> typing.List[str]: + def keys(self, carrier: typing.Dict[str, CarrierValT]) -> typing.List[str]: return list(carrier.keys()) -class HelperGetter(Getter[TextMapPropagatorT]): +class CustomGetter(Getter[TextMapPropagatorT]): def __init__( self, get: typing.Callable[[TextMapPropagatorT, str], typing.List[str]], @@ -96,23 +97,23 @@ class TextMapPropagator(abc.ABC): @abc.abstractmethod def extract( self, - get_from_carrier: Getter[TextMapPropagatorT], + getter: Getter[TextMapPropagatorT], carrier: TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: """Create a Context from values in the carrier. The extract function should retrieve values from the carrier - object using get_from_carrier, and use values to populate a + object using getter, and use values to populate a Context value and return it. Args: - get_from_carrier: a function that can retrieve zero + getter: a function that can retrieve zero or more values from the carrier. In the case that the value does not exist, return an empty list. carrier: and object which contains values that are used to construct a Context. This object - must be paired with an appropriate get_from_carrier + must be paired with an appropriate getter which understands how to extract a value from it. context: an optional Context to use. Defaults to current context if not set. diff --git a/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py b/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py index 1d4a8cb62b4..57933ef9c41 100644 --- a/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py +++ b/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py @@ -59,7 +59,7 @@ class TraceContextTextMapPropagator(textmap.TextMapPropagator): def extract( self, - get_from_carrier: textmap.Getter[textmap.TextMapPropagatorT], + getter: textmap.Getter[textmap.TextMapPropagatorT], carrier: textmap.TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: @@ -67,7 +67,7 @@ def extract( See `opentelemetry.trace.propagation.textmap.TextMapPropagator.extract` """ - header = get_from_carrier.get(carrier, self._TRACEPARENT_HEADER_NAME) + header = getter.get(carrier, self._TRACEPARENT_HEADER_NAME) if not header: return trace.set_span_in_context(trace.INVALID_SPAN, context) @@ -90,9 +90,7 @@ def extract( if version == "ff": return trace.set_span_in_context(trace.INVALID_SPAN, context) - tracestate_headers = get_from_carrier.get( - carrier, self._TRACESTATE_HEADER_NAME - ) + tracestate_headers = getter.get(carrier, self._TRACESTATE_HEADER_NAME) tracestate = _parse_tracestate(tracestate_headers) span_context = trace.SpanContext( diff --git a/opentelemetry-api/tests/baggage/test_baggage_propagation.py b/opentelemetry-api/tests/baggage/test_baggage_propagation.py index 7562d59c2a5..4c7b3de215a 100644 --- a/opentelemetry-api/tests/baggage/test_baggage_propagation.py +++ b/opentelemetry-api/tests/baggage/test_baggage_propagation.py @@ -18,16 +18,9 @@ from opentelemetry import baggage from opentelemetry.baggage.propagation import BaggagePropagator from opentelemetry.context import get_current -from opentelemetry.trace.propagation.textmap import DictGetter, HelperGetter +from opentelemetry.trace.propagation.textmap import DictGetter - -def get_as_list( - dict_object: typing.Dict[str, typing.List[str]], key: str -) -> typing.List[str]: - return dict_object.get(key, []) - - -getter = HelperGetter(get_as_list, DictGetter.keys) +carrier_getter = DictGetter() class TestBaggagePropagation(unittest.TestCase): @@ -37,7 +30,7 @@ def setUp(self): def _extract(self, header_value): """Test helper""" header = {"baggage": [header_value]} - return baggage.get_all(self.propagator.extract(getter, header)) + return baggage.get_all(self.propagator.extract(carrier_getter, header)) def _inject(self, values): """Test helper""" @@ -49,7 +42,9 @@ def _inject(self, values): return output.get("baggage") def test_no_context_header(self): - baggage_entries = baggage.get_all(self.propagator.extract(getter, {})) + baggage_entries = baggage.get_all( + self.propagator.extract(carrier_getter, {}) + ) self.assertEqual(baggage_entries, {}) def test_empty_context_header(self): diff --git a/opentelemetry-api/tests/propagators/test_global_httptextformat.py b/opentelemetry-api/tests/propagators/test_global_httptextformat.py index 2ce11f3c6b0..b704207ed5f 100644 --- a/opentelemetry-api/tests/propagators/test_global_httptextformat.py +++ b/opentelemetry-api/tests/propagators/test_global_httptextformat.py @@ -18,17 +18,9 @@ from opentelemetry import baggage, trace from opentelemetry.propagators import extract, inject from opentelemetry.trace import get_current_span, set_span_in_context -from opentelemetry.trace.propagation.textmap import DictGetter, HelperGetter +from opentelemetry.trace.propagation.textmap import DictGetter - -def get_as_list( - 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 [] - - -getter = HelperGetter(get_as_list, DictGetter.keys) +carrier_getter = DictGetter() class TestDefaultGlobalPropagator(unittest.TestCase): @@ -48,7 +40,7 @@ def test_propagation(self): "traceparent": [traceparent_value], "tracestate": [tracestate_value], } - ctx = extract(getter, headers) + ctx = extract(carrier_getter, headers) baggage_entries = baggage.get_all(context=ctx) expected = {"key1": "val1", "key2": "val2"} self.assertEqual(baggage_entries, expected) diff --git a/opentelemetry-api/tests/trace/propagation/test_tracecontexthttptextformat.py b/opentelemetry-api/tests/trace/propagation/test_tracecontexthttptextformat.py index 006afd1d3e9..f012be2a233 100644 --- a/opentelemetry-api/tests/trace/propagation/test_tracecontexthttptextformat.py +++ b/opentelemetry-api/tests/trace/propagation/test_tracecontexthttptextformat.py @@ -17,19 +17,11 @@ from opentelemetry import trace from opentelemetry.trace.propagation import tracecontext -from opentelemetry.trace.propagation.textmap import DictGetter, HelperGetter +from opentelemetry.trace.propagation.textmap import DictGetter FORMAT = tracecontext.TraceContextTextMapPropagator() - -def get_as_list( - 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 [] - - -getter = HelperGetter(get_as_list, DictGetter.keys) +carrier_getter = DictGetter() class TestTraceContextFormat(unittest.TestCase): @@ -46,7 +38,7 @@ def test_no_traceparent_header(self): trace-id and parent-id that represents the current request. """ output = {} # type:typing.Dict[str, typing.List[str]] - span = trace.get_current_span(FORMAT.extract(getter, output)) + span = trace.get_current_span(FORMAT.extract(carrier_getter, output)) self.assertIsInstance(span.get_span_context(), trace.SpanContext) def test_headers_with_tracestate(self): @@ -60,7 +52,7 @@ def test_headers_with_tracestate(self): tracestate_value = "foo=1,bar=2,baz=3" span_context = trace.get_current_span( FORMAT.extract( - getter, + carrier_getter, { "traceparent": [traceparent_value], "tracestate": [tracestate_value], @@ -104,7 +96,7 @@ def test_invalid_trace_id(self): """ span = trace.get_current_span( FORMAT.extract( - getter, + carrier_getter, { "traceparent": [ "00-00000000000000000000000000000000-1234567890123456-00" @@ -135,7 +127,7 @@ def test_invalid_parent_id(self): """ span = trace.get_current_span( FORMAT.extract( - getter, + carrier_getter, { "traceparent": [ "00-00000000000000000000000000000000-0000000000000000-00" @@ -173,7 +165,7 @@ def test_format_not_supported(self): """ span = trace.get_current_span( FORMAT.extract( - getter, + carrier_getter, { "traceparent": [ "00-12345678901234567890123456789012-" @@ -197,7 +189,7 @@ def test_tracestate_empty_header(self): """ span = trace.get_current_span( FORMAT.extract( - getter, + carrier_getter, { "traceparent": [ "00-12345678901234567890123456789012-1234567890123456-00" @@ -213,7 +205,7 @@ def test_tracestate_header_with_trailing_comma(self): """ span = trace.get_current_span( FORMAT.extract( - getter, + carrier_getter, { "traceparent": [ "00-12345678901234567890123456789012-1234567890123456-00" @@ -237,7 +229,7 @@ def test_tracestate_keys(self): ) span = trace.get_current_span( FORMAT.extract( - getter, + carrier_getter, { "traceparent": [ "00-12345678901234567890123456789012-1234567890123456-00" diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/propagation/b3_format.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/propagation/b3_format.py index 0a9d9caba19..c629d107d36 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/propagation/b3_format.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/propagation/b3_format.py @@ -43,7 +43,7 @@ class B3Format(TextMapPropagator): def extract( self, - get_from_carrier: Getter[TextMapPropagatorT], + getter: Getter[TextMapPropagatorT], carrier: TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: @@ -53,7 +53,7 @@ def extract( flags = None single_header = _extract_first_element( - get_from_carrier.get(carrier, self.SINGLE_HEADER_KEY) + getter.get(carrier, self.SINGLE_HEADER_KEY) ) if single_header: # The b3 spec calls for the sampling state to be @@ -74,27 +74,19 @@ def extract( return trace.set_span_in_context(trace.INVALID_SPAN) else: trace_id = ( - _extract_first_element( - get_from_carrier.get(carrier, self.TRACE_ID_KEY) - ) + _extract_first_element(getter.get(carrier, self.TRACE_ID_KEY)) or trace_id ) span_id = ( - _extract_first_element( - get_from_carrier.get(carrier, self.SPAN_ID_KEY) - ) + _extract_first_element(getter.get(carrier, self.SPAN_ID_KEY)) or span_id ) sampled = ( - _extract_first_element( - get_from_carrier.get(carrier, self.SAMPLED_KEY) - ) + _extract_first_element(getter.get(carrier, self.SAMPLED_KEY)) or sampled ) flags = ( - _extract_first_element( - get_from_carrier.get(carrier, self.FLAGS_KEY) - ) + _extract_first_element(getter.get(carrier, self.FLAGS_KEY)) or flags ) diff --git a/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py b/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py index e89d7cf9c91..d68ebe20d77 100644 --- a/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py +++ b/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py @@ -19,17 +19,17 @@ import opentelemetry.sdk.trace.propagation.b3_format as b3_format import opentelemetry.trace as trace_api from opentelemetry.context import get_current -from opentelemetry.trace.propagation.textmap import DictGetter, HelperGetter +from opentelemetry.trace.propagation.textmap import CustomGetter, DictGetter FORMAT = b3_format.B3Format() -getter = DictGetter() +carrier_getter = DictGetter() def get_child_parent_new_carrier(old_carrier): - ctx = FORMAT.extract(getter, old_carrier) + ctx = FORMAT.extract(carrier_getter, old_carrier) parent_span_context = trace_api.get_current_span(ctx).get_span_context() parent = trace._Span("parent", parent_span_context) @@ -230,7 +230,7 @@ def test_invalid_single_header(self): invalid SpanContext. """ carrier = {FORMAT.SINGLE_HEADER_KEY: "0-1-2-3-4-5-6-7"} - ctx = FORMAT.extract(getter, carrier) + ctx = FORMAT.extract(carrier_getter, carrier) span_context = trace_api.get_current_span(ctx).get_span_context() self.assertEqual(span_context.trace_id, trace_api.INVALID_TRACE_ID) self.assertEqual(span_context.span_id, trace_api.INVALID_SPAN_ID) @@ -242,7 +242,7 @@ def test_missing_trace_id(self): FORMAT.FLAGS_KEY: "1", } - ctx = FORMAT.extract(getter, carrier) + ctx = FORMAT.extract(carrier_getter, carrier) span_context = trace_api.get_current_span(ctx).get_span_context() self.assertEqual(span_context.trace_id, trace_api.INVALID_TRACE_ID) @@ -266,7 +266,7 @@ def test_invalid_trace_id( FORMAT.FLAGS_KEY: "1", } - ctx = FORMAT.extract(getter, carrier) + ctx = FORMAT.extract(carrier_getter, carrier) span_context = trace_api.get_current_span(ctx).get_span_context() self.assertEqual(span_context.trace_id, 1) @@ -292,7 +292,7 @@ def test_invalid_span_id( FORMAT.FLAGS_KEY: "1", } - ctx = FORMAT.extract(getter, carrier) + ctx = FORMAT.extract(carrier_getter, carrier) span_context = trace_api.get_current_span(ctx).get_span_context() self.assertEqual(span_context.trace_id, 1) @@ -305,7 +305,7 @@ def test_missing_span_id(self): FORMAT.FLAGS_KEY: "1", } - ctx = FORMAT.extract(getter, carrier) + ctx = FORMAT.extract(carrier_getter, carrier) span_context = trace_api.get_current_span(ctx).get_span_context() self.assertEqual(span_context.span_id, trace_api.INVALID_SPAN_ID) @@ -327,6 +327,6 @@ def setter(carrier, key, value): carrier[key] = value ctx = FORMAT.extract( - HelperGetter(default_span_getter, DictGetter.keys), {} + CustomGetter(default_span_getter, DictGetter().keys), {} ) FORMAT.inject(setter, {}, ctx) diff --git a/tests/util/src/opentelemetry/test/mock_textmap.py b/tests/util/src/opentelemetry/test/mock_textmap.py index 83498123ded..1fee6d2a9ca 100644 --- a/tests/util/src/opentelemetry/test/mock_textmap.py +++ b/tests/util/src/opentelemetry/test/mock_textmap.py @@ -33,7 +33,7 @@ class NOOPTextMapPropagator(TextMapPropagator): def extract( self, - get_from_carrier: Getter[TextMapPropagatorT], + getter: Getter[TextMapPropagatorT], carrier: TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: @@ -56,12 +56,12 @@ class MockTextMapPropagator(TextMapPropagator): def extract( self, - get_from_carrier: Getter[TextMapPropagatorT], + getter: Getter[TextMapPropagatorT], carrier: TextMapPropagatorT, context: typing.Optional[Context] = None, ) -> Context: - trace_id_list = get_from_carrier.get(carrier, self.TRACE_ID_KEY) - span_id_list = get_from_carrier.get(carrier, self.SPAN_ID_KEY) + trace_id_list = getter.get(carrier, self.TRACE_ID_KEY) + span_id_list = getter.get(carrier, self.SPAN_ID_KEY) if not trace_id_list or not span_id_list: return trace.set_span_in_context(trace.INVALID_SPAN) From f3afa1f05b771566be56c28a30d7b5b297a778d5 Mon Sep 17 00:00:00 2001 From: Prajilesh N Date: Sat, 24 Oct 2020 02:07:04 +0530 Subject: [PATCH 08/11] added textMap.DictGetter in nitpick_ignore --- docs/conf.py | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/conf.py b/docs/conf.py index 68b871aaac2..b227cb51818 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -102,6 +102,7 @@ ("py:class", "ObjectProxy"), # TODO: Understand why sphinx is not able to find this local class ("py:class", "opentelemetry.trace.propagation.textmap.TextMapPropagator",), + ("py:class", "opentelemetry.trace.propagation.textmap.DictGetter",), ( "any", "opentelemetry.trace.propagation.textmap.TextMapPropagator.extract", From 7c3a32cdb19ee62833e906f93e6a4c138b27ae9e Mon Sep 17 00:00:00 2001 From: Prajilesh N Date: Sat, 24 Oct 2020 16:50:28 +0530 Subject: [PATCH 09/11] adding Iterable to DefaultDict of TextMap --- .../trace/propagation/textmap.py | 41 ++++++------------- .../tests/trace/propagation/test_b3_format.py | 11 +++-- 2 files changed, 17 insertions(+), 35 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py b/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py index 0960bc73bad..ec505135bec 100644 --- a/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py +++ b/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py @@ -34,11 +34,12 @@ def get(self, carrier: TextMapPropagatorT, key: str) -> typing.List[str]: or more values from the carrier. In the case that the value does not exist, returns an empty list. - Args: carrier: and object which contains values that are used to - construct a Context. This object must be paired with an appropriate - getter which understands how to extract a value from it. - key: key of a field in carrier. Returns: first value of the - propagation key or an empty list if the key doesn't exist. + Args: + carrier: An object which contains values that are used to + construct a Context. + key: key of a field in carrier. + Returns: first value of the propagation key or an empty list if the + key doesn't exist. """ raise NotImplementedError() @@ -46,10 +47,8 @@ def keys(self, carrier: TextMapPropagatorT) -> typing.List[str]: """Function that can retrieve all the keys in a carrier object. Args: - carrier: and object which contains values that are - used to construct a Context. This object - must be paired with an appropriate getter - which understands how to extract a value from it. + carrier: An object which contains values that are + used to construct a Context. Returns: list of keys from the carrier. """ @@ -60,31 +59,15 @@ class DictGetter(Getter[typing.Dict[str, CarrierValT]]): def get( self, carrier: typing.Dict[str, CarrierValT], key: str ) -> typing.List[str]: - val = carrier.get(key, None) - if not val: - return [] - return val if isinstance(val, typing.List) else [val] + val = carrier.get(key, []) + if isinstance(val, typing.Iterable) and not isinstance(val, str): + return list(val) + return [val] def keys(self, carrier: typing.Dict[str, CarrierValT]) -> typing.List[str]: return list(carrier.keys()) -class CustomGetter(Getter[TextMapPropagatorT]): - def __init__( - self, - get: typing.Callable[[TextMapPropagatorT, str], typing.List[str]], - keys: typing.Callable[[TextMapPropagatorT], typing.List[str]], - ): - self._get = get - self._keys = keys - - def get(self, carrier: TextMapPropagatorT, key: str) -> typing.List[str]: - return self._get(carrier, key) - - def keys(self, carrier: TextMapPropagatorT) -> typing.List[str]: - return self._keys(carrier) - - class TextMapPropagator(abc.ABC): """This class provides an interface that enables extracting and injecting context into headers of HTTP requests. HTTP frameworks and clients diff --git a/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py b/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py index d68ebe20d77..79c4618aee7 100644 --- a/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py +++ b/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py @@ -19,7 +19,7 @@ import opentelemetry.sdk.trace.propagation.b3_format as b3_format import opentelemetry.trace as trace_api from opentelemetry.context import get_current -from opentelemetry.trace.propagation.textmap import CustomGetter, DictGetter +from opentelemetry.trace.propagation.textmap import DictGetter FORMAT = b3_format.B3Format() @@ -320,13 +320,12 @@ def test_inject_empty_context(): def test_default_span(): """Make sure propagator does not crash when working with DefaultSpan""" - def default_span_getter(carrier, key): - return carrier.get(key, None) + class CarrierGetter(DictGetter): + def get(self, carrier, key): + return carrier.get(key, None) def setter(carrier, key, value): carrier[key] = value - ctx = FORMAT.extract( - CustomGetter(default_span_getter, DictGetter().keys), {} - ) + ctx = FORMAT.extract(CarrierGetter(), {}) FORMAT.inject(setter, {}, ctx) From 136eac65a17561feb40ebe95d0d9ffdaaa1989a5 Mon Sep 17 00:00:00 2001 From: Prajilesh N Date: Sun, 25 Oct 2020 21:52:35 +0530 Subject: [PATCH 10/11] updated the get arguments as per the spec --- .../instrumentation/asgi/__init__.py | 21 +++++++++++++------ .../instrumentation/wsgi/__init__.py | 16 ++++++++------ 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index 9975047af06..4c28c48ca37 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -33,14 +33,23 @@ from opentelemetry.trace.status import Status, StatusCanonicalCode -# pylint:disable=arguments-differ class CarrierGetter(DictGetter): - def get(self, scope: dict, header_name: str) -> typing.List[str]: - headers = scope.get("headers") + def get(self, carrier: dict, key: str) -> typing.List[str]: + """Getter implementation to retrieve a HTTP header value from the ASGI + scope. + + Args: + carrier: ASGI scope object + key: header name in scope + Returns: + A list with a single string with the header value if it exists, + else an empty list. + """ + headers = carrier.get("headers") return [ - value.decode("utf8") - for (key, value) in headers - if key.decode("utf8") == header_name + _value.decode("utf8") + for (_key, _value) in headers + if _key.decode("utf8") == key ] diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index 7db6afda1e3..5f81c7978c2 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -67,16 +67,20 @@ def hello(): _HTTP_VERSION_PREFIX = "HTTP/" -# pylint:disable=arguments-differ class CarrierGetter(DictGetter): - def get(self, environ: dict, header_name: str) -> typing.List[str]: - """Retrieve a HTTP header value from the PEP3333-conforming WSGI environ. + def get(self, carrier: dict, key: str) -> typing.List[str]: + """Getter implementation to retrieve a HTTP header value from the + PEP3333-conforming WSGI environ + Args: + carrier: WSGI environ object + key: header name in environ object Returns: - A list with a single string with the header value if it exists, else an empty list. + A list with a single string with the header value if it exists, + else an empty list. """ - environ_key = "HTTP_" + header_name.upper().replace("-", "_") - value = environ.get(environ_key) + environ_key = "HTTP_" + key.upper().replace("-", "_") + value = carrier.get(environ_key) if value is not None: return [value] return [] From d51396141b1d300e046b712eb35b40243dc0ca4c Mon Sep 17 00:00:00 2001 From: alrex Date: Sun, 1 Nov 2020 19:26:40 -0800 Subject: [PATCH 11/11] fix a a bad merge conflict resolution tried to fix a merge conflict and accidentally removed the import --- .../src/opentelemetry/instrumentation/grpc/_server.py | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py index a0ee3e1df5c..0b2caa92f74 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py @@ -28,6 +28,7 @@ from opentelemetry import propagators, trace from opentelemetry.context import attach, detach +from opentelemetry.trace.propagation.textmap import DictGetter from opentelemetry.trace.status import Status, StatusCode logger = logging.getLogger(__name__)