From 3d9de97d7c38dcf0d78d35409c37de07c6d9473e Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Thu, 16 Nov 2023 13:45:20 -0600 Subject: [PATCH] Fix typing in ReadableSpan (#3528) Co-authored-by: Srikanth Chekuri --- .../src/opentelemetry/sdk/trace/__init__.py | 149 ++++++++---------- opentelemetry-sdk/tests/trace/test_trace.py | 9 +- 2 files changed, 74 insertions(+), 84 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index ea015bc6068..8897585607a 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -21,7 +21,6 @@ import threading import traceback import typing -from collections import OrderedDict from contextlib import contextmanager from os import environ from time import time_ns @@ -31,6 +30,7 @@ Callable, Dict, Iterator, + List, Optional, Sequence, Tuple, @@ -353,19 +353,19 @@ class ReadableSpan: def __init__( self, - name: str = None, - context: trace_api.SpanContext = None, + name: str, + context: Optional[trace_api.SpanContext] = None, parent: Optional[trace_api.SpanContext] = None, - resource: Resource = None, + resource: Optional[Resource] = None, attributes: types.Attributes = None, events: Sequence[Event] = (), links: Sequence[trace_api.Link] = (), kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL, - instrumentation_info: InstrumentationInfo = None, + instrumentation_info: Optional[InstrumentationInfo] = None, status: Status = Status(StatusCode.UNSET), start_time: Optional[int] = None, end_time: Optional[int] = None, - instrumentation_scope: InstrumentationScope = None, + instrumentation_scope: Optional[InstrumentationScope] = None, ) -> None: self._name = name self._context = context @@ -386,19 +386,19 @@ def __init__( @property def dropped_attributes(self) -> int: - if self._attributes: + if isinstance(self._attributes, BoundedAttributes): return self._attributes.dropped return 0 @property def dropped_events(self) -> int: - if self._events: + if isinstance(self._events, BoundedList): return self._events.dropped return 0 @property def dropped_links(self) -> int: - if self._links: + if isinstance(self._links, BoundedList): return self._links.dropped return 0 @@ -435,7 +435,7 @@ def status(self) -> trace_api.Status: @property def attributes(self) -> types.Attributes: - return MappingProxyType(self._attributes) + return MappingProxyType(self._attributes or {}) @property def events(self) -> Sequence[Event]: @@ -453,23 +453,17 @@ def resource(self) -> Resource: @deprecated( version="1.11.1", reason="You should use instrumentation_scope" ) - def instrumentation_info(self) -> InstrumentationInfo: + def instrumentation_info(self) -> Optional[InstrumentationInfo]: return self._instrumentation_info @property - def instrumentation_scope(self) -> InstrumentationScope: + def instrumentation_scope(self) -> Optional[InstrumentationScope]: return self._instrumentation_scope - def to_json(self, indent=4): + def to_json(self, indent: int = 4): parent_id = None if self.parent is not None: - if isinstance(self.parent, Span): - ctx = self.parent.context - parent_id = f"0x{trace_api.format_span_id(ctx.span_id)}" - elif isinstance(self.parent, SpanContext): - parent_id = ( - f"0x{trace_api.format_span_id(self.parent.span_id)}" - ) + parent_id = f"0x{trace_api.format_span_id(self.parent.span_id)}" start_time = None if self._start_time: @@ -479,77 +473,72 @@ def to_json(self, indent=4): if self._end_time: end_time = util.ns_to_iso_str(self._end_time) - if self._status is not None: - status = OrderedDict() - status["status_code"] = str(self._status.status_code.name) - if self._status.description: - status["description"] = self._status.description - - f_span = OrderedDict() - - f_span["name"] = self._name - f_span["context"] = self._format_context(self._context) - f_span["kind"] = str(self.kind) - f_span["parent_id"] = parent_id - f_span["start_time"] = start_time - f_span["end_time"] = end_time - if self._status is not None: - f_span["status"] = status - f_span["attributes"] = self._format_attributes(self._attributes) - f_span["events"] = self._format_events(self._events) - f_span["links"] = self._format_links(self._links) - f_span["resource"] = json.loads(self.resource.to_json()) + status = { + "status_code": str(self._status.status_code.name), + } + if self._status.description: + status["description"] = self._status.description + + f_span = { + "name": self._name, + "context": self._format_context(self._context) + if self._context + else None, + "kind": str(self.kind), + "parent_id": parent_id, + "start_time": start_time, + "end_time": end_time, + "status": status, + "attributes": self._format_attributes(self._attributes), + "events": self._format_events(self._events), + "links": self._format_links(self._links), + "resource": json.loads(self.resource.to_json()), + } return json.dumps(f_span, indent=indent) @staticmethod - def _format_context(context): - x_ctx = OrderedDict() - x_ctx["trace_id"] = f"0x{trace_api.format_trace_id(context.trace_id)}" - x_ctx["span_id"] = f"0x{trace_api.format_span_id(context.span_id)}" - x_ctx["trace_state"] = repr(context.trace_state) - return x_ctx + def _format_context(context: SpanContext) -> Dict[str, str]: + return { + "trace_id": f"0x{trace_api.format_trace_id(context.trace_id)}", + "span_id": f"0x{trace_api.format_span_id(context.span_id)}", + "trace_state": repr(context.trace_state), + } @staticmethod - def _format_attributes(attributes): - if isinstance(attributes, BoundedAttributes): - return attributes._dict # pylint: disable=protected-access - if isinstance(attributes, MappingProxyType): - return attributes.copy() + def _format_attributes( + attributes: types.Attributes, + ) -> Optional[Dict[str, Any]]: + if attributes is not None and not isinstance(attributes, dict): + return dict(attributes) return attributes @staticmethod - def _format_events(events): - f_events = [] - for event in events: - f_event = OrderedDict() - f_event["name"] = event.name - f_event["timestamp"] = util.ns_to_iso_str(event.timestamp) - f_event[ - "attributes" - ] = Span._format_attributes( # pylint: disable=protected-access - event.attributes - ) - f_events.append(f_event) - return f_events + def _format_events(events: Sequence[Event]) -> List[Dict[str, Any]]: + return [ + { + "name": event.name, + "timestamp": util.ns_to_iso_str(event.timestamp), + "attributes": Span._format_attributes( # pylint: disable=protected-access + event.attributes + ), + } + for event in events + ] @staticmethod - def _format_links(links): - f_links = [] - for link in links: - f_link = OrderedDict() - f_link[ - "context" - ] = Span._format_context( # pylint: disable=protected-access - link.context - ) - f_link[ - "attributes" - ] = Span._format_attributes( # pylint: disable=protected-access - link.attributes - ) - f_links.append(f_link) - return f_links + def _format_links(links: Sequence[trace_api.Link]) -> List[Dict[str, Any]]: + return [ + { + "context": Span._format_context( # pylint: disable=protected-access + link.context + ), + "attributes": Span._format_attributes( # pylint: disable=protected-access + link.attributes + ), + } + for link in links + ] class SpanLimits: diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index b08afce4a6d..83a4ece5ed7 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -605,10 +605,11 @@ def test_surplus_span_attributes(self): class TestReadableSpan(unittest.TestCase): def test_links(self): - span = trace.ReadableSpan() + span = trace.ReadableSpan("test") self.assertEqual(span.links, ()) span = trace.ReadableSpan( + "test", links=[trace_api.Link(context=trace_api.INVALID_SPAN_CONTEXT)] * 2, ) self.assertEqual(len(span.links), 2) @@ -616,13 +617,13 @@ def test_links(self): self.assertFalse(link.context.is_valid) def test_events(self): - span = trace.ReadableSpan() + span = trace.ReadableSpan("test") self.assertEqual(span.events, ()) events = [ trace.Event("foo1", {"bar1": "baz1"}), trace.Event("foo2", {"bar2": "baz2"}), ] - span = trace.ReadableSpan(events=events) + span = trace.ReadableSpan("test", events=events) self.assertEqual(span.events, tuple(events)) @@ -1376,7 +1377,7 @@ def test_to_json(self): ) parent = trace._Span("parent-name", context, resource=Resource({})) span = trace._Span( - "span-name", context, resource=Resource({}), parent=parent + "span-name", context, resource=Resource({}), parent=parent.context ) self.assertEqual(