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

[exporter/signalfx] Fix memory leaks #31772

Merged
merged 18 commits into from
Mar 25, 2024

Conversation

crobert-1
Copy link
Member

@crobert-1 crobert-1 commented Mar 14, 2024

Description:

Changes in PR:

  1. Add correlation client Shutdown function that blocks on the waitgroup. This is the main fix of this PR that should fix the leaking goroutines.
  2. Re-organize the shutdown process of the apm client correlation test suite to properly synchronize the shutting down process.
  3. Fix typo
  4. Only block request sender until context is cancelled. The request processor is shutdown when the context is cancelled, so this would result in Shutdown waiting forever, since the request would never be processed.
  5. Enable goleak in some more packages.

Note: This is contains the exact same contents as #30887, but change number 4 is new, and should resolve the test issue the original PR was causing.

Link to tracking Issue:
Resolves #30864
#30438

Testing:
All existing tests are passing, as well as added goleak checks. I'm going to run this a number of times to try to help ensure it's not flaky anymore.

@crobert-1 crobert-1 requested a review from a team March 14, 2024 22:03
@crobert-1 crobert-1 marked this pull request as draft March 14, 2024 22:25
@crobert-1 crobert-1 marked this pull request as ready for review March 22, 2024 21:13
@crobert-1
Copy link
Member Author

This has passed 10+ times now without any failures. It should be ready to merge.

@atoulme atoulme added the ready to merge Code review completed; ready to merge by maintainers label Mar 22, 2024
@mx-psi mx-psi merged commit 19eba68 into open-telemetry:main Mar 25, 2024
149 checks passed
@github-actions github-actions bot added this to the next release milestone Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exporter/signalfx ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[exporter/signalfx] Failing goleak test
4 participants