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

Make sure trace flags are parsed properly when extracting traceparent and expand tests #2577

Merged
merged 3 commits into from
Apr 1, 2022
Merged
Show file tree
Hide file tree
Changes from all 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 CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.10.0-0.29b0...HEAD)

- Fix parsing of trace flags when extracting traceparent
([#2577](https://github.com/open-telemetry/opentelemetry-python/pull/2577))
- Add default aggregation
([#2543](https://github.com/open-telemetry/opentelemetry-python/pull/2543))
- Fix incorrect installation of some exporter “convenience” packages into
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def extract(
trace_id=int(trace_id, 16),
span_id=int(span_id, 16),
is_remote=True,
trace_flags=trace.TraceFlags(trace_flags),
trace_flags=trace.TraceFlags(int(trace_flags, 16)),
srikanthccv marked this conversation as resolved.
Show resolved Hide resolved
trace_state=tracestate,
)
return trace.set_span_in_context(
Expand Down
147 changes: 147 additions & 0 deletions opentelemetry-api/tests/propagators/test_propagators.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
from unittest import TestCase
from unittest.mock import Mock, patch

from opentelemetry import trace
from opentelemetry.baggage.propagation import W3CBaggagePropagator
from opentelemetry.context.context import Context
from opentelemetry.environment_variables import OTEL_PROPAGATORS
from opentelemetry.trace.propagation.tracecontext import (
TraceContextTextMapPropagator,
Expand All @@ -44,6 +46,7 @@ def test_propagators(propagators):
**{"side_effect": test_propagators}
)

# pylint: disable=import-outside-toplevel
import opentelemetry.propagate

reload(opentelemetry.propagate)
Expand Down Expand Up @@ -79,6 +82,7 @@ def test_propagators(propagators):
**{"side_effect": test_propagators}
)

# pylint: disable=import-outside-toplevel
import opentelemetry.propagate

reload(opentelemetry.propagate)
Expand All @@ -88,6 +92,7 @@ def test_propagators(propagators):
)
def test_composite_propagators_error(self):

# pylint: disable=import-outside-toplevel
import opentelemetry.propagate

with self.assertRaises(Exception):
Expand All @@ -97,3 +102,145 @@ def test_composite_propagators_error(self):
"Failed to load configured propagator `unknown`",
err.output[0],
)


class TestTraceContextTextMapPropagator(TestCase):
def setUp(self):
self.propagator = TraceContextTextMapPropagator()

def traceparent_helper(
self,
carrier,
):
# We purposefully start with an empty context so we can test later if anything is added to it.
initial_context = Context()

context = self.propagator.extract(carrier, context=initial_context)
self.assertIsNotNone(context)
self.assertIsInstance(context, Context)

return context

def traceparent_helper_generator(
self,
version=0x00,
trace_id=0x00000000000000000000000000000001,
span_id=0x0000000000000001,
trace_flags=0x00,
suffix="",
):
traceparent = f"{version:02x}-{trace_id:032x}-{span_id:016x}-{trace_flags:02x}{suffix}"
carrier = {"traceparent": traceparent}
return self.traceparent_helper(carrier)

def valid_traceparent_helper(
self,
version=0x00,
trace_id=0x00000000000000000000000000000001,
span_id=0x0000000000000001,
trace_flags=0x00,
suffix="",
assert_context_msg="A valid traceparent was provided, so the context should be non-empty.",
):
context = self.traceparent_helper_generator(
version=version,
trace_id=trace_id,
span_id=span_id,
trace_flags=trace_flags,
suffix=suffix,
)

self.assertNotEqual(
context,
Context(),
assert_context_msg,
)

span = trace.get_current_span(context)
self.assertIsNotNone(span)
self.assertIsInstance(span, trace.span.Span)

span_context = span.get_span_context()
self.assertIsNotNone(span_context)
self.assertIsInstance(span_context, trace.span.SpanContext)

# Note: No version in SpanContext, it is only used locally in TraceContextTextMapPropagator
self.assertEqual(span_context.trace_id, trace_id)
self.assertEqual(span_context.span_id, span_id)
self.assertEqual(span_context.trace_flags, trace_flags)

self.assertIsInstance(span_context.trace_state, trace.TraceState)
self.assertCountEqual(span_context.trace_state, [])
self.assertEqual(span_context.is_remote, True)

return context, span, span_context

def invalid_traceparent_helper(
self,
version=0x00,
trace_id=0x00000000000000000000000000000001,
span_id=0x0000000000000001,
trace_flags=0x00,
suffix="",
assert_context_msg="An invalid traceparent was provided, so the context should still be empty.",
):
context = self.traceparent_helper_generator(
version=version,
trace_id=trace_id,
span_id=span_id,
trace_flags=trace_flags,
suffix=suffix,
)

self.assertEqual(
context,
Context(),
assert_context_msg,
)

return context

def test_extract_nothing(self):
context = self.traceparent_helper(carrier={})
self.assertEqual(
context,
{},
"We didn't provide a valid traceparent, so we should still have an empty Context.",
)

def test_extract_simple_traceparent(self):
self.valid_traceparent_helper()

# https://www.w3.org/TR/trace-context/#version
def test_extract_version_forbidden_ff(self):
self.invalid_traceparent_helper(
version=0xFF,
assert_context_msg="We provided ann invalid traceparent with a forbidden version=0xff, so the context should still be empty.",
)

# https://www.w3.org/TR/trace-context/#version-format
def test_extract_version_00_with_unsupported_suffix(self):
self.invalid_traceparent_helper(
suffix="-f00",
assert_context_msg="We provided an invalid traceparent with version=0x00 and suffix information which is not supported in this version, so the context should still be empty.",
)

# https://www.w3.org/TR/trace-context/#versioning-of-traceparent
# See the parsing of the sampled bit of flags.
def test_extract_future_version_with_future_suffix_data(self):
self.valid_traceparent_helper(
version=0x99,
suffix="-f00",
assert_context_msg="We provided a traceparent that is possibly valid in the future with version=0x99 and suffix information, so the context be non-empty.",
)

# https://www.w3.org/TR/trace-context/#trace-id
def test_extract_trace_id_invalid_all_zeros(self):
self.invalid_traceparent_helper(trace_id=0)

# https://www.w3.org/TR/trace-context/#parent-id
def test_extract_span_id_invalid_all_zeros(self):
self.invalid_traceparent_helper(span_id=0)

def test_extract_non_decimal_trace_flags(self):
self.valid_traceparent_helper(trace_flags=0xA0)
24 changes: 24 additions & 0 deletions opentelemetry-sdk/tests/trace/test_trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -1745,3 +1745,27 @@ def _test_span_no_limits(self, tracer):
self.assertEqual(len(root.attributes), num_attributes)
for attr_val in root.attributes.values():
self.assertEqual(attr_val, self.long_val)


class TestTraceFlags(unittest.TestCase):
def test_constant_default(self):
self.assertEqual(trace_api.TraceFlags.DEFAULT, 0)

def test_constant_sampled(self):
self.assertEqual(trace_api.TraceFlags.SAMPLED, 1)

def test_get_default(self):
self.assertEqual(
trace_api.TraceFlags.get_default(), trace_api.TraceFlags.DEFAULT
)

def test_sampled_true(self):
self.assertTrue(trace_api.TraceFlags(0xF1).sampled)

def test_sampled_false(self):
self.assertFalse(trace_api.TraceFlags(0xF0).sampled)

def test_constant_default_trace_options(self):
self.assertEqual(
trace_api.DEFAULT_TRACE_OPTIONS, trace_api.TraceFlags.DEFAULT
)