Skip to content
This repository has been archived by the owner on Sep 13, 2022. It is now read-only.

Fix HTTP sender, consume response data to free up memory #343

Merged
merged 1 commit into from Feb 11, 2019
Merged

Fix HTTP sender, consume response data to free up memory #343

merged 1 commit into from Feb 11, 2019

Conversation

ghost
Copy link

@ghost ghost commented Feb 11, 2019 •
edited by ghost
Loading

Signed-off-by: Sergey Skupoy sergey.skupoy@gmail.com

Which problem is this PR solving?

  • Fixes memory leak caused by unconsumed response body.

Short description of the changes

  • Just resume() response to free up memory

@ghost ghost mentioned this pull request Feb 11, 2019
Closed
@codecov
Copy link

codecov bot commented Feb 11, 2019 •
edited
Loading

Codecov Report

Merging #343 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #343      +/-   ##
==========================================
+ Coverage   99.12%   99.12%   +<.01%     
==========================================
  Files          43       43              
  Lines        1713     1714       +1     
  Branches      336      336              
==========================================
+ Hits         1698     1699       +1     
  Misses         15       15
Impacted Files Coverage Δ
src/reporters/http_sender.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e21500...fc9679a. Read the comment docs.

black-adder
black-adder reviewed Feb 11, 2019
View reviewed changes
Copy link
Contributor

@black-adder black-adder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cheers, great catch. This looks good to land, do you mind signing off on the PR? See https://github.com/jaegertracing/jaeger-client-node/blob/master/CONTRIBUTING.md#sign-your-work

@ghost
Copy link
Author

ghost commented Feb 11, 2019 •
edited by ghost
Loading

cheers, great catch. This looks good to land, do you mind signing off on the PR? See https://github.com/jaegertracing/jaeger-client-node/blob/master/CONTRIBUTING.md#sign-your-work

I signed off the commit, didn't I?

UPD. Oops, fixed signature :)

Fix HTTP sender, consume response data to free up memory
fc9679a 
Signed-off-by: Sergey Skupoy <sergey.skupoy@gmail.com>
@black-adder
Copy link
Contributor

Odd, DCO tools is showing: Expected "Sergey Skupoy sergey.skupoy@gmail.com", but got "Sergey.skupoy sergey.skupoy@gmail.com"

I'll take a look

@ghost
Copy link
Author

ghost commented Feb 11, 2019

Seems like it fixed.

@black-adder black-adder merged commit eb665cd into jaegertracing:master Feb 11, 2019
@tiffon tiffon mentioned this pull request May 10, 2019
Closed
Iuriy-Budnikov pushed a commit to agile-pm/jaeger-client-node that referenced this pull request Sep 25, 2021
@mayurkale22
chore: update HTTP Readme (jaegertracing#343)
d85ee88
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@black-adder