-
Notifications
You must be signed in to change notification settings - Fork 331
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
fix: support aws_tag_select with non-standard resource id in metric #648
Conversation
ae9553e
to
fb5a6f8
Compare
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.
Thank you for tackling this! I like giving users the option to supply patterns.
I see that we can both encode known patterns in ARN_RESOURCE_ID_REGEXP_FOR_TYPE
and allow configuring custom patterns. What guidance can we give future contributors on whether they should add patterns to the code or the examples?
Would it be "good enough" to keep the exporter oblivious of specific patterns, and only add the elasticloadbalancing:targetgroup
to the examples?
Will this replace the workaround outlined in #571 or is it separate? |
For me it would be good enough, but for the general user it would certainly be preferable for as a much as possible to work out of the box. |
It would replace the label_replace trick. |
I'm afraid this exporter does not work "out of the box" at all 😬 The example configurations are the main way to "get started", and where we do collect service specifics, so I think to keep things clear and maintainable going forward, I would rather remove |
This has the added benefit of showing how to use it, so that hopefully users for other services can help themselves without having to wait for exporter code changes and releases. |
Awesome!
…On Sun, Mar 3, 2024 at 4:08 PM Mårten Svantesson ***@***.***> wrote:
Will this replace the workaround outlined in #571
<#571> or is it
separate?
It would replace the label_replace trick.
—
Reply to this email directly, view it on GitHub
<#648 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABAEBTXLLNBK3J3TI2QUHLYWM4HTAVCNFSM6AAAAABDAE7NZ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZVGE4TEMBVHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Fixes prometheus#268 Signed-off-by: Mårten Svantesson <[email protected]>
fb5a6f8
to
d181c72
Compare
I have now removed ARN_RESOURCE_ID_REGEXP_FOR_TYPE from the code. I'll look at documentation soon |
Signed-off-by: Mårten Svantesson <[email protected]>
Now I have documented the new option and updated a couple of examples to use it |
Awesome, thank you! Merging despite the |
Fixes #268
@matthiasr I'll add an update to the README when I get the word that the code is at least in the ball park ready to be merged.