Merge pull request Netflix#1064 from mattrjacobs/safer-enum-reference…

…s-in-metric-consumers Making metric references to enums lazy to prevent NoSuchFieldError, if possible
mikedehaan · Jan 25, 2016 · 7b54e5f · 7b54e5f
2 parents 2bae17d + 848b810
commit 7b54e5f
Show file tree

Hide file tree

Showing 12 changed files with 1,264 additions and 177 deletions.
diff --git a/...ix/hystrix/contrib/codahalemetricspublisher/HystrixCodaHaleMetricsPublisherCollapser.java b/...ix/hystrix/contrib/codahalemetricspublisher/HystrixCodaHaleMetricsPublisherCollapser.java
@@ -22,6 +22,9 @@
 import com.netflix.hystrix.HystrixCollapserProperties;
 import com.netflix.hystrix.strategy.metrics.HystrixMetricsPublisherCollapser;
 import com.netflix.hystrix.util.HystrixRollingNumberEvent;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import rx.functions.Func0;
 
 /**
  * Implementation of {@link HystrixMetricsPublisherCollapser} using Coda Hale Metrics (https://github.com/codahale/metrics)
@@ -33,6 +36,8 @@ public class HystrixCodaHaleMetricsPublisherCollapser implements HystrixMetricsP
     private final MetricRegistry metricRegistry;
     private final String metricType;
 
+    static final Logger logger = LoggerFactory.getLogger(HystrixCodaHaleMetricsPublisherCollapser.class);
+
     public HystrixCodaHaleMetricsPublisherCollapser(HystrixCollapserKey collapserKey, HystrixCollapserMetrics metrics, HystrixCollapserProperties properties, MetricRegistry metricRegistry) {
         this.key = collapserKey;
         this.metrics = metrics;
@@ -41,6 +46,12 @@ public HystrixCodaHaleMetricsPublisherCollapser(HystrixCollapserKey collapserKey
         this.metricType = key.name();
     }
 
+    /**
+     * An implementation note.  If there's a version mismatch between hystrix-core and hystrix-codahale-metrics-publisher,
+     * the code below may reference a HystrixRollingNumberEvent that does not exist in hystrix-core.  If this happens,
+     * a j.l.NoSuchFieldError occurs.  Since this data is not being generated by hystrix-core, it's safe to count it as 0
+     * and we should log an error to get users to update their dependency set.
+     */
     @Override
     public void initialize() {
         // allow monitor to know exactly at what point in time these stats are for so they can be plotted accurately
@@ -52,14 +63,50 @@ public Long getValue() {
         });
 
         // cumulative counts
-        createCumulativeCountForEvent("countRequestsBatched", HystrixRollingNumberEvent.COLLAPSER_REQUEST_BATCHED);
-        createCumulativeCountForEvent("countBatches", HystrixRollingNumberEvent.COLLAPSER_BATCH);
-        createCumulativeCountForEvent("countResponsesFromCache", HystrixRollingNumberEvent.RESPONSE_FROM_CACHE);
+        safelyCreateCumulativeCountForEvent("countRequestsBatched", new Func0<HystrixRollingNumberEvent>() {
+
+            @Override
+            public HystrixRollingNumberEvent call() {
+                return HystrixRollingNumberEvent.COLLAPSER_REQUEST_BATCHED;
+            }
+        });
+        safelyCreateCumulativeCountForEvent("countBatches", new Func0<HystrixRollingNumberEvent>() {
+
+            @Override
+            public HystrixRollingNumberEvent call() {
+                return HystrixRollingNumberEvent.COLLAPSER_BATCH;
+            }
+        });
+        safelyCreateCumulativeCountForEvent("countResponsesFromCache", new Func0<HystrixRollingNumberEvent>() {
+
+            @Override
+            public HystrixRollingNumberEvent call() {
+                return HystrixRollingNumberEvent.RESPONSE_FROM_CACHE;
+            }
+        });
 
         // rolling counts
-        createRollingCountForEvent("rollingRequestsBatched", HystrixRollingNumberEvent.COLLAPSER_REQUEST_BATCHED);
-        createRollingCountForEvent("rollingBatches", HystrixRollingNumberEvent.COLLAPSER_BATCH);
-        createRollingCountForEvent("rollingCountResponsesFromCache", HystrixRollingNumberEvent.RESPONSE_FROM_CACHE);
+        safelyCreateRollingCountForEvent("rollingRequestsBatched", new Func0<HystrixRollingNumberEvent>() {
+
+            @Override
+            public HystrixRollingNumberEvent call() {
+                return HystrixRollingNumberEvent.COLLAPSER_REQUEST_BATCHED;
+            }
+        });
+        safelyCreateRollingCountForEvent("rollingBatches", new Func0<HystrixRollingNumberEvent>() {
+
+            @Override
+            public HystrixRollingNumberEvent call() {
+                return HystrixRollingNumberEvent.COLLAPSER_BATCH;
+            }
+        });
+        safelyCreateRollingCountForEvent("rollingCountResponsesFromCache", new Func0<HystrixRollingNumberEvent>() {
+
+            @Override
+            public HystrixRollingNumberEvent call() {
+                return HystrixRollingNumberEvent.RESPONSE_FROM_CACHE;
+            }
+        });
 
         // batch size metrics
         metricRegistry.register(createMetricName("batchSize_mean"), new Gauge<Integer>() {
@@ -183,7 +230,7 @@ protected String createMetricName(String name) {
         return MetricRegistry.name("", metricType, name);
     }
 
-    protected void createCumulativeCountForEvent(String name, final HystrixRollingNumberEvent event) {
+    protected void createCumulativeCountForEvent(final String name, final HystrixRollingNumberEvent event) {
         metricRegistry.register(createMetricName(name), new Gauge<Long>() {
             @Override
             public Long getValue() {
@@ -192,12 +239,40 @@ public Long getValue() {
         });
     }
 
-    protected void createRollingCountForEvent(String name, final HystrixRollingNumberEvent event) {
+    protected void safelyCreateCumulativeCountForEvent(final String name, final Func0<HystrixRollingNumberEvent> eventThunk) {
+        metricRegistry.register(createMetricName(name), new Gauge<Long>() {
+            @Override
+            public Long getValue() {
+                try {
+                    return metrics.getCumulativeCount(eventThunk.call());
+                } catch (NoSuchFieldError error) {
+                    logger.error("While publishing CodaHale metrics, error looking up eventType for : " + name + ".  Please check that all Hystrix versions are the same!");
+                    return 0L;
+                }
+            }
+        });
+    }
+
+    protected void createRollingCountForEvent(final String name, final HystrixRollingNumberEvent event) {
         metricRegistry.register(createMetricName(name), new Gauge<Long>() {
             @Override
             public Long getValue() {
                 return metrics.getRollingCount(event);
             }
         });
     }
+
+    protected void safelyCreateRollingCountForEvent(final String name, final Func0<HystrixRollingNumberEvent> eventThunk) {
+        metricRegistry.register(createMetricName(name), new Gauge<Long>() {
+            @Override
+            public Long getValue() {
+                try {
+                    return metrics.getRollingCount(eventThunk.call());
+                } catch (NoSuchFieldError error) {
+                    logger.error("While publishing CodaHale metrics, error looking up eventType for : " + name + ".  Please check that all Hystrix versions are the same!");
+                    return 0L;
+                }
+            }
+        });
+    }
 }