Skip to content

Commit

Permalink
Wrap span context properly
Browse files Browse the repository at this point in the history
We shouldn't return OpenTelemetry objects to the OpenTracing API.
  • Loading branch information
johananl committed Oct 23, 2019
1 parent 0ede012 commit f060ca1
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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?).
Expand Down
41 changes: 30 additions & 11 deletions ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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):
Expand All @@ -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.
Expand All @@ -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).
Expand Down

0 comments on commit f060ca1

Please sign in to comment.