From ada53ff5120e3554fe055b1c953b0c3087628ef7 Mon Sep 17 00:00:00 2001 From: alrex Date: Wed, 15 Jan 2020 13:40:21 -0800 Subject: [PATCH] Separate no-op from interfaces (#311) Fixes #66 --- .../tests/test_dbapi_integration.py | 2 +- .../tests/test_requests_integration.py | 4 +- .../tests/test_mysql_integration.py | 2 +- .../src/opentelemetry/metrics/__init__.py | 36 ++++- .../src/opentelemetry/trace/__init__.py | 125 +++++++++++++++--- .../tests/metrics/test_metrics.py | 2 +- .../tests/test_implementation.py | 20 ++- opentelemetry-api/tests/test_loader.py | 9 +- opentelemetry-api/tests/trace/test_tracer.py | 4 +- 9 files changed, 166 insertions(+), 38 deletions(-) diff --git a/ext/opentelemetry-ext-dbapi/tests/test_dbapi_integration.py b/ext/opentelemetry-ext-dbapi/tests/test_dbapi_integration.py index f5d1299e838..afe5a49a02f 100644 --- a/ext/opentelemetry-ext-dbapi/tests/test_dbapi_integration.py +++ b/ext/opentelemetry-ext-dbapi/tests/test_dbapi_integration.py @@ -21,7 +21,7 @@ class TestDBApiIntegration(unittest.TestCase): def setUp(self): - self.tracer = trace_api.Tracer() + self.tracer = trace_api.DefaultTracer() self.span = MockSpan() self.start_current_span_patcher = mock.patch.object( self.tracer, diff --git a/ext/opentelemetry-ext-http-requests/tests/test_requests_integration.py b/ext/opentelemetry-ext-http-requests/tests/test_requests_integration.py index 35cf3110f3e..de659f20e18 100644 --- a/ext/opentelemetry-ext-http-requests/tests/test_requests_integration.py +++ b/ext/opentelemetry-ext-http-requests/tests/test_requests_integration.py @@ -29,8 +29,8 @@ class TestRequestsIntegration(unittest.TestCase): # TODO: Copy & paste from test_wsgi_middleware def setUp(self): self.span_attrs = {} - self.tracer_source = trace.TracerSource() - self.tracer = trace.Tracer() + self.tracer_source = trace.DefaultTracerSource() + self.tracer = trace.DefaultTracer() self.get_tracer_patcher = mock.patch.object( self.tracer_source, "get_tracer", diff --git a/ext/opentelemetry-ext-mysql/tests/test_mysql_integration.py b/ext/opentelemetry-ext-mysql/tests/test_mysql_integration.py index 1bcd851750c..3b6eaa0c642 100644 --- a/ext/opentelemetry-ext-mysql/tests/test_mysql_integration.py +++ b/ext/opentelemetry-ext-mysql/tests/test_mysql_integration.py @@ -23,7 +23,7 @@ class TestMysqlIntegration(unittest.TestCase): def test_trace_integration(self): - tracer = trace_api.Tracer() + tracer = trace_api.DefaultTracer() span = mock.create_autospec(trace_api.Span, spec_set=True) start_current_span_patcher = mock.patch.object( tracer, diff --git a/opentelemetry-api/src/opentelemetry/metrics/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/__init__.py index 4946300e15c..947d57b976d 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/__init__.py +++ b/opentelemetry-api/src/opentelemetry/metrics/__init__.py @@ -184,7 +184,7 @@ def record(self, label_set: LabelSet, value: ValueT) -> None: # pylint: disable=unused-argument -class Meter: +class Meter(abc.ABC): """An interface to allow the recording of metrics. `Metric` s are used for recording pre-defined aggregation (gauge and @@ -192,6 +192,7 @@ class Meter: for the exported metric are deferred. """ + @abc.abstractmethod def record_batch( self, label_set: LabelSet, @@ -211,6 +212,7 @@ def record_batch( corresponding value to record for that metric. """ + @abc.abstractmethod def create_metric( self, name: str, @@ -236,9 +238,8 @@ def create_metric( Returns: A new ``metric_type`` metric with values of ``value_type``. """ - # pylint: disable=no-self-use - return DefaultMetric() + @abc.abstractmethod def get_label_set(self, labels: Dict[str, str]) -> "LabelSet": """Gets a `LabelSet` with the given labels. @@ -247,6 +248,33 @@ def get_label_set(self, labels: Dict[str, str]) -> "LabelSet": Returns: A `LabelSet` object canonicalized using the given input. """ + + +class DefaultMeter(Meter): + """The default Meter used when no Meter implementation is available.""" + + def record_batch( + self, + label_set: LabelSet, + record_tuples: Sequence[Tuple["Metric", ValueT]], + ) -> None: + pass + + def create_metric( + self, + name: str, + description: str, + unit: str, + value_type: Type[ValueT], + metric_type: Type[MetricT], + label_keys: Sequence[str] = (), + enabled: bool = True, + monotonic: bool = False, + ) -> "Metric": + # pylint: disable=no-self-use + return DefaultMetric() + + def get_label_set(self, labels: Dict[str, str]) -> "LabelSet": # pylint: disable=no-self-use return DefaultLabelSet() @@ -269,7 +297,7 @@ def meter() -> Meter: if _METER is None: # pylint:disable=protected-access - _METER = loader._load_impl(Meter, _METER_FACTORY) + _METER = loader._load_impl(DefaultMeter, _METER_FACTORY) del _METER_FACTORY return _METER diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index e426d11a1a7..014e82b3bc2 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -66,6 +66,7 @@ by `tracer_source`. """ +import abc import enum import types as python_types import typing @@ -151,9 +152,10 @@ class SpanKind(enum.Enum): CONSUMER = 4 -class Span: +class Span(abc.ABC): """A span represents a single operation within a trace.""" + @abc.abstractmethod def end(self, end_time: typing.Optional[int] = None) -> None: """Sets the current time as the span's end time. @@ -163,6 +165,7 @@ def end(self, end_time: typing.Optional[int] = None) -> None: implementations are free to ignore or raise on further calls. """ + @abc.abstractmethod def get_context(self) -> "SpanContext": """Gets the span's SpanContext. @@ -172,15 +175,15 @@ def get_context(self) -> "SpanContext": Returns: A :class:`.SpanContext` with a copy of this span's immutable state. """ - # pylint: disable=no-self-use - return INVALID_SPAN_CONTEXT + @abc.abstractmethod def set_attribute(self, key: str, value: types.AttributeValue) -> None: """Sets an Attribute. Sets a single Attribute with the key and value passed as arguments. """ + @abc.abstractmethod def add_event( self, name: str, @@ -194,12 +197,14 @@ def add_event( timestamp if the `timestamp` argument is omitted. """ + @abc.abstractmethod def add_lazy_event(self, event: Event) -> None: """Adds an `Event`. Adds an `Event` that has previously been created. """ + @abc.abstractmethod def update_name(self, name: str) -> None: """Updates the `Span` name. @@ -209,15 +214,15 @@ def update_name(self, name: str) -> None: on the implementation. """ + @abc.abstractmethod def is_recording_events(self) -> bool: """Returns whether this span will be recorded. Returns true if this Span is active and recording information like events with the add_event operation and attributes using set_attribute. """ - # pylint: disable=no-self-use - return False + @abc.abstractmethod def set_status(self, status: Status) -> None: """Sets the Status of the Span. If used, this will override the default Span status, which is OK. @@ -362,6 +367,29 @@ def get_context(self) -> "SpanContext": def is_recording_events(self) -> bool: return False + def end(self, end_time: typing.Optional[int] = None) -> None: + pass + + def set_attribute(self, key: str, value: types.AttributeValue) -> None: + pass + + def add_event( + self, + name: str, + attributes: types.Attributes = None, + timestamp: typing.Optional[int] = None, + ) -> None: + pass + + def add_lazy_event(self, event: Event) -> None: + pass + + def update_name(self, name: str) -> None: + pass + + def set_status(self, status: Status) -> None: + pass + INVALID_SPAN_ID = 0x0000000000000000 INVALID_TRACE_ID = 0x00000000000000000000000000000000 @@ -374,8 +402,8 @@ def is_recording_events(self) -> bool: INVALID_SPAN = DefaultSpan(INVALID_SPAN_CONTEXT) -class TracerSource: - # pylint:disable=no-self-use,unused-argument +class TracerSource(abc.ABC): + @abc.abstractmethod def get_tracer( self, instrumenting_module_name: str, @@ -402,10 +430,24 @@ def get_tracer( instrumenting library. Usually this should be the same as ``pkg_resources.get_distribution(instrumenting_library_name).version``. """ - return Tracer() -class Tracer: +class DefaultTracerSource(TracerSource): + """The default TracerSource, used when no implementation is available. + + All operations are no-op. + """ + + def get_tracer( + self, + instrumenting_module_name: str, + instrumenting_library_version: str = "", + ) -> "Tracer": + # pylint:disable=no-self-use,unused-argument + return DefaultTracer() + + +class Tracer(abc.ABC): """Handles span creation and in-process context propagation. This class provides methods for manipulating the context, creating spans, @@ -414,8 +456,9 @@ class Tracer: # Constant used to represent the current span being used as a parent. # This is the default behavior when creating spans. - CURRENT_SPAN = Span() + CURRENT_SPAN = DefaultSpan(INVALID_SPAN_CONTEXT) + @abc.abstractmethod def get_current_span(self) -> "Span": """Gets the currently active span from the context. @@ -426,9 +469,8 @@ def get_current_span(self) -> "Span": The currently active :class:`.Span`, or a placeholder span with an invalid :class:`.SpanContext`. """ - # pylint: disable=no-self-use - return INVALID_SPAN + @abc.abstractmethod def start_span( self, name: str, @@ -478,10 +520,9 @@ def start_span( Returns: The newly-created span. """ - # pylint: disable=unused-argument,no-self-use - return INVALID_SPAN @contextmanager # type: ignore + @abc.abstractmethod def start_as_current_span( self, name: str, @@ -531,10 +572,8 @@ def start_as_current_span( The newly-created span. """ - # pylint: disable=unused-argument,no-self-use - yield INVALID_SPAN - @contextmanager # type: ignore + @abc.abstractmethod def use_span( self, span: "Span", end_on_exit: bool = False ) -> typing.Iterator[None]: @@ -552,6 +591,47 @@ def use_span( end_on_exit: Whether to end the span automatically when leaving the context manager. """ + + +class DefaultTracer(Tracer): + """The default Tracer, used when no Tracer implementation is available. + + All operations are no-op. + """ + + def get_current_span(self) -> "Span": + # pylint: disable=no-self-use + return INVALID_SPAN + + def start_span( + self, + name: str, + parent: ParentSpan = Tracer.CURRENT_SPAN, + kind: SpanKind = SpanKind.INTERNAL, + attributes: typing.Optional[types.Attributes] = None, + links: typing.Sequence[Link] = (), + start_time: typing.Optional[int] = None, + set_status_on_exception: bool = True, + ) -> "Span": + # pylint: disable=unused-argument,no-self-use + return INVALID_SPAN + + @contextmanager # type: ignore + def start_as_current_span( + self, + name: str, + parent: ParentSpan = Tracer.CURRENT_SPAN, + kind: SpanKind = SpanKind.INTERNAL, + attributes: typing.Optional[types.Attributes] = None, + links: typing.Sequence[Link] = (), + ) -> typing.Iterator["Span"]: + # pylint: disable=unused-argument,no-self-use + yield INVALID_SPAN + + @contextmanager # type: ignore + def use_span( + self, span: "Span", end_on_exit: bool = False + ) -> typing.Iterator[None]: # pylint: disable=unused-argument,no-self-use yield @@ -576,9 +656,14 @@ def tracer_source() -> TracerSource: if _TRACER_SOURCE is None: # pylint:disable=protected-access - _TRACER_SOURCE = loader._load_impl( - TracerSource, _TRACER_SOURCE_FACTORY - ) + try: + _TRACER_SOURCE = loader._load_impl( + TracerSource, _TRACER_SOURCE_FACTORY # type: ignore + ) + except TypeError: + # if we raised an exception trying to instantiate an + # abstract class, default to no-op tracer impl + _TRACER_SOURCE = DefaultTracerSource() del _TRACER_SOURCE_FACTORY return _TRACER_SOURCE diff --git a/opentelemetry-api/tests/metrics/test_metrics.py b/opentelemetry-api/tests/metrics/test_metrics.py index f8610a6fa4c..a8959266b28 100644 --- a/opentelemetry-api/tests/metrics/test_metrics.py +++ b/opentelemetry-api/tests/metrics/test_metrics.py @@ -20,7 +20,7 @@ # pylint: disable=no-self-use class TestMeter(unittest.TestCase): def setUp(self): - self.meter = metrics.Meter() + self.meter = metrics.DefaultMeter() def test_record_batch(self): counter = metrics.Counter() diff --git a/opentelemetry-api/tests/test_implementation.py b/opentelemetry-api/tests/test_implementation.py index cd126229f9a..c7d1d453a1c 100644 --- a/opentelemetry-api/tests/test_implementation.py +++ b/opentelemetry-api/tests/test_implementation.py @@ -26,7 +26,12 @@ class TestAPIOnlyImplementation(unittest.TestCase): """ def test_tracer(self): - tracer_source = trace.TracerSource() + with self.assertRaises(TypeError): + # pylint: disable=abstract-class-instantiated + trace.TracerSource() # type:ignore + + def test_default_tracer(self): + tracer_source = trace.DefaultTracerSource() tracer = tracer_source.get_tracer(__name__) with tracer.start_span("test") as span: self.assertEqual(span.get_context(), trace.INVALID_SPAN_CONTEXT) @@ -40,9 +45,9 @@ def test_tracer(self): self.assertIs(span2.is_recording_events(), False) def test_span(self): - span = trace.Span() - self.assertEqual(span.get_context(), trace.INVALID_SPAN_CONTEXT) - self.assertIs(span.is_recording_events(), False) + with self.assertRaises(TypeError): + # pylint: disable=abstract-class-instantiated + trace.Span() # type:ignore def test_default_span(self): span = trace.DefaultSpan(trace.INVALID_SPAN_CONTEXT) @@ -50,6 +55,11 @@ def test_default_span(self): self.assertIs(span.is_recording_events(), False) def test_meter(self): - meter = metrics.Meter() + with self.assertRaises(TypeError): + # pylint: disable=abstract-class-instantiated + metrics.Meter() # type:ignore + + def test_default_meter(self): + meter = metrics.DefaultMeter() metric = meter.create_metric("", "", "", float, metrics.Counter) self.assertIsInstance(metric, metrics.DefaultMetric) diff --git a/opentelemetry-api/tests/test_loader.py b/opentelemetry-api/tests/test_loader.py index 8ac397afcb3..eda241615fe 100644 --- a/opentelemetry-api/tests/test_loader.py +++ b/opentelemetry-api/tests/test_loader.py @@ -25,7 +25,12 @@ class DummyTracerSource(trace.TracerSource): - pass + def get_tracer( + self, + instrumenting_module_name: str, + instrumenting_library_version: str = "", + ) -> "trace.Tracer": + return trace.DefaultTracer() def get_opentelemetry_implementation(type_): @@ -49,7 +54,7 @@ def setUp(self): def test_get_default(self): tracer_source = trace.tracer_source() - self.assertIs(type(tracer_source), trace.TracerSource) + self.assertIs(type(tracer_source), trace.DefaultTracerSource) def test_preferred_impl(self): trace.set_preferred_tracer_source_implementation( diff --git a/opentelemetry-api/tests/trace/test_tracer.py b/opentelemetry-api/tests/trace/test_tracer.py index b57f2ff6d2b..20c218ad8f1 100644 --- a/opentelemetry-api/tests/trace/test_tracer.py +++ b/opentelemetry-api/tests/trace/test_tracer.py @@ -19,7 +19,7 @@ class TestTracer(unittest.TestCase): def setUp(self): - self.tracer = trace.Tracer() + self.tracer = trace.DefaultTracer() def test_get_current_span(self): span = self.tracer.get_current_span() @@ -34,6 +34,6 @@ def test_start_as_current_span(self): self.assertIsInstance(span, trace.Span) def test_use_span(self): - span = trace.Span() + span = trace.DefaultSpan(trace.INVALID_SPAN_CONTEXT) with self.tracer.use_span(span): pass