Skip to content

Commit

Permalink
Ensure the API returns right value types (open-telemetry#307)
Browse files Browse the repository at this point in the history
Fixes open-telemetry#142

Enabling --strict mode for mypy. Added a test in the sdk and the same test in
the api to test the different behaviours between the Tracer, Span and Metric
classes.
  • Loading branch information
alrex authored and c24t committed Dec 6, 2019
1 parent 15991af commit cfecca1
Show file tree
Hide file tree
Showing 8 changed files with 129 additions and 13 deletions.
5 changes: 5 additions & 0 deletions mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@
disallow_incomplete_defs = True
check_untyped_defs = True
disallow_untyped_decorators = True
warn_unused_configs = True
warn_unused_ignores = True
warn_return_any = True
warn_redundant_casts = True
strict_equality = True
strict_optional = True
no_implicit_optional = True
no_implicit_reexport = True
2 changes: 1 addition & 1 deletion opentelemetry-api/src/opentelemetry/metrics/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ def create_metric(
unit: str,
value_type: Type[ValueT],
metric_type: Type[MetricT],
label_keys: Sequence[str] = None,
label_keys: Sequence[str] = (),
enabled: bool = True,
monotonic: bool = False,
) -> "Metric":
Expand Down
12 changes: 8 additions & 4 deletions opentelemetry-api/src/opentelemetry/trace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ class SpanKind(enum.Enum):
class Span:
"""A span represents a single operation within a trace."""

def end(self, end_time: int = None) -> None:
def end(self, end_time: typing.Optional[int] = None) -> None:
"""Sets the current time as the span's end time.
The span's end time is the wall time at which the operation finished.
Expand All @@ -163,6 +163,8 @@ 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

def set_attribute(self, key: str, value: types.AttributeValue) -> None:
"""Sets an Attribute.
Expand All @@ -174,7 +176,7 @@ def add_event(
self,
name: str,
attributes: types.Attributes = None,
timestamp: int = None,
timestamp: typing.Optional[int] = None,
) -> None:
"""Adds an `Event`.
Expand Down Expand Up @@ -204,6 +206,8 @@ def is_recording_events(self) -> bool:
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

def set_status(self, status: Status) -> None:
"""Sets the Status of the Span. If used, this will override the default
Expand Down Expand Up @@ -298,8 +302,8 @@ def __init__(
self,
trace_id: int,
span_id: int,
trace_options: "TraceOptions" = None,
trace_state: "TraceState" = None,
trace_options: "TraceOptions" = DEFAULT_TRACE_OPTIONS,
trace_state: "TraceState" = DEFAULT_TRACE_STATE,
) -> None:
if trace_options is None:
trace_options = DEFAULT_TRACE_OPTIONS
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-api/src/opentelemetry/trace/sampling.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def __repr__(self) -> str:
def __init__(
self,
sampled: bool = False,
attributes: Mapping[str, "AttributeValue"] = None,
attributes: Optional[Mapping[str, "AttributeValue"]] = None,
) -> None:
self.sampled = sampled # type: bool
if attributes is None:
Expand Down
54 changes: 54 additions & 0 deletions opentelemetry-api/tests/test_implementation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# Copyright 2019, OpenTelemetry Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import unittest

from opentelemetry import metrics, trace


class TestAPIOnlyImplementation(unittest.TestCase):
"""
This test is in place to ensure the API is returning values that
are valid. The same tests have been added to the SDK with
different expected results. See issue for more details:
https://github.com/open-telemetry/opentelemetry-python/issues/142
"""

def test_tracer(self):
tracer = trace.Tracer()
with tracer.start_span("test") as span:
self.assertEqual(span.get_context(), trace.INVALID_SPAN_CONTEXT)
self.assertEqual(span, trace.INVALID_SPAN)
self.assertIs(span.is_recording_events(), False)
with tracer.start_span("test2") as span2:
self.assertEqual(
span2.get_context(), trace.INVALID_SPAN_CONTEXT
)
self.assertEqual(span2, trace.INVALID_SPAN)
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)

def test_default_span(self):
span = trace.DefaultSpan(trace.INVALID_SPAN_CONTEXT)
self.assertEqual(span.get_context(), trace.INVALID_SPAN_CONTEXT)
self.assertIs(span.is_recording_events(), False)

def test_meter(self):
meter = metrics.Meter()
metric = meter.create_metric("", "", "", float, metrics.Counter)
self.assertIsInstance(metric, metrics.DefaultMetric)
10 changes: 5 additions & 5 deletions opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def __init__(
description: str,
unit: str,
value_type: Type[metrics_api.ValueT],
label_keys: Sequence[str] = None,
label_keys: Sequence[str] = (),
enabled: bool = True,
monotonic: bool = False,
):
Expand Down Expand Up @@ -150,7 +150,7 @@ def __init__(
description: str,
unit: str,
value_type: Type[metrics_api.ValueT],
label_keys: Sequence[str] = None,
label_keys: Sequence[str] = (),
enabled: bool = True,
monotonic: bool = True,
):
Expand Down Expand Up @@ -186,7 +186,7 @@ def __init__(
description: str,
unit: str,
value_type: Type[metrics_api.ValueT],
label_keys: Sequence[str] = None,
label_keys: Sequence[str] = (),
enabled: bool = True,
monotonic: bool = False,
):
Expand Down Expand Up @@ -222,7 +222,7 @@ def __init__(
description: str,
unit: str,
value_type: Type[metrics_api.ValueT],
label_keys: Sequence[str] = None,
label_keys: Sequence[str] = (),
enabled: bool = False,
monotonic: bool = False,
):
Expand Down Expand Up @@ -269,7 +269,7 @@ def create_metric(
unit: str,
value_type: Type[metrics_api.ValueT],
metric_type: Type[metrics_api.MetricT],
label_keys: Sequence[str] = None,
label_keys: Sequence[str] = (),
enabled: bool = True,
monotonic: bool = False,
) -> metrics_api.MetricT:
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 @@ -204,7 +204,7 @@ def add_event(
self,
name: str,
attributes: types.Attributes = None,
timestamp: int = None,
timestamp: Optional[int] = None,
) -> None:
self.add_lazy_event(
trace_api.Event(
Expand Down Expand Up @@ -241,7 +241,7 @@ def start(self, start_time: Optional[int] = None) -> None:
return
self.span_processor.on_start(self)

def end(self, end_time: int = None) -> None:
def end(self, end_time: Optional[int] = None) -> None:
with self._lock:
if not self.is_recording_events():
return
Expand Down
53 changes: 53 additions & 0 deletions opentelemetry-sdk/tests/test_implementation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# Copyright 2019, OpenTelemetry Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import unittest

from opentelemetry.metrics import DefaultMetric
from opentelemetry.sdk import metrics, trace
from opentelemetry.trace import INVALID_SPAN, INVALID_SPAN_CONTEXT


class TestSDKImplementation(unittest.TestCase):
"""
This test is in place to ensure the SDK implementation of the API
is returning values that are valid. The same tests have been added
to the API with different expected results. See issue for more details:
https://github.com/open-telemetry/opentelemetry-python/issues/142
"""

def test_tracer(self):
tracer = trace.Tracer()
with tracer.start_span("test") as span:
self.assertNotEqual(span.get_context(), INVALID_SPAN_CONTEXT)
self.assertNotEqual(span, INVALID_SPAN)
self.assertIs(span.is_recording_events(), True)
with tracer.start_span("test2") as span2:
self.assertNotEqual(span2.get_context(), INVALID_SPAN_CONTEXT)
self.assertNotEqual(span2, INVALID_SPAN)
self.assertIs(span2.is_recording_events(), True)

def test_span(self):
with self.assertRaises(Exception):
# pylint: disable=no-value-for-parameter
span = trace.Span()

span = trace.Span("name", INVALID_SPAN_CONTEXT)
self.assertEqual(span.get_context(), INVALID_SPAN_CONTEXT)
self.assertIs(span.is_recording_events(), True)

def test_meter(self):
meter = metrics.Meter()
metric = meter.create_metric("", "", "", float, metrics.Counter)
self.assertNotIsInstance(metric, DefaultMetric)

0 comments on commit cfecca1

Please sign in to comment.