-
-
Notifications
You must be signed in to change notification settings - Fork 29
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
Remove "path" from labels to keep cardinality low #1
Comments
Thanks for the issue. Raising cardinality problems is also good 😁. I‘ve added some tests to ensure that the default normalizer does at least something:
Reference to the spec: https://github.com/tdeekens/promster/blob/master/packages/metrics/modules/normalizers/path/path.spec.js |
As a side note: you can always pass a custom normalizer during configuration to be more strict. However, let me know if we definitely don’t normalize enough. |
I elaborated on the expectations towards the default normalisation. It meets by expectations towards a sensible default for well-formed urls. It's also what most other express/hapi exporters settled up doing. |
I got a bit lost but ok, as long as you are aware of the cardinality problem and that the library handles this I'm fine 😇 |
It tries to offer a reasonably sane default. It's mostly aligned with the 3-4 other exporters which do the same. Given that more have this we can either drop the url or try to be even more strict. I've looked into how and can't really think of a way for now. Thanks again for raising it! |
No description provided.
The text was updated successfully, but these errors were encountered: