search

artprima / prometheus-metrics-bundle

Symfony 5/6 Prometheus Metrics Bundle
MIT License
133 stars 29 forks source link

Collect status code in label #37

Closed L3tum closed 3 years ago

L3tum commented 3 years ago

Hey, thanks for this library!

This is a small change that would really go a long way IMO.

While it's already good to know whether a request was successful (2xx/3xx) or failed (4xx/5xx), it would be better to be able to distinguish why it failed. E.g. a 404 is dramatically different from a 401 or 403.

L3tum commented 3 years ago

I'm not sure why the integration tests are failing (I can't even find them in the repo)

denisvmedia commented 3 years ago

Not only integration tests are broken, unit tests are broken as well. You can find integration tests pipeline here: https://github.com/artprima/prometheus-metrics-bundle/blob/master/.github/workflows/symfony.yml

Also, could you please set develop branch as a destination of your PR? Thank you.

L3tum commented 3 years ago

The tests appear to be all fixed now. One thing I do not know is how you want to handle the all "action" and whether the status should be "all" as well, like it is now, or if there should be an additional null check.

I doubt the status can ever actually be null, but I wanted to stay consistent with the rest of the code.

denisvmedia commented 3 years ago

Hi @L3tum . I decided to not merge this MR. It appears so that it may have undesired consequences in projects that use this library. But I have a solution that should work for you:

artprima_prometheus_metrics:
    disable_default_metrics: true

By doing this, you can effectively disable the default metrics and use your own solution in your project.