From 82685c22f510e559727b594aa81d20061f364907 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Fri, 17 Jul 2020 01:35:21 -0600 Subject: [PATCH] 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/opentracing_shim/__init__.py | 18 +++++----- .../tests/test_shim.py | 34 +++++++++++-------- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py b/ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py index d35a65a1fa8..b9b009c2159 100644 --- a/ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py +++ b/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): diff --git a/ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py b/ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py index fb1a36fac0c..3b899851d6c 100644 --- a/ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py +++ b/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: