Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't write headers on Write if they were already written #6055

Merged
merged 3 commits into from
Aug 29, 2024

Conversation

dmathieu
Copy link
Member

The change in #5916 introduced a regression, as we don't check whether the header was written before writing it in Write().
We need to only write if the header wasn't written yet.

Fixes #6053.

@dmathieu dmathieu marked this pull request as ready for review August 27, 2024 13:21
@dmathieu dmathieu requested a review from a team August 27, 2024 13:21
@MrAlias MrAlias added this to the v1.30.0 milestone Aug 29, 2024
@MrAlias MrAlias added the bug Something isn't working label Aug 29, 2024
Copy link

codecov bot commented Aug 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.5%. Comparing base (b62a80e) to head (d525b53).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #6055   +/-   ##
=====================================
  Coverage   65.5%   65.5%           
=====================================
  Files        203     203           
  Lines      12984   12985    +1     
=====================================
+ Hits        8505    8506    +1     
  Misses      4219    4219           
  Partials     260     260           
Files with missing lines Coverage Δ
...p/otelhttp/internal/request/resp_writer_wrapper.go 100.0% <100.0%> (ø)

@MrAlias MrAlias merged commit 54d6916 into open-telemetry:main Aug 29, 2024
27 checks passed
BuJo added a commit to synyx/tuwat that referenced this pull request Sep 1, 2024
johnduhart pushed a commit to mdsol/opentelemetry-go-contrib that referenced this pull request Sep 6, 2024
…emetry#6055)

The change in open-telemetry#5916 introduced a regression, as we don't check whether
the header was written before writing it in `Write()`.
We need to only write if the header wasn't written yet.

Fixes open-telemetry#6053.

---------

Co-authored-by: Tyler Yahn <[email protected]>
XSAM added a commit that referenced this pull request Sep 11, 2024
### Added

- Add `NewProducer` to
`go.opentelemetry.io/contrib/instrumentation/runtime`, which allows
collecting the `go.schedule.duration` histogram metric from the Go
runtime. (#5991)
- Add gRPC protocol support for OTLP log exporter in
`go.opentelemetry.io/contrib/exporters/autoexport`. (#6083)

### Removed

- Drop support for [Go 1.21]. (#6046, #6047)

### Fixed

- 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)

[Go 1.21]: https://go.dev/doc/go1.21

---------

Co-authored-by: Tyler Yahn <[email protected]>
@dmathieu dmathieu deleted the write-header-once branch October 10, 2024 08:32
felipeweb added a commit to funccloud/weaver that referenced this pull request Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error logs coming for http: superfluous response.WriteHeader call when using otelhttp handler
3 participants