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

Update GetAllTargetsByCollectorAndJob to use TargetItem hash #1086

Merged
merged 4 commits into from
Sep 12, 2022
Merged

Update GetAllTargetsByCollectorAndJob to use TargetItem hash #1086

merged 4 commits into from
Sep 12, 2022

Conversation

kelseyma
Copy link
Contributor

@kelseyma kelseyma commented Sep 9, 2022

Fixes #1083

Before (scraping was inconsistent due to labels being overwritten):

$ grep "ResourceMetrics #" collector.log 
2022-09-07T22:50:11.869Z	info	ResourceMetrics #0
2022-09-07T22:52:17.864Z	info	ResourceMetrics #0
2022-09-07T22:53:11.871Z	info	ResourceMetrics #0
2022-09-07T22:55:11.871Z	info	ResourceMetrics #0
2022-09-07T22:55:17.863Z	info	ResourceMetrics #0
2022-09-07T22:57:17.870Z	info	ResourceMetrics #0
2022-09-07T22:59:11.868Z	info	ResourceMetrics #0
2022-09-07T23:01:17.862Z	info	ResourceMetrics #0
2022-09-07T23:07:11.870Z	info	ResourceMetrics #0

After:

$ grep "ResourceMetrics #" collector.log 
2022-09-09T22:07:46.537Z	info	ResourceMetrics #0
2022-09-09T22:08:46.530Z	info	ResourceMetrics #0
2022-09-09T22:09:46.532Z	info	ResourceMetrics #0
2022-09-09T22:10:46.529Z	info	ResourceMetrics #0
2022-09-09T22:11:46.529Z	info	ResourceMetrics #0
2022-09-09T22:12:46.534Z	info	ResourceMetrics #0
2022-09-09T22:13:46.528Z	info	ResourceMetrics #0
2022-09-09T22:14:46.530Z	info	ResourceMetrics #0
2022-09-09T22:15:46.528Z	info	ResourceMetrics #0
2022-09-09T22:16:46.528Z	info	ResourceMetrics #0
2022-09-09T22:17:46.529Z	info	ResourceMetrics #0
2022-09-09T22:18:46.528Z	info	ResourceMetrics #0
2022-09-09T22:19:46.529Z	info	ResourceMetrics #0
2022-09-09T22:20:46.529Z	info	ResourceMetrics #0
2022-09-09T22:21:46.530Z	info	ResourceMetrics #0
2022-09-09T22:22:46.528Z	info	ResourceMetrics #0
2022-09-09T22:23:46.528Z	info	ResourceMetrics #0

@kelseyma
Update GetAllTargetsByCollectorAndJob to use TargetItem hash to preve…
271a1ac 
…nt overwriting of labels for targets with the same address
@kelseyma kelseyma requested a review from a team September 9, 2022 23:28
jaronoff97
jaronoff97 approved these changes Sep 10, 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.

This is great! Thanks for fixing this issue :D

@pavolloffay pavolloffay merged commit e48373c into open-telemetry:main Sep 12, 2022
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
@kelseyma @pavolloffay
Update GetAllTargetsByCollectorAndJob to use TargetItem hash (open-te…
50dbe7a 
…lemetry#1086)

* Update GetAllTargetsByCollectorAndJob to use TargetItem hash to prevent overwriting of labels for targets with the same address

* Fix unit test after merging main

Co-authored-by: Pavol Loffay <p.loffay@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaronoff97 jaronoff97 jaronoff97 approved these changes

@pavolloffay pavolloffay pavolloffay approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[target-allocator] GetAllTargetsByCollectorAndJob overwrites labels for targets that share the same address
3 participants
@kelseyma @pavolloffay @jaronoff97