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

Record exception on context manager exit #1162

Merged
merged 1 commit into from
Oct 7, 2020
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
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,11 @@
from opentelemetry.instrumentation.requests.version import __version__
from opentelemetry.instrumentation.utils import http_status_to_canonical_code
from opentelemetry.trace import SpanKind, get_tracer
from opentelemetry.trace.status import Status, StatusCanonicalCode
from opentelemetry.trace.status import (
EXCEPTION_STATUS_FIELD,
Status,
StatusCanonicalCode,
)

# A key to a context variable to avoid creating duplicate spans when instrumenting
# both, Session.request and Session.send, since Session.request calls into Session.send
Expand Down Expand Up @@ -121,8 +125,6 @@ def _instrumented_requests_call(
method = method.upper()
span_name = "HTTP {}".format(method)

exception = None

recorder = RequestsInstrumentor().metric_recorder

labels = {}
Expand All @@ -132,6 +134,7 @@ def _instrumented_requests_call(
with get_tracer(
__name__, __version__, tracer_provider
).start_as_current_span(span_name, kind=SpanKind.CLIENT) as span:
exception = None
with recorder.record_duration(labels):
if span.is_recording():
span.set_attribute("component", "http")
Expand All @@ -150,16 +153,15 @@ def _instrumented_requests_call(
result = call_wrapped() # *** PROCEED
except Exception as exc: # pylint: disable=W0703
exception = exc
setattr(
owais marked this conversation as resolved.
Show resolved Hide resolved
exception,
EXCEPTION_STATUS_FIELD,
owais marked this conversation as resolved.
Show resolved Hide resolved
_exception_to_canonical_code(exception),
)
result = getattr(exc, "response", None)
finally:
context.detach(token)

if exception is not None and span.is_recording():
span.set_status(
Status(_exception_to_canonical_code(exception))
)
span.record_exception(exception)

if result is not None:
if span.is_recording():
span.set_attribute(
Expand All @@ -184,8 +186,8 @@ def _instrumented_requests_call(
if span_callback is not None:
span_callback(span, result)

if exception is not None:
raise exception.with_traceback(exception.__traceback__)
if exception is not None:
raise exception.with_traceback(exception.__traceback__)

return result

Expand Down
16 changes: 14 additions & 2 deletions opentelemetry-api/src/opentelemetry/trace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ def start_as_current_span(
kind: SpanKind = SpanKind.INTERNAL,
attributes: types.Attributes = None,
links: typing.Sequence[Link] = (),
record_exception: bool = True,
) -> typing.Iterator["Span"]:
"""Context manager for creating a new span and set it
as the current span in this tracer's context.
Expand Down Expand Up @@ -320,6 +321,8 @@ def start_as_current_span(
meaningful even if there is no parent.
attributes: The span's attributes.
links: Links span to other spans
record_exception: Whether to record any exceptions raised within the
context as error event on the span.

Yields:
The newly-created span.
Expand All @@ -328,7 +331,10 @@ def start_as_current_span(
@contextmanager # type: ignore
@abc.abstractmethod
def use_span(
self, span: "Span", end_on_exit: bool = False
self,
span: "Span",
end_on_exit: bool = False,
record_exception: bool = True,
) -> typing.Iterator[None]:
"""Context manager for setting the passed span as the
current span in the context, as well as resetting the
Expand All @@ -345,6 +351,8 @@ def use_span(
span: The span to start and make current.
end_on_exit: Whether to end the span automatically when leaving the
context manager.
record_exception: Whether to record any exceptions raised within the
context as error event on the span.
"""


Expand Down Expand Up @@ -375,13 +383,17 @@ def start_as_current_span(
kind: SpanKind = SpanKind.INTERNAL,
attributes: types.Attributes = None,
links: typing.Sequence[Link] = (),
record_exception: bool = True,
) -> typing.Iterator["Span"]:
# pylint: disable=unused-argument,no-self-use
yield INVALID_SPAN

@contextmanager # type: ignore
def use_span(
self, span: "Span", end_on_exit: bool = False
self,
span: "Span",
end_on_exit: bool = False,
record_exception: bool = True,
) -> typing.Iterator[None]:
# pylint: disable=unused-argument,no-self-use
yield
Expand Down
3 changes: 3 additions & 0 deletions opentelemetry-api/src/opentelemetry/trace/status.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
logger = logging.getLogger(__name__)


EXCEPTION_STATUS_FIELD = "_otel_status_code"


class StatusCanonicalCode(enum.Enum):
"""Represents the canonical set of status codes of a finished Span."""

Expand Down
1 change: 1 addition & 0 deletions opentelemetry-sdk/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
([#1203](https://github.com/open-telemetry/opentelemetry-python/pull/1203))
- Protect access to Span implementation
([#1188](https://github.com/open-telemetry/opentelemetry-python/pull/1188))
- `start_as_current_span` and `use_span` can now optionally auto-record any exceptions raised inside the context manager. ([#1162](https://github.com/open-telemetry/opentelemetry-python/pull/1162))

## Version 0.13b0

Expand Down
48 changes: 31 additions & 17 deletions opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@
from opentelemetry.sdk.util.instrumentation import InstrumentationInfo
from opentelemetry.trace import SpanContext
from opentelemetry.trace.propagation import SPAN_KEY
from opentelemetry.trace.status import Status, StatusCanonicalCode
from opentelemetry.trace.status import (
EXCEPTION_STATUS_FIELD,
Status,
StatusCanonicalCode,
)
from opentelemetry.util import time_ns, types

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -699,9 +703,12 @@ def start_as_current_span(
kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL,
attributes: types.Attributes = None,
links: Sequence[trace_api.Link] = (),
record_exception: bool = True,
) -> Iterator[trace_api.Span]:
span = self.start_span(name, parent, kind, attributes, links)
return self.use_span(span, end_on_exit=True)
span = self.start_span(name, parent, kind, attributes, links,)
return self.use_span(
span, end_on_exit=True, record_exception=record_exception
)

def start_span( # pylint: disable=too-many-locals
self,
Expand Down Expand Up @@ -780,7 +787,10 @@ def start_span( # pylint: disable=too-many-locals

@contextmanager
def use_span(
self, span: trace_api.Span, end_on_exit: bool = False
self,
span: trace_api.Span,
end_on_exit: bool = False,
record_exception: bool = True,
) -> Iterator[trace_api.Span]:
try:
token = context_api.attach(context_api.set_value(SPAN_KEY, span))
Expand All @@ -790,20 +800,24 @@ def use_span(
context_api.detach(token)

except Exception as error: # pylint: disable=broad-except
if (
isinstance(span, Span)
and span.status is None
and span._set_status_on_exception # pylint:disable=protected-access # noqa
):
span.set_status(
Status(
canonical_code=StatusCanonicalCode.UNKNOWN,
description="{}: {}".format(
type(error).__name__, error
),
# pylint:disable=protected-access
if isinstance(span, Span):
if record_exception:
span.record_exception(error)

if span.status is None and span._set_status_on_exception:
span.set_status(
Status(
canonical_code=getattr(
error,
EXCEPTION_STATUS_FIELD,
StatusCanonicalCode.UNKNOWN,
),
description="{}: {}".format(
type(error).__name__, error
),
)
)
)

raise

finally:
Expand Down
32 changes: 32 additions & 0 deletions opentelemetry-sdk/tests/trace/test_trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -805,6 +805,38 @@ def test_record_exception(self):
exception_event.attributes["exception.stacktrace"],
)

def test_record_exception_context_manager(self):
try:
with self.tracer.start_as_current_span("span") as span:
raise RuntimeError("example error")
except RuntimeError:
pass
finally:
self.assertEqual(len(span.events), 1)
event = span.events[0]
self.assertEqual("exception", event.name)
self.assertEqual(
"RuntimeError", event.attributes["exception.type"]
)
self.assertEqual(
"example error", event.attributes["exception.message"]
)

stacktrace = """in test_record_exception_context_manager
raise RuntimeError("example error")
RuntimeError: example error"""
self.assertIn(stacktrace, event.attributes["exception.stacktrace"])

try:
with self.tracer.start_as_current_span(
"span", record_exception=False
) as span:
raise RuntimeError("example error")
except RuntimeError:
pass
finally:
self.assertEqual(len(span.events), 0)


def span_event_start_fmt(span_processor_name, span_name):
return span_processor_name + ":" + span_name + ":start"
Expand Down