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/loadbalancing] Fix memory leaks #31050

Merged
merged 9 commits into from
Mar 11, 2024

Conversation

crobert-1
Copy link
Member

Description:

This fixes a few goroutine leaks in the loadbalancing exporter.

  1. metrics, traces, and logs exporters were starting their respective load balancers, but were not shutting them down. This adds each respective shutdown call.
  2. The loadbalancer was starting the resolver but never shutting it down. This adds a shutdown call to the resolver.
  3. The static resolver was starting resolvers for each passed in exporter, but never shut them down. This adds a shutdown call for each resolver in the static resolver.

Also added a couple missing Shutdown calls from tests.

Link to tracking Issue:
#30438

Testing:
All existing tests are passing as well as added goleak checks.

@crobert-1
Copy link
Member Author

CI/CD failures are #31494 and #31443

@jpkrohling jpkrohling merged commit 71f1083 into open-telemetry:main Mar 11, 2024
142 checks passed
@github-actions github-actions bot added this to the next release milestone Mar 11, 2024
DougManton pushed a commit to DougManton/opentelemetry-collector-contrib that referenced this pull request Mar 13, 2024
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
This fixes a few goroutine leaks in the loadbalancing exporter.

1. `metrics`, `traces`, and `logs` exporters were starting their
respective load balancers, but were not shutting them down. This adds
each respective shutdown call.
2. The `loadbalancer` was starting the resolver but never shutting it
down. This adds a shutdown call to the resolver.
3. The static resolver was starting resolvers for each passed in
exporter, but never shut them down. This adds a shutdown call for each
resolver in the static resolver.

Also added a couple missing `Shutdown` calls from tests.

**Link to tracking Issue:** <Issue number if applicable>
open-telemetry#30438

**Testing:** <Describe what testing was performed and which tests were
added.>
All existing tests are passing as well as added goleak checks.
@crobert-1 crobert-1 deleted the goleak_loadbalancingexp branch March 13, 2024 17:09
XinRanZhAWS pushed a commit to XinRanZhAWS/opentelemetry-collector-contrib that referenced this pull request Mar 13, 2024
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
This fixes a few goroutine leaks in the loadbalancing exporter.

1. `metrics`, `traces`, and `logs` exporters were starting their
respective load balancers, but were not shutting them down. This adds
each respective shutdown call.
2. The `loadbalancer` was starting the resolver but never shutting it
down. This adds a shutdown call to the resolver.
3. The static resolver was starting resolvers for each passed in
exporter, but never shut them down. This adds a shutdown call for each
resolver in the static resolver.

Also added a couple missing `Shutdown` calls from tests.

**Link to tracking Issue:** <Issue number if applicable>
open-telemetry#30438

**Testing:** <Describe what testing was performed and which tests were
added.>
All existing tests are passing as well as added goleak checks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants