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

Set status for ended spans #297

Merged
merged 1 commit into from
Dec 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions opentelemetry-api/src/opentelemetry/trace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,12 @@ class SpanKind(enum.Enum):
https://github.com/open-telemetry/opentelemetry-specification/pull/226.
"""

#: Default value. Indicates that the span is used internally in the application.
#: Default value. Indicates that the span is used internally in the
# application.
INTERNAL = 0

#: Indicates that the span describes an operation that handles a remote request.
#: Indicates that the span describes an operation that handles a remote
# request.
SERVER = 1

#: Indicates that the span describes a request to some remote service.
Expand Down Expand Up @@ -228,6 +230,7 @@ def __exit__(
exc_tb: typing.Optional[python_types.TracebackType],
) -> None:
"""Ends context manager and calls `end` on the `Span`."""

self.end()


Expand Down Expand Up @@ -396,6 +399,7 @@ def start_span(
attributes: typing.Optional[types.Attributes] = None,
links: typing.Sequence[Link] = (),
start_time: typing.Optional[int] = None,
set_status_on_exception: bool = True,
) -> "Span":
"""Starts a span.

Expand Down Expand Up @@ -427,6 +431,11 @@ def start_span(
attributes: The span's attributes.
links: Links span to other spans
start_time: Sets the start time of a span
set_status_on_exception: Only relevant if the returned span is used
in a with/context manager. Defines wether the span status will
be automatically set to UNKNOWN when an uncaught exception is
raised in the span with block. The span status won't be set by
this mechanism if it was previousy set manually.

Returns:
The newly-created span.
Expand Down
4 changes: 2 additions & 2 deletions opentelemetry-api/src/opentelemetry/trace/status.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,14 +163,14 @@ class Status:

def __init__(
self,
canonical_code: "StatusCanonicalCode" = StatusCanonicalCode.OK,
canonical_code: StatusCanonicalCode = StatusCanonicalCode.OK,
description: typing.Optional[str] = None,
):
self._canonical_code = canonical_code
self._description = description

@property
def canonical_code(self) -> "StatusCanonicalCode":
def canonical_code(self) -> StatusCanonicalCode:
"""Represents the canonical status code of a finished Span."""
return self._canonical_code

Expand Down
43 changes: 39 additions & 4 deletions opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@
import random
import threading
from contextlib import contextmanager
from typing import Iterator, Optional, Sequence, Tuple
from types import TracebackType
from typing import Iterator, Optional, Sequence, Tuple, Type

from opentelemetry import trace as trace_api
from opentelemetry.context import Context
from opentelemetry.sdk import util
from opentelemetry.sdk.util import BoundedDict, BoundedList
from opentelemetry.trace import sampling
from opentelemetry.trace.status import Status, StatusCanonicalCode
from opentelemetry.util import time_ns, types

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -134,6 +136,7 @@ def __init__(
links: Sequence[trace_api.Link] = (),
kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL,
span_processor: SpanProcessor = SpanProcessor(),
set_status_on_exception: bool = True,
) -> None:

self.name = name
Expand All @@ -143,9 +146,10 @@ def __init__(
self.trace_config = trace_config
self.resource = resource
self.kind = kind
self._set_status_on_exception = set_status_on_exception

self.span_processor = span_processor
self.status = trace_api.Status()
self.status = None
self._lock = threading.Lock()

if attributes is None:
Expand Down Expand Up @@ -174,7 +178,10 @@ def __repr__(self):
)

def __str__(self):
return '{}(name="{}", context={}, kind={}, parent={}, start_time={}, end_time={})'.format(
return (
'{}(name="{}", context={}, kind={}, '
"parent={}, start_time={}, end_time={})"
ocelotl marked this conversation as resolved.
Show resolved Hide resolved
).format(
type(self).__name__,
self.name,
self.context,
Expand Down Expand Up @@ -254,6 +261,9 @@ def end(self, end_time: Optional[int] = None) -> None:
logger.warning("Calling end() on an ended span.")
return

if self.status is None:
Oberon00 marked this conversation as resolved.
Show resolved Hide resolved
self.set_status(Status(canonical_code=StatusCanonicalCode.OK))

self.span_processor.on_end(self)

def update_name(self, name: str) -> None:
Expand All @@ -275,6 +285,29 @@ def set_status(self, status: trace_api.Status) -> None:
return
self.status = status

def __exit__(
self,
exc_type: Optional[Type[BaseException]],
exc_val: Optional[BaseException],
exc_tb: Optional[TracebackType],
) -> None:
"""Ends context manager and calls `end` on the `Span`."""

if (
self.status is None
and self._set_status_on_exception
and exc_val is not None
):

self.set_status(
Status(
canonical_code=StatusCanonicalCode.UNKNOWN,
description="{}: {}".format(exc_type.__name__, exc_val),
)
)

super().__exit__(exc_type, exc_val, exc_tb)


def generate_span_id() -> int:
"""Get a new random span ID.
Expand Down Expand Up @@ -334,14 +367,15 @@ def start_as_current_span(
span = self.start_span(name, parent, kind, attributes, links)
return self.use_span(span, end_on_exit=True)

def start_span(
def start_span( # pylint: disable=too-many-locals
self,
name: str,
parent: trace_api.ParentSpan = trace_api.Tracer.CURRENT_SPAN,
kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL,
attributes: Optional[types.Attributes] = None,
links: Sequence[trace_api.Link] = (),
start_time: Optional[int] = None,
set_status_on_exception: bool = True,
) -> "Span":
"""See `opentelemetry.trace.Tracer.start_span`."""

Expand Down Expand Up @@ -401,6 +435,7 @@ def start_span(
span_processor=self._active_span_processor,
kind=kind,
links=links,
set_status_on_exception=set_status_on_exception,
)
span.start(start_time=start_time)
else:
Expand Down
26 changes: 13 additions & 13 deletions opentelemetry-sdk/tests/trace/test_trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from opentelemetry import trace as trace_api
from opentelemetry.sdk import trace
from opentelemetry.trace import sampling
from opentelemetry.trace.status import StatusCanonicalCode
from opentelemetry.util import time_ns


Expand Down Expand Up @@ -455,12 +456,7 @@ def test_start_span(self):
span.start()
self.assertEqual(start_time, span.start_time)

# default status
self.assertTrue(span.status.is_ok)
self.assertIs(
span.status.canonical_code, trace_api.status.StatusCanonicalCode.OK
)
self.assertIs(span.status.description, None)
self.assertIs(span.status, None)

# status
new_status = trace_api.status.Status(
Expand Down Expand Up @@ -515,13 +511,17 @@ def test_ended_span(self):
"Test description",
)
root.set_status(new_status)
# default status
self.assertTrue(root.status.is_ok)
self.assertEqual(
root.status.canonical_code,
trace_api.status.StatusCanonicalCode.OK,
)
self.assertIs(root.status.description, None)
self.assertIs(root.status, None)

def test_error_status(self):
try:
with trace.Tracer("test_error_status").start_span("root") as root:
raise Exception("unknown")
except Exception: # pylint: disable=broad-except
pass

self.assertIs(root.status.canonical_code, StatusCanonicalCode.UNKNOWN)
self.assertEqual(root.status.description, "Exception: unknown")


def span_event_start_fmt(span_processor_name, span_name):
Expand Down