Replaced Tracer.use_span() with opentelemetry.trace.use_span() (#364)

owais · web-flow · commit f436514554f7 · 2021-03-08T09:33:49.000-08:00
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -6,7 +6,7 @@ on:
     - 'release/*'
   pull_request:
 env:
-  CORE_REPO_SHA: 9bf28fb451a85fd9e9a4f2276c3eebd484e55d02
+  CORE_REPO_SHA: ddff32ac77e2e22b3193b71f1e71a590a99d1eda
 
 jobs:
   build:
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python-contrib/compare/v0.18b0...HEAD)
+- Updated instrumentations to use `opentelemetry.trace.use_span` instead of `Tracer.use_span()`
+  ([#364](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/364))
 
 ### Changed
 - Rename `IdsGenerator` to `IdGenerator`
diff --git a/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/aiopg_integration.py b/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/aiopg_integration.py
@@ -8,7 +8,6 @@
     DatabaseApiIntegration,
 )
 from opentelemetry.trace import SpanKind
-from opentelemetry.trace.status import Status, StatusCode
 
 
 # pylint: disable=abstract-method
@@ -117,13 +116,7 @@ async def traced_execution(
             name, kind=SpanKind.CLIENT
         ) as span:
             self._populate_span(span, cursor, *args)
-            try:
-                result = await query_method(*args, **kwargs)
-                return result
-            except Exception as ex:  # pylint: disable=broad-except
-                if span.is_recording():
-                    span.set_status(Status(StatusCode.ERROR, str(ex)))
-                raise ex
+            return await query_method(*args, **kwargs)
 
 
 def get_traced_cursor_proxy(cursor, db_api_integration, *args, **kwargs):
diff --git a/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py b/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py
@@ -289,8 +289,6 @@ def test_span_not_recording(self):
         mock_span = mock.Mock()
         mock_span.is_recording.return_value = False
         mock_tracer.start_span.return_value = mock_span
-        mock_tracer.use_span.return_value.__enter__ = mock_span
-        mock_tracer.use_span.return_value.__exit__ = True
         db_integration = AiopgIntegration(
             mock_tracer, "testcomponent", connection_attributes
         )
@@ -322,7 +320,7 @@ def test_span_failed(self):
         self.assertIs(
             span.status.status_code, trace_api.status.StatusCode.ERROR
         )
-        self.assertEqual(span.status.description, "Test Exception")
+        self.assertEqual(span.status.description, "Exception: Test Exception")
 
     def test_executemany(self):
         db_integration = AiopgIntegration(self.tracer, "testcomponent")
diff --git a/instrumentation/opentelemetry-instrumentation-boto/tests/test_boto_instrumentation.py b/instrumentation/opentelemetry-instrumentation-boto/tests/test_boto_instrumentation.py
@@ -82,8 +82,6 @@ def test_not_recording(self):
         mock_span = Mock()
         mock_span.is_recording.return_value = False
         mock_tracer.start_span.return_value = mock_span
-        mock_tracer.use_span.return_value.__enter__ = mock_span
-        mock_tracer.use_span.return_value.__exit__ = True
         with patch("opentelemetry.trace.get_tracer") as tracer:
             tracer.return_value = mock_tracer
             ec2 = boto.ec2.connect_to_region("us-west-2")
diff --git a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py
@@ -81,8 +81,6 @@ def test_not_recording(self):
         mock_span = Mock()
         mock_span.is_recording.return_value = False
         mock_tracer.start_span.return_value = mock_span
-        mock_tracer.use_span.return_value.__enter__ = mock_span
-        mock_tracer.use_span.return_value.__exit__ = True
         with patch("opentelemetry.trace.get_tracer") as tracer:
             tracer.return_value = mock_tracer
             ec2 = self.session.create_client("ec2", region_name="us-west-2")
diff --git a/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py b/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py
@@ -137,8 +137,8 @@ def _trace_prerun(self, *args, **kwargs):
             operation_name, context=tracectx, kind=trace.SpanKind.CONSUMER
         )
 
-        activation = self._tracer.use_span(span, end_on_exit=True)
-        activation.__enter__()
+        activation = trace.use_span(span, end_on_exit=True)
+        activation.__enter__()  # pylint: disable=E1101
         utils.attach_span(task, task_id, (span, activation))
 
     @staticmethod
@@ -186,8 +186,9 @@ def _trace_before_publish(self, *args, **kwargs):
             span.set_attribute(_TASK_NAME_KEY, task.name)
             utils.set_attributes_from_context(span, kwargs)
 
-        activation = self._tracer.use_span(span, end_on_exit=True)
-        activation.__enter__()
+        activation = trace.use_span(span, end_on_exit=True)
+        activation.__enter__()  # pylint: disable=E1101
+
         utils.attach_span(task, task_id, (span, activation), is_publish=True)
 
         headers = kwargs.get("headers")
@@ -208,7 +209,7 @@ def _trace_after_publish(*args, **kwargs):
             logger.warning("no existing span found for task_id=%s", task_id)
             return
 
-        activation.__exit__(None, None, None)
+        activation.__exit__(None, None, None)  # pylint: disable=E1101
         utils.detach_span(task, task_id, is_publish=True)
 
     @staticmethod
diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py
@@ -371,13 +371,7 @@ def traced_execution(
             name, kind=SpanKind.CLIENT
         ) as span:
             self._populate_span(span, cursor, *args)
-            try:
-                result = query_method(*args, **kwargs)
-                return result
-            except Exception as ex:  # pylint: disable=broad-except
-                if span.is_recording():
-                    span.set_status(Status(StatusCode.ERROR, str(ex)))
-                raise ex
+            return query_method(*args, **kwargs)
 
 
 def get_traced_cursor_proxy(cursor, db_api_integration, *args, **kwargs):
diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py
@@ -146,8 +146,6 @@ def test_span_not_recording(self):
         mock_span = mock.Mock()
         mock_span.is_recording.return_value = False
         mock_tracer.start_span.return_value = mock_span
-        mock_tracer.use_span.return_value.__enter__ = mock_span
-        mock_tracer.use_span.return_value.__exit__ = True
         db_integration = dbapi.DatabaseApiIntegration(
             mock_tracer, "testcomponent", connection_attributes
         )
@@ -179,7 +177,7 @@ def test_span_failed(self):
         self.assertIs(
             span.status.status_code, trace_api.status.StatusCode.ERROR
         )
-        self.assertEqual(span.status.description, "Test Exception")
+        self.assertEqual(span.status.description, "Exception: Test Exception")
 
     def test_executemany(self):
         db_integration = dbapi.DatabaseApiIntegration(
diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py
@@ -24,7 +24,7 @@
     collect_request_attributes,
 )
 from opentelemetry.propagate import extract
-from opentelemetry.trace import SpanKind, get_tracer
+from opentelemetry.trace import SpanKind, get_tracer, use_span
 from opentelemetry.util.http import get_excluded_urls, get_traced_request_attrs
 
 try:
@@ -118,8 +118,8 @@ def process_request(self, request):
             for key, value in attributes.items():
                 span.set_attribute(key, value)
 
-        activation = tracer.use_span(span, end_on_exit=True)
-        activation.__enter__()
+        activation = use_span(span, end_on_exit=True)
+        activation.__enter__()  # pylint: disable=E1101
 
         request.META[self._environ_activation_key] = activation
         request.META[self._environ_span_key] = span
diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py
@@ -146,8 +146,6 @@ def test_not_recording(self):
         mock_span = Mock()
         mock_span.is_recording.return_value = False
         mock_tracer.start_span.return_value = mock_span
-        mock_tracer.use_span.return_value.__enter__ = mock_span
-        mock_tracer.use_span.return_value.__exit__ = True
         with patch("opentelemetry.trace.get_tracer") as tracer:
             tracer.return_value = mock_tracer
             Client().get("/traced/")
diff --git a/instrumentation/opentelemetry-instrumentation-elasticsearch/src/opentelemetry/instrumentation/elasticsearch/__init__.py b/instrumentation/opentelemetry-instrumentation-elasticsearch/src/opentelemetry/instrumentation/elasticsearch/__init__.py
@@ -136,19 +136,15 @@ def wrapper(wrapped, _, args, kwargs):
                     attributes["elasticsearch.params"] = str(params)
                 for key, value in attributes.items():
                     span.set_attribute(key, value)
-            try:
-                rv = wrapped(*args, **kwargs)
-                if isinstance(rv, dict) and span.is_recording():
-                    for member in _ATTRIBUTES_FROM_RESULT:
-                        if member in rv:
-                            span.set_attribute(
-                                "elasticsearch.{0}".format(member),
-                                str(rv[member]),
-                            )
-                return rv
-            except Exception as ex:  # pylint: disable=broad-except
-                if span.is_recording():
-                    span.set_status(Status(StatusCode.ERROR, str(ex)))
-                raise ex
+
+            rv = wrapped(*args, **kwargs)
+            if isinstance(rv, dict) and span.is_recording():
+                for member in _ATTRIBUTES_FROM_RESULT:
+                    if member in rv:
+                        span.set_attribute(
+                            "elasticsearch.{0}".format(member),
+                            str(rv[member]),
+                        )
+            return rv
 
     return wrapper
diff --git a/instrumentation/opentelemetry-instrumentation-elasticsearch/tests/test_elasticsearch.py b/instrumentation/opentelemetry-instrumentation-elasticsearch/tests/test_elasticsearch.py
@@ -23,6 +23,7 @@
 from elasticsearch_dsl import Search
 
 import opentelemetry.instrumentation.elasticsearch
+from opentelemetry import trace
 from opentelemetry.instrumentation.elasticsearch import (
     ElasticsearchInstrumentor,
 )
@@ -94,8 +95,6 @@ def test_span_not_recording(self, request_mock):
         mock_span = mock.Mock()
         mock_span.is_recording.return_value = False
         mock_tracer.start_span.return_value = mock_span
-        mock_tracer.use_span.return_value.__enter__ = mock_span
-        mock_tracer.use_span.return_value.__exit__ = mock_span
         with mock.patch("opentelemetry.trace.get_tracer") as tracer:
             tracer.return_value = mock_tracer
             Elasticsearch()
@@ -174,7 +173,9 @@ def _test_trace_error(self, code, exc):
         span = spans[0]
         self.assertFalse(span.status.is_ok)
         self.assertEqual(span.status.status_code, code)
-        self.assertEqual(span.status.description, str(exc))
+        self.assertEqual(
+            span.status.description, "{}: {}".format(type(exc).__name__, exc)
+        )
 
     def test_parent(self, request_mock):
         request_mock.return_value = (1, {}, {})
@@ -201,7 +202,7 @@ def test_multithread(self, request_mock):
         # 2. Trace something from thread-2, make thread-1 join before finishing.
         # 3. Check the spans got different parents, and are in the expected order.
         def target1(parent_span):
-            with self.tracer.use_span(parent_span):
+            with trace.use_span(parent_span):
                 es.get(index="test-index", doc_type="tweet", id=1)
                 ev.set()
                 ev.wait()
diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py
@@ -104,6 +104,7 @@ def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
     def __call__(self, env, start_response):
+        # pylint: disable=E1101
         if _excluded_urls.url_disabled(env.get("PATH_INFO", "/")):
             return super().__call__(env, start_response)
 
@@ -120,7 +121,7 @@ def __call__(self, env, start_response):
             for key, value in attributes.items():
                 span.set_attribute(key, value)
 
-        activation = self._tracer.use_span(span, end_on_exit=True)
+        activation = trace.use_span(span, end_on_exit=True)
         activation.__enter__()
         env[_ENVIRON_SPAN_KEY] = span
         env[_ENVIRON_ACTIVATION_KEY] = activation
diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py
@@ -199,8 +199,6 @@ def test_traced_not_recording(self):
         mock_span = Mock()
         mock_span.is_recording.return_value = False
         mock_tracer.start_span.return_value = mock_span
-        mock_tracer.use_span.return_value.__enter__ = mock_span
-        mock_tracer.use_span.return_value.__exit__ = mock_span
         with patch("opentelemetry.trace.get_tracer") as tracer:
             tracer.return_value = mock_tracer
             self.client().simulate_get(path="/hello?q=abc")
diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py
@@ -138,8 +138,8 @@ def _before_request():
             for key, value in attributes.items():
                 span.set_attribute(key, value)
 
-        activation = tracer.use_span(span, end_on_exit=True)
-        activation.__enter__()
+        activation = trace.use_span(span, end_on_exit=True)
+        activation.__enter__()  # pylint: disable=E1101
         flask_request_environ[_ENVIRON_ACTIVATION_KEY] = activation
         flask_request_environ[_ENVIRON_SPAN_KEY] = span
         flask_request_environ[_ENVIRON_TOKEN] = token
@@ -148,6 +148,7 @@ def _before_request():
 
 
 def _teardown_request(exc):
+    # pylint: disable=E1101
     if _excluded_urls.url_disabled(flask.request.url):
         return
 
diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py
@@ -125,8 +125,6 @@ def test_not_recording(self):
         mock_span = Mock()
         mock_span.is_recording.return_value = False
         mock_tracer.start_span.return_value = mock_span
-        mock_tracer.use_span.return_value.__enter__ = mock_span
-        mock_tracer.use_span.return_value.__exit__ = mock_span
         with patch("opentelemetry.trace.get_tracer") as tracer:
             tracer.return_value = mock_tracer
             self.client.get("/hello/123")
diff --git a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py
@@ -191,7 +191,9 @@ def _set_remote_context(self, servicer_context):
         else:
             yield
 
-    def _start_span(self, handler_call_details, context):
+    def _start_span(
+        self, handler_call_details, context, set_status_on_exception=False
+    ):
 
         # standard attributes
         attributes = {
@@ -234,6 +236,7 @@ def _start_span(self, handler_call_details, context):
             name=handler_call_details.method,
             kind=trace.SpanKind.SERVER,
             attributes=attributes,
+            set_status_on_exception=set_status_on_exception,
         )
 
     def intercept_service(self, continuation, handler_call_details):
@@ -251,7 +254,9 @@ def telemetry_interceptor(request_or_iterator, context):
 
                 with self._set_remote_context(context):
                     with self._start_span(
-                        handler_call_details, context
+                        handler_call_details,
+                        context,
+                        set_status_on_exception=False,
                     ) as span:
                         # wrap the context
                         context = _OpenTelemetryServicerContext(context, span)
@@ -283,7 +288,9 @@ def _intercept_server_stream(
     ):
 
         with self._set_remote_context(context):
-            with self._start_span(handler_call_details, context) as span:
+            with self._start_span(
+                handler_call_details, context, set_status_on_exception=False
+            ) as span:
                 context = _OpenTelemetryServicerContext(context, span)
 
                 try:
diff --git a/instrumentation/opentelemetry-instrumentation-jinja2/tests/test_jinja2.py b/instrumentation/opentelemetry-instrumentation-jinja2/tests/test_jinja2.py
@@ -59,8 +59,6 @@ def test_render_not_recording(self):
         mock_span = mock.Mock()
         mock_span.is_recording.return_value = False
         mock_tracer.start_span.return_value = mock_span
-        mock_tracer.use_span.return_value.__enter__ = mock_span
-        mock_tracer.use_span.return_value.__exit__ = mock_span
         with mock.patch("opentelemetry.trace.get_tracer") as tracer:
             tracer.return_value = mock_tracer
             jinja2.environment.Template("Hello {{name}}!")
diff --git a/instrumentation/opentelemetry-instrumentation-psycopg2/tests/test_psycopg2_integration.py b/instrumentation/opentelemetry-instrumentation-psycopg2/tests/test_psycopg2_integration.py
@@ -121,8 +121,6 @@ def test_not_recording(self):
         mock_span = mock.Mock()
         mock_span.is_recording.return_value = False
         mock_tracer.start_span.return_value = mock_span
-        mock_tracer.use_span.return_value.__enter__ = mock_span
-        mock_tracer.use_span.return_value.__exit__ = True
         Psycopg2Instrumentor().instrument()
         with mock.patch("opentelemetry.trace.get_tracer") as tracer:
             tracer.return_value = mock_tracer
diff --git a/instrumentation/opentelemetry-instrumentation-pymemcache/tests/test_pymemcache.py b/instrumentation/opentelemetry-instrumentation-pymemcache/tests/test_pymemcache.py
@@ -84,8 +84,6 @@ def test_set_not_recording(self):
         mock_span = mock.Mock()
         mock_span.is_recording.return_value = False
         mock_tracer.start_span.return_value = mock_span
-        mock_tracer.use_span.return_value.__enter__ = mock_span
-        mock_tracer.use_span.return_value.__exit__ = True
         with mock.patch("opentelemetry.trace.get_tracer") as tracer:
             tracer.return_value = mock_tracer
             client = self.make_client([b"STORED\r\n"])
diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py b/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py
@@ -76,8 +76,6 @@ def test_not_recording(self):
         mock_span = mock.Mock()
         mock_span.is_recording.return_value = False
         mock_tracer.start_span.return_value = mock_span
-        mock_tracer.use_span.return_value.__enter__ = mock_span
-        mock_tracer.use_span.return_value.__exit__ = True
         mock_event = MockEvent({})
         command_tracer = CommandTracer(mock_tracer)
         command_tracer.started(event=mock_event)
diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py
@@ -83,8 +83,8 @@ def _before_traversal(event):
         for key, value in attributes.items():
             span.set_attribute(key, value)
 
-    activation = tracer.use_span(span, end_on_exit=True)
-    activation.__enter__()
+    activation = trace.use_span(span, end_on_exit=True)
+    activation.__enter__()  # pylint: disable=E1101
     request_environ[_ENVIRON_ACTIVATION_KEY] = activation
     request_environ[_ENVIRON_SPAN_KEY] = span
     request_environ[_ENVIRON_TOKEN] = token
@@ -105,6 +105,7 @@ def disabled_tween(request):
 
     # make a request tracing function
     def trace_tween(request):
+        # pylint: disable=E1101
         if _excluded_urls.url_disabled(request.url):
             request.environ[_ENVIRON_ENABLED_KEY] = False
             # short-circuit when we don't want to trace anything
diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py
@@ -104,8 +104,6 @@ def test_not_recording(self):
         mock_span = Mock()
         mock_span.is_recording.return_value = False
         mock_tracer.start_span.return_value = mock_span
-        mock_tracer.use_span.return_value.__enter__ = mock_span
-        mock_tracer.use_span.return_value.__exit__ = True
         with patch("opentelemetry.trace.get_tracer"):
             self.client.get("/hello/123")
             span_list = self.memory_exporter.get_finished_spans()
diff --git a/instrumentation/opentelemetry-instrumentation-redis/tests/test_redis.py b/instrumentation/opentelemetry-instrumentation-redis/tests/test_redis.py
diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py
diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py
diff --git a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py
diff --git a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py
diff --git a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py
diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py
diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py
