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

Metric pattern validation in simpleclient_dropwizard is overly strict #518

Open
LiamClarkeNZ opened this issue Dec 18, 2019 · 6 comments
Open

Comments

@LiamClarkeNZ
Copy link

LiamClarkeNZ commented Dec 18, 2019

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 to appMetric.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 and GraphiteNamePattern (used within the CustomMappingSampleBuilder) expect via regex a very particular form of metric naming - app_example will not survive validation, but app.example will.

Given that Dropwizard itself does not enforce these metric naming conventions, I'm unsure as to why simpleclient_dropwizard does.

@brian-brazil
Copy link
Contributor

Would you like to send a PR?

@LiamClarkeNZ
Copy link
Author

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

@brian-brazil
Copy link
Contributor

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.

@LiamClarkeNZ
Copy link
Author

LiamClarkeNZ commented Dec 20, 2019 via email

@LiamClarkeNZ
Copy link
Author

PR: #519

@peetzen
Copy link

peetzen commented Feb 1, 2020

I was running into the exact same problem when writing a dropwizard wrapper bundle.
When trying to map io.dropwizard.jetty.MutableServletContextHandler.*-requests to servlet.requests with a label defined as method: ${0} the validation does not let it pass through due to * not followed by a period.

The whole match pattern validation seem unnecessary, as what matters at the end is the actual metric name.

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

No branches or pull requests

3 participants