From d376df1b3f2b0ff234b40d41a71a332188fbeaf0 Mon Sep 17 00:00:00 2001
From: Leighton Chen <lechen@microsoft.com>
Date: Mon, 14 Sep 2020 13:13:44 -0700
Subject: [PATCH] Use is_recording flag in requests instrumentation (#1087)

---
 .../instrumentation/requests/__init__.py        | 15 ++++++++-------
 .../tests/test_requests_integration.py          | 17 +++++++++++++++++
 tests/util/setup.cfg                            |  1 +
 3 files changed, 26 insertions(+), 7 deletions(-)

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 16e8952fea4..fef67c5d0da 100644
--- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py
+++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py
@@ -56,8 +56,8 @@
 # pylint: disable=unused-argument
 def _instrument(tracer_provider=None, span_callback=None):
     """Enables tracing of all requests calls that go through
-      :code:`requests.session.Session.request` (this includes
-      :code:`requests.get`, etc.)."""
+    :code:`requests.session.Session.request` (this includes
+    :code:`requests.get`, etc.)."""
 
     # Since
     # https://github.com/psf/requests/commit/d72d1162142d1bf8b1b5711c664fbbd674f349d1
@@ -121,9 +121,10 @@ def _instrumented_requests_call(
         with get_tracer(
             __name__, __version__, tracer_provider
         ).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)
+            if span.is_recording():
+                span.set_attribute("component", "http")
+                span.set_attribute("http.method", method.upper())
+                span.set_attribute("http.url", url)
 
             headers = get_or_create_headers()
             propagators.inject(type(headers).__setitem__, headers)
@@ -139,13 +140,13 @@ def _instrumented_requests_call(
             finally:
                 context.detach(token)
 
-            if exception is not None:
+            if exception is not None and span.is_recording():
                 span.set_status(
                     Status(_exception_to_canonical_code(exception))
                 )
                 span.record_exception(exception)
 
-            if result is not None:
+            if result is not None and span.is_recording():
                 span.set_attribute("http.status_code", result.status_code)
                 span.set_attribute("http.status_text", result.reason)
                 span.set_status(
diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py
index 41f5bc39d96..c3457b73923 100644
--- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py
+++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py
@@ -147,6 +147,23 @@ def test_suppress_instrumentation(self):
 
         self.assert_span(num_spans=0)
 
+    def test_not_recording(self):
+        with mock.patch("opentelemetry.trace.INVALID_SPAN") as mock_span:
+            RequestsInstrumentor().uninstrument()
+            # original_tracer_provider returns a default tracer provider, which
+            # in turn will return an INVALID_SPAN, which is always not recording
+            RequestsInstrumentor().instrument(
+                tracer_provider=self.original_tracer_provider
+            )
+            mock_span.is_recording.return_value = False
+            result = self.perform_request(self.URL)
+            self.assertEqual(result.text, "Hello!")
+            self.assert_span(None, 0)
+            self.assertFalse(mock_span.is_recording())
+            self.assertTrue(mock_span.is_recording.called)
+            self.assertFalse(mock_span.set_attribute.called)
+            self.assertFalse(mock_span.set_status.called)
+
     def test_distributed_context(self):
         previous_propagator = propagators.get_global_textmap()
         try:
diff --git a/tests/util/setup.cfg b/tests/util/setup.cfg
index b68c5e3a623..cfe66aa7dfe 100644
--- a/tests/util/setup.cfg
+++ b/tests/util/setup.cfg
@@ -39,6 +39,7 @@ package_dir=
 packages=find_namespace:
 install_requires =
     opentelemetry-api
+    opentelemetry-sdk
 
 [options.extras_require]
 test = flask~=1.0