Skip to content

Commit

Permalink
Use instanceof to check if responses are valid Response objects
Browse files Browse the repository at this point in the history
There are some exception types that set the response attribute to a
non-Response type, e.g. "AttributeError: 'dict' object has no attribute
'status_code'"
  • Loading branch information
David Grochowski committed Dec 23, 2020
1 parent 65801c3 commit b800adc
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#216](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/216))
- `opentelemetry-instrumentation-grpc` Add tests for grpc span attributes, grpc `abort()` conditions
([#236](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/236))
- `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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@

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

Expand Down Expand Up @@ -158,7 +159,7 @@ def _instrumented_requests_call(
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@
from opentelemetry.trace.status import StatusCode


class InvalidResponseObjectException(Exception):
def __init__(self):
super().__init__()
self.response = {}


class RequestsIntegrationTestBase(abc.ABC):
# pylint: disable=no-member

Expand Down Expand Up @@ -307,6 +313,42 @@ def test_requests_exception_without_response(self, *_, **__):
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),
Expand Down

0 comments on commit b800adc

Please sign in to comment.