[java-opensaml] branch main updated: IDP-2116 - Add metrics for MetadataResolvers

Scott Cantor cantor.2 at osu.edu
Tue May 23 20:04:51 UTC 2023


This is an automated email from the git hooks/post-receive script.

scantor pushed a commit to branch main
in repository java-opensaml.

View the commit online:
http://git.shibboleth.net/view/?p=java-opensaml.git;a=commit;h=577464b0bc72a5d5943de63588b2ff5dc38e8bb2

The following commit(s) were added to refs/heads/main by this push:
     new 577464b0b IDP-2116 - Add metrics for MetadataResolvers
577464b0b is described below

commit 577464b0bc72a5d5943de63588b2ff5dc38e8bb2
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Tue May 23 16:04:49 2023 -0400

    IDP-2116 - Add metrics for MetadataResolvers
    
    https://shibboleth.atlassian.net/browse/IDP-2116
    
    Move metricsBaseName to root of class tree.
    Add refresh timer for reloadable resolvers around new metadata loads.
---
 .../impl/AbstractDynamicMetadataResolver.java      | 26 ++----------------
 .../resolver/impl/AbstractMetadataResolver.java    | 26 ++++++++++++++++++
 .../impl/AbstractReloadingMetadataResolver.java    | 32 ++++++++++++++++++++++
 3 files changed, 60 insertions(+), 24 deletions(-)

diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractDynamicMetadataResolver.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractDynamicMetadataResolver.java
index f0b21e838..5bcb85227 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractDynamicMetadataResolver.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractDynamicMetadataResolver.java
@@ -109,10 +109,7 @@ public abstract class AbstractDynamicMetadataResolver extends AbstractMetadataRe
     
     /** Class logger. */
     @Nonnull private final Logger log = LoggerFactory.getLogger(AbstractDynamicMetadataResolver.class);
-    
-    /** Base name for Metrics instrumentation names. */
-    @NonnullAfterInit private String metricsBaseName;
-    
+        
     /** Metrics Timer for {@link #resolve(CriteriaSet)}. */
     @Nullable private com.codahale.metrics.Timer timerResolve;
     
@@ -570,25 +567,6 @@ public abstract class AbstractDynamicMetadataResolver extends AbstractMetadataRe
         
         cleanupTaskInterval = interval;
     }
-
-    /**
-     * Get the base name for Metrics instrumentation.
-     * 
-     * @return the Metrics base name
-     */
-    @NonnullAfterInit public String getMetricsBaseName() {
-        return metricsBaseName;
-    }
-    
-    /**
-     * Set the base name for Metrics instrumentation.
-     * 
-     * @param baseName the Metrics base name
-     */
-    public void setMetricsBaseName(@Nullable final String baseName) {
-        checkSetterPreconditions();
-        metricsBaseName = StringSupport.trimOrNull(baseName);
-    }
     
     /**
      * Get the configured indexes.
@@ -1326,7 +1304,7 @@ public abstract class AbstractDynamicMetadataResolver extends AbstractMetadataRe
      */
     private void initializeMetricsInstrumentation() {
         if (getMetricsBaseName() == null) {
-            setMetricsBaseName(MetricRegistry.name(this.getClass(), getId()));
+            setMetricsBaseName(MetricRegistry.name(getClass().getName(), getId()));
         }
         
         final MetricRegistry metricRegistry = MetricsSupport.getMetricRegistry();
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractMetadataResolver.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractMetadataResolver.java
index bf66ca4f8..51bd69c57 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractMetadataResolver.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractMetadataResolver.java
@@ -81,6 +81,9 @@ public abstract class AbstractMetadataResolver extends AbstractIdentifiableIniti
     /** Resolver type. */
     @Nullable @NotEmpty private String resolverType;
     
+    /** Base name for Metrics instrumentation names. */
+    @NonnullAfterInit private String metricsBaseName;
+    
     /** Unmarshaller factory used to get an unmarshaller for the metadata DOM. */
     @NonnullAfterInit private UnmarshallerFactory unmarshallerFactory;
 
@@ -143,6 +146,29 @@ public abstract class AbstractMetadataResolver extends AbstractIdentifiableIniti
         resolverType = StringSupport.trimOrNull(type);
     }
     
+    /**
+     * Get the base name for Metrics instrumentation.
+     * 
+     * @return the Metrics base name
+     * 
+     * @since 5.0.0
+     */
+    @NonnullAfterInit public String getMetricsBaseName() {
+        return metricsBaseName;
+    }
+    
+    /**
+     * Set the base name for Metrics instrumentation.
+     * 
+     * @param baseName the Metrics base name
+     * 
+     * @since 5.0.0
+     */
+    public void setMetricsBaseName(@Nullable final String baseName) {
+        checkSetterPreconditions();
+        metricsBaseName = StringSupport.trimOrNull(baseName);
+    }
+    
     /** {@inheritDoc} */
     @Override public boolean isRequireValidMetadata() {
         return requireValidMetadata;
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractReloadingMetadataResolver.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractReloadingMetadataResolver.java
index 2abb664f5..8de994b1e 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractReloadingMetadataResolver.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractReloadingMetadataResolver.java
@@ -30,6 +30,7 @@ import java.util.TimerTask;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
+import org.opensaml.core.metrics.MetricsSupport;
 import org.opensaml.core.xml.XMLObject;
 import org.opensaml.core.xml.io.UnmarshallingException;
 import org.opensaml.saml.metadata.resolver.RefreshableMetadataResolver;
@@ -40,6 +41,10 @@ import org.slf4j.Logger;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer.Context;
+
+import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.component.ComponentInitializationException;
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.LoggerFactory;
@@ -63,6 +68,9 @@ import net.shibboleth.shared.resolver.ResolverException;
 public abstract class AbstractReloadingMetadataResolver extends AbstractBatchMetadataResolver 
         implements RefreshableMetadataResolver {
 
+    /** Metric name for the timer for {@link #refresh()}. */
+    @Nonnull @NotEmpty public static final String METRIC_TIMER_REFRESH = "timer.refresh";
+
     /** Class logger. */
     @Nonnull private final Logger log = LoggerFactory.getLogger(AbstractReloadingMetadataResolver.class);
 
@@ -114,6 +122,8 @@ public abstract class AbstractReloadingMetadataResolver extends AbstractBatchMet
     /** Reason for the failure of the last refresh.  Will be null if last refresh was success. */
     @Nullable private Throwable lastFailureCause;
 
+    /** Metrics Timer for {@link #refresh()}. */
+    @Nullable private com.codahale.metrics.Timer timerRefresh;
 
     /** Constructor. */
     protected AbstractReloadingMetadataResolver() {
@@ -302,6 +312,7 @@ public abstract class AbstractReloadingMetadataResolver extends AbstractBatchMet
             taskTimer.cancel();
         }
         
+        timerRefresh = null;
         expirationTime = null;
         lastRefresh = null;
         lastUpdate = null;
@@ -318,6 +329,16 @@ public abstract class AbstractReloadingMetadataResolver extends AbstractBatchMet
     protected void initMetadataResolver() throws ComponentInitializationException {
         super.initMetadataResolver();
         
+        if (getMetricsBaseName() == null) {
+            setMetricsBaseName(MetricRegistry.name(getClass().getName(), getId()));
+        }
+
+        final MetricRegistry metricRegistry = MetricsSupport.getMetricRegistry();
+        if (metricRegistry != null) {
+            timerRefresh = metricRegistry.timer(
+                    MetricRegistry.name(getMetricsBaseName(), METRIC_TIMER_REFRESH));
+        }
+        
         try {
             refresh();
         } catch (final ResolverException e) {
@@ -330,6 +351,7 @@ public abstract class AbstractReloadingMetadataResolver extends AbstractBatchMet
         }
     }
 
+// Checkstyle: MethodLength OFF
     /**
      * Refreshes the metadata from its source.
      * 
@@ -341,7 +363,9 @@ public abstract class AbstractReloadingMetadataResolver extends AbstractBatchMet
         String mdId = null;
         trackRefreshSuccess = false;
 
+        Context contextRefresh = null;
         try {
+
             // In case a destroy() thread beat this thread into the monitor.
             if (isDestroyed()) {
                 return;
@@ -364,6 +388,8 @@ public abstract class AbstractReloadingMetadataResolver extends AbstractBatchMet
                 processCachedMetadata(mdId, now);
             } else {
                 log.debug("{} Processing new metadata from '{}'", getLogPrefix(), mdId);
+                // Start timer for refresh.
+                contextRefresh = MetricsSupport.startTimer(timerRefresh);
                 processNewMetadata(mdId, now, mdBytes);
             }
         } catch (final Throwable t) {
@@ -377,6 +403,11 @@ public abstract class AbstractReloadingMetadataResolver extends AbstractBatchMet
             throw new ResolverException(String.format("Saw an error of type '%s' with message '%s'", 
                     t.getClass().getName(), t.getMessage()));
         } finally {
+            // Close out timer if started.
+            if (contextRefresh != null) {
+                MetricsSupport.stopTimer(contextRefresh);
+            }
+            
             if (now != null) {
                 logCachedMetadataExpiration(now);
             }
@@ -400,6 +431,7 @@ public abstract class AbstractReloadingMetadataResolver extends AbstractBatchMet
             lastRefresh = now;
         }
     }
+// Checkstyle: MethodLength ON
 
     /**
      * Check cached metadata for expiration or pending expiration and log appropriately.

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list