-
Notifications
You must be signed in to change notification settings - Fork 808
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
Metric pattern validation in simpleclient_dropwizard is overly strict #518
Comments
Would you like to send a PR? |
Hi @brian-brazil, Yep, I'm intending to, I have a few days free coming up so will put something together then. Can I ask your opinion before I do, though? I was intending, in order to preserve backwards compatibility for anyone using the existing CustomMappingSampleBuilder, to create class(es) providing similar functionality alongside it, instead of modifying the classes mentioned above directly. How does that sit with you? Cheers, Liam Clarke |
I'd modify directly, and I'm not even sure how this could break anyone. I'd suggest seeing what the statsd/graphite exporter do here to, as we should be in line with them. |
Thanks Brian, look forward to your feedback on the PR.
…On Fri, Dec 20, 2019 at 12:18 PM Brian Brazil ***@***.***> wrote:
I'd modify directly, and I'm not even sure how this could break anyone.
I'd suggest seeing what the statsd/graphite exporter do here to, as we
should be in line with them.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#518?email_source=notifications&email_token=AC2Q2JG7QVJH3FY7EBBXWCLQZP6LJA5CNFSM4J4EDD22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHLLQ3A#issuecomment-567720044>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AC2Q2JCXCJFMXSH7NVXYOXDQZP6LJANCNFSM4J4EDD2Q>
.
|
PR: #519 |
I was running into the exact same problem when writing a dropwizard wrapper bundle. The whole match pattern validation seem unnecessary, as what matters at the end is the actual metric name. |
I am exposing metrics to Prometheus in an existing app that uses Dropwizard, and they have a wide variety of names. From
app_metric_some_count
toappMetric.someCount
. For legacy monitoring reasons I am unable to rename these metrics.When attempting to use a CustomMappingSampleBuilder to map these existing names to metric names that align with Prometheus best practices, I am unable to do so because both
MappingConfig
andGraphiteNamePattern
(used within theCustomMappingSampleBuilder
) expect via regex a very particular form of metric naming -app_example
will not survive validation, butapp.example
will.Given that Dropwizard itself does not enforce these metric naming conventions, I'm unsure as to why
simpleclient_dropwizard
does.The text was updated successfully, but these errors were encountered: