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

receiver/prometheus: remove assumption that scraped metrics share a resource #36479

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ridwanmsharif
Copy link
Contributor

@ridwanmsharif ridwanmsharif commented Nov 20, 2024

Description

This change removes as assumption that all metrics in a single scrape come from the same resource. This is indeed not true when honor_labels is set to true AND when the scraped metrics contain a job or instance label.

Link to tracking issue

Fixes #36477

Testing

Added unit tests

Documentation

N/A

@github-actions github-actions bot added the receiver/prometheus Prometheus receiver label Nov 20, 2024
…esource

This change removes as assumption that all metrics in a single scrape come from
the same resource. This is indeed not true when `honor_labels` is set to `true`
AND when the scraped metrics contain a `job` or `instance` label.
@ridwanmsharif ridwanmsharif force-pushed the ridwanmsharif/adjuster-resource-fix branch from 4a675f9 to b4af9c7 Compare November 21, 2024 21:37
@ridwanmsharif ridwanmsharif marked this pull request as ready for review November 21, 2024 21:38
@ridwanmsharif ridwanmsharif requested a review from a team as a code owner November 21, 2024 21:38
for i := 0; i < metrics.ResourceMetrics().Len(); i++ {
rm := metrics.ResourceMetrics().At(i)
job, found := rm.Resource().Attributes().Get(semconv.AttributeServiceName)
if !found {
return errors.New("adjusting metrics without job")
Copy link
Contributor

Choose a reason for hiding this comment

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

This does have the potential to be functionally different than the previous implementation - if one or more resources at the beginning of the slice have the appropriate attributes, and later resources do not have the appropriate attributes, some metrics will be adjusted, and some will not.

Should we iterate through all resources first and confirm they all have the appropriate attributes? From what I can tell the pmetric.Metrics object isn't re-used after Commit() fails, but in case of future updates changing that behavior it may be worth ensuring we don't partially modify the metrics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
receiver/prometheus Prometheus receiver
Projects
None yet
Development

Successfully merging this pull request may close these issues.

receiver/prometheus: Metric adjuster sets start time incorrectly when honor_labels==true
3 participants