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

[chore][receiver/awscontainerinsight] Enable goleak checks in internal packages #32406

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

crobert-1
Copy link
Member

Description:

This PR contains a few changes:

  1. Plumb through context when initializing objects that have long running background operations. This is to ensure the receiver's proper context is used for shutting down. context.Background shouldn't be referenced except for tests here, as it opens the door to possible leaks. This was done for ECSInfo and HostInfo.
  2. Add contexts with cancels to tests. This is valid use case for cancelling contexts in test because in practice, the plumbed through context is cancelled by the receiver's shutdown method. These unit tests are operating at a lower level than the receiver, so it's just doing the same thing the receiver shutdown would do.
  3. Fix defer calls to shutdown to be inside anonymous func to ensure they're not called until the defer is actually executed.
  4. Enable goleak checks on the remaining internal packages of the AWS Container Insights receiver to help ensure no goroutines are being leaked.

Link to tracking Issue:
#30438

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

Copy link
Contributor

github-actions bot commented May 7, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label May 7, 2024
@crobert-1 crobert-1 removed the Stale label May 7, 2024
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label May 22, 2024
@crobert-1 crobert-1 removed the Stale label May 22, 2024
Copy link
Contributor

github-actions bot commented Jun 6, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jun 6, 2024
Copy link
Contributor

github-actions bot commented Jul 6, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@atoulme
Copy link
Contributor

atoulme commented Jul 20, 2024

@pxaws @Aneurysm9 would you please take a look?

Copy link
Contributor

github-actions bot commented Aug 3, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Aug 3, 2024
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Aug 20, 2024
Copy link
Contributor

github-actions bot commented Sep 4, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Sep 4, 2024
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.

3 participants