diff --git a/ext/opentelemetry-ext-requests/src/opentelemetry/ext/requests/__init__.py b/ext/opentelemetry-ext-requests/src/opentelemetry/ext/requests/__init__.py index 1621c4a95e6..65bc21370f6 100644 --- a/ext/opentelemetry-ext-requests/src/opentelemetry/ext/requests/__init__.py +++ b/ext/opentelemetry-ext-requests/src/opentelemetry/ext/requests/__init__.py @@ -45,12 +45,14 @@ import types from urllib.parse import urlparse +from requests import Timeout, URLRequired +from requests.exceptions import InvalidSchema, InvalidURL, MissingSchema from requests.sessions import Session from opentelemetry import context, propagators, trace from opentelemetry.auto_instrumentation.instrumentor import BaseInstrumentor from opentelemetry.ext.requests.version import __version__ -from opentelemetry.trace import SpanKind, get_tracer +from opentelemetry.trace import SpanKind from opentelemetry.trace.status import Status, StatusCanonicalCode @@ -80,31 +82,49 @@ def instrumented_request(self, method, url, *args, **kwargs): # https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/http.md#http-client try: parsed_url = urlparse(url) + span_name = parsed_url.path except ValueError as exc: # Invalid URL - path = "".format(exc) - else: - if parsed_url is None: - path = "" - path = parsed_url.path + span_name = "".format(exc) - with tracer.start_as_current_span(path, kind=SpanKind.CLIENT) as span: + exception = None + + with tracer.start_as_current_span( + span_name, kind=SpanKind.CLIENT + ) as span: span.set_attribute("component", "http") span.set_attribute("http.method", method.upper()) span.set_attribute("http.url", url) headers = kwargs.setdefault("headers", {}) propagators.inject(type(headers).__setitem__, headers) - result = wrapped(self, method, url, *args, **kwargs) # *** PROCEED - span.set_attribute("http.status_code", result.status_code) - span.set_attribute("http.status_text", result.reason) - span.set_status( - Status(_http_status_to_canonical_code(result.status_code)) - ) + try: + result = wrapped( + self, method, url, *args, **kwargs + ) # *** PROCEED + except Exception as exc: # pylint: disable=W0703 + exception = exc + result = getattr(exc, "response", None) + + if exception is not None: + span.set_status( + Status(_exception_to_canonical_code(exception)) + ) + + if result is not None: + span.set_attribute("http.status_code", result.status_code) + span.set_attribute("http.status_text", result.reason) + span.set_status( + Status(_http_status_to_canonical_code(result.status_code)) + ) + if span_callback is not None: span_callback(span, result) - return result + if exception is not None: + raise exception.with_traceback(exception.__traceback__) + + return result instrumented_request.opentelemetry_ext_requests_applied = True @@ -157,6 +177,17 @@ def _http_status_to_canonical_code(code: int, allow_redirect: bool = True): return StatusCanonicalCode.UNKNOWN +def _exception_to_canonical_code(exc: Exception) -> StatusCanonicalCode: + if isinstance( + exc, + (InvalidURL, InvalidSchema, MissingSchema, URLRequired, ValueError), + ): + return StatusCanonicalCode.INVALID_ARGUMENT + if isinstance(exc, Timeout): + return StatusCanonicalCode.DEADLINE_EXCEEDED + return StatusCanonicalCode.UNKNOWN + + class RequestsInstrumentor(BaseInstrumentor): """An instrumentor for requests See `BaseInstrumentor` diff --git a/ext/opentelemetry-ext-requests/tests/test_requests_integration.py b/ext/opentelemetry-ext-requests/tests/test_requests_integration.py index 28359d8f38a..0ad5b9d19df 100644 --- a/ext/opentelemetry-ext-requests/tests/test_requests_integration.py +++ b/ext/opentelemetry-ext-requests/tests/test_requests_integration.py @@ -12,11 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. -import sys +from unittest import mock import httpretty import requests -import urllib3 import opentelemetry.ext.requests from opentelemetry import context, propagators, trace @@ -24,6 +23,7 @@ from opentelemetry.sdk import resources from opentelemetry.test.mock_httptextformat import MockHTTPTextFormat from opentelemetry.test.test_base import TestBase +from opentelemetry.trace.status import StatusCanonicalCode class TestRequestsIntegration(TestBase): @@ -92,13 +92,8 @@ def test_not_foundbasic(self): def test_invalid_url(self): url = "http://[::1/nope" - exception_type = requests.exceptions.InvalidURL - if sys.version_info[:2] < (3, 5) and tuple( - map(int, urllib3.__version__.split(".")[:2]) - ) < (1, 25): - exception_type = ValueError - with self.assertRaises(exception_type): + with self.assertRaises(ValueError): requests.post(url) span_list = self.memory_exporter.get_finished_spans() @@ -110,6 +105,9 @@ def test_invalid_url(self): span.attributes, {"component": "http", "http.method": "POST", "http.url": url}, ) + self.assertEqual( + span.status.canonical_code, StatusCanonicalCode.INVALID_ARGUMENT + ) def test_uninstrument(self): RequestsInstrumentor().uninstrument() @@ -229,3 +227,75 @@ def test_custom_tracer_provider(self): span = span_list[0] self.assertIs(span.resource, resource) + + @mock.patch("requests.Session.send", side_effect=requests.RequestException) + def test_requests_exception_without_response(self, *_, **__): + + with self.assertRaises(requests.RequestException): + requests.get(self.URL) + + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + span = span_list[0] + self.assertEqual( + span.attributes, + {"component": "http", "http.method": "GET", "http.url": self.URL}, + ) + self.assertEqual( + span.status.canonical_code, StatusCanonicalCode.UNKNOWN + ) + + mocked_response = requests.Response() + mocked_response.status_code = 500 + mocked_response.reason = "Internal Server Error" + + @mock.patch( + "requests.Session.send", + side_effect=requests.RequestException(response=mocked_response), + ) + def test_requests_exception_with_response(self, *_, **__): + + with self.assertRaises(requests.RequestException): + requests.get(self.URL) + + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + span = span_list[0] + self.assertEqual( + span.attributes, + { + "component": "http", + "http.method": "GET", + "http.url": self.URL, + "http.status_code": 500, + "http.status_text": "Internal Server Error", + }, + ) + self.assertEqual( + span.status.canonical_code, StatusCanonicalCode.INTERNAL + ) + + @mock.patch("requests.Session.send", side_effect=Exception) + def test_requests_basic_exception(self, *_, **__): + + with self.assertRaises(Exception): + requests.get(self.URL) + + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + self.assertEqual( + span_list[0].status.canonical_code, StatusCanonicalCode.UNKNOWN + ) + + @mock.patch("requests.Session.send", side_effect=requests.Timeout) + def test_requests_timeout_exception(self, *_, **__): + + with self.assertRaises(Exception): + requests.get(self.URL) + + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + self.assertEqual( + span_list[0].status.canonical_code, + StatusCanonicalCode.DEADLINE_EXCEEDED, + )