diff --git a/CHANGELOG.md b/CHANGELOG.md index 52b8b0e33..dbff5818b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#227](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/227])) - `opentelemetry-instrumentation-botocore` Adds a field to report the number of retries it take to complete an API call ([#275](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/275)) +- `opentelemetry-instrumentation-requests` Use instanceof to check if responses are valid Response objects + ([#273](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/273)) ### Changed - `opentelemetry-instrumentation-asgi`, `opentelemetry-instrumentation-wsgi` Return `None` for `CarrierGetter` if key not found 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 993309c7e..13f6d8448 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -38,6 +38,7 @@ import types from requests import Timeout, URLRequired from requests.exceptions import InvalidSchema, InvalidURL, MissingSchema +from requests.models import Response from requests.sessions import Session from requests.structures import CaseInsensitiveDict @@ -158,7 +159,7 @@ def _instrument(tracer_provider=None, span_callback=None, name_callback=None): finally: context.detach(token) - if result is not None: + if isinstance(result, Response): if span.is_recording(): span.set_attribute( "http.status_code", result.status_code diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index e871ecfba..7749c1f8a 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -28,6 +28,12 @@ from opentelemetry.test.test_base import TestBase from opentelemetry.trace.status import StatusCode +class InvalidResponseObjectException(Exception): + def __init__(self): + super().__init__() + self.response = {} + + class RequestsIntegrationTestBase(abc.ABC): # pylint: disable=no-member @@ -307,6 +313,42 @@ class RequestsIntegrationTestBase(abc.ABC): mocked_response.status_code = 500 mocked_response.reason = "Internal Server Error" + @mock.patch( + "requests.adapters.HTTPAdapter.send", + side_effect=InvalidResponseObjectException, + ) + def test_requests_exception_without_proper_response_type(self, *_, **__): + with self.assertRaises(InvalidResponseObjectException): + self.perform_request(self.URL) + + span = self.assert_span() + self.assertEqual( + span.attributes, + {"component": "http", "http.method": "GET", "http.url": self.URL}, + ) + self.assertEqual(span.status.status_code, StatusCode.ERROR) + + self.assertIsNotNone(RequestsInstrumentor().meter) + self.assertEqual(len(RequestsInstrumentor().meter.instruments), 1) + recorder = list(RequestsInstrumentor().meter.instruments.values())[0] + match_key = get_dict_as_key( + { + "http.method": "GET", + "http.url": "http://httpbin.org/status/200", + } + ) + for key in recorder.bound_instruments.keys(): + self.assertEqual(key, match_key) + # pylint: disable=protected-access + bound = recorder.bound_instruments.get(key) + for view_data in bound.view_datas: + self.assertEqual(view_data.labels, key) + self.assertEqual(view_data.aggregator.current.count, 1) + + mocked_response = requests.Response() + mocked_response.status_code = 500 + mocked_response.reason = "Internal Server Error" + @mock.patch( "requests.adapters.HTTPAdapter.send", side_effect=requests.RequestException(response=mocked_response),