Skip to content

Commit

Permalink
Fix RequestsInstrumentor for custom transport adapters (#562)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
mariojonke committed Jul 14, 2021
1 parent 9206e5d commit 2ccf120
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 13 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#558](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/558))
- Change `opentelemetry-instrumentation-httpx` to replace `client` classes with instrumented versions.
([#577](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/577))
- `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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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__()
Expand All @@ -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"

Expand Down Expand Up @@ -335,6 +355,26 @@ 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):
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
Expand Down

0 comments on commit 2ccf120

Please sign in to comment.