From d474e7a2b0ee99d75cd9afbab8ba0b61fda2fb76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Em=C3=ADdio=20Neto?= <9735060+emdneto@users.noreply.github.com> Date: Mon, 24 Jun 2024 13:26:20 -0300 Subject: [PATCH] Validate links at span creation (#3991) --- CHANGELOG.md | 4 ++- .../src/opentelemetry/sdk/trace/__init__.py | 29 +++++++++++-------- opentelemetry-sdk/tests/trace/test_trace.py | 25 ++++++++++++++++ 3 files changed, 45 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 32698e9436..510c9bf630 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,8 +24,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#3973](https://github.com/open-telemetry/opentelemetry-python/pull/3966)) - Update semantic conventions to version 1.26.0. ([#3964](https://github.com/open-telemetry/opentelemetry-python/pull/3964)) - - Use semconv exception attributes for record exceptions in spans +- Use semconv exception attributes for record exceptions in spans ([#3979](https://github.com/open-telemetry/opentelemetry-python/pull/3979)) +- Validate links at span creation + ([#3991](https://github.com/open-telemetry/opentelemetry-python/pull/3991)) ## Version 1.25.0/0.46b0 (2024-05-30) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 97fb843f6f..a7094b547c 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -816,16 +816,7 @@ def __init__( ) self._events.append(event) - if links is None: - self._links = self._new_links() - else: - for link in links: - link._attributes = BoundedAttributes( - self._limits.max_link_attributes, - link.attributes, - max_value_len=self._limits.max_attribute_length, - ) - self._links = BoundedList.from_seq(self._limits.max_links, links) + self._links = self._new_links(links) def __repr__(self): return f'{type(self).__name__}(name="{self._name}", context={self._context})' @@ -833,8 +824,22 @@ def __repr__(self): def _new_events(self): return BoundedList(self._limits.max_events) - def _new_links(self): - return BoundedList(self._limits.max_links) + def _new_links(self, links: Sequence[trace_api.Link]): + if not links: + return BoundedList(self._limits.max_links) + + valid_links = [] + for link in links: + if link and _is_valid_link(link.context, link.attributes): + # pylint: disable=protected-access + link._attributes = BoundedAttributes( + self._limits.max_link_attributes, + link.attributes, + max_value_len=self._limits.max_attribute_length, + ) + valid_links.append(link) + + return BoundedList.from_seq(self._limits.max_links, valid_links) def get_span_context(self): return self._context diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 07db25474f..9f37564309 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -954,14 +954,29 @@ def test_add_link_with_invalid_span_context(self): root.add_link(None) self.assertEqual(len(root.links), 0) + with self.tracer.start_as_current_span( + "root", links=[trace_api.Link(other_context), None] + ) as root: + 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) root.add_link(invalid_context, {"name": "neighbor"}) self.assertEqual(len(root.links), 1) self.assertEqual(root.links[0].attributes, {"name": "neighbor"}) + with self.tracer.start_as_current_span( + "root", + links=[ + trace_api.Link(invalid_context, {"name": "neighbor"}), + trace_api.Link(invalid_context), + ], + ) as root: + self.assertEqual(len(root.links), 1) + def test_add_link_with_invalid_span_context_with_tracestate(self): invalid_context = trace.SpanContext( trace_api.INVALID_TRACE_ID, @@ -972,9 +987,19 @@ def test_add_link_with_invalid_span_context_with_tracestate(self): with self.tracer.start_as_current_span("root") as root: root.add_link(invalid_context) + root.add_link(trace_api.INVALID_SPAN_CONTEXT) self.assertEqual(len(root.links), 1) self.assertEqual(root.links[0].context.trace_state, "foo=bar") + with self.tracer.start_as_current_span( + "root", + links=[ + trace_api.Link(invalid_context), + trace_api.Link(trace_api.INVALID_SPAN_CONTEXT), + ], + ) as root: + self.assertEqual(len(root.links), 1) + def test_update_name(self): with self.tracer.start_as_current_span("root") as root: # name