Merge lp:~wesmason/txstatsd/fix-counters into lp:txstatsd

Proposed by Wes Mason on 2015-01-09
Status: Merged
Approved by: James Westby on 2015-01-22
Approved revision: 114
Merged at revision: 112
Proposed branch: lp:~wesmason/txstatsd/fix-counters
Merge into: lp:txstatsd
Diff against target: 170 lines (+86/-13)
3 files modified
txstatsd/metrics/countermetric.py (+5/-2)
txstatsd/server/configurableprocessor.py (+16/-6)
txstatsd/tests/test_configurableprocessor.py (+65/-5)
To merge this branch: bzr merge lp:~wesmason/txstatsd/fix-counters
Reviewer Review Type Date Requested Status
James Westby (community) 2015-01-09 Approve on 2015-01-22
Review via email: mp+246021@code.launchpad.net

Commit message

Fix ConfigurableMessageProcessor not processing counter stats correctly, and add rate tests.

Description of the change

This a fix for ConfigurableMessageProcessor to actually update the value for counters rather than overwriting, and flushes correctly too.

Patch by ~james-w, and I've tested it with values from canonistack deployed SSO and and manually with `nc`.

To post a comment you must log in.
James Westby (james-w) wrote :

Hi,

Is there a test in the other processor for the RATE regex matching that we could
co-opt here? Or maybe there's even a way to re-use that code directly.

Thanks,

James

Wes Mason (wesmason) wrote :

Will investigate, I think we discussed that before the Christmas break but I'd forgotten.

Thanks!

Wes Mason (wesmason) wrote :

There's no ready-to-reuse RATE test so working on one.

Wes Mason (wesmason) wrote :

Added tests to ensure rate is picked from stat datagram and that both whole and fractional decimals are parsed (and applied to the rate correctly).

James Westby (james-w) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'txstatsd/metrics/countermetric.py'
2--- txstatsd/metrics/countermetric.py 2012-06-28 17:29:26 +0000
3+++ txstatsd/metrics/countermetric.py 2015-01-22 14:26:14 +0000
4@@ -80,9 +80,12 @@
5 self.prefix = prefix
6 self.count = 0
7
8- def mark(self, value):
9- self.count = value
10+ def update(self, value, rate):
11+ self.count += value * (1 / float(rate))
12
13 def report(self, timestamp):
14 return [(self.prefix + self.name + ".count",
15 math.trunc(self.count), timestamp)]
16+
17+ def reset(self):
18+ self.count = 0
19
20=== modified file 'txstatsd/server/configurableprocessor.py'
21--- txstatsd/server/configurableprocessor.py 2014-10-16 11:13:07 +0000
22+++ txstatsd/server/configurableprocessor.py 2015-01-22 14:26:14 +0000
23@@ -25,7 +25,10 @@
24 from txstatsd.metrics.gaugemetric import GaugeMetricReporter
25 from txstatsd.metrics.metermetric import MeterMetricReporter
26 from txstatsd.metrics.timermetric import TimerMetricReporter
27-from txstatsd.server.processor import MessageProcessor
28+from txstatsd.server.processor import (
29+ MessageProcessor,
30+ RATE,
31+)
32
33
34 class ConfigurableMessageProcessor(MessageProcessor):
35@@ -71,14 +74,20 @@
36 value = float(composite[0])
37 except (TypeError, ValueError):
38 return self.fail(message)
39-
40- self.compose_counter_metric(key, value)
41-
42- def compose_counter_metric(self, key, value):
43+ rate = 1
44+ if len(composite) == 3:
45+ match = RATE.match(composite[2])
46+ if match is None:
47+ return self.fail(message)
48+ rate = match.group(1)
49+
50+ self.compose_counter_metric(key, value, rate)
51+
52+ def compose_counter_metric(self, key, value, rate):
53 if not key in self.counter_metrics:
54 metric = CounterMetricReporter(key, prefix=self.message_prefix)
55 self.counter_metrics[key] = metric
56- self.counter_metrics[key].mark(value)
57+ self.counter_metrics[key].update(value, rate)
58
59 def compose_gauge_metric(self, key, value, delta=False):
60 if not key in self.gauge_metrics:
61@@ -96,6 +105,7 @@
62 def flush_counter_metrics(self, interval, timestamp):
63 for metric in self.counter_metrics.itervalues():
64 messages = metric.report(timestamp)
65+ metric.reset()
66 yield messages
67
68 def flush_gauge_metrics(self, timestamp):
69
70=== modified file 'txstatsd/tests/test_configurableprocessor.py'
71--- txstatsd/tests/test_configurableprocessor.py 2014-10-16 11:29:24 +0000
72+++ txstatsd/tests/test_configurableprocessor.py 2015-01-22 14:26:14 +0000
73@@ -20,7 +20,6 @@
74 # SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
75
76 import time
77-
78 from unittest import TestCase
79
80 from twisted.plugins.distinct_plugin import distinct_metric_factory
81@@ -53,8 +52,7 @@
82 configurable_processor.process("gorets:17|c")
83 messages = list(configurable_processor.flush())
84 self.assertEqual(("test.metric.gorets.count", 17, 42), messages[0])
85- self.assertEqual(("test.metric.statsd.numStats", 1, 42),
86- messages[1])
87+ self.assertEqual(("test.metric.statsd.numStats", 1, 42), messages[1])
88
89 def test_flush_counter_with_internal_prefix(self):
90 """
91@@ -66,8 +64,62 @@
92 configurable_processor.process("gorets:17|c")
93 messages = list(configurable_processor.flush())
94 self.assertEqual(("test.metric.gorets.count", 17, 42), messages[0])
95- self.assertEqual(("statsd.foo.numStats", 1, 42),
96- messages[1])
97+ self.assertEqual(("statsd.foo.numStats", 1, 42), messages[1])
98+
99+ def test_flush_counter_with_empty_prefix_with_rate(self):
100+ """
101+ Ensure no prefix features if none is supplied.
102+ B{Note}: The C{ConfigurableMessageProcessor} reports
103+ the counter value, and not the normalized version as
104+ seen in the StatsD-compliant C{Processor}.
105+ """
106+ configurable_processor = ConfigurableMessageProcessor(
107+ time_function=lambda: 42)
108+
109+ configurable_processor.process("gorets:17|c|@2")
110+ messages = list(configurable_processor.flush())
111+ self.assertEqual(("gorets.count", 8, 42), messages[0])
112+ self.assertEqual(("statsd.numStats", 1, 42), messages[1])
113+
114+ configurable_processor.process("gorets:17|c|@1.5")
115+ messages = list(configurable_processor.flush())
116+ self.assertEqual(("gorets.count", 11, 42), messages[0])
117+ self.assertEqual(("statsd.numStats", 1, 42), messages[1])
118+
119+ def test_flush_counter_with_prefix_with_rate(self):
120+ """
121+ Ensure the prefix features if one is supplied.
122+ """
123+ configurable_processor = ConfigurableMessageProcessor(
124+ time_function=lambda: 42, message_prefix="test.metric")
125+
126+ configurable_processor.process("gorets:17|c|@2")
127+ messages = list(configurable_processor.flush())
128+ self.assertEqual(("test.metric.gorets.count", 8, 42), messages[0])
129+ self.assertEqual(("test.metric.statsd.numStats", 1, 42), messages[1])
130+
131+ configurable_processor.process("gorets:17|c|@1.5")
132+ messages = list(configurable_processor.flush())
133+ self.assertEqual(("test.metric.gorets.count", 11, 42), messages[0])
134+ self.assertEqual(("test.metric.statsd.numStats", 1, 42), messages[1])
135+
136+ def test_flush_counter_with_internal_prefix_with_rate(self):
137+ """
138+ Ensure the prefix features if one is supplied.
139+ """
140+ configurable_processor = ConfigurableMessageProcessor(
141+ time_function=lambda: 42, message_prefix="test.metric",
142+ internal_metrics_prefix="statsd.foo.")
143+
144+ configurable_processor.process("gorets:17|c|@2")
145+ messages = list(configurable_processor.flush())
146+ self.assertEqual(("test.metric.gorets.count", 8, 42), messages[0])
147+ self.assertEqual(("statsd.foo.numStats", 1, 42), messages[1])
148+
149+ configurable_processor.process("gorets:17|c|@1.5")
150+ messages = list(configurable_processor.flush())
151+ self.assertEqual(("test.metric.gorets.count", 11, 42), messages[0])
152+ self.assertEqual(("statsd.foo.numStats", 1, 42), messages[1])
153
154 def test_flush_plugin(self):
155 """
156@@ -178,6 +230,14 @@
157
158 class ComposeTests(TestCase):
159
160+ def test_compose_counter_metric(self):
161+ configurable_processor = ConfigurableMessageProcessor()
162+ configurable_processor.compose_counter_metric('a', 1, 1.0)
163+ configurable_processor.compose_counter_metric('a', 1, 1.0)
164+ self.assertEqual(
165+ configurable_processor.counter_metrics['a'].count,
166+ 2)
167+
168 def test_compose_gauge_metric(self):
169 configurable_processor = ConfigurableMessageProcessor()
170 configurable_processor.compose_gauge_metric('a', 1)

Subscribers

People subscribed via source and target branches

to all changes: