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

Deprecated and moved writeflusher impl to internal pkg #47718

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

krissetto
Copy link
Contributor

@krissetto krissetto commented Apr 15, 2024

What I did

  • Deprecated and moved writeflusher impl to internal pkg (since it was exported and a few external projects seem to use it)
  • Added new internal writeflusher implementation, removing some old buggy bits and adjusting the Flush() func to also call WriteHeader() on the underlying io.Writer if it's an http.ResponseWriter.
    This is needed to ensure that we don't try to send headers multiple times if the writer has already been wrapped by OTEL instrumentation (which doesn't wrap the Flush() func. See Syslog entry: "superfluous response.WriteHeader call" in Docker 25.x #47448)

How I did it
I moved the old impl to internal/writeflusher, and created a NewLegacyWriteFlusher() func that the old exported NewWriteFlusher() func calls to maintain the original behavior. All code using this deprecated version from the ioutils pkg should be unaffected

How to verify it
Usual tests should run as usual, as this shouldn't have any effect

Description for the changelog

Create new internal implementation for writeflusher and deprecate the old one

A picture of a cute animal (not mandatory but encouraged)
image

@krissetto krissetto force-pushed the writeflusher-internal-impl branch 2 times, most recently from 0ad6eb4 to e76e088 Compare April 15, 2024 13:13
@krissetto krissetto marked this pull request as ready for review April 17, 2024 10:49
@laurazard laurazard added status/2-code-review kind/refactor PR's that refactor, or clean-up code labels Apr 17, 2024
…Flusher

Signed-off-by: Christopher Petito <47751006+krissetto@users.noreply.github.com>
…riteHeader() on first Flush())

Signed-off-by: Christopher Petito <47751006+krissetto@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/refactor PR's that refactor, or clean-up code status/2-code-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants