Change RegistryService to singleton, and add unit test

- Remove static from declaration of RegistryService (metrics) Resolves: https://www.pivotaltracker.com/story/show/77113576
fcrepo · Sep 5, 2014 · 10060d7 · 10060d7
1 parent 6b559a5
commit 10060d7
Show file tree

Hide file tree

Showing 15 changed files with 147 additions and 37 deletions.
diff --git a/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/DatastreamImpl.java b/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/DatastreamImpl.java
@@ -19,12 +19,12 @@
 import static org.fcrepo.kernel.impl.services.ServiceHelpers.getNodePropertySize;
 import static org.fcrepo.kernel.impl.utils.FedoraTypesUtils.isFedoraDatastream;
 import static org.fcrepo.kernel.impl.utils.FedoraTypesUtils.isFrozen;
-import static org.fcrepo.metrics.RegistryService.getMetrics;
 import static org.modeshape.jcr.api.JcrConstants.JCR_CONTENT;
 import static org.modeshape.jcr.api.JcrConstants.JCR_DATA;
 import static org.modeshape.jcr.api.JcrConstants.JCR_MIME_TYPE;
 import static org.modeshape.jcr.api.JcrConstants.NT_RESOURCE;
 import static org.slf4j.LoggerFactory.getLogger;
+import org.fcrepo.metrics.RegistryService;
 
 import java.io.InputStream;
 import java.net.URI;
@@ -59,7 +59,7 @@ public class DatastreamImpl extends FedoraResourceImpl implements Datastream {
     private static final Logger LOGGER = getLogger(DatastreamImpl.class);
 
     static final Histogram contentSizeHistogram =
-            getMetrics().histogram(name(DatastreamImpl.class, "content-size"));
+            RegistryService.getInstance().getMetrics().histogram(name(DatastreamImpl.class, "content-size"));
 
     /**
      * The JCR node for this datastream

diff --git a/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/identifiers/UUIDPathMinter.java b/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/identifiers/UUIDPathMinter.java
@@ -19,7 +19,7 @@
 import static com.google.common.base.Joiner.on;
 import static com.google.common.base.Splitter.fixedLength;
 import static java.util.UUID.randomUUID;
-import static org.fcrepo.metrics.RegistryService.getMetrics;
+import org.fcrepo.metrics.RegistryService;
 
 import com.codahale.metrics.Timer;
 
@@ -30,7 +30,7 @@
  */
 public class UUIDPathMinter extends BasePidMinter {
 
-    static final Timer timer = getMetrics().timer(
+    static final Timer timer = RegistryService.getInstance().getMetrics().timer(
             name(UUIDPathMinter.class, "mint"));
 
     private static final int DEFAULT_LENGTH = 2;

diff --git a/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/identifiers/UUIDPidMinter.java b/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/identifiers/UUIDPidMinter.java
@@ -17,7 +17,7 @@
 
 import static com.codahale.metrics.MetricRegistry.name;
 import static java.util.UUID.randomUUID;
-import static org.fcrepo.metrics.RegistryService.getMetrics;
+import org.fcrepo.metrics.RegistryService;
 
 import com.codahale.metrics.Timer;
 
@@ -30,7 +30,7 @@
  */
 public class UUIDPidMinter extends BasePidMinter {
 
-    static final Timer timer = getMetrics().timer(
+    static final Timer timer = RegistryService.getInstance().getMetrics().timer(
             name(UUIDPidMinter.class, "mint"));
 
     /**

diff --git a/...o-kernel-impl/src/main/java/org/fcrepo/kernel/impl/observer/NodeRemovalEventObserver.java b/...o-kernel-impl/src/main/java/org/fcrepo/kernel/impl/observer/NodeRemovalEventObserver.java
@@ -18,8 +18,8 @@
 import static com.codahale.metrics.MetricRegistry.name;
 import static com.google.common.base.Throwables.propagate;
 import static javax.jcr.observation.Event.NODE_REMOVED;
-import static org.fcrepo.metrics.RegistryService.getMetrics;
 import static org.slf4j.LoggerFactory.getLogger;
+import org.fcrepo.metrics.RegistryService;
 
 import javax.annotation.PostConstruct;
 import javax.annotation.PreDestroy;
@@ -51,7 +51,7 @@ public class NodeRemovalEventObserver implements EventListener {
     /**
      * A simple counter of events that pass through this observer
      */
-    static final Counter EVENT_COUNTER = getMetrics().counter(
+    static final Counter EVENT_COUNTER = RegistryService.getInstance().getMetrics().counter(
             name(
             NodeRemovalEventObserver.class, "onEvent"));
 

diff --git a/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/observer/SimpleObserver.java b/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/observer/SimpleObserver.java
@@ -24,8 +24,8 @@
 import static javax.jcr.observation.Event.PROPERTY_ADDED;
 import static javax.jcr.observation.Event.PROPERTY_CHANGED;
 import static javax.jcr.observation.Event.PROPERTY_REMOVED;
-import static org.fcrepo.metrics.RegistryService.getMetrics;
 import static org.slf4j.LoggerFactory.getLogger;
+import  org.fcrepo.metrics.RegistryService;
 
 import java.util.Iterator;
 
@@ -62,7 +62,8 @@ public class SimpleObserver implements EventListener {
     /**
      * A simple counter of events that pass through this observer
      */
-    static final Counter EVENT_COUNTER = getMetrics().counter(name(SimpleObserver.class, "onEvent"));
+    static final Counter EVENT_COUNTER =
+            RegistryService.getInstance().getMetrics().counter(name(SimpleObserver.class, "onEvent"));
 
     static final Integer EVENT_TYPES = NODE_ADDED + NODE_REMOVED + NODE_MOVED + PROPERTY_ADDED + PROPERTY_CHANGED
             + PROPERTY_REMOVED;

diff --git a/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/rdf/impl/RootRdfContext.java b/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/rdf/impl/RootRdfContext.java
@@ -29,8 +29,8 @@
 import static org.fcrepo.kernel.RdfLexicon.REPOSITORY_NAMESPACE;
 import static org.fcrepo.kernel.impl.services.ServiceHelpers.getRepositoryCount;
 import static org.fcrepo.kernel.impl.services.ServiceHelpers.getRepositorySize;
-import static org.fcrepo.metrics.RegistryService.getMetrics;
 import static org.slf4j.LoggerFactory.getLogger;
+import org.fcrepo.metrics.RegistryService;
 
 import java.util.Map;
 import java.util.SortedMap;
@@ -60,6 +60,7 @@
 public class RootRdfContext extends NodeRdfContext {
 
     private static final Logger LOGGER = getLogger(RootRdfContext.class);
+    static final RegistryService registryService = RegistryService.getInstance();
 
     /**
      * Ordinary constructor.
@@ -115,7 +116,7 @@ public RootRdfContext(final Node node, final IdentifierTranslator graphSubjects)
         }
 
         // retrieve the metrics from the service
-        final SortedMap<String, Counter> counters = getMetrics().getCounters();
+        final SortedMap<String, Counter> counters = registryService.getMetrics().getCounters();
         // and add the repository metrics to the RDF model
         if (counters.containsKey("LowLevelStorageService.fixity-check-counter")) {
             b.add(create(subject(), HAS_FIXITY_CHECK_COUNT.asNode(),

diff --git a/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/services/DatastreamServiceImpl.java b/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/services/DatastreamServiceImpl.java
@@ -18,10 +18,10 @@
 import static com.codahale.metrics.MetricRegistry.name;
 import static com.google.common.collect.Collections2.filter;
 import static com.google.common.collect.ImmutableSet.copyOf;
-import static org.fcrepo.metrics.RegistryService.getMetrics;
 import static org.modeshape.jcr.api.JcrConstants.JCR_CONTENT;
 import static org.modeshape.jcr.api.JcrConstants.JCR_DATA;
 import static org.slf4j.LoggerFactory.getLogger;
+import org.fcrepo.metrics.RegistryService;
 
 import java.io.InputStream;
 import java.net.URI;
@@ -62,10 +62,11 @@ public class DatastreamServiceImpl extends AbstractService implements Datastream
     @Autowired(required = false)
     StoragePolicyDecisionPoint storagePolicyDecisionPoint;
 
+    static final RegistryService registryService = RegistryService.getInstance();
     static final Counter fixityCheckCounter
-        = getMetrics().counter(name(DatastreamService.class, "fixity-check-counter"));
+        = registryService.getMetrics().counter(name(DatastreamService.class, "fixity-check-counter"));
 
-    static final Timer timer = getMetrics().timer(
+    static final Timer timer = registryService.getMetrics().timer(
             name(Datastream.class, "fixity-check-time"));
 
     private static final Logger LOGGER = getLogger(DatastreamService.class);

diff --git a/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/services/RepositoryServiceImpl.java b/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/services/RepositoryServiceImpl.java
@@ -25,8 +25,8 @@
 import static org.fcrepo.kernel.RdfLexicon.SEARCH_HAS_MORE;
 import static org.fcrepo.kernel.RdfLexicon.SEARCH_HAS_TOTAL_RESULTS;
 import static org.fcrepo.kernel.impl.services.ServiceHelpers.getRepositoryCount;
-import static org.fcrepo.metrics.RegistryService.getMetrics;
 import static org.slf4j.LoggerFactory.getLogger;
+import org.fcrepo.metrics.RegistryService;
 
 import java.io.File;
 import java.util.Iterator;
@@ -74,7 +74,7 @@ public class RepositoryServiceImpl extends AbstractService implements Repository
 
     private static final Logger LOGGER = getLogger(RepositoryServiceImpl.class);
 
-    private final Timer objectSizeCalculationTimer = getMetrics().timer(
+    private final Timer objectSizeCalculationTimer = RegistryService.getInstance().getMetrics().timer(
             name(RepositoryService.class, "objectSizeCalculation"));
 
     /**

diff --git a/fcrepo-metrics/pom.xml b/fcrepo-metrics/pom.xml
@@ -63,6 +63,10 @@
       <version>1.1.1</version>
       <scope>provided</scope>
     </dependency>
+    <dependency>
+      <groupId>javax.jcr</groupId>
+      <artifactId>jcr</artifactId>
+    </dependency>
   </dependencies>
 
   <build>
@@ -73,4 +77,4 @@
       </plugin>
     </plugins>
   </build>
-</project>
+</project>
diff --git a/fcrepo-metrics/src/main/java/org/fcrepo/metrics/MetricsContextListener.java b/fcrepo-metrics/src/main/java/org/fcrepo/metrics/MetricsContextListener.java
@@ -15,8 +15,6 @@
  */
 package org.fcrepo.metrics;
 
-import static org.fcrepo.metrics.RegistryService.getMetrics;
-
 import javax.servlet.annotation.WebListener;
 
 import com.codahale.metrics.MetricRegistry;
@@ -40,7 +38,7 @@ public class MetricsContextListener extends AdminServletContextListener {
      */
     @Override
     protected MetricRegistry getMetricRegistry() {
-        return getMetrics();
+        return RegistryService.getInstance().getMetrics();
     }
 
     /**

diff --git a/fcrepo-metrics/src/main/java/org/fcrepo/metrics/MetricsProvider.java b/fcrepo-metrics/src/main/java/org/fcrepo/metrics/MetricsProvider.java
@@ -15,10 +15,10 @@
  */
 package org.fcrepo.metrics;
 
-import javax.ws.rs.ext.Provider;
-
 import com.codahale.metrics.jersey.InstrumentedResourceMethodDispatchAdapter;
 
+import javax.ws.rs.ext.Provider;
+
 /**
  * Provide a metrics registry for Jersey instrumenting
  *
@@ -31,6 +31,6 @@ public class MetricsProvider extends InstrumentedResourceMethodDispatchAdapter {
      * Default constructor that provides a MetricsRegistry
      */
     public MetricsProvider() {
-        super(RegistryService.getMetrics());
+        super(RegistryService.getInstance().getMetrics());
     }
 }
diff --git a/fcrepo-metrics/src/main/java/org/fcrepo/metrics/RegistryService.java b/fcrepo-metrics/src/main/java/org/fcrepo/metrics/RegistryService.java
@@ -31,17 +31,41 @@
  * @author cbeer
  * @since Mar 22, 2013
  */
-public abstract class RegistryService {
+public class RegistryService {
 
     private static final MetricRegistry METRICS = getOrCreate("fcrepo-metrics");
 
+    private static volatile RegistryService instance = null;
+
+    private RegistryService() {
+      // New instances should come from the singleton
+    }
+
+    /**
+     * Create the instance
+     * Inspired by: http://en.wikipedia.org/wiki/Double-checked_locking
+     */
+    public static RegistryService getInstance() {
+        RegistryService local = instance;
+        if (null == local) {
+
+            synchronized (METRICS) {
+                local = instance;
+                if (null == local) {
+                    instance = local = new RegistryService();
+                }
+            }
+        }
+        return local;
+    }
+
     /**
      * Get the current registry service
      * 
      * TODO the new upstream SharedMetricRegistries may make this obsolete
      * @return the current registry service
      */
-    public static MetricRegistry getMetrics() {
+    public MetricRegistry getMetrics() {
         return METRICS;
     }
 
@@ -50,7 +74,7 @@ public static MetricRegistry getMetrics() {
      * 
      * @param os
      */
-    public static void dumpMetrics(final PrintStream os) {
+    public void dumpMetrics(final PrintStream os) {
 
         final MetricRegistry registry = getMetrics();
 

diff --git a/fcrepo-metrics/src/main/java/org/fcrepo/metrics/ReporterFactory.java b/fcrepo-metrics/src/main/java/org/fcrepo/metrics/ReporterFactory.java
@@ -20,7 +20,6 @@
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.concurrent.TimeUnit.MINUTES;
 import static java.util.concurrent.TimeUnit.SECONDS;
-import static org.fcrepo.metrics.RegistryService.getMetrics;
 import static org.slf4j.LoggerFactory.getLogger;
 
 import javax.management.MBeanServer;
@@ -41,6 +40,8 @@ public class ReporterFactory {
 
     private static final Logger LOGGER = getLogger(ReporterFactory.class);
 
+    private RegistryService registryService = RegistryService.getInstance();
+
     /**
      * Start a new GraphiteReporter, with reports every minute
      * 
@@ -51,7 +52,7 @@ public class ReporterFactory {
     public GraphiteReporter getGraphiteReporter(final String prefix,
             final Graphite g) {
         final GraphiteReporter reporter =
-                GraphiteReporter.forRegistry(getMetrics()).prefixedWith(prefix)
+                GraphiteReporter.forRegistry(registryService.getMetrics()).prefixedWith(prefix)
                         .convertRatesTo(SECONDS).convertDurationsTo(
                                 MILLISECONDS).filter(ALL).build(g);
         reporter.start(1, MINUTES);
@@ -68,7 +69,7 @@ public GraphiteReporter getGraphiteReporter(final String prefix,
     public JmxReporter getJmxReporter(final String prefix) {
         final MBeanServer mbs = getPlatformMBeanServer();
         final JmxReporter reporter =
-                JmxReporter.forRegistry(getMetrics()).registerWith(mbs)
+                JmxReporter.forRegistry(registryService.getMetrics()).registerWith(mbs)
                         .inDomain("org.fcrepo")
                         .convertDurationsTo(MILLISECONDS).convertRatesTo(
                                 SECONDS).filter(ALL).build();

diff --git a/fcrepo-metrics/src/test/java/org/fcrepo/services/MetricsServiceTest.java b/fcrepo-metrics/src/test/java/org/fcrepo/services/MetricsServiceTest.java
@@ -15,27 +15,23 @@
  */
 package org.fcrepo.services;
 
-import static org.fcrepo.metrics.RegistryService.dumpMetrics;
-
 import java.io.PrintStream;
 
+import org.fcrepo.metrics.RegistryService;
 import org.junit.Test;
 
-import com.codahale.metrics.MetricRegistry;
-
 /**
  * <p>MetricsServiceTest class.</p>
  *
  * @author eddies
  */
 public class MetricsServiceTest {
 
-    MetricRegistry mockMetricRegistry;
-
     PrintStream mockPrintStream;
 
     @Test
     public void testDumpMetrics() {
-        dumpMetrics(mockPrintStream);
+      final RegistryService registryService = RegistryService.getInstance();
+      registryService.dumpMetrics(mockPrintStream);
     }
 }