Fix typing in ReadableSpan (#3528)

Co-authored-by: Srikanth Chekuri <[email protected]>

Author: adriangb

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:

opentelemetry-sdk/tests/trace/test_trace.py

@@ -605,24 +605,25 @@ 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)
         for link in span.links:
             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(