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

[APM][Otel] Errors: Fallback when handled is not defined #194123

Closed
jennypavlova opened this issue Sep 26, 2024 · 8 comments
Closed

[APM][Otel] Errors: Fallback when handled is not defined #194123

jennypavlova opened this issue Sep 26, 2024 · 8 comments
Assignees
Labels
enhancement New value added to drive a business result OpenTelemetry Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team

Comments

@jennypavlova
Copy link
Member

jennypavlova commented Sep 26, 2024

Related to #192336

In the case of Otel data, there is a possibility that the error.exception.handled value is not present and we want to show a fallback message in that case.

The texts we have for the status are:

  • Unhandled when false (ref),
  • And we should add Unknown when it's undefined

And the UI should not show any errors in case handled is undefined.

@jennypavlova jennypavlova added enhancement New value added to drive a business result Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team labels Sep 26, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

@MiriamAparicio
Copy link
Contributor

There are two places where we show an unhandled error

Errors table list

Image

Errors sample

Image

@MiriamAparicio
Copy link
Contributor

Thinking about the possible solution to this, I think the best might be just to not show the badge at all if error.exception.handled is not present, Unknown per se doesn't give any information to the customer, if we add the Unknown badge we will need to add extra information to the customer about why we are showing that badge, ie. We couldn’t retrieve information about if the error has been handled

The Unhandled badge has the porpoise to inform the customer that the exception is not handled, this attribute does not exist in Otel convention attributes for exceptions, so not showing anything, would be my approach
What do you think? @jennypavlova @AlexanderWert

@MiriamAparicio MiriamAparicio self-assigned this Sep 26, 2024
@AlexanderWert
Copy link
Member

The Unhandled badge has the porpoise to inform the customer that the exception is not handled, this attribute does not exist in Otel convention attributes for exceptions, so not showing anything, would be my approach
What do you think? @jennypavlova @AlexanderWert

Makes sense! If error.exception.handled is not present, let's just not show the badge at all!

@smith
Copy link
Contributor

smith commented Sep 26, 2024

Is error.exception.handled something we've proposed to add to Exception semantic conventions?

@AlexanderWert
Copy link
Member

Is error.exception.handled something we've proposed to add to Exception semantic conventions?

Good question! Looking at the semconv for that, isn't exception.escaped sort of the same (or actually exactly the opposite semantical meaning)?
@gregkalapos are we using that field already to derive error.exception.handled? (would be nice to have inverse aliases in ES :-D )

@smith to the point of this issue. Even if we already use exception.escaped to derive error.exception.handled, it's a recommended and not a required attribute, so we definitely need to expect it to be unset in some situations and need to handle that properly.

@gregkalapos
Copy link
Contributor

gregkalapos commented Sep 27, 2024

Is error.exception.handled something we've proposed to add to Exception semantic conventions?

Good question! Looking at the semconv for that, isn't exception.escaped sort of the same (or actually exactly the opposite semantical meaning)?
@gregkalapos are we using that field already to derive error.exception.handled? (would be nice to have inverse aliases in ES :-D )

Yes, exception.escaped is already defined as stable and that covers what we need for handled in our data model - but as Alex says it's the opposite meaning. I don't think we can have "inverse aliases" short term. But we already take care of this in the enrichment lib (or well, there is an open PR, but not yet merged) - we just set an attribute called exception.handled with the inverted exception.escaped value.

So for the UI: if the OTel data has this information (typically in exception.escaped), the enrichment lib will make sure we populate exception.handled based on that information. But as said above, this field is not required, and we won't always have this value. My personal note: with lots of languages, it's impossible to get this value, so in real world, it'll happen a lot, that we don't have exception.handled. Btw. the values we had in our old APM data model were also wrong in lots of cases.

@MiriamAparicio
Copy link
Contributor

Hi, After checking and testing this
It seems that a UI change it's not necessary on the React component, the error is a server error on the request, I'm unsure about the error sampler as I couldn't test it due to another error with the null fields, I will wait to retest after this issue has been done #192336

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result OpenTelemetry Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team
Projects
None yet
Development

No branches or pull requests

6 participants