-
Notifications
You must be signed in to change notification settings - Fork 208
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
Add a prometheus label mapping component #2025
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some initial doc review comments
docs/sources/reference/components/prometheus/prometheus.mapping.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/prometheus/prometheus.mapping.md
Outdated
Show resolved
Hide resolved
Each time series is uniquely identified by its metric name, plus optional | ||
key-value pairs called labels. Each sample represents a datapoint in the | ||
time series and contains a value and an optional timestamp. | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add a language here... is this just plaintext? YAML? etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It come from prometheus.relabel
documentation.Should I amend it as well ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets fix it here and I'll spin up a PR to align the relabel doc.
docs/sources/reference/components/prometheus/prometheus.mapping.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/prometheus/prometheus.mapping.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/prometheus/prometheus.mapping.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/prometheus/prometheus.mapping.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/prometheus/prometheus.mapping.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/prometheus/prometheus.mapping.md
Outdated
Show resolved
Hide resolved
Thanks for contributing.
Could you share the use case and performance differences? In many cases users can use If the reason to add this is that prometheus.relabel it's not efficient, we will need some more data on how inefficient it is and whether it could be optimised to work faster. Any new solution we would consider will require benchmarks to prove that it's more efficient. We also generally want to avoid having many ways of doing the same thing - but there can be exceptions. |
I have to map a label holding the service ID to a label holding the tenant ID. My test case is using X services ID mapped to Y tenants. My test case is mapping 100 different services ID to a single tenant. Using Using As my worst case can be X == Y, i haven't try to summarize regex so i have a rule block for each service ID |
PR Description
This PR add a prometheus component to create a label based on a source_label value and a static mapping table.
Which issue(s) this PR fixes
For a large mapping table, using regex with
prometheus.relabel
is inefficientNotes to the Reviewer
PR Checklist