-
-
Notifications
You must be signed in to change notification settings - Fork 56
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
feat: configurable cardinality for endpoint pattern label #72
Comments
Humm I'm not 💯 on this one. I totally get the need you have, that's clear. I'm just not convinced You can (I think) change your code to have two/however many exceptions as separate route declarations, even if they call the same code (a macro can easily help you do this), which would mean you get this for free in |
If I understand correctly you saying that I should generate the route declaration to actix on the fly based on user configuration? |
Oooh I think I misunderstood. You're saying that your |
Disclaimer: I'm not a martin developer For my personal use of martin, I have only 3 sources, so that's quite limited. AFAIK, I don't think many users have more than 10 sources. I don't think the "explode" qualifier is appropriate here. But the issue is that the first argument is not "source_id" but "source_ids" to account for different combinations of sources. We could have a work around that to limit the cardinality. To mitigate the issue, we will probably turn off the cardinality by default. So only the users that know what their doing can use that. As for the usefulness, for example, for our team, it would be useful to know which source combination has a performance issue or is the most used. |
I'll happily look at a PR (if you'd like to give it a shot) but I think if that's useful for your limited use-case, then perhaps it would be better to go about it in a different way. Perhaps creating explicit histograms/counters for |
Also one addition: So for example: so we let the martin code handle what cardinality to use with a hook, the hook will sort the source_ids in alphabetical order and then remove the cardinality for x, y and z |
Okay I just found out that the "[actix-web-prom/src/lib.rs:446] req.match_info()" = Path {
path: Url {
uri: "/address,parcels/13/4146/2816",
path: None,
},
skip: 29,
segments: [
(
"source_ids",
Segment(
1,
16,
),
),
(
"z",
Segment(
17,
19,
),
),
(
"x",
Segment(
20,
24,
),
),
(
"y",
Segment(
25,
29,
),
),
],
} |
Yes I see, but the issue for me is when I'm creating a different metric family (with different metric name), I will have to handle a special case just for that in my Grafana dashboard for example, the fact that everything is under a same metric name would simplify the setup for everyone else. |
Hello, first thanks for this wonderful crate.
Like discussed in #20 and implemented in #30 we currently try to get the match_pattern of the request and use that as the label value for endpoint.
This is a feature request to be able to configure the cardinality for the endpoint pattern in the label of the metrics. I started to develop a POC and I will probably do a PR.
This is related to this downstream issue maplibre/martin#773. For the use case of the martin tile server we need to have the details by
sources_ids
, to be able to tell which source is causing which latencies or which source is being the most requested. The cardinality here is limited since we have a fixed number of sources.To be clear we need to have a way to transform this:
into:
What are your take on this?
The text was updated successfully, but these errors were encountered: