From b755b0c8ca77776e1569081465e2ad0159acfa3b Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Thu, 5 Sep 2019 21:23:49 -0700 Subject: [PATCH] Fixing some errors Making the requests test pass again. Fixing propagator impl in the SDK. The correct fields were not being used. --- .../ext/http_requests/__init__.py | 3 +- .../tests/test_requests_integration.py | 6 ++- .../src/opentelemetry/ext/wsgi/__init__.py | 1 - .../src/opentelemetry/propagator/__init__.py | 41 +++++++------------ opentelemetry-api/tests/test_loader.py | 5 +-- .../opentelemetry/sdk/propagator/__init__.py | 8 ++-- .../tests/propagator/test_b3_format.py | 4 +- opentelemetry-sdk/tests/trace/test_trace.py | 2 +- 8 files changed, 30 insertions(+), 40 deletions(-) diff --git a/ext/opentelemetry-ext-http-requests/src/opentelemetry/ext/http_requests/__init__.py b/ext/opentelemetry-ext-http-requests/src/opentelemetry/ext/http_requests/__init__.py index 59885060fd..db9a34ca0d 100644 --- a/ext/opentelemetry-ext-http-requests/src/opentelemetry/ext/http_requests/__init__.py +++ b/ext/opentelemetry-ext-http-requests/src/opentelemetry/ext/http_requests/__init__.py @@ -21,6 +21,7 @@ from urllib.parse import urlparse from requests.sessions import Session + from opentelemetry import propagator @@ -79,7 +80,7 @@ def instrumented_request(self, method, url, *args, **kwargs): span.set_attribute("http.status_text", result.reason) propagator.global_propagator().inject( - result.headers.set, result.headers + result.headers.__setitem__, result.headers ) return result diff --git a/ext/opentelemetry-ext-http-requests/tests/test_requests_integration.py b/ext/opentelemetry-ext-http-requests/tests/test_requests_integration.py index fa7741695c..584e63ca22 100644 --- a/ext/opentelemetry-ext-http-requests/tests/test_requests_integration.py +++ b/ext/opentelemetry-ext-http-requests/tests/test_requests_integration.py @@ -2,16 +2,20 @@ import unittest from unittest import mock -import opentelemetry.ext.http_requests import requests import urllib3 + +import opentelemetry.ext.http_requests from opentelemetry import trace +from opentelemetry import propagator +from opentelemetry.context import Context class TestRequestsIntegration(unittest.TestCase): # TODO: Copy & paste from test_wsgi_middleware def setUp(self): + propagator.set_propagator(propagator.Propagator(Context, None, None)) self.span_attrs = {} self.tracer = trace.tracer() self.span_context_manager = mock.MagicMock() diff --git a/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py b/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py index eb37a51144..974a6998a8 100644 --- a/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py +++ b/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py @@ -22,7 +22,6 @@ import wsgiref.util as wsgiref_util from opentelemetry import trace -from opentelemetry import propagator from opentelemetry.ext.wsgi.version import __version__ # noqa diff --git a/opentelemetry-api/src/opentelemetry/propagator/__init__.py b/opentelemetry-api/src/opentelemetry/propagator/__init__.py index cc4081818b..abbde023af 100644 --- a/opentelemetry-api/src/opentelemetry/propagator/__init__.py +++ b/opentelemetry-api/src/opentelemetry/propagator/__init__.py @@ -11,14 +11,14 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -import abc -from opentelemetry import loader -import opentelemetry.propagator.httptextformat as httptextformat +import typing + import opentelemetry.propagator.binaryformat as binaryformat -from opentelemetry.context import BaseRuntimeContext, Context +import opentelemetry.propagator.httptextformat as httptextformat +from opentelemetry.context import BaseRuntimeContext -class Propagator(abc.ABC): +class Propagator: """Class which encapsulates propagation of values to and from context. In contract to using the formatters directly, a propagator object can @@ -30,26 +30,16 @@ class Propagator(abc.ABC): def __init__( self, context: BaseRuntimeContext, - http_format: httptextformat.HTTPTextFormat, - binary_format: binaryformat.BinaryFormat, + httptextformat_instance: httptextformat.HTTPTextFormat, + binaryformat_instance: binaryformat.BinaryFormat, ): self._context = context - self._http_format = http_format - self._binary_format = binary_format - - @classmethod - def create( - cls, - http_format: httptextformat.HTTPTextFormat, - binary_format: binaryformat.BinaryFormat, - ) -> "Propagator": - """Create a propagator with the current context.""" - return Propagator(Context, http_format, binary_format) - - @abc.abstractmethod + self._httptextformat = httptextformat_instance + self._binaryformat = binaryformat_instance + def extract( self, get_from_carrier: httptextformat.Getter, carrier: object - ): + ) -> None: """Extract context data from the carrier, add to the context. Using the http_format specified in the constructor, extract the @@ -65,7 +55,6 @@ def extract( which understands how to extract a value from it. """ - @abc.abstractmethod def inject( self, set_in_carrier: httptextformat.Setter, carrier: object ) -> None: @@ -84,7 +73,6 @@ def inject( know how to set header values on the carrier. """ - @abc.abstractmethod def from_bytes(self, byte_representation: bytes) -> None: """Populate context with data that existed in the byte representation. @@ -95,11 +83,10 @@ def from_bytes(self, byte_representation: bytes) -> None: byte_representation: the bytes to deserialize. """ - @abc.abstractmethod def to_bytes(self) -> bytes: """Creates a byte representation of the context configured. - to_bytes should read values from the configured context and + to_bytes should read values from the configured context and return a bytes object to represent it. Returns: @@ -110,11 +97,11 @@ def to_bytes(self) -> bytes: def set_propagator(propagator_instance: Propagator) -> None: """Set the propagator instance singleton. """ - global _PROPAGATOR + global _PROPAGATOR # pylint:disable=global-statement _PROPAGATOR = propagator_instance -def global_propagator() -> Propagator: +def global_propagator() -> typing.Optional[Propagator]: """Return the singleton propagator instance.""" return _PROPAGATOR diff --git a/opentelemetry-api/tests/test_loader.py b/opentelemetry-api/tests/test_loader.py index 9123154b41..9c75329b83 100644 --- a/opentelemetry-api/tests/test_loader.py +++ b/opentelemetry-api/tests/test_loader.py @@ -12,13 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. -from importlib import reload import os import sys import unittest +from importlib import reload -from opentelemetry import loader -from opentelemetry import trace +from opentelemetry import loader, trace DUMMY_TRACER = None diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/propagator/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/propagator/__init__.py index a43bf80e05..028caf4c20 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/propagator/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/propagator/__init__.py @@ -3,13 +3,13 @@ class Propagator(PropagatorAPI): def extract(self, get_from_carrier, carrier): - self.http_format.extract(self.context, get_from_carrier, carrier) + self._httptextformat.extract(self._context, get_from_carrier, carrier) def inject(self, set_in_carrier, carrier): - self.http_format.inject(self.context, set_in_carrier, carrier) + self._httptextformat.inject(self._context, set_in_carrier, carrier) def from_bytes(self, byte_representation): - self.binary_formatter.from_bytes(self.context, byte_representation) + self._binaryformat.from_bytes(self._context, byte_representation) def to_bytes(self): - return self.binary_formatter.to_bytes(self.context) + return self._binaryformat.to_bytes(self._context) diff --git a/opentelemetry-sdk/tests/propagator/test_b3_format.py b/opentelemetry-sdk/tests/propagator/test_b3_format.py index d65af264ed..e4803c462f 100644 --- a/opentelemetry-sdk/tests/propagator/test_b3_format.py +++ b/opentelemetry-sdk/tests/propagator/test_b3_format.py @@ -13,11 +13,11 @@ # limitations under the License. import unittest -import opentelemetry.trace as api_trace + import opentelemetry.context as context import opentelemetry.sdk.propagator.b3_format as b3_format import opentelemetry.sdk.trace as trace - +import opentelemetry.trace as api_trace from opentelemetry.sdk.trace import tracer FORMAT = b3_format.B3Format() diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 240972344c..95a178a78d 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -12,8 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -from unittest import mock import unittest +from unittest import mock from opentelemetry import trace as trace_api from opentelemetry.sdk import trace