Merge lp:~lucio.torre/txstatsd/add-sli into lp:txstatsd

[Lucio Torre]

SLI metrics to measure service performance.

SLI metrics to measure service performance.

It will produce metrics with counts of values that match conditions, so you can then derive the service level based on that.

You should first use routing to redirect metrics that you are interested in to be of type sli and then add something like this to the configuration:

[plugin_sli]
rules =
   test => red IF below 5
   test => green IF between 0.1 3
   other => red IF above 4
   *t* => yellow IF above 3

[Sidnei da Silva (sidnei)]

Not super happy about the count_* naming. What about:

 count -> count.total
 count_%s -> count.%s

The rules parsing also doesn't look great. Simply using split() like that is bound to give cryptic errors on badly formed rules. I'd replace it by a regex and some strict checking on each of the items with explicit errors raised if one is missing.

review: Needs Fixing

[Lucio Torre (lucio.torre)]

On 06/04/2012 02:05 PM, Sidnei da Silva wrote:
> Review: Needs Fixing
>
> Not super happy about the count_* naming. What about:
>
> count -> count.total
> count_%s -> count.%s
that makes a rule named total collide with the total.

> The rules parsing also doesn't look great. Simply using split() like that is bound to give cryptic errors on badly formed rules. I'd replace it by a regex and some strict checking on each of the items with explicit errors raised if one is missing.
i am not comfortable with regexps enough to do that.

[Sidnei da Silva (sidnei)]

Simply forbid a rule named total?

re.search('(?P<name>[\w]+)\s+IF\s+(?P<condition>(above|between|below))\s+(?P<value1>[\d\.]+)(\s+(?P<value2>[\d\.]+))?', 'green IF between 0.1 3').groupdict()

If re.search() is None: raise ValueError('invalid rule: %r' % rule)
If value2 is None and condition == between: raise ValueError('missing 2nd argument to between condition')
If name == 'total': raise ValueError('total is a reserved name')

What else?

[Lucio Torre (lucio.torre)]

ah, well, i did the regexp thing before seeing your other comment, got over the lazyness.

having total be a reserved word breaks uniformity and removes "count" from the name, that shows that what we are looking at is a count.

[Sidnei da Silva (sidnei)]

review: Approve

[Sidnei da Silva (sidnei)]

Tested IRL, missing '_-' from the regex.

[Sidnei da Silva (sidnei)]

Also, make sure prefix contains a dot:

if prefix:
   prefix += "."

in build_metric.

[lp:~lucio.torre/txstatsd/add-sli]

99. By Lucio Torre on 2012-06-05

report rates not counts, will depend on proper carbon schema config for bigger buckets

100. By Lucio Torre on 2012-06-05

better regexp, make sure prefix has '.'

=== modified file 'twisted/plugins/distinct_plugin.py'
--- twisted/plugins/distinct_plugin.py	2011-11-18 19:42:56 +0000
+++ twisted/plugins/distinct_plugin.py	2012-06-05 16:24:19 +0000
@@ -4,6 +4,7 @@
 from txstatsd.itxstatsd import IMetricFactory
 from txstatsd.metrics.distinctmetric import DistinctMetricReporter
 
+
 class DistinctMetricFactory(object):
     implements(IMetricFactory, IPlugin)
 

=== added file 'twisted/plugins/sli_plugin.py'
--- twisted/plugins/sli_plugin.py	1970-01-01 00:00:00 +0000
+++ twisted/plugins/sli_plugin.py	2012-06-05 16:24:19 +0000
@@ -0,0 +1,75 @@
+# -*- coding: utf-8 *-*
+import fnmatch
+import re
+
+from zope.interface import implements
+
+from twisted.plugin import IPlugin
+from txstatsd.itxstatsd import IMetricFactory
+from txstatsd.metrics.slimetric import (
+    SLIMetricReporter, BetweenCondition, AboveCondition, BelowCondition)
+
+
+class SLIMetricFactory(object):
+    implements(IMetricFactory, IPlugin)
+
+    name = "SLI"
+    metric_type = "sli"
+
+    def __init__(self):
+        self.config = {}
+
+    def build_metric(self, prefix, name, wall_time_func=None):
+        if prefix:
+            if not prefix[-1] == ".":
+                prefix = prefix + "."
+            path = prefix + name
+        else:
+            path = name
+        result = {}
+        for pattern, conditions in self.config.items():
+            if fnmatch.fnmatch(path, pattern):
+                result.update(conditions)
+        return SLIMetricReporter(path, result)
+
+    def configure(self, options):
+        self.section = dict(options.get("plugin_sli", {}))
+
+        rules = self.section.get("rules", None)
+
+        if rules is None:
+            return
+
+        rules = rules.strip()
+        regexp = "([\w\.\*\?\_\-]+) => (\w+) IF (\w+)(.*)"
+        mo = re.compile(regexp)
+        for line_no, rule in enumerate(rules.split("\n")):
+
+            result = mo.match(rule)
+            if result is None:
+                raise TypeError("Did not match rule spec: %s (rule %d: %s)"
+                    % (regexp, line_no, rule))
+
+            head, label, cname, cparams = result.groups()
+            cparams = cparams[1:]
+
+            self.config.setdefault(head, {})
+
+            method = getattr(self, "build_" + cname, None)
+            if method is None:
+                raise TypeError("cannot build condition: %s %s" % (
+                    cname, cparams))
+
+            cobj = method(*cparams.split(" "))
+            self.config[head][label] = cobj
+
+    def build_above(self, value):
+        return AboveCondition(float(value))
+
+    def build_below(self, value):
+        return BelowCondition(float(value))
+
+    def build_between(self, low, hi):
+        return BetweenCondition(float(low), float(hi))
+
+sli_metric_factory = SLIMetricFactory()

=== added file 'txstatsd/metrics/slimetric.py'
--- txstatsd/metrics/slimetric.py	1970-01-01 00:00:00 +0000
+++ txstatsd/metrics/slimetric.py	2012-06-05 16:24:19 +0000
@@ -0,0 +1,61 @@
+# -*- coding: utf-8 *-*
+
+
+class BelowCondition(object):
+
+    def __init__(self, value):
+        self.value = value
+
+    def __call__(self, value):
+        return value < self.value
+
+
+class AboveCondition(object):
+
+    def __init__(self, value):
+        self.value = value
+
+    def __call__(self, value):
+        return value > self.value
+
+class BetweenCondition(object):
+
+    def __init__(self, low, hi):
+        self.low = low
+        self.hi = hi
+
+    def __call__(self, value):
+        return self.low < value < self.hi
+
+
+class SLIMetricReporter(object):
+    def __init__(self, name, conditions):
+        self.name = name
+        self.conditions = conditions
+        self.conditions = conditions
+        self.clear()
+
+    def clear(self):
+        self.counts = dict((k, 0) for k in self.conditions)
+        self.count = 0
+
+    def process(self, fields):
+        self.update(float(fields[0]))
+
+    def update(self, value):
+        self.count += 1
+        for k, condition in self.conditions.items():
+
+            if condition(value):
+                self.counts[k] += 1
+
+    def flush(self, interval, timestamp):
+        metrics = []
+        for item, value in self.counts.items():
+            metrics.append((self.name + ".count_" + item,
+                            value, timestamp))
+        metrics.append((self.name + ".count",
+                            self.count, timestamp))
+
+        self.clear()
+        return metrics

=== modified file 'txstatsd/server/router.py'
--- txstatsd/server/router.py	2012-05-09 08:04:07 +0000
+++ txstatsd/server/router.py	2012-06-05 16:24:19 +0000
@@ -42,6 +42,7 @@
 from txstatsd.server.processor import BaseMessageProcessor
 from txstatsd.client import StatsDClientProtocol, TwistedStatsDClient
 
+
 class StopProcessingException(Exception):
 
     pass

=== added file 'txstatsd/tests/metrics/test_sli.py'
--- txstatsd/tests/metrics/test_sli.py	1970-01-01 00:00:00 +0000
+++ txstatsd/tests/metrics/test_sli.py	2012-06-05 16:24:19 +0000
@@ -0,0 +1,97 @@
+# -*- coding: utf-8 *-*
+import ConfigParser
+from cStringIO import StringIO
+from twisted.trial.unittest import TestCase
+
+from twisted.plugins.sli_plugin import SLIMetricFactory
+from txstatsd.metrics.slimetric import (
+    SLIMetricReporter, BetweenCondition, AboveCondition, BelowCondition)
+from txstatsd import service
+
+
+class TestConditions(TestCase):
+
+    def test_below(self):
+        c = BelowCondition(5)
+        self.assertEquals(c(2), True)
+        self.assertEquals(c(6), False)
+
+    def test_above(self):
+        c = AboveCondition(5)
+        self.assertEquals(c(2), False)
+        self.assertEquals(c(6), True)
+
+    def test_between(self):
+        c = BetweenCondition(2.5, 5)
+        self.assertEquals(c(2), False)
+        self.assertEquals(c(6), False)
+        self.assertEquals(c(2.6), True)
+
+
+class TestMetric(TestCase):
+    def setUp(self):
+        self.sli = SLIMetricReporter('test', {
+                        "red": BelowCondition(5),
+                        "yellow": BelowCondition(3)})
+
+    def test_count_all(self):
+        self.sli.update(1)
+        self.sli.update(1)
+        self.assertEquals(self.sli.count, 2)
+
+    def test_count_threshold(self):
+        self.assertEquals(self.sli.count, 0)
+        self.assertEquals(self.sli.counts["red"], 0)
+        self.assertEquals(self.sli.counts["yellow"], 0)
+        for i in range(1, 7):
+            self.sli.update(i)
+        self.assertEquals(self.sli.count, 6)
+        self.assertEquals(self.sli.counts["red"], 4)
+        self.assertEquals(self.sli.counts["yellow"], 2)
+
+    def test_reports(self):
+        self.test_count_threshold()
+        rows = sorted(self.sli.flush(0, 0))
+        self.assertEquals(
+            [("test.count", 6, 0),
+            ("test.count_red", 4, 0),
+            ("test.count_yellow", 2, 0)],
+            rows)
+
+    def test_clear(self):
+        self.sli.update(1)
+        self.sli.update(1)
+        self.assertEquals(self.sli.count, 2)
+        self.sli.flush(0, 0)
+        self.sli.update(1)
+        self.assertEquals(self.sli.count, 1)
+
+
+class TestFactory(TestCase):
+    def test_configure(self):
+        class TestOptions(service.OptionsGlue):
+            optParameters = [["test", "t", "default", "help"]]
+            config_section = "statsd"
+
+        o = TestOptions()
+        config_file = ConfigParser.RawConfigParser()
+        config_file.readfp(StringIO("[statsd]\n\n[plugin_sli]\n"
+            "rules = \n"
+            "   test_o-k => red IF below 5\n"
+            "   test_o-k => green IF between 0.1 3\n"
+            "   other* => red IF above 4\n"))
+        o.configure(config_file)
+        smf = SLIMetricFactory()
+        smf.configure(o)
+        smr = smf.build_metric("", "test_o-k")
+        rc = smr.conditions["red"]
+        self.assertTrue(isinstance(rc, BelowCondition))
+        self.assertEquals(rc.value, 5)
+        gc = smr.conditions["green"]
+        self.assertTrue(isinstance(gc, BetweenCondition))
+        self.assertEquals(gc.hi, 3)
+        self.assertEquals(gc.low, 0.1)
+        smr = smf.build_metric("", "otherXX")
+        rc = smr.conditions["red"]
+        self.assertTrue(isinstance(rc, AboveCondition))
+        self.assertEquals(rc.value, 4)