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/dockerstats] Fix ToMetricLabels behaviour #21160

Merged
merged 1 commit into from
May 15, 2023

Conversation

paologallinaharbur
Copy link
Member

Description:
Only one label/envVar was added even if multiple ones were specified
by ContainerLabelsToMetricLabels or EnvVarsToMetricLabels.
Adding a local variable shadowing the loop one solves the issue.

Without such fix the label variable assumes the value of the last element computed in the loop.

More details can be find reading #21113

Link to tracking Issue:
Fix #21113

Testing:
The tests that were already existing have been updated to cover the broken use-case

Copy link
Contributor

@jamesmoessis jamesmoessis left a comment

Choose a reason for hiding this comment

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

Thanks!

@paologallinaharbur
Copy link
Member Author

paologallinaharbur commented May 9, 2023

Is there some other work needed? I fear it is getting stale 😕

@atoulme atoulme added the ready to merge Code review completed; ready to merge by maintainers label May 9, 2023
@atoulme
Copy link
Contributor

atoulme commented May 9, 2023

I am rerunning your failing test run, approved and marked the PR as ready to merge.

Only one label/envVar was added even if multiple ones were specified
by ContainerLabelsToMetricLabels or EnvVarsToMetricLabels.
Adding a local variable shadowing the loop one solves the issue.

Signed-off-by: paologallinaharbur <paologallina1992@gmail.com>
@paologallinaharbur
Copy link
Member Author

paologallinaharbur commented May 15, 2023

Thanks, @atoulme, I force-pushed to rebase and trigger tests execution again, but I've not changed the code.
Everything should be ready now 😄

@codeboten codeboten merged commit 6eb5eff into open-telemetry:main May 15, 2023
84 checks passed
@github-actions github-actions bot added this to the next release milestone May 15, 2023
varunraiko pushed a commit to varunraiko/opentelemetry-collector-contrib that referenced this pull request May 17, 2023
…1160)

Only one label/envVar was added even if multiple ones were specified
by ContainerLabelsToMetricLabels or EnvVarsToMetricLabels.
Adding a local variable shadowing the loop one solves the issue.

Signed-off-by: paologallinaharbur <paologallina1992@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Code review completed; ready to merge by maintainers receiver/dockerstats
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[receiver/dockerstat] Only one label/envVar is added even if multiple ones are specified
5 participants