From ca794fb48ef28a476960de2a85fbbaecf0f8fc1d Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Wed, 27 Nov 2019 08:16:59 -0800 Subject: [PATCH 1/7] enable --strict flag for mypy Signed-off-by: Alex Boten --- opentelemetry-api/src/opentelemetry/metrics/__init__.py | 2 +- opentelemetry-api/src/opentelemetry/trace/__init__.py | 8 ++++---- opentelemetry-api/src/opentelemetry/trace/sampling.py | 2 +- tox.ini | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/metrics/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/__init__.py index e866aa97cfa..e113efedc2e 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/__init__.py +++ b/opentelemetry-api/src/opentelemetry/metrics/__init__.py @@ -198,7 +198,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": diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index f2abf8ff9b7..383ff659612 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/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: int = 0) -> 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. @@ -174,7 +174,7 @@ def add_event( self, name: str, attributes: types.Attributes = None, - timestamp: int = None, + timestamp: int = 0, ) -> None: """Adds an `Event`. @@ -298,8 +298,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 diff --git a/opentelemetry-api/src/opentelemetry/trace/sampling.py b/opentelemetry-api/src/opentelemetry/trace/sampling.py index 967f4fd46b9..77ec2e3bc38 100644 --- a/opentelemetry-api/src/opentelemetry/trace/sampling.py +++ b/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: Mapping[str, "AttributeValue"] = {}, ) -> None: self.sampled = sampled # type: bool if attributes is None: diff --git a/tox.ini b/tox.ini index 9d8e679dfa0..6121e60a327 100644 --- a/tox.ini +++ b/tox.ini @@ -93,7 +93,7 @@ commands = test: pytest coverage: {toxinidir}/scripts/coverage.sh - mypy: mypy --namespace-packages opentelemetry-api/src/opentelemetry/ + mypy: mypy --strict --namespace-packages opentelemetry-api/src/opentelemetry/ ; For test code, we don't want to enforce the full mypy strictness mypy: mypy --namespace-packages --config-file=mypy-relaxed.ini opentelemetry-api/tests/ From 6583ad6331f8ea8be3b7423bc304de97873feb77 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 28 Nov 2019 10:53:48 -0800 Subject: [PATCH 2/7] Adding test app to both sdk/api Signed-off-by: Alex Boten --- .../src/opentelemetry/trace/__init__.py | 4 ++ .../src/opentelemetry/trace/sampling.py | 2 +- opentelemetry-api/tests/test_app.py | 47 +++++++++++++++++++ opentelemetry-sdk/tests/test_app.py | 46 ++++++++++++++++++ 4 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 opentelemetry-api/tests/test_app.py create mode 100644 opentelemetry-sdk/tests/test_app.py diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 383ff659612..91069aab577 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -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. @@ -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 diff --git a/opentelemetry-api/src/opentelemetry/trace/sampling.py b/opentelemetry-api/src/opentelemetry/trace/sampling.py index 77ec2e3bc38..e0f5f17c756 100644 --- a/opentelemetry-api/src/opentelemetry/trace/sampling.py +++ b/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"] = {}, + attributes: Optional[Mapping[str, "AttributeValue"]] = None, ) -> None: self.sampled = sampled # type: bool if attributes is None: diff --git a/opentelemetry-api/tests/test_app.py b/opentelemetry-api/tests/test_app.py new file mode 100644 index 00000000000..048e8db6d7c --- /dev/null +++ b/opentelemetry-api/tests/test_app.py @@ -0,0 +1,47 @@ +# 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 TestAppWithAPI(unittest.TestCase): + 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) diff --git a/opentelemetry-sdk/tests/test_app.py b/opentelemetry-sdk/tests/test_app.py new file mode 100644 index 00000000000..64c79d30af9 --- /dev/null +++ b/opentelemetry-sdk/tests/test_app.py @@ -0,0 +1,46 @@ +# 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 TestAppWithSDK(unittest.TestCase): + 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) From 175bb3ba40849032b15217b2a28d9f8470c75245 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Tue, 3 Dec 2019 09:54:02 -0800 Subject: [PATCH 3/7] moving --strict to mypy config Signed-off-by: Alex Boten --- mypy.ini | 5 +++++ tox.ini | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/mypy.ini b/mypy.ini index ba375b62b1d..dca41f8c6be 100644 --- a/mypy.ini +++ b/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 \ No newline at end of file diff --git a/tox.ini b/tox.ini index 0615829c8ec..9898094ca3d 100644 --- a/tox.ini +++ b/tox.ini @@ -93,7 +93,7 @@ commands = test: pytest coverage: {toxinidir}/scripts/coverage.sh - mypy: mypy --strict --namespace-packages opentelemetry-api/src/opentelemetry/ + mypy: mypy --namespace-packages opentelemetry-api/src/opentelemetry/ ; For test code, we don't want to enforce the full mypy strictness mypy: mypy --namespace-packages --config-file=mypy-relaxed.ini opentelemetry-api/tests/ From f715c3f01d0f27200a5350d7ee5a380f3fee5515 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Wed, 4 Dec 2019 16:42:12 -0800 Subject: [PATCH 4/7] setting default for label_keys in sdk Signed-off-by: Alex Boten --- .../src/opentelemetry/sdk/metrics/__init__.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index bb495bc1be3..753c35a32e7 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/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: From 1fde0643c7b2582bb3107e54fe16fd0e92884fcc Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Wed, 4 Dec 2019 16:55:31 -0800 Subject: [PATCH 5/7] updating int to Optional[int] Signed-off-by: Alex Boten --- opentelemetry-api/src/opentelemetry/trace/__init__.py | 4 ++-- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 91069aab577..27361b9a436 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/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 = 0) -> 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. @@ -176,7 +176,7 @@ def add_event( self, name: str, attributes: types.Attributes = None, - timestamp: int = 0, + timestamp: typing.Optional[int] = None, ) -> None: """Adds an `Event`. diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index c6fe5829cce..5967960ba34 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/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 From 3c7b7f0dd3f884082bccf6e16521df96fabbd634 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Wed, 4 Dec 2019 19:56:13 -0800 Subject: [PATCH 6/7] adding comment to test Signed-off-by: Alex Boten --- opentelemetry-api/tests/test_app.py | 6 ++++++ opentelemetry-sdk/tests/test_app.py | 8 +++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/opentelemetry-api/tests/test_app.py b/opentelemetry-api/tests/test_app.py index 048e8db6d7c..0cc3d73a6b9 100644 --- a/opentelemetry-api/tests/test_app.py +++ b/opentelemetry-api/tests/test_app.py @@ -12,6 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. +""" +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 +""" import unittest from opentelemetry import metrics, trace diff --git a/opentelemetry-sdk/tests/test_app.py b/opentelemetry-sdk/tests/test_app.py index 64c79d30af9..4c36a464cf6 100644 --- a/opentelemetry-sdk/tests/test_app.py +++ b/opentelemetry-sdk/tests/test_app.py @@ -12,6 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. +""" +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 +""" import unittest from opentelemetry.metrics import DefaultMetric @@ -36,7 +42,7 @@ def test_span(self): # pylint: disable=no-value-for-parameter span = trace.Span() - span = trace.Span("name", INVALID_SPAN_CONTEXT) + span = trace.Span("name", INVALISpanD_SPAN_CONTEXT) self.assertEqual(span.get_context(), INVALID_SPAN_CONTEXT) self.assertIs(span.is_recording_events(), True) From a5ae35d11ebf278c9b37dcb11115c0de531a34c6 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 5 Dec 2019 09:28:05 -0800 Subject: [PATCH 7/7] renaming test and test files, fixing typo Signed-off-by: Alex Boten --- .../{test_app.py => test_implementation.py} | 15 ++++++++------- .../{test_app.py => test_implementation.py} | 17 +++++++++-------- 2 files changed, 17 insertions(+), 15 deletions(-) rename opentelemetry-api/tests/{test_app.py => test_implementation.py} (84%) rename opentelemetry-sdk/tests/{test_app.py => test_implementation.py} (81%) diff --git a/opentelemetry-api/tests/test_app.py b/opentelemetry-api/tests/test_implementation.py similarity index 84% rename from opentelemetry-api/tests/test_app.py rename to opentelemetry-api/tests/test_implementation.py index 0cc3d73a6b9..60bf9dd9fad 100644 --- a/opentelemetry-api/tests/test_app.py +++ b/opentelemetry-api/tests/test_implementation.py @@ -12,18 +12,19 @@ # See the License for the specific language governing permissions and # limitations under the License. -""" -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 -""" import unittest from opentelemetry import metrics, trace -class TestAppWithAPI(unittest.TestCase): +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: diff --git a/opentelemetry-sdk/tests/test_app.py b/opentelemetry-sdk/tests/test_implementation.py similarity index 81% rename from opentelemetry-sdk/tests/test_app.py rename to opentelemetry-sdk/tests/test_implementation.py index 4c36a464cf6..9aaa5fc35a3 100644 --- a/opentelemetry-sdk/tests/test_app.py +++ b/opentelemetry-sdk/tests/test_implementation.py @@ -12,12 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -""" -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 -""" import unittest from opentelemetry.metrics import DefaultMetric @@ -25,7 +19,14 @@ from opentelemetry.trace import INVALID_SPAN, INVALID_SPAN_CONTEXT -class TestAppWithSDK(unittest.TestCase): +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: @@ -42,7 +43,7 @@ def test_span(self): # pylint: disable=no-value-for-parameter span = trace.Span() - span = trace.Span("name", INVALISpanD_SPAN_CONTEXT) + span = trace.Span("name", INVALID_SPAN_CONTEXT) self.assertEqual(span.get_context(), INVALID_SPAN_CONTEXT) self.assertIs(span.is_recording_events(), True)