From 42b9141326a6694fb4849d5d153f89716ea104a5 Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Thu, 12 Nov 2020 19:28:01 -0300 Subject: [PATCH] [KEYCLOAK-13639] - Improvements to metrics and health status --- .../quarkus/deployment/KeycloakProcessor.java | 50 +++++++++++++++++-- .../quarkus/QuarkusRequestFilter.java | 11 ---- .../keycloak/quarkus/KeycloakRecorder.java | 22 -------- .../keycloak/services/NotFoundHandler.java | 29 +++++++++++ .../health/KeycloakMetricsHandler.java | 29 +++++++++++ .../src/main/resources/application.properties | 7 ++- .../transaction/JtaTransactionWrapper.java | 6 --- 7 files changed, 108 insertions(+), 46 deletions(-) create mode 100644 quarkus/runtime/src/main/java/org/keycloak/services/NotFoundHandler.java create mode 100644 quarkus/runtime/src/main/java/org/keycloak/services/health/KeycloakMetricsHandler.java diff --git a/quarkus/deployment/src/main/java/org/keycloak/quarkus/deployment/KeycloakProcessor.java b/quarkus/deployment/src/main/java/org/keycloak/quarkus/deployment/KeycloakProcessor.java index c2868cc5cb..13487a6c12 100644 --- a/quarkus/deployment/src/main/java/org/keycloak/quarkus/deployment/KeycloakProcessor.java +++ b/quarkus/deployment/src/main/java/org/keycloak/quarkus/deployment/KeycloakProcessor.java @@ -37,6 +37,10 @@ import io.quarkus.deployment.IsDevelopment; import io.quarkus.deployment.builditem.HotDeploymentWatchedFileBuildItem; import io.quarkus.deployment.builditem.IndexDependencyBuildItem; import io.quarkus.hibernate.orm.deployment.HibernateOrmConfig; +import io.quarkus.smallrye.health.runtime.SmallRyeHealthHandler; +import io.quarkus.vertx.http.deployment.RouteBuildItem; +import io.vertx.core.Handler; +import io.vertx.ext.web.RoutingContext; import org.hibernate.cfg.AvailableSettings; import org.hibernate.jpa.boot.spi.PersistenceUnitDescriptor; import org.jboss.logging.Logger; @@ -66,7 +70,9 @@ import io.quarkus.deployment.annotations.Record; import io.quarkus.deployment.builditem.FeatureBuildItem; import io.quarkus.hibernate.orm.deployment.PersistenceUnitDescriptorBuildItem; import io.quarkus.vertx.http.deployment.FilterBuildItem; +import org.keycloak.services.NotFoundHandler; import org.keycloak.services.ServicesLogger; +import org.keycloak.services.health.KeycloakMetricsHandler; import org.keycloak.services.resources.KeycloakApplication; import org.keycloak.transaction.JBossJtaTransactionManagerLookup; import org.keycloak.util.Environment; @@ -75,6 +81,8 @@ class KeycloakProcessor { private static final Logger logger = Logger.getLogger(KeycloakProcessor.class); + private static final String DEFAULT_HEALTH_ENDPOINT = "/health"; + @BuildStep FeatureBuildItem getFeature() { return new FeatureBuildItem("keycloak"); @@ -195,13 +203,41 @@ class KeycloakProcessor { indexDependencyBuildItemBuildProducer.produce(new IndexDependencyBuildItem("org.keycloak", "keycloak-services")); } - @Record(ExecutionTime.RUNTIME_INIT) @BuildStep - void initializeFilter(BuildProducer routes, KeycloakRecorder recorder) { - Optional metricsEnabled = Configuration.getOptionalBooleanValue(MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX.concat("metrics.enabled")); + void initializeFilter(BuildProducer filters) { + filters.produce(new FilterBuildItem(new QuarkusRequestFilter(),FilterBuildItem.AUTHORIZATION - 10)); + } - routes.produce(new FilterBuildItem(recorder.createFilter(metricsEnabled.orElse(false)), - FilterBuildItem.AUTHORIZATION - 10)); + /** + *

Initialize metrics and health endpoints. + * + *

The only reason for manually registering these endpoints is that by default they run as blocking hence + * running in a different thread than the worker thread started by {@link QuarkusRequestFilter}. + * See https://github.com/quarkusio/quarkus/issues/12990. + * + *

By doing this, custom health checks such as {@link org.keycloak.services.health.KeycloakReadyHealthCheck} is + * executed within an active {@link org.keycloak.models.KeycloakSession}, making possible to use it when calculating the + * status. + * + * @param routes + */ + @BuildStep + void initializeMetrics(BuildProducer routes) { + Handler healthHandler; + Handler metricsHandler; + + if (isMetricsEnabled()) { + healthHandler = new SmallRyeHealthHandler(); + metricsHandler = new KeycloakMetricsHandler(); + } else { + healthHandler = new NotFoundHandler(); + metricsHandler = new NotFoundHandler(); + } + + routes.produce(new RouteBuildItem(DEFAULT_HEALTH_ENDPOINT, healthHandler)); + routes.produce(new RouteBuildItem(DEFAULT_HEALTH_ENDPOINT.concat("/live"), healthHandler)); + routes.produce(new RouteBuildItem(DEFAULT_HEALTH_ENDPOINT.concat("/ready"), healthHandler)); + routes.produce(new RouteBuildItem(KeycloakMetricsHandler.DEFAULT_METRICS_ENDPOINT, metricsHandler)); } @BuildStep(onlyIf = IsDevelopment.class) @@ -310,4 +346,8 @@ class KeycloakProcessor { throw new RuntimeException("No valid ConfigProvider found"); } } + + private boolean isMetricsEnabled() { + return Configuration.getOptionalBooleanValue(MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX.concat("metrics.enabled")).orElse(false); + } } diff --git a/quarkus/runtime/src/main/java/org/keycloak/provider/quarkus/QuarkusRequestFilter.java b/quarkus/runtime/src/main/java/org/keycloak/provider/quarkus/QuarkusRequestFilter.java index 28a4996c17..dbe032afba 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/provider/quarkus/QuarkusRequestFilter.java +++ b/quarkus/runtime/src/main/java/org/keycloak/provider/quarkus/QuarkusRequestFilter.java @@ -40,15 +40,8 @@ public class QuarkusRequestFilter extends AbstractRequestFilter implements Handl // we don't really care about the result because any exception thrown should be handled by the parent class }; - private Predicate enabledEndpoints; - @Override public void handle(RoutingContext context) { - if (!enabledEndpoints.test(context)) { - context.fail(404); - return; - } - // our code should always be run as blocking until we don't provide a better support for running non-blocking code // in the event loop context.vertx().executeBlocking(promise -> { @@ -102,8 +95,4 @@ public class QuarkusRequestFilter extends AbstractRequestFilter implements Handl } }; } - - public void setEnabledEndpoints(Predicate disabledEndpoints) { - this.enabledEndpoints = disabledEndpoints; - } } diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/KeycloakRecorder.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/KeycloakRecorder.java index 211aad1272..9d59b11501 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/KeycloakRecorder.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/KeycloakRecorder.java @@ -26,8 +26,6 @@ import java.util.function.Predicate; import java.util.stream.StreamSupport; import io.smallrye.config.ConfigValue; -import io.vertx.core.Handler; -import io.vertx.ext.web.RoutingContext; import org.jboss.logging.Logger; import org.keycloak.QuarkusKeycloakSessionFactory; import org.keycloak.cli.ShowConfigCommand; @@ -45,7 +43,6 @@ import org.keycloak.provider.Spi; import io.quarkus.runtime.annotations.Recorder; import liquibase.logging.LogFactory; import liquibase.servicelocator.ServiceLocator; -import org.keycloak.provider.quarkus.QuarkusRequestFilter; import org.keycloak.util.Environment; @Recorder @@ -213,23 +210,4 @@ public class KeycloakRecorder { } }); } - - public Handler createFilter(boolean metricsEnabled) { - QuarkusRequestFilter handler = new QuarkusRequestFilter(); - - handler.setEnabledEndpoints(new Predicate() { - @Override - public boolean test(RoutingContext context) { - - if (context.request().uri().startsWith("/metrics") || - context.request().uri().startsWith("/health")) { - return metricsEnabled; - } - - return true; - } - }); - - return handler; - } } diff --git a/quarkus/runtime/src/main/java/org/keycloak/services/NotFoundHandler.java b/quarkus/runtime/src/main/java/org/keycloak/services/NotFoundHandler.java new file mode 100644 index 0000000000..8ff3b5cf9e --- /dev/null +++ b/quarkus/runtime/src/main/java/org/keycloak/services/NotFoundHandler.java @@ -0,0 +1,29 @@ +/* + * Copyright 2020 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.services; + +import io.vertx.core.Handler; +import io.vertx.ext.web.RoutingContext; + +public class NotFoundHandler implements Handler { + + @Override + public void handle(RoutingContext event) { + event.fail(404); + } +} diff --git a/quarkus/runtime/src/main/java/org/keycloak/services/health/KeycloakMetricsHandler.java b/quarkus/runtime/src/main/java/org/keycloak/services/health/KeycloakMetricsHandler.java new file mode 100644 index 0000000000..4c8a193c72 --- /dev/null +++ b/quarkus/runtime/src/main/java/org/keycloak/services/health/KeycloakMetricsHandler.java @@ -0,0 +1,29 @@ +/* + * Copyright 2020 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.services.health; + +import io.quarkus.smallrye.metrics.runtime.SmallRyeMetricsHandler; + +public class KeycloakMetricsHandler extends SmallRyeMetricsHandler { + + public static final String DEFAULT_METRICS_ENDPOINT = "/metrics"; + + public KeycloakMetricsHandler() { + setMetricsPath(DEFAULT_METRICS_ENDPOINT); + } +} diff --git a/quarkus/server/src/main/resources/application.properties b/quarkus/server/src/main/resources/application.properties index 0e59493fae..62475b15dc 100644 --- a/quarkus/server/src/main/resources/application.properties +++ b/quarkus/server/src/main/resources/application.properties @@ -9,6 +9,9 @@ quarkus.http.root-path=/ quarkus.application.name=Keycloak quarkus.banner.enabled=false -# Disable the default data source health check by Agroal extension, since we provide our own (default is true) -quarkus.datasource.health.enabled=false +# Disable health checks from extensions, since we provide our own (default is true) +quarkus.health.extensions.enabled=false + +# Default transaction timeout +quarkus.transaction-manager.default-transaction-timeout=300 diff --git a/services/src/main/java/org/keycloak/transaction/JtaTransactionWrapper.java b/services/src/main/java/org/keycloak/transaction/JtaTransactionWrapper.java index 9b3eeae298..e3382b27f5 100644 --- a/services/src/main/java/org/keycloak/transaction/JtaTransactionWrapper.java +++ b/services/src/main/java/org/keycloak/transaction/JtaTransactionWrapper.java @@ -88,9 +88,6 @@ public class JtaTransactionWrapper implements KeycloakTransaction { @Override public void commit() { try { - if (Status.STATUS_NO_TRANSACTION == tm.getStatus()) { - return; - } logger.debug("JtaTransactionWrapper commit"); tm.commit(); } catch (Exception e) { @@ -103,9 +100,6 @@ public class JtaTransactionWrapper implements KeycloakTransaction { @Override public void rollback() { try { - if (Status.STATUS_NO_TRANSACTION == tm.getStatus()) { - return; - } logger.debug("JtaTransactionWrapper rollback"); tm.rollback(); } catch (Exception e) {