Ensure the API returns right value types (open-telemetry#307)

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.

Author: alrex

mypy.ini

@@ -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

opentelemetry-api/src/opentelemetry/metrics/__init__.py

@@ -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":

opentelemetry-api/src/opentelemetry/trace/__init__.py

@@ -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.
@@ -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.
@@ -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`.
 
@@ -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
@@ -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

opentelemetry-api/src/opentelemetry/trace/sampling.py

@@ -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:

opentelemetry-api/tests/test_implementation.py

@@ -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)

opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py

@@ -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,
     ):
@@ -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,
     ):
@@ -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,
     ):
@@ -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,
     ):
@@ -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:

opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py

@@ -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(
@@ -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

opentelemetry-sdk/tests/test_implementation.py

@@ -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)