Make trace_api.use_span() record BaseException as well as Exception #4406
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The Python SDK has inconsistent handling of
BaseException
s such asKeyboardInterrupt
andStopIteration
.Consider the following code:
The
BaseException
will be passed intoSpan.__exit__()
(code), which will typically callspan.record_exception(BaseException("test"))
; set the span's status toERROR
; and then emit the span.However,
start_as_current_span()
behaves differently:The
BaseException
will be thrown intotrace_api.use_span()
(code). However,use_span()
only handlesexcept Exception
, notexcept BaseException
. Sospan.record_exception()
is not called, and the span's status is not set toERROR
. However, the span is still emitted via thefinally:
block.This behavior surprised me. In particular, when a span exits due to
asyncio.CancelledError
ortrio.Cancelled
, I was surprised that the span was emitted without recording any exception or setting the status toERROR
. I'm guessing this probably wasn't an intentional decision.This PR proposes to change the
except Exception
toexcept BaseException
, so thatuse_span()
will now recordBaseException
s in the same way as regularException
s.Type of change
How Has This Been Tested?
opentelemetry-api/tests/trace/test_globals.py
to ensure thatBaseException
is recordedDoes This PR Require a Contrib Repo Change?
Checklist: