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

[internal/metadataproviders/azure] Fix memory leak on AKS #32574

Merged
merged 2 commits into from
Apr 22, 2024

Conversation

crobert-1
Copy link
Member

Description:

From client.Do documentation, a client's response can safely be ignored when an error is returned, but must be handled in all other cases, even when a response code is not 2XX. This fixes the internal AKS detection to properly close the response body in the case of a non-200 status returned.

This also enables goleak on the internal/metadata/azure package which helps ensure no goroutines are being leaked. goleak is what detected this bug.

Link to tracking Issue:
#30438

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

@crobert-1 crobert-1 requested a review from a team April 19, 2024 23:02
@crobert-1 crobert-1 changed the title [processor/resourcedetection] Fix memory leak on AKS [internal/metadata/azure] Fix memory leak on AKS Apr 19, 2024
@crobert-1 crobert-1 changed the title [internal/metadata/azure] Fix memory leak on AKS [internal/metadataproviders/azure] Fix memory leak on AKS Apr 19, 2024
@crobert-1 crobert-1 added processor/resourcedetection Resource detection processor exporter/datadog Datadog components labels Apr 19, 2024
@crobert-1
Copy link
Member Author

I've added references to the resource detection processor and Datadog exporter as this internal library is used by both components, and they're both potential victims of the leak.

@mx-psi mx-psi merged commit ce2d146 into open-telemetry:main Apr 22, 2024
164 of 166 checks passed
@github-actions github-actions bot added this to the next release milestone Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants