-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
otlp: Clients to close body uniformly #5954
otlp: Clients to close body uniformly #5954
Conversation
There were inconsistencies in closing the response body. For traces, the Close happened in a defer statement and any error was logged. Logs and metrics were less rigorous. It appeared Close() wasn't always called, and when it was, errors were returned sometimes and ignored at other times. This applies the defer logic from traces to the other two and removes other Close() calls.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5954 +/- ##
=====================================
Coverage 84.6% 84.6%
=====================================
Files 272 272
Lines 22896 22897 +1
=====================================
+ Hits 19389 19390 +1
Misses 3163 3163
Partials 344 344 |
Could you add a changelog entry? (the check wasn't failing, but it appears that's because your branch was a bit far behind main). |
I added it under |
@mark-pictor-csec, thanks for your contribution ❤️ |
And thank you all for the massive amount of effort you put in! |
There were inconsistencies in closing the response body. For traces, the Close happened in a defer statement and any error was logged. Logs and metrics were less rigorous. It appeared Close() wasn't always called, and when it was, errors were returned sometimes and ignored at other times.
This applies the defer logic from traces to the other two and removes other Close() calls.
This was part of PR #5929, and has been split out as requested.