-
-
Notifications
You must be signed in to change notification settings - Fork 92
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 and label naming #115
Comments
Hey Luca and thanks for reaching out! Let me address a couple of things in your issue description prior to giving you my thoughts on this feature request (if I follow correctly that is haha).
Prometheus will create a new time-series for a unique combination of metric names and their unique key-value label values. As a result, regardless of whether the application name is in the label or in the name, the same number of time-series will be created in Prometheus.
I remember reading this style guide several times over prior to writing a single line of code for PromEx...and my take away from it is perhaps a little different from yours it seems. When I read: "A metric name should have a (single-word) application prefix relevant to the domain the metric belongs to.", my feeling is that the "domain" is your application and not the PromEx plugin. Another piece of literature that guided my decision (that I think corroborates the previous sentiment) was from the Robust Perception blog who I consider to be leaders in the monitoring space:
With that being said, I am hesitant (not against 100% mind you) to support this kind of option in PromEx. One of the reasons that I am hesitant is that the Grafana dashboard support would effectively double as each PromQL query would need two versions ( Sorry to be long winded, but this is a big change and I want to make sure that it is thoroughly discussed prior to me committing to anything. If you do want to create a PoC with this I would be happy to look it over and provide guidance. Off the top of my head, you would need to do the following:
Hope that helps and looking forward to your response :) |
Hi Alexander, thank you so much for the answer. I agree with you that regardless of whether the application name is in the label or in the name, the same number of time-series will be created in Prometheus, we consider this as its "internal" optimization. We proposed this for different reasons:
I agree with you that this would become a maintenance nightmare supporting two "different styles", we adopt as metric naming convention what stated in
where we consider What inspires us to this way is this example in instrumentation:
I think that before starting with the PoC you can imagine the result, which would be to make sure that it is thoroughly discussed with pros and cons about those two styles and maybe asking help directly to Prometheus developer in order to have some guidance about this issue. Thank you for your time and support 🙏 |
I haven't forgotten about this issue :P. Still pondering on it and trying to think about how to proceed. |
No problem, I understand that it's not a simple issue to reason about, if I can help in some ways don't hesitate to contact me. 🙏 |
I found this article about label cardinality, maybe this can help with this discussion |
I think i have a good way to do this without making it difficult to maintain. I scheduled it for release 1.9. If you want to tackle this, lmk and we can sync up as to what I have in mind. |
Yep, I would like to tackle this issue, thank you! |
Awesome! That is much appreciated. When I have a moment in the next couple days, I'll jot down some notes here as that what I think could help move this feature along :). |
Alrighty, as promised, here are some of my thoughts as to how I would possibly approach this problem (feel free to deviate from these suggestions as they are just suggestions 😄 ):
Hopefully that helps guide you in the right direction. LMK if you have any questions or suggestions 😄! |
@lucazulian @akoutmos any update on this one? |
Hey, @HarshBalyan, thanks for your interest. I'm so sorry, but I'm still working on this as it would be a not so easy issue. |
If anyone else is interested in renaming their metric prefixes, this is how I did it:
{Plugins.Phoenix, metric_prefix: [:my, :custom, :prefix, :here]},
phoenix_metric_prefix: ["my", "custom", "prefix", "here"], This option is listed on all default plugins' docs: I was also able to use a blank prefix on metrics by adding |
If you are going to consider changes to the metric names (I know here it is only about the prefix, but just in case) it'd be great to collaborate. I was thinking it'd be fortunate if prom_ex's dashboards worked with OpenTelemetry metrics (https://opentelemetry.io/) by default and was looking at the metric names used as we prepare to define the metrics exported by OpenTelemetry. See https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/runtime-environment-metrics.md for an example of how these get defined, right now it just has the JVM. It may just be that we use your existing metric names (minus the app name prefix), in which case we'll be compatible except for the prefix, so thought this issue was a good place to put this. |
Using
prom_ex
in all application in a company could be raise the problem about metrics and labels.The problem raises, for example, with multiple application with the same metric semantic but with different metric name, i.e.:
my_cool_app
->my_cool_app_application_dependency_info{modules="69",name="hex",version="0.20.5"} 1
my_other_cool_app
->my_other_cool_app_application_dependency_info{modules="69",name="hex",version="0.20.5"} 1
This causes an explosion of metrics with the same semantic.
Based on this style guide I propose to have a configuration with which to choose to move to a different way of metric naming. With the previous example this would be:
my_cool_app
->application_dependency_info{modules="69",name="hex",version="0.20.5",application="my_cool_app"} 1
my_other_cool_app
->application_dependency_info{modules="69",name="hex",version="0.20.5",application="my_other_cool_app"} 1
This would be a breaking change, but this should be managed with an opt-in configuration.
I would like to work on it if this could be interesting.
The text was updated successfully, but these errors were encountered: