-
Notifications
You must be signed in to change notification settings - Fork 706
Ensure the API returns right value types #307
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
Changes from 5 commits
ca794fb
3316b2b
6583ad6
3117a8e
175bb3b
2ce41ed
f715c3f
1fde064
3c7b7f0
a5ae35d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call. Updated to |
||
"""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 | ||
c24t marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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: int = 0, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment about the epoch here. |
||
) -> 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: | ||
Oberon00 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
trace_options = DEFAULT_TRACE_OPTIONS | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test looks great, but might be hard to understand out of context. An explanation in the test or module docstring and reference to the other test in each file would help, especially since we'll need to keep these in sync with each other. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a comment to help clarify it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer having that comment as the docstring for the test class instead of the test module, since I routinely skip until after the import statements when reading a file. But that's just my personal preference. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this called There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the suggestion, it's much clearer. I've renamed the files to test_implementation as well. |
||
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) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now I'm confused. Why is there |
||
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) |
Uh oh!
There was an error while loading. Please reload this page.