Skip to content
This repository has been archived by the owner on Oct 19, 2023. It is now read-only.

Commit

Permalink
api: change order of arguments in add_event
Browse files Browse the repository at this point in the history
e4d8949 ("OpenTracing Bridge - Initial implementation (open-telemetry#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.
  • Loading branch information
mauriciovasquezbernal committed Nov 5, 2019
1 parent e53edd5 commit 243fc19
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
4 changes: 2 additions & 2 deletions opentelemetry-api/src/opentelemetry/trace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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`.
Expand Down
4 changes: 2 additions & 2 deletions opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
)

Expand Down
26 changes: 18 additions & 8 deletions opentelemetry-sdk/tests/trace/test_trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,28 +264,38 @@ def test_span_members(self):
self.assertEqual(root.attributes["attr-key"], "attr-value2")

# events
# 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)

self.assertEqual(root.events[3].name, "event3")
self.assertEqual(root.events[3].attributes, {"name": "hello"})
self.assertEqual(root.events[3].timestamp, now)

# links
root.add_link(other_context1)
root.add_link(other_context2, {"name": "neighbor"})
Expand Down

0 comments on commit 243fc19

Please sign in to comment.