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

Generic events by client_id #82

Closed
wants to merge 2 commits into from
Closed

Generic events by client_id #82

wants to merge 2 commits into from

Conversation

dmittriy13
Copy link

Motivation

Add label "client_id" to generic event counters.

What

Add label "client_id" to generic event counters.

Why

To separate metrics by client_id.

How

Added label "client_id" to generic event counters.

Verification Steps

  1. Build the SPI from this branch and start Keycloak with it.
  2. Login with users
  3. Open the metrics endpoint in a browser.

Checklist:

  • Code has been tested locally by PR requester
  • Changes have been successfully verified by another team member

Progress

  • Finished task

Additional Notes

@pb82
Copy link
Contributor

pb82 commented Mar 5, 2021

@dmittriy13 the change looks fine, the only concern is that this increases the total amount of logs (because now those metrics are multiplied by the number of clients). I think the number of clients is typically rather low, but still something to be aware.

Is there a specific use case for requiring this grouping?

@efbgirotto
Copy link

Is there a specific use case for requiring this grouping?

I have a use case where I need to measure not all generic event types but only TOKEN_EXCHANGE and TOKEN_EXCHANGE_ERROR. Maybe this PR could evolve to enable labels on generic fields by configuration.

@alexted
Copy link

alexted commented Nov 8, 2024

@dmittriy13 Is this PR still relevant?

@dmittriy13
Copy link
Author

@dmittriy13 Is this PR still relevant?

Hi, this is no longer relevant, this is not the intended use of metric tags because tag values should be finite.

@dmittriy13 dmittriy13 closed this Nov 9, 2024
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

Successfully merging this pull request may close these issues.

4 participants