Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate no-op from interfaces #311

Merged
merged 13 commits into from
Jan 15, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
36 changes: 32 additions & 4 deletions opentelemetry-api/src/opentelemetry/metrics/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,14 +184,15 @@ 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
counter), or raw values (measure) in which the aggregation and labels
for the exported metric are deferred.
"""

@abc.abstractmethod
def record_batch(
self,
label_set: LabelSet,
Expand All @@ -211,6 +212,7 @@ def record_batch(
corresponding value to record for that metric.
"""

@abc.abstractmethod
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are your thoughts on abstract methods having implementations? I see how it could be useful to have a safe default implementation that the user has to explicitly call with super, but I see that DefaultMeter has its own implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, I'll remove the default implementation from the interfaces. I don't have a strong preference either ways, but it's less changes to remove the few that were left behind than to add an implementation for all abstract methods.

def create_metric(
self,
name: str,
Expand All @@ -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.

Expand All @@ -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()

Expand All @@ -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
Expand Down
125 changes: 105 additions & 20 deletions opentelemetry-api/src/opentelemetry/trace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
by `tracer_source`.
"""

import abc
import enum
import types as python_types
import typing
Expand Down Expand Up @@ -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.

Expand All @@ -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.

Expand All @@ -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,
Expand All @@ -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.

Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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)
c24t marked this conversation as resolved.
Show resolved Hide resolved

@abc.abstractmethod
def get_current_span(self) -> "Span":
"""Gets the currently active span from the context.

Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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]:
Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the right solution for context manager types? I see we've been adding # type: ignore to these since (at least) #92, but couldn't find a discussion about this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is tracked in #219. The best solution I know is still #198 (comment).

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

Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-api/tests/metrics/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading