From bbd877bdcf8fc642ac05a505831e6ee01933fd5f Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 9 Nov 2020 12:43:08 -0500 Subject: [PATCH 1/8] exp --- opentelemetry-sdk/CHANGELOG.md | 2 + .../src/opentelemetry/sdk/trace/__init__.py | 68 +++++++++++-------- 2 files changed, 43 insertions(+), 27 deletions(-) diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index f1790c0a584..f846d0d7cbd 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 + ([#1362](https://github.com/open-telemetry/opentelemetry-python/pull/1362)) ## Version 0.15b0 diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 6b99888a461..73bd7982095 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -426,6 +426,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 +437,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 +665,23 @@ 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: + span.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 +690,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 +704,7 @@ def record_exception( "exception.type": exception.__class__.__name__, "exception.message": str(exception), "exception.stacktrace": stacktrace, + "exception.escaped": escaped, } if attributes: _attributes.update(attributes) @@ -740,12 +747,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 +771,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 +833,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) @@ -828,7 +846,6 @@ def use_span( 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)) @@ -837,11 +854,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 +869,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 ), ) ) From 870f3f526354e50320901abb228cbcc294c279ed Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 9 Nov 2020 12:56:14 -0500 Subject: [PATCH 2/8] lint --- opentelemetry-sdk/CHANGELOG.md | 2 +- .../src/opentelemetry/sdk/trace/__init__.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index f846d0d7cbd..2a3ca9b5867 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -4,7 +4,7 @@ - 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 - ([#1362](https://github.com/open-telemetry/opentelemetry-python/pull/1362)) + ([#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 73bd7982095..52566cc6bce 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -669,7 +669,7 @@ def __exit__( # Record the exception as an event # pylint:disable=protected-access if self._record_exception: - span.record_exception(exception=exc_val, escaped=True) + 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 ( @@ -679,7 +679,9 @@ def __exit__( self.set_status( Status( status_code=StatusCode.ERROR, - description="{}: {}".format(exc_type.__name__, exc_val), + description="{}: {}".format( + exc_type.__name__, exc_val + ), ) ) @@ -843,9 +845,7 @@ 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, ) -> Iterator[trace_api.Span]: try: token = context_api.attach(context_api.set_value(SPAN_KEY, span)) From 4bf9f1a48a0e51655b20597a9d5a2805b17710b2 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 9 Nov 2020 13:38:19 -0500 Subject: [PATCH 3/8] test --- .../src/opentelemetry/trace/__init__.py | 3 +++ .../src/opentelemetry/trace/span.py | 1 + .../src/opentelemetry/sdk/trace/__init__.py | 1 + opentelemetry-sdk/tests/trace/test_trace.py | 25 +++++++++++++++++++ 4 files changed, 30 insertions(+) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index d747734629a..5aab8944e01 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -374,6 +374,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,7 +388,9 @@ def start_as_current_span( kind: SpanKind = SpanKind.INTERNAL, attributes: types.Attributes = None, links: typing.Sequence[Link] = (), + start_time: 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 diff --git a/opentelemetry-api/src/opentelemetry/trace/span.py b/opentelemetry-api/src/opentelemetry/trace/span.py index 681e9f7ec3f..79436139ec3 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.""" diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 52566cc6bce..8e88c122d1f 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, diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 521bde00c8c..8bb84d1d7d5 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: From 7eb1719decb1f975c2cc1fa1c0c6c12ab2475a66 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 9 Nov 2020 13:47:56 -0500 Subject: [PATCH 4/8] fix error --- opentelemetry-api/src/opentelemetry/trace/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 5aab8944e01..f291b7bdac2 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -388,7 +388,7 @@ def start_as_current_span( kind: SpanKind = SpanKind.INTERNAL, attributes: types.Attributes = None, links: typing.Sequence[Link] = (), - start_time: Optional[int] = None, + start_time: typing.Optional[int] = None, record_exception: bool = True, set_status_on_exception: bool = True, ) -> typing.Iterator["Span"]: From b2b166c322078e1a065c4142914def8d567fbfce Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 9 Nov 2020 13:55:50 -0500 Subject: [PATCH 5/8] typing --- .../src/opentelemetry/trace/__init__.py | 15 +++++++++++---- opentelemetry-api/src/opentelemetry/trace/span.py | 1 + .../src/opentelemetry/sdk/trace/__init__.py | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index f291b7bdac2..1dbacc8e378 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. @@ -324,9 +329,14 @@ def start_as_current_span( kind: The span's kind (relationship to parent). Note that is 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. @@ -338,7 +348,6 @@ def use_span( 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 @@ -355,8 +364,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. """ diff --git a/opentelemetry-api/src/opentelemetry/trace/span.py b/opentelemetry-api/src/opentelemetry/trace/span.py index 79436139ec3..c173cf8b40e 100644 --- a/opentelemetry-api/src/opentelemetry/trace/span.py +++ b/opentelemetry-api/src/opentelemetry/trace/span.py @@ -276,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/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 8e88c122d1f..facef291ac5 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -707,7 +707,7 @@ def record_exception( "exception.type": exception.__class__.__name__, "exception.message": str(exception), "exception.stacktrace": stacktrace, - "exception.escaped": escaped, + "exception.escaped": str(escaped), } if attributes: _attributes.update(attributes) From 85badfa3481e273c4eef4ca47ea405e12073ec8c Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 9 Nov 2020 13:59:02 -0500 Subject: [PATCH 6/8] lint --- opentelemetry-api/src/opentelemetry/trace/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 1dbacc8e378..328fc91f6a0 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -345,9 +345,7 @@ 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, ) -> typing.Iterator[None]: """Context manager for setting the passed span as the current span in the context, as well as resetting the From fe70e760d276249ac84e141a063b5ed519f64811 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 9 Nov 2020 14:20:33 -0500 Subject: [PATCH 7/8] lint --- opentelemetry-api/src/opentelemetry/trace/__init__.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 328fc91f6a0..486f41ae59b 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -402,10 +402,7 @@ def start_as_current_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 From 604c054afbff76f0f122a3861e9491ca336b5474 Mon Sep 17 00:00:00 2001 From: Leighton Date: Thu, 12 Nov 2020 10:28:27 -0500 Subject: [PATCH 8/8] comment --- opentelemetry-api/src/opentelemetry/trace/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 486f41ae59b..2b13b5b0560 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -329,6 +329,7 @@ def start_as_current_span( kind: The span's kind (relationship to parent). Note that is 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.