From ab3515dff8b14c6b894a371730477fa27885ab4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mauricio=20V=C3=A1squez?= Date: Tue, 10 Mar 2020 14:39:27 -0500 Subject: [PATCH] Add support for lazy events and links The computation of attributes is expensive in some conditions, even in some cases those attributes are not used at all. This commit fixes the add_lazy_event method by providing a true solution that delays the attributes calculation until attributes are accessed. It also provides a new LazyLink class. --- .../tests/test_jaeger_exporter.py | 2 +- .../tests/test_otcollector_exporter.py | 2 +- .../tests/test_zipkin_exporter.py | 2 +- .../src/opentelemetry/trace/__init__.py | 73 +++++++---- .../src/opentelemetry/util/types.py | 3 +- .../src/opentelemetry/sdk/trace/__init__.py | 120 +++++++++++++++--- opentelemetry-sdk/tests/trace/test_trace.py | 19 ++- 7 files changed, 169 insertions(+), 52 deletions(-) diff --git a/ext/opentelemetry-ext-jaeger/tests/test_jaeger_exporter.py b/ext/opentelemetry-ext-jaeger/tests/test_jaeger_exporter.py index 08c5a4adeda..6a44b2fe8af 100644 --- a/ext/opentelemetry-ext-jaeger/tests/test_jaeger_exporter.py +++ b/ext/opentelemetry-ext-jaeger/tests/test_jaeger_exporter.py @@ -144,7 +144,7 @@ def test_translate_to_jaeger(self): } event_timestamp = base_time + 50 * 10 ** 6 - event = trace_api.Event( + event = trace.Event( name="event0", timestamp=event_timestamp, attributes=event_attributes, diff --git a/ext/opentelemetry-ext-otcollector/tests/test_otcollector_exporter.py b/ext/opentelemetry-ext-otcollector/tests/test_otcollector_exporter.py index 9a17ea6c94d..ed2dfc2cb56 100644 --- a/ext/opentelemetry-ext-otcollector/tests/test_otcollector_exporter.py +++ b/ext/opentelemetry-ext-otcollector/tests/test_otcollector_exporter.py @@ -103,7 +103,7 @@ def test_translate_to_collector(self): "key_float": 0.3, } event_timestamp = base_time + 50 * 10 ** 6 - event = trace_api.Event( + event = trace.Event( name="event0", timestamp=event_timestamp, attributes=event_attributes, diff --git a/ext/opentelemetry-ext-zipkin/tests/test_zipkin_exporter.py b/ext/opentelemetry-ext-zipkin/tests/test_zipkin_exporter.py index c779c7388f6..84caf0685b2 100644 --- a/ext/opentelemetry-ext-zipkin/tests/test_zipkin_exporter.py +++ b/ext/opentelemetry-ext-zipkin/tests/test_zipkin_exporter.py @@ -126,7 +126,7 @@ def test_export(self): } event_timestamp = base_time + 50 * 10 ** 6 - event = trace_api.Event( + event = trace.Event( name="event0", timestamp=event_timestamp, attributes=event_attributes, diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index a6633e434a0..21fc5977bfd 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -82,48 +82,59 @@ ParentSpan = typing.Optional[typing.Union["Span", "SpanContext"]] -class Link: - """A link to a `Span`.""" - - def __init__( - self, context: "SpanContext", attributes: types.Attributes = None - ) -> None: +class LinkBase(abc.ABC): + def __init__(self, context: "SpanContext") -> None: self._context = context - if attributes is None: - self._attributes = {} # type: types.Attributes - else: - self._attributes = attributes @property def context(self) -> "SpanContext": return self._context @property + @abc.abstractmethod def attributes(self) -> types.Attributes: - return self._attributes + pass -class Event: - """A text annotation with a set of attributes.""" +class Link(LinkBase): + """A link to a `Span`. + + Args: + context: `SpanContext` of the `Span` to link to. + attributes: Link's attributes. + """ def __init__( - self, name: str, attributes: types.Attributes, timestamp: int + self, context: "SpanContext", attributes: types.Attributes = None, ) -> None: - self._name = name + super().__init__(context) self._attributes = attributes - self._timestamp = timestamp - - @property - def name(self) -> str: - return self._name @property def attributes(self) -> types.Attributes: return self._attributes + +class LazyLink(LinkBase): + """A link to a `Span`. + + Args: + context: `SpanContext` of the `Span` to link to. + link_formatter: Callable object that returns the attributes of the + Link. + """ + + def __init__( + self, + context: "SpanContext", + link_formatter: types.AttributesFormatter, + ) -> None: + super().__init__(context) + self._link_formatter = link_formatter + @property - def timestamp(self) -> int: - return self._timestamp + def attributes(self) -> types.Attributes: + return self._link_formatter() class SpanKind(enum.Enum): @@ -201,10 +212,17 @@ def add_event( """ @abc.abstractmethod - def add_lazy_event(self, event: Event) -> None: + def add_lazy_event( + self, + name: str, + event_formatter: types.AttributesFormatter, + timestamp: typing.Optional[int] = None, + ) -> None: """Adds an `Event`. - Adds an `Event` that has previously been created. + Adds a single `Event` with the name, an event formatter that calculates + the attributes lazily and, optionally, a timestamp. Implementations + should generate a timestamp if the `timestamp` argument is omitted. """ @abc.abstractmethod @@ -384,7 +402,12 @@ def add_event( ) -> None: pass - def add_lazy_event(self, event: Event) -> None: + def add_lazy_event( + self, + name: str, + event_formatter: types.AttributesFormatter, + timestamp: typing.Optional[int] = None, + ) -> None: pass def update_name(self, name: str) -> None: diff --git a/opentelemetry-api/src/opentelemetry/util/types.py b/opentelemetry-api/src/opentelemetry/util/types.py index 5ce93d84b25..6b55afea39b 100644 --- a/opentelemetry-api/src/opentelemetry/util/types.py +++ b/opentelemetry-api/src/opentelemetry/util/types.py @@ -13,7 +13,7 @@ # limitations under the License. -from typing import Dict, Optional, Sequence, Union +from typing import Callable, Dict, Optional, Sequence, Union AttributeValue = Union[ str, @@ -26,3 +26,4 @@ Sequence[float], ] Attributes = Optional[Dict[str, AttributeValue]] +AttributesFormatter = Callable[[], Optional[Dict[str, AttributeValue]]] diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 0c98cc33af6..234c353477c 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -13,6 +13,7 @@ # limitations under the License. +import abc import atexit import logging import random @@ -114,6 +115,77 @@ def shutdown(self) -> None: sp.shutdown() +class EventBase(abc.ABC): + def __init__(self, name: str, timestamp: Optional[int] = None) -> None: + self._name = name + if timestamp is None: + self._timestamp = time_ns() + else: + self._timestamp = timestamp + + @property + def name(self) -> str: + return self._name + + @property + def timestamp(self) -> int: + return self._timestamp + + @property + @abc.abstractmethod + def attributes(self) -> types.Attributes: + pass + + +class Event(EventBase): + """A text annotation with a set of attributes. + + Args: + name: Name of the event. + attributes: Attributes of the event. + timestamp: Timestamp of the event. If `None` it will filled + automatically. + """ + + def __init__( + self, + name: str, + attributes: types.Attributes = None, + timestamp: Optional[int] = None, + ) -> None: + super().__init__(name, timestamp) + self._attributes = attributes + + @property + def attributes(self) -> types.Attributes: + return self._attributes + + +class LazyEvent(EventBase): + """A text annotation with a set of attributes. + + Args: + name: Name of the event. + event_formatter: Callable object that returns the attributes of the + event. + timestamp: Timestamp of the event. If `None` it will filled + automatically. + """ + + def __init__( + self, + name: str, + event_formatter: types.AttributesFormatter, + timestamp: Optional[int] = None, + ) -> None: + super().__init__(name, timestamp) + self._event_formatter = event_formatter + + @property + def attributes(self) -> types.Attributes: + return self._event_formatter() + + class Span(trace_api.Span): """See `opentelemetry.trace.Span`. @@ -149,7 +221,7 @@ def __init__( trace_config: None = None, # TODO resource: None = None, attributes: types.Attributes = None, # TODO - events: Sequence[trace_api.Event] = None, # TODO + events: Sequence[Event] = None, # TODO links: Sequence[trace_api.Link] = (), kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL, span_processor: SpanProcessor = SpanProcessor(), @@ -266,21 +338,7 @@ def _check_attribute_value_sequence(sequence: Sequence) -> Optional[str]: return "different type" return None - def add_event( - self, - name: str, - attributes: types.Attributes = None, - timestamp: Optional[int] = None, - ) -> None: - self.add_lazy_event( - trace_api.Event( - name, - Span._empty_attributes if attributes is None else attributes, - time_ns() if timestamp is None else timestamp, - ) - ) - - def add_lazy_event(self, event: trace_api.Event) -> None: + def _add_event(self, event: EventBase) -> None: with self._lock: if not self.is_recording_events(): return @@ -293,6 +351,36 @@ def add_lazy_event(self, event: trace_api.Event) -> None: return self.events.append(event) + def add_event( + self, + name: str, + attributes: types.Attributes = None, + timestamp: Optional[int] = None, + ) -> None: + if attributes is None: + attributes = Span._empty_attributes + self._add_event( + Event( + name=name, + attributes=attributes, + timestamp=time_ns() if timestamp is None else timestamp, + ) + ) + + def add_lazy_event( + self, + name: str, + event_formatter: types.AttributesFormatter, + timestamp: Optional[int] = None, + ) -> None: + self._add_event( + LazyEvent( + name=name, + event_formatter=event_formatter, + timestamp=time_ns() if timestamp is None else timestamp, + ) + ) + def start(self, start_time: Optional[int] = None) -> None: with self._lock: if not self.is_recording_events(): diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index a0e22f93113..bb935f6c7bd 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -539,10 +539,11 @@ def test_events(self): now = time_ns() root.add_event("event2", {"name": "birthday"}, now) + def event_formatter(): + return {"name": "hello"} + # lazy event - root.add_lazy_event( - trace_api.Event("event3", {"name": "hello"}, now) - ) + root.add_lazy_event("event3", event_formatter, now) self.assertEqual(len(root.events), 4) @@ -573,11 +574,15 @@ def test_links(self): trace_id=trace.generate_trace_id(), span_id=trace.generate_span_id(), ) - links = [ + + def get_link_attributes(): + return {"component": "http"} + + links = ( trace_api.Link(other_context1), trace_api.Link(other_context2, {"name": "neighbor"}), - trace_api.Link(other_context3, {"component": "http"}), - ] + trace_api.LazyLink(other_context3, get_link_attributes), + ) with self.tracer.start_as_current_span("root", links=links) as root: self.assertEqual(len(root.links), 3) @@ -587,7 +592,7 @@ def test_links(self): self.assertEqual( root.links[0].context.span_id, other_context1.span_id ) - self.assertEqual(root.links[0].attributes, {}) + self.assertEqual(root.links[0].attributes, None) self.assertEqual( root.links[1].context.trace_id, other_context2.trace_id )