From 1665929be6316bc3d143dde48e950e4c613e1c8c Mon Sep 17 00:00:00 2001 From: Anton Manakin Date: Wed, 4 Sep 2024 19:30:10 +0300 Subject: [PATCH 1/2] Fix superfluous writing header after flush in otelhttp --- CHANGELOG.md | 1 + .../internal/request/resp_writer_wrapper.go | 7 +++++- .../net/http/otelhttp/test/handler_test.go | 23 +++++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 564503d1493..55b98fcdf2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed +- Superfluous call to `WriteHeader` when flushing after setting a status code in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (___) - Superfluous call to `WriteHeader` when writing the response body after setting a status code in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (#6055) diff --git a/instrumentation/net/http/otelhttp/internal/request/resp_writer_wrapper.go b/instrumentation/net/http/otelhttp/internal/request/resp_writer_wrapper.go index c5f2fa80524..fbc344cbdda 100644 --- a/instrumentation/net/http/otelhttp/internal/request/resp_writer_wrapper.go +++ b/instrumentation/net/http/otelhttp/internal/request/resp_writer_wrapper.go @@ -82,7 +82,12 @@ func (w *RespWriterWrapper) writeHeader(statusCode int) { // Flush implements [http.Flusher]. func (w *RespWriterWrapper) Flush() { - w.WriteHeader(http.StatusOK) + w.mu.Lock() + defer w.mu.Unlock() + + if !w.wroteHeader { + w.writeHeader(http.StatusOK) + } if f, ok := w.ResponseWriter.(http.Flusher); ok { f.Flush() diff --git a/instrumentation/net/http/otelhttp/test/handler_test.go b/instrumentation/net/http/otelhttp/test/handler_test.go index 780ec6fab54..831db4c0a85 100644 --- a/instrumentation/net/http/otelhttp/test/handler_test.go +++ b/instrumentation/net/http/otelhttp/test/handler_test.go @@ -215,6 +215,12 @@ func (rw *respWriteHeaderCounter) WriteHeader(statusCode int) { rw.ResponseWriter.WriteHeader(statusCode) } +func (rw *respWriteHeaderCounter) Flush() { + if f, ok := rw.ResponseWriter.(http.Flusher); ok { + f.Flush() + } +} + func TestHandlerPropagateWriteHeaderCalls(t *testing.T) { testCases := []struct { name string @@ -265,6 +271,23 @@ func TestHandlerPropagateWriteHeaderCalls(t *testing.T) { }, expectHeadersWritten: []int{http.StatusBadRequest}, }, + { + name: "When writing the header indirectly through flush", + handler: func(w http.ResponseWriter, r *http.Request) { + f := w.(http.Flusher) + f.Flush() + }, + expectHeadersWritten: []int{http.StatusOK}, + }, + { + name: "With a header already written when flushing", + handler: func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusBadRequest) + f := w.(http.Flusher) + f.Flush() + }, + expectHeadersWritten: []int{http.StatusBadRequest}, + }, } for _, tc := range testCases { From 34031061457bf0ffcb1bac07aea1e3747883847d Mon Sep 17 00:00:00 2001 From: Anton Manakin Date: Wed, 4 Sep 2024 19:33:07 +0300 Subject: [PATCH 2/2] fix changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55b98fcdf2b..83c4c16202f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed -- Superfluous call to `WriteHeader` when flushing after setting a status code in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (___) +- Superfluous call to `WriteHeader` when flushing after setting a status code in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (#6074) - Superfluous call to `WriteHeader` when writing the response body after setting a status code in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (#6055)