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

Record links with invalid SpanContext #3917

Merged
merged 11 commits into from
May 24, 2024
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#3823] (https://github.com/open-telemetry/opentelemetry-python/pull/3823))
- Add span flags to OTLP spans and links
([#3881](https://github.com/open-telemetry/opentelemetry-python/pull/3881))
- Record links with invalid SpanContext if either attributes or TraceState are not empty
([#3917](https://github.com/open-telemetry/opentelemetry-python/pull/3917/))
- Add OpenTelemetry trove classifiers to PyPI packages
([#3913] (https://github.com/open-telemetry/opentelemetry-python/pull/3913))

Expand Down
3 changes: 2 additions & 1 deletion opentelemetry-api/src/opentelemetry/trace/span.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ def add_link( # pylint: disable=no-self-use

Adds a single `Link` with the `SpanContext` of the span to link to and,
optionally, attributes passed as arguments. Implementations may ignore
calls with an invalid span context.
calls with an invalid span context if both attributes and TraceState
are empty.

Note: It is preferred to add links at span creation, instead of calling
this method later since samplers can only consider information already
Expand Down
9 changes: 8 additions & 1 deletion opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,12 @@ def wrapper(self, *args, **kwargs):
return wrapper


def _is_valid_link(context: SpanContext, attributes: types.Attributes) -> bool:
return bool(
xrmx marked this conversation as resolved.
Show resolved Hide resolved
context and (context.is_valid or (attributes or context.trace_state))
)


class ReadableSpan:
"""Provides read-only access to span attributes.

Expand Down Expand Up @@ -867,7 +873,8 @@ def add_link(
context: SpanContext,
attributes: types.Attributes = None,
) -> None:
if context is None or not context.is_valid:

if not _is_valid_link(context, attributes):
return

attributes = BoundedAttributes(
Expand Down
23 changes: 22 additions & 1 deletion opentelemetry-sdk/tests/trace/test_trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -941,9 +941,30 @@ def test_add_link_with_invalid_span_context(self):

with self.tracer.start_as_current_span("root") as root:
root.add_link(other_context)

root.add_link(None)
self.assertEqual(len(root.links), 0)

def test_add_link_with_invalid_span_context_with_attributes(self):
invalid_context = trace_api.INVALID_SPAN_CONTEXT

with self.tracer.start_as_current_span("root") as root:
root.add_link(invalid_context, {"name": "neighbor"})
self.assertEqual(len(root.links), 1)
self.assertEqual(root.links[0].attributes, {"name": "neighbor"})

def test_add_link_with_invalid_span_context_with_tracestate(self):
invalid_context = trace.SpanContext(
trace_api.INVALID_TRACE_ID,
trace_api.INVALID_SPAN_ID,
is_remote=False,
trace_state="foo=bar",
)

with self.tracer.start_as_current_span("root") as root:
root.add_link(invalid_context)
self.assertEqual(len(root.links), 1)
self.assertEqual(root.links[0].context.trace_state, "foo=bar")

def test_update_name(self):
with self.tracer.start_as_current_span("root") as root:
# name
Expand Down