Skip to content

Commit

Permalink
Introduce respWriter.Flush so we don't write the status twice (#5634)
Browse files Browse the repository at this point in the history
Closes #5438.
  • Loading branch information
dmathieu authored May 28, 2024
1 parent 6e36a40 commit 9479910
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 0 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

- The `go.opentelemetry.io/contrib/config` add support to configure periodic reader interval and timeout. (#5661)

### Fixed

- The superfluous `response.WriteHeader` call in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp` when the response writer is flushed. (#5634)

### Deprecated

- The `go.opentelemetry.io/contrib/instrumentation/go.mongodb.org/mongo-driver/mongo/otelmongo` package is deprecated.
Expand Down
3 changes: 3 additions & 0 deletions instrumentation/net/http/otelhttp/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,9 @@ func (h *middleware) serveHTTP(w http.ResponseWriter, r *http.Request, next http
WriteHeader: func(httpsnoop.WriteHeaderFunc) httpsnoop.WriteHeaderFunc {
return rww.WriteHeader
},
Flush: func(httpsnoop.FlushFunc) httpsnoop.FlushFunc {
return rww.Flush
},
})

labeler := &Labeler{}
Expand Down
3 changes: 3 additions & 0 deletions instrumentation/net/http/otelhttp/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ func TestHandler(t *testing.T) {
return otelhttp.NewHandler(
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
assert.Implements(t, (*http.Flusher)(nil), w)

w.(http.Flusher).Flush()
_, _ = io.WriteString(w, "Hello, world!\n")
}), "test_handler",
)
},
Expand Down
10 changes: 10 additions & 0 deletions instrumentation/net/http/otelhttp/wrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,13 @@ func (w *respWriterWrapper) WriteHeader(statusCode int) {
}
w.ResponseWriter.WriteHeader(statusCode)
}

func (w *respWriterWrapper) Flush() {
if !w.wroteHeader {
w.WriteHeader(http.StatusOK)
}

if f, ok := w.ResponseWriter.(http.Flusher); ok {
f.Flush()
}
}
60 changes: 60 additions & 0 deletions instrumentation/net/http/otelhttp/wrap_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package otelhttp

import (
"net/http"
"net/http/httptest"
"testing"

"github.com/stretchr/testify/assert"
)

func TestRespWriterWriteHeader(t *testing.T) {
rw := &respWriterWrapper{
ResponseWriter: &httptest.ResponseRecorder{},
record: func(int64) {},
}

rw.WriteHeader(http.StatusTeapot)
assert.Equal(t, http.StatusTeapot, rw.statusCode)
assert.True(t, rw.wroteHeader)

rw.WriteHeader(http.StatusGone)
assert.Equal(t, http.StatusTeapot, rw.statusCode)
}

func TestRespWriterFlush(t *testing.T) {
rw := &respWriterWrapper{
ResponseWriter: &httptest.ResponseRecorder{},
record: func(int64) {},
}

rw.Flush()
assert.Equal(t, http.StatusOK, rw.statusCode)
assert.True(t, rw.wroteHeader)
}

type nonFlushableResponseWriter struct{}

func (_ nonFlushableResponseWriter) Header() http.Header {
return http.Header{}
}

func (_ nonFlushableResponseWriter) Write([]byte) (int, error) {
return 0, nil
}

func (_ nonFlushableResponseWriter) WriteHeader(int) {}

func TestRespWriterFlushNoFlusher(t *testing.T) {
rw := &respWriterWrapper{
ResponseWriter: nonFlushableResponseWriter{},
record: func(int64) {},
}

rw.Flush()
assert.Equal(t, http.StatusOK, rw.statusCode)
assert.True(t, rw.wroteHeader)
}

0 comments on commit 9479910

Please sign in to comment.