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

add metric for total number of requests #90

Closed
xinau opened this issue Dec 23, 2020 · 2 comments
Closed

add metric for total number of requests #90

xinau opened this issue Dec 23, 2020 · 2 comments

Comments

@xinau
Copy link
Contributor

xinau commented Dec 23, 2020

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

// We are only interested in recording the response status if it was an error
// (either a 4xx or 5xx). No point in counting the successful responses
if (status >= 400) {
PrometheusExporter.instance().recordResponseError(status, req.getMethod());
}

@pb82
Copy link
Contributor

pb82 commented Mar 5, 2021

@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
Copy link
Contributor Author

xinau commented Mar 12, 2021

@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.

xinau pushed a commit to xinau/keycloak-metrics-spi that referenced this issue Apr 19, 2021
issue: aerogear#90

this change adds a new metric for reporting the total number of
requests that where made and not only the number of failed requests.
this makes it possible to calculate the ratio of errors to total
number of requests.
xinau added a commit to xinau/keycloak-metrics-spi that referenced this issue Apr 19, 2021
issue: aerogear#90

this change adds a new metric for reporting the total number of
requests that where made and not only the number of failed requests.
this makes it possible to calculate the ratio of errors to total
number of requests.
xinau added a commit to xinau/keycloak-metrics-spi that referenced this issue May 7, 2021
issue: aerogear#90

this change adds a new metric for reporting the total number of
requests that where made and not only the number of failed requests.
this makes it possible to calculate the ratio of errors to total
number of requests.
@pb82 pb82 closed this as completed May 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants