From 2ccf12055e81ba3c7bcd2043200ea41db43ae20a Mon Sep 17 00:00:00 2001
From: Mario Jonke <mario.jonke@dynatrace.com>
Date: Wed, 14 Jul 2021 19:51:09 +0200
Subject: [PATCH] Fix RequestsInstrumentor for custom transport adapters (#562)

* 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.
---
 CHANGELOG.md                                  |  3 ++
 .../instrumentation/requests/__init__.py      | 13 ------
 .../tests/test_requests_integration.py        | 40 +++++++++++++++++++
 3 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5ccc90d0c..df68c6593 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
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 b844264d8..73c81e1de 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 _instrument(tracer, span_callback=None, name_callback=None):
 
         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 _instrument(tracer, span_callback=None, name_callback=None):
                     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 e00bd3bd4..db9a5e762 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 @@ from unittest import mock
 
 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.test.test_base import TestBase
 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 @@ class InvalidResponseObjectException(Exception):
 
 class RequestsIntegrationTestBase(abc.ABC):
     # pylint: disable=no-member
+    # pylint: disable=too-many-public-methods
 
     URL = "http://httpbin.org/status/200"
 
@@ -335,6 +355,26 @@ class RequestsIntegrationTestBase(abc.ABC):
         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