Skip to content

Commit

Permalink
Expose optional ResponseWriter interfaces.
Browse files Browse the repository at this point in the history
http.ResponseWriters may implement additional interfaces
(http.CloseNotifier, http.Flusher, http.Hijacker, http.Pusher,
io.ReaderFrom) that get lost when the ResponseWriter is wrapped in
another object. This change uses the httpsnoop package to wrap the
ResponseWriter so that the resulting object implements any of the
optional interfaces that the original ResponseWriter implements as
well as using the replacement ResponseWriter methods that gather
information for tracing.
  • Loading branch information
andy-retailnext committed Jul 28, 2020
1 parent fd54b6c commit c4b304f
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 1 deletion.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ require (
github.com/DataDog/sketches-go v0.0.0-20190923095040-43f19ad77ff7
github.com/benbjohnson/clock v1.0.3
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/felixge/httpsnoop v1.0.1
github.com/golang/protobuf v1.4.2
github.com/google/go-cmp v0.5.1
github.com/google/gofuzz v1.0.0 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98=
github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
github.com/felixge/httpsnoop v1.0.1 h1:lvB5Jl89CsZtGIWuTcDM1E/vkVs49/Ml7JJe07l8SPQ=
github.com/felixge/httpsnoop v1.0.1/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b h1:VKtxabqXZkF25pY9ekfRL6a582T4P37/31XEstQ5p58=
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A=
Expand Down
19 changes: 18 additions & 1 deletion instrumentation/othttp/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"net/http"
"time"

"github.com/felixge/httpsnoop"
"go.opentelemetry.io/otel/api/global"
"go.opentelemetry.io/otel/api/kv"
"go.opentelemetry.io/otel/api/metric"
Expand Down Expand Up @@ -152,7 +153,23 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {

rww := &respWriterWrapper{ResponseWriter: w, record: writeRecordFunc, ctx: ctx, props: h.propagators}

h.handler.ServeHTTP(rww, r.WithContext(ctx))
// Wrap w to use our ResponseWriter methods while also exposing
// other interfaces that w may implement (http.CloseNotifier,
// http.Flusher, http.Hijacker, http.Pusher, io.ReaderFrom).

w = httpsnoop.Wrap(w, httpsnoop.Hooks{
Header: func(httpsnoop.HeaderFunc) httpsnoop.HeaderFunc {
return rww.Header
},
Write: func(httpsnoop.WriteFunc) httpsnoop.WriteFunc {
return rww.Write
},
WriteHeader: func(httpsnoop.WriteHeaderFunc) httpsnoop.WriteHeaderFunc {
return rww.WriteHeader
},
})

h.handler.ServeHTTP(w, r.WithContext(ctx))

setAfterServeAttributes(span, bw.read, rww.written, rww.statusCode, bw.err, rww.err)

Expand Down
30 changes: 30 additions & 0 deletions instrumentation/othttp/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,33 @@ func TestHandlerNoWrite(t *testing.T) {
t.Fatalf("Expected *moctrace.MockSpan, got %T", span)
}
}

func TestResponseWriterOptionalInterfaces(t *testing.T) {
rr := httptest.NewRecorder()

var id uint64
tracer := mocktrace.MockTracer{StartSpanID: &id}

// ResponseRecorder implements the Flusher interface. Make sure the
// wrapped ResponseWriter passed to the handler still implements
// Flusher.

var isFlusher bool
h := NewHandler(
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_, isFlusher = w.(http.Flusher)
if _, err := io.WriteString(w, "hello world"); err != nil {
t.Fatal(err)
}
}), "test_handler",
WithTracer(&tracer))

r, err := http.NewRequest(http.MethodGet, "http://localhost/", nil)
if err != nil {
t.Fatal(err)
}
h.ServeHTTP(rr, r)
if !isFlusher {
t.Fatal("http.Flusher interface not exposed")
}
}

0 comments on commit c4b304f

Please sign in to comment.