diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index d747734629..2b13b5b056 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -234,6 +234,7 @@ def start_span( attributes: types.Attributes = None, links: typing.Sequence[Link] = (), start_time: typing.Optional[int] = None, + record_exception: bool = True, set_status_on_exception: bool = True, ) -> "Span": """Starts a span. @@ -266,6 +267,8 @@ def start_span( attributes: The span's attributes. links: Links span to other spans start_time: Sets the start time of a span + record_exception: Whether to record any exceptions raised within the + context as error event on the 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 ERROR when an uncaught exception is @@ -285,7 +288,9 @@ def start_as_current_span( kind: SpanKind = SpanKind.INTERNAL, attributes: types.Attributes = None, links: typing.Sequence[Link] = (), + start_time: typing.Optional[int] = None, record_exception: bool = True, + set_status_on_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. @@ -325,8 +330,14 @@ def start_as_current_span( meaningful even if there is no parent. attributes: The span's attributes. links: Links span to other spans + start_time: Sets the start time of a span record_exception: Whether to record any exceptions raised within the context as error event on the 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 ERROR when an uncaught exception is + raised in the span with block. The span status won't be set by + this mechanism if it was previously set manually. Yields: The newly-created span. @@ -335,10 +346,7 @@ def start_as_current_span( @contextmanager # type: ignore @abc.abstractmethod def use_span( - self, - span: "Span", - end_on_exit: bool = False, - record_exception: bool = True, + self, span: "Span", end_on_exit: bool = False, ) -> typing.Iterator[None]: """Context manager for setting the passed span as the current span in the context, as well as resetting the @@ -355,8 +363,6 @@ 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. """ @@ -374,6 +380,7 @@ def start_span( attributes: types.Attributes = None, links: typing.Sequence[Link] = (), start_time: typing.Optional[int] = None, + record_exception: bool = True, set_status_on_exception: bool = True, ) -> "Span": # pylint: disable=unused-argument,no-self-use @@ -387,17 +394,16 @@ def start_as_current_span( kind: SpanKind = SpanKind.INTERNAL, attributes: types.Attributes = None, links: typing.Sequence[Link] = (), + start_time: typing.Optional[int] = None, record_exception: bool = True, + set_status_on_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, - record_exception: bool = True, + self, span: "Span", end_on_exit: bool = False, ) -> typing.Iterator[None]: # pylint: disable=unused-argument,no-self-use yield diff --git a/opentelemetry-api/src/opentelemetry/trace/span.py b/opentelemetry-api/src/opentelemetry/trace/span.py index 681e9f7ec3..c173cf8b40 100644 --- a/opentelemetry-api/src/opentelemetry/trace/span.py +++ b/opentelemetry-api/src/opentelemetry/trace/span.py @@ -86,6 +86,7 @@ def record_exception( exception: Exception, attributes: types.Attributes = None, timestamp: typing.Optional[int] = None, + escaped: bool = False, ) -> None: """Records an exception as a span event.""" @@ -275,6 +276,7 @@ def record_exception( exception: Exception, attributes: types.Attributes = None, timestamp: typing.Optional[int] = None, + escaped: bool = False, ) -> None: pass diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index f1790c0a58..2a3ca9b586 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -3,6 +3,8 @@ ## Unreleased - Add optional parameter to `record_exception` method ([#1314](https://github.com/open-telemetry/opentelemetry-python/pull/1314)) +- Update exception handling optional parameters, add escaped attribute to record_exception + ([#1365](https://github.com/open-telemetry/opentelemetry-python/pull/1365)) ## Version 0.15b0 diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 6b99888a46..facef291ac 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -412,6 +412,7 @@ def __new__(cls, *args, **kwargs): raise TypeError("Span must be instantiated via a tracer.") return super().__new__(cls) + # pylint: disable=too-many-locals def __init__( self, name: str, @@ -426,6 +427,7 @@ def __init__( kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL, span_processor: SpanProcessor = SpanProcessor(), instrumentation_info: InstrumentationInfo = None, + record_exception: bool = True, set_status_on_exception: bool = True, ) -> None: @@ -436,6 +438,7 @@ def __init__( self.trace_config = trace_config self.resource = resource self.kind = kind + self._record_exception = record_exception self._set_status_on_exception = set_status_on_exception self.span_processor = span_processor @@ -663,20 +666,25 @@ def __exit__( exc_tb: Optional[TracebackType], ) -> None: """Ends context manager and calls `end` on the `Span`.""" - # Records status if span is used as context manager - # i.e. with tracer.start_span() as span: - # TODO: Record exception - if ( - self.status.status_code is StatusCode.UNSET - and self._set_status_on_exception - and exc_val is not None - ): - self.set_status( - Status( - status_code=StatusCode.ERROR, - description="{}: {}".format(exc_type.__name__, exc_val), + if exc_val is not None: + # Record the exception as an event + # pylint:disable=protected-access + if self._record_exception: + self.record_exception(exception=exc_val, escaped=True) + # Records status if span is used as context manager + # i.e. with tracer.start_span() as span: + if ( + self.status.status_code is StatusCode.UNSET + and self._set_status_on_exception + ): + self.set_status( + Status( + status_code=StatusCode.ERROR, + description="{}: {}".format( + exc_type.__name__, exc_val + ), + ) ) - ) super().__exit__(exc_type, exc_val, exc_tb) @@ -685,6 +693,7 @@ def record_exception( exception: Exception, attributes: types.Attributes = None, timestamp: Optional[int] = None, + escaped: bool = False, ) -> None: """Records an exception as a span event.""" try: @@ -698,6 +707,7 @@ def record_exception( "exception.type": exception.__class__.__name__, "exception.message": str(exception), "exception.stacktrace": stacktrace, + "exception.escaped": str(escaped), } if attributes: _attributes.update(attributes) @@ -740,12 +750,21 @@ def start_as_current_span( kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL, attributes: types.Attributes = None, links: Sequence[trace_api.Link] = (), + start_time: Optional[int] = None, record_exception: bool = True, + set_status_on_exception: bool = True, ) -> Iterator[trace_api.Span]: - span = self.start_span(name, context, kind, attributes, links) - return self.use_span( - span, end_on_exit=True, record_exception=record_exception + span = self.start_span( + name=name, + context=context, + kind=kind, + attributes=attributes, + links=links, + start_time=start_time, + record_exception=record_exception, + set_status_on_exception=set_status_on_exception, ) + return self.use_span(span, end_on_exit=True) def start_span( # pylint: disable=too-many-locals self, @@ -755,6 +774,7 @@ def start_span( # pylint: disable=too-many-locals attributes: types.Attributes = None, links: Sequence[trace_api.Link] = (), start_time: Optional[int] = None, + record_exception: bool = True, set_status_on_exception: bool = True, ) -> trace_api.Span: @@ -816,6 +836,7 @@ def start_span( # pylint: disable=too-many-locals kind=kind, links=links, instrumentation_info=self.instrumentation_info, + record_exception=record_exception, set_status_on_exception=set_status_on_exception, ) span.start(start_time=start_time, parent_context=context) @@ -825,10 +846,7 @@ def start_span( # pylint: disable=too-many-locals @contextmanager def use_span( - self, - span: trace_api.Span, - end_on_exit: bool = False, - record_exception: bool = True, + self, span: trace_api.Span, end_on_exit: bool = False, ) -> Iterator[trace_api.Span]: try: token = context_api.attach(context_api.set_value(SPAN_KEY, span)) @@ -837,11 +855,12 @@ def use_span( finally: context_api.detach(token) - except Exception as error: # pylint: disable=broad-except - # pylint:disable=protected-access + except Exception as exc: # pylint: disable=broad-except + # Record the exception as an event if isinstance(span, Span): - if record_exception: - span.record_exception(error) + # pylint:disable=protected-access + if span._record_exception: + span.record_exception(exc) # Records status if use_span is used # i.e. with tracer.start_as_current_span() as span: @@ -851,13 +870,9 @@ def use_span( ): span.set_status( Status( - status_code=getattr( - error, - EXCEPTION_STATUS_FIELD, - StatusCode.ERROR, - ), + status_code=StatusCode.ERROR, description="{}: {}".format( - type(error).__name__, error + type(exc).__name__, exc ), ) ) diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 521bde00c8..8bb84d1d7d 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -889,6 +889,9 @@ def test_record_exception_with_attributes(self): self.assertEqual( "RuntimeError", exception_event.attributes["exception.type"] ) + self.assertEqual( + "False", exception_event.attributes["exception.escaped"] + ) self.assertIn( "RuntimeError: error", exception_event.attributes["exception.stacktrace"], @@ -900,6 +903,28 @@ def test_record_exception_with_attributes(self): True, exception_event.attributes["has_additional_attributes"], ) + def test_record_exception_escaped(self): + span = trace._Span("name", mock.Mock(spec=trace_api.SpanContext)) + try: + raise RuntimeError("error") + except RuntimeError as err: + span.record_exception(exception=err, escaped=True) + exception_event = span.events[0] + self.assertEqual("exception", exception_event.name) + self.assertEqual( + "error", exception_event.attributes["exception.message"] + ) + self.assertEqual( + "RuntimeError", exception_event.attributes["exception.type"] + ) + self.assertIn( + "RuntimeError: error", + exception_event.attributes["exception.stacktrace"], + ) + self.assertEqual( + "True", exception_event.attributes["exception.escaped"] + ) + def test_record_exception_with_timestamp(self): span = trace._Span("name", mock.Mock(spec=trace_api.SpanContext)) try: