nlopes / actix-web-prom

Actix-web middleware to expose Prometheus metrics
MIT License
92 stars 56 forks source link

Question: How to avoid `WARN actix_web_prom: Cannot build mixed cardinality pattern` ? #82

Open helio-frota opened 4 months ago

helio-frota commented 4 months ago

Hi, I'm excluding swagger-ui but I'm still seeing a warning in logs

let metrics = PrometheusMetricsBuilder::new(namespace.as_ref())
            .registry(registry.into())
            .exclude("/swagger-ui")
            .exclude_regex("/swagger-ui/.*")
            .build();
2024-05-13T14:04:36.844346Z  WARN actix_web_prom: Cannot build mixed cardinality pattern "/swagger-ui/{_:.*}"
2024-05-13T14:04:36.850008Z  WARN actix_web_prom: Cannot build mixed cardinality pattern "/swagger-ui/{_:.*}"

Am I using the correct approach? thanks

helio-frota commented 4 months ago

Hi I noticed that it successfully excludes the paths, but continues showing the warning in logs https://github.com/trustification/trustify/issues/261#issuecomment-2108509183

nlopes commented 1 month ago

Hi @helio-frota,

Can you try to point your branch at main of actix-web-prom?

I just pushed the change below (I've linked the code entry) which should give me better visibility on the problem. As it stands it seems that the warning is appearing because the params passed to the endpoint aren't parsed properly but this should help.

https://github.com/nlopes/actix-web-prom/blob/9fbe5b99740e6a98a53ed748ff51f8173c903568/src/lib.rs#L800

helio-frota commented 1 month ago

@nlopes hi,

thanks for the feedback, I found the place where this happened https://github.com/nlopes/actix-web-prom/pull/83/files but I closed the PR since that was not a good way to fix

lefuturiste commented 1 month ago

Hi @helio-frota,

Can you try to point your branch at main of actix-web-prom?

I just pushed the change below (I've linked the code entry) which should give me better visibility on the problem. As it stands it seems that the warning is appearing because the params passed to the endpoint aren't parsed properly but this should help.

https://github.com/nlopes/actix-web-prom/blob/9fbe5b99740e6a98a53ed748ff51f8173c903568/src/lib.rs#L800

Okay I think this is my fault, in code introduced in #73.

The root issue is in the code that build a mixed pattern:

The full_pattern variable is populated from req.match_pattern() and my code is expecting to only have simple brace expression with only simple identifier. The full_pattern is then passed to the strfmt crate that of course doesn't accept any wild card patterns like in the route matching pattern "/swagger-ui/{_:.*}"

The issue is that the way we parse the match pattern with strfmt is different that the way that actix-web parse it. The current code is a crappy workaround because actix-web doesn't expose the parsed match pattern. So may be we should review if we can use or call the code in actix-web that parse the match pattern.