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

Ensure http response wrapper always sets a statusCode #2822

Merged
merged 4 commits into from Nov 8, 2022
Merged

Ensure http response wrapper always sets a statusCode #2822

merged 4 commits into from Nov 8, 2022

Conversation

dmathieu
Copy link
Member

If a wrapper doesn't write anything, we don't have any status code available. In which case we can assume it is going to be 200.
This behavior is also what the net/http library assumes if the response writer is a flusher: https://cs.opensource.google/go/go/+/refs/tags/go1.19.1:src/net/http/server.go;l=1702

Closes #2821.

@codecov
Copy link

codecov bot commented Sep 29, 2022

Codecov Report

Merging #2822 (80de95c) into main (e97d789) will increase coverage by 0.0%.
The diff coverage is 89.4%.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #2822   +/-   ##
=====================================
  Coverage   69.6%   69.6%           
=====================================
  Files        147     147           
  Lines       6785    6795   +10     
=====================================
+ Hits        4725    4735   +10     
  Misses      1944    1944           
  Partials     116     116           
Impacted Files Coverage Δ
samplers/jaegerremote/sampler_remote.go 87.4% <50.0%> (ø)
instrumentation/net/http/otelhttp/handler.go 81.9% <100.0%> (+0.5%) ⬆️
samplers/jaegerremote/sampler_remote_options.go 100.0% <100.0%> (ø)

CHANGELOG.md Show resolved Hide resolved
@MrAlias MrAlias merged commit 0252c36 into open-telemetry:main Nov 8, 2022
@dmathieu dmathieu deleted the flush-wrapper branch November 8, 2022 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set http.status_code span attribute for handlers that do not write explicit response
6 participants