Close gzip readers after use #9770
Conversation
| @@ -78,10 +79,10 @@ func OTLPHandler( | |||
| return httpgrpc.Errorf(http.StatusUnsupportedMediaType, "unsupported compression: %s. Only \"gzip\" or no compression supported", contentEncoding) | |||
| } | |||
|
|
|||
| var decoderFunc func(io.ReadCloser) (req pmetricotlp.ExportRequest, uncompressedBodySize int, err error) | |||
| var decoderFunc func(io.Reader) (req pmetricotlp.ExportRequest, uncompressedBodySize int, err error) | |||
There was a problem hiding this comment.
This function shouldn't close the passed reader, so might as well use io.Reader.
aknuds1
force-pushed
the
arve/otlp-close-reader
branch
from
b121937
to
7d7b883
Compare
Signed-off-by: Arve Knudsen <[email protected]>
aknuds1
force-pushed
the
arve/otlp-close-reader
branch
from
7d7b883
to
e33c20a
Compare
| if err != nil { | ||
| return nil, errors.Wrap(err, "create gzip reader") | ||
| } | ||
|
|
||
| defer func() { | ||
| _ = gzReader.Close() |
There was a problem hiding this comment.
There's no logger available in this context, to log an eventual error.
Are you reading that from here? |
I'm reading it from here (stdlib vs klauspost).
To me it seems clear that it's referring to the returned AFAICT, the gzip compressor Close method doesn't do anything but return an eventual error (for invalid checksum?), but that's an implementation detail and could change (considering the documentation indicates the caller should close the reader). Also, I don't think it's a bad idea to consistently log any returned error (as we already do in one call site), if we do have a logger. |
What this PR does
I noticed that in most cases we don't close gzip readers after use, even though it's the caller's responsibility.
Which issue(s) this PR fixes or relates to
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX].about-versioning.mdupdated with experimental features.