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

Ignore exception attributes for a given route or status (Pyramid) #1037

Closed
gregbuehler opened this issue Apr 6, 2022 · 2 comments · Fixed by #1136 or #1137
Closed

Ignore exception attributes for a given route or status (Pyramid) #1037

gregbuehler opened this issue Apr 6, 2022 · 2 comments · Fixed by #1136 or #1137
Assignees

Comments

@gregbuehler
Copy link
Contributor

It would be convenient to provide a way to specify routes and/or status codes are not exceptional.

A common pattern within Pyramid is to raise the desired response for certain business flows (e.g. raise a 301 to force a redirect, raise a 204 after deleting an entity, etc) rather than properly return. Because the response was raised and handled as an HTTPException the span is decorated with exception attributes even though the request was not actually exceptional. This causes many vendors to interpret a false-positive when calculating things like error rate.

There is history of this functionality in other instrumentation:

@nstawski
Copy link
Contributor

Having investigated this further, it looks like Pyramid instrumentation is miscategorizing most 200s and 300s exceptions as errors. Created a pull to fix that.

@owais
Copy link
Contributor

owais commented Jun 16, 2022

I if understand correctly, we should ignore 4xx as well and only record 5xx exceptions as errors. According to the spec:

For HTTP status codes in the 4xx range span status MUST be left unset in case of SpanKind.SERVER and MUST be set to Error in case of SpanKind.CLIENT.

For HTTP status codes in the 5xx range, as well as any other code the client failed to interpret, span status MUST be set to Error.

I think we are talking only about server spans here right? In that case only 5xx should be set to the span status as an error unless I misunderstood something.

https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/http/#status

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment