search

aerogear / keycloak-metrics-spi

Adds a Metrics Endpoint to Keycloak
Apache License 2.0
523 stars 151 forks source link

closes #155 make compatible with keycloak >= 21.0.0 #157

Closed ghenadiibatalski closed 1 year ago

ghenadiibatalski commented 1 year ago

see naming conventions https://prometheus.io/docs/instrumenting/writing_exporters/#naming

The SPI seems to be incompatible with Keycloak >=21.0.0 This PR is switched to Java 17 and Keycloak 21.0 + Prometheus 0.16.0 compatibility

In this form it works on Keycloak 21.0.1 Quarkus on Java 17

Motivation

Add references to relevant tickets or a short description about what motivated you do it. (E.g JIRA: https://issues.jboss.org/browse/AEROGEAR-{} AND/OR ISSUE: https://github.com/aerogear/standards/issues/{})

What

Add an short answer for: What was done in this PR? (E.g Don't allow users has access to the feature X.)

Why

Add an short answer for: Why it was done? (E.g The feature X was deprecated.)

How

Add an short answer for: How it was done? (E.g By removing this feature from ... OR By removing just the button but not its implementation ... )

Verification Steps

Add the steps required to check this change. Following an example.

  1. Go to XX >> YY >> SS
  2. Create a new item N with the info X
  3. Try to edit this item
  4. Check if in the left menu the feature X is not so long present.

Checklist:

Progress

Additional Notes

PS.: Add images and/or .gifs to illustrate what was changed if this pull request modifies the appearance/output of something presented to the users.

pb82 commented 1 year ago

@ghenadiibatalski is this the only change required to make it compatible with Keycloak 21?

ghenadiibatalski commented 1 year ago

@pb82 it works for me, but you should check the metrics names, they are suffixed now with _total, _sum, etc. I think, it is a feature of prometheus.

pb82 commented 1 year ago

@ghenadiibatalski I've tested this with KC 19, 20 and 21. The only problem is the source compatibility bump, this makes it incompatible with older version. Is this needed? Other than that i'd like to get this merged and released :+1:

pb82 commented 1 year ago

@ghenadiibatalski thanks, I'll merge this and follow up with another PR to set the source compatibility to Java 11.