Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ext/opentracing-shim: implement inject and extract #256

Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ext/opentelemetry-ext-opentracing-shim/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Unreleased

- Implement extract and inject support for HTTP_HEADERS and TEXT_MAP formats.

## 0.2a.0

Released 2019-10-29
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import opentracing

from opentelemetry import propagators
from opentelemetry.ext.opentracing_shim import util

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -179,6 +180,10 @@ class TracerShim(opentracing.Tracer):
def __init__(self, tracer):
super().__init__(scope_manager=ScopeManagerShim(self))
self._otel_tracer = tracer
self._supported_formats = (
opentracing.Format.TEXT_MAP,
opentracing.Format.HTTP_HEADERS,
)

def unwrap(self):
"""Returns the wrapped OpenTelemetry `Tracer` object."""
Expand Down Expand Up @@ -244,16 +249,30 @@ def start_span(

def inject(self, span_context, format, carrier):
# pylint: disable=redefined-builtin
logger.warning(
"Calling unimplemented method inject() on class %s",
self.__class__.__name__,
)
# TODO: Implement.
# This implementation does not perform the injecting by itself but
# uses the configured propagators in opentelemetry.propagators.
# TODO: Support Format.BINARY once it is supported in
# opentelemetry-python.
if format not in self._supported_formats:
raise opentracing.UnsupportedFormatException

propagator = propagators.get_global_httptextformat()
propagator.inject(span_context.unwrap(), dict.__setitem__, carrier)
mauriciovasquezbernal marked this conversation as resolved.
Show resolved Hide resolved

def extract(self, format, carrier):
# pylint: disable=redefined-builtin
logger.warning(
"Calling unimplemented method extract() on class %s",
self.__class__.__name__,
)
# TODO: Implement.
# This implementation does not perform the extracing by itself but
# uses the configured propagators in opentelemetry.propagators.
# TODO: Support Format.BINARY once it is supported in
# opentelemetry-python.
if format not in self._supported_formats:
raise opentracing.UnsupportedFormatException

def get_as_list(dict_object, key):
value = dict_object.get(key)
return [value] if value is not None else []

propagator = propagators.get_global_httptextformat()
otel_context = propagator.extract(get_as_list, carrier)

return SpanContextShim(otel_context)
102 changes: 101 additions & 1 deletion ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
import opentracing

import opentelemetry.ext.opentracing_shim as opentracingshim
from opentelemetry import trace
from opentelemetry import propagators, trace
from opentelemetry.context.propagation.httptextformat import HTTPTextFormat
from opentelemetry.ext.opentracing_shim import util
from opentelemetry.sdk.trace import Tracer

Expand All @@ -40,6 +41,17 @@ def setUpClass(cls):

trace.set_preferred_tracer_implementation(lambda T: Tracer())

# Save current propagator to be restored on teardown.
cls._previous_propagator = propagators.get_global_httptextformat()

# Set mock propagator for testing.
propagators.set_global_httptextformat(MockHTTPTextFormat)

@classmethod
def tearDownClass(cls):
# Restore previous propagator.
propagators.set_global_httptextformat(cls._previous_propagator)

def test_shim_type(self):
# Verify shim is an OpenTracing tracer.
self.assertIsInstance(self.shim, opentracing.Tracer)
Expand Down Expand Up @@ -431,3 +443,91 @@ def test_span_on_error(self):
self.assertEqual(
scope.span.unwrap().events[0].attributes["error.kind"], Exception
)

def test_inject_http_headers(self):
"""Test `inject()` method for Format.HTTP_HEADERS."""

otel_context = trace.SpanContext(trace_id=1220, span_id=7478)
context = opentracingshim.SpanContextShim(otel_context)

headers = {}
self.shim.inject(context, opentracing.Format.HTTP_HEADERS, headers)
self.assertEqual(headers[MockHTTPTextFormat.TRACE_ID_KEY], str(1220))
self.assertEqual(headers[MockHTTPTextFormat.SPAN_ID_KEY], str(7478))

def test_inject_text_map(self):
"""Test `inject()` method for Format.TEXT_MAP."""

otel_context = trace.SpanContext(trace_id=1220, span_id=7478)
context = opentracingshim.SpanContextShim(otel_context)

# Verify Format.TEXT_MAP
text_map = {}
self.shim.inject(context, opentracing.Format.TEXT_MAP, text_map)
self.assertEqual(text_map[MockHTTPTextFormat.TRACE_ID_KEY], str(1220))
self.assertEqual(text_map[MockHTTPTextFormat.SPAN_ID_KEY], str(7478))

def test_inject_binary(self):
"""Test `inject()` method for Format.BINARY."""

otel_context = trace.SpanContext(trace_id=1220, span_id=7478)
context = opentracingshim.SpanContextShim(otel_context)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we have the error as a separated test case? ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I separated different formats in different test cases in 30de890.

# Verify exception for non supported binary format.
with self.assertRaises(opentracing.UnsupportedFormatException):
self.shim.inject(context, opentracing.Format.BINARY, bytearray())

def test_extract_http_headers(self):
"""Test `extract()` method for Format.HTTP_HEADERS."""

carrier = {
MockHTTPTextFormat.TRACE_ID_KEY: 1220,
MockHTTPTextFormat.SPAN_ID_KEY: 7478,
}

ctx = self.shim.extract(opentracing.Format.HTTP_HEADERS, carrier)
self.assertEqual(ctx.unwrap().trace_id, 1220)
self.assertEqual(ctx.unwrap().span_id, 7478)

def test_extract_text_map(self):
"""Test `extract()` method for Format.TEXT_MAP."""

carrier = {
MockHTTPTextFormat.TRACE_ID_KEY: 1220,
MockHTTPTextFormat.SPAN_ID_KEY: 7478,
}

ctx = self.shim.extract(opentracing.Format.TEXT_MAP, carrier)
self.assertEqual(ctx.unwrap().trace_id, 1220)
self.assertEqual(ctx.unwrap().span_id, 7478)

def test_extract_binary(self):
"""Test `extract()` method for Format.BINARY."""

# Verify exception for non supported binary format.
with self.assertRaises(opentracing.UnsupportedFormatException):
self.shim.extract(opentracing.Format.BINARY, bytearray())


class MockHTTPTextFormat(HTTPTextFormat):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming it's not so trivial to verify against the actual builtin format(s)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not that hard actually but I wanted to keep these tests independently from actual implementations. Of course I am open to discuss why it could be better to test with the actual implementation instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me, but lets wait for others to chime in ;)

"""Mock propagator for testing purposes."""

TRACE_ID_KEY = "mock-traceid"
SPAN_ID_KEY = "mock-spanid"

@classmethod
def extract(cls, get_from_carrier, carrier):
trace_id_list = get_from_carrier(carrier, cls.TRACE_ID_KEY)
span_id_list = get_from_carrier(carrier, cls.SPAN_ID_KEY)

if not trace_id_list or not span_id_list:
return trace.INVALID_SPAN_CONTEXT

return trace.SpanContext(
trace_id=int(trace_id_list[0]), span_id=int(span_id_list[0])
)

@classmethod
def inject(cls, context, set_in_carrier, carrier):
set_in_carrier(carrier, cls.TRACE_ID_KEY, str(context.trace_id))
set_in_carrier(carrier, cls.SPAN_ID_KEY, str(context.span_id))