contrib/gin-gonic/gin: incorrect 200 status upon panic recovery hides real problems

[seiyab]

I'm not sure that this issue is a bug report, a feature request or something else.
Anyway, I think the current behavior can be problematic under practical situations.
Indeed, I couldn't find failed requests when an incident happened.

dd-trace-go/contrib/internal/httptrace/httptrace.go

Lines 59 to 60 in 25044cf

	if status == 0 {
	statusStr = "200"

Version of dd-trace-go

v1.49.1

Describe what happened:
Trace for a request that returns 500 error was recorded as 200.

Describe what you expected:
It is recorded as 500, or a special one like "unknown".

Steps to reproduce the issue:

1. Use gin and its Recovery middleware.
   • NOTE: We want to use Recovery before dd-trace, because we want to recover not only when handler panics but also when dd-trace panics.
2. Let handler panic.

dd-trace sees status code 0 and record it as 200.
Then, Recovery middleware sets status code 500.

Additional environment details (Version of Go, Operating System, etc.): N/A

History that I investigated

• As contrib/go-chi/chi: Handle 0 status code correctly #740, status code 0 had been considered as 200 only for chi.
• Look for zero responses from http integrations #745 was created (but no progress?).
• contrib: refactor http request span tags #1286 looks to have changed behavior applying contrib/go-chi/chi: Handle 0 status code correctly #740 into all the contrib packages.

[seiyab]

Anyone?

[katiehockman]

@Julio-Guerra it looks like this is something that changed ~a year ago based on a PR you contributed. The current code uses the ResponseWriter's status code to determine the http.status_code, which isn't playing nicely with Recovery middleware. LMK if this is something you'd like to look into, or if you want to pass ownership over to @DataDog/apm-ecosystems

[seiyab]

Any update?