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.

Sign up for GitHub

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

Jump to bottom

Target Allocator Support for Telegraf Based Prometheus Receiver #1394

Merged

okankoAMZ merged 29 commits into main from ta-collector

Nov 14, 2024

Contributor

okankoAMZ commented Oct 15, 2024 •

edited

Loading

Description of the issue

We have a new tool called Target Allocator, which provides scrape configs. These changes allows telegraf based prometheus receiver to pull scrape configs from Target Allocator endpoint.

There was a bug on config returning if TA exist instead of does not exist on this PR(#1390) thus reverted and created this one.

Description of changes

Added Target Allocator Manager(TAM), based on contrib version but with a wrapper
TAM is now included and checked during initial and reload configs to make sure TA is enabled or disabled.
When TA is enabled the TAM will provide the prometheus config to telegraf prometheus receiver directly. (without a file read)

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

Added some unit tests which pass
Manually tested where I can see metrics on CloudWatch when Agent is deployed.

Requirements

Before commit the code, please do the following steps.

Run make fmt and make fmt-sh
Run make lint

okankoAMZ and others added 11 commits

October 4, 2024 19:18


          Basic Working Copy

9954a07


          clean-up

e8e509d


          labelled temporary function

b67d022


          added unit test and ran go fmt

79e030f


          fixed blocking channel issue with reloadConfig

db10d75


          Merge branch 'target-allocator' into ta-collector


          ran go mod tidy, go fmt, go lint

bf0c65a


          addressed comments

1ad4aae


          Target Allocator Support for Native Prometheus Receiver (#1390)

7a5a026


          Revert "Target Allocator Support for Native Prometheus Receiver" (#1393)

43bbf95


          fixed nil typo

93a2ae0

okankoAMZ requested a review from a team as a code owner

October 15, 2024 17:38


          Merge branch 'target-allocator' into ta-collector

90bfd8f

Paramadon reviewed

View reviewed changes

plugins/inputs/prometheus/start.go Outdated Show resolved Hide resolved


          changed error to warning for TA config load

4550d56

mitali-salvi reviewed

View reviewed changes

go.mod Outdated Show resolved Hide resolved

plugins/inputs/prometheus/start.go Outdated Show resolved Hide resolved

plugins/inputs/prometheus/start.go Show resolved Hide resolved

plugins/inputs/prometheus/start.go Outdated Show resolved Hide resolved

plugins/inputs/prometheus/start.go Outdated Show resolved Hide resolved

plugins/inputs/prometheus/target_allocator.go Outdated Show resolved Hide resolved


          addressed comments

aa6a7b2

jefchien reviewed

View reviewed changes

plugins/inputs/prometheus/target_allocator.go Outdated Show resolved Hide resolved

plugins/inputs/prometheus/target_allocator.go Show resolved Hide resolved

plugins/inputs/prometheus/target_allocator.go Outdated Show resolved Hide resolved

jefchien previously approved these changes

View reviewed changes


          addressed comments

d15be9c

musa-asad reviewed

View reviewed changes

plugins/inputs/prometheus/start.go Show resolved Hide resolved

musa-asad reviewed

View reviewed changes

plugins/inputs/prometheus/start.go Outdated Show resolved Hide resolved

musa-asad reviewed

View reviewed changes

plugins/inputs/prometheus/target_allocator.go Outdated Show resolved Hide resolved

musa-asad reviewed

View reviewed changes

plugins/inputs/prometheus/target_allocator.go Outdated Show resolved Hide resolved

Paramadon reviewed

View reviewed changes

plugins/inputs/prometheus/metrics_receiver_test.go Show resolved Hide resolved

Paramadon reviewed

View reviewed changes

plugins/inputs/prometheus/metrics_receiver_test.go Outdated Show resolved Hide resolved

Paramadon reviewed

View reviewed changes

plugins/inputs/prometheus/target_allocator.go Outdated Show resolved Hide resolved

Paramadon reviewed

View reviewed changes

plugins/inputs/prometheus/metrics_receiver_test.go Outdated Show resolved Hide resolved

Paramadon previously approved these changes

View reviewed changes

Contributor

Paramadon left a comment

Logic looks good just need some more error/path handling and I would also like to see more comments in the code to explain key decision points for example when modifying the configuration.

Paramadon self-requested a review

October 22, 2024 15:57

okankoAMZ mentioned this pull request

Support Otel Prometheus receiver for AMP as a destination #1386

Closed

okankoAMZ changed the title ~~Target Allocator Support for Native Prometheus Receiver~~ Target Allocator Support for Telegraf Based Prometheus Receiver

jefchien reviewed

View reviewed changes

plugins/inputs/prometheus/target_allocator.go Outdated Show resolved Hide resolved

okankoAMZ added 2 commits

October 25, 2024 14:14


          addressed comments and added reload interval

e0ccd20


          Changed 2 channel shutdown to 1 and added log level

f200e4d

okankoAMZ changed the base branch from target-allocator to main

October 30, 2024 14:40

okankoAMZ dismissed stale reviews from musa-asad, Paramadon, and jefchien

October 30, 2024 14:40

The base branch was changed.

okankoAMZ and others added 3 commits

November 4, 2024 16:01


          go mod update

e3bb6cf


          lint

205603b


          Merge branch 'main' into ta-collector

afca216

jefchien reviewed

View reviewed changes

plugins/inputs/prometheus/target_allocator.go Outdated Show resolved Hide resolved

plugins/inputs/prometheus/target_allocator.go

+              func (tam *TargetAllocatorManager) AttachReloadConfigHandler(handler func(config *promconfig.Config)) {
+              	tam.reloadConfigHandler = handler
+              }
+              func (tam *TargetAllocatorManager) reloadConfigTicker() error {

Contributor

jefchien Nov 12, 2024

nit: The function name sounds like it's reloading a ticker, but it's actually creating a reload ticker.

plugins/inputs/prometheus/target_allocator.go Outdated Show resolved Hide resolved


          changed fmt to logger

c3a0db2

okankoAMZ force-pushed the ta-collector branch from 7eda302 to c3a0db2 Compare

November 12, 2024 19:53

musa-asad previously approved these changes

View reviewed changes

jefchien previously approved these changes

View reviewed changes


          Merge branch 'main' into ta-collector

8b7824d

okankoAMZ dismissed stale reviews from jefchien and musa-asad via

8b7824d

November 13, 2024 15:58

okankoAMZ force-pushed the ta-collector branch 2 times, most recently from fd1f5b2 to 48aaa7f Compare

November 13, 2024 18:00


          fixed unit-tests

fb0ea4b

okankoAMZ force-pushed the ta-collector branch from 48aaa7f to fb0ea4b Compare

November 13, 2024 18:38

Contributor

sky333999 commented Nov 13, 2024

Approving from prior reviews

sky333999 approved these changes

View reviewed changes

jefchien approved these changes

View reviewed changes


          Merge branch 'main' into ta-collector

b370d59

okankoAMZ merged commit 1e78609 into main

6 checks passed

okankoAMZ deleted the ta-collector branch

November 14, 2024 17:14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mitali-salvi mitali-salvi left review comments

sky333999 sky333999 approved these changes

jefchien jefchien approved these changes

musa-asad musa-asad left review comments

Paramadon Awaiting requested review from Paramadon

Labels

None yet