Skip to content

Commit

Permalink
Add remaining fixes
Browse files Browse the repository at this point in the history
Fixes #242
  • Loading branch information
ocelotl committed Jul 17, 2020
1 parent 82685c2 commit 03c6aa7
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@
from deprecated import deprecated

from opentelemetry import propagators
from opentelemetry.context import Context
from opentelemetry.context import attach, detach, get_value, set_value
from opentelemetry.context import Context, attach, get_value, set_value
from opentelemetry.ext.opentracing_shim import util
from opentelemetry.ext.opentracing_shim.version import __version__
from opentelemetry.trace import (
Expand Down Expand Up @@ -328,7 +327,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))
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.
Expand Down Expand Up @@ -394,8 +393,6 @@ def close(self):
else:
self._span.unwrap().end()

detach(self._token)


class ScopeManagerShim(opentracing.ScopeManager):
"""Implements :class:`opentracing.ScopeManager` by setting and getting the
Expand Down
54 changes: 35 additions & 19 deletions ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py
Original file line number Diff line number Diff line change
Expand Up @@ -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("TestSpxn") as scope:
with self.shim.start_active_span("TestSpan0") as scope:
# Verify correct type of Scope and Span objects.
self.assertIsInstance(scope, opentracing.Scope)
self.assertIsInstance(scope.span, opentracing.Span)
Expand Down Expand Up @@ -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("TestSpzn") as span:
with self.shim.start_span("TestSpan1") as span:
# Verify correct type of Span object.
self.assertIsInstance(span, opentracing.Span)

Expand All @@ -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("TestSpTn")
span = self.shim.start_span("TestSpan2")

# Verify span is started.
self.assertIsNotNone(span.unwrap().start_time)
Expand All @@ -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("TestSpcn")
span = self.shim.start_span("TestSpan3")

# Verify span hasn't ended.
self.assertIsNone(span.unwrap().end_time)
Expand All @@ -134,9 +134,7 @@ def test_explicit_start_time(self):
"""Test `start_time` argument."""

now = time.time()
from ipdb import set_trace
set_trace()
with self.shim.start_active_span("TestSp5n", start_time=now) as scope:
with self.shim.start_active_span("TestSpan4", 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
Expand All @@ -147,7 +145,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("TestSpmn")
span = self.shim.start_span("TestSpan5")
now = time.time()
span.finish(now)

Expand All @@ -161,7 +159,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("TestSp99n")
span = self.shim.start_span("TestSpan6")

# Verify no span is currently active.
self.assertIsNone(self.shim.active_span)
Expand All @@ -182,7 +180,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(
"TestSp88n", finish_on_close=True
"TestSpan7", finish_on_close=True
) as scope:
# Verify span hasn't ended.
self.assertIsNone(scope.span.unwrap().end_time)
Expand All @@ -191,7 +189,7 @@ def test_start_active_span_finish_on_close(self):
self.assertIsNotNone(scope.span.unwrap().end_time)

with self.shim.start_active_span(
"TestSp23n", finish_on_close=False
"TestSpan8", finish_on_close=False
) as scope:
# Verify span hasn't ended.
self.assertIsNone(scope.span.unwrap().end_time)
Expand All @@ -204,7 +202,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("TestSp533n")
span = self.shim.start_span("TestSpan9")

with self.shim.scope_manager.activate(
span, finish_on_close=True
Expand All @@ -218,7 +216,7 @@ def test_activate_finish_on_close(self):
# Verify span has ended.
self.assertIsNotNone(span.unwrap().end_time)

span = self.shim.start_span("TestSp009n")
span = self.shim.start_span("TestSpan10")

with self.shim.scope_manager.activate(
span, finish_on_close=False
Expand Down Expand Up @@ -404,13 +402,13 @@ def test_tags(self):
def test_span_tracer(self):
"""Test the `tracer` property on `Span` objects."""

with self.shim.start_active_span("TestSp423n") as scope:
with self.shim.start_active_span("TestSpan11") 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("TestSp44444n") as span:
with self.shim.start_span("TestSpan12") as span:
span.log_kv({"foo": "bar"})
self.assertEqual(span.unwrap().events[0].attributes["foo"], "bar")
# Verify timestamp was generated automatically.
Expand All @@ -432,7 +430,7 @@ def test_log_kv(self):
def test_log(self):
"""Test the deprecated `log` method on `Span` objects."""

with self.shim.start_span("TestSp3232n") as span:
with self.shim.start_span("TestSpan13") as span:
with self.assertWarns(DeprecationWarning):
span.log(event="foo", payload="bar")

Expand All @@ -443,7 +441,7 @@ def test_log(self):
def test_log_event(self):
"""Test the deprecated `log_event` method on `Span` objects."""

with self.shim.start_span("TestSpun") as span:
with self.shim.start_span("TestSpan14") as span:
with self.assertWarns(DeprecationWarning):
span.log_event("foo", "bar")

Expand Down Expand Up @@ -534,8 +532,6 @@ 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:
Expand All @@ -561,6 +557,7 @@ def test_extract_binary(self):
self.shim.extract(opentracing.Format.BINARY, bytearray())

def test_baggage(self):
"""Test SpanShim baggage being set and being immutable"""

span_context_shim = SpanContextShim(
trace.SpanContext(1234, 5678, is_remote=False)
Expand All @@ -579,3 +576,22 @@ def test_baggage(self):
span_shim.set_baggage_item(1, 2)

self.assertTrue(span_shim.get_baggage_item(1), 2)

def test_active(self):
"""Test that the active property and start_active_span return the same
object"""

# Verify no span is currently active.
self.assertIsNone(self.shim.active_span)

with self.shim.start_active_span("TestSpan15") as scope:
# Verify span is active.
self.assertEqual(
self.shim.active_span.context.unwrap(),
scope.span.context.unwrap(),
)

self.assertIs(self.shim.scope_manager.active, scope)

# Verify no span is active.
self.assertIsNone(self.shim.active_span)

0 comments on commit 03c6aa7

Please sign in to comment.