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

Metrics transform processor: replace version in label values #876

Merged

Conversation

james-bebbington
Copy link
Member

Replace {{version}} in any label values in the metrics transform processor with the app version.

@james-bebbington james-bebbington requested a review from a team September 1, 2020 11:11
@james-bebbington james-bebbington changed the title Metrics transform processor: replace {{version}} in label values Metrics transform processor: replace version in label values Sep 1, 2020
@codecov
Copy link

codecov bot commented Sep 1, 2020

Codecov Report

Merging #876 into master will decrease coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #876      +/-   ##
==========================================
- Coverage   88.30%   88.28%   -0.03%     
==========================================
  Files         234      234              
  Lines       12464    12466       +2     
==========================================
- Hits        11006    11005       -1     
- Misses       1108     1111       +3     
  Partials      350      350              
Flag Coverage Δ
#integration 71.28% <ø> (ø)
#unit 88.10% <100.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
processor/metricstransformprocessor/factory.go 100.00% <100.00%> (ø)
receiver/prometheusexecreceiver/receiver.go 83.08% <0.00%> (-2.21%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6604bb1...36c1856. Read the comment docs.

operations:
- action: add_label
new_label: version
new_value: opentelemetry collector {{version}}
Copy link
Member

Choose a reason for hiding this comment

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

nit: Ideally there should be a way to escape this and be able to pass the {{version}} string as is (without replacement).
Though in practice I doubt anyone would ever run into this limitation.

Copy link
Member

Choose a reason for hiding this comment

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

I guess it's possible that some other processor also does substitution for {{version}}, but in a different way / with a different value. In that case the ordering of processors will matter.
Still don't think this is a real issue though.

Copy link
Member Author

@james-bebbington james-bebbington Sep 2, 2020

Choose a reason for hiding this comment

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

Yea this is a very simple substitution so I only defined trivial semantics.

Note the substitution only applies the configured values themselves, not the metric label values (so I don't think your second comment is relevant).

I may come back and add something more sophisticated later - I agree there should eventually be a way to escape this.

processor/metricstransformprocessor/README.md Outdated Show resolved Hide resolved
@bogdandrutu bogdandrutu merged commit dbb36f6 into open-telemetry:master Sep 2, 2020
dyladan referenced this pull request in dynatrace-oss-contrib/opentelemetry-collector-contrib Jan 29, 2021
ljmsc referenced this pull request in ljmsc/opentelemetry-collector-contrib Feb 21, 2022
* Prepare for releasing v0.7.0

* Update Changelog for v0.7.0 release

Co-authored-by: Joshua MacDonald <jmacd@users.noreply.github.com>
codeboten pushed a commit that referenced this pull request Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants