search

aerogear / keycloak-metrics-spi

Adds a Metrics Endpoint to Keycloak
Apache License 2.0
551 stars 156 forks source link

add metric for total number of requests #90

Closed xinau closed 3 years ago

xinau commented 3 years ago

currently only response errors are counted, while this it self is useful it would be more complete to also log the total number of requests. by comparing the ratio of errors and total requests a operator is able to determine the healthiness of their installations.

the concrete change needs to be made somewhere along these lines. i'm still unsure how to implement it "correctly", but would like to contribute to the project. 2 solutions that come to my mind would be to either.

  1. drop the if clause and count all responses
  2. add a new metric keycloak_responses_total similar to keycloak_response_errors that counts all responses

https://github.com/aerogear/keycloak-metrics-spi/blob/a9ede42d7a462c6670cdaa6772d27806c2500de9/src/main/java/org/jboss/aerogear/keycloak/metrics/MetricsFilter.java#L44-L48

pb82 commented 3 years ago

@xinau That sounds reasonable. But if the goal is assessing the health, wouldn't a rate on the response errors also do the job?

xinau commented 3 years ago

@pb82 Using the error rate can be misleading. I.x. I do know when there is a larger response errors than expected, but this could also be the case when the overall traffic went up and shouldn't need to worry me in the case the error to total response ratio remains the same.

If it's okay with you I can provide a timely patch and tests.