Use is_recording flag in requests instrumentation (#1087)

Author: lzchen

Diff for: 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 for: 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 for: 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