Add section on span status for databases #1560
Conversation
Co-authored-by: Trask Stalnaker <[email protected]>
lmolkova
force-pushed
the
database-span-status
branch
from
b4a9c3a
to
566bf08
Compare
| When the operation fails with an exception, instrumentation SHOULD record | ||
| an [exception event](../exceptions/exceptions-spans.md) by default if and only if the | ||
| span being recorded is a local root span (does not have a local parent). | ||
|
|
||
| > [!NOTE] | ||
| > Exception stack traces could be very long and are expensive to capture and store. | ||
| > Exceptions which are not handled by instrumented libraries are likely to be handled | ||
| > and logged by the caller. | ||
| > Exceptions that are not handled will be recorded by the outermost (local root) | ||
| > instrumentation such as HTTP or gRPC server. |
There was a problem hiding this comment.
let's ask for feedback on this point from the Specification SIG before we merge
There was a problem hiding this comment.
I think this is a directional shift in logging exceptions.
I see two things here:
- We still can't determine "layers" of instrumentation, i.e. we can't know if someone else will log the exception or not.
- This violates the previously recorded wisdom of how to handle exceptions and stack traces.
My opinion:
This is the right direction to go. We should assume that there is "outer" instrumentation that logs/catches exceptions, particularly for database "client" spans. I.e. we may be able to blanketly state that "client" spans should never log unhandled exceptions and that we encourage "outer" instrumentation that will (e.g. server spans).
There was a problem hiding this comment.
I added to next week's spec SIG agenda
|
|
||
| - SHOULD set the span status code to `Error` | ||
| - SHOULD set the `error.type` attribute | ||
| - SHOULD set the span status description when it has additional information |
There was a problem hiding this comment.
should this have any requirements about not having any confidential data? Similar to how we have on metrics errors where we don't add if it shows actual values of queries, etc
| span being recorded is a local root span (does not have a local parent). | ||
|
|
||
| > [!NOTE] | ||
| > Exception stack traces could be very long and are expensive to capture and store. |
There was a problem hiding this comment.
| > Exception stack traces could be very long and are expensive to capture and store. | |
| > | |
| > Exception stack traces could be very long and are expensive to capture and store. |
I think you need an empty line here so the icon shows up.
| **Status**: [Experimental][DocumentStatus] | ||
|
|
||
| When the operation fails with an exception, instrumentation SHOULD record | ||
| an [exception event](../exceptions/exceptions-spans.md) by default if and only if the |
There was a problem hiding this comment.
| an [exception event](../exceptions/exceptions-spans.md) by default if and only if the | |
| an [exception event](../exceptions/exceptions-spans.md) by default if, and only if, the |
Addresses the DB part of #1536.
Let's polish it for databases and then expand to other conventions (and potentially have a generic guidance).
The intention is to keep exception event part experimental since
Merge requirement checklist
[chore]