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.

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

Implement prometheus receiver target allocator #12586

Merged
merged 39 commits into from
Aug 19, 2022
Merged

Implement prometheus receiver target allocator #12586

merged 39 commits into from
Aug 19, 2022

Conversation

secustor
Copy link
Member

Description:

Adding the capability to dynamically request jobs from a TargetAllocator instance ( OpenTelemetryOperator ) or compatible API.

Supersedes:

Link to tracking Issue: #7944

Testing:

Added unit test to confirm that the correct jobs have been added.

Documentation:
Documentation has been added to the README of prometheusreceiver

cc: @jaronoff97 @Aneurysm9

secustor added 21 commits May 24, 2022 11:53
@secustor
feat(prometheusreciever): add target_collector config base line
64b6533
@secustor
implementation draft
4c2adf4
@secustor
feat(prometheusreceiver): functioning target allocator job retrieval
178466a
@secustor
docs(prometheusreceiver): add changelog entry
e110849
@secustor
chore(prometheusreceiver): go mod tidy
701efd0
@secustor
chore(prometheusreceiver): code style
b4caa00
@secustor
chore(prometheusreceiver): fixup root go mod
6e98d17
@secustor
Merge branch 'main' into implement_prometheus_receiver_target_allocator
ec1cd9d
@secustor
fix linting
e0a7f5a
@secustor
test: create tests
a91d8c9
@secustor
test: add targetallocator tests
e09b19b
@secustor
Merge branch 'main' into implement_prometheus_receiver_target_allocator
da0c7f6 
# Conflicts:
#	CHANGELOG.md
#	exporter/prometheusexporter/go.sum
#	go.sum
#	processor/spanmetricsprocessor/go.sum
#	receiver/prometheusreceiver/go.mod
#	receiver/prometheusreceiver/go.sum
#	testbed/go.sum
@secustor
Merge branch 'main' into implement_prometheus_receiver_target_allocator
9dbbed4 
# Conflicts:
#	CHANGELOG.md
#	processor/spanmetricsprocessor/go.sum
#	receiver/prometheusreceiver/go.mod
@secustor
chore: update Prometheus to 0.37.0
f4b693c
@secustor
fix: main drift
9c44fca
@secustor
chore(prometheusreceiver): add targetallocator docs and changelog rel…
9d06bfd 
…easer
@secustor
fix(prometheusreceiver): apply whole Promconfig instead of only disco…
c3025e6 
…very
@secustor
fix(prometheusreceiver): only abort during validation if both scrape_…
af07a72 
…config and target_allocator is missing
@secustor
chore(prometheusreceiver): fix linting errors
8a673f8
@secustor
chore: go mod tidy all
8098a89
@secustor
Merge branch 'main' into implement_prometheus_receiver_target_allocator
7c186f0 
# Conflicts:
#	cmd/configschema/go.mod
#	cmd/configschema/go.sum
#	go.mod
#	go.sum
@secustor secustor requested a review from a team July 18, 2022 19:44
jaronoff97
jaronoff97 approved these changes Jul 19, 2022
View reviewed changes
Copy link
Contributor

@jaronoff97 jaronoff97 left a comment

Choose a reason for hiding this comment

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

Overall, I like the approach here. Left one question following up on @dashpole's thread. One question: How could we prevent some jobs from getting target-allocated with this configuration? The use case is for a collector doing self-monitoring on it's prometheus port 0.0.0.0:8888 where if that gets target allocated only one collector is monitored. The workaround is to just have a service and use a service monitor, but i imagine some people won't want that.

receiver/prometheusreceiver/metrics_receiver.go Show resolved Hide resolved
@secustor secustor requested a review from Aneurysm9 July 23, 2022 19:30
@mx-psi mx-psi requested a review from dashpole July 25, 2022 08:12
This was referenced Jul 26, 2022
Closed
Closed
@secustor secustor mentioned this pull request Aug 3, 2022
Closed
@secustor secustor requested a review from dashpole August 3, 2022 21:08
@dashpole dashpole added the ready to merge Code review completed; ready to merge by maintainers label Aug 4, 2022
@bogdandrutu
Copy link
Member

I am so sorry that I missed this, can you rebase the PR and will merge this.

@secustor
Copy link
Member Author

@bogdandrutu no worries.

I have cleaned up the errors which come from the modified receiver, but there seem still to be issues with the rabbitmqreceiver

@secustor
Copy link
Member Author

@bogdandrutu The issues have been fixed. you can now merge.

@bogdandrutu bogdandrutu merged commit 41eceae into open-telemetry:main Aug 19, 2022
@secustor secustor deleted the implement_prometheus_receiver_target_allocator branch August 19, 2022 14:26
@dashpole dashpole added the receiver/prometheus Prometheus receiver label Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bogdandrutu bogdandrutu bogdandrutu left review comments

@jaronoff97 jaronoff97 jaronoff97 approved these changes

@dashpole dashpole dashpole approved these changes

@Aneurysm9 Aneurysm9 Aneurysm9 approved these changes

@dmitryax dmitryax Awaiting requested review from dmitryax

Assignees

@mx-psi mx-psi

Labels
ready to merge Code review completed; ready to merge by maintainers receiver/prometheus Prometheus receiver
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@secustor @bogdandrutu @Aneurysm9 @dashpole @jaronoff97 @mx-psi