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

Controller name as metric label as option with .UsePrometheusRequestDurations() #44

Closed
billpieper opened this issue Mar 25, 2022 · 5 comments

Comments

@billpieper
Copy link
Contributor

We are migrating our applications from Prometheus-net to Prometheus.Client.

Prometheus-net had an extension method to IApplicationBuilder called UseHttpMetrics(). With the implementation in Prometheus.Client.HttpRequestDurations, I have been unable to identify a way to incorporate the controller name as a label in the metric. We really need this as if we use the path instead -- we end up with significant label explosion due to many parameters that are inorporated into our api route paths.

Is there an existing way to get 'Controller' name as a label? and if not, is this something that could be added?

@sanych-sun
Copy link
Member

Try to use "UseRouteName" it could resolve route name/pattern instead of raw path.

@phnx47
Copy link
Member

phnx47 commented Mar 30, 2022

@billpieper "UseRouteName" is working for you?

@billpieper
Copy link
Contributor Author

No, unfortunately this does not meet our needs. We have [Route()] attributes decorating our controllers at a class level classes like this [Route("api/[controller]")]. Then at methods where we may need to take in a parameter in the route we will have a route attribute defined there too -- something like this: [Route("{id}")]

The UseRouteName option ends up setting a label that doesn't let us distinguish between our controllers and routes.

I ended up forking and adding the option to include a label for Controller and for Action. This mimics what was available in the old sdk. Additionally, I added an option to enable the metric to include the Timestamp Is this something you would consider adding and/or accepting a PR to include?

@phnx47
Copy link
Member

phnx47 commented Mar 31, 2022

@billpieper Yes, raise your PR, if work is already done, please. I want to create options to add Controller and Action to labels, but by default without this, something like this:

app.UsePrometheusRequestDurations(q =>
{
  q.IncludeConroller = true;
  q.IncludeAction = true;
});

@phnx47
Copy link
Member

phnx47 commented Apr 6, 2022

Published v3.5.0

@phnx47 phnx47 closed this as completed Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants