iZettle / dropwizard-metrics-influxdb

Dropwizard Metrics v3 InfluxDB
Apache License 2.0
88 stars 37 forks source link

canSkipMetric not called for counters #78

Open Trundle opened 6 years ago

Trundle commented 6 years ago

canSkipMetric is currently not called for counters and hence counters are always reported, even if they didn't change. The following patch fixes it. In case it was an oversight and not intentional, I can also submit it again as pull request.

diff --git a/metrics-influxdb/src/main/java/com/izettle/metrics/influxdb/InfluxDbReporter.java b/metrics-influxdb/src/main/java/com/izettle/metrics/influxdb/InfluxDbReporter.java
index 1be66a7..26a1e1f 100644
--- a/metrics-influxdb/src/main/java/com/izettle/metrics/influxdb/InfluxDbReporter.java
+++ b/metrics-influxdb/src/main/java/com/izettle/metrics/influxdb/InfluxDbReporter.java
@@ -394,6 +394,9 @@ public final class InfluxDbReporter extends ScheduledReporter {
     }

     private void reportCounter(String name, Counter counter, long now) {
+        if (canSkipMetric(name, counter)) {
+            return;
+        }
         Map<String, Object> fields = new HashMap<String, Object>();
         fields.put("count", counter.getCount());
         Map<String, String> tags = new HashMap<String, String>();
diff --git a/metrics-influxdb/src/test/java/com/izettle/metrics/influxdb/InfluxDbReporterTest.java b/metrics-influxdb/src/test/java/com/izettle/metrics/influxdb/InfluxDbReporterTest.java
index 2aca940..d6e6d9a 100644
--- a/metrics-influxdb/src/test/java/com/izettle/metrics/influxdb/InfluxDbReporterTest.java
+++ b/metrics-influxdb/src/test/java/com/izettle/metrics/influxdb/InfluxDbReporterTest.java
@@ -517,6 +517,26 @@ public class InfluxDbReporterTest {
         verify(influxDb, times(1)).appendPoints(Mockito.any(InfluxDbPoint.class));
     }

+    @Test
+    public void shouldSkipIdleCounters() {
+        final InfluxDbReporter skippingReporter = InfluxDbReporter
+                .forRegistry(registry)
+                .convertRatesTo(TimeUnit.SECONDS)
+                .convertDurationsTo(TimeUnit.MILLISECONDS)
+                .filter(MetricFilter.ALL)
+                .withTags(globalTags)
+                .skipIdleMetrics(true)
+                .build(influxDb);
+
+        final Counter counter = mock(Counter.class);
+        when(counter.getCount()).thenReturn(100L);
+
+        skippingReporter.report(map(), map("counter", counter), map(), map(), map());
+        skippingReporter.report(map(), map("counter", counter), map(), map(), map());
+
+        verify(influxDb, times(1)).appendPoints(Mockito.any(InfluxDbPoint.class));
+    }
+
     @Test
     public void shouldCatchExceptions() throws Exception {
         doThrow(ConnectException.class).when(influxDb).flush();
rickard-von-essen-iz commented 6 years ago

@nzroller Any reason this was not added to gauges and counters?