From 1788cf2b092130adcc478e34f2d60f5447152369 Mon Sep 17 00:00:00 2001 From: Alexander Schwartz Date: Fri, 8 Mar 2024 16:09:07 +0100 Subject: [PATCH] Enable Infinispan metrics automatically if overall metrics are enabled Closes #27724 Signed-off-by: Alexander Schwartz --- .../topics/changes/changes-25_0_0.adoc | 5 ++ docs/guides/server/caching.adoc | 27 ++---- .../org/keycloak/config/CachingOptions.java | 8 ++ .../mappers/CachingPropertyMappers.java | 5 ++ .../mappers/MetricsPropertyMappers.java | 6 ++ .../infinispan/CacheManagerFactory.java | 7 ++ .../runtime/src/main/resources/cache-ispn.xml | 1 - .../src/main/resources/cache-local.xml | 1 - .../keycloak/it/cli/dist/MetricsDistTest.java | 35 +++----- .../src/test/resources/cache-local.xml | 87 ------------------- ...Test.testStartDevHelpAll.unix.approved.txt | 3 + ...istTest.testStartHelpAll.unix.approved.txt | 3 + ...estStartOptimizedHelpAll.unix.approved.txt | 3 + 13 files changed, 57 insertions(+), 134 deletions(-) delete mode 100644 quarkus/tests/integration/src/test/resources/cache-local.xml diff --git a/docs/documentation/upgrading/topics/changes/changes-25_0_0.adoc b/docs/documentation/upgrading/topics/changes/changes-25_0_0.adoc index ba82f9e337..9a59da934c 100644 --- a/docs/documentation/upgrading/topics/changes/changes-25_0_0.adoc +++ b/docs/documentation/upgrading/topics/changes/changes-25_0_0.adoc @@ -1,3 +1,8 @@ += Metrics for embedded caches enabled by default + +Metrics for the embedded caches are now enabled by default. +To enable histograms for latencies, set the option `cache-metrics-histograms-enabled` to `true`. + = Nonce claim is only added to the ID token The nonce claim is now only added to the ID token strictly following the OpenID Connect Core 1.0 specification. As indicated in the specification, the claim is compulsory inside the https://openid.net/specs/openid-connect-core-1_0.html#IDToken[ID token] when the same parameter was sent in the authorization request. The specification also recommends to not add the `nonce` after a https://openid.net/specs/openid-connect-core-1_0.html#RefreshTokenResponse[refresh request]. Previously, the claim was set to all the tokens (Access, Refresh and ID) in all the responses (refresh included). diff --git a/docs/guides/server/caching.adoc b/docs/guides/server/caching.adoc index a2e4bc4082..c6b3fd8a48 100644 --- a/docs/guides/server/caching.adoc +++ b/docs/guides/server/caching.adoc @@ -258,26 +258,13 @@ For more information about securing cache communication, see the https://infinis == Exposing metrics from caches -By default, metrics from caches are not automatically exposed when the metrics are enabled. +Metrics from caches are automatically exposed when the metrics are enabled. + +To enable histograms for the cache metrics, set `cache-metrics-histograms-enabled` to `true`. +While these metrics provide more insights into the latency distribution, collecting them might have a performance impact, so you should be cautious to activate them in an already saturated system. + +<@kc.start parameters="--metrics-enabled true --cache-metrics-histograms-enabled true"/> + For more details about how to enable metrics, see <@links.server id="configuration-metrics"/>. -To enable global metrics for all caches within the `cache-container`, you need to change your cache configuration file (e.g.: `conf/cache-ispn.xml`) to enable `statistics` at the `cache-container` level as follows: - -.enabling metrics for all caches -[source] ----- - - ... - ----- - -Similarly, you can enable metrics individually for each cache by enabling `statistics` as follows: - -.enabling metrics for a specific cache ----- - - ... - ----- - diff --git a/quarkus/config-api/src/main/java/org/keycloak/config/CachingOptions.java b/quarkus/config-api/src/main/java/org/keycloak/config/CachingOptions.java index b6c3cc5226..63a4acd3e9 100644 --- a/quarkus/config-api/src/main/java/org/keycloak/config/CachingOptions.java +++ b/quarkus/config-api/src/main/java/org/keycloak/config/CachingOptions.java @@ -19,6 +19,9 @@ public class CachingOptions { public static final String CACHE_REMOTE_USERNAME_PROPERTY = CACHE_REMOTE_PREFIX + "-username"; public static final String CACHE_REMOTE_PASSWORD_PROPERTY = CACHE_REMOTE_PREFIX + "-password"; + private static final String CACHE_METRICS_PREFIX = "cache-metrics"; + public static final String CACHE_METRICS_HISTOGRAMS_ENABLED_PROPERTY = CACHE_METRICS_PREFIX + "-histograms-enabled"; + public enum Mechanism { ispn, local @@ -117,4 +120,9 @@ public class CachingOptions { CACHE_CONFIG_FILE_PROPERTY, CACHE_REMOTE_HOST_PROPERTY, CACHE_REMOTE_USERNAME_PROPERTY)) .build(); + public static final Option CACHE_METRICS_HISTOGRAMS_ENABLED = new OptionBuilder<>(CACHE_METRICS_HISTOGRAMS_ENABLED_PROPERTY, Boolean.class) + .category(OptionCategory.CACHE) + .description("Enable histograms for metrics for the embedded caches.") + .build(); + } diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/CachingPropertyMappers.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/CachingPropertyMappers.java index 29b34d66cd..38bc0ba7f5 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/CachingPropertyMappers.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/CachingPropertyMappers.java @@ -61,6 +61,11 @@ final class CachingPropertyMappers { .paramLabel("password") .isMasked(true) .build(), + + fromOption(CachingOptions.CACHE_METRICS_HISTOGRAMS_ENABLED) + .isEnabled(MetricsPropertyMappers::metricsEnabled, MetricsPropertyMappers.METRICS_ENABLED_MSG) + .build(), + }; } diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/MetricsPropertyMappers.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/MetricsPropertyMappers.java index 8d5fb03670..c49200b8df 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/MetricsPropertyMappers.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/MetricsPropertyMappers.java @@ -2,11 +2,14 @@ package org.keycloak.quarkus.runtime.configuration.mappers; import org.keycloak.config.MetricsOptions; +import static org.keycloak.quarkus.runtime.configuration.Configuration.isTrue; import static org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper.fromOption; final class MetricsPropertyMappers { + public static final String METRICS_ENABLED_MSG = "metrics are enabled"; + private MetricsPropertyMappers(){} public static PropertyMapper[] getMetricsPropertyMappers() { @@ -18,4 +21,7 @@ final class MetricsPropertyMappers { }; } + public static boolean metricsEnabled() { + return isTrue(MetricsOptions.METRICS_ENABLED); + } } diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/storage/legacy/infinispan/CacheManagerFactory.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/storage/legacy/infinispan/CacheManagerFactory.java index 025e2ef846..1254b5d5ed 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/storage/legacy/infinispan/CacheManagerFactory.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/storage/legacy/infinispan/CacheManagerFactory.java @@ -50,6 +50,7 @@ import static org.keycloak.config.CachingOptions.CACHE_EMBEDDED_MTLS_KEYSTORE_FI import static org.keycloak.config.CachingOptions.CACHE_EMBEDDED_MTLS_KEYSTORE_PASSWORD_PROPERTY; import static org.keycloak.config.CachingOptions.CACHE_EMBEDDED_MTLS_TRUSTSTORE_FILE_PROPERTY; import static org.keycloak.config.CachingOptions.CACHE_EMBEDDED_MTLS_TRUSTSTORE_PASSWORD_PROPERTY; +import static org.keycloak.config.CachingOptions.CACHE_METRICS_HISTOGRAMS_ENABLED_PROPERTY; import static org.keycloak.config.CachingOptions.CACHE_REMOTE_HOST_PROPERTY; import static org.keycloak.config.CachingOptions.CACHE_REMOTE_PASSWORD_PROPERTY; import static org.keycloak.config.CachingOptions.CACHE_REMOTE_PORT_PROPERTY; @@ -110,6 +111,12 @@ public class CacheManagerFactory { if (metricsEnabled) { builder.getGlobalConfigurationBuilder().addModule(MicrometerMeterRegisterConfigurationBuilder.class); builder.getGlobalConfigurationBuilder().module(MicrometerMeterRegisterConfigurationBuilder.class).meterRegistry(Metrics.globalRegistry); + builder.getGlobalConfigurationBuilder().cacheContainer().statistics(true); + builder.getGlobalConfigurationBuilder().metrics().namesAsTags(true); + if (booleanProperty(CACHE_METRICS_HISTOGRAMS_ENABLED_PROPERTY)) { + builder.getGlobalConfigurationBuilder().metrics().histograms(true); + } + builder.getNamedConfigurationBuilders().forEach((s, configurationBuilder) -> configurationBuilder.statistics().enabled(true)); } // For Infinispan 10, we go with the JBoss marshalling. diff --git a/quarkus/runtime/src/main/resources/cache-ispn.xml b/quarkus/runtime/src/main/resources/cache-ispn.xml index 72cf71785e..b50cb34762 100644 --- a/quarkus/runtime/src/main/resources/cache-ispn.xml +++ b/quarkus/runtime/src/main/resources/cache-ispn.xml @@ -23,7 +23,6 @@ - diff --git a/quarkus/runtime/src/main/resources/cache-local.xml b/quarkus/runtime/src/main/resources/cache-local.xml index 09791b33e0..b44185402a 100644 --- a/quarkus/runtime/src/main/resources/cache-local.xml +++ b/quarkus/runtime/src/main/resources/cache-local.xml @@ -22,7 +22,6 @@ xmlns="urn:infinispan:config:14.0"> - diff --git a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/MetricsDistTest.java b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/MetricsDistTest.java index ba6605c152..2aed228c17 100644 --- a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/MetricsDistTest.java +++ b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/MetricsDistTest.java @@ -21,15 +21,11 @@ import static io.restassured.RestAssured.when; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.not; -import java.nio.file.Paths; import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; -import java.util.function.Consumer; import org.junit.jupiter.api.Test; -import org.keycloak.it.junit5.extension.BeforeStartDistribution; import org.keycloak.it.junit5.extension.DistributionTest; -import org.keycloak.it.junit5.extension.RawDistOnly; import org.keycloak.it.utils.KeycloakDistribution; import io.quarkus.test.junit.main.Launch; @@ -52,26 +48,21 @@ public class MetricsDistTest { when().get("/metrics").then() .statusCode(200) .body(containsString("jvm_gc_")) - .body(not(containsString("vendor_cache_manager_keycloak_cache_realms_"))); - } + .body(containsString("vendor_statistics_hit_ratio")) + .body(not(containsString("vendor_statistics_miss_times_seconds_bucket"))); - @Test - @Launch({ "start-dev", "--metrics-enabled=true", "--cache-config-file=cache-local.xml" }) - @BeforeStartDistribution(EnableCachingStatistics.class) - @RawDistOnly(reason = "No support mounting files to containers. Testing raw dist is enough.") - void testExposeCachingMetrics() { - when().get("/metrics").then() - .statusCode(200) - .body(containsString("vendor_cache_manager_keycloak_cache_")); - } - - @Test - @Launch({ "start-dev", "--metrics-enabled=true" }) - void testMetricsEndpointDoesNotEnableHealth() { when().get("/health").then() .statusCode(404); } + @Test + @Launch({ "start-dev", "--metrics-enabled=true", "--cache-metrics-histograms-enabled=true" }) + void testMetricsEndpointWithCacheMetricsHistograms() { + when().get("/metrics").then() + .statusCode(200) + .body(containsString("vendor_statistics_miss_times_seconds_bucket")); + } + @Test void testUsingRelativePath(KeycloakDistribution distribution) { for (String relativePath : List.of("/auth", "/auth/", "auth")) { @@ -113,10 +104,4 @@ public class MetricsDistTest { } } - public static class EnableCachingStatistics implements Consumer { - @Override - public void accept(KeycloakDistribution dist) { - dist.copyOrReplaceFileFromClasspath("/cache-local.xml", Paths.get("conf", "cache-local.xml")); - } - } } diff --git a/quarkus/tests/integration/src/test/resources/cache-local.xml b/quarkus/tests/integration/src/test/resources/cache-local.xml deleted file mode 100644 index c414d55eb1..0000000000 --- a/quarkus/tests/integration/src/test/resources/cache-local.xml +++ /dev/null @@ -1,87 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartDevHelpAll.unix.approved.txt b/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartDevHelpAll.unix.approved.txt index 12c9ef47b6..d1ad2d303f 100644 --- a/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartDevHelpAll.unix.approved.txt +++ b/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartDevHelpAll.unix.approved.txt @@ -39,6 +39,9 @@ Cache: 'cache-mtls-truststore.p12' under conf/ directory. --cache-embedded-mtls-trust-store-password The password to access the Truststore. +--cache-metrics-histograms-enabled + Enable histograms for metrics for the embedded caches. Default: false. + Available only when metrics are enabled. --cache-remote-host The hostname of the remote server for the remote store configuration. It replaces the 'host' attribute of 'remote-server' tag of the configuration diff --git a/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartHelpAll.unix.approved.txt b/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartHelpAll.unix.approved.txt index c0de4fbc1e..43be4d5009 100644 --- a/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartHelpAll.unix.approved.txt +++ b/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartHelpAll.unix.approved.txt @@ -40,6 +40,9 @@ Cache: 'cache-mtls-truststore.p12' under conf/ directory. --cache-embedded-mtls-trust-store-password The password to access the Truststore. +--cache-metrics-histograms-enabled + Enable histograms for metrics for the embedded caches. Default: false. + Available only when metrics are enabled. --cache-remote-host The hostname of the remote server for the remote store configuration. It replaces the 'host' attribute of 'remote-server' tag of the configuration diff --git a/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartOptimizedHelpAll.unix.approved.txt b/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartOptimizedHelpAll.unix.approved.txt index c1380196c2..961fbb926f 100644 --- a/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartOptimizedHelpAll.unix.approved.txt +++ b/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartOptimizedHelpAll.unix.approved.txt @@ -32,6 +32,9 @@ Cache: 'cache-mtls-truststore.p12' under conf/ directory. --cache-embedded-mtls-trust-store-password The password to access the Truststore. +--cache-metrics-histograms-enabled + Enable histograms for metrics for the embedded caches. Default: false. + Available only when metrics are enabled. --cache-remote-host The hostname of the remote server for the remote store configuration. It replaces the 'host' attribute of 'remote-server' tag of the configuration