From f060ca10fbb06932e1fa51e003c4196ba5a235aa Mon Sep 17 00:00:00 2001 From: Johannes Liebermann <johannes@kinvolk.io> Date: Thu, 17 Oct 2019 14:56:00 +0200 Subject: [PATCH] Wrap span context properly We shouldn't return OpenTelemetry objects to the OpenTracing API. --- .../ext/opentracing_shim/__init__.py | 6 ++- .../tests/test_shim.py | 41 ++++++++++++++----- 2 files changed, 34 insertions(+), 13 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 9dfce87e60c..2d012f848c9 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 @@ -130,8 +130,9 @@ def __init__(self, manager, span=None, span_cm=None): # it, wrap the extracted span and save it as an attribute. if self._span_cm is not None: otel_span = self._span_cm.__enter__() + span_context = SpanContextShim(otel_span.get_context()) self._span = SpanShim( - self._manager.tracer, otel_span.get_context(), otel_span + self._manager.tracer, span_context, otel_span ) def close(self): @@ -166,7 +167,8 @@ def active(self): if span is None: return None - wrapped_span = SpanShim(self._tracer, span.get_context(), span) + span_context = SpanContextShim(span.get_context()) + wrapped_span = SpanShim(self._tracer, span_context, span) return ScopeShim(self, span=wrapped_span) # TODO: Return a saved instance of SpanShim instead of constructing # a new object (and the same for ScopeShim?). diff --git a/ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py b/ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py index 4845bd7fb2b..b6691911ddc 100644 --- a/ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py +++ b/ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py @@ -56,7 +56,10 @@ def test_start_active_span(self): self.assertIsNotNone(scope.span.unwrap().start_time) # Verify span is active. - self.assertEqual(self.shim.active_span.context, scope.span.context) + self.assertEqual( + self.shim.active_span.context.unwrap(), + scope.span.context.unwrap(), + ) # TODO: We can't check for equality of self.shim.active_span and # scope.span because the same OpenTelemetry span is returned inside # different SpanShim objects. A possible solution is described @@ -149,7 +152,10 @@ def test_explicit_span_activation(self): span, finish_on_close=True ) as scope: # Verify span is active. - self.assertEqual(self.shim.active_span.context, scope.span.context) + self.assertEqual( + self.shim.active_span.context.unwrap(), + scope.span.context.unwrap(), + ) # Verify no span is active. self.assertIsNone(self.shim.active_span) @@ -186,7 +192,10 @@ def test_activate_finish_on_close(self): span, finish_on_close=True ) as scope: # Verify span is active. - self.assertEqual(self.shim.active_span.context, scope.span.context) + self.assertEqual( + self.shim.active_span.context.unwrap(), + scope.span.context.unwrap(), + ) # Verify span has ended. self.assertIsNotNone(span.unwrap().end_time) @@ -197,7 +206,10 @@ def test_activate_finish_on_close(self): span, finish_on_close=False ) as scope: # Verify span is active. - self.assertEqual(self.shim.active_span.context, scope.span.context) + self.assertEqual( + self.shim.active_span.context.unwrap(), + scope.span.context.unwrap(), + ) # Verify span hasn't ended. self.assertIsNone(span.unwrap().end_time) @@ -210,13 +222,17 @@ def test_explicit_scope_close(self): with self.shim.start_active_span("ParentSpan") as parent: # Verify parent span is active. self.assertEqual( - self.shim.active_span.context, parent.span.context + self.shim.active_span.context.unwrap(), + parent.span.context.unwrap(), ) child = self.shim.start_active_span("ChildSpan") # Verify child span is active. - self.assertEqual(self.shim.active_span.context, child.span.context) + self.assertEqual( + self.shim.active_span.context.unwrap(), + child.span.context.unwrap(), + ) # Verify child span hasn't ended. self.assertIsNone(child.span.unwrap().end_time) @@ -228,7 +244,8 @@ def test_explicit_scope_close(self): # Verify parent span becomes active again. self.assertEqual( - self.shim.active_span.context, parent.span.context + self.shim.active_span.context.unwrap(), + parent.span.context.unwrap(), ) def test_parent_child_implicit(self): @@ -239,13 +256,15 @@ def test_parent_child_implicit(self): with self.shim.start_active_span("ParentSpan") as parent: # Verify parent span is the active span. self.assertEqual( - self.shim.active_span.context, parent.span.context + self.shim.active_span.context.unwrap(), + parent.span.context.unwrap(), ) with self.shim.start_active_span("ChildSpan") as child: # Verify child span is the active span. self.assertEqual( - self.shim.active_span.context, child.span.context + self.shim.active_span.context.unwrap(), + child.span.context.unwrap(), ) # Verify parent-child relationship. @@ -259,8 +278,8 @@ def test_parent_child_implicit(self): # Verify parent span becomes the active span again. self.assertEqual( - self.shim.active_span.context, - parent.span.context + self.shim.active_span.context.unwrap(), + parent.span.context.unwrap() # TODO: Check equality of the spans themselves rather than # their context once the SpanShim reconstruction problem has # been addressed (see previous TODO).