From 3a149e9b62eeece4bb4b67c710add6be11047486 Mon Sep 17 00:00:00 2001 From: Andrew Pielage Date: Fri, 17 Aug 2018 11:27:57 +0100 Subject: [PATCH 01/15] PAYARA-2721 Add in support for getting exception from execution context, and update circuit breaker behaviour --- .../interceptors/AsynchronousInterceptor.java | 4 +- .../interceptors/BulkheadInterceptor.java | 2 +- .../CircuitBreakerInterceptor.java | 48 ++++++++++++------- .../interceptors/RetryInterceptor.java | 2 +- .../interceptors/TimeoutInterceptor.java | 16 ++++--- .../interceptors/fallback/FallbackPolicy.java | 13 +++-- appserver/pom.xml | 2 +- 7 files changed, 54 insertions(+), 33 deletions(-) diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/AsynchronousInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/AsynchronousInterceptor.java index a7695e0f60c..24dd3f7175a 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/AsynchronousInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/AsynchronousInterceptor.java @@ -123,7 +123,7 @@ public Object intercept(InvocationContext invocationContext) throws Exception { if (fallback != null) { logger.log(Level.FINE, "Fallback annotation found on method - falling back from Asynchronous"); FallbackPolicy fallbackPolicy = new FallbackPolicy(fallback, config, invocationContext); - proceededInvocationContext = fallbackPolicy.fallback(invocationContext); + proceededInvocationContext = fallbackPolicy.fallback(invocationContext, ex); } else { throw ex; } @@ -195,7 +195,7 @@ public Object get(long timeout, TimeUnit unit) throws InterruptedException, Exec } } catch (InterruptedException | ExecutionException | TimeoutException ex) { if (ex.getCause() instanceof FaultToleranceException) { - throw (FaultToleranceException) ex.getCause(); + throw new ExecutionException(ex.getCause()); } else { throw ex; } diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java index 9a09c3d3a09..6f85c4aeb35 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java @@ -126,7 +126,7 @@ public Object intercept(InvocationContext invocationContext) throws Exception { logger.log(Level.FINE, "Fallback annotation found on method, and no Retry annotation - " + "falling back from Bulkhead"); FallbackPolicy fallbackPolicy = new FallbackPolicy(fallback, config, invocationContext); - proceededInvocationContext = fallbackPolicy.fallback(invocationContext); + proceededInvocationContext = fallbackPolicy.fallback(invocationContext, ex); } else { logger.log(Level.FINE, "Fallback annotation not found on method, propagating error upwards.", ex); throw ex; diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java index 5af194624e3..62b6f050128 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java @@ -125,7 +125,7 @@ public Object intercept(InvocationContext invocationContext) throws Exception { logger.log(Level.FINE, "Fallback annotation found on method, and no Retry annotation - " + "falling back from CircuitBreaker"); FallbackPolicy fallbackPolicy = new FallbackPolicy(fallback, config, invocationContext); - proceededInvocationContext = fallbackPolicy.fallback(invocationContext); + proceededInvocationContext = fallbackPolicy.fallback(invocationContext, ex); } else { throw ex; } @@ -207,7 +207,7 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio // If closed, attempt to proceed the invocation context try { logger.log(Level.FINER, "Proceeding CircuitBreaker context"); - proceededInvocationContext = invocationContext.proceed(); + proceededInvocationContext = invocationContext.proceed(); } catch (Exception ex) { logger.log(Level.FINE, "Exception executing CircuitBreaker context"); @@ -216,29 +216,27 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio logger.log(Level.FINE, "Caught exception is included in CircuitBreaker failOn, " + "recording failure against CircuitBreaker"); // Add a failure result to the queue - circuitBreakerState.recordClosedResult(Boolean.FALSE); - - // Calculate the failure threshold - long failureThreshold = Math.round(requestVolumeThreshold * failureRatio); - - // If we're over the failure threshold, open the circuit - if (circuitBreakerState.isOverFailureThreshold(failureThreshold)) { - logger.log(Level.FINE, "CircuitBreaker is over failure threshold {0}, opening circuit", - failureThreshold); - - circuitBreakerState.setCircuitState(CircuitBreakerState.CircuitState.OPEN); - - // Kick off a thread that will half-open the circuit after the specified delay - scheduleHalfOpen(delayMillis, circuitBreakerState); - } + circuitBreakerState.recordClosedResult(Boolean.FALSE); + + // Calculate the failure ratio, and if we're over it, open the circuit + breakCircuitIfRequired( + Math.round(requestVolumeThreshold * failureRatio), + circuitBreakerState, + delayMillis); } // Finally, propagate the error upwards throw ex; } - // If everything is bon, just add a success value + // If everything is bon, add a success value circuitBreakerState.recordClosedResult(Boolean.TRUE); + + // Calculate the failure ratio, and if we're over it, open the circuit + breakCircuitIfRequired( + Math.round(requestVolumeThreshold * failureRatio), + circuitBreakerState, + delayMillis); break; case HALF_OPEN: // If half-open, attempt to proceed the invocation context @@ -344,4 +342,18 @@ private boolean shouldFail(Class[] failOn, Exception ex) { return shouldFail; } + + private void breakCircuitIfRequired(long failureThreshold, CircuitBreakerState circuitBreakerState, + long delayMillis) throws NamingException { + // If we're over the failure threshold, open the circuit + if (circuitBreakerState.isOverFailureThreshold(failureThreshold)) { + logger.log(Level.FINE, "CircuitBreaker is over failure threshold {0}, opening circuit", + failureThreshold); + + circuitBreakerState.setCircuitState(CircuitBreakerState.CircuitState.OPEN); + + // Kick off a thread that will half-open the circuit after the specified delay + scheduleHalfOpen(delayMillis, circuitBreakerState); + } + } } diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java index 48436a74ee6..b07c936a6a9 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java @@ -112,7 +112,7 @@ public Object intercept(InvocationContext invocationContext) throws Exception { if (fallback != null) { logger.log(Level.FINE, "Fallback annotation found on method - falling back from Retry"); FallbackPolicy fallbackPolicy = new FallbackPolicy(fallback, config, invocationContext); - proceededInvocationContext = fallbackPolicy.fallback(invocationContext); + proceededInvocationContext = fallbackPolicy.fallback(invocationContext, ex); } else { throw ex; } diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java index e7b73438ac8..ac2a5bb85ca 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java @@ -121,7 +121,7 @@ public Object intercept(InvocationContext invocationContext) throws Exception { logger.log(Level.FINE, "Fallback annotation found on method, and no Retry annotation - " + "falling back from Timeout"); FallbackPolicy fallbackPolicy = new FallbackPolicy(fallback, config, invocationContext); - proceededInvocationContext = fallbackPolicy.fallback(invocationContext); + proceededInvocationContext = fallbackPolicy.fallback(invocationContext, ex); } else { throw ex; } @@ -156,20 +156,23 @@ private Object timeout(InvocationContext invocationContext) throws Exception { .orElse(timeout.unit()); Future timeoutFuture = null; - ThreadLocal timedOut = new ThreadLocal<>(); - timedOut.set(false); long timeoutMillis = Duration.of(value, unit).toMillis(); long timeoutTime = System.currentTimeMillis() + timeoutMillis; try { - timeoutFuture = startTimeout(timeoutMillis, timedOut); + timeoutFuture = startTimeout(timeoutMillis); proceededInvocationContext = invocationContext.proceed(); stopTimeout(timeoutFuture); - if (System.currentTimeMillis() > timeoutTime || timedOut.get()) { + if (System.currentTimeMillis() > timeoutTime) { logger.log(Level.FINE, "Execution timed out"); throw new TimeoutException(); } + } catch (InterruptedException ie) { + if (System.currentTimeMillis() > timeoutTime) { + logger.log(Level.FINE, "Execution timed out"); + throw new TimeoutException(ie); + } } catch (Exception ex) { stopTimeout(timeoutFuture); throw ex; @@ -185,12 +188,11 @@ private Object timeout(InvocationContext invocationContext) throws Exception { * @return A future that can be cancelled if the method execution completes before the interrupt happens * @throws NamingException If the configured ManagedScheduledExecutorService could not be found */ - private Future startTimeout(long timeoutMillis, ThreadLocal timedOut) throws NamingException { + private Future startTimeout(long timeoutMillis) throws NamingException { final Thread thread = Thread.currentThread(); Runnable timeoutTask = () -> { thread.interrupt(); - timedOut.set(true); }; FaultToleranceService faultToleranceService = Globals.getDefaultBaseServiceLocator() diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/fallback/FallbackPolicy.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/fallback/FallbackPolicy.java index e7f503fbce6..503015edf75 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/fallback/FallbackPolicy.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/fallback/FallbackPolicy.java @@ -85,7 +85,7 @@ public FallbackPolicy(Fallback fallback, Config config, InvocationContext invoca * @return The result of the executed fallback method * @throws Exception If the fallback method itself fails. */ - public Object fallback(InvocationContext invocationContext) throws Exception { + public Object fallback(InvocationContext invocationContext, Throwable exception) throws Exception { Object fallbackInvocationContext = null; FaultToleranceService faultToleranceService = @@ -106,7 +106,7 @@ public Object fallback(InvocationContext invocationContext) throws Exception { logger.log(Level.FINE, "Using fallback class: {0}", fallbackClass.getName()); ExecutionContext executionContext = new FaultToleranceExecutionContext(invocationContext.getMethod(), - invocationContext.getParameters()); + invocationContext.getParameters(), exception); fallbackInvocationContext = fallbackClass .getDeclaredMethod(FALLBACK_HANDLER_METHOD_NAME, ExecutionContext.class) @@ -126,10 +126,12 @@ private class FaultToleranceExecutionContext implements ExecutionContext { private final Method method; private final Object[] parameters; + private final Throwable failure; - public FaultToleranceExecutionContext(Method method, Object[] parameters) { + public FaultToleranceExecutionContext(Method method, Object[] parameters, Throwable failure) { this.method = method; this.parameters = parameters; + this.failure = failure; } @Override @@ -141,5 +143,10 @@ public Method getMethod() { public Object[] getParameters() { return parameters; } + + @Override + public Throwable getFailure() { + return failure; + } } } diff --git a/appserver/pom.xml b/appserver/pom.xml index d96c5014a97..c8acd6a5ecb 100644 --- a/appserver/pom.xml +++ b/appserver/pom.xml @@ -209,7 +209,7 @@ 1.0.0-Beta - 1.0.payara-p1 + 1.1.1 1.1.payara-p1 1.0.payara-p1 1.1.payara-p1 From 38aa9dbf354579e8bf269192b7b46c05d7ceb99f Mon Sep 17 00:00:00 2001 From: Andrew Pielage Date: Fri, 17 Aug 2018 15:48:39 +0100 Subject: [PATCH 02/15] PAYARA-2721 Fix IllegalArgumentException caused by ChronoUnit convertor removal in MP Config 1.3, and fix Global Config override --- .../cdi/FaultToleranceCdiUtils.java | 37 +++++++------------ .../CircuitBreakerInterceptor.java | 3 +- .../interceptors/RetryInterceptor.java | 9 +++-- .../interceptors/TimeoutInterceptor.java | 3 +- 4 files changed, 23 insertions(+), 29 deletions(-) diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java index c2c5f01a297..f279036c479 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java @@ -178,36 +178,25 @@ public static Optional getOverrideValue(Config config, Cl logger.log(Level.FINER, "Getting config override for annotated method..."); value = config.getOptionalValue(annotatedClassCanonicalName + "/" + annotatedMethodName + "/" + annotationName + "/" + parameterName, parameterType); - - // If there wasn't a config override for the method, check if the method has the annotation + + // If there wasn't a config override for the method, check if there's one for the class if (!value.isPresent()) { - logger.log(Level.FINER, "No config override for annotated method, checking if the method is " - + "annotated directly..."); - // If the method is annotated directly, simply return - if (invocationContext.getMethod().getAnnotation(annotationClass) != null) { - logger.log(Level.FINER, "Method is annotated directly, returning."); - return value; - } - - // If there wasn't a config override for the method, check if there's one for the class + logger.log(Level.FINER, "No config override for annotated method, getting config override for the " + + "annotated class..."); + value = config.getOptionalValue(annotatedClassCanonicalName + "/" + annotationName + + "/" + parameterName, parameterType); + + // If there wasn't a config override for the class either, check if there's a global one if (!value.isPresent()) { - logger.log(Level.FINER, "No config override for annotated method, getting config override for the " - + "annotated class..."); - value = config.getOptionalValue(annotatedClassCanonicalName + "/" + annotationName - + "/" + parameterName, parameterType); + logger.log(Level.FINER, "No config override for the annotated class, getting application wide " + + "config override..."); + value = config.getOptionalValue(annotationName + "/" + parameterName, parameterType); - // If there wasn't a config override for the class either, check if there's a global one if (!value.isPresent()) { - logger.log(Level.FINER, "No config override for the annotated class, getting application wide " - + "config override..."); - value = config.getOptionalValue(annotationName + "/" + parameterName, parameterType); - - if (!value.isPresent()) { - logger.log(Level.FINER, "No config overrides"); - } + logger.log(Level.FINER, "No config overrides"); } } - } + } } else { logger.log(Level.FINE, "No config to get override parameters from."); } diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java index 62b6f050128..e124b0f6410 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java @@ -175,8 +175,9 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio long delay = (Long) FaultToleranceCdiUtils.getOverrideValue( config, CircuitBreaker.class, "value", invocationContext, Long.class) .orElse(circuitBreaker.delay()); + // Look for a String and cast to ChronoUnit - Use the Common Sense Convertor ChronoUnit delayUnit = (ChronoUnit) FaultToleranceCdiUtils.getOverrideValue( - config, CircuitBreaker.class, "delayUnit", invocationContext, ChronoUnit.class) + config, CircuitBreaker.class, "delayUnit", invocationContext, String.class) .orElse(circuitBreaker.delayUnit()); int requestVolumeThreshold = (Integer) FaultToleranceCdiUtils.getOverrideValue( config, CircuitBreaker.class, "requestVolumeThreshold", invocationContext, Integer.class) diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java index b07c936a6a9..3c70cdad58a 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java @@ -200,20 +200,23 @@ private Object retry(InvocationContext invocationContext) throws Exception { long delay = (Long) FaultToleranceCdiUtils.getOverrideValue( config, Retry.class, "delay", invocationContext, Long.class) .orElse(retry.delay()); + // Look for a String and cast to ChronoUnit - Use the Common Sense Convertor ChronoUnit delayUnit = (ChronoUnit) FaultToleranceCdiUtils.getOverrideValue( - config, Retry.class, "delayUnit", invocationContext, ChronoUnit.class) + config, Retry.class, "delayUnit", invocationContext, String.class) .orElse(retry.delayUnit()); long maxDuration = (Long) FaultToleranceCdiUtils.getOverrideValue( config, Retry.class, "maxDuration", invocationContext, Long.class) .orElse(retry.maxDuration()); + // Look for a String and cast to ChronoUnit - Use the Common Sense Convertor ChronoUnit durationUnit = (ChronoUnit) FaultToleranceCdiUtils.getOverrideValue( - config, Retry.class, "durationUnit", invocationContext, ChronoUnit.class) + config, Retry.class, "durationUnit", invocationContext, String.class) .orElse(retry.durationUnit()); long jitter = (Long) FaultToleranceCdiUtils.getOverrideValue( config, Retry.class, "jitter", invocationContext, Long.class) .orElse(retry.jitter()); + // Look for a String and cast to ChronoUnit - Use the Common Sense Convertor ChronoUnit jitterDelayUnit = (ChronoUnit) FaultToleranceCdiUtils.getOverrideValue( - config, Retry.class, "jitterDelayUnit", invocationContext, ChronoUnit.class) + config, Retry.class, "jitterDelayUnit", invocationContext, String.class) .orElse(retry.jitterDelayUnit()); long delayMillis = Duration.of(delay, delayUnit).toMillis(); diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java index ac2a5bb85ca..0dfce7db590 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java @@ -151,8 +151,9 @@ private Object timeout(InvocationContext invocationContext) throws Exception { long value = (Long) FaultToleranceCdiUtils.getOverrideValue( config, Timeout.class, "value", invocationContext, Long.class) .orElse(timeout.value()); + // Look for a String and cast to ChronoUnit - Use the Common Sense Convertor ChronoUnit unit = (ChronoUnit) FaultToleranceCdiUtils.getOverrideValue( - config, Timeout.class, "unit", invocationContext, ChronoUnit.class) + config, Timeout.class, "unit", invocationContext, String.class) .orElse(timeout.unit()); Future timeoutFuture = null; From 33319e2b507b51cfd10d24a6d3b47bf4dbabfede Mon Sep 17 00:00:00 2001 From: Andrew Pielage Date: Fri, 17 Aug 2018 17:06:30 +0100 Subject: [PATCH 03/15] PAYARA-2721 Correct config override priorities --- .../cdi/FaultToleranceCdiUtils.java | 39 ++++++++++++------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java index f279036c479..741cbff5b99 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java @@ -176,24 +176,35 @@ public static Optional getOverrideValue(Config config, Cl // Check if there's a config override for the method if (config != null) { logger.log(Level.FINER, "Getting config override for annotated method..."); - value = config.getOptionalValue(annotatedClassCanonicalName + "/" + annotatedMethodName - + "/" + annotationName + "/" + parameterName, parameterType); - - // If there wasn't a config override for the method, check if there's one for the class + + // Check if there's a config override for this specific method + value = config.getOptionalValue(annotatedClassCanonicalName + "/" + + annotatedMethodName + "/" + annotationName + "/" + parameterName, parameterType); if (!value.isPresent()) { - logger.log(Level.FINER, "No config override for annotated method, getting config override for the " - + "annotated class..."); - value = config.getOptionalValue(annotatedClassCanonicalName + "/" + annotationName - + "/" + parameterName, parameterType); - - // If there wasn't a config override for the class either, check if there's a global one - if (!value.isPresent()) { - logger.log(Level.FINER, "No config override for the annotated class, getting application wide " - + "config override..."); + logger.log(Level.FINER, "No config override for annotated method, checking if the method is " + + "annotated directly..."); + // If the method is annotated directly, check for a global override and return + if (invocationContext.getMethod().getAnnotation(annotationClass) != null) { + logger.log(Level.FINER, "Method is annotated directly, checking for global override."); value = config.getOptionalValue(annotationName + "/" + parameterName, parameterType); + + if (value.isPresent()) { + logger.log(Level.FINER, "Global override found."); + } else { + logger.log(Level.FINER, "No config overrides."); + } + return value; + } + + // If there wasn't a config override for the method, check if there's one for the class + if (!value.isPresent()) { + logger.log(Level.FINER, "No config override for annotated method, getting config override for the " + + "annotated class..."); + value = config.getOptionalValue(annotatedClassCanonicalName + "/" + annotationName + + "/" + parameterName, parameterType); if (!value.isPresent()) { - logger.log(Level.FINER, "No config overrides"); + logger.log(Level.FINER, "No config overrides."); } } } From 21d6f540d2f70ddd2fc68fdbcbadf3c9d81932b9 Mon Sep 17 00:00:00 2001 From: Andrew Pielage Date: Fri, 17 Aug 2018 17:47:50 +0100 Subject: [PATCH 04/15] PAYARA-2721 Further config priority fixes, and fix config classloader lookup for non-managed threads --- .../faulttolerance/cdi/FaultToleranceCdiUtils.java | 9 ++++++++- .../config/spi/ConfigProviderResolverImpl.java | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java index 741cbff5b99..472a920f811 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java @@ -204,7 +204,14 @@ public static Optional getOverrideValue(Config config, Cl + "/" + parameterName, parameterType); if (!value.isPresent()) { - logger.log(Level.FINER, "No config overrides."); + logger.log(Level.FINER, "No config override for annotated class, checking for global override."); + value = config.getOptionalValue(annotationName + "/" + parameterName, parameterType); + + if (value.isPresent()) { + logger.log(Level.FINER, "Global override found."); + } else { + logger.log(Level.FINER, "No config overrides."); + } } } } diff --git a/nucleus/payara-modules/nucleus-microprofile/config-service/src/main/java/fish/payara/nucleus/microprofile/config/spi/ConfigProviderResolverImpl.java b/nucleus/payara-modules/nucleus-microprofile/config-service/src/main/java/fish/payara/nucleus/microprofile/config/spi/ConfigProviderResolverImpl.java index 9403fe3570a..c19ec7f58ac 100644 --- a/nucleus/payara-modules/nucleus-microprofile/config-service/src/main/java/fish/payara/nucleus/microprofile/config/spi/ConfigProviderResolverImpl.java +++ b/nucleus/payara-modules/nucleus-microprofile/config-service/src/main/java/fish/payara/nucleus/microprofile/config/spi/ConfigProviderResolverImpl.java @@ -199,7 +199,7 @@ ApplicationInfo getAppInfo(ClassLoader loader) { // fast check fails search the app registry for (String name : applicationRegistry.getAllApplicationNames()) { ApplicationInfo testInfo = applicationRegistry.get(name); - if (testInfo.getClassLoaders().contains(loader)) { + if (testInfo.getClassLoaders().contains(loader) || testInfo.getAppClassLoader().equals(loader)) { return testInfo; } } From 744614a1ce279f39874911859a15a40e687ca1c0 Mon Sep 17 00:00:00 2001 From: Andrew Pielage Date: Tue, 21 Aug 2018 09:59:16 +0100 Subject: [PATCH 05/15] PAYARA-2721 Improve application name registration --- .../faulttolerance/FaultToleranceService.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/FaultToleranceService.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/FaultToleranceService.java index bcf2d29e65d..53778fe36c5 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/FaultToleranceService.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/FaultToleranceService.java @@ -67,6 +67,7 @@ import org.glassfish.hk2.api.ServiceLocator; import org.glassfish.hk2.runlevel.RunLevel; import org.glassfish.internal.data.ApplicationInfo; +import org.glassfish.internal.data.ApplicationRegistry; import org.glassfish.internal.deployment.Deployment; import org.jvnet.hk2.annotations.Optional; import org.jvnet.hk2.annotations.Service; @@ -488,6 +489,20 @@ public String getApplicationName(InvocationManager invocationManager, Invocation if (appName == null) { appName = invocationManager.getCurrentInvocation().getComponentId(); + // If we've found a component name, check if there's an application registered with the same name + if (appName != null) { + ApplicationRegistry applicationRegistry = habitat.getService(ApplicationRegistry.class); + + // If it's not directly in the registry, it's possible due to how the componentId is constructed + if (applicationRegistry.get(appName) == null) { + String[] componentIds = appName.split("_/"); + + // The application name should be the first component + appName = componentIds[0]; + } + } + + // If we still don't have a name - just construct it from the method signature if (appName == null) { appName = getFullMethodSignature(invocationContext.getMethod()); } From 16e316dfc93464adb698fc25706adcd588d762e9 Mon Sep 17 00:00:00 2001 From: Andrew Pielage Date: Tue, 21 Aug 2018 10:32:07 +0100 Subject: [PATCH 06/15] PAYARA-2721 Fix config lookup of CircuitBreaker delay --- .../faulttolerance/interceptors/CircuitBreakerInterceptor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java index e124b0f6410..32f0d18c11b 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java @@ -173,7 +173,7 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio } long delay = (Long) FaultToleranceCdiUtils.getOverrideValue( - config, CircuitBreaker.class, "value", invocationContext, Long.class) + config, CircuitBreaker.class, "delay", invocationContext, Long.class) .orElse(circuitBreaker.delay()); // Look for a String and cast to ChronoUnit - Use the Common Sense Convertor ChronoUnit delayUnit = (ChronoUnit) FaultToleranceCdiUtils.getOverrideValue( From 3963aa164682cc87eff94c8cafd10427581235bc Mon Sep 17 00:00:00 2001 From: Andrew Pielage Date: Tue, 21 Aug 2018 12:58:57 +0100 Subject: [PATCH 07/15] PAYARA-2721 Add in class-level config override, and allow disabling individual annotations --- .../cdi/FaultToleranceCdiUtils.java | 19 ++++++++++++++----- .../interceptors/AsynchronousInterceptor.java | 12 +++++++++--- .../interceptors/BulkheadInterceptor.java | 11 ++++++++--- .../CircuitBreakerInterceptor.java | 16 ++++++++++++---- .../interceptors/RetryInterceptor.java | 13 ++++++++++--- .../interceptors/TimeoutInterceptor.java | 13 ++++++++++--- 6 files changed, 63 insertions(+), 21 deletions(-) diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java index 472a920f811..ec5a2dcbc8d 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java @@ -183,16 +183,25 @@ public static Optional getOverrideValue(Config config, Cl if (!value.isPresent()) { logger.log(Level.FINER, "No config override for annotated method, checking if the method is " + "annotated directly..."); - // If the method is annotated directly, check for a global override and return + // If the method is annotated directly, check for a class or global level override and return if (invocationContext.getMethod().getAnnotation(annotationClass) != null) { - logger.log(Level.FINER, "Method is annotated directly, checking for global override."); - value = config.getOptionalValue(annotationName + "/" + parameterName, parameterType); + logger.log(Level.FINER, "Method is annotated directly, checking for class-level override."); + value = config.getOptionalValue(annotatedClassCanonicalName + "/" + annotationName + + "/" + parameterName, parameterType); if (value.isPresent()) { - logger.log(Level.FINER, "Global override found."); + logger.log(Level.FINER, "Class-level override found."); } else { - logger.log(Level.FINER, "No config overrides."); + logger.log(Level.FINER, "No class-level override found, checking for global override."); + value = config.getOptionalValue(annotationName + "/" + parameterName, parameterType); + + if (value.isPresent()) { + logger.log(Level.FINER, "Global override found."); + } else { + logger.log(Level.FINER, "No config overrides."); + } } + return value; } diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/AsynchronousInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/AsynchronousInterceptor.java index 24dd3f7175a..328d11808e4 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/AsynchronousInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/AsynchronousInterceptor.java @@ -100,9 +100,13 @@ public Object intercept(InvocationContext invocationContext) throws Exception { } try { + String appName = faultToleranceService.getApplicationName(invocationManager, invocationContext); + // Attempt to proceed the InvocationContext with Asynchronous semantics if Fault Tolerance is enabled - if (faultToleranceService.isFaultToleranceEnabled(faultToleranceService.getApplicationName( - invocationManager, invocationContext), config)) { + if (faultToleranceService.isFaultToleranceEnabled(appName, config) + && ((Boolean) FaultToleranceCdiUtils.getOverrideValue( + config, Asynchronous.class, "enabled", invocationContext, Boolean.class) + .orElse(Boolean.TRUE))) { Callable callable = () -> { return invocationContext.proceed(); }; @@ -120,7 +124,9 @@ public Object intercept(InvocationContext invocationContext) throws Exception { Fallback fallback = FaultToleranceCdiUtils.getAnnotation(beanManager, Fallback.class, invocationContext); // If the method was annotated with Fallback, attempt it, otherwise just propagate the exception upwards - if (fallback != null) { + if (fallback != null && ((Boolean) FaultToleranceCdiUtils.getOverrideValue( + config, Fallback.class, "enabled", invocationContext, Boolean.class) + .orElse(Boolean.TRUE))) { logger.log(Level.FINE, "Fallback annotation found on method - falling back from Asynchronous"); FallbackPolicy fallbackPolicy = new FallbackPolicy(fallback, config, invocationContext); proceededInvocationContext = fallbackPolicy.fallback(invocationContext, ex); diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java index 6f85c4aeb35..6c75eaa590d 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java @@ -100,8 +100,11 @@ public Object intercept(InvocationContext invocationContext) throws Exception { try { String appName = faultToleranceService.getApplicationName(invocationManager, invocationContext); - // Attempt to proceed the InvocationContext with Bulkhead semantics if Fault Tolerance is enabled - if (faultToleranceService.isFaultToleranceEnabled(appName, config)) { + // Attempt to proceed the InvocationContext with Asynchronous semantics if Fault Tolerance is enabled + if (faultToleranceService.isFaultToleranceEnabled(appName, config) + && ((Boolean) FaultToleranceCdiUtils.getOverrideValue( + config, Bulkhead.class, "enabled", invocationContext, Boolean.class) + .orElse(Boolean.TRUE))) { logger.log(Level.FINER, "Proceeding invocation with bulkhead semantics"); proceededInvocationContext = bulkhead(invocationContext); } else { @@ -122,7 +125,9 @@ public Object intercept(InvocationContext invocationContext) throws Exception { invocationContext); // If the method was annotated with Fallback, attempt it, otherwise just propagate the exception upwards - if (fallback != null) { + if (fallback != null && ((Boolean) FaultToleranceCdiUtils.getOverrideValue( + config, Fallback.class, "enabled", invocationContext, Boolean.class) + .orElse(Boolean.TRUE))) { logger.log(Level.FINE, "Fallback annotation found on method, and no Retry annotation - " + "falling back from Bulkhead"); FallbackPolicy fallbackPolicy = new FallbackPolicy(fallback, config, invocationContext); diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java index 32f0d18c11b..19d8e8dc905 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java @@ -99,10 +99,16 @@ public Object intercept(InvocationContext invocationContext) throws Exception { logger.log(Level.INFO, "No config could be found", ex); } - // Attempt to proceed the invocation with CircuitBreaker semantics if Fault Tolerance is enabled + // Attempt to proceed the invocation with CircuitBreaker semantics if Fault Tolerance is enabled, and if + // this particular method hasn't been excluded try { - if (faultToleranceService.isFaultToleranceEnabled(faultToleranceService.getApplicationName( - invocationManager, invocationContext), config)) { + String appName = faultToleranceService.getApplicationName(invocationManager, invocationContext); + + // Attempt to proceed the InvocationContext with Asynchronous semantics if Fault Tolerance is enabled + if (faultToleranceService.isFaultToleranceEnabled(appName, config) + && ((Boolean) FaultToleranceCdiUtils.getOverrideValue( + config, CircuitBreaker.class, "enabled", invocationContext, Boolean.class) + .orElse(Boolean.TRUE))) { logger.log(Level.FINER, "Proceeding invocation with circuitbreaker semantics"); proceededInvocationContext = circuitBreak(invocationContext); } else { @@ -121,7 +127,9 @@ public Object intercept(InvocationContext invocationContext) throws Exception { } else { Fallback fallback = FaultToleranceCdiUtils.getAnnotation(beanManager, Fallback.class, invocationContext); - if (fallback != null) { + if (fallback != null && ((Boolean) FaultToleranceCdiUtils.getOverrideValue( + config, Fallback.class, "enabled", invocationContext, Boolean.class) + .orElse(Boolean.TRUE))) { logger.log(Level.FINE, "Fallback annotation found on method, and no Retry annotation - " + "falling back from CircuitBreaker"); FallbackPolicy fallbackPolicy = new FallbackPolicy(fallback, config, invocationContext); diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java index 3c70cdad58a..9682161ed12 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java @@ -96,8 +96,13 @@ public Object intercept(InvocationContext invocationContext) throws Exception { } try { - if (faultToleranceService.isFaultToleranceEnabled(faultToleranceService.getApplicationName( - invocationManager, invocationContext), config)) { + String appName = faultToleranceService.getApplicationName(invocationManager, invocationContext); + + // Attempt to proceed the InvocationContext with Asynchronous semantics if Fault Tolerance is enabled + if (faultToleranceService.isFaultToleranceEnabled(appName, config) + && ((Boolean) FaultToleranceCdiUtils.getOverrideValue( + config, Retry.class, "enabled", invocationContext, Boolean.class) + .orElse(Boolean.TRUE))) { logger.log(Level.FINER, "Proceeding invocation with retry semantics"); proceededInvocationContext = retry(invocationContext); } else { @@ -109,7 +114,9 @@ public Object intercept(InvocationContext invocationContext) throws Exception { } catch (Exception ex) { Fallback fallback = FaultToleranceCdiUtils.getAnnotation(beanManager, Fallback.class, invocationContext); - if (fallback != null) { + if (fallback != null && ((Boolean) FaultToleranceCdiUtils.getOverrideValue( + config, Fallback.class, "enabled", invocationContext, Boolean.class) + .orElse(Boolean.TRUE))) { logger.log(Level.FINE, "Fallback annotation found on method - falling back from Retry"); FallbackPolicy fallbackPolicy = new FallbackPolicy(fallback, config, invocationContext); proceededInvocationContext = fallbackPolicy.fallback(invocationContext, ex); diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java index 0dfce7db590..68cc83894f8 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java @@ -97,8 +97,13 @@ public Object intercept(InvocationContext invocationContext) throws Exception { } try { - if (faultToleranceService.isFaultToleranceEnabled(faultToleranceService.getApplicationName( - invocationManager, invocationContext), config)) { + String appName = faultToleranceService.getApplicationName(invocationManager, invocationContext); + + // Attempt to proceed the InvocationContext with Asynchronous semantics if Fault Tolerance is enabled + if (faultToleranceService.isFaultToleranceEnabled(appName, config) + && ((Boolean) FaultToleranceCdiUtils.getOverrideValue( + config, Timeout.class, "enabled", invocationContext, Boolean.class) + .orElse(Boolean.TRUE))) { logger.log(Level.FINER, "Proceeding invocation with timeout semantics"); proceededInvocationContext = timeout(invocationContext); } else { @@ -117,7 +122,9 @@ public Object intercept(InvocationContext invocationContext) throws Exception { Fallback fallback = FaultToleranceCdiUtils.getAnnotation(beanManager, Fallback.class, invocationContext); - if (fallback != null) { + if (fallback != null && ((Boolean) FaultToleranceCdiUtils.getOverrideValue( + config, Fallback.class, "enabled", invocationContext, Boolean.class) + .orElse(Boolean.TRUE))) { logger.log(Level.FINE, "Fallback annotation found on method, and no Retry annotation - " + "falling back from Timeout"); FallbackPolicy fallbackPolicy = new FallbackPolicy(fallback, config, invocationContext); From 390d15af58fefe7346fa580a6110b72ce4a1e960 Mon Sep 17 00:00:00 2001 From: Andrew Pielage Date: Tue, 21 Aug 2018 15:31:35 +0100 Subject: [PATCH 08/15] Refactor for readability --- .../cdi/FaultToleranceCdiUtils.java | 82 +++++++++++-------- 1 file changed, 50 insertions(+), 32 deletions(-) diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java index ec5a2dcbc8d..e072add1199 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java @@ -173,54 +173,31 @@ public static Optional getOverrideValue(Config config, Cl String annotatedMethodName = invocationContext.getMethod().getName(); String annotatedClassCanonicalName = invocationContext.getMethod().getDeclaringClass().getCanonicalName(); - // Check if there's a config override for the method + // Check if there's a config override if (config != null) { logger.log(Level.FINER, "Getting config override for annotated method..."); // Check if there's a config override for this specific method value = config.getOptionalValue(annotatedClassCanonicalName + "/" + annotatedMethodName + "/" + annotationName + "/" + parameterName, parameterType); + if (!value.isPresent()) { logger.log(Level.FINER, "No config override for annotated method, checking if the method is " + "annotated directly..."); // If the method is annotated directly, check for a class or global level override and return if (invocationContext.getMethod().getAnnotation(annotationClass) != null) { - logger.log(Level.FINER, "Method is annotated directly, checking for class-level override."); - value = config.getOptionalValue(annotatedClassCanonicalName + "/" + annotationName - + "/" + parameterName, parameterType); - - if (value.isPresent()) { - logger.log(Level.FINER, "Class-level override found."); - } else { - logger.log(Level.FINER, "No class-level override found, checking for global override."); - value = config.getOptionalValue(annotationName + "/" + parameterName, parameterType); - - if (value.isPresent()) { - logger.log(Level.FINER, "Global override found."); - } else { - logger.log(Level.FINER, "No config overrides."); - } - } - - return value; + value = checkAnnotatedMethodForOverrides(annotatedClassCanonicalName, annotationName, parameterName, + parameterType, config); } // If there wasn't a config override for the method, check if there's one for the class if (!value.isPresent()) { - logger.log(Level.FINER, "No config override for annotated method, getting config override for the " - + "annotated class..."); - value = config.getOptionalValue(annotatedClassCanonicalName + "/" + annotationName - + "/" + parameterName, parameterType); - + value = checkForClassLevelOverride(annotatedClassCanonicalName, annotationName, parameterName, + parameterType, config); + + // If there wasn't a config override for the class, check if there's a global one if (!value.isPresent()) { - logger.log(Level.FINER, "No config override for annotated class, checking for global override."); - value = config.getOptionalValue(annotationName + "/" + parameterName, parameterType); - - if (value.isPresent()) { - logger.log(Level.FINER, "Global override found."); - } else { - logger.log(Level.FINER, "No config overrides."); - } + value = checkForGlobalLevelOverride(annotationName, parameterName, parameterType, config); } } } @@ -230,4 +207,45 @@ public static Optional getOverrideValue(Config config, Cl return value; } + + private static void logOverride(Optional value, String level) { + if (value.isPresent()) { + logger.log(Level.FINER, "{0} override found.", level); + } else { + logger.log(Level.FINER, "No config overrides."); + } + } + + private static Optional checkAnnotatedMethodForOverrides(String annotatedClassCanonicalName, String annotationName, + String parameterName, Class parameterType, Config config) { + logger.log(Level.FINER, "Method is annotated directly, checking for class-level override."); + Optional value = checkForClassLevelOverride(annotatedClassCanonicalName, annotationName, parameterName, + parameterType, config); + + if (!value.isPresent()) { + value = checkForGlobalLevelOverride(annotationName, parameterName, parameterType, config); + } + + return value; + } + + private static Optional checkForClassLevelOverride(String annotatedClassCanonicalName, String annotationName, + String parameterName, Class parameterType, Config config) { + logger.log(Level.FINER, "No config override for annotated method, getting config override for the " + + "annotated class..."); + Optional value = config.getOptionalValue(annotatedClassCanonicalName + "/" + annotationName + + "/" + parameterName, parameterType); + logOverride(value, "Class-level"); + + return value; + } + + private static Optional checkForGlobalLevelOverride(String annotationName, String parameterName, Class parameterType, + Config config) { + logger.log(Level.FINER, "No config override for annotated class, checking for global override."); + Optional value = config.getOptionalValue(annotationName + "/" + parameterName, parameterType); + logOverride(value, "Global"); + + return value; + } } From 2fe7984fe73819c4e42e84a438f7c04b43b98642 Mon Sep 17 00:00:00 2001 From: Andrew Pielage Date: Wed, 22 Aug 2018 14:55:40 +0100 Subject: [PATCH 09/15] PAYARA-2721 Fix config priorities --- .../cdi/FaultToleranceCdiUtils.java | 100 ++++++++++++++++-- .../interceptors/AsynchronousInterceptor.java | 8 +- .../interceptors/BulkheadInterceptor.java | 8 +- .../CircuitBreakerInterceptor.java | 8 +- .../interceptors/RetryInterceptor.java | 8 +- .../interceptors/TimeoutInterceptor.java | 8 +- .../interceptors/fallback/FallbackPolicy.java | 3 +- 7 files changed, 111 insertions(+), 32 deletions(-) diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java index e072add1199..083e3d0c778 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java @@ -69,7 +69,7 @@ public class FaultToleranceCdiUtils { public static A getAnnotation(BeanManager beanManager, Class annotationClass, InvocationContext invocationContext) { A annotation = null; - Class annotatedClass = invocationContext.getMethod().getDeclaringClass(); + Class annotatedClass = getAnnotatedMethodClass(invocationContext, annotationClass); logger.log(Level.FINER, "Attempting to get annotation {0} from {1}", new String[]{annotationClass.getSimpleName(), invocationContext.getMethod().getName()}); @@ -171,7 +171,8 @@ public static Optional getOverrideValue(Config config, Cl // Get the annotation, method, and class names String annotationName = annotationClass.getSimpleName(); String annotatedMethodName = invocationContext.getMethod().getName(); - String annotatedClassCanonicalName = invocationContext.getMethod().getDeclaringClass().getCanonicalName(); + String annotatedClassCanonicalName = getAnnotatedMethodClassCanonicalName( + getAnnotatedMethodClass(invocationContext, annotationClass)); // Check if there's a config override if (config != null) { @@ -184,13 +185,14 @@ public static Optional getOverrideValue(Config config, Cl if (!value.isPresent()) { logger.log(Level.FINER, "No config override for annotated method, checking if the method is " + "annotated directly..."); - // If the method is annotated directly, check for a class or global level override and return + // If the method is annotated directly, check for a global level override and return if (invocationContext.getMethod().getAnnotation(annotationClass) != null) { - value = checkAnnotatedMethodForOverrides(annotatedClassCanonicalName, annotationName, parameterName, - parameterType, config); + logger.log(Level.FINER, "Method is annotated directly, checking for global override..."); + // The only thing that should override a method-level annotation is a method or global-level config + return checkForGlobalLevelOverride(annotationName, parameterName, parameterType, config); } - // If there wasn't a config override for the method, check if there's one for the class + // If the method wasn't annotated directly, check if there's an override for the class if (!value.isPresent()) { value = checkForClassLevelOverride(annotatedClassCanonicalName, annotationName, parameterName, parameterType, config); @@ -208,6 +210,52 @@ public static Optional getOverrideValue(Config config, Cl return value; } + public static Optional getEnabledOverrideValue(Config config, Class annotationClass, + InvocationContext invocationContext, Class parameterType) { + Optional value = Optional.empty(); + + // Get the annotation, method, and class names + String annotationName = annotationClass.getSimpleName(); + String annotatedMethodName = invocationContext.getMethod().getName(); + String annotatedClassCanonicalName = getAnnotatedMethodClassCanonicalName( + getAnnotatedMethodClass(invocationContext, annotationClass)); + + // Check if there's a config override + if (config != null) { + logger.log(Level.FINER, "Getting config override for annotated method..."); + + // Check if there's a config override for this specific method + value = config.getOptionalValue(annotatedClassCanonicalName + "/" + + annotatedMethodName + "/" + annotationName + "/" + "enabled", parameterType); + + if (!value.isPresent()) { + logger.log(Level.FINER, "No config override for annotated method, checking if the method is " + + "annotated directly..."); + // If the method is annotated directly, check for a cless or global-level override + if (invocationContext.getMethod().getAnnotation(annotationClass) != null) { + logger.log(Level.FINER, "Method is annotated directly, checking for class or global override..."); + checkForMethodLevelOverride(annotatedClassCanonicalName, annotationName, "enabled", + parameterType, config); + } + + // If the method wasn't annotated directly, check if there's an override for the class + if (!value.isPresent()) { + value = checkForClassLevelOverride(annotatedClassCanonicalName, annotationName, "enabled", + parameterType, config); + + // If there wasn't a config override for the class, check if there's a global one + if (!value.isPresent()) { + value = checkForGlobalLevelOverride(annotationName, "enabled", parameterType, config); + } + } + } + } else { + logger.log(Level.FINE, "No config to get override parameters from."); + } + + return value; + } + private static void logOverride(Optional value, String level) { if (value.isPresent()) { logger.log(Level.FINER, "{0} override found.", level); @@ -215,17 +263,16 @@ private static void logOverride(Optional value, String level) { logger.log(Level.FINER, "No config overrides."); } } - - private static Optional checkAnnotatedMethodForOverrides(String annotatedClassCanonicalName, String annotationName, + + private static Optional checkForMethodLevelOverride(String annotatedClassCanonicalName, String annotationName, String parameterName, Class parameterType, Config config) { - logger.log(Level.FINER, "Method is annotated directly, checking for class-level override."); Optional value = checkForClassLevelOverride(annotatedClassCanonicalName, annotationName, parameterName, parameterType, config); if (!value.isPresent()) { - value = checkForGlobalLevelOverride(annotationName, parameterName, parameterType, config); + checkForGlobalLevelOverride(annotationName, parameterName, parameterType, config); } - + return value; } @@ -248,4 +295,35 @@ private static Optional checkForGlobalLevelOverride(String annotationName, Strin return value; } + + /** + * Returns either the CDI Proxy class (for cases where a class extends another without overriding the target method), + * or the actual declaring class if the annotation isn't present on the proxy class. + * @param The class of the annotation + * @param invocationContext The context of the method invocation + * @param annotationClass The class of the annotation + * @return + */ + public static Class getAnnotatedMethodClass(InvocationContext invocationContext, + Class annotationClass) { + Class targetClass = invocationContext.getTarget().getClass(); + + if (targetClass.isAnnotationPresent(annotationClass)) { + return targetClass; + } else { + return invocationContext.getMethod().getDeclaringClass(); + } + } + + public static String getAnnotatedMethodClassCanonicalName(Class annotatedClass) { + String canonicalClassName = annotatedClass.getCanonicalName(); + + // If the class was a proxy from Weld, cut away the appended gubbins + // Author Note: There's probably a better way of doing this... + if (canonicalClassName.contains("$Proxy$_$$_WeldSubclass")) { + return canonicalClassName.split("\\$Proxy\\$_\\$\\$_WeldSubclass")[0]; + } else { + return canonicalClassName; + } + } } diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/AsynchronousInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/AsynchronousInterceptor.java index 328d11808e4..128a5e9c7f0 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/AsynchronousInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/AsynchronousInterceptor.java @@ -104,8 +104,8 @@ public Object intercept(InvocationContext invocationContext) throws Exception { // Attempt to proceed the InvocationContext with Asynchronous semantics if Fault Tolerance is enabled if (faultToleranceService.isFaultToleranceEnabled(appName, config) - && ((Boolean) FaultToleranceCdiUtils.getOverrideValue( - config, Asynchronous.class, "enabled", invocationContext, Boolean.class) + && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( + config, Asynchronous.class, invocationContext, Boolean.class) .orElse(Boolean.TRUE))) { Callable callable = () -> { return invocationContext.proceed(); @@ -124,8 +124,8 @@ public Object intercept(InvocationContext invocationContext) throws Exception { Fallback fallback = FaultToleranceCdiUtils.getAnnotation(beanManager, Fallback.class, invocationContext); // If the method was annotated with Fallback, attempt it, otherwise just propagate the exception upwards - if (fallback != null && ((Boolean) FaultToleranceCdiUtils.getOverrideValue( - config, Fallback.class, "enabled", invocationContext, Boolean.class) + if (fallback != null && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( + config, Fallback.class, invocationContext, Boolean.class) .orElse(Boolean.TRUE))) { logger.log(Level.FINE, "Fallback annotation found on method - falling back from Asynchronous"); FallbackPolicy fallbackPolicy = new FallbackPolicy(fallback, config, invocationContext); diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java index 6c75eaa590d..9f459758bf2 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java @@ -102,8 +102,8 @@ public Object intercept(InvocationContext invocationContext) throws Exception { // Attempt to proceed the InvocationContext with Asynchronous semantics if Fault Tolerance is enabled if (faultToleranceService.isFaultToleranceEnabled(appName, config) - && ((Boolean) FaultToleranceCdiUtils.getOverrideValue( - config, Bulkhead.class, "enabled", invocationContext, Boolean.class) + && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( + config, Bulkhead.class, invocationContext, Boolean.class) .orElse(Boolean.TRUE))) { logger.log(Level.FINER, "Proceeding invocation with bulkhead semantics"); proceededInvocationContext = bulkhead(invocationContext); @@ -125,8 +125,8 @@ public Object intercept(InvocationContext invocationContext) throws Exception { invocationContext); // If the method was annotated with Fallback, attempt it, otherwise just propagate the exception upwards - if (fallback != null && ((Boolean) FaultToleranceCdiUtils.getOverrideValue( - config, Fallback.class, "enabled", invocationContext, Boolean.class) + if (fallback != null && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( + config, Fallback.class, invocationContext, Boolean.class) .orElse(Boolean.TRUE))) { logger.log(Level.FINE, "Fallback annotation found on method, and no Retry annotation - " + "falling back from Bulkhead"); diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java index 19d8e8dc905..e942b1a49fa 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java @@ -106,8 +106,8 @@ public Object intercept(InvocationContext invocationContext) throws Exception { // Attempt to proceed the InvocationContext with Asynchronous semantics if Fault Tolerance is enabled if (faultToleranceService.isFaultToleranceEnabled(appName, config) - && ((Boolean) FaultToleranceCdiUtils.getOverrideValue( - config, CircuitBreaker.class, "enabled", invocationContext, Boolean.class) + && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( + config, CircuitBreaker.class, invocationContext, Boolean.class) .orElse(Boolean.TRUE))) { logger.log(Level.FINER, "Proceeding invocation with circuitbreaker semantics"); proceededInvocationContext = circuitBreak(invocationContext); @@ -127,8 +127,8 @@ public Object intercept(InvocationContext invocationContext) throws Exception { } else { Fallback fallback = FaultToleranceCdiUtils.getAnnotation(beanManager, Fallback.class, invocationContext); - if (fallback != null && ((Boolean) FaultToleranceCdiUtils.getOverrideValue( - config, Fallback.class, "enabled", invocationContext, Boolean.class) + if (fallback != null && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( + config, Fallback.class, invocationContext, Boolean.class) .orElse(Boolean.TRUE))) { logger.log(Level.FINE, "Fallback annotation found on method, and no Retry annotation - " + "falling back from CircuitBreaker"); diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java index 9682161ed12..f8ce22d7635 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java @@ -100,8 +100,8 @@ public Object intercept(InvocationContext invocationContext) throws Exception { // Attempt to proceed the InvocationContext with Asynchronous semantics if Fault Tolerance is enabled if (faultToleranceService.isFaultToleranceEnabled(appName, config) - && ((Boolean) FaultToleranceCdiUtils.getOverrideValue( - config, Retry.class, "enabled", invocationContext, Boolean.class) + && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( + config, Retry.class, invocationContext, Boolean.class) .orElse(Boolean.TRUE))) { logger.log(Level.FINER, "Proceeding invocation with retry semantics"); proceededInvocationContext = retry(invocationContext); @@ -114,8 +114,8 @@ public Object intercept(InvocationContext invocationContext) throws Exception { } catch (Exception ex) { Fallback fallback = FaultToleranceCdiUtils.getAnnotation(beanManager, Fallback.class, invocationContext); - if (fallback != null && ((Boolean) FaultToleranceCdiUtils.getOverrideValue( - config, Fallback.class, "enabled", invocationContext, Boolean.class) + if (fallback != null && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( + config, Fallback.class, invocationContext, Boolean.class) .orElse(Boolean.TRUE))) { logger.log(Level.FINE, "Fallback annotation found on method - falling back from Retry"); FallbackPolicy fallbackPolicy = new FallbackPolicy(fallback, config, invocationContext); diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java index 68cc83894f8..22eac5ed8d6 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java @@ -101,8 +101,8 @@ public Object intercept(InvocationContext invocationContext) throws Exception { // Attempt to proceed the InvocationContext with Asynchronous semantics if Fault Tolerance is enabled if (faultToleranceService.isFaultToleranceEnabled(appName, config) - && ((Boolean) FaultToleranceCdiUtils.getOverrideValue( - config, Timeout.class, "enabled", invocationContext, Boolean.class) + && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( + config, Timeout.class, invocationContext, Boolean.class) .orElse(Boolean.TRUE))) { logger.log(Level.FINER, "Proceeding invocation with timeout semantics"); proceededInvocationContext = timeout(invocationContext); @@ -122,8 +122,8 @@ public Object intercept(InvocationContext invocationContext) throws Exception { Fallback fallback = FaultToleranceCdiUtils.getAnnotation(beanManager, Fallback.class, invocationContext); - if (fallback != null && ((Boolean) FaultToleranceCdiUtils.getOverrideValue( - config, Fallback.class, "enabled", invocationContext, Boolean.class) + if (fallback != null && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( + config, Fallback.class, invocationContext, Boolean.class) .orElse(Boolean.TRUE))) { logger.log(Level.FINE, "Fallback annotation found on method, and no Retry annotation - " + "falling back from Timeout"); diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/fallback/FallbackPolicy.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/fallback/FallbackPolicy.java index 503015edf75..5677b90ba93 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/fallback/FallbackPolicy.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/fallback/FallbackPolicy.java @@ -99,7 +99,8 @@ public Object fallback(InvocationContext invocationContext, Throwable exception) if (fallbackMethod != null && !fallbackMethod.isEmpty()) { logger.log(Level.FINE, "Using fallback method: {0}", fallbackMethod); - fallbackInvocationContext = invocationContext.getMethod().getDeclaringClass() + fallbackInvocationContext = FaultToleranceCdiUtils + .getAnnotatedMethodClass(invocationContext, Fallback.class) .getDeclaredMethod(fallbackMethod, invocationContext.getMethod().getParameterTypes()) .invoke(invocationContext.getTarget(), invocationContext.getParameters()); } else { From 521a03a1847aa301d442239de3ca23c7ec30f05a Mon Sep 17 00:00:00 2001 From: Andrew Pielage Date: Thu, 23 Aug 2018 10:05:46 +0100 Subject: [PATCH 10/15] Comments and some cleanup --- .../cdi/FaultToleranceCdiUtils.java | 70 +++++++++++++++---- .../interceptors/AsynchronousInterceptor.java | 10 +-- .../interceptors/BulkheadInterceptor.java | 10 +-- .../CircuitBreakerInterceptor.java | 8 +-- .../interceptors/RetryInterceptor.java | 8 ++- .../interceptors/TimeoutInterceptor.java | 8 ++- 6 files changed, 84 insertions(+), 30 deletions(-) diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java index 083e3d0c778..0c062969a6e 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java @@ -210,8 +210,18 @@ public static Optional getOverrideValue(Config config, Cl return value; } + /** + * Gets overriding config enabled parameter value if it's present from an invocation context. + * This follows a different priority logic than other parameter overrides. + * + * @param The annotation type + * @param config The config to get the overriding enabled value from + * @param annotationClass The annotation class + * @param invocationContext The context of the invoking request + * @return + */ public static Optional getEnabledOverrideValue(Config config, Class annotationClass, - InvocationContext invocationContext, Class parameterType) { + InvocationContext invocationContext) { Optional value = Optional.empty(); // Get the annotation, method, and class names @@ -226,7 +236,7 @@ public static Optional getEnabledOverrideValue(Config con // Check if there's a config override for this specific method value = config.getOptionalValue(annotatedClassCanonicalName + "/" - + annotatedMethodName + "/" + annotationName + "/" + "enabled", parameterType); + + annotatedMethodName + "/" + annotationName + "/" + "enabled", Boolean.class); if (!value.isPresent()) { logger.log(Level.FINER, "No config override for annotated method, checking if the method is " @@ -234,18 +244,17 @@ public static Optional getEnabledOverrideValue(Config con // If the method is annotated directly, check for a cless or global-level override if (invocationContext.getMethod().getAnnotation(annotationClass) != null) { logger.log(Level.FINER, "Method is annotated directly, checking for class or global override..."); - checkForMethodLevelOverride(annotatedClassCanonicalName, annotationName, "enabled", - parameterType, config); + checkAnnotatedMethodForEnabledOverride(annotatedClassCanonicalName, annotationName, config); } // If the method wasn't annotated directly, check if there's an override for the class if (!value.isPresent()) { value = checkForClassLevelOverride(annotatedClassCanonicalName, annotationName, "enabled", - parameterType, config); + Boolean.class, config); // If there wasn't a config override for the class, check if there's a global one if (!value.isPresent()) { - value = checkForGlobalLevelOverride(annotationName, "enabled", parameterType, config); + value = checkForGlobalLevelOverride(annotationName, "enabled", Boolean.class, config); } } } @@ -256,6 +265,11 @@ public static Optional getEnabledOverrideValue(Config con return value; } + /** + * Helper method that logs whether an override was found. + * @param value The Optional value to check if an override was found for + * @param level A String denoting the level of override you were checking for + */ private static void logOverride(Optional value, String level) { if (value.isPresent()) { logger.log(Level.FINER, "{0} override found.", level); @@ -264,18 +278,35 @@ private static void logOverride(Optional value, String level) { } } - private static Optional checkForMethodLevelOverride(String annotatedClassCanonicalName, String annotationName, - String parameterName, Class parameterType, Config config) { - Optional value = checkForClassLevelOverride(annotatedClassCanonicalName, annotationName, parameterName, - parameterType, config); + /** + * Helper method that checks if a directly annotated method has a class or global-level override for the enabled + * parameter. This gets its own method as the enabled parameter has a different override priority. + * @param annotatedClassCanonicalName The canonical name of the annotated class + * @param annotationName The name of the annotation + * @param config The config to get the overriding enabled value from + * @return An overriding enabled value if there is one + */ + private static Optional checkAnnotatedMethodForEnabledOverride(String annotatedClassCanonicalName, + String annotationName, Config config) { + Optional value = checkForClassLevelOverride(annotatedClassCanonicalName, annotationName, "enabled", + Boolean.class, config); if (!value.isPresent()) { - checkForGlobalLevelOverride(annotationName, parameterName, parameterType, config); + checkForGlobalLevelOverride(annotationName, "enabled", Boolean.class, config); } return value; } + /** + * Helper method that checks if there is an override for a parameter at the class level. + * @param annotatedClassCanonicalName The canonical name of the annotated class + * @param annotationName The name of the annotation + * @param parameterName The name of the parameter + * @param parameterType The parameter class + * @param config The config to get the overriding value from + * @return An overriding value for the provided parameter if there is one + */ private static Optional checkForClassLevelOverride(String annotatedClassCanonicalName, String annotationName, String parameterName, Class parameterType, Config config) { logger.log(Level.FINER, "No config override for annotated method, getting config override for the " @@ -287,6 +318,14 @@ private static Optional checkForClassLevelOverride(String annotatedClassCanonica return value; } + /** + * Helper method that checks if there is an override for a parameter at the global level. + * @param annotationName The name of the annotation + * @param parameterName The name of the parameter + * @param parameterType The parameter class + * @param config The config to get the overriding value from + * @return + */ private static Optional checkForGlobalLevelOverride(String annotationName, String parameterName, Class parameterType, Config config) { logger.log(Level.FINER, "No config override for annotated class, checking for global override."); @@ -302,7 +341,7 @@ private static Optional checkForGlobalLevelOverride(String annotationName, Strin * @param The class of the annotation * @param invocationContext The context of the method invocation * @param annotationClass The class of the annotation - * @return + * @return The class of the annotated method */ public static Class getAnnotatedMethodClass(InvocationContext invocationContext, Class annotationClass) { @@ -315,6 +354,13 @@ public static Class getAnnotatedMethodClass(InvocationCon } } + /** + * Helper method that gets the canonical name of an annotated class. This is used to strip out any Weld proxy + * naming that gets appended to the real class name. + * @param The class of the annotation + * @param annotatedClass The class of the annotation + * @return The canonical name of the annotated method's class + */ public static String getAnnotatedMethodClassCanonicalName(Class annotatedClass) { String canonicalClassName = annotatedClass.getCanonicalName(); diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/AsynchronousInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/AsynchronousInterceptor.java index 128a5e9c7f0..5737f1716bb 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/AsynchronousInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/AsynchronousInterceptor.java @@ -102,10 +102,11 @@ public Object intercept(InvocationContext invocationContext) throws Exception { try { String appName = faultToleranceService.getApplicationName(invocationManager, invocationContext); - // Attempt to proceed the InvocationContext with Asynchronous semantics if Fault Tolerance is enabled + // Attempt to proceed the InvocationContext with Asynchronous semantics if Fault Tolerance is enabled for + // this method if (faultToleranceService.isFaultToleranceEnabled(appName, config) && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( - config, Asynchronous.class, invocationContext, Boolean.class) + config, Asynchronous.class, invocationContext) .orElse(Boolean.TRUE))) { Callable callable = () -> { return invocationContext.proceed(); @@ -123,9 +124,10 @@ public Object intercept(InvocationContext invocationContext) throws Exception { // We should only get here if executing synchronously, as the exception wouldn't get thrown in this thread Fallback fallback = FaultToleranceCdiUtils.getAnnotation(beanManager, Fallback.class, invocationContext); - // If the method was annotated with Fallback, attempt it, otherwise just propagate the exception upwards + // If the method was annotated with Fallback and the annotation is enabled, attempt it, otherwise just + // propagate the exception upwards if (fallback != null && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( - config, Fallback.class, invocationContext, Boolean.class) + config, Fallback.class, invocationContext) .orElse(Boolean.TRUE))) { logger.log(Level.FINE, "Fallback annotation found on method - falling back from Asynchronous"); FallbackPolicy fallbackPolicy = new FallbackPolicy(fallback, config, invocationContext); diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java index 9f459758bf2..529aaf458b3 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java @@ -100,10 +100,11 @@ public Object intercept(InvocationContext invocationContext) throws Exception { try { String appName = faultToleranceService.getApplicationName(invocationManager, invocationContext); - // Attempt to proceed the InvocationContext with Asynchronous semantics if Fault Tolerance is enabled + // Attempt to proceed the InvocationContext with Asynchronous semantics if Fault Tolerance is enabled for this + // method if (faultToleranceService.isFaultToleranceEnabled(appName, config) && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( - config, Bulkhead.class, invocationContext, Boolean.class) + config, Bulkhead.class, invocationContext) .orElse(Boolean.TRUE))) { logger.log(Level.FINER, "Proceeding invocation with bulkhead semantics"); proceededInvocationContext = bulkhead(invocationContext); @@ -124,9 +125,10 @@ public Object intercept(InvocationContext invocationContext) throws Exception { Fallback fallback = FaultToleranceCdiUtils.getAnnotation(beanManager, Fallback.class, invocationContext); - // If the method was annotated with Fallback, attempt it, otherwise just propagate the exception upwards + // If the method was annotated with Fallback and the annotation is enabled, attempt it, otherwise just + // propagate the exception upwards if (fallback != null && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( - config, Fallback.class, invocationContext, Boolean.class) + config, Fallback.class, invocationContext) .orElse(Boolean.TRUE))) { logger.log(Level.FINE, "Fallback annotation found on method, and no Retry annotation - " + "falling back from Bulkhead"); diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java index e942b1a49fa..8ae21e712cf 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java @@ -99,15 +99,14 @@ public Object intercept(InvocationContext invocationContext) throws Exception { logger.log(Level.INFO, "No config could be found", ex); } - // Attempt to proceed the invocation with CircuitBreaker semantics if Fault Tolerance is enabled, and if - // this particular method hasn't been excluded + // Attempt to proceed the invocation with CircuitBreaker semantics if Fault Tolerance is enabled for this method try { String appName = faultToleranceService.getApplicationName(invocationManager, invocationContext); // Attempt to proceed the InvocationContext with Asynchronous semantics if Fault Tolerance is enabled if (faultToleranceService.isFaultToleranceEnabled(appName, config) && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( - config, CircuitBreaker.class, invocationContext, Boolean.class) + config, CircuitBreaker.class, invocationContext) .orElse(Boolean.TRUE))) { logger.log(Level.FINER, "Proceeding invocation with circuitbreaker semantics"); proceededInvocationContext = circuitBreak(invocationContext); @@ -127,8 +126,9 @@ public Object intercept(InvocationContext invocationContext) throws Exception { } else { Fallback fallback = FaultToleranceCdiUtils.getAnnotation(beanManager, Fallback.class, invocationContext); + // Only fall back if the annotation hasn't been disabled if (fallback != null && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( - config, Fallback.class, invocationContext, Boolean.class) + config, Fallback.class, invocationContext) .orElse(Boolean.TRUE))) { logger.log(Level.FINE, "Fallback annotation found on method, and no Retry annotation - " + "falling back from CircuitBreaker"); diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java index f8ce22d7635..2e7d84b264c 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java @@ -98,10 +98,11 @@ public Object intercept(InvocationContext invocationContext) throws Exception { try { String appName = faultToleranceService.getApplicationName(invocationManager, invocationContext); - // Attempt to proceed the InvocationContext with Asynchronous semantics if Fault Tolerance is enabled + // Attempt to proceed the InvocationContext with Asynchronous semantics if Fault Tolerance is enabled for this + // method if (faultToleranceService.isFaultToleranceEnabled(appName, config) && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( - config, Retry.class, invocationContext, Boolean.class) + config, Retry.class, invocationContext) .orElse(Boolean.TRUE))) { logger.log(Level.FINER, "Proceeding invocation with retry semantics"); proceededInvocationContext = retry(invocationContext); @@ -114,8 +115,9 @@ public Object intercept(InvocationContext invocationContext) throws Exception { } catch (Exception ex) { Fallback fallback = FaultToleranceCdiUtils.getAnnotation(beanManager, Fallback.class, invocationContext); + // Only fall back if the annotation hasn't been disabled if (fallback != null && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( - config, Fallback.class, invocationContext, Boolean.class) + config, Fallback.class, invocationContext) .orElse(Boolean.TRUE))) { logger.log(Level.FINE, "Fallback annotation found on method - falling back from Retry"); FallbackPolicy fallbackPolicy = new FallbackPolicy(fallback, config, invocationContext); diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java index 22eac5ed8d6..b0b76568afc 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java @@ -99,10 +99,11 @@ public Object intercept(InvocationContext invocationContext) throws Exception { try { String appName = faultToleranceService.getApplicationName(invocationManager, invocationContext); - // Attempt to proceed the InvocationContext with Asynchronous semantics if Fault Tolerance is enabled + // Attempt to proceed the InvocationContext with Asynchronous semantics if Fault Tolerance is enabled for this + // method if (faultToleranceService.isFaultToleranceEnabled(appName, config) && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( - config, Timeout.class, invocationContext, Boolean.class) + config, Timeout.class, invocationContext) .orElse(Boolean.TRUE))) { logger.log(Level.FINER, "Proceeding invocation with timeout semantics"); proceededInvocationContext = timeout(invocationContext); @@ -122,8 +123,9 @@ public Object intercept(InvocationContext invocationContext) throws Exception { Fallback fallback = FaultToleranceCdiUtils.getAnnotation(beanManager, Fallback.class, invocationContext); + // Only fall back if the annotation hasn't been disabled if (fallback != null && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( - config, Fallback.class, invocationContext, Boolean.class) + config, Fallback.class, invocationContext) .orElse(Boolean.TRUE))) { logger.log(Level.FINE, "Fallback annotation found on method, and no Retry annotation - " + "falling back from Timeout"); From 291214c8412ec0e67e074ecfd297e4a086e7078a Mon Sep 17 00:00:00 2001 From: Andrew Pielage Date: Thu, 23 Aug 2018 15:30:48 +0100 Subject: [PATCH 11/15] PAYARA-2721 Initial stab at metrics integration --- .../microprofile/fault-tolerance/pom.xml | 5 + .../cdi/FaultToleranceCdiUtils.java | 13 ++ .../interceptors/BulkheadInterceptor.java | 117 +++++++++++++- .../CircuitBreakerInterceptor.java | 149 ++++++++++++++++-- .../interceptors/RetryInterceptor.java | 29 ++++ .../interceptors/TimeoutInterceptor.java | 34 ++++ .../interceptors/fallback/FallbackPolicy.java | 14 +- .../state/CircuitBreakerState.java | 11 +- 8 files changed, 356 insertions(+), 16 deletions(-) diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/pom.xml b/appserver/payara-appserver-modules/microprofile/fault-tolerance/pom.xml index 44c4cd2cffe..ea1aa03ddf8 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/pom.xml +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/pom.xml @@ -73,6 +73,11 @@ microprofile-config-api ${microprofile-config.version} + + org.eclipse.microprofile.metrics + microprofile-metrics-api + ${microprofile-metrics.version} + org.glassfish.main.common internal-api diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java index 0c062969a6e..48a593b7779 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceCdiUtils.java @@ -372,4 +372,17 @@ public static String getAnnotatedMethodClassCanonicalName return canonicalClassName; } } + + /** + * Gets the full method signature from an invocation context, stripping out any Weld proxy name gubbins. + * @param The class of the annotation + * @param invocationContext The context of the method invocation + * @param annotationClass The class of the annotation + * @return + */ + public static String getFullAnnotatedMethodSignature(InvocationContext invocationContext, + Class annotationClass) { + return getAnnotatedMethodClassCanonicalName(getAnnotatedMethodClass(invocationContext, annotationClass)) + "." + + invocationContext.getMethod().getName(); + } } diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java index 529aaf458b3..0ccbe675e8a 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java @@ -42,6 +42,7 @@ import fish.payara.microprofile.faulttolerance.interceptors.fallback.FallbackPolicy; import fish.payara.microprofile.faulttolerance.FaultToleranceService; import fish.payara.microprofile.faulttolerance.cdi.FaultToleranceCdiUtils; +//import fish.payara.microprofile.metrics.impl.GaugeImpl; import java.io.Serializable; import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; @@ -55,6 +56,7 @@ import javax.interceptor.InvocationContext; import fish.payara.notification.requesttracing.RequestTraceSpan; +import javax.enterprise.inject.spi.CDI; import org.eclipse.microprofile.config.Config; import org.eclipse.microprofile.config.ConfigProvider; import org.eclipse.microprofile.faulttolerance.Asynchronous; @@ -62,6 +64,8 @@ import org.eclipse.microprofile.faulttolerance.Fallback; import org.eclipse.microprofile.faulttolerance.Retry; import org.eclipse.microprofile.faulttolerance.exceptions.BulkheadException; +import org.eclipse.microprofile.metrics.Gauge; +import org.eclipse.microprofile.metrics.MetricRegistry; import org.glassfish.api.invocation.InvocationManager; import org.glassfish.internal.api.Globals; @@ -136,6 +140,13 @@ public Object intercept(InvocationContext invocationContext) throws Exception { proceededInvocationContext = fallbackPolicy.fallback(invocationContext, ex); } else { logger.log(Level.FINE, "Fallback annotation not found on method, propagating error upwards.", ex); + + // Increment the failure counter metric + MetricRegistry metricRegistry = CDI.current().select(MetricRegistry.class).get(); + String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, + Fallback.class); + metricRegistry.counter("ft." + fullMethodSignature + ".invocations.failed.total").inc(); + throw ex; } } @@ -164,7 +175,7 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { } catch (IllegalArgumentException ex) { logger.log(Level.INFO, "No config could be found", ex); } - + int value = (Integer) FaultToleranceCdiUtils.getOverrideValue( config, Bulkhead.class, "value", invocationContext, Integer.class) .orElse(bulkhead.value()); @@ -177,14 +188,46 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { String appName = faultToleranceService.getApplicationName(invocationManager, invocationContext); + + MetricRegistry metricRegistry = CDI.current().select(MetricRegistry.class).get(); + String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, + Bulkhead.class); + + // Only increment the invocations metric if the Retry annotation isn't present + if (FaultToleranceCdiUtils.getAnnotation(beanManager, Retry.class, invocationContext) == null) { + metricRegistry.counter("ft." + fullMethodSignature + ".invocations.total").inc(); + } + Semaphore bulkheadExecutionSemaphore = faultToleranceService.getBulkheadExecutionSemaphore(appName, invocationContext.getMethod(), value); + Gauge concurrentExecutionsGauge = metricRegistry.getGauges() + .get("ft." + fullMethodSignature + ".bulkhead.waitingQueue.population"); + + // Register a bulkhead concurrent executions metric if there isn't one + if (concurrentExecutionsGauge == null) { + concurrentExecutionsGauge = () -> getConcurrentExecutionsCount(value, bulkheadExecutionSemaphore); + + metricRegistry.register("ft." + fullMethodSignature + ".bulkhead.concurrentExecutions", + concurrentExecutionsGauge); + } + // If the Asynchronous annotation is present, use threadpool style, otherwise use semaphore style if (FaultToleranceCdiUtils.getAnnotation(beanManager, Asynchronous.class, invocationContext) != null) { Semaphore bulkheadExecutionQueueSemaphore = faultToleranceService.getBulkheadExecutionQueueSemaphore( appName, invocationContext.getMethod(), waitingTaskQueue); - + + Gauge waitingQueueGauge = metricRegistry.getGauges() + .get("ft." + fullMethodSignature + ".bulkhead.waitingQueue.population"); + + // Register a bulkhead queue metric if there isn't one + if (waitingQueueGauge == null) { + waitingQueueGauge = () -> getWaitingQueueCount(waitingTaskQueue, bulkheadExecutionQueueSemaphore); + + metricRegistry.register("ft." + fullMethodSignature + ".bulkhead.waitingQueue.population", + waitingQueueGauge); + } + // Check if there are any free permits for concurrent execution if (!bulkheadExecutionSemaphore.tryAcquire(0, TimeUnit.SECONDS)) { logger.log(Level.FINER, "Attempting to acquire bulkhead queue semaphore."); @@ -192,6 +235,12 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { if (bulkheadExecutionQueueSemaphore.tryAcquire(0, TimeUnit.SECONDS)) { logger.log(Level.FINER, "Acquired bulkhead queue semaphore."); + // Update the bulkhead queueing metric +// waitingQueueGauge. + + // Start measuring the queue duration for MP Metrics + long queueStartTime = System.nanoTime(); + // If there is a free queue permit, queue for an executor permit try { logger.log(Level.FINER, "Attempting to acquire bulkhead execution semaphore."); @@ -202,6 +251,10 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { } finally { // Make sure we end the trace right here faultToleranceService.endFaultToleranceSpan(); + + // Record the execution time for MP Metrics + metricRegistry.histogram("ft." + fullMethodSignature + ".bulkhead.waiting.duration").update( + System.nanoTime() - queueStartTime); } logger.log(Level.FINER, "Acquired bulkhead queue semaphore."); @@ -209,6 +262,12 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { // Release the queue permit bulkheadExecutionQueueSemaphore.release(); + // Incremement the MP Metrics callsAccepted counter + metricRegistry.counter("ft." + fullMethodSignature + ".bulkhead.callsAccepted.total").inc(); + + // Start measuring the execution duration for MP Metrics + long executionStartTime = System.nanoTime(); + // Proceed the invocation and wait for the response try { logger.log(Level.FINER, "Proceeding bulkhead context"); @@ -220,20 +279,39 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { bulkheadExecutionSemaphore.release(); bulkheadExecutionQueueSemaphore.release(); + // Record the execution time for MP Metrics + metricRegistry.histogram("ft." + fullMethodSignature + ".bulkhead.executionDuration").update( + System.nanoTime() - executionStartTime); + // Let the exception propagate further up - we just want to release the semaphores throw ex; } + // Record the execution time for MP Metrics + metricRegistry.histogram("ft." + fullMethodSignature + ".bulkhead.executionDuration").update( + System.nanoTime() - executionStartTime); + // Release the execution permit bulkheadExecutionSemaphore.release(); } catch (InterruptedException ex) { + // Incremement the MP Metrics callsRejected counter + metricRegistry.counter("ft." + fullMethodSignature + ".bulkhead.callsRejected.total").inc(); + logger.log(Level.INFO, "Interrupted acquiring bulkhead semaphore", ex); throw new BulkheadException(ex); } } else { + // Incremement the MP Metrics callsRejected counter + metricRegistry.counter("ft." + fullMethodSignature + ".bulkhead.callsRejected.total").inc(); throw new BulkheadException("No free work or queue permits."); } } else { + // Incremement the MP Metrics callsAccepted counter + metricRegistry.counter("ft." + fullMethodSignature + ".bulkhead.callsAccepted.total").inc(); + + // Start measuring the execution duration for MP Metrics + long executionStartTime = System.nanoTime(); + // Proceed the invocation and wait for the response try { logger.log(Level.FINER, "Proceeding bulkhead context"); @@ -243,17 +321,31 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { // Generic catch, as we need to release the semaphore permits bulkheadExecutionSemaphore.release(); - + + // Record the execution time for MP Metrics + metricRegistry.histogram("ft." + fullMethodSignature + ".bulkhead.executionDuration").update( + System.nanoTime() - executionStartTime); + // Let the exception propagate further up - we just want to release the semaphores throw ex; } - + + // Record the execution time for MP Metrics + metricRegistry.histogram("ft." + fullMethodSignature + ".bulkhead.executionDuration").update( + System.nanoTime() - executionStartTime); + // Release the permit bulkheadExecutionSemaphore.release(); } } else { // Try to get an execution permit if (bulkheadExecutionSemaphore.tryAcquire(0, TimeUnit.SECONDS)) { + // Incremement the MP Metrics callsAccepted counter + metricRegistry.counter("ft." + fullMethodSignature + ".bulkhead.callsAccepted.total").inc(); + + // Start measuring the execution duration for MP Metrics + long executionStartTime = System.nanoTime(); + // Proceed the invocation and wait for the response try { logger.log(Level.FINER, "Proceeding bulkhead context"); @@ -264,13 +356,24 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { // Generic catch, as we need to release the semaphore permits bulkheadExecutionSemaphore.release(); + // Record the execution time for MP Metrics + metricRegistry.histogram("ft." + fullMethodSignature + ".bulkhead.executionDuration").update( + System.nanoTime() - executionStartTime); + // Let the exception propagate further up - we just want to release the semaphores throw ex; } + // Record the execution time for MP Metrics + metricRegistry.histogram("ft." + fullMethodSignature + ".bulkhead.executionDuration").update( + System.nanoTime() - executionStartTime); + // Release the permit bulkheadExecutionSemaphore.release(); } else { + // Incremement the MP Metrics callsRejected counter + metricRegistry.counter("ft." + fullMethodSignature + ".bulkhead.callsRejected.total").inc(); + throw new BulkheadException("No free work permits."); } } @@ -278,5 +381,11 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { return proceededInvocationContext; } + private Long getConcurrentExecutionsCount(int bulkheadValue, Semaphore bulkheadExecutionSemaphore) { + return ((Number) (bulkheadValue - bulkheadExecutionSemaphore.availablePermits())).longValue(); + } + private Long getWaitingQueueCount(int waitingTaskQueue, Semaphore bulkheadExecutionQueueSemaphore) { + return ((Number) (waitingTaskQueue - bulkheadExecutionQueueSemaphore.availablePermits())).longValue(); + } } diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java index 8ae21e712cf..e21726ba30e 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java @@ -55,6 +55,7 @@ import javax.annotation.Priority; import javax.enterprise.concurrent.ManagedScheduledExecutorService; import javax.enterprise.inject.spi.BeanManager; +import javax.enterprise.inject.spi.CDI; import javax.inject.Inject; import javax.interceptor.AroundInvoke; import javax.interceptor.Interceptor; @@ -62,10 +63,13 @@ import javax.naming.NamingException; import org.eclipse.microprofile.config.Config; import org.eclipse.microprofile.config.ConfigProvider; +import org.eclipse.microprofile.faulttolerance.Bulkhead; import org.eclipse.microprofile.faulttolerance.CircuitBreaker; import org.eclipse.microprofile.faulttolerance.Fallback; import org.eclipse.microprofile.faulttolerance.Retry; import org.eclipse.microprofile.faulttolerance.exceptions.CircuitBreakerOpenException; +import org.eclipse.microprofile.metrics.Gauge; +import org.eclipse.microprofile.metrics.MetricRegistry; import org.glassfish.api.invocation.InvocationManager; import org.glassfish.internal.api.Globals; @@ -135,6 +139,12 @@ public Object intercept(InvocationContext invocationContext) throws Exception { FallbackPolicy fallbackPolicy = new FallbackPolicy(fallback, config, invocationContext); proceededInvocationContext = fallbackPolicy.fallback(invocationContext, ex); } else { + // Increment the failure counter metric + MetricRegistry metricRegistry = CDI.current().select(MetricRegistry.class).get(); + String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, + Fallback.class); + metricRegistry.counter("ft." + fullMethodSignature + ".invocations.failed.total").inc(); + throw ex; } } @@ -151,6 +161,16 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio CircuitBreaker circuitBreaker = FaultToleranceCdiUtils.getAnnotation(beanManager, CircuitBreaker.class, invocationContext); + MetricRegistry metricRegistry = CDI.current().select(MetricRegistry.class).get(); + String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, + CircuitBreaker.class); + + // Only increment the invocations metric if the Retry and Bulkhead annotations aren't present + if (FaultToleranceCdiUtils.getAnnotation(beanManager, Bulkhead.class, invocationContext) == null + && FaultToleranceCdiUtils.getAnnotation(beanManager, Retry.class, invocationContext) == null) { + metricRegistry.counter("ft." + fullMethodSignature + ".invocations.total").inc(); + } + Config config = null; try { config = ConfigProvider.getConfig(); @@ -205,10 +225,24 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio faultToleranceService.getApplicationName(invocationManager, invocationContext), invocationContext.getMethod(), circuitBreaker); + Gauge openTimeGauge = metricRegistry.getGauges() + .get("ft." + fullMethodSignature + ".circuitbreaker.open.total"); + + Gauge halfOpenTimeGauge = metricRegistry.getGauges() + .get("ft." + fullMethodSignature + ".circuitbreaker.open.total"); + + Gauge closedTimeGauge = metricRegistry.getGauges() + .get("ft." + fullMethodSignature + ".circuitbreaker.open.total"); + + registerGaugesIfNecessary(openTimeGauge, halfOpenTimeGauge, closedTimeGauge, metricRegistry, circuitBreakerState, + fullMethodSignature); + switch (circuitBreakerState.getCircuitState()) { case OPEN: logger.log(Level.FINER, "CircuitBreaker is Open, throwing exception"); + metricRegistry.counter("ft." + fullMethodSignature + ".circuitbreaker.callsPrevented.total").inc(); + // If open, immediately throw an error throw new CircuitBreakerOpenException("CircuitBreaker for method " + invocationContext.getMethod().getName() + " is in state OPEN."); @@ -227,11 +261,15 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio // Add a failure result to the queue circuitBreakerState.recordClosedResult(Boolean.FALSE); + metricRegistry.counter("ft." + fullMethodSignature + ".circuitbreaker.callsFailed.total").inc(); + // Calculate the failure ratio, and if we're over it, open the circuit breakCircuitIfRequired( Math.round(requestVolumeThreshold * failureRatio), circuitBreakerState, - delayMillis); + delayMillis, + metricRegistry, + fullMethodSignature); } // Finally, propagate the error upwards @@ -240,12 +278,15 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio // If everything is bon, add a success value circuitBreakerState.recordClosedResult(Boolean.TRUE); + metricRegistry.counter("ft." + fullMethodSignature + ".circuitbreaker.callsSucceeded.total").inc(); // Calculate the failure ratio, and if we're over it, open the circuit breakCircuitIfRequired( Math.round(requestVolumeThreshold * failureRatio), circuitBreakerState, - delayMillis); + delayMillis, + metricRegistry, + fullMethodSignature); break; case HALF_OPEN: // If half-open, attempt to proceed the invocation context @@ -260,10 +301,13 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio logger.log(Level.FINE, "Caught exception is included in CircuitBreaker failOn, " + "reopening half open circuit"); + metricRegistry.counter("ft." + fullMethodSignature + ".circuitbreaker.callsFailed.total").inc(); + // Open the circuit again, and reset the half-open result counter - circuitBreakerState.setCircuitState(CircuitBreakerState.CircuitState.OPEN); + updateCircuitState(circuitBreakerState, CircuitBreakerState.CircuitState.OPEN, metricRegistry, + fullMethodSignature); circuitBreakerState.resetHalfOpenSuccessfulResultCounter(); - scheduleHalfOpen(delayMillis, circuitBreakerState); + scheduleHalfOpen(delayMillis, circuitBreakerState, metricRegistry, fullMethodSignature); } throw ex; @@ -271,6 +315,7 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio // If the invocation context hasn't thrown an error, record a success circuitBreakerState.incrementHalfOpenSuccessfulResultCounter(); + metricRegistry.counter("ft." + fullMethodSignature + ".circuitbreaker.callsSucceeded.total").inc(); logger.log(Level.FINER, "Number of consecutive successful circuitbreaker executions = {0}", circuitBreakerState.getHalfOpenSuccessFulResultCounter()); @@ -279,7 +324,8 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio logger.log(Level.FINE, "Number of consecutive successful CircuitBreaker executions is above " + "threshold {0}, closing circuit", successThreshold); - circuitBreakerState.setCircuitState(CircuitBreakerState.CircuitState.CLOSED); + updateCircuitState(circuitBreakerState, CircuitBreakerState.CircuitState.CLOSED, + metricRegistry, fullMethodSignature); // Reset the counter for when we next need to use it circuitBreakerState.resetHalfOpenSuccessfulResultCounter(); @@ -300,9 +346,11 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio * @param circuitBreakerState The CircuitBreakerState to set the state of * @throws NamingException If the ManagedScheduledExecutor couldn't be found */ - private void scheduleHalfOpen(long delayMillis, CircuitBreakerState circuitBreakerState) throws NamingException { + private void scheduleHalfOpen(long delayMillis, CircuitBreakerState circuitBreakerState, + MetricRegistry metricRegistry, String fullMethodSignature) throws NamingException { Runnable halfOpen = () -> { - circuitBreakerState.setCircuitState(CircuitBreakerState.CircuitState.HALF_OPEN); + updateCircuitState(circuitBreakerState, CircuitBreakerState.CircuitState.HALF_OPEN, metricRegistry, + fullMethodSignature); logger.log(Level.FINE, "Setting CircuitBreaker state to half open"); }; @@ -353,16 +401,97 @@ private boolean shouldFail(Class[] failOn, Exception ex) { } private void breakCircuitIfRequired(long failureThreshold, CircuitBreakerState circuitBreakerState, - long delayMillis) throws NamingException { + long delayMillis, MetricRegistry metricRegistry, String fullMethodSignature) throws NamingException { // If we're over the failure threshold, open the circuit if (circuitBreakerState.isOverFailureThreshold(failureThreshold)) { logger.log(Level.FINE, "CircuitBreaker is over failure threshold {0}, opening circuit", failureThreshold); - circuitBreakerState.setCircuitState(CircuitBreakerState.CircuitState.OPEN); + // Update the circuit state and metric timers + updateCircuitState(circuitBreakerState, CircuitBreakerState.CircuitState.OPEN, metricRegistry, + fullMethodSignature); + + // Update the opened metric counter + metricRegistry.counter("ft." + fullMethodSignature + ".circuitbreaker.opened.total").inc(); // Kick off a thread that will half-open the circuit after the specified delay - scheduleHalfOpen(delayMillis, circuitBreakerState); + scheduleHalfOpen(delayMillis, circuitBreakerState, metricRegistry, fullMethodSignature); + } + } + + private void updateCircuitState(CircuitBreakerState circuitBreakerState, + CircuitBreakerState.CircuitState circuitState, MetricRegistry metricRegistry, String fullMethodSignature) { + switch (circuitBreakerState.getCircuitState()) { + case OPEN: + getTimeOpen(metricRegistry, circuitBreakerState, fullMethodSignature); + break; + case HALF_OPEN: + getTimeHalfOpen(metricRegistry, circuitBreakerState, fullMethodSignature); + break; + case CLOSED: + getTimeClosed(metricRegistry, circuitBreakerState, fullMethodSignature); + break; + } + + circuitBreakerState.setCircuitState(circuitState); + circuitBreakerState.resetTimer(); + } + + private Long getTimeOpen(MetricRegistry metricRegistry, CircuitBreakerState circuitBreakerState, + String fullMethodSignature) { + if (circuitBreakerState.getCircuitState().equals(CircuitBreakerState.CircuitState.OPEN)) { + return (System.nanoTime() - circuitBreakerState.getTimer()) + (Long) metricRegistry.getGauges() + .get("ft." + fullMethodSignature + ".circuitbreaker.open.total").getValue(); + } else { + return (Long) metricRegistry.getGauges().get("ft." + fullMethodSignature + ".circuitbreaker.open.total") + .getValue(); + } + } + + private Long getTimeHalfOpen(MetricRegistry metricRegistry, CircuitBreakerState circuitBreakerState, + String fullMethodSignature) { + if (circuitBreakerState.getCircuitState().equals(CircuitBreakerState.CircuitState.HALF_OPEN)) { + return (System.nanoTime() - circuitBreakerState.getTimer()) + (Long) metricRegistry.getGauges() + .get("ft." + fullMethodSignature + ".circuitbreaker.halfOpen.total").getValue(); + } else { + return (Long) metricRegistry.getGauges().get("ft." + fullMethodSignature + ".circuitbreaker.halfOpen.total") + .getValue(); + } + } + + private Long getTimeClosed(MetricRegistry metricRegistry, CircuitBreakerState circuitBreakerState, + String fullMethodSignature) { + if (circuitBreakerState.getCircuitState().equals(CircuitBreakerState.CircuitState.CLOSED)) { + return (System.nanoTime() - circuitBreakerState.getTimer()) + (Long) metricRegistry.getGauges() + .get("ft." + fullMethodSignature + ".circuitbreaker.closed.total").getValue(); + } else { + return (Long) metricRegistry.getGauges().get("ft." + fullMethodSignature + ".circuitbreaker.closed.total") + .getValue(); + } + } + + private void registerGaugesIfNecessary(Gauge openTimeGauge, Gauge halfOpenTimeGauge, Gauge closedTimeGauge, + MetricRegistry metricRegistry, CircuitBreakerState circuitBreakerState, String fullMethodSignature) { + + // Register a open time gauge if there isn't one + if (openTimeGauge == null) { + openTimeGauge = () -> getTimeOpen(metricRegistry, circuitBreakerState, fullMethodSignature); + + metricRegistry.register("ft." + fullMethodSignature + ".circuitbreaker.open.total", openTimeGauge); + } + + // Register a open time gauge if there isn't one + if (halfOpenTimeGauge == null) { + halfOpenTimeGauge = () -> getTimeHalfOpen(metricRegistry, circuitBreakerState, fullMethodSignature); + + metricRegistry.register("ft." + fullMethodSignature + ".circuitbreaker.open.total", halfOpenTimeGauge); + } + + // Register a open time gauge if there isn't one + if (closedTimeGauge == null) { + closedTimeGauge = () -> getTimeClosed(metricRegistry, circuitBreakerState, fullMethodSignature); + + metricRegistry.register("ft." + fullMethodSignature + ".circuitbreaker.open.total", closedTimeGauge); } } } diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java index 2e7d84b264c..520042a5cb3 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java @@ -58,10 +58,12 @@ import javax.interceptor.InvocationContext; import fish.payara.notification.requesttracing.RequestTraceSpan; +import javax.enterprise.inject.spi.CDI; import org.eclipse.microprofile.config.Config; import org.eclipse.microprofile.config.ConfigProvider; import org.eclipse.microprofile.faulttolerance.Fallback; import org.eclipse.microprofile.faulttolerance.Retry; +import org.eclipse.microprofile.metrics.MetricRegistry; import org.glassfish.api.invocation.InvocationManager; import org.glassfish.internal.api.Globals; @@ -123,6 +125,12 @@ public Object intercept(InvocationContext invocationContext) throws Exception { FallbackPolicy fallbackPolicy = new FallbackPolicy(fallback, config, invocationContext); proceededInvocationContext = fallbackPolicy.fallback(invocationContext, ex); } else { + // Increment the failure counter metric + MetricRegistry metricRegistry = CDI.current().select(MetricRegistry.class).get(); + String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, + Fallback.class); + metricRegistry.counter("ft." + fullMethodSignature + ".invocations.failed.total").inc(); + throw ex; } } @@ -146,8 +154,16 @@ private Object retry(InvocationContext invocationContext) throws Exception { InvocationManager invocationManager = Globals.getDefaultBaseServiceLocator() .getService(InvocationManager.class); + MetricRegistry metricRegistry = CDI.current().select(MetricRegistry.class).get(); + String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, + Retry.class); + + // Increment the invocations metric + metricRegistry.counter("ft." + fullMethodSignature + ".invocations.total").inc(); + try { proceededInvocationContext = invocationContext.proceed(); + metricRegistry.counter("ft." + fullMethodSignature + ".retry.callsSucceededNotRetried.total").inc(); } catch (Exception ex) { Config config = null; try { @@ -243,8 +259,11 @@ private Object retry(InvocationContext invocationContext) throws Exception { while (System.currentTimeMillis() < timeoutTime) { try { proceededInvocationContext = invocationContext.proceed(); + metricRegistry.counter("ft." + fullMethodSignature + ".retry.callsSucceededRetried.total") + .inc(); break; } catch (Exception caughtException) { + metricRegistry.counter("ft." + fullMethodSignature + ".retry.retries.total").inc(); retryException = caughtException; if (!shouldRetry(retryOn, abortOn, caughtException)) { break; @@ -266,8 +285,11 @@ private Object retry(InvocationContext invocationContext) throws Exception { while (true) { try { proceededInvocationContext = invocationContext.proceed(); + metricRegistry.counter("ft." + fullMethodSignature + ".retry.callsSucceededRetried.total") + .inc(); break; } catch (Exception caughtException) { + metricRegistry.counter("ft." + fullMethodSignature + ".retry.retries.total").inc(); retryException = caughtException; if (!shouldRetry(retryOn, abortOn, caughtException)) { break; @@ -291,8 +313,11 @@ private Object retry(InvocationContext invocationContext) throws Exception { while (maxRetries > 0 && System.currentTimeMillis() < timeoutTime) { try { proceededInvocationContext = invocationContext.proceed(); + metricRegistry.counter("ft." + fullMethodSignature + ".retry.callsSucceededRetried.total") + .inc(); break; } catch (Exception caughtException) { + metricRegistry.counter("ft." + fullMethodSignature + ".retry.retries.total").inc(); retryException = caughtException; if (!shouldRetry(retryOn, abortOn, caughtException)) { break; @@ -316,8 +341,11 @@ private Object retry(InvocationContext invocationContext) throws Exception { while (maxRetries > 0) { try { proceededInvocationContext = invocationContext.proceed(); + metricRegistry.counter("ft." + fullMethodSignature + ".retry.callsSucceededRetried.total") + .inc(); break; } catch (Exception caughtException) { + metricRegistry.counter("ft." + fullMethodSignature + ".retry.retries.total").inc(); retryException = caughtException; if (!shouldRetry(retryOn, abortOn, caughtException)) { break; @@ -342,6 +370,7 @@ private Object retry(InvocationContext invocationContext) throws Exception { } if (proceededInvocationContext == null) { + metricRegistry.counter("ft." + fullMethodSignature + ".retry.callsFailed.total").inc(); throw retryException; } } diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java index b0b76568afc..13f4f0a266a 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java @@ -52,6 +52,7 @@ import javax.annotation.Priority; import javax.enterprise.concurrent.ManagedScheduledExecutorService; import javax.enterprise.inject.spi.BeanManager; +import javax.enterprise.inject.spi.CDI; import javax.inject.Inject; import javax.interceptor.AroundInvoke; import javax.interceptor.Interceptor; @@ -59,10 +60,13 @@ import javax.naming.NamingException; import org.eclipse.microprofile.config.Config; import org.eclipse.microprofile.config.ConfigProvider; +import org.eclipse.microprofile.faulttolerance.Bulkhead; +import org.eclipse.microprofile.faulttolerance.CircuitBreaker; import org.eclipse.microprofile.faulttolerance.Fallback; import org.eclipse.microprofile.faulttolerance.Retry; import org.eclipse.microprofile.faulttolerance.Timeout; import org.eclipse.microprofile.faulttolerance.exceptions.TimeoutException; +import org.eclipse.microprofile.metrics.MetricRegistry; import org.glassfish.api.invocation.InvocationManager; import org.glassfish.internal.api.Globals; @@ -132,6 +136,12 @@ public Object intercept(InvocationContext invocationContext) throws Exception { FallbackPolicy fallbackPolicy = new FallbackPolicy(fallback, config, invocationContext); proceededInvocationContext = fallbackPolicy.fallback(invocationContext, ex); } else { + // Increment the failure counter metric + MetricRegistry metricRegistry = CDI.current().select(MetricRegistry.class).get(); + String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, + Fallback.class); + metricRegistry.counter("ft." + fullMethodSignature + ".invocations.failed.total").inc(); + throw ex; } } @@ -150,6 +160,17 @@ private Object timeout(InvocationContext invocationContext) throws Exception { Object proceededInvocationContext = null; Timeout timeout = FaultToleranceCdiUtils.getAnnotation(beanManager, Timeout.class, invocationContext); + MetricRegistry metricRegistry = CDI.current().select(MetricRegistry.class).get(); + String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, + Timeout.class); + + // Only increment the invocations metric if the Retry, Bulkhead, and CircuitBreaker annotations aren't present + if (FaultToleranceCdiUtils.getAnnotation(beanManager, Bulkhead.class, invocationContext) == null + && FaultToleranceCdiUtils.getAnnotation(beanManager, Retry.class, invocationContext) == null + && FaultToleranceCdiUtils.getAnnotation(beanManager, CircuitBreaker.class, invocationContext) == null) { + metricRegistry.counter("ft." + fullMethodSignature + ".invocations.total").inc(); + } + Config config = null; try { config = ConfigProvider.getConfig(); @@ -168,6 +189,7 @@ private Object timeout(InvocationContext invocationContext) throws Exception { Future timeoutFuture = null; long timeoutMillis = Duration.of(value, unit).toMillis(); long timeoutTime = System.currentTimeMillis() + timeoutMillis; + long executionStartTime = System.nanoTime(); try { timeoutFuture = startTimeout(timeoutMillis); @@ -175,11 +197,23 @@ private Object timeout(InvocationContext invocationContext) throws Exception { stopTimeout(timeoutFuture); if (System.currentTimeMillis() > timeoutTime) { + // Record the timeout for MP Metrics + metricRegistry.counter("ft." + fullMethodSignature + ".timeout.callsTimedOut.total").inc(); + logger.log(Level.FINE, "Execution timed out"); throw new TimeoutException(); } + + // Record the execution time for MP Metrics + metricRegistry.histogram("ft." + fullMethodSignature + ".timeout.executionDuration").update( + System.nanoTime() - executionStartTime); + // Record the successfuly completion for MP Metrics + metricRegistry.counter("ft." + fullMethodSignature + ".timeout.callsNotTimedOut.total").inc(); } catch (InterruptedException ie) { if (System.currentTimeMillis() > timeoutTime) { + // Record the timeout for MP Metrics + metricRegistry.counter("ft." + fullMethodSignature + ".timeout.callsTimedOut.total").inc(); + logger.log(Level.FINE, "Execution timed out"); throw new TimeoutException(ie); } diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/fallback/FallbackPolicy.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/fallback/FallbackPolicy.java index 5677b90ba93..2b79d49b287 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/fallback/FallbackPolicy.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/fallback/FallbackPolicy.java @@ -53,6 +53,7 @@ import org.eclipse.microprofile.faulttolerance.Fallback; import org.eclipse.microprofile.faulttolerance.FallbackHandler; import javax.enterprise.inject.spi.CDI; +import org.eclipse.microprofile.metrics.MetricRegistry; import org.glassfish.api.invocation.InvocationManager; import org.glassfish.internal.api.Globals; @@ -93,7 +94,11 @@ public Object fallback(InvocationContext invocationContext, Throwable exception) InvocationManager invocationManager = Globals.getDefaultBaseServiceLocator() .getService(InvocationManager.class); faultToleranceService.startFaultToleranceSpan(new RequestTraceSpan("executeFallbackMethod"), - invocationManager, invocationContext); + invocationManager, invocationContext); + + MetricRegistry metricRegistry = CDI.current().select(MetricRegistry.class).get(); + String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, + Fallback.class); try { if (fallbackMethod != null && !fallbackMethod.isEmpty()) { @@ -103,6 +108,7 @@ public Object fallback(InvocationContext invocationContext, Throwable exception) .getAnnotatedMethodClass(invocationContext, Fallback.class) .getDeclaredMethod(fallbackMethod, invocationContext.getMethod().getParameterTypes()) .invoke(invocationContext.getTarget(), invocationContext.getParameters()); + metricRegistry.counter("ft." + fullMethodSignature + ".fallback.calls.total").inc(); } else { logger.log(Level.FINE, "Using fallback class: {0}", fallbackClass.getName()); @@ -112,7 +118,13 @@ public Object fallback(InvocationContext invocationContext, Throwable exception) fallbackInvocationContext = fallbackClass .getDeclaredMethod(FALLBACK_HANDLER_METHOD_NAME, ExecutionContext.class) .invoke(CDI.current().select(fallbackClass).get(), executionContext); + metricRegistry.counter("ft." + fullMethodSignature + ".fallback.calls.total").inc(); } + } catch (Exception ex) { + // Increment the failure counter metric + metricRegistry.counter("ft." + fullMethodSignature + ".invocations.failed.total").inc(); + + throw ex; } finally { faultToleranceService.endFaultToleranceSpan(); } diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/state/CircuitBreakerState.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/state/CircuitBreakerState.java index cf26b32efe1..597be13d96d 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/state/CircuitBreakerState.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/state/CircuitBreakerState.java @@ -59,6 +59,7 @@ public enum CircuitState { private final BlockingQueue closedResultsQueue; private volatile int halfOpenSuccessfulResultsCounter; private volatile CircuitState circuitState = CircuitState.CLOSED; + private volatile long timer = System.nanoTime(); public CircuitBreakerState(int requestVolumeThreshold) { closedResultsQueue = new LinkedBlockingQueue<>(requestVolumeThreshold); @@ -75,7 +76,7 @@ public CircuitState getCircuitState() { /** * Sets the CircuitBreaker state to the provided enum value. - * @param circuitState The state to set the CIrcuitBreaker to. + * @param circuitState The state to set the CircuitBreaker to. */ public void setCircuitState(CircuitState circuitState) { this.circuitState = circuitState; @@ -149,4 +150,12 @@ public Boolean isOverFailureThreshold(long failureThreshold) { return over; } + + public long getTimer() { + return timer; + } + + public void resetTimer() { + timer = System.nanoTime(); + } } From 422845b14fd3c1f9a2eb2319606f442c5cd187a9 Mon Sep 17 00:00:00 2001 From: Andrew Pielage Date: Fri, 24 Aug 2018 17:00:21 +0100 Subject: [PATCH 12/15] PAYARA-2721 Various FT metrics integration fixes --- .../interceptors/BulkheadInterceptor.java | 37 +++---- .../CircuitBreakerInterceptor.java | 100 ++++++------------ .../interceptors/RetryInterceptor.java | 30 ++++-- .../interceptors/TimeoutInterceptor.java | 31 ++++-- .../state/CircuitBreakerState.java | 56 +++++++++- 5 files changed, 142 insertions(+), 112 deletions(-) diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java index 0ccbe675e8a..ccb7a7b42a6 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java @@ -42,7 +42,6 @@ import fish.payara.microprofile.faulttolerance.interceptors.fallback.FallbackPolicy; import fish.payara.microprofile.faulttolerance.FaultToleranceService; import fish.payara.microprofile.faulttolerance.cdi.FaultToleranceCdiUtils; -//import fish.payara.microprofile.metrics.impl.GaugeImpl; import java.io.Serializable; import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; @@ -93,6 +92,10 @@ public Object intercept(InvocationContext invocationContext) throws Exception { InvocationManager invocationManager = Globals.getDefaultBaseServiceLocator() .getService(InvocationManager.class); + MetricRegistry metricRegistry = CDI.current().select(MetricRegistry.class).get(); + String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, + Bulkhead.class); + Config config = null; try { @@ -110,6 +113,11 @@ public Object intercept(InvocationContext invocationContext) throws Exception { && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( config, Bulkhead.class, invocationContext) .orElse(Boolean.TRUE))) { + // Only increment the invocations metric if the Retry annotation isn't present + if (FaultToleranceCdiUtils.getAnnotation(beanManager, Retry.class, invocationContext) == null) { + metricRegistry.counter("ft." + fullMethodSignature + ".invocations.total").inc(); + } + logger.log(Level.FINER, "Proceeding invocation with bulkhead semantics"); proceededInvocationContext = bulkhead(invocationContext); } else { @@ -142,9 +150,6 @@ public Object intercept(InvocationContext invocationContext) throws Exception { logger.log(Level.FINE, "Fallback annotation not found on method, propagating error upwards.", ex); // Increment the failure counter metric - MetricRegistry metricRegistry = CDI.current().select(MetricRegistry.class).get(); - String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, - Fallback.class); metricRegistry.counter("ft." + fullMethodSignature + ".invocations.failed.total").inc(); throw ex; @@ -193,16 +198,11 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, Bulkhead.class); - // Only increment the invocations metric if the Retry annotation isn't present - if (FaultToleranceCdiUtils.getAnnotation(beanManager, Retry.class, invocationContext) == null) { - metricRegistry.counter("ft." + fullMethodSignature + ".invocations.total").inc(); - } - Semaphore bulkheadExecutionSemaphore = faultToleranceService.getBulkheadExecutionSemaphore(appName, invocationContext.getMethod(), value); Gauge concurrentExecutionsGauge = metricRegistry.getGauges() - .get("ft." + fullMethodSignature + ".bulkhead.waitingQueue.population"); + .get("ft." + fullMethodSignature + ".bulkhead.concurrentExecutions"); // Register a bulkhead concurrent executions metric if there isn't one if (concurrentExecutionsGauge == null) { @@ -227,7 +227,10 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { metricRegistry.register("ft." + fullMethodSignature + ".bulkhead.waitingQueue.population", waitingQueueGauge); } - + + // Start measuring the queue duration for MP Metrics + long queueStartTime = System.nanoTime(); + // Check if there are any free permits for concurrent execution if (!bulkheadExecutionSemaphore.tryAcquire(0, TimeUnit.SECONDS)) { logger.log(Level.FINER, "Attempting to acquire bulkhead queue semaphore."); @@ -235,12 +238,6 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { if (bulkheadExecutionQueueSemaphore.tryAcquire(0, TimeUnit.SECONDS)) { logger.log(Level.FINER, "Acquired bulkhead queue semaphore."); - // Update the bulkhead queueing metric -// waitingQueueGauge. - - // Start measuring the queue duration for MP Metrics - long queueStartTime = System.nanoTime(); - // If there is a free queue permit, queue for an executor permit try { logger.log(Level.FINER, "Attempting to acquire bulkhead execution semaphore."); @@ -252,7 +249,7 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { // Make sure we end the trace right here faultToleranceService.endFaultToleranceSpan(); - // Record the execution time for MP Metrics + // Record the queue time for MP Metrics metricRegistry.histogram("ft." + fullMethodSignature + ".bulkhead.waiting.duration").update( System.nanoTime() - queueStartTime); } @@ -309,6 +306,10 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { // Incremement the MP Metrics callsAccepted counter metricRegistry.counter("ft." + fullMethodSignature + ".bulkhead.callsAccepted.total").inc(); + // Record the queue time for MP Metrics + metricRegistry.histogram("ft." + fullMethodSignature + ".bulkhead.waiting.duration").update( + System.nanoTime() - queueStartTime); + // Start measuring the execution duration for MP Metrics long executionStartTime = System.nanoTime(); diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java index e21726ba30e..b24b329f44a 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java @@ -96,6 +96,10 @@ public Object intercept(InvocationContext invocationContext) throws Exception { InvocationManager invocationManager = Globals.getDefaultBaseServiceLocator() .getService(InvocationManager.class); + MetricRegistry metricRegistry = CDI.current().select(MetricRegistry.class).get(); + String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, + CircuitBreaker.class); + Config config = null; try { config = ConfigProvider.getConfig(); @@ -112,6 +116,12 @@ public Object intercept(InvocationContext invocationContext) throws Exception { && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( config, CircuitBreaker.class, invocationContext) .orElse(Boolean.TRUE))) { + // Only increment the invocations metric if the Retry and Bulkhead annotations aren't present + if (FaultToleranceCdiUtils.getAnnotation(beanManager, Bulkhead.class, invocationContext) == null + && FaultToleranceCdiUtils.getAnnotation(beanManager, Retry.class, invocationContext) == null) { + metricRegistry.counter("ft." + fullMethodSignature + ".invocations.total").inc(); + } + logger.log(Level.FINER, "Proceeding invocation with circuitbreaker semantics"); proceededInvocationContext = circuitBreak(invocationContext); } else { @@ -140,9 +150,6 @@ public Object intercept(InvocationContext invocationContext) throws Exception { proceededInvocationContext = fallbackPolicy.fallback(invocationContext, ex); } else { // Increment the failure counter metric - MetricRegistry metricRegistry = CDI.current().select(MetricRegistry.class).get(); - String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, - Fallback.class); metricRegistry.counter("ft." + fullMethodSignature + ".invocations.failed.total").inc(); throw ex; @@ -165,12 +172,6 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, CircuitBreaker.class); - // Only increment the invocations metric if the Retry and Bulkhead annotations aren't present - if (FaultToleranceCdiUtils.getAnnotation(beanManager, Bulkhead.class, invocationContext) == null - && FaultToleranceCdiUtils.getAnnotation(beanManager, Retry.class, invocationContext) == null) { - metricRegistry.counter("ft." + fullMethodSignature + ".invocations.total").inc(); - } - Config config = null; try { config = ConfigProvider.getConfig(); @@ -229,10 +230,10 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio .get("ft." + fullMethodSignature + ".circuitbreaker.open.total"); Gauge halfOpenTimeGauge = metricRegistry.getGauges() - .get("ft." + fullMethodSignature + ".circuitbreaker.open.total"); + .get("ft." + fullMethodSignature + ".circuitbreaker.halfOpen.total"); Gauge closedTimeGauge = metricRegistry.getGauges() - .get("ft." + fullMethodSignature + ".circuitbreaker.open.total"); + .get("ft." + fullMethodSignature + ".circuitbreaker.closed.total"); registerGaugesIfNecessary(openTimeGauge, halfOpenTimeGauge, closedTimeGauge, metricRegistry, circuitBreakerState, fullMethodSignature); @@ -304,10 +305,9 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio metricRegistry.counter("ft." + fullMethodSignature + ".circuitbreaker.callsFailed.total").inc(); // Open the circuit again, and reset the half-open result counter - updateCircuitState(circuitBreakerState, CircuitBreakerState.CircuitState.OPEN, metricRegistry, - fullMethodSignature); + updateCircuitState(circuitBreakerState, CircuitBreakerState.CircuitState.OPEN); circuitBreakerState.resetHalfOpenSuccessfulResultCounter(); - scheduleHalfOpen(delayMillis, circuitBreakerState, metricRegistry, fullMethodSignature); + scheduleHalfOpen(delayMillis, circuitBreakerState); } throw ex; @@ -324,8 +324,7 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio logger.log(Level.FINE, "Number of consecutive successful CircuitBreaker executions is above " + "threshold {0}, closing circuit", successThreshold); - updateCircuitState(circuitBreakerState, CircuitBreakerState.CircuitState.CLOSED, - metricRegistry, fullMethodSignature); + updateCircuitState(circuitBreakerState, CircuitBreakerState.CircuitState.CLOSED); // Reset the counter for when we next need to use it circuitBreakerState.resetHalfOpenSuccessfulResultCounter(); @@ -346,11 +345,9 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio * @param circuitBreakerState The CircuitBreakerState to set the state of * @throws NamingException If the ManagedScheduledExecutor couldn't be found */ - private void scheduleHalfOpen(long delayMillis, CircuitBreakerState circuitBreakerState, - MetricRegistry metricRegistry, String fullMethodSignature) throws NamingException { + private void scheduleHalfOpen(long delayMillis, CircuitBreakerState circuitBreakerState) throws NamingException { Runnable halfOpen = () -> { - updateCircuitState(circuitBreakerState, CircuitBreakerState.CircuitState.HALF_OPEN, metricRegistry, - fullMethodSignature); + updateCircuitState(circuitBreakerState, CircuitBreakerState.CircuitState.HALF_OPEN); logger.log(Level.FINE, "Setting CircuitBreaker state to half open"); }; @@ -408,66 +405,31 @@ private void breakCircuitIfRequired(long failureThreshold, CircuitBreakerState c failureThreshold); // Update the circuit state and metric timers - updateCircuitState(circuitBreakerState, CircuitBreakerState.CircuitState.OPEN, metricRegistry, - fullMethodSignature); + updateCircuitState(circuitBreakerState, CircuitBreakerState.CircuitState.OPEN); // Update the opened metric counter metricRegistry.counter("ft." + fullMethodSignature + ".circuitbreaker.opened.total").inc(); // Kick off a thread that will half-open the circuit after the specified delay - scheduleHalfOpen(delayMillis, circuitBreakerState, metricRegistry, fullMethodSignature); + scheduleHalfOpen(delayMillis, circuitBreakerState); } } private void updateCircuitState(CircuitBreakerState circuitBreakerState, - CircuitBreakerState.CircuitState circuitState, MetricRegistry metricRegistry, String fullMethodSignature) { - switch (circuitBreakerState.getCircuitState()) { - case OPEN: - getTimeOpen(metricRegistry, circuitBreakerState, fullMethodSignature); - break; - case HALF_OPEN: - getTimeHalfOpen(metricRegistry, circuitBreakerState, fullMethodSignature); - break; - case CLOSED: - getTimeClosed(metricRegistry, circuitBreakerState, fullMethodSignature); - break; - } - + CircuitBreakerState.CircuitState circuitState) { circuitBreakerState.setCircuitState(circuitState); - circuitBreakerState.resetTimer(); } - private Long getTimeOpen(MetricRegistry metricRegistry, CircuitBreakerState circuitBreakerState, - String fullMethodSignature) { - if (circuitBreakerState.getCircuitState().equals(CircuitBreakerState.CircuitState.OPEN)) { - return (System.nanoTime() - circuitBreakerState.getTimer()) + (Long) metricRegistry.getGauges() - .get("ft." + fullMethodSignature + ".circuitbreaker.open.total").getValue(); - } else { - return (Long) metricRegistry.getGauges().get("ft." + fullMethodSignature + ".circuitbreaker.open.total") - .getValue(); - } + private Long getTimeOpen(CircuitBreakerState circuitBreakerState) { + return circuitBreakerState.updateAndGetOpenTime(System.nanoTime()); } - private Long getTimeHalfOpen(MetricRegistry metricRegistry, CircuitBreakerState circuitBreakerState, - String fullMethodSignature) { - if (circuitBreakerState.getCircuitState().equals(CircuitBreakerState.CircuitState.HALF_OPEN)) { - return (System.nanoTime() - circuitBreakerState.getTimer()) + (Long) metricRegistry.getGauges() - .get("ft." + fullMethodSignature + ".circuitbreaker.halfOpen.total").getValue(); - } else { - return (Long) metricRegistry.getGauges().get("ft." + fullMethodSignature + ".circuitbreaker.halfOpen.total") - .getValue(); - } + private Long getTimeHalfOpen(CircuitBreakerState circuitBreakerState) { + return circuitBreakerState.updateAndGetHalfOpenTime(System.nanoTime()); } - private Long getTimeClosed(MetricRegistry metricRegistry, CircuitBreakerState circuitBreakerState, - String fullMethodSignature) { - if (circuitBreakerState.getCircuitState().equals(CircuitBreakerState.CircuitState.CLOSED)) { - return (System.nanoTime() - circuitBreakerState.getTimer()) + (Long) metricRegistry.getGauges() - .get("ft." + fullMethodSignature + ".circuitbreaker.closed.total").getValue(); - } else { - return (Long) metricRegistry.getGauges().get("ft." + fullMethodSignature + ".circuitbreaker.closed.total") - .getValue(); - } + private Long getTimeClosed(CircuitBreakerState circuitBreakerState) { + return circuitBreakerState.updateAndGetClosedTime(System.nanoTime()); } private void registerGaugesIfNecessary(Gauge openTimeGauge, Gauge halfOpenTimeGauge, Gauge closedTimeGauge, @@ -475,23 +437,23 @@ private void registerGaugesIfNecessary(Gauge openTimeGauge, Gauge halfOpenTimeGa // Register a open time gauge if there isn't one if (openTimeGauge == null) { - openTimeGauge = () -> getTimeOpen(metricRegistry, circuitBreakerState, fullMethodSignature); + openTimeGauge = () -> getTimeOpen(circuitBreakerState); metricRegistry.register("ft." + fullMethodSignature + ".circuitbreaker.open.total", openTimeGauge); } // Register a open time gauge if there isn't one if (halfOpenTimeGauge == null) { - halfOpenTimeGauge = () -> getTimeHalfOpen(metricRegistry, circuitBreakerState, fullMethodSignature); + halfOpenTimeGauge = () -> getTimeHalfOpen(circuitBreakerState); - metricRegistry.register("ft." + fullMethodSignature + ".circuitbreaker.open.total", halfOpenTimeGauge); + metricRegistry.register("ft." + fullMethodSignature + ".circuitbreaker.halfOpen.total", halfOpenTimeGauge); } // Register a open time gauge if there isn't one if (closedTimeGauge == null) { - closedTimeGauge = () -> getTimeClosed(metricRegistry, circuitBreakerState, fullMethodSignature); + closedTimeGauge = () -> getTimeClosed(circuitBreakerState); - metricRegistry.register("ft." + fullMethodSignature + ".circuitbreaker.open.total", closedTimeGauge); + metricRegistry.register("ft." + fullMethodSignature + ".circuitbreaker.closed.total", closedTimeGauge); } } } diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java index 520042a5cb3..167a1bdd632 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java @@ -90,6 +90,10 @@ public Object intercept(InvocationContext invocationContext) throws Exception { InvocationManager invocationManager = Globals.getDefaultBaseServiceLocator() .getService(InvocationManager.class); + MetricRegistry metricRegistry = CDI.current().select(MetricRegistry.class).get(); + String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, + Retry.class); + Config config = null; try { config = ConfigProvider.getConfig(); @@ -106,6 +110,9 @@ public Object intercept(InvocationContext invocationContext) throws Exception { && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( config, Retry.class, invocationContext) .orElse(Boolean.TRUE))) { + // Increment the invocations metric + metricRegistry.counter("ft." + fullMethodSignature + ".invocations.total").inc(); + logger.log(Level.FINER, "Proceeding invocation with retry semantics"); proceededInvocationContext = retry(invocationContext); } else { @@ -126,9 +133,6 @@ public Object intercept(InvocationContext invocationContext) throws Exception { proceededInvocationContext = fallbackPolicy.fallback(invocationContext, ex); } else { // Increment the failure counter metric - MetricRegistry metricRegistry = CDI.current().select(MetricRegistry.class).get(); - String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, - Fallback.class); metricRegistry.counter("ft." + fullMethodSignature + ".invocations.failed.total").inc(); throw ex; @@ -158,9 +162,6 @@ private Object retry(InvocationContext invocationContext) throws Exception { String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, Retry.class); - // Increment the invocations metric - metricRegistry.counter("ft." + fullMethodSignature + ".invocations.total").inc(); - try { proceededInvocationContext = invocationContext.proceed(); metricRegistry.counter("ft." + fullMethodSignature + ".retry.callsSucceededNotRetried.total").inc(); @@ -252,18 +253,22 @@ private Object retry(InvocationContext invocationContext) throws Exception { faultToleranceService.startFaultToleranceSpan(new RequestTraceSpan("retryMethod"), invocationManager, invocationContext); + + boolean succeeded = false; + try { if (maxRetries == -1 && maxDuration > 0) { logger.log(Level.FINER, "Retrying until maxDuration is breached."); while (System.currentTimeMillis() < timeoutTime) { + metricRegistry.counter("ft." + fullMethodSignature + ".retry.retries.total").inc(); try { proceededInvocationContext = invocationContext.proceed(); + succeeded = true; metricRegistry.counter("ft." + fullMethodSignature + ".retry.callsSucceededRetried.total") .inc(); break; } catch (Exception caughtException) { - metricRegistry.counter("ft." + fullMethodSignature + ".retry.retries.total").inc(); retryException = caughtException; if (!shouldRetry(retryOn, abortOn, caughtException)) { break; @@ -283,13 +288,14 @@ private Object retry(InvocationContext invocationContext) throws Exception { } else if (maxRetries == -1 && maxDuration == 0) { logger.log(Level.INFO, "Retrying potentially forever!"); while (true) { + metricRegistry.counter("ft." + fullMethodSignature + ".retry.retries.total").inc(); try { proceededInvocationContext = invocationContext.proceed(); metricRegistry.counter("ft." + fullMethodSignature + ".retry.callsSucceededRetried.total") .inc(); + succeeded = true; break; } catch (Exception caughtException) { - metricRegistry.counter("ft." + fullMethodSignature + ".retry.retries.total").inc(); retryException = caughtException; if (!shouldRetry(retryOn, abortOn, caughtException)) { break; @@ -311,13 +317,14 @@ private Object retry(InvocationContext invocationContext) throws Exception { "Retrying as long as maxDuration ({0}ms) isn''t breached, and no more than {1} times", new Object[]{Duration.of(maxDuration, durationUnit).toMillis(), maxRetries}); while (maxRetries > 0 && System.currentTimeMillis() < timeoutTime) { + metricRegistry.counter("ft." + fullMethodSignature + ".retry.retries.total").inc(); try { proceededInvocationContext = invocationContext.proceed(); metricRegistry.counter("ft." + fullMethodSignature + ".retry.callsSucceededRetried.total") .inc(); + succeeded = true; break; } catch (Exception caughtException) { - metricRegistry.counter("ft." + fullMethodSignature + ".retry.retries.total").inc(); retryException = caughtException; if (!shouldRetry(retryOn, abortOn, caughtException)) { break; @@ -339,13 +346,14 @@ private Object retry(InvocationContext invocationContext) throws Exception { } else { logger.log(Level.INFO, "Retrying no more than {0} times", maxRetries); while (maxRetries > 0) { + metricRegistry.counter("ft." + fullMethodSignature + ".retry.retries.total").inc(); try { proceededInvocationContext = invocationContext.proceed(); metricRegistry.counter("ft." + fullMethodSignature + ".retry.callsSucceededRetried.total") .inc(); + succeeded = true; break; } catch (Exception caughtException) { - metricRegistry.counter("ft." + fullMethodSignature + ".retry.retries.total").inc(); retryException = caughtException; if (!shouldRetry(retryOn, abortOn, caughtException)) { break; @@ -369,7 +377,7 @@ private Object retry(InvocationContext invocationContext) throws Exception { faultToleranceService.endFaultToleranceSpan(); } - if (proceededInvocationContext == null) { + if (!succeeded) { metricRegistry.counter("ft." + fullMethodSignature + ".retry.callsFailed.total").inc(); throw retryException; } diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java index 13f4f0a266a..d57381dc674 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java @@ -93,6 +93,10 @@ public Object intercept(InvocationContext invocationContext) throws Exception { InvocationManager invocationManager = Globals.getDefaultBaseServiceLocator() .getService(InvocationManager.class); + MetricRegistry metricRegistry = CDI.current().select(MetricRegistry.class).get(); + String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, + Timeout.class); + Config config = null; try { config = ConfigProvider.getConfig(); @@ -109,6 +113,14 @@ public Object intercept(InvocationContext invocationContext) throws Exception { && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( config, Timeout.class, invocationContext) .orElse(Boolean.TRUE))) { + // Only increment the invocations metric if the Retry, Bulkhead, and CircuitBreaker annotations aren't present + if (FaultToleranceCdiUtils.getAnnotation(beanManager, Bulkhead.class, invocationContext) == null + && FaultToleranceCdiUtils.getAnnotation(beanManager, Retry.class, invocationContext) == null + && FaultToleranceCdiUtils.getAnnotation( + beanManager, CircuitBreaker.class, invocationContext) == null) { + metricRegistry.counter("ft." + fullMethodSignature + ".invocations.total").inc(); + } + logger.log(Level.FINER, "Proceeding invocation with timeout semantics"); proceededInvocationContext = timeout(invocationContext); } else { @@ -137,9 +149,6 @@ public Object intercept(InvocationContext invocationContext) throws Exception { proceededInvocationContext = fallbackPolicy.fallback(invocationContext, ex); } else { // Increment the failure counter metric - MetricRegistry metricRegistry = CDI.current().select(MetricRegistry.class).get(); - String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, - Fallback.class); metricRegistry.counter("ft." + fullMethodSignature + ".invocations.failed.total").inc(); throw ex; @@ -164,13 +173,6 @@ private Object timeout(InvocationContext invocationContext) throws Exception { String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, Timeout.class); - // Only increment the invocations metric if the Retry, Bulkhead, and CircuitBreaker annotations aren't present - if (FaultToleranceCdiUtils.getAnnotation(beanManager, Bulkhead.class, invocationContext) == null - && FaultToleranceCdiUtils.getAnnotation(beanManager, Retry.class, invocationContext) == null - && FaultToleranceCdiUtils.getAnnotation(beanManager, CircuitBreaker.class, invocationContext) == null) { - metricRegistry.counter("ft." + fullMethodSignature + ".invocations.total").inc(); - } - Config config = null; try { config = ConfigProvider.getConfig(); @@ -218,6 +220,15 @@ private Object timeout(InvocationContext invocationContext) throws Exception { throw new TimeoutException(ie); } } catch (Exception ex) { + // Deal with cases where someone has caught the thread.interrupt() and thrown the exception as something else + if (ex.getCause() instanceof InterruptedException && System.currentTimeMillis() > timeoutTime) { + // Record the timeout for MP Metrics + metricRegistry.counter("ft." + fullMethodSignature + ".timeout.callsTimedOut.total").inc(); + + logger.log(Level.FINE, "Execution timed out"); + throw new TimeoutException(ex); + } + stopTimeout(timeoutFuture); throw ex; } diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/state/CircuitBreakerState.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/state/CircuitBreakerState.java index 597be13d96d..ed42b21a30e 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/state/CircuitBreakerState.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/state/CircuitBreakerState.java @@ -60,6 +60,9 @@ public enum CircuitState { private volatile int halfOpenSuccessfulResultsCounter; private volatile CircuitState circuitState = CircuitState.CLOSED; private volatile long timer = System.nanoTime(); + private volatile long closedTime = 0; + private volatile long openTime = 0; + private volatile long halfOpenTime = 0; public CircuitBreakerState(int requestVolumeThreshold) { closedResultsQueue = new LinkedBlockingQueue<>(requestVolumeThreshold); @@ -79,6 +82,19 @@ public CircuitState getCircuitState() { * @param circuitState The state to set the CircuitBreaker to. */ public void setCircuitState(CircuitState circuitState) { + switch (this.circuitState) { + case OPEN: + updateOpenTime(System.nanoTime()); + break; + case HALF_OPEN: + updateHalfOpenTime(System.nanoTime()); + break; + case CLOSED: + updateClosedTime(System.nanoTime()); + break; + } + + resetTimer(); this.circuitState = circuitState; } @@ -151,11 +167,43 @@ public Boolean isOverFailureThreshold(long failureThreshold) { return over; } - public long getTimer() { - return timer; + private void resetTimer() { + timer = System.nanoTime(); } - public void resetTimer() { - timer = System.nanoTime(); + public long updateAndGetClosedTime(long nanoseconds) { + if (circuitState.equals(CircuitState.CLOSED)) { + updateClosedTime(nanoseconds); + } + + return closedTime; + } + + private void updateClosedTime(long nanoseconds) { + closedTime =+ (nanoseconds - timer); + } + + public long updateAndGetOpenTime(long nanoseconds) { + if (circuitState.equals(CircuitState.OPEN)) { + updateOpenTime(nanoseconds); + } + + return openTime; + } + + private void updateOpenTime(long nanoseconds) { + openTime =+ (nanoseconds - timer); + } + + public long updateAndGetHalfOpenTime(long nanoseconds) { + if (circuitState.equals(CircuitState.HALF_OPEN)) { + updateHalfOpenTime(nanoseconds); + } + + return halfOpenTime; + } + + private void updateHalfOpenTime(long nanoseconds) { + halfOpenTime =+ (nanoseconds - timer); } } From 6f618efa8a5876270349c234468e9f60eb671264 Mon Sep 17 00:00:00 2001 From: Andrew Pielage Date: Fri, 24 Aug 2018 17:12:40 +0100 Subject: [PATCH 13/15] Remove refactored method I never used --- .../interceptors/CircuitBreakerInterceptor.java | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java index b24b329f44a..ac301a387b8 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java @@ -305,7 +305,7 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio metricRegistry.counter("ft." + fullMethodSignature + ".circuitbreaker.callsFailed.total").inc(); // Open the circuit again, and reset the half-open result counter - updateCircuitState(circuitBreakerState, CircuitBreakerState.CircuitState.OPEN); + circuitBreakerState.setCircuitState(CircuitBreakerState.CircuitState.OPEN); circuitBreakerState.resetHalfOpenSuccessfulResultCounter(); scheduleHalfOpen(delayMillis, circuitBreakerState); } @@ -324,7 +324,7 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio logger.log(Level.FINE, "Number of consecutive successful CircuitBreaker executions is above " + "threshold {0}, closing circuit", successThreshold); - updateCircuitState(circuitBreakerState, CircuitBreakerState.CircuitState.CLOSED); + circuitBreakerState.setCircuitState(CircuitBreakerState.CircuitState.CLOSED); // Reset the counter for when we next need to use it circuitBreakerState.resetHalfOpenSuccessfulResultCounter(); @@ -347,7 +347,7 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio */ private void scheduleHalfOpen(long delayMillis, CircuitBreakerState circuitBreakerState) throws NamingException { Runnable halfOpen = () -> { - updateCircuitState(circuitBreakerState, CircuitBreakerState.CircuitState.HALF_OPEN); + circuitBreakerState.setCircuitState(CircuitBreakerState.CircuitState.HALF_OPEN); logger.log(Level.FINE, "Setting CircuitBreaker state to half open"); }; @@ -405,7 +405,7 @@ private void breakCircuitIfRequired(long failureThreshold, CircuitBreakerState c failureThreshold); // Update the circuit state and metric timers - updateCircuitState(circuitBreakerState, CircuitBreakerState.CircuitState.OPEN); + circuitBreakerState.setCircuitState(CircuitBreakerState.CircuitState.OPEN); // Update the opened metric counter metricRegistry.counter("ft." + fullMethodSignature + ".circuitbreaker.opened.total").inc(); @@ -415,11 +415,6 @@ private void breakCircuitIfRequired(long failureThreshold, CircuitBreakerState c } } - private void updateCircuitState(CircuitBreakerState circuitBreakerState, - CircuitBreakerState.CircuitState circuitState) { - circuitBreakerState.setCircuitState(circuitState); - } - private Long getTimeOpen(CircuitBreakerState circuitBreakerState) { return circuitBreakerState.updateAndGetOpenTime(System.nanoTime()); } From e1f0b5047f9e83fa45da625bca10fe186b8b56e6 Mon Sep 17 00:00:00 2001 From: Andrew Pielage Date: Sat, 25 Aug 2018 15:00:15 +0100 Subject: [PATCH 14/15] PAYARA-2721 Record execution duration of timeouts that fail --- .../faulttolerance/interceptors/TimeoutInterceptor.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java index d57381dc674..440d6a45c01 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java @@ -212,6 +212,10 @@ private Object timeout(InvocationContext invocationContext) throws Exception { // Record the successfuly completion for MP Metrics metricRegistry.counter("ft." + fullMethodSignature + ".timeout.callsNotTimedOut.total").inc(); } catch (InterruptedException ie) { + // Record the execution time for MP Metrics + metricRegistry.histogram("ft." + fullMethodSignature + ".timeout.executionDuration").update( + System.nanoTime() - executionStartTime); + if (System.currentTimeMillis() > timeoutTime) { // Record the timeout for MP Metrics metricRegistry.counter("ft." + fullMethodSignature + ".timeout.callsTimedOut.total").inc(); @@ -220,6 +224,10 @@ private Object timeout(InvocationContext invocationContext) throws Exception { throw new TimeoutException(ie); } } catch (Exception ex) { + // Record the execution time for MP Metrics + metricRegistry.histogram("ft." + fullMethodSignature + ".timeout.executionDuration").update( + System.nanoTime() - executionStartTime); + // Deal with cases where someone has caught the thread.interrupt() and thrown the exception as something else if (ex.getCause() instanceof InterruptedException && System.currentTimeMillis() > timeoutTime) { // Record the timeout for MP Metrics From 67479991dd970de1e5d3ec94d78067c436caa90d Mon Sep 17 00:00:00 2001 From: Andrew Pielage Date: Sat, 25 Aug 2018 17:08:26 +0100 Subject: [PATCH 15/15] PAYARA-2721 Add capability to disable metrics --- .../faulttolerance/FaultToleranceService.java | 63 ++++++++- .../interceptors/BulkheadInterceptor.java | 124 +++++++++++------- .../CircuitBreakerInterceptor.java | 67 ++++++---- .../interceptors/RetryInterceptor.java | 61 +++++---- .../interceptors/TimeoutInterceptor.java | 44 +++++-- .../interceptors/fallback/FallbackPolicy.java | 18 ++- 6 files changed, 265 insertions(+), 112 deletions(-) diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/FaultToleranceService.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/FaultToleranceService.java index 53778fe36c5..e50d2092014 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/FaultToleranceService.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/FaultToleranceService.java @@ -59,6 +59,8 @@ import javax.naming.NamingException; import org.eclipse.microprofile.config.Config; import org.eclipse.microprofile.faulttolerance.CircuitBreaker; +import org.eclipse.microprofile.metrics.Gauge; +import org.eclipse.microprofile.metrics.MetricRegistry; import org.glassfish.api.StartupRunLevel; import org.glassfish.api.admin.ServerEnvironment; import org.glassfish.api.event.EventListener; @@ -82,6 +84,7 @@ public class FaultToleranceService implements EventListener { public static final String FAULT_TOLERANCE_ENABLED_PROPERTY = "MP_Fault_Tolerance_NonFallback_Enabled"; + public static final String METRICS_ENABLED_PROPERTY = "MP_Fault_Tolerance_Metrics_Enabled"; public static final String FALLBACK_HANDLER_METHOD_NAME = "handle"; private static final Logger logger = Logger.getLogger(FaultToleranceService.class.getName()); @@ -101,12 +104,14 @@ public class FaultToleranceService implements EventListener { Events events; private final Map enabledMap; + private final Map metricsEnabledMap; private final Map> bulkheadExecutionSemaphores; private final Map> bulkheadExecutionQueueSemaphores; private final Map> circuitBreakerStates; public FaultToleranceService() { enabledMap = new ConcurrentHashMap<>(); + metricsEnabledMap = new ConcurrentHashMap<>(); bulkheadExecutionSemaphores = new ConcurrentHashMap<>(); bulkheadExecutionQueueSemaphores = new ConcurrentHashMap<>(); circuitBreakerStates = new ConcurrentHashMap<>(); @@ -147,7 +152,20 @@ public Boolean isFaultToleranceEnabled(String applicationName, Config config) { setFaultToleranceEnabled(applicationName, config); return enabledMap.get(applicationName); } - + } + + public Boolean isFaultToleranceMetricsEnabled(String applicationName, Config config) { + try { + if (metricsEnabledMap.containsKey(applicationName)) { + return metricsEnabledMap.get(applicationName); + } else { + setFaultToleranceMetricsEnabled(applicationName, config); + return metricsEnabledMap.get(applicationName); + } + } catch (NullPointerException npe) { + setFaultToleranceMetricsEnabled(applicationName, config); + return metricsEnabledMap.get(applicationName); + } } /** @@ -184,6 +202,27 @@ private synchronized void setFaultToleranceEnabled(String applicationName, Confi } } + /** + * Helper method that sets the enabled status for a given application. + * @param applicationName The name of the application to register + * @param config The config to check for override values from + */ + private synchronized void setFaultToleranceMetricsEnabled(String applicationName, Config config) { + // Double lock as multiple methods can get inside the calling if at the same time + logger.log(Level.FINER, "Checking double lock to see if something else has added the application"); + if (!metricsEnabledMap.containsKey(applicationName)) { + if (config != null) { + // Set the enabled value to the override value from the config, or true if it isn't configured + metricsEnabledMap.put(applicationName, + config.getOptionalValue(METRICS_ENABLED_PROPERTY, Boolean.class).orElse(Boolean.TRUE)); + } else { + logger.log(Level.FINE, "No config found, so enabling fault tolerance metrics for application: {0}", + applicationName); + metricsEnabledMap.put(applicationName, Boolean.TRUE); + } + } + } + /** * Gets the configured ManagedExecutorService. * @return The configured ManagedExecutorService, or the default ManagedExecutorService if the configured one @@ -453,6 +492,7 @@ private synchronized CircuitBreakerState registerCircuitBreaker(String applicati */ private void deregisterApplication(String applicationName) { enabledMap.remove(applicationName); + metricsEnabledMap.remove(applicationName); deregisterCircuitBreaker(applicationName); deregisterBulkhead(applicationName); } @@ -547,4 +587,25 @@ private void addGenericFaultToleranceRequestTracingDetails(RequestTraceSpan span span.addSpanTag("Class Name", invocationContext.getMethod().getDeclaringClass().getName()); span.addSpanTag("Method Name", invocationContext.getMethod().getName()); } + + public void incrementCounterMetric(MetricRegistry metricRegistry, String metricName, String applicationName, + Config config) { + if (isFaultToleranceMetricsEnabled(applicationName, config)) { + metricRegistry.counter(metricName).inc(); + } + } + + public void updateHistogramMetric(MetricRegistry metricRegistry, String metricName, int value, + String applicationName, Config config) { + if (isFaultToleranceMetricsEnabled(applicationName, config)) { + metricRegistry.histogram(metricName).update(value); + } + } + + public void updateHistogramMetric(MetricRegistry metricRegistry, String metricName, long value, + String applicationName, Config config) { + if (isFaultToleranceMetricsEnabled(applicationName, config)) { + metricRegistry.histogram(metricName).update(value); + } + } } diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java index ccb7a7b42a6..acf89334c3d 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/BulkheadInterceptor.java @@ -113,11 +113,15 @@ public Object intercept(InvocationContext invocationContext) throws Exception { && ((Boolean) FaultToleranceCdiUtils.getEnabledOverrideValue( config, Bulkhead.class, invocationContext) .orElse(Boolean.TRUE))) { - // Only increment the invocations metric if the Retry annotation isn't present - if (FaultToleranceCdiUtils.getAnnotation(beanManager, Retry.class, invocationContext) == null) { - metricRegistry.counter("ft." + fullMethodSignature + ".invocations.total").inc(); + if (faultToleranceService.isFaultToleranceMetricsEnabled(appName, config)) { + // Only increment the invocations metric if the Retry annotation isn't present + if (FaultToleranceCdiUtils.getAnnotation(beanManager, Retry.class, invocationContext) == null) { + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".invocations.total", appName, config); + } } + logger.log(Level.FINER, "Proceeding invocation with bulkhead semantics"); proceededInvocationContext = bulkhead(invocationContext); } else { @@ -150,7 +154,10 @@ public Object intercept(InvocationContext invocationContext) throws Exception { logger.log(Level.FINE, "Fallback annotation not found on method, propagating error upwards.", ex); // Increment the failure counter metric - metricRegistry.counter("ft." + fullMethodSignature + ".invocations.failed.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".invocations.failed.total", + faultToleranceService.getApplicationName(invocationManager, invocationContext), + config); throw ex; } @@ -201,31 +208,35 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { Semaphore bulkheadExecutionSemaphore = faultToleranceService.getBulkheadExecutionSemaphore(appName, invocationContext.getMethod(), value); - Gauge concurrentExecutionsGauge = metricRegistry.getGauges() - .get("ft." + fullMethodSignature + ".bulkhead.concurrentExecutions"); - - // Register a bulkhead concurrent executions metric if there isn't one - if (concurrentExecutionsGauge == null) { - concurrentExecutionsGauge = () -> getConcurrentExecutionsCount(value, bulkheadExecutionSemaphore); - - metricRegistry.register("ft." + fullMethodSignature + ".bulkhead.concurrentExecutions", - concurrentExecutionsGauge); + if (faultToleranceService.isFaultToleranceMetricsEnabled(appName, config)) { + Gauge concurrentExecutionsGauge = metricRegistry.getGauges() + .get("ft." + fullMethodSignature + ".bulkhead.concurrentExecutions"); + + // Register a bulkhead concurrent executions metric if there isn't one + if (concurrentExecutionsGauge == null) { + concurrentExecutionsGauge = () -> getConcurrentExecutionsCount(value, bulkheadExecutionSemaphore); + + metricRegistry.register("ft." + fullMethodSignature + ".bulkhead.concurrentExecutions", + concurrentExecutionsGauge); + } } - + // If the Asynchronous annotation is present, use threadpool style, otherwise use semaphore style if (FaultToleranceCdiUtils.getAnnotation(beanManager, Asynchronous.class, invocationContext) != null) { Semaphore bulkheadExecutionQueueSemaphore = faultToleranceService.getBulkheadExecutionQueueSemaphore( appName, invocationContext.getMethod(), waitingTaskQueue); - Gauge waitingQueueGauge = metricRegistry.getGauges() - .get("ft." + fullMethodSignature + ".bulkhead.waitingQueue.population"); + if (faultToleranceService.isFaultToleranceMetricsEnabled(appName, config)) { + Gauge waitingQueueGauge = metricRegistry.getGauges() + .get("ft." + fullMethodSignature + ".bulkhead.waitingQueue.population"); - // Register a bulkhead queue metric if there isn't one - if (waitingQueueGauge == null) { - waitingQueueGauge = () -> getWaitingQueueCount(waitingTaskQueue, bulkheadExecutionQueueSemaphore); + // Register a bulkhead queue metric if there isn't one + if (waitingQueueGauge == null) { + waitingQueueGauge = () -> getWaitingQueueCount(waitingTaskQueue, bulkheadExecutionQueueSemaphore); - metricRegistry.register("ft." + fullMethodSignature + ".bulkhead.waitingQueue.population", - waitingQueueGauge); + metricRegistry.register("ft." + fullMethodSignature + ".bulkhead.waitingQueue.population", + waitingQueueGauge); + } } // Start measuring the queue duration for MP Metrics @@ -250,8 +261,10 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { faultToleranceService.endFaultToleranceSpan(); // Record the queue time for MP Metrics - metricRegistry.histogram("ft." + fullMethodSignature + ".bulkhead.waiting.duration").update( - System.nanoTime() - queueStartTime); + faultToleranceService.updateHistogramMetric(metricRegistry, + "ft." + fullMethodSignature + ".bulkhead.waiting.duration", + System.nanoTime() - queueStartTime, + appName, config); } logger.log(Level.FINER, "Acquired bulkhead queue semaphore."); @@ -260,7 +273,8 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { bulkheadExecutionQueueSemaphore.release(); // Incremement the MP Metrics callsAccepted counter - metricRegistry.counter("ft." + fullMethodSignature + ".bulkhead.callsAccepted.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".bulkhead.callsAccepted.total", appName, config); // Start measuring the execution duration for MP Metrics long executionStartTime = System.nanoTime(); @@ -276,39 +290,49 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { bulkheadExecutionSemaphore.release(); bulkheadExecutionQueueSemaphore.release(); - // Record the execution time for MP Metrics - metricRegistry.histogram("ft." + fullMethodSignature + ".bulkhead.executionDuration").update( - System.nanoTime() - executionStartTime); + // Record the execution time for MP Metrics + faultToleranceService.updateHistogramMetric(metricRegistry, + "ft." + fullMethodSignature + ".bulkhead.executionDuration", + System.nanoTime() - executionStartTime, + appName, config); // Let the exception propagate further up - we just want to release the semaphores throw ex; } - + // Record the execution time for MP Metrics - metricRegistry.histogram("ft." + fullMethodSignature + ".bulkhead.executionDuration").update( - System.nanoTime() - executionStartTime); + faultToleranceService.updateHistogramMetric(metricRegistry, + "ft." + fullMethodSignature + ".bulkhead.executionDuration", + System.nanoTime() - executionStartTime, + appName, config); // Release the execution permit bulkheadExecutionSemaphore.release(); } catch (InterruptedException ex) { // Incremement the MP Metrics callsRejected counter - metricRegistry.counter("ft." + fullMethodSignature + ".bulkhead.callsRejected.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".bulkhead.callsRejected.total", appName, config); logger.log(Level.INFO, "Interrupted acquiring bulkhead semaphore", ex); throw new BulkheadException(ex); } } else { // Incremement the MP Metrics callsRejected counter - metricRegistry.counter("ft." + fullMethodSignature + ".bulkhead.callsRejected.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".bulkhead.callsRejected.total", appName, config); + throw new BulkheadException("No free work or queue permits."); } } else { // Incremement the MP Metrics callsAccepted counter - metricRegistry.counter("ft." + fullMethodSignature + ".bulkhead.callsAccepted.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".bulkhead.callsAccepted.total", appName, config); // Record the queue time for MP Metrics - metricRegistry.histogram("ft." + fullMethodSignature + ".bulkhead.waiting.duration").update( - System.nanoTime() - queueStartTime); + faultToleranceService.updateHistogramMetric(metricRegistry, + "ft." + fullMethodSignature + ".bulkhead.waiting.duration", + System.nanoTime() - queueStartTime, + appName, config); // Start measuring the execution duration for MP Metrics long executionStartTime = System.nanoTime(); @@ -324,16 +348,20 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { bulkheadExecutionSemaphore.release(); // Record the execution time for MP Metrics - metricRegistry.histogram("ft." + fullMethodSignature + ".bulkhead.executionDuration").update( - System.nanoTime() - executionStartTime); + faultToleranceService.updateHistogramMetric(metricRegistry, + "ft." + fullMethodSignature + ".bulkhead.executionDuration", + System.nanoTime() - executionStartTime, + appName, config); // Let the exception propagate further up - we just want to release the semaphores throw ex; } // Record the execution time for MP Metrics - metricRegistry.histogram("ft." + fullMethodSignature + ".bulkhead.executionDuration").update( - System.nanoTime() - executionStartTime); + faultToleranceService.updateHistogramMetric(metricRegistry, + "ft." + fullMethodSignature + ".bulkhead.executionDuration", + System.nanoTime() - executionStartTime, + appName, config); // Release the permit bulkheadExecutionSemaphore.release(); @@ -342,7 +370,8 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { // Try to get an execution permit if (bulkheadExecutionSemaphore.tryAcquire(0, TimeUnit.SECONDS)) { // Incremement the MP Metrics callsAccepted counter - metricRegistry.counter("ft." + fullMethodSignature + ".bulkhead.callsAccepted.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".bulkhead.callsAccepted.total", appName, config); // Start measuring the execution duration for MP Metrics long executionStartTime = System.nanoTime(); @@ -358,22 +387,27 @@ private Object bulkhead(InvocationContext invocationContext) throws Exception { bulkheadExecutionSemaphore.release(); // Record the execution time for MP Metrics - metricRegistry.histogram("ft." + fullMethodSignature + ".bulkhead.executionDuration").update( - System.nanoTime() - executionStartTime); + faultToleranceService.updateHistogramMetric(metricRegistry, + "ft." + fullMethodSignature + ".bulkhead.executionDuration", + System.nanoTime() - executionStartTime, + appName, config); // Let the exception propagate further up - we just want to release the semaphores throw ex; } // Record the execution time for MP Metrics - metricRegistry.histogram("ft." + fullMethodSignature + ".bulkhead.executionDuration").update( - System.nanoTime() - executionStartTime); + faultToleranceService.updateHistogramMetric(metricRegistry, + "ft." + fullMethodSignature + ".bulkhead.executionDuration", + System.nanoTime() - executionStartTime, + appName, config); // Release the permit bulkheadExecutionSemaphore.release(); } else { // Incremement the MP Metrics callsRejected counter - metricRegistry.counter("ft." + fullMethodSignature + ".bulkhead.callsRejected.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".bulkhead.callsRejected.total", appName, config); throw new BulkheadException("No free work permits."); } diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java index ac301a387b8..edcd4e88534 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/CircuitBreakerInterceptor.java @@ -119,7 +119,8 @@ public Object intercept(InvocationContext invocationContext) throws Exception { // Only increment the invocations metric if the Retry and Bulkhead annotations aren't present if (FaultToleranceCdiUtils.getAnnotation(beanManager, Bulkhead.class, invocationContext) == null && FaultToleranceCdiUtils.getAnnotation(beanManager, Retry.class, invocationContext) == null) { - metricRegistry.counter("ft." + fullMethodSignature + ".invocations.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".invocations.total", appName, config); } logger.log(Level.FINER, "Proceeding invocation with circuitbreaker semantics"); @@ -150,7 +151,10 @@ public Object intercept(InvocationContext invocationContext) throws Exception { proceededInvocationContext = fallbackPolicy.fallback(invocationContext, ex); } else { // Increment the failure counter metric - metricRegistry.counter("ft." + fullMethodSignature + ".invocations.failed.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".invocations.failed.total", + faultToleranceService.getApplicationName(invocationManager, invocationContext), + config); throw ex; } @@ -179,6 +183,10 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio logger.log(Level.INFO, "No config could be found", ex); } + String appName = faultToleranceService.getApplicationName( + Globals.getDefaultBaseServiceLocator().getService(InvocationManager.class), + invocationContext); + Class[] failOn = circuitBreaker.failOn(); try { String failOnString = ((String) FaultToleranceCdiUtils.getOverrideValue( @@ -226,23 +234,27 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio faultToleranceService.getApplicationName(invocationManager, invocationContext), invocationContext.getMethod(), circuitBreaker); - Gauge openTimeGauge = metricRegistry.getGauges() - .get("ft." + fullMethodSignature + ".circuitbreaker.open.total"); + if (faultToleranceService.isFaultToleranceMetricsEnabled(appName, config)) { + Gauge openTimeGauge = metricRegistry.getGauges() + .get("ft." + fullMethodSignature + ".circuitbreaker.open.total"); - Gauge halfOpenTimeGauge = metricRegistry.getGauges() - .get("ft." + fullMethodSignature + ".circuitbreaker.halfOpen.total"); + Gauge halfOpenTimeGauge = metricRegistry.getGauges() + .get("ft." + fullMethodSignature + ".circuitbreaker.halfOpen.total"); - Gauge closedTimeGauge = metricRegistry.getGauges() - .get("ft." + fullMethodSignature + ".circuitbreaker.closed.total"); + Gauge closedTimeGauge = metricRegistry.getGauges() + .get("ft." + fullMethodSignature + ".circuitbreaker.closed.total"); - registerGaugesIfNecessary(openTimeGauge, halfOpenTimeGauge, closedTimeGauge, metricRegistry, circuitBreakerState, - fullMethodSignature); + registerGaugesIfNecessary(openTimeGauge, halfOpenTimeGauge, closedTimeGauge, metricRegistry, circuitBreakerState, + fullMethodSignature); + } + switch (circuitBreakerState.getCircuitState()) { case OPEN: logger.log(Level.FINER, "CircuitBreaker is Open, throwing exception"); - metricRegistry.counter("ft." + fullMethodSignature + ".circuitbreaker.callsPrevented.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".circuitbreaker.callsPrevented.total", appName, config); // If open, immediately throw an error throw new CircuitBreakerOpenException("CircuitBreaker for method " @@ -262,15 +274,14 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio // Add a failure result to the queue circuitBreakerState.recordClosedResult(Boolean.FALSE); - metricRegistry.counter("ft." + fullMethodSignature + ".circuitbreaker.callsFailed.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".circuitbreaker.callsFailed.total", appName, config); // Calculate the failure ratio, and if we're over it, open the circuit breakCircuitIfRequired( Math.round(requestVolumeThreshold * failureRatio), - circuitBreakerState, - delayMillis, - metricRegistry, - fullMethodSignature); + circuitBreakerState, delayMillis, metricRegistry, fullMethodSignature, + faultToleranceService, appName, config); } // Finally, propagate the error upwards @@ -279,15 +290,14 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio // If everything is bon, add a success value circuitBreakerState.recordClosedResult(Boolean.TRUE); - metricRegistry.counter("ft." + fullMethodSignature + ".circuitbreaker.callsSucceeded.total").inc(); - + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".circuitbreaker.callsSucceeded.total", appName, config); + // Calculate the failure ratio, and if we're over it, open the circuit breakCircuitIfRequired( Math.round(requestVolumeThreshold * failureRatio), - circuitBreakerState, - delayMillis, - metricRegistry, - fullMethodSignature); + circuitBreakerState, delayMillis, metricRegistry, fullMethodSignature, + faultToleranceService, appName, config); break; case HALF_OPEN: // If half-open, attempt to proceed the invocation context @@ -302,7 +312,8 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio logger.log(Level.FINE, "Caught exception is included in CircuitBreaker failOn, " + "reopening half open circuit"); - metricRegistry.counter("ft." + fullMethodSignature + ".circuitbreaker.callsFailed.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".circuitbreaker.callsFailed.total", appName, config); // Open the circuit again, and reset the half-open result counter circuitBreakerState.setCircuitState(CircuitBreakerState.CircuitState.OPEN); @@ -315,7 +326,9 @@ private Object circuitBreak(InvocationContext invocationContext) throws Exceptio // If the invocation context hasn't thrown an error, record a success circuitBreakerState.incrementHalfOpenSuccessfulResultCounter(); - metricRegistry.counter("ft." + fullMethodSignature + ".circuitbreaker.callsSucceeded.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".circuitbreaker.callsSucceeded.total", appName, config); + logger.log(Level.FINER, "Number of consecutive successful circuitbreaker executions = {0}", circuitBreakerState.getHalfOpenSuccessFulResultCounter()); @@ -398,7 +411,8 @@ private boolean shouldFail(Class[] failOn, Exception ex) { } private void breakCircuitIfRequired(long failureThreshold, CircuitBreakerState circuitBreakerState, - long delayMillis, MetricRegistry metricRegistry, String fullMethodSignature) throws NamingException { + long delayMillis, MetricRegistry metricRegistry, String fullMethodSignature, + FaultToleranceService faultToleranceService, String appName, Config config) throws NamingException { // If we're over the failure threshold, open the circuit if (circuitBreakerState.isOverFailureThreshold(failureThreshold)) { logger.log(Level.FINE, "CircuitBreaker is over failure threshold {0}, opening circuit", @@ -408,7 +422,8 @@ private void breakCircuitIfRequired(long failureThreshold, CircuitBreakerState c circuitBreakerState.setCircuitState(CircuitBreakerState.CircuitState.OPEN); // Update the opened metric counter - metricRegistry.counter("ft." + fullMethodSignature + ".circuitbreaker.opened.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".circuitbreaker.opened.total", appName, config); // Kick off a thread that will half-open the circuit after the specified delay scheduleHalfOpen(delayMillis, circuitBreakerState); diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java index 167a1bdd632..f195d3ce721 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/RetryInterceptor.java @@ -97,8 +97,8 @@ public Object intercept(InvocationContext invocationContext) throws Exception { Config config = null; try { config = ConfigProvider.getConfig(); - } catch (IllegalArgumentException ex) { - logger.log(Level.INFO, "No config could be found", ex); + } catch (IllegalArgumentException iae) { + logger.log(Level.INFO, "No config could be found", iae); } try { @@ -111,7 +111,8 @@ public Object intercept(InvocationContext invocationContext) throws Exception { config, Retry.class, invocationContext) .orElse(Boolean.TRUE))) { // Increment the invocations metric - metricRegistry.counter("ft." + fullMethodSignature + ".invocations.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".invocations.total", appName, config); logger.log(Level.FINER, "Proceeding invocation with retry semantics"); proceededInvocationContext = retry(invocationContext); @@ -133,7 +134,10 @@ public Object intercept(InvocationContext invocationContext) throws Exception { proceededInvocationContext = fallbackPolicy.fallback(invocationContext, ex); } else { // Increment the failure counter metric - metricRegistry.counter("ft." + fullMethodSignature + ".invocations.failed.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".invocations.failed.total", + faultToleranceService.getApplicationName(invocationManager, invocationContext), + config); throw ex; } @@ -161,18 +165,20 @@ private Object retry(InvocationContext invocationContext) throws Exception { MetricRegistry metricRegistry = CDI.current().select(MetricRegistry.class).get(); String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, Retry.class); + String appName = faultToleranceService.getApplicationName(invocationManager, invocationContext); + + Config config = null; + try { + config = ConfigProvider.getConfig(); + } catch (IllegalArgumentException iae) { + logger.log(Level.INFO, "No config could be found", iae); + } try { proceededInvocationContext = invocationContext.proceed(); - metricRegistry.counter("ft." + fullMethodSignature + ".retry.callsSucceededNotRetried.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".retry.callsSucceededNotRetried.total", appName, config); } catch (Exception ex) { - Config config = null; - try { - config = ConfigProvider.getConfig(); - } catch (IllegalArgumentException iae) { - logger.log(Level.INFO, "No config could be found", ex); - } - Class[] retryOn = retry.retryOn(); try { String retryOnString = ((String) FaultToleranceCdiUtils.getOverrideValue( @@ -261,12 +267,13 @@ private Object retry(InvocationContext invocationContext) throws Exception { logger.log(Level.FINER, "Retrying until maxDuration is breached."); while (System.currentTimeMillis() < timeoutTime) { - metricRegistry.counter("ft." + fullMethodSignature + ".retry.retries.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".retry.retries.total", appName, config); try { proceededInvocationContext = invocationContext.proceed(); succeeded = true; - metricRegistry.counter("ft." + fullMethodSignature + ".retry.callsSucceededRetried.total") - .inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".retry.callsSucceededRetried.total", appName, config); break; } catch (Exception caughtException) { retryException = caughtException; @@ -288,11 +295,12 @@ private Object retry(InvocationContext invocationContext) throws Exception { } else if (maxRetries == -1 && maxDuration == 0) { logger.log(Level.INFO, "Retrying potentially forever!"); while (true) { - metricRegistry.counter("ft." + fullMethodSignature + ".retry.retries.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".retry.retries.total", appName, config); try { proceededInvocationContext = invocationContext.proceed(); - metricRegistry.counter("ft." + fullMethodSignature + ".retry.callsSucceededRetried.total") - .inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".retry.callsSucceededRetried.total", appName, config); succeeded = true; break; } catch (Exception caughtException) { @@ -317,11 +325,12 @@ private Object retry(InvocationContext invocationContext) throws Exception { "Retrying as long as maxDuration ({0}ms) isn''t breached, and no more than {1} times", new Object[]{Duration.of(maxDuration, durationUnit).toMillis(), maxRetries}); while (maxRetries > 0 && System.currentTimeMillis() < timeoutTime) { - metricRegistry.counter("ft." + fullMethodSignature + ".retry.retries.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".retry.retries.total", appName, config); try { proceededInvocationContext = invocationContext.proceed(); - metricRegistry.counter("ft." + fullMethodSignature + ".retry.callsSucceededRetried.total") - .inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".retry.callsSucceededRetried.total", appName, config); succeeded = true; break; } catch (Exception caughtException) { @@ -346,11 +355,12 @@ private Object retry(InvocationContext invocationContext) throws Exception { } else { logger.log(Level.INFO, "Retrying no more than {0} times", maxRetries); while (maxRetries > 0) { - metricRegistry.counter("ft." + fullMethodSignature + ".retry.retries.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".retry.retries.total", appName, config); try { proceededInvocationContext = invocationContext.proceed(); - metricRegistry.counter("ft." + fullMethodSignature + ".retry.callsSucceededRetried.total") - .inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".retry.callsSucceededRetried.total", appName, config); succeeded = true; break; } catch (Exception caughtException) { @@ -378,7 +388,8 @@ private Object retry(InvocationContext invocationContext) throws Exception { } if (!succeeded) { - metricRegistry.counter("ft." + fullMethodSignature + ".retry.callsFailed.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".retry.callsFailed.total", appName, config); throw retryException; } } diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java index 440d6a45c01..f287a9ff539 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/TimeoutInterceptor.java @@ -118,7 +118,8 @@ public Object intercept(InvocationContext invocationContext) throws Exception { && FaultToleranceCdiUtils.getAnnotation(beanManager, Retry.class, invocationContext) == null && FaultToleranceCdiUtils.getAnnotation( beanManager, CircuitBreaker.class, invocationContext) == null) { - metricRegistry.counter("ft." + fullMethodSignature + ".invocations.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".invocations.total", appName, config); } logger.log(Level.FINER, "Proceeding invocation with timeout semantics"); @@ -149,7 +150,10 @@ public Object intercept(InvocationContext invocationContext) throws Exception { proceededInvocationContext = fallbackPolicy.fallback(invocationContext, ex); } else { // Increment the failure counter metric - metricRegistry.counter("ft." + fullMethodSignature + ".invocations.failed.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".invocations.failed.total", + faultToleranceService.getApplicationName(invocationManager, invocationContext), + config); throw ex; } @@ -167,11 +171,17 @@ public Object intercept(InvocationContext invocationContext) throws Exception { */ private Object timeout(InvocationContext invocationContext) throws Exception { Object proceededInvocationContext = null; + + FaultToleranceService faultToleranceService = + Globals.getDefaultBaseServiceLocator().getService(FaultToleranceService.class); Timeout timeout = FaultToleranceCdiUtils.getAnnotation(beanManager, Timeout.class, invocationContext); MetricRegistry metricRegistry = CDI.current().select(MetricRegistry.class).get(); String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, Timeout.class); + String appName = faultToleranceService.getApplicationName( + Globals.getDefaultBaseServiceLocator().getService(InvocationManager.class), + invocationContext); Config config = null; try { @@ -200,38 +210,48 @@ private Object timeout(InvocationContext invocationContext) throws Exception { if (System.currentTimeMillis() > timeoutTime) { // Record the timeout for MP Metrics - metricRegistry.counter("ft." + fullMethodSignature + ".timeout.callsTimedOut.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".timeout.callsTimedOut.total", appName, config); logger.log(Level.FINE, "Execution timed out"); throw new TimeoutException(); } // Record the execution time for MP Metrics - metricRegistry.histogram("ft." + fullMethodSignature + ".timeout.executionDuration").update( - System.nanoTime() - executionStartTime); + faultToleranceService.updateHistogramMetric(metricRegistry, + "ft." + fullMethodSignature + ".timeout.executionDuration", + System.nanoTime() - executionStartTime, + appName, config); // Record the successfuly completion for MP Metrics - metricRegistry.counter("ft." + fullMethodSignature + ".timeout.callsNotTimedOut.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".timeout.callsNotTimedOut.total", appName, config); } catch (InterruptedException ie) { // Record the execution time for MP Metrics - metricRegistry.histogram("ft." + fullMethodSignature + ".timeout.executionDuration").update( - System.nanoTime() - executionStartTime); + faultToleranceService.updateHistogramMetric(metricRegistry, + "ft." + fullMethodSignature + ".timeout.executionDuration", + System.nanoTime() - executionStartTime, + appName, config); if (System.currentTimeMillis() > timeoutTime) { // Record the timeout for MP Metrics - metricRegistry.counter("ft." + fullMethodSignature + ".timeout.callsTimedOut.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".timeout.callsTimedOut.total", appName, config); logger.log(Level.FINE, "Execution timed out"); throw new TimeoutException(ie); } } catch (Exception ex) { // Record the execution time for MP Metrics - metricRegistry.histogram("ft." + fullMethodSignature + ".timeout.executionDuration").update( - System.nanoTime() - executionStartTime); + faultToleranceService.updateHistogramMetric(metricRegistry, + "ft." + fullMethodSignature + ".timeout.executionDuration", + System.nanoTime() - executionStartTime, + appName, config); // Deal with cases where someone has caught the thread.interrupt() and thrown the exception as something else if (ex.getCause() instanceof InterruptedException && System.currentTimeMillis() > timeoutTime) { // Record the timeout for MP Metrics - metricRegistry.counter("ft." + fullMethodSignature + ".timeout.callsTimedOut.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".timeout.callsTimedOut.total", appName, config); logger.log(Level.FINE, "Execution timed out"); throw new TimeoutException(ex); diff --git a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/fallback/FallbackPolicy.java b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/fallback/FallbackPolicy.java index 2b79d49b287..94b1424d8e1 100644 --- a/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/fallback/FallbackPolicy.java +++ b/appserver/payara-appserver-modules/microprofile/fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/interceptors/fallback/FallbackPolicy.java @@ -53,6 +53,7 @@ import org.eclipse.microprofile.faulttolerance.Fallback; import org.eclipse.microprofile.faulttolerance.FallbackHandler; import javax.enterprise.inject.spi.CDI; +import org.eclipse.microprofile.config.ConfigProvider; import org.eclipse.microprofile.metrics.MetricRegistry; import org.glassfish.api.invocation.InvocationManager; import org.glassfish.internal.api.Globals; @@ -99,6 +100,14 @@ public Object fallback(InvocationContext invocationContext, Throwable exception) MetricRegistry metricRegistry = CDI.current().select(MetricRegistry.class).get(); String fullMethodSignature = FaultToleranceCdiUtils.getFullAnnotatedMethodSignature(invocationContext, Fallback.class); + String appName = faultToleranceService.getApplicationName(invocationManager, invocationContext); + + Config config = null; + try { + config = ConfigProvider.getConfig(); + } catch (IllegalArgumentException ex) { + logger.log(Level.INFO, "No config could be found", ex); + } try { if (fallbackMethod != null && !fallbackMethod.isEmpty()) { @@ -108,7 +117,8 @@ public Object fallback(InvocationContext invocationContext, Throwable exception) .getAnnotatedMethodClass(invocationContext, Fallback.class) .getDeclaredMethod(fallbackMethod, invocationContext.getMethod().getParameterTypes()) .invoke(invocationContext.getTarget(), invocationContext.getParameters()); - metricRegistry.counter("ft." + fullMethodSignature + ".fallback.calls.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".fallback.calls.total", appName, config); } else { logger.log(Level.FINE, "Using fallback class: {0}", fallbackClass.getName()); @@ -118,11 +128,13 @@ public Object fallback(InvocationContext invocationContext, Throwable exception) fallbackInvocationContext = fallbackClass .getDeclaredMethod(FALLBACK_HANDLER_METHOD_NAME, ExecutionContext.class) .invoke(CDI.current().select(fallbackClass).get(), executionContext); - metricRegistry.counter("ft." + fullMethodSignature + ".fallback.calls.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".fallback.calls.total", appName, config); } } catch (Exception ex) { // Increment the failure counter metric - metricRegistry.counter("ft." + fullMethodSignature + ".invocations.failed.total").inc(); + faultToleranceService.incrementCounterMetric(metricRegistry, + "ft." + fullMethodSignature + ".invocations.failed.total", appName, config); throw ex; } finally {