Store ScopeShim in context

This uses the OpenTelemetry context management mechanism to store a
ScopeShim object in order to make active return the same object as the
one returned by start_active_span.

WIP: there is one failing test case, apparently the context does not get
cleared before running every test case. This seems related to the order
in which test cases are being run.

ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py

@@ -92,6 +92,7 @@
 
 from opentelemetry import propagators
 from opentelemetry.context import Context
+from opentelemetry.context import attach, detach, get_value, set_value
 from opentelemetry.ext.opentracing_shim import util
 from opentelemetry.ext.opentracing_shim.version import __version__
 from opentelemetry.trace import (
@@ -327,6 +328,7 @@ class ScopeShim(opentracing.Scope):
     def __init__(self, manager, span, span_cm=None):
         super().__init__(manager, span)
         self._span_cm = span_cm
+        self._token = attach(set_value("scope_shim", self))
 
     # TODO: Change type of `manager` argument to `opentracing.ScopeManager`? We
     # need to get rid of `manager.tracer` for this.
@@ -392,6 +394,8 @@ def close(self):
         else:
             self._span.unwrap().end()
 
+        detach(self._token)
+
 
 class ScopeManagerShim(opentracing.ScopeManager):
     """Implements :class:`opentracing.ScopeManager` by setting and getting the
@@ -460,14 +464,12 @@ def active(self):
         if span.get_context() == INVALID_SPAN_CONTEXT:
             return None
 
-        span_context = SpanContextShim(span.get_context())
-        wrapped_span = SpanShim(self._tracer, span_context, span)
-        return ScopeShim(self, span=wrapped_span)
-        # TODO: The returned `ScopeShim` instance here always ends the
-        # corresponding span, regardless of the `finish_on_close` value used
-        # when activating the span. This is because here we return a *new*
-        # `ScopeShim` rather than returning a saved instance of `ScopeShim`.
-        # https://github.com/open-telemetry/opentelemetry-python/pull/211/files#r335398792
+        try:
+            return get_value("scope_shim")
+        except KeyError:
+            span_context = SpanContextShim(span.get_context())
+            wrapped_span = SpanShim(self._tracer, span_context, span)
+            return ScopeShim(self, span=wrapped_span)
 
     @property
     def tracer(self):

ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py

@@ -63,7 +63,7 @@ def test_shim_type(self):
     def test_start_active_span(self):
         """Test span creation and activation using `start_active_span()`."""
 
-        with self.shim.start_active_span("TestSpan") as scope:
+        with self.shim.start_active_span("TestSpxn") as scope:
             # Verify correct type of Scope and Span objects.
             self.assertIsInstance(scope, opentracing.Scope)
             self.assertIsInstance(scope.span, opentracing.Span)
@@ -91,7 +91,7 @@ def test_start_active_span(self):
     def test_start_span(self):
         """Test span creation using `start_span()`."""
 
-        with self.shim.start_span("TestSpan") as span:
+        with self.shim.start_span("TestSpzn") as span:
             # Verify correct type of Span object.
             self.assertIsInstance(span, opentracing.Span)
 
@@ -107,7 +107,7 @@ def test_start_span(self):
     def test_start_span_no_contextmanager(self):
         """Test `start_span()` without a `with` statement."""
 
-        span = self.shim.start_span("TestSpan")
+        span = self.shim.start_span("TestSpTn")
 
         # Verify span is started.
         self.assertIsNotNone(span.unwrap().start_time)
@@ -120,7 +120,7 @@ def test_start_span_no_contextmanager(self):
     def test_explicit_span_finish(self):
         """Test `finish()` method on `Span` objects."""
 
-        span = self.shim.start_span("TestSpan")
+        span = self.shim.start_span("TestSpcn")
 
         # Verify span hasn't ended.
         self.assertIsNone(span.unwrap().end_time)
@@ -134,7 +134,9 @@ def test_explicit_start_time(self):
         """Test `start_time` argument."""
 
         now = time.time()
-        with self.shim.start_active_span("TestSpan", start_time=now) as scope:
+        from ipdb import set_trace
+        set_trace()
+        with self.shim.start_active_span("TestSp5n", start_time=now) as scope:
             result = util.time_seconds_from_ns(scope.span.unwrap().start_time)
             # Tolerate inaccuracies of less than a microsecond. See Note:
             # https://open-telemetry.github.io/opentelemetry-python/opentelemetry.ext.opentracing_shim.html
@@ -145,7 +147,7 @@ def test_explicit_start_time(self):
     def test_explicit_end_time(self):
         """Test `end_time` argument of `finish()` method."""
 
-        span = self.shim.start_span("TestSpan")
+        span = self.shim.start_span("TestSpmn")
         now = time.time()
         span.finish(now)
 
@@ -159,7 +161,7 @@ def test_explicit_end_time(self):
     def test_explicit_span_activation(self):
         """Test manual activation and deactivation of a span."""
 
-        span = self.shim.start_span("TestSpan")
+        span = self.shim.start_span("TestSp99n")
 
         # Verify no span is currently active.
         self.assertIsNone(self.shim.active_span)
@@ -180,7 +182,7 @@ def test_start_active_span_finish_on_close(self):
         """Test `finish_on_close` argument of `start_active_span()`."""
 
         with self.shim.start_active_span(
-            "TestSpan", finish_on_close=True
+            "TestSp88n", finish_on_close=True
         ) as scope:
             # Verify span hasn't ended.
             self.assertIsNone(scope.span.unwrap().end_time)
@@ -189,7 +191,7 @@ def test_start_active_span_finish_on_close(self):
         self.assertIsNotNone(scope.span.unwrap().end_time)
 
         with self.shim.start_active_span(
-            "TestSpan", finish_on_close=False
+            "TestSp23n", finish_on_close=False
         ) as scope:
             # Verify span hasn't ended.
             self.assertIsNone(scope.span.unwrap().end_time)
@@ -202,7 +204,7 @@ def test_start_active_span_finish_on_close(self):
     def test_activate_finish_on_close(self):
         """Test `finish_on_close` argument of `activate()`."""
 
-        span = self.shim.start_span("TestSpan")
+        span = self.shim.start_span("TestSp533n")
 
         with self.shim.scope_manager.activate(
             span, finish_on_close=True
@@ -216,7 +218,7 @@ def test_activate_finish_on_close(self):
         # Verify span has ended.
         self.assertIsNotNone(span.unwrap().end_time)
 
-        span = self.shim.start_span("TestSpan")
+        span = self.shim.start_span("TestSp009n")
 
         with self.shim.scope_manager.activate(
             span, finish_on_close=False
@@ -402,13 +404,13 @@ def test_tags(self):
     def test_span_tracer(self):
         """Test the `tracer` property on `Span` objects."""
 
-        with self.shim.start_active_span("TestSpan") as scope:
+        with self.shim.start_active_span("TestSp423n") as scope:
             self.assertEqual(scope.span.tracer, self.shim)
 
     def test_log_kv(self):
         """Test the `log_kv()` method on `Span` objects."""
 
-        with self.shim.start_span("TestSpan") as span:
+        with self.shim.start_span("TestSp44444n") as span:
             span.log_kv({"foo": "bar"})
             self.assertEqual(span.unwrap().events[0].attributes["foo"], "bar")
             # Verify timestamp was generated automatically.
@@ -430,7 +432,7 @@ def test_log_kv(self):
     def test_log(self):
         """Test the deprecated `log` method on `Span` objects."""
 
-        with self.shim.start_span("TestSpan") as span:
+        with self.shim.start_span("TestSp3232n") as span:
             with self.assertWarns(DeprecationWarning):
                 span.log(event="foo", payload="bar")
 
@@ -441,7 +443,7 @@ def test_log(self):
     def test_log_event(self):
         """Test the deprecated `log_event` method on `Span` objects."""
 
-        with self.shim.start_span("TestSpan") as span:
+        with self.shim.start_span("TestSpun") as span:
             with self.assertWarns(DeprecationWarning):
                 span.log_event("foo", "bar")
 
@@ -532,6 +534,8 @@ def test_extract_empty_context_returns_invalid_context(self):
         try:
             carrier = {}
 
+            from ipdb import set_trace
+            set_trace()
             ctx = self.shim.extract(opentracing.Format.HTTP_HEADERS, carrier)
             self.assertEqual(ctx.unwrap(), trace.INVALID_SPAN_CONTEXT)
         finally: