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

reuse connections: always close the response.Body #536

Closed
wants to merge 1 commit into from

Conversation

mhratson
Copy link

Commented on seeing errors with lost traces[1]

Problem

After digging deeper into [1] my hypothesis is

  1. lost traces errors are caused due new connections being established which may sometimes exceed 1s(configured) timeout
  2. connections aren't being reused because response.Body doesn't get always closed
  3. every new connection may potentially result in lost traces errors since it may exceed the 1s timeout

Observations

I've patched the transport locally and confirming my expectations:

  • no more repeated lost traces: because connections are mostly reused
  • but occasional lost trace(on startup) likely because of 3. since 1s timeout is still too short for new connection

References

Commented on seeing errors with lost traces[1] 

After digging deeper is the my hypothesis was that 
1. `lost traces` errors are caused due new connections being established which may sometimes exceed 1s(configured) timeout
2. connections aren't being reused because `response.Body` doesn't get always closed
3. every new connection may potentially result in `lost traces` errors since it may exceed the 1s timeout

I've patched the transport locally and seeing exactly what expected:
- no more repeated lost traces: because connections are mostly reused
- but occasional lost trace(on startup) likely because of 3. since 1s timeout is still too short for new connection


- [1] DataDog#181 (comment)
@mhratson
Copy link
Author

Looks like it won't work… closing…

@mhratson mhratson closed this Nov 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant