API: change order of arguments in add_event (#270)

e4d8949 ("OpenTracing Bridge - Initial implementation (#211)") introduced
a new timestamp argument to the add_event method. This commit moves that
argument to be the last one because it is more common to have event attributes
than an explicitly timestamp.

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

@@ -75,7 +75,7 @@ def log_kv(self, key_values, timestamp=None):
             event_timestamp = None
 
         event_name = util.event_name_from_kv(key_values)
-        self._otel_span.add_event(event_name, event_timestamp, key_values)
+        self._otel_span.add_event(event_name, key_values, event_timestamp)
         return self
 
     @deprecated(reason="This method is deprecated in favor of log_kv")

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

@@ -95,7 +95,7 @@ class Event:
     """A text annotation with a set of attributes."""
 
     def __init__(
-        self, name: str, timestamp: int, attributes: types.Attributes = None
+        self, name: str, attributes: types.Attributes, timestamp: int
     ) -> None:
         self._name = name
         self._attributes = attributes
@@ -182,8 +182,8 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None:
     def add_event(
         self,
         name: str,
-        timestamp: int = None,
         attributes: types.Attributes = None,
+        timestamp: int = None,
     ) -> None:
         """Adds an `Event`.
 

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

@@ -202,14 +202,14 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None:
     def add_event(
         self,
         name: str,
-        timestamp: int = None,
         attributes: types.Attributes = None,
+        timestamp: int = None,
     ) -> None:
         self.add_lazy_event(
             trace_api.Event(
                 name,
-                time_ns() if timestamp is None else timestamp,
                 Span.empty_attributes if attributes is None else attributes,
+                time_ns() if timestamp is None else timestamp,
             )
         )
 

opentelemetry-sdk/tests/trace/test_trace.py

@@ -213,30 +213,15 @@ def test_start_as_current_span_explicit(self):
 
 
 class TestSpan(unittest.TestCase):
+    def setUp(self):
+        self.tracer = trace.Tracer("test_span")
+
     def test_basic_span(self):
         span = trace.Span("name", mock.Mock(spec=trace_api.SpanContext))
         self.assertEqual(span.name, "name")
 
-    def test_span_members(self):
-        tracer = trace.Tracer("test_span_members")
-
-        other_context1 = trace_api.SpanContext(
-            trace_id=trace.generate_trace_id(),
-            span_id=trace.generate_span_id(),
-        )
-        other_context2 = trace_api.SpanContext(
-            trace_id=trace.generate_trace_id(),
-            span_id=trace.generate_span_id(),
-        )
-        other_context3 = trace_api.SpanContext(
-            trace_id=trace.generate_trace_id(),
-            span_id=trace.generate_span_id(),
-        )
-
-        self.assertIsNone(tracer.get_current_span())
-
-        with tracer.start_as_current_span("root") as root:
-            # attributes
+    def test_attributes(self):
+        with self.tracer.start_as_current_span("root") as root:
             root.set_attribute("component", "http")
             root.set_attribute("http.method", "GET")
             root.set_attribute(
@@ -263,30 +248,57 @@ def test_span_members(self):
             self.assertEqual(root.attributes["misc.pi"], 3.14)
             self.assertEqual(root.attributes["attr-key"], "attr-value2")
 
-            # events
+    def test_events(self):
+        self.assertIsNone(self.tracer.get_current_span())
+
+        with self.tracer.start_as_current_span("root") as root:
+            # only event name
             root.add_event("event0")
+
+            # event name and attributes
             now = time_ns()
-            root.add_event(
-                "event1", timestamp=now, attributes={"name": "birthday"}
-            )
+            root.add_event("event1", {"name": "pluto"})
+
+            # event name, attributes and timestamp
+            now = time_ns()
+            root.add_event("event2", {"name": "birthday"}, now)
+
+            # lazy event
             root.add_lazy_event(
-                trace_api.Event("event2", now, {"name": "hello"})
+                trace_api.Event("event3", {"name": "hello"}, now)
             )
 
-            self.assertEqual(len(root.events), 3)
+            self.assertEqual(len(root.events), 4)
 
             self.assertEqual(root.events[0].name, "event0")
             self.assertEqual(root.events[0].attributes, {})
 
             self.assertEqual(root.events[1].name, "event1")
-            self.assertEqual(root.events[1].attributes, {"name": "birthday"})
-            self.assertEqual(root.events[1].timestamp, now)
+            self.assertEqual(root.events[1].attributes, {"name": "pluto"})
 
             self.assertEqual(root.events[2].name, "event2")
-            self.assertEqual(root.events[2].attributes, {"name": "hello"})
+            self.assertEqual(root.events[2].attributes, {"name": "birthday"})
             self.assertEqual(root.events[2].timestamp, now)
 
-            # links
+            self.assertEqual(root.events[3].name, "event3")
+            self.assertEqual(root.events[3].attributes, {"name": "hello"})
+            self.assertEqual(root.events[3].timestamp, now)
+
+    def test_links(self):
+        other_context1 = trace_api.SpanContext(
+            trace_id=trace.generate_trace_id(),
+            span_id=trace.generate_span_id(),
+        )
+        other_context2 = trace_api.SpanContext(
+            trace_id=trace.generate_trace_id(),
+            span_id=trace.generate_span_id(),
+        )
+        other_context3 = 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:
             root.add_link(other_context1)
             root.add_link(other_context2, {"name": "neighbor"})
             root.add_lazy_link(
@@ -313,6 +325,8 @@ def test_span_members(self):
             )
             self.assertEqual(root.links[2].attributes, {"component": "http"})
 
+    def test_update_name(self):
+        with self.tracer.start_as_current_span("root") as root:
             # name
             root.update_name("toor")
             self.assertEqual(root.name, "toor")
@@ -359,14 +373,13 @@ def test_span_override_start_and_end_time(self):
 
     def test_ended_span(self):
         """"Events, attributes are not allowed after span is ended"""
-        tracer = trace.Tracer("test_ended_span")
 
         other_context1 = trace_api.SpanContext(
             trace_id=trace.generate_trace_id(),
             span_id=trace.generate_span_id(),
         )
 
-        with tracer.start_as_current_span("root") as root:
+        with self.tracer.start_as_current_span("root") as root:
             # everything should be empty at the beginning
             self.assertEqual(len(root.attributes), 0)
             self.assertEqual(len(root.events), 0)