From 0e7fc3d352550d86737b177874d28abd31cf55bc Mon Sep 17 00:00:00 2001 From: Mario Jonke Date: Fri, 2 Jul 2021 08:36:23 +0200 Subject: [PATCH 1/3] Fix RequestsInstrumentor for custom transport adapters remove dead/leftover code from an early metrics implementation which tried to access the raw.version attribute on the response object. The 'version' attribute might not be present in every case, especially when custom transport adapters are used. --- .../instrumentation/requests/__init__.py | 13 ------ .../tests/test_requests_integration.py | 41 +++++++++++++++++++ 2 files changed, 41 insertions(+), 13 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index b844264d85..73c81e1de5 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -131,10 +131,6 @@ def _instrumented_requests_call( url = remove_url_credentials(url) - labels = {} - labels[SpanAttributes.HTTP_METHOD] = method - labels[SpanAttributes.HTTP_URL] = url - with tracer.start_as_current_span( span_name, kind=SpanKind.CLIENT ) as span: @@ -165,15 +161,6 @@ def _instrumented_requests_call( span.set_status( Status(http_status_to_status_code(result.status_code)) ) - labels[SpanAttributes.HTTP_STATUS_CODE] = str( - result.status_code - ) - if result.raw and result.raw.version: - labels[SpanAttributes.HTTP_FLAVOR] = ( - str(result.raw.version)[:1] - + "." - + str(result.raw.version)[:-1] - ) if span_callback is not None: span_callback(span, result) diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index e00bd3bd4f..44cef3a1a2 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -17,6 +17,8 @@ import httpretty import requests +from requests.adapters import BaseAdapter +from requests.models import Response import opentelemetry.instrumentation.requests from opentelemetry import context, trace @@ -30,6 +32,23 @@ from opentelemetry.trace import StatusCode +class TransportMock: + def read(self, *args, **kwargs): + pass + + +class MyAdapter(BaseAdapter): + def __init__(self, response): + super().__init__() + self._response = response + + def send(self, *args, **kwargs): # pylint:disable=signature-differs + return self._response + + def close(self): + pass + + class InvalidResponseObjectException(Exception): def __init__(self): super().__init__() @@ -38,6 +57,7 @@ def __init__(self): class RequestsIntegrationTestBase(abc.ABC): # pylint: disable=no-member + # pylint: disable=too-many-public-methods URL = "http://httpbin.org/status/200" @@ -335,6 +355,27 @@ def test_requests_timeout_exception(self, *_, **__): span = self.assert_span() self.assertEqual(span.status.status_code, StatusCode.ERROR) + def test_adapter_with_custom_response(self): + # See ONE-56722 + response = Response() + response.status_code = 210 + response.reason = "hello adapter" + response.raw = TransportMock() + + session = requests.Session() + session.mount(self.URL, MyAdapter(response)) + + self.perform_request(self.URL, session) + span = self.assert_span() + self.assertEqual( + span.attributes, + { + "http.method": "GET", + "http.url": self.URL, + "http.status_code": 210, + }, + ) + class TestRequestsIntegration(RequestsIntegrationTestBase, TestBase): @staticmethod From feab3de86ecdf34628c08af92ef554913d97cbc4 Mon Sep 17 00:00:00 2001 From: Mario Jonke Date: Fri, 2 Jul 2021 09:26:15 +0200 Subject: [PATCH 2/3] changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3713fc9333..288fc0297e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#543](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/543)) - Require aiopg to be less than 1.3.0 ([#560](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/560)) +- `opentelemetry-instrumentation-requests` Fix potential `AttributeError` when `requests` + is used with a custom transport adapter. + ([#562](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/562)) ### Added - `opentelemetry-instrumentation-httpx` Add `httpx` instrumentation From a06f4758a984dbc391d136291bd17cb55169f035 Mon Sep 17 00:00:00 2001 From: Mario Jonke Date: Fri, 9 Jul 2021 07:03:01 +0200 Subject: [PATCH 3/3] remove comment --- .../tests/test_requests_integration.py | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index 44cef3a1a2..db9a5e7625 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -356,7 +356,6 @@ def test_requests_timeout_exception(self, *_, **__): self.assertEqual(span.status.status_code, StatusCode.ERROR) def test_adapter_with_custom_response(self): - # See ONE-56722 response = Response() response.status_code = 210 response.reason = "hello adapter"