Skip to content

Add cert watcher for target allocator TLS config #290

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

Open
wants to merge 8 commits into
base: aws-cwa-dev
Choose a base branch
from

Conversation

musa-asad
Copy link

@musa-asad musa-asad commented Mar 17, 2025

Description of the issue

The Target Allocator manager from the Prometheus receiver doesn't have a mechanism to reload the HTTP client when one of the TLS certificates (CAFile, CertFile, or KeyFile) change due to a refresh. This causes an error when the Target Allocator refreshes its credentials and the OTel Collector doesn't restart since it has a static HTTP client with old credentials.

Description of changes

  • Move httpClient initialization to sync() function so that it's called repeatedly.
  • Implement the setupTLSWatchers() function to call the sync() function every time there is a change in one of the credentials.

License

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Tests

Before

Target Allocator

Note: This occurs even after repetitively restarting the pod.

Screenshot 2025-03-18 at 2 51 22 AM

OTel Collector

Screenshot 2025-03-18 at 2 51 56 AM

After

Target Allocator

Note: This occurs even after repetitively restarting the pod.

Screenshot 2025-03-18 at 2 57 16 AM

OTel Collector

Screenshot 2025-03-18 at 2 57 00 AM

@musa-asad musa-asad marked this pull request as ready for review March 18, 2025 06:58
Copy link

github-actions bot commented May 5, 2025

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 5, 2025
Copy link

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this May 20, 2025
@musa-asad musa-asad reopened this May 20, 2025
}

// sync request jobs from targetAllocator and update underlying receiver, if the response does not match the provided compareHash.
// baseDiscoveryCfg can be used to provide additional ScrapeConfigs which will be added to the retrieved jobs.
func (m *Manager) sync(compareHash uint64, httpClient *http.Client) (uint64, error) {
func (m *Manager) sync(ctx context.Context, compareHash uint64) (uint64, error) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just pass the Https client directly why do need to make one client every sync

@github-actions github-actions bot removed the Stale label May 21, 2025
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.

2 participants