[BUG] Support for http.ResponseController timeouts in WrapHandler
Closed this issue · 2 comments
This is somewhere between a bug and a feature request as this is to accommodate relatively new (as of version 1.20) functionality in Go in a contrib package.
Version of dd-trace-go
v1.62.0
Describe what happened:
The issue is with the net/http
contrib/
package, specifically with the WrapHandler
function.
We were attempting to use http.ResponseController
to add read and write timeouts to our HTTP handlers, roughly like so:
import (
"context"
"net/http"
"time"
datadog "gopkg.in/DataDog/dd-trace-go.v1/contrib/net/http"
)
// We use something like this to wrap all of our HTTP handlers (it's old, don't judge me)
func WrapHandler(ctx context.Context, handlerConfig *HandlerConfig) http.Handler {
fn := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// ...
rc := http.NewResponseController(w)
err := rc.SetReadDeadline(time.Now().Add(handlerConfig.ReadTimeout))
if err != nil {
logger.WithError(err).Error("error setting handler read deadline")
}
err = rc.SetWriteDeadline(time.Now().Add(handlerConfig.WriteTimeout))
if err != nil {
logger.WithError(err).Error("error setting handler write deadline")
}
// ...
return handlerConfig.Handler(w, r)
})
return datadog.WrapHandler(fn, "service-name", "resource-name")
}
But when those endpoints are hit, we see an internal-to-Go error: feature not supported
I was able to trace the issue like so:
WrapHandler
callsTraceAndServe
here: https://github.com/DataDog/dd-trace-go/blob/main/contrib/net/http/http.go#L91TraceAndServe
passes thehttp.ResponseWriter
object towrapResponseWriter
, overwriting it with the result here: https://github.com/DataDog/dd-trace-go/blob/main/contrib/net/http/trace.go#L68wrapResponseWriter
then tries to check the object to see if it matches a bunch of commonhttp
interfaces likeFlusher
,Pusher
,Hijacker
, etc., and creates a new struct on the fly, making a new instance of it passing in the originalhttp.ResponseWriter
object.
It's the last step where I think the problem is. http.ResponseController
tries to call SetReadDeadline
and SetWriteDeadline
on the http.ResponseWriter
object but that doesn't match any of the interfaces wrapResponseWriter
is coercing the object into so those methods apparently seem to get dropped from it.
Describe what you expected:
I can use http.ResponseController
with SetReadDeadline
and SetWriteDeadline
methods to set read/write timeouts on an HTTP handler with dd-trace-go
's WrapHandler
.
Steps to reproduce the issue:
(described above)
Additional environment details (Version of Go, Operating System, etc.):
Go version: 1.22.1
Hey @chrisforrette! After taking a closer look it seems that the http.ResponseController
code was updated to include new methods (SetReadDeadline
and SetWriteDeadline
) after our initial implementation
We will work on a fix as soon as we can but feel free to open your own PR if you'd like. Regardless, this should be fixed in the near future.
This issue has been fixed in #2775 and it should be included in v1.66.0
(it is expected to be released in July).