-
Notifications
You must be signed in to change notification settings - Fork 652
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
Changes from all commits
244a4f9
30de890
67e45cd
c260d27
cea2918
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
||
|
@@ -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) | ||
|
@@ -453,3 +465,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) | ||
|
||
# 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): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)) |
There was a problem hiding this comment.
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? ;)
There was a problem hiding this comment.
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.