Removing add_link and add_lazy_link from api/sdk (#259)

Prevents adding links after spans are created for OTEP 0006.

Author: alrex

ext/opentelemetry-ext-jaeger/examples/jaeger_exporter_example.py

@@ -33,10 +33,11 @@
     time.sleep(0.1)
     foo.set_attribute("my_atribbute", True)
     foo.add_event("event in foo", {"name": "foo1"})
-    with tracer.start_as_current_span("bar") as bar:
+    with tracer.start_as_current_span(
+        "bar", links=[trace.Link(foo.get_context())]
+    ) as bar:
         time.sleep(0.2)
         bar.set_attribute("speed", 100.0)
-        bar.add_link(foo.get_context())
 
         with tracer.start_as_current_span("baz") as baz:
             time.sleep(0.3)

ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py

@@ -17,6 +17,7 @@
 import opentracing
 from deprecated import deprecated
 
+import opentelemetry.trace as trace_api
 from opentelemetry import propagators
 from opentelemetry.ext.opentracing_shim import util
 
@@ -231,11 +232,15 @@ def start_span(
         # Use the specified parent or the active span if possible. Otherwise,
         # use a `None` parent, which triggers the creation of a new trace.
         parent = child_of.unwrap() if child_of else None
-        span = self._otel_tracer.create_span(operation_name, parent)
 
+        links = []
         if references:
             for ref in references:
-                span.add_link(ref.referenced_context.unwrap())
+                links.append(trace_api.Link(ref.referenced_context.unwrap()))
+
+        span = self._otel_tracer.create_span(
+            operation_name, parent, links=links
+        )
 
         if tags:
             for key, value in tags.items():

opentelemetry-api/src/opentelemetry/trace/__init__.py

@@ -80,7 +80,10 @@ def __init__(
         self, context: "SpanContext", attributes: types.Attributes = None
     ) -> None:
         self._context = context
-        self._attributes = attributes
+        if attributes is None:
+            self._attributes = {}  # type: types.Attributes
+        else:
+            self._attributes = attributes
 
     @property
     def context(self) -> "SpanContext":
@@ -198,23 +201,6 @@ def add_lazy_event(self, event: Event) -> None:
         Adds an `Event` that has previously been created.
         """
 
-    def add_link(
-        self,
-        link_target_context: "SpanContext",
-        attributes: types.Attributes = None,
-    ) -> None:
-        """Adds a `Link` to another span.
-
-        Adds a single `Link` from this Span to another Span identified by the
-        `SpanContext` passed as argument.
-        """
-
-    def add_lazy_link(self, link: "Link") -> None:
-        """Adds a `Link` to another span.
-
-        Adds a `Link` that has previously been created.
-        """
-
     def update_name(self, name: str) -> None:
         """Updates the `Span` name.
 
@@ -416,6 +402,8 @@ def start_span(
         name: str,
         parent: ParentSpan = CURRENT_SPAN,
         kind: SpanKind = SpanKind.INTERNAL,
+        attributes: typing.Optional[types.Attributes] = None,
+        links: typing.Sequence[Link] = (),
     ) -> "Span":
         """Starts a span.
 
@@ -444,6 +432,8 @@ def start_span(
             parent: The span's parent. Defaults to the current span.
             kind: The span's kind (relationship to parent). Note that is
                 meaningful even if there is no parent.
+            attributes: The span's attributes.
+            links: Links span to other spans
 
         Returns:
             The newly-created span.
@@ -457,6 +447,8 @@ def start_as_current_span(
         name: str,
         parent: ParentSpan = CURRENT_SPAN,
         kind: SpanKind = SpanKind.INTERNAL,
+        attributes: typing.Optional[types.Attributes] = None,
+        links: typing.Sequence[Link] = (),
     ) -> typing.Iterator["Span"]:
         """Context manager for creating a new span and set it
         as the current span in this tracer's context.
@@ -492,6 +484,8 @@ def start_as_current_span(
             parent: The span's parent. Defaults to the current span.
             kind: The span's kind (relationship to parent). Note that is
                 meaningful even if there is no parent.
+            attributes: The span's attributes.
+            links: Links span to other spans
 
         Yields:
             The newly-created span.
@@ -505,6 +499,8 @@ def create_span(
         name: str,
         parent: ParentSpan = CURRENT_SPAN,
         kind: SpanKind = SpanKind.INTERNAL,
+        attributes: typing.Optional[types.Attributes] = None,
+        links: typing.Sequence[Link] = (),
     ) -> "Span":
         """Creates a span.
 
@@ -534,6 +530,8 @@ def create_span(
             parent: The span's parent. Defaults to the current span.
             kind: The span's kind (relationship to parent). Note that is
                 meaningful even if there is no parent.
+            attributes: The span's attributes.
+            links: Links span to other spans
 
         Returns:
             The newly-created span.

opentelemetry-api/src/opentelemetry/trace/sampling.py

@@ -17,7 +17,7 @@
 
 # pylint: disable=unused-import
 from opentelemetry.trace import Link, SpanContext
-from opentelemetry.util.types import AttributeValue
+from opentelemetry.util.types import Attributes, AttributeValue
 
 
 class Decision:
@@ -53,6 +53,7 @@ def should_sample(
         trace_id: int,
         span_id: int,
         name: str,
+        attributes: Optional[Attributes] = None,
         links: Sequence["Link"] = (),
     ) -> "Decision":
         pass
@@ -70,6 +71,7 @@ def should_sample(
         trace_id: int,
         span_id: int,
         name: str,
+        attributes: Optional[Attributes] = None,
         links: Sequence["Link"] = (),
     ) -> "Decision":
         return self._decision
@@ -107,6 +109,7 @@ def should_sample(
         trace_id: int,
         span_id: int,
         name: str,
+        attributes: Optional[Attributes] = None,  # TODO
         links: Sequence["Link"] = (),
     ) -> "Decision":
         if parent_context is not None:

opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py

@@ -130,7 +130,7 @@ def __init__(
         resource: None = None,  # TODO
         attributes: types.Attributes = None,  # TODO
         events: Sequence[trace_api.Event] = None,  # TODO
-        links: Sequence[trace_api.Link] = None,  # TODO
+        links: Sequence[trace_api.Link] = (),
         kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL,
         span_processor: SpanProcessor = SpanProcessor(),
     ) -> None:
@@ -226,30 +226,6 @@ def add_lazy_event(self, event: trace_api.Event) -> None:
             return
         self.events.append(event)
 
-    def add_link(
-        self,
-        link_target_context: "trace_api.SpanContext",
-        attributes: types.Attributes = None,
-    ) -> None:
-        if attributes is None:
-            attributes = (
-                Span.empty_attributes
-            )  # TODO: empty_attributes is not a Dict. Use Mapping?
-        self.add_lazy_link(trace_api.Link(link_target_context, attributes))
-
-    def add_lazy_link(self, link: "trace_api.Link") -> None:
-        with self._lock:
-            if not self.is_recording_events():
-                return
-            has_ended = self.end_time is not None
-            if not has_ended:
-                if self.links is Span.empty_links:
-                    self.links = BoundedList(MAX_NUM_LINKS)
-        if has_ended:
-            logger.warning("Calling add_link() on an ended span.")
-            return
-        self.links.append(link)
-
     def start(self, start_time: Optional[int] = None) -> None:
         with self._lock:
             if not self.is_recording_events():
@@ -343,10 +319,12 @@ def start_span(
         name: str,
         parent: trace_api.ParentSpan = trace_api.Tracer.CURRENT_SPAN,
         kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL,
+        attributes: Optional[types.Attributes] = None,
+        links: Sequence[trace_api.Link] = (),
     ) -> "Span":
         """See `opentelemetry.trace.Tracer.start_span`."""
 
-        span = self.create_span(name, parent, kind)
+        span = self.create_span(name, parent, kind, attributes, links)
         span.start()
         return span
 
@@ -355,17 +333,21 @@ def start_as_current_span(
         name: str,
         parent: trace_api.ParentSpan = trace_api.Tracer.CURRENT_SPAN,
         kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL,
+        attributes: Optional[types.Attributes] = None,
+        links: Sequence[trace_api.Link] = (),
     ) -> Iterator[trace_api.Span]:
         """See `opentelemetry.trace.Tracer.start_as_current_span`."""
 
-        span = self.start_span(name, parent, kind)
+        span = self.start_span(name, parent, kind, attributes, links)
         return self.use_span(span, end_on_exit=True)
 
     def create_span(
         self,
         name: str,
         parent: trace_api.ParentSpan = trace_api.Tracer.CURRENT_SPAN,
         kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL,
+        attributes: Optional[types.Attributes] = None,
+        links: Sequence[trace_api.Link] = (),
     ) -> "trace_api.Span":
         """See `opentelemetry.trace.Tracer.create_span`.
 
@@ -412,18 +394,26 @@ def create_span(
             context.trace_id,
             context.span_id,
             name,
-            {},  # TODO: links
+            attributes,
+            links,
         )
 
         if sampling_decision.sampled:
+            if attributes is None:
+                span_attributes = sampling_decision.attributes
+            else:
+                # apply sampling decision attributes after initial attributes
+                span_attributes = attributes.copy()
+                span_attributes.update(sampling_decision.attributes)
             return Span(
                 name=name,
                 context=context,
                 parent=parent,
                 sampler=self.sampler,
-                attributes=sampling_decision.attributes,
+                attributes=span_attributes,
                 span_processor=self._active_span_processor,
                 kind=kind,
+                links=links,
             )
 
         return trace_api.DefaultSpan(context=context)

opentelemetry-sdk/tests/trace/test_trace.py

@@ -248,6 +248,46 @@ def test_attributes(self):
             self.assertEqual(root.attributes["misc.pi"], 3.14)
             self.assertEqual(root.attributes["attr-key"], "attr-value2")
 
+        attributes = {
+            "attr-key": "val",
+            "attr-key2": "val2",
+            "attr-in-both": "span-attr",
+        }
+        with self.tracer.start_as_current_span(
+            "root2", attributes=attributes
+        ) as root:
+            self.assertEqual(len(root.attributes), 3)
+            self.assertEqual(root.attributes["attr-key"], "val")
+            self.assertEqual(root.attributes["attr-key2"], "val2")
+            self.assertEqual(root.attributes["attr-in-both"], "span-attr")
+
+        decision_attributes = {
+            "sampler-attr": "sample-val",
+            "attr-in-both": "decision-attr",
+        }
+        self.tracer.sampler = sampling.StaticSampler(
+            sampling.Decision(sampled=True, attributes=decision_attributes)
+        )
+
+        with self.tracer.start_as_current_span("root2") as root:
+            self.assertEqual(len(root.attributes), 2)
+            self.assertEqual(root.attributes["sampler-attr"], "sample-val")
+            self.assertEqual(root.attributes["attr-in-both"], "decision-attr")
+
+        attributes = {
+            "attr-key": "val",
+            "attr-key2": "val2",
+            "attr-in-both": "span-attr",
+        }
+        with self.tracer.start_as_current_span(
+            "root2", attributes=attributes
+        ) as root:
+            self.assertEqual(len(root.attributes), 4)
+            self.assertEqual(root.attributes["attr-key"], "val")
+            self.assertEqual(root.attributes["attr-key2"], "val2")
+            self.assertEqual(root.attributes["sampler-attr"], "sample-val")
+            self.assertEqual(root.attributes["attr-in-both"], "decision-attr")
+
     def test_events(self):
         self.assertIsNone(self.tracer.get_current_span())
 
@@ -297,13 +337,12 @@ def test_links(self):
             trace_id=trace.generate_trace_id(),
             span_id=trace.generate_span_id(),
         )
-
-        with self.tracer.start_as_current_span("root") as root:
-            root.add_link(other_context1)
-            root.add_link(other_context2, {"name": "neighbor"})
-            root.add_lazy_link(
-                trace_api.Link(other_context3, {"component": "http"})
-            )
+        links = [
+            trace_api.Link(other_context1),
+            trace_api.Link(other_context2, {"name": "neighbor"}),
+            trace_api.Link(other_context3, {"component": "http"}),
+        ]
+        with self.tracer.start_as_current_span("root", links=links) as root:
 
             self.assertEqual(len(root.links), 3)
             self.assertEqual(
@@ -374,11 +413,6 @@ def test_span_override_start_and_end_time(self):
     def test_ended_span(self):
         """"Events, attributes are not allowed after span is ended"""
 
-        other_context1 = trace_api.SpanContext(
-            trace_id=trace.generate_trace_id(),
-            span_id=trace.generate_span_id(),
-        )
-
         with self.tracer.start_as_current_span("root") as root:
             # everything should be empty at the beginning
             self.assertEqual(len(root.attributes), 0)
@@ -400,9 +434,6 @@ def test_ended_span(self):
             root.add_event("event1")
             self.assertEqual(len(root.events), 0)
 
-            root.add_link(other_context1)
-            self.assertEqual(len(root.links), 0)
-
             root.update_name("xxx")
             self.assertEqual(root.name, "root")