ensure that 0-valued metrics are not collected if their reset_on_collect flag is set (#615)

Author: beniwohli

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## Unreleased
+[Check the diff](https://github.com/elastic/apm-agent-python/compare/v5.2.2...master)
+
+### Bugfixes
+
+ * ensure that metrics with value 0 are not collected if they have the `reset_on_collect` flag set (#615)
+
 ## v5.2.2
 [Check the diff](https://github.com/elastic/apm-agent-python/compare/v5.2.1...v5.2.2)
 

elasticapm/metrics/base_metrics.py

@@ -198,21 +198,26 @@ def collect(self):
         if self._counters:
             for (name, labels), c in compat.iteritems(self._counters):
                 if c is not noop_metric:
-                    samples[labels].update({name: {"value": c.val}})
+                    val = c.val
+                    if val or not c.reset_on_collect:
+                        samples[labels].update({name: {"value": val}})
                     if c.reset_on_collect:
                         c.reset()
         if self._gauges:
             for (name, labels), g in compat.iteritems(self._gauges):
                 if g is not noop_metric:
-                    samples[labels].update({name: {"value": g.val}})
+                    val = g.val
+                    if val or not g.reset_on_collect:
+                        samples[labels].update({name: {"value": val}})
                     if g.reset_on_collect:
                         g.reset()
         if self._timers:
             for (name, labels), t in compat.iteritems(self._timers):
                 if t is not noop_metric:
                     val, count = t.val
-                    samples[labels].update({name + ".sum.us": {"value": int(val * 1000000)}})
-                    samples[labels].update({name + ".count": {"value": count}})
+                    if val or not t.reset_on_collect:
+                        samples[labels].update({name + ".sum.us": {"value": int(val * 1000000)}})
+                        samples[labels].update({name + ".count": {"value": count}})
                     if t.reset_on_collect:
                         t.reset()
         if samples:

tests/metrics/base_tests.py

@@ -163,3 +163,33 @@ def test_metric_limit(caplog):
     assert len(caplog.records) == 1
     record = caplog.records[0]
     assert "The limit of 3 metricsets has been reached" in record.message
+
+
+def test_metrics_not_collected_if_zero_and_reset():
+    m = MetricsSet(MetricsRegistry(0, lambda x: None))
+    counter = m.counter("counter", reset_on_collect=False)
+    resetting_counter = m.counter("resetting_counter", reset_on_collect=True)
+    gauge = m.gauge("gauge", reset_on_collect=False)
+    resetting_gauge = m.gauge("resetting_gauge", reset_on_collect=True)
+    timer = m.timer("timer", reset_on_collect=False)
+    resetting_timer = m.timer("resetting_timer", reset_on_collect=True)
+
+    counter.inc(), resetting_counter.inc()
+    gauge.val = 5
+    resetting_gauge.val = 5
+    timer.update(1, 1)
+    resetting_timer.update(1, 1)
+
+    data = list(m.collect())
+    more_data = list(m.collect())
+    assert set(data[0]["samples"].keys()) == {
+        "counter",
+        "resetting_counter",
+        "gauge",
+        "resetting_gauge",
+        "timer.count",
+        "timer.sum.us",
+        "resetting_timer.count",
+        "resetting_timer.sum.us",
+    }
+    assert set(more_data[0]["samples"].keys()) == {"counter", "gauge", "timer.count", "timer.sum.us"}