Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

canSkipMetric not called for counters #78

Open
Trundle opened this issue Oct 23, 2017 · 1 comment
Open

canSkipMetric not called for counters #78

Trundle opened this issue Oct 23, 2017 · 1 comment

Comments

@Trundle
Copy link

Trundle commented Oct 23, 2017

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
Copy link
Contributor

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

arteam pushed a commit to arteam/dropwizard-metrics-influxdb that referenced this issue Oct 20, 2018
This is the same issue as reported in iZettle#78. For some reason,
idle counters still get reported. It would be nice if we could
not send anything from the client during its idle period.
arteam pushed a commit to arteam/dropwizard-metrics-influxdb that referenced this issue Oct 20, 2018
This is the same issue as reported in iZettle#78. For some reason,
idle counters still get reported. It would be nice if we could
not send anything from the client during its idle period.
arteam pushed a commit to arteam/dropwizard-metrics-influxdb that referenced this issue Oct 20, 2018
This is the same issue as reported in iZettle#78. For some reason,
idle counters still get reported. It would be nice if we could
not send anything from the client during its idle period.
xiaodong-izettle pushed a commit that referenced this issue Oct 21, 2018
This is the same issue as reported in #78. For some reason,
idle counters still get reported. It would be nice if we could
not send anything from the client during its idle period.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants