Mark span as internal in presence of another span for tornado and use (#889)

_start_internal_or_server_span function for flask, pyramid and django

Adding changelog entry

Adding unit test and fixing lint errors

Refactoring to use _start_internal_or_server_span function

Removing unwanted imports and variables

Fixing lint errors

adding changes from tox -e generate

Fixing build errors

CHANGELOG.md

@@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - `opentelemetry-instrumentation-pika` requires `packaging` dependency
 
+- `opentelemetry-instrumentation-tornado` Tornado: Conditionally create SERVER spans
+  ([#867](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/889))
 
 ## [1.9.0-0.28b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.9.0-0.28b0) - 2022-01-26
 

instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py

@@ -20,25 +20,21 @@ from typing import Callable
 from django import VERSION as django_version
 from django.http import HttpRequest, HttpResponse
 
-from opentelemetry.context import attach, detach
+from opentelemetry.context import detach
 from opentelemetry.instrumentation.propagators import (
     get_global_response_propagator,
 )
-from opentelemetry.instrumentation.utils import extract_attributes_from_object
+from opentelemetry.instrumentation.utils import (
+    _start_internal_or_server_span,
+    extract_attributes_from_object,
+)
 from opentelemetry.instrumentation.wsgi import add_response_attributes
 from opentelemetry.instrumentation.wsgi import (
     collect_request_attributes as wsgi_collect_request_attributes,
 )
 from opentelemetry.instrumentation.wsgi import wsgi_getter
-from opentelemetry.propagate import extract
 from opentelemetry.semconv.trace import SpanAttributes
-from opentelemetry.trace import (
-    INVALID_SPAN,
-    Span,
-    SpanKind,
-    get_current_span,
-    use_span,
-)
+from opentelemetry.trace import Span, use_span
 from opentelemetry.util.http import get_excluded_urls, get_traced_request_attrs
 
 try:
@@ -189,19 +185,14 @@ class _DjangoMiddleware(MiddlewareMixin):
             carrier_getter = wsgi_getter
             collect_request_attributes = wsgi_collect_request_attributes
 
-        token = context = None
-        span_kind = SpanKind.INTERNAL
-        if get_current_span() is INVALID_SPAN:
-            context = extract(carrier, getter=carrier_getter)
-            token = attach(context)
-            span_kind = SpanKind.SERVER
-        span = self._tracer.start_span(
-            self._get_span_name(request),
-            context,
-            kind=span_kind,
+        span, token = _start_internal_or_server_span(
+            tracer=self._tracer,
+            span_name=self._get_span_name(request),
             start_time=request_meta.get(
                 "opentelemetry-instrumentor-django.starttime_key"
             ),
+            context_carrier=carrier,
+            context_getter=carrier_getter,
         )
 
         attributes = collect_request_attributes(carrier)

instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py

@@ -102,7 +102,7 @@ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
 from opentelemetry.instrumentation.propagators import (
     get_global_response_propagator,
 )
-from opentelemetry.propagate import extract
+from opentelemetry.instrumentation.utils import _start_internal_or_server_span
 from opentelemetry.semconv.trace import SpanAttributes
 from opentelemetry.util._time import _time_ns
 from opentelemetry.util.http import get_excluded_urls, parse_excluded_urls
@@ -177,21 +177,12 @@ def _wrapped_before_request(
         flask_request_environ = flask.request.environ
         span_name = get_default_span_name()
 
-        token = ctx = span_kind = None
-
-        if trace.get_current_span() is trace.INVALID_SPAN:
-            ctx = extract(flask_request_environ, getter=otel_wsgi.wsgi_getter)
-            token = context.attach(ctx)
-            span_kind = trace.SpanKind.SERVER
-        else:
-            ctx = context.get_current()
-            span_kind = trace.SpanKind.INTERNAL
-
-        span = tracer.start_span(
-            span_name,
-            ctx,
-            kind=span_kind,
+        span, token = _start_internal_or_server_span(
+            tracer=tracer,
+            span_name=span_name,
             start_time=flask_request_environ.get(_ENVIRON_STARTTIME_KEY),
+            context_carrier=flask_request_environ,
+            context_getter=otel_wsgi.wsgi_getter,
         )
 
         if request_hook:

instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py

@@ -25,7 +25,7 @@ from opentelemetry.instrumentation.propagators import (
     get_global_response_propagator,
 )
 from opentelemetry.instrumentation.pyramid.version import __version__
-from opentelemetry.propagate import extract
+from opentelemetry.instrumentation.utils import _start_internal_or_server_span
 from opentelemetry.semconv.trace import SpanAttributes
 from opentelemetry.util._time import _time_ns
 from opentelemetry.util.http import get_excluded_urls
@@ -81,9 +81,6 @@ def _before_traversal(event):
         return
 
     start_time = request_environ.get(_ENVIRON_STARTTIME_KEY)
-
-    token = ctx = None
-    span_kind = trace.SpanKind.INTERNAL
     tracer = trace.get_tracer(__name__, __version__)
 
     if request.matched_route:
@@ -91,15 +88,12 @@ def _before_traversal(event):
     else:
         span_name = otel_wsgi.get_default_span_name(request_environ)
 
-    if trace.get_current_span() is trace.INVALID_SPAN:
-        ctx = extract(request_environ, getter=otel_wsgi.wsgi_getter)
-        token = context.attach(ctx)
-        span_kind = trace.SpanKind.SERVER
-    span = tracer.start_span(
-        span_name,
-        ctx,
-        kind=span_kind,
+    span, token = _start_internal_or_server_span(
+        tracer=tracer,
+        span_name=span_name,
         start_time=start_time,
+        context_carrier=request_environ,
+        context_getter=otel_wsgi.wsgi_getter,
     )
 
     if span.is_recording():

instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py

@@ -119,11 +119,12 @@ from opentelemetry.instrumentation.propagators import (
 from opentelemetry.instrumentation.tornado.package import _instruments
 from opentelemetry.instrumentation.tornado.version import __version__
 from opentelemetry.instrumentation.utils import (
+    _start_internal_or_server_span,
     extract_attributes_from_object,
     http_status_to_status_code,
     unwrap,
 )
-from opentelemetry.propagate import extract
+from opentelemetry.propagators import textmap
 from opentelemetry.semconv.trace import SpanAttributes
 from opentelemetry.trace.status import Status, StatusCode
 from opentelemetry.util._time import _time_ns
@@ -292,13 +293,14 @@ def _get_full_handler_name(handler):
 
 
 def _start_span(tracer, handler, start_time) -> _TraceContext:
-    token = context.attach(extract(handler.request.headers))
-
-    span = tracer.start_span(
-        _get_operation_name(handler, handler.request),
-        kind=trace.SpanKind.SERVER,
+    span, token = _start_internal_or_server_span(
+        tracer=tracer,
+        span_name=_get_operation_name(handler, handler.request),
         start_time=start_time,
+        context_carrier=handler.request.headers,
+        context_getter=textmap.default_getter,
     )
+
     if span.is_recording():
         attributes = _get_attributes_from_request(handler.request)
         for key, value in attributes.items():
@@ -359,5 +361,6 @@ def _finish_span(tracer, handler, error=None):
         )
 
     ctx.activation.__exit__(*finish_args)  # pylint: disable=E1101
-    context.detach(ctx.token)
+    if ctx.token:
+        context.detach(ctx.token)
     delattr(handler, _HANDLER_CONTEXT_KEY)

instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py

@@ -574,3 +574,33 @@ class TestTornadoUninstrument(TornadoTest):
         self.assertEqual(len(spans), 1)
         manual = spans[0]
         self.assertEqual(manual.name, "manual")
+
+
+class TestTornadoWrappedWithOtherFramework(TornadoTest):
+    def get_app(self):
+        tracer = trace.get_tracer(__name__)
+        app = make_app(tracer)
+
+        def middleware(request):
+            """Wraps the request with a server span"""
+            with tracer.start_as_current_span(
+                "test", kind=trace.SpanKind.SERVER
+            ):
+                app(request)
+
+        return middleware
+
+    def test_mark_span_internal_in_presence_of_another_span(self):
+        response = self.fetch("/")
+        self.assertEqual(response.code, 201)
+        spans = self.sorted_spans(self.memory_exporter.get_finished_spans())
+        self.assertEqual(len(spans), 4)
+
+        tornado_handler_span = spans[1]
+        self.assertEqual(trace.SpanKind.INTERNAL, tornado_handler_span.kind)
+
+        test_span = spans[2]
+        self.assertEqual(trace.SpanKind.SERVER, test_span.kind)
+        self.assertEqual(
+            test_span.context.span_id, tornado_handler_span.parent.span_id
+        )