develop icon indicating copy to clipboard operation
develop copied to clipboard

`http.status_code` tag in the transaction payload

Open marandaneto opened this issue 4 years ago • 12 comments

unknown

The UI special case the http.status_code tag in the transaction payload (not spans) to show the status sublabel with the HTTP status code (see flask image).

This looks suboptimal due to, let's say we have a transaction that finishes with the status OK, this transaction has 2 spans which are HTTP requests, one of them is a 200 and the other one is a 500, both are running in parallel and since the tag is in the transaction tags and not span tags, the span that finishes the later always wins setting the tag, overwriting the previous tag.

The same transaction may have a 200 or 500 randomly and can be confusing.

is it an issue? should we address that? also, http.status_code isn't part of the spec so most SDKs are not setting it.

marandaneto avatar Oct 05 '21 07:10 marandaneto

For web servers, on the transaction level, the HTTP status is the status of the incoming request -- and not of any outgoing requests (child spans).

Does that clarify the case you are describing?

rhcarvalho avatar Oct 05 '21 09:10 rhcarvalho

@rhcarvalho mmmm, that makes sense, since each incoming request will be its own transaction.

should we doc this special tag @rhcarvalho ?

marandaneto avatar Oct 05 '21 09:10 marandaneto

should we doc this special tag @rhcarvalho ?

Yes, let's document it, it is used in the Sentry frontend and more SDKs could report it.

  • https://github.com/getsentry/sentry/blob/9707fd3f98fa51d5d363cbe34b59aa51588db246/static/app/components/events/rootSpanStatus.tsx#L48
  • https://github.com/getsentry/sentry/blob/0e372653c1ab02139718f29e0903e12e452f1f88/static/app/views/performance/transactionDetails/eventMetas.tsx#L230

And apparently that expectation holds for more than two years, even before GA.

rhcarvalho avatar Oct 05 '21 11:10 rhcarvalho

we only document the HTTP client integrations https://develop.sentry.dev/sdk/features/#http-client-integrations but not Web servers, or is it documented somewhere else?

it should not be that different from the client one tho.

marandaneto avatar Oct 06 '21 09:10 marandaneto

~~Should be resolved by https://github.com/getsentry/sentry-java/pull/2287~~

~~https://github.com/getsentry/sentry-java/blob/ba4957762dc4c113ed21c09398a94deec7588407/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt#L196-L205~~

@marandaneto please confirm and maybe close

adinauer avatar Oct 20 '22 12:10 adinauer

Nope, this is not the same thing, this issue is about transactions and not events.

marandaneto avatar Oct 20 '22 12:10 marandaneto

Whoever updates the spec, please share this with client-infra to make sure we're aligned between SDKs

romtsn avatar Nov 02 '22 14:11 romtsn

Cocoa also sends this tag for spans at least, I believe it might be related to https://github.com/getsentry/sentry-cocoa/blob/1453a8aaf8141ad4c304e723f1f17a674e2a839f/Sources/Sentry/SentryNetworkTracker.m#L250-L251

marandaneto avatar Nov 03 '22 08:11 marandaneto

For Spring WebMVC we're currently unable to get the correct response code. For Spring WebFlux we've added the response code to contexts/response/status_code for transactions in https://github.com/getsentry/sentry-java/pull/2870.

We have to check if the UI has already been updated to also look at http.response.status_code in Span data and contexts/response/status_code for transactions.

adinauer avatar Aug 07 '23 08:08 adinauer

Just confirmed, UI does not look at contexts/response/status_code.

adinauer avatar Aug 07 '23 08:08 adinauer

Also not http.response.status_code in Span data

adinauer avatar Aug 07 '23 08:08 adinauer

We do not want to look at span data here as the status is supposed to display the status of the transaction.

Opened https://github.com/getsentry/sentry/issues/54274 for reading the status code from response context in the UI.

Once the UI can show status code from there we can do start changing SDKs.

  • [ ] We also have to document where to put the status code for transactions.

adinauer avatar Aug 07 '23 08:08 adinauer