From 6d74e4662102d306c036f800cbd3a3b58477f2b8 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Mon, 9 Mar 2015 17:01:11 +0000
Subject: Fix tests

---
 tests/handlers/test_presence.py       | 4 ++--
 tests/handlers/test_presencelike.py   | 8 ++++----
 tests/rest/client/v1/test_presence.py | 8 ++++----
 3 files changed, 10 insertions(+), 10 deletions(-)

(limited to 'tests')

diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py
index 6ffc3c99cc..04eba4289e 100644
--- a/tests/handlers/test_presence.py
+++ b/tests/handlers/test_presence.py
@@ -100,7 +100,7 @@ class PresenceTestCase(unittest.TestCase):
         self.room_members = []
 
         room_member_handler = handlers.room_member_handler = Mock(spec=[
-            "get_rooms_for_user",
+            "get_joined_rooms_for_user",
             "get_room_members",
             "fetch_room_distributions_into",
         ])
@@ -111,7 +111,7 @@ class PresenceTestCase(unittest.TestCase):
                 return defer.succeed([self.room_id])
             else:
                 return defer.succeed([])
-        room_member_handler.get_rooms_for_user = get_rooms_for_user
+        room_member_handler.get_joined_rooms_for_user = get_rooms_for_user
 
         def get_room_members(room_id):
             if room_id == self.room_id:
diff --git a/tests/handlers/test_presencelike.py b/tests/handlers/test_presencelike.py
index 18cac9a846..977e832da7 100644
--- a/tests/handlers/test_presencelike.py
+++ b/tests/handlers/test_presencelike.py
@@ -64,7 +64,7 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase):
                 "set_presence_state",
                 "is_presence_visible",
                 "set_profile_displayname",
-                "get_rooms_for_user_where_membership_is",
+                "get_rooms_for_user",
             ]),
             handlers=None,
             resource_for_federation=Mock(),
@@ -124,9 +124,9 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase):
                 self.mock_update_client)
 
         hs.handlers.room_member_handler = Mock(spec=[
-            "get_rooms_for_user",
+            "get_joined_rooms_for_user",
         ])
-        hs.handlers.room_member_handler.get_rooms_for_user = (
+        hs.handlers.room_member_handler.get_joined_rooms_for_user = (
                 lambda u: defer.succeed([]))
 
         # Some local users to test with
@@ -138,7 +138,7 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase):
         self.u_potato = UserID.from_string("@potato:remote")
 
         self.mock_get_joined = (
-            self.datastore.get_rooms_for_user_where_membership_is
+            self.datastore.get_rooms_for_user
         )
 
     @defer.inlineCallbacks
diff --git a/tests/rest/client/v1/test_presence.py b/tests/rest/client/v1/test_presence.py
index 5f2ef64efc..b9c03383a2 100644
--- a/tests/rest/client/v1/test_presence.py
+++ b/tests/rest/client/v1/test_presence.py
@@ -79,13 +79,13 @@ class PresenceStateTestCase(unittest.TestCase):
 
         room_member_handler = hs.handlers.room_member_handler = Mock(
             spec=[
-                "get_rooms_for_user",
+                "get_joined_rooms_for_user",
             ]
         )
 
         def get_rooms_for_user(user):
             return defer.succeed([])
-        room_member_handler.get_rooms_for_user = get_rooms_for_user
+        room_member_handler.get_joined_rooms_for_user = get_rooms_for_user
 
         presence.register_servlets(hs, self.mock_resource)
 
@@ -166,7 +166,7 @@ class PresenceListTestCase(unittest.TestCase):
 
         hs.handlers.room_member_handler = Mock(
             spec=[
-                "get_rooms_for_user",
+                "get_joined_rooms_for_user",
             ]
         )
 
@@ -291,7 +291,7 @@ class PresenceEventStreamTestCase(unittest.TestCase):
                 return ["a-room"]
             else:
                 return []
-        hs.handlers.room_member_handler.get_rooms_for_user = get_rooms_for_user
+        hs.handlers.room_member_handler.get_joined_rooms_for_user = get_rooms_for_user
 
         self.mock_datastore = hs.get_datastore()
         self.mock_datastore.get_app_service_by_token = Mock(return_value=None)
-- 
cgit 1.4.1


From e7420a3bef308e12d2b202c7a2c256d15eee0983 Mon Sep 17 00:00:00 2001
From: "Paul \"LeoNerd\" Evans" <paul@matrix.org>
Date: Tue, 24 Feb 2015 16:58:26 +0000
Subject: Initial tiny attempt at (vectorable) counter metrics

---
 synapse/metrics/metric.py    | 54 +++++++++++++++++++++++++++++++++++++++
 tests/metrics/__init__.py    |  0
 tests/metrics/test_metric.py | 61 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 115 insertions(+)
 create mode 100644 synapse/metrics/metric.py
 create mode 100644 tests/metrics/__init__.py
 create mode 100644 tests/metrics/test_metric.py

(limited to 'tests')

diff --git a/synapse/metrics/metric.py b/synapse/metrics/metric.py
new file mode 100644
index 0000000000..f5a98763cc
--- /dev/null
+++ b/synapse/metrics/metric.py
@@ -0,0 +1,54 @@
+# -*- coding: utf-8 -*-
+# Copyright 2015 OpenMarket Ltd
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+
+class CounterMetric(object):
+
+    def __init__(self, name, keys=[]):
+        self.name = name
+        self.keys = keys # OK not to clone as we never write it
+
+        self.counts = {}
+
+        # Scalar metrics are never empty
+        if not len(keys):
+            self.counts[()] = 0
+
+    def inc(self, *values):
+        if len(values) != len(self.keys):
+            raise ValueError("Expected as many values to inc() as keys (%d)" %
+                (len(self.keys))
+            )
+
+        # TODO: should assert that the tag values are all strings
+
+        if values not in self.counts:
+            self.counts[values] = 1
+        else:
+            self.counts[values] += 1
+
+    def fetch(self):
+        return dict(self.counts)
+
+    def _render_key(self, values):
+        # TODO: some kind of value escape
+        return ",".join(["%s=%s" % kv for kv in zip(self.keys, values)])
+
+    def render(self):
+        if not len(self.keys):
+            return ["%s %d" % (self.name, self.counts[()])]
+
+        return ["%s{%s} %d" % (self.name, self._render_key(k), self.counts[k])
+                for k in sorted(self.counts.keys())]
diff --git a/tests/metrics/__init__.py b/tests/metrics/__init__.py
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/metrics/test_metric.py b/tests/metrics/test_metric.py
new file mode 100644
index 0000000000..a4fd52a9d5
--- /dev/null
+++ b/tests/metrics/test_metric.py
@@ -0,0 +1,61 @@
+# -*- coding: utf-8 -*-
+# Copyright 2015 OpenMarket Ltd
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from tests import unittest
+
+from synapse.metrics.metric import CounterMetric
+
+
+class CounterMetricTestCase(unittest.TestCase):
+
+    def test_scalar(self):
+        counter = CounterMetric("scalar")
+
+        self.assertEquals(counter.render(), [
+            "scalar 0",
+        ])
+
+        counter.inc()
+
+        self.assertEquals(counter.render(), [
+            "scalar 1",
+        ])
+
+        counter.inc()
+        counter.inc()
+
+        self.assertEquals(counter.render(), [
+            "scalar 3"
+        ])
+
+    def test_vector(self):
+        counter = CounterMetric("vector", keys=["method"])
+
+        # Empty counter doesn't yet know what values it has
+        self.assertEquals(counter.render(), [])
+
+        counter.inc("GET")
+
+        self.assertEquals(counter.render(), [
+            "vector{method=GET} 1",
+        ])
+
+        counter.inc("GET")
+        counter.inc("PUT")
+
+        self.assertEquals(counter.render(), [
+            "vector{method=GET} 2",
+            "vector{method=PUT} 1",
+        ])
-- 
cgit 1.4.1


From ce8b5769f7e08515edf8988281d17df7b0ddfdaa Mon Sep 17 00:00:00 2001
From: "Paul \"LeoNerd\" Evans" <paul@matrix.org>
Date: Wed, 4 Mar 2015 15:47:23 +0000
Subject: Create the concept of a cachecounter metric; generating two counters
 specific to caches

---
 synapse/metrics/__init__.py  | 11 ++++++++++-
 synapse/metrics/metric.py    | 43 +++++++++++++++++++++++++++++++++++++------
 tests/metrics/test_metric.py | 27 ++++++++++++++++++++++++++-
 3 files changed, 73 insertions(+), 8 deletions(-)

(limited to 'tests')

diff --git a/synapse/metrics/__init__.py b/synapse/metrics/__init__.py
index 125845eb30..d5c30bbe41 100644
--- a/synapse/metrics/__init__.py
+++ b/synapse/metrics/__init__.py
@@ -13,7 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from .metric import CounterMetric
+from .metric import CounterMetric, CacheCounterMetric
 
 
 # We'll keep all the available metrics in a single toplevel dict, one shared
@@ -43,6 +43,15 @@ class Metrics(object):
 
         return metric
 
+    def register_cachecounter(self, name, *args, **kwargs):
+        full_name = "%s.%s" % (self.name_prefix, name)
+
+        metric = CacheCounterMetric(full_name, *args, **kwargs)
+
+        self._register(metric)
+
+        return metric
+
     def counted(self, func):
         """ A method decorator that registers a counter, to count invocations
         of this method. """
diff --git a/synapse/metrics/metric.py b/synapse/metrics/metric.py
index f5a98763cc..00b149f6f6 100644
--- a/synapse/metrics/metric.py
+++ b/synapse/metrics/metric.py
@@ -14,16 +14,28 @@
 # limitations under the License.
 
 
-class CounterMetric(object):
+class BaseMetric(object):
 
     def __init__(self, name, keys=[]):
         self.name = name
         self.keys = keys # OK not to clone as we never write it
 
+    def _render_key(self, values):
+        # TODO: some kind of value escape
+        return ",".join(["%s=%s" % kv for kv in zip(self.keys, values)])
+
+
+class CounterMetric(BaseMetric):
+    """The simplest kind of metric; one that stores a monotonically-increasing
+    integer that counts events."""
+
+    def __init__(self, *args, **kwargs):
+        super(CounterMetric, self).__init__(*args, **kwargs)
+
         self.counts = {}
 
         # Scalar metrics are never empty
-        if not len(keys):
+        if not len(self.keys):
             self.counts[()] = 0
 
     def inc(self, *values):
@@ -42,13 +54,32 @@ class CounterMetric(object):
     def fetch(self):
         return dict(self.counts)
 
-    def _render_key(self, values):
-        # TODO: some kind of value escape
-        return ",".join(["%s=%s" % kv for kv in zip(self.keys, values)])
-
     def render(self):
         if not len(self.keys):
             return ["%s %d" % (self.name, self.counts[()])]
 
         return ["%s{%s} %d" % (self.name, self._render_key(k), self.counts[k])
                 for k in sorted(self.counts.keys())]
+
+
+class CacheCounterMetric(object):
+    """A combination of two CounterMetrics, one to count cache hits and one to
+    count misses.
+
+    This metric generates standard metric name pairs, so that monitoring rules
+    can easily be applied to measure hit ratio."""
+
+    def __init__(self, name, keys=[]):
+        self.name = name
+
+        self.hits   = CounterMetric(name + ":hits",   keys=keys)
+        self.misses = CounterMetric(name + ":misses", keys=keys)
+
+    def inc_hits(self, *values):
+        self.hits.inc(*values)
+
+    def inc_misses(self, *values):
+        self.misses.inc(*values)
+
+    def render(self):
+        return self.hits.render() + self.misses.render()
diff --git a/tests/metrics/test_metric.py b/tests/metrics/test_metric.py
index a4fd52a9d5..93e8e27e4f 100644
--- a/tests/metrics/test_metric.py
+++ b/tests/metrics/test_metric.py
@@ -15,7 +15,7 @@
 
 from tests import unittest
 
-from synapse.metrics.metric import CounterMetric
+from synapse.metrics.metric import CounterMetric, CacheCounterMetric
 
 
 class CounterMetricTestCase(unittest.TestCase):
@@ -59,3 +59,28 @@ class CounterMetricTestCase(unittest.TestCase):
             "vector{method=GET} 2",
             "vector{method=PUT} 1",
         ])
+
+
+class CacheCounterMetricTestCase(unittest.TestCase):
+
+    def test_cachecounter(self):
+        counter = CacheCounterMetric("cache")
+
+        self.assertEquals(counter.render(), [
+            "cache:hits 0",
+            "cache:misses 0",
+        ])
+
+        counter.inc_misses()
+
+        self.assertEquals(counter.render(), [
+            "cache:hits 0",
+            "cache:misses 1",
+        ])
+
+        counter.inc_hits()
+
+        self.assertEquals(counter.render(), [
+            "cache:hits 1",
+            "cache:misses 1",
+        ])
-- 
cgit 1.4.1


From d8caa5454d781a76a65fa4ce75336541b973f624 Mon Sep 17 00:00:00 2001
From: "Paul \"LeoNerd\" Evans" <paul@matrix.org>
Date: Wed, 4 Mar 2015 16:46:44 +0000
Subject: Initial attempt at a scalar callback-based metric to give
 instantaneous snapshot gauges

---
 synapse/metrics/__init__.py  | 11 ++++++++++-
 synapse/metrics/metric.py    | 14 ++++++++++++++
 tests/metrics/test_metric.py | 22 +++++++++++++++++++++-
 3 files changed, 45 insertions(+), 2 deletions(-)

(limited to 'tests')

diff --git a/synapse/metrics/__init__.py b/synapse/metrics/__init__.py
index d5c30bbe41..d7584fc0bc 100644
--- a/synapse/metrics/__init__.py
+++ b/synapse/metrics/__init__.py
@@ -13,7 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from .metric import CounterMetric, CacheCounterMetric
+from .metric import CounterMetric, CallbackMetric, CacheCounterMetric
 
 
 # We'll keep all the available metrics in a single toplevel dict, one shared
@@ -43,6 +43,15 @@ class Metrics(object):
 
         return metric
 
+    def register_callback(self, name, callback, *args, **kwargs):
+        full_name = "%s.%s" % (self.name_prefix, name)
+
+        metric = CallbackMetric(full_name, *args, callback=callback, **kwargs)
+
+        self._register(metric)
+
+        return metric
+
     def register_cachecounter(self, name, *args, **kwargs):
         full_name = "%s.%s" % (self.name_prefix, name)
 
diff --git a/synapse/metrics/metric.py b/synapse/metrics/metric.py
index 00b149f6f6..8a497fc154 100644
--- a/synapse/metrics/metric.py
+++ b/synapse/metrics/metric.py
@@ -62,6 +62,20 @@ class CounterMetric(BaseMetric):
                 for k in sorted(self.counts.keys())]
 
 
+class CallbackMetric(BaseMetric):
+    """A metric that returns the numeric value returned by a callback whenever
+    it is rendered. Typically this is used to implement gauges that yield the
+    size or other state of some in-memory object by actively querying it."""
+
+    def __init__(self, name, callback, keys=[]):
+        super(CallbackMetric, self).__init__(name, keys=keys)
+
+        self.callback = callback
+
+    def render(self):
+        # TODO(paul): work out something we can do with keys and vectors
+        return ["%s %d" % (self.name, self.callback())]
+
 class CacheCounterMetric(object):
     """A combination of two CounterMetrics, one to count cache hits and one to
     count misses.
diff --git a/tests/metrics/test_metric.py b/tests/metrics/test_metric.py
index 93e8e27e4f..b7bb375ce0 100644
--- a/tests/metrics/test_metric.py
+++ b/tests/metrics/test_metric.py
@@ -15,7 +15,9 @@
 
 from tests import unittest
 
-from synapse.metrics.metric import CounterMetric, CacheCounterMetric
+from synapse.metrics.metric import (
+    CounterMetric, CallbackMetric, CacheCounterMetric
+)
 
 
 class CounterMetricTestCase(unittest.TestCase):
@@ -61,6 +63,24 @@ class CounterMetricTestCase(unittest.TestCase):
         ])
 
 
+class CallbackMetricTestCase(unittest.TestCase):
+
+    def test_callback(self):
+        d = dict()
+
+        metric = CallbackMetric("size", lambda: len(d))
+
+        self.assertEquals(metric.render(), [
+            "size 0",
+        ])
+
+        d["key"] = "value"
+
+        self.assertEquals(metric.render(), [
+            "size 1",
+        ])
+
+
 class CacheCounterMetricTestCase(unittest.TestCase):
 
     def test_cachecounter(self):
-- 
cgit 1.4.1


From 8664599af77ba0ed6268b3112174dc8e0c91101b Mon Sep 17 00:00:00 2001
From: "Paul \"LeoNerd\" Evans" <paul@matrix.org>
Date: Wed, 4 Mar 2015 17:34:23 +0000
Subject: Rename CacheCounterMetric to just CacheMetric; add a CallbackMetric
 component to give the size of the cache

---
 synapse/metrics/__init__.py  |  6 +++---
 synapse/metrics/metric.py    | 13 +++++++++----
 synapse/storage/_base.py     |  6 +++---
 tests/metrics/test_metric.py | 24 +++++++++++++++---------
 4 files changed, 30 insertions(+), 19 deletions(-)

(limited to 'tests')

diff --git a/synapse/metrics/__init__.py b/synapse/metrics/__init__.py
index d967b04eee..442fd70cdf 100644
--- a/synapse/metrics/__init__.py
+++ b/synapse/metrics/__init__.py
@@ -15,7 +15,7 @@
 
 import logging
 
-from .metric import CounterMetric, CallbackMetric, CacheCounterMetric
+from .metric import CounterMetric, CallbackMetric, CacheMetric
 
 
 logger = logging.getLogger(__name__)
@@ -57,10 +57,10 @@ class Metrics(object):
 
         return metric
 
-    def register_cachecounter(self, name, *args, **kwargs):
+    def register_cache(self, name, *args, **kwargs):
         full_name = "%s.%s" % (self.name_prefix, name)
 
-        metric = CacheCounterMetric(full_name, *args, **kwargs)
+        metric = CacheMetric(full_name, *args, **kwargs)
 
         self._register(metric)
 
diff --git a/synapse/metrics/metric.py b/synapse/metrics/metric.py
index 8a497fc154..7e47f86155 100644
--- a/synapse/metrics/metric.py
+++ b/synapse/metrics/metric.py
@@ -76,19 +76,24 @@ class CallbackMetric(BaseMetric):
         # TODO(paul): work out something we can do with keys and vectors
         return ["%s %d" % (self.name, self.callback())]
 
-class CacheCounterMetric(object):
+class CacheMetric(object):
     """A combination of two CounterMetrics, one to count cache hits and one to
-    count misses.
+    count misses, and a callback metric to yield the current size.
 
     This metric generates standard metric name pairs, so that monitoring rules
     can easily be applied to measure hit ratio."""
 
-    def __init__(self, name, keys=[]):
+    def __init__(self, name, size_callback, keys=[]):
         self.name = name
 
         self.hits   = CounterMetric(name + ":hits",   keys=keys)
         self.misses = CounterMetric(name + ":misses", keys=keys)
 
+        self.size = CallbackMetric(name + ":size",
+            callback=size_callback,
+            keys=keys,
+        )
+
     def inc_hits(self, *values):
         self.hits.inc(*values)
 
@@ -96,4 +101,4 @@ class CacheCounterMetric(object):
         self.misses.inc(*values)
 
     def render(self):
-        return self.hits.render() + self.misses.render()
+        return self.hits.render() + self.misses.render() + self.size.render()
diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py
index 804655e34d..d3c2bc7bfb 100644
--- a/synapse/storage/_base.py
+++ b/synapse/storage/_base.py
@@ -59,7 +59,7 @@ def cached(max_entries=1000):
     def wrap(orig):
         cache = OrderedDict()
 
-        counter = metrics.register_cachecounter(orig.__name__)
+        counter = metrics.register_cache(orig.__name__, lambda: len(cache))
 
         def prefill(key, value):
             while len(cache) > max_entries:
@@ -183,8 +183,8 @@ class SQLBaseStore(object):
         self._get_event_counters = PerformanceCounters()
 
         self._get_event_cache = LruCache(hs.config.event_cache_size)
-        self._get_event_cache_counter = metrics.register_cachecounter(
-            "get_event"
+        self._get_event_cache_counter = metrics.register_cache("get_event",
+            size_callback=lambda: len(self._get_event_cache),
         )
 
     def start_profiling(self):
diff --git a/tests/metrics/test_metric.py b/tests/metrics/test_metric.py
index b7bb375ce0..32fd178ed4 100644
--- a/tests/metrics/test_metric.py
+++ b/tests/metrics/test_metric.py
@@ -16,7 +16,7 @@
 from tests import unittest
 
 from synapse.metrics.metric import (
-    CounterMetric, CallbackMetric, CacheCounterMetric
+    CounterMetric, CallbackMetric, CacheMetric
 )
 
 
@@ -81,26 +81,32 @@ class CallbackMetricTestCase(unittest.TestCase):
         ])
 
 
-class CacheCounterMetricTestCase(unittest.TestCase):
+class CacheMetricTestCase(unittest.TestCase):
 
-    def test_cachecounter(self):
-        counter = CacheCounterMetric("cache")
+    def test_cache(self):
+        d = dict()
 
-        self.assertEquals(counter.render(), [
+        metric = CacheMetric("cache", lambda: len(d))
+
+        self.assertEquals(metric.render(), [
             "cache:hits 0",
             "cache:misses 0",
+            "cache:size 0",
         ])
 
-        counter.inc_misses()
+        metric.inc_misses()
+        d["key"] = "value"
 
-        self.assertEquals(counter.render(), [
+        self.assertEquals(metric.render(), [
             "cache:hits 0",
             "cache:misses 1",
+            "cache:size 1",
         ])
 
-        counter.inc_hits()
+        metric.inc_hits()
 
-        self.assertEquals(counter.render(), [
+        self.assertEquals(metric.render(), [
             "cache:hits 1",
             "cache:misses 1",
+            "cache:size 1",
         ])
-- 
cgit 1.4.1


From 23ab0c68c28e60e0f8774ee4099b2abe876374d0 Mon Sep 17 00:00:00 2001
From: "Paul \"LeoNerd\" Evans" <paul@matrix.org>
Date: Wed, 4 Mar 2015 17:58:10 +0000
Subject: Implement vector CallbackMetrics

---
 synapse/metrics/metric.py    |  8 ++++++--
 tests/metrics/test_metric.py | 18 +++++++++++++++++-
 2 files changed, 23 insertions(+), 3 deletions(-)

(limited to 'tests')

diff --git a/synapse/metrics/metric.py b/synapse/metrics/metric.py
index e8c15a60e8..4df5ebfda6 100644
--- a/synapse/metrics/metric.py
+++ b/synapse/metrics/metric.py
@@ -79,9 +79,13 @@ class CallbackMetric(BaseMetric):
         self.callback = callback
 
     def render(self):
-        # TODO(paul): work out something we can do with keys and vectors
-        return ["%s %d" % (self.name, self.callback())]
+        value = self.callback()
 
+        if self.is_scalar():
+            return ["%s %d" % (self.name, value)]
+
+        return ["%s{%s} %d" % (self.name, self._render_key(k), value[k])
+                for k in sorted(value.keys())]
 
 class CacheMetric(object):
     """A combination of two CounterMetrics, one to count cache hits and one to
diff --git a/tests/metrics/test_metric.py b/tests/metrics/test_metric.py
index 32fd178ed4..b7facb8587 100644
--- a/tests/metrics/test_metric.py
+++ b/tests/metrics/test_metric.py
@@ -65,7 +65,7 @@ class CounterMetricTestCase(unittest.TestCase):
 
 class CallbackMetricTestCase(unittest.TestCase):
 
-    def test_callback(self):
+    def test_scalar(self):
         d = dict()
 
         metric = CallbackMetric("size", lambda: len(d))
@@ -80,6 +80,22 @@ class CallbackMetricTestCase(unittest.TestCase):
             "size 1",
         ])
 
+    def test_vector(self):
+        vals = dict()
+
+        metric = CallbackMetric("values", lambda: vals, keys=["type"])
+
+        self.assertEquals(metric.render(), [])
+
+        # Keys have to be tuples, even if they're 1-element
+        vals[("foo",)] = 1
+        vals[("bar",)] = 2
+
+        self.assertEquals(metric.render(), [
+            "values{type=bar} 2",
+            "values{type=foo} 1",
+        ])
+
 
 class CacheMetricTestCase(unittest.TestCase):
 
-- 
cgit 1.4.1


From 72625f2f4d633e9fe59e61bb371a118927e5c66c Mon Sep 17 00:00:00 2001
From: "Paul \"LeoNerd\" Evans" <paul@matrix.org>
Date: Wed, 4 Mar 2015 19:22:14 +0000
Subject: Initial hack at a TimerMetric; for storing counts + duration
 accumulators

---
 synapse/metrics/metric.py    | 48 ++++++++++++++++++++++++++++++++++++++++++++
 tests/metrics/test_metric.py | 36 ++++++++++++++++++++++++++++++++-
 2 files changed, 83 insertions(+), 1 deletion(-)

(limited to 'tests')

diff --git a/synapse/metrics/metric.py b/synapse/metrics/metric.py
index 4df5ebfda6..7175881941 100644
--- a/synapse/metrics/metric.py
+++ b/synapse/metrics/metric.py
@@ -14,6 +14,15 @@
 # limitations under the License.
 
 
+from itertools import chain
+
+
+# TODO(paul): I can't believe Python doesn't have one of these
+def map_concat(func, items):
+    # flatten a list-of-lists
+    return list(chain.from_iterable(map(func, items)))
+
+
 class BaseMetric(object):
 
     def __init__(self, name, keys=[]):
@@ -87,6 +96,45 @@ class CallbackMetric(BaseMetric):
         return ["%s{%s} %d" % (self.name, self._render_key(k), value[k])
                 for k in sorted(value.keys())]
 
+
+class TimerMetric(CounterMetric):
+    """A combination of an event counter and a time accumulator, which counts
+    both the number of events and how long each one takes.
+
+    TODO(paul): Try to export some heatmap-style stats?
+    """
+
+    def __init__(self, *args, **kwargs):
+        super(TimerMetric, self).__init__(*args, **kwargs)
+
+        self.times = {}
+
+        # Scalar metrics are never empty
+        if self.is_scalar():
+            self.times[()] = 0
+
+    def inc_time(self, msec, *values):
+        self.inc(*values)
+
+        if values not in self.times:
+            self.times[values] = msec
+        else:
+            self.times[values] += msec
+
+    def render(self):
+        if self.is_scalar():
+            return ["%s:count %d" % (self.name, self.counts[()]),
+                    "%s:msec %d" % (self.name, self.times[()])]
+
+        def render_item(k):
+            keystr = self._render_key(k)
+
+            return ["%s{%s}:count %d" % (self.name, keystr, self.counts[k]),
+                    "%s{%s}:msec %d" % (self.name, keystr, self.times[k])]
+
+        return map_concat(render_item, sorted(self.counts.keys()))
+
+
 class CacheMetric(object):
     """A combination of two CounterMetrics, one to count cache hits and one to
     count misses, and a callback metric to yield the current size.
diff --git a/tests/metrics/test_metric.py b/tests/metrics/test_metric.py
index b7facb8587..b25520821d 100644
--- a/tests/metrics/test_metric.py
+++ b/tests/metrics/test_metric.py
@@ -16,7 +16,7 @@
 from tests import unittest
 
 from synapse.metrics.metric import (
-    CounterMetric, CallbackMetric, CacheMetric
+    CounterMetric, CallbackMetric, TimerMetric, CacheMetric
 )
 
 
@@ -97,6 +97,40 @@ class CallbackMetricTestCase(unittest.TestCase):
         ])
 
 
+class TimerMetricTestCase(unittest.TestCase):
+
+    def test_scalar(self):
+        metric = TimerMetric("thing")
+
+        self.assertEquals(metric.render(), [
+            "thing:count 0",
+            "thing:msec 0",
+        ])
+
+        metric.inc_time(500)
+
+        self.assertEquals(metric.render(), [
+            "thing:count 1",
+            "thing:msec 500",
+        ])
+
+    def test_vector(self):
+        metric = TimerMetric("queries", keys=["verb"])
+
+        self.assertEquals(metric.render(), [])
+
+        metric.inc_time(300, "SELECT")
+        metric.inc_time(200, "SELECT")
+        metric.inc_time(800, "INSERT")
+
+        self.assertEquals(metric.render(), [
+            "queries{verb=INSERT}:count 1",
+            "queries{verb=INSERT}:msec 800",
+            "queries{verb=SELECT}:count 2",
+            "queries{verb=SELECT}:msec 500",
+        ])
+
+
 class CacheMetricTestCase(unittest.TestCase):
 
     def test_cache(self):
-- 
cgit 1.4.1


From f9478e475bf645038b4f1f163240d7fd0ec02af0 Mon Sep 17 00:00:00 2001
From: "Paul \"LeoNerd\" Evans" <paul@matrix.org>
Date: Fri, 6 Mar 2015 15:28:06 +0000
Subject: Rename Metrics' "keys" to "labels"

---
 synapse/federation/transaction_queue.py |  4 ++--
 synapse/http/client.py                  |  4 ++--
 synapse/http/matrixfederationclient.py  |  4 ++--
 synapse/http/server.py                  |  4 ++--
 synapse/metrics/metric.py               | 24 ++++++++++++------------
 synapse/storage/_base.py                |  6 +++---
 tests/metrics/test_metric.py            |  6 +++---
 7 files changed, 26 insertions(+), 26 deletions(-)

(limited to 'tests')

diff --git a/synapse/federation/transaction_queue.py b/synapse/federation/transaction_queue.py
index b9d3f89324..ae62c69fc3 100644
--- a/synapse/federation/transaction_queue.py
+++ b/synapse/federation/transaction_queue.py
@@ -71,11 +71,11 @@ class TransactionQueue(object):
 
         metrics.register_callback("pending_pdus",
             lambda: {(dest,): len(pdus[dest]) for dest in pdus.keys()},
-            keys=["dest"],
+            labels=["dest"],
         )
         metrics.register_callback("pending_edus",
             lambda: {(dest,): len(edus[dest]) for dest in edus.keys()},
-            keys=["dest"],
+            labels=["dest"],
         )
 
     def can_send_to(self, destination):
diff --git a/synapse/http/client.py b/synapse/http/client.py
index e40e82e80b..ad2c9c05ec 100644
--- a/synapse/http/client.py
+++ b/synapse/http/client.py
@@ -35,10 +35,10 @@ logger = logging.getLogger(__name__)
 metrics = synapse.metrics.get_metrics_for(__name__)
 
 outgoing_requests_counter = metrics.register_counter("outgoing_requests",
-    keys=["method"],
+    labels=["method"],
 )
 incoming_responses_counter = metrics.register_counter("incoming_responses",
-    keys=["method","code"],
+    labels=["method","code"],
 )
 
 
diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 0091527693..6b6d79a044 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -44,10 +44,10 @@ logger = logging.getLogger(__name__)
 metrics = synapse.metrics.get_metrics_for(__name__)
 
 outgoing_requests_counter = metrics.register_counter("outgoing_requests",
-    keys=["method"],
+    labels=["method"],
 )
 incoming_responses_counter = metrics.register_counter("incoming_responses",
-    keys=["method","code"],
+    labels=["method","code"],
 )
 
 
diff --git a/synapse/http/server.py b/synapse/http/server.py
index ac893bb40c..35bd3a00ba 100644
--- a/synapse/http/server.py
+++ b/synapse/http/server.py
@@ -38,10 +38,10 @@ logger = logging.getLogger(__name__)
 metrics = synapse.metrics.get_metrics_for(__name__)
 
 incoming_requests_counter = metrics.register_counter("incoming_requests",
-    keys=["method"],
+    labels=["method"],
 )
 outgoing_responses_counter = metrics.register_counter("outgoing_responses",
-    keys=["method","code"],
+    labels=["method","code"],
 )
 
 
diff --git a/synapse/metrics/metric.py b/synapse/metrics/metric.py
index 4a6ab9cd74..8ba13075f7 100644
--- a/synapse/metrics/metric.py
+++ b/synapse/metrics/metric.py
@@ -25,22 +25,22 @@ def map_concat(func, items):
 
 class BaseMetric(object):
 
-    def __init__(self, name, keys=[]):
+    def __init__(self, name, labels=[]):
         self.name = name
-        self.keys = keys # OK not to clone as we never write it
+        self.labels = labels # OK not to clone as we never write it
 
     def dimension(self):
-        return len(self.keys)
+        return len(self.labels)
 
     def is_scalar(self):
-        return not len(self.keys)
+        return not len(self.labels)
 
     def _render_key(self, values):
         if self.is_scalar():
             return ""
         # TODO: some kind of value escape
         return "{%s}" % (
-            ",".join(["%s=%s" % kv for kv in zip(self.keys, values)])
+            ",".join(["%s=%s" % kv for kv in zip(self.labels, values)])
         )
 
     def render(self):
@@ -62,7 +62,7 @@ class CounterMetric(BaseMetric):
 
     def inc(self, *values):
         if len(values) != self.dimension():
-            raise ValueError("Expected as many values to inc() as keys (%d)" %
+            raise ValueError("Expected as many values to inc() as labels (%d)" %
                 (self.dimension())
             )
 
@@ -85,8 +85,8 @@ class CallbackMetric(BaseMetric):
     it is rendered. Typically this is used to implement gauges that yield the
     size or other state of some in-memory object by actively querying it."""
 
-    def __init__(self, name, callback, keys=[]):
-        super(CallbackMetric, self).__init__(name, keys=keys)
+    def __init__(self, name, callback, labels=[]):
+        super(CallbackMetric, self).__init__(name, labels=labels)
 
         self.callback = callback
 
@@ -139,15 +139,15 @@ class CacheMetric(object):
     This metric generates standard metric name pairs, so that monitoring rules
     can easily be applied to measure hit ratio."""
 
-    def __init__(self, name, size_callback, keys=[]):
+    def __init__(self, name, size_callback, labels=[]):
         self.name = name
 
-        self.hits   = CounterMetric(name + ":hits",   keys=keys)
-        self.misses = CounterMetric(name + ":misses", keys=keys)
+        self.hits   = CounterMetric(name + ":hits",   labels=labels)
+        self.misses = CounterMetric(name + ":misses", labels=labels)
 
         self.size = CallbackMetric(name + ":size",
             callback=size_callback,
-            keys=keys,
+            labels=labels,
         )
 
     def inc_hits(self, *values):
diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py
index d8c5a60c71..a38b603584 100644
--- a/synapse/storage/_base.py
+++ b/synapse/storage/_base.py
@@ -38,9 +38,9 @@ transaction_logger = logging.getLogger("synapse.storage.txn")
 
 metrics = synapse.metrics.get_metrics_for("synapse.storage")
 
-sql_query_timer = metrics.register_timer("queries", keys=["verb"])
-sql_txn_timer = metrics.register_timer("transactions", keys=["desc"])
-sql_getevents_timer = metrics.register_timer("get_events", keys=["desc"])
+sql_query_timer = metrics.register_timer("queries", labels=["verb"])
+sql_txn_timer = metrics.register_timer("transactions", labels=["desc"])
+sql_getevents_timer = metrics.register_timer("get_events", labels=["desc"])
 
 
 # TODO(paul):
diff --git a/tests/metrics/test_metric.py b/tests/metrics/test_metric.py
index b25520821d..fefe1a5867 100644
--- a/tests/metrics/test_metric.py
+++ b/tests/metrics/test_metric.py
@@ -43,7 +43,7 @@ class CounterMetricTestCase(unittest.TestCase):
         ])
 
     def test_vector(self):
-        counter = CounterMetric("vector", keys=["method"])
+        counter = CounterMetric("vector", labels=["method"])
 
         # Empty counter doesn't yet know what values it has
         self.assertEquals(counter.render(), [])
@@ -83,7 +83,7 @@ class CallbackMetricTestCase(unittest.TestCase):
     def test_vector(self):
         vals = dict()
 
-        metric = CallbackMetric("values", lambda: vals, keys=["type"])
+        metric = CallbackMetric("values", lambda: vals, labels=["type"])
 
         self.assertEquals(metric.render(), [])
 
@@ -115,7 +115,7 @@ class TimerMetricTestCase(unittest.TestCase):
         ])
 
     def test_vector(self):
-        metric = TimerMetric("queries", keys=["verb"])
+        metric = TimerMetric("queries", labels=["verb"])
 
         self.assertEquals(metric.render(), [])
 
-- 
cgit 1.4.1


From b3a0179d64c2c3b4f57688bdcceb818d0124c858 Mon Sep 17 00:00:00 2001
From: "Paul \"LeoNerd\" Evans" <paul@matrix.org>
Date: Fri, 6 Mar 2015 15:35:23 +0000
Subject: Bugfix to rendering output of vectored TimerMetrics

---
 synapse/metrics/metric.py    | 5 ++---
 tests/metrics/test_metric.py | 8 ++++----
 2 files changed, 6 insertions(+), 7 deletions(-)

(limited to 'tests')

diff --git a/synapse/metrics/metric.py b/synapse/metrics/metric.py
index 8ba13075f7..17cd673891 100644
--- a/synapse/metrics/metric.py
+++ b/synapse/metrics/metric.py
@@ -127,9 +127,8 @@ class TimerMetric(CounterMetric):
     def render_item(self, k):
         keystr = self._render_key(k)
 
-        return ["%s%s:count %d" % (self.name, keystr, self.counts[k]),
-                "%s%s:msec %d" % (self.name, keystr, self.times[k])]
-
+        return ["%s:count%s %d" % (self.name, keystr, self.counts[k]),
+                "%s:msec%s %d" % (self.name, keystr, self.times[k])]
 
 
 class CacheMetric(object):
diff --git a/tests/metrics/test_metric.py b/tests/metrics/test_metric.py
index fefe1a5867..75b6cbc924 100644
--- a/tests/metrics/test_metric.py
+++ b/tests/metrics/test_metric.py
@@ -124,10 +124,10 @@ class TimerMetricTestCase(unittest.TestCase):
         metric.inc_time(800, "INSERT")
 
         self.assertEquals(metric.render(), [
-            "queries{verb=INSERT}:count 1",
-            "queries{verb=INSERT}:msec 800",
-            "queries{verb=SELECT}:count 2",
-            "queries{verb=SELECT}:msec 500",
+            "queries:count{verb=INSERT} 1",
+            "queries:msec{verb=INSERT} 800",
+            "queries:count{verb=SELECT} 2",
+            "queries:msec{verb=SELECT} 500",
         ])
 
 
-- 
cgit 1.4.1


From 0e847540c3aa1c471a00b3200f7f18e48004b48d Mon Sep 17 00:00:00 2001
From: "Paul \"LeoNerd\" Evans" <paul@matrix.org>
Date: Fri, 6 Mar 2015 18:40:20 +0000
Subject: Prometheus needs "escaped" label values

---
 synapse/metrics/metric.py    |  8 +++++--
 tests/metrics/test_metric.py | 54 ++++++++++++++++++++++----------------------
 2 files changed, 33 insertions(+), 29 deletions(-)

(limited to 'tests')

diff --git a/synapse/metrics/metric.py b/synapse/metrics/metric.py
index 93508eeacc..922cb5a6f1 100644
--- a/synapse/metrics/metric.py
+++ b/synapse/metrics/metric.py
@@ -35,12 +35,16 @@ class BaseMetric(object):
     def is_scalar(self):
         return not len(self.labels)
 
+    def _render_labelvalue(self, value):
+        # TODO: some kind of value escape
+        return '"%s"' % (value)
+
     def _render_key(self, values):
         if self.is_scalar():
             return ""
-        # TODO: some kind of value escape
         return "{%s}" % (
-            ",".join(["%s=%s" % kv for kv in zip(self.labels, values)])
+            ",".join(["%s=%s" % (k, self._render_labelvalue(v))
+                      for k, v in zip(self.labels, values)])
         )
 
     def render(self):
diff --git a/tests/metrics/test_metric.py b/tests/metrics/test_metric.py
index 75b6cbc924..1919630feb 100644
--- a/tests/metrics/test_metric.py
+++ b/tests/metrics/test_metric.py
@@ -26,20 +26,20 @@ class CounterMetricTestCase(unittest.TestCase):
         counter = CounterMetric("scalar")
 
         self.assertEquals(counter.render(), [
-            "scalar 0",
+            'scalar 0',
         ])
 
         counter.inc()
 
         self.assertEquals(counter.render(), [
-            "scalar 1",
+            'scalar 1',
         ])
 
         counter.inc()
         counter.inc()
 
         self.assertEquals(counter.render(), [
-            "scalar 3"
+            'scalar 3'
         ])
 
     def test_vector(self):
@@ -51,15 +51,15 @@ class CounterMetricTestCase(unittest.TestCase):
         counter.inc("GET")
 
         self.assertEquals(counter.render(), [
-            "vector{method=GET} 1",
+            'vector{method="GET"} 1',
         ])
 
         counter.inc("GET")
         counter.inc("PUT")
 
         self.assertEquals(counter.render(), [
-            "vector{method=GET} 2",
-            "vector{method=PUT} 1",
+            'vector{method="GET"} 2',
+            'vector{method="PUT"} 1',
         ])
 
 
@@ -71,13 +71,13 @@ class CallbackMetricTestCase(unittest.TestCase):
         metric = CallbackMetric("size", lambda: len(d))
 
         self.assertEquals(metric.render(), [
-            "size 0",
+            'size 0',
         ])
 
         d["key"] = "value"
 
         self.assertEquals(metric.render(), [
-            "size 1",
+            'size 1',
         ])
 
     def test_vector(self):
@@ -92,8 +92,8 @@ class CallbackMetricTestCase(unittest.TestCase):
         vals[("bar",)] = 2
 
         self.assertEquals(metric.render(), [
-            "values{type=bar} 2",
-            "values{type=foo} 1",
+            'values{type="bar"} 2',
+            'values{type="foo"} 1',
         ])
 
 
@@ -103,15 +103,15 @@ class TimerMetricTestCase(unittest.TestCase):
         metric = TimerMetric("thing")
 
         self.assertEquals(metric.render(), [
-            "thing:count 0",
-            "thing:msec 0",
+            'thing:count 0',
+            'thing:msec 0',
         ])
 
         metric.inc_time(500)
 
         self.assertEquals(metric.render(), [
-            "thing:count 1",
-            "thing:msec 500",
+            'thing:count 1',
+            'thing:msec 500',
         ])
 
     def test_vector(self):
@@ -124,10 +124,10 @@ class TimerMetricTestCase(unittest.TestCase):
         metric.inc_time(800, "INSERT")
 
         self.assertEquals(metric.render(), [
-            "queries:count{verb=INSERT} 1",
-            "queries:msec{verb=INSERT} 800",
-            "queries:count{verb=SELECT} 2",
-            "queries:msec{verb=SELECT} 500",
+            'queries:count{verb="INSERT"} 1',
+            'queries:msec{verb="INSERT"} 800',
+            'queries:count{verb="SELECT"} 2',
+            'queries:msec{verb="SELECT"} 500',
         ])
 
 
@@ -139,24 +139,24 @@ class CacheMetricTestCase(unittest.TestCase):
         metric = CacheMetric("cache", lambda: len(d))
 
         self.assertEquals(metric.render(), [
-            "cache:hits 0",
-            "cache:misses 0",
-            "cache:size 0",
+            'cache:hits 0',
+            'cache:misses 0',
+            'cache:size 0',
         ])
 
         metric.inc_misses()
         d["key"] = "value"
 
         self.assertEquals(metric.render(), [
-            "cache:hits 0",
-            "cache:misses 1",
-            "cache:size 1",
+            'cache:hits 0',
+            'cache:misses 1',
+            'cache:size 1',
         ])
 
         metric.inc_hits()
 
         self.assertEquals(metric.render(), [
-            "cache:hits 1",
-            "cache:misses 1",
-            "cache:size 1",
+            'cache:hits 1',
+            'cache:misses 1',
+            'cache:size 1',
         ])
-- 
cgit 1.4.1


From cbc0406be844894cac08c457544f1eb0c28435bb Mon Sep 17 00:00:00 2001
From: "Paul \"LeoNerd\" Evans" <paul@matrix.org>
Date: Mon, 9 Mar 2015 20:35:33 +0000
Subject: Export CacheMetric as hits+total, rather than hits+misses, as it's
 easier to derive hit ratio from that

---
 synapse/metrics/metric.py    | 11 ++++++-----
 tests/metrics/test_metric.py |  6 +++---
 2 files changed, 9 insertions(+), 8 deletions(-)

(limited to 'tests')

diff --git a/synapse/metrics/metric.py b/synapse/metrics/metric.py
index 922cb5a6f1..6b7d3358bc 100644
--- a/synapse/metrics/metric.py
+++ b/synapse/metrics/metric.py
@@ -134,7 +134,7 @@ class TimerMetric(CounterMetric):
 
 class CacheMetric(object):
     """A combination of two CounterMetrics, one to count cache hits and one to
-    count misses, and a callback metric to yield the current size.
+    count a total, and a callback metric to yield the current size.
 
     This metric generates standard metric name pairs, so that monitoring rules
     can easily be applied to measure hit ratio."""
@@ -142,8 +142,8 @@ class CacheMetric(object):
     def __init__(self, name, size_callback, labels=[]):
         self.name = name
 
-        self.hits   = CounterMetric(name + ":hits",   labels=labels)
-        self.misses = CounterMetric(name + ":misses", labels=labels)
+        self.hits  = CounterMetric(name + ":hits",  labels=labels)
+        self.total = CounterMetric(name + ":total", labels=labels)
 
         self.size = CallbackMetric(name + ":size",
             callback=size_callback,
@@ -152,9 +152,10 @@ class CacheMetric(object):
 
     def inc_hits(self, *values):
         self.hits.inc(*values)
+        self.total.inc(*values)
 
     def inc_misses(self, *values):
-        self.misses.inc(*values)
+        self.total.inc(*values)
 
     def render(self):
-        return self.hits.render() + self.misses.render() + self.size.render()
+        return self.hits.render() + self.total.render() + self.size.render()
diff --git a/tests/metrics/test_metric.py b/tests/metrics/test_metric.py
index 1919630feb..193908b44e 100644
--- a/tests/metrics/test_metric.py
+++ b/tests/metrics/test_metric.py
@@ -140,7 +140,7 @@ class CacheMetricTestCase(unittest.TestCase):
 
         self.assertEquals(metric.render(), [
             'cache:hits 0',
-            'cache:misses 0',
+            'cache:total 0',
             'cache:size 0',
         ])
 
@@ -149,7 +149,7 @@ class CacheMetricTestCase(unittest.TestCase):
 
         self.assertEquals(metric.render(), [
             'cache:hits 0',
-            'cache:misses 1',
+            'cache:total 1',
             'cache:size 1',
         ])
 
@@ -157,6 +157,6 @@ class CacheMetricTestCase(unittest.TestCase):
 
         self.assertEquals(metric.render(), [
             'cache:hits 1',
-            'cache:misses 1',
+            'cache:total 2',
             'cache:size 1',
         ])
-- 
cgit 1.4.1


From f1fbe3e09f5573ac7ea9159635b02cc579e19720 Mon Sep 17 00:00:00 2001
From: "Paul \"LeoNerd\" Evans" <paul@matrix.org>
Date: Tue, 10 Mar 2015 15:21:03 +0000
Subject: Rename TimerMetric to DistributionMetric; as it could count more than
 just time

---
 synapse/metrics/__init__.py  |  8 +++++---
 synapse/metrics/metric.py    | 24 +++++++++++++-----------
 synapse/storage/_base.py     | 14 +++++++-------
 tests/metrics/test_metric.py | 24 ++++++++++++------------
 4 files changed, 37 insertions(+), 33 deletions(-)

(limited to 'tests')

diff --git a/synapse/metrics/__init__.py b/synapse/metrics/__init__.py
index 1acaa3fd09..c161c17e9f 100644
--- a/synapse/metrics/__init__.py
+++ b/synapse/metrics/__init__.py
@@ -19,7 +19,9 @@ from __future__ import absolute_import
 import logging
 from resource import getrusage, getpagesize, RUSAGE_SELF
 
-from .metric import CounterMetric, CallbackMetric, TimerMetric, CacheMetric
+from .metric import (
+    CounterMetric, CallbackMetric, DistributionMetric, CacheMetric
+)
 
 
 logger = logging.getLogger(__name__)
@@ -59,8 +61,8 @@ class Metrics(object):
     def register_callback(self, *args, **kwargs):
         return self._register(CallbackMetric, *args, **kwargs)
 
-    def register_timer(self, *args, **kwargs):
-        return self._register(TimerMetric, *args, **kwargs)
+    def register_distribution(self, *args, **kwargs):
+        return self._register(DistributionMetric, *args, **kwargs)
 
     def register_cache(self, *args, **kwargs):
         return self._register(CacheMetric, *args, **kwargs)
diff --git a/synapse/metrics/metric.py b/synapse/metrics/metric.py
index 6b7d3358bc..45d2752a20 100644
--- a/synapse/metrics/metric.py
+++ b/synapse/metrics/metric.py
@@ -101,35 +101,37 @@ class CallbackMetric(BaseMetric):
                 for k in sorted(value.keys())]
 
 
-class TimerMetric(CounterMetric):
-    """A combination of an event counter and a time accumulator, which counts
-    both the number of events and how long each one takes.
+class DistributionMetric(CounterMetric):
+    """A combination of an event counter and an accumulator, which counts
+    both the number of events and accumulates the total value. Typically this
+    could be used to keep track of method-running times, or other distributions
+    of values that occur in discrete occurances.
 
     TODO(paul): Try to export some heatmap-style stats?
     """
 
     def __init__(self, *args, **kwargs):
-        super(TimerMetric, self).__init__(*args, **kwargs)
+        super(DistributionMetric, self).__init__(*args, **kwargs)
 
-        self.times = {}
+        self.totals = {}
 
         # Scalar metrics are never empty
         if self.is_scalar():
-            self.times[()] = 0
+            self.totals[()] = 0
 
-    def inc_time(self, msec, *values):
+    def inc_by(self, inc, *values):
         self.inc(*values)
 
-        if values not in self.times:
-            self.times[values] = msec
+        if values not in self.totals:
+            self.totals[values] = inc
         else:
-            self.times[values] += msec
+            self.totals[values] += inc
 
     def render_item(self, k):
         keystr = self._render_key(k)
 
         return ["%s:count%s %d" % (self.name, keystr, self.counts[k]),
-                "%s:msec%s %d" % (self.name, keystr, self.times[k])]
+                "%s:total%s %d" % (self.name, keystr, self.totals[k])]
 
 
 class CacheMetric(object):
diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py
index 2708d3c5b6..104e8e3cf6 100644
--- a/synapse/storage/_base.py
+++ b/synapse/storage/_base.py
@@ -38,9 +38,9 @@ transaction_logger = logging.getLogger("synapse.storage.txn")
 
 metrics = synapse.metrics.get_metrics_for("synapse.storage")
 
-sql_query_timer = metrics.register_timer("queries", labels=["verb"])
-sql_txn_timer = metrics.register_timer("transactions", labels=["desc"])
-sql_getevents_timer = metrics.register_timer("getEvents", labels=["desc"])
+sql_query_timer = metrics.register_distribution("queries", labels=["verb"])
+sql_txn_timer = metrics.register_distribution("transactions", labels=["desc"])
+sql_getevents_timer = metrics.register_distribution("getEvents", labels=["desc"])
 
 caches_by_name = {}
 cache_counter = metrics.register_cache(
@@ -143,7 +143,7 @@ class LoggingTransaction(object):
         finally:
             msecs = (time.time() * 1000) - start
             sql_logger.debug("[SQL time] {%s} %f", self.name, msecs)
-            sql_query_timer.inc_time(msecs, sql.split()[0])
+            sql_query_timer.inc_by(msecs, sql.split()[0])
 
 
 class PerformanceCounters(object):
@@ -268,7 +268,7 @@ class SQLBaseStore(object):
                     self._current_txn_total_time += end - start
                     self._txn_perf_counters.update(desc, start, end)
 
-                    sql_txn_timer.inc_time(self._current_txn_total_time, desc)
+                    sql_txn_timer.inc_by(self._current_txn_total_time, desc)
 
         with PreserveLoggingContext():
             result = yield self._db_pool.runInteraction(
@@ -672,7 +672,7 @@ class SQLBaseStore(object):
 
         def update_counter(desc, last_time):
             curr_time = self._get_event_counters.update(desc, last_time)
-            sql_getevents_timer.inc_time(curr_time - last_time, desc)
+            sql_getevents_timer.inc_by(curr_time - last_time, desc)
             return curr_time
 
         cache = self._get_event_cache.setdefault(event_id, {})
@@ -727,7 +727,7 @@ class SQLBaseStore(object):
 
         def update_counter(desc, last_time):
             curr_time = self._get_event_counters.update(desc, last_time)
-            sql_getevents_timer.inc_time(curr_time - last_time, desc)
+            sql_getevents_timer.inc_by(curr_time - last_time, desc)
             return curr_time
 
         d = json.loads(js)
diff --git a/tests/metrics/test_metric.py b/tests/metrics/test_metric.py
index 193908b44e..1ca3e45a26 100644
--- a/tests/metrics/test_metric.py
+++ b/tests/metrics/test_metric.py
@@ -16,7 +16,7 @@
 from tests import unittest
 
 from synapse.metrics.metric import (
-    CounterMetric, CallbackMetric, TimerMetric, CacheMetric
+    CounterMetric, CallbackMetric, DistributionMetric, CacheMetric
 )
 
 
@@ -97,37 +97,37 @@ class CallbackMetricTestCase(unittest.TestCase):
         ])
 
 
-class TimerMetricTestCase(unittest.TestCase):
+class DistributionMetricTestCase(unittest.TestCase):
 
     def test_scalar(self):
-        metric = TimerMetric("thing")
+        metric = DistributionMetric("thing")
 
         self.assertEquals(metric.render(), [
             'thing:count 0',
-            'thing:msec 0',
+            'thing:total 0',
         ])
 
-        metric.inc_time(500)
+        metric.inc_by(500)
 
         self.assertEquals(metric.render(), [
             'thing:count 1',
-            'thing:msec 500',
+            'thing:total 500',
         ])
 
     def test_vector(self):
-        metric = TimerMetric("queries", labels=["verb"])
+        metric = DistributionMetric("queries", labels=["verb"])
 
         self.assertEquals(metric.render(), [])
 
-        metric.inc_time(300, "SELECT")
-        metric.inc_time(200, "SELECT")
-        metric.inc_time(800, "INSERT")
+        metric.inc_by(300, "SELECT")
+        metric.inc_by(200, "SELECT")
+        metric.inc_by(800, "INSERT")
 
         self.assertEquals(metric.render(), [
             'queries:count{verb="INSERT"} 1',
-            'queries:msec{verb="INSERT"} 800',
+            'queries:total{verb="INSERT"} 800',
             'queries:count{verb="SELECT"} 2',
-            'queries:msec{verb="SELECT"} 500',
+            'queries:total{verb="SELECT"} 500',
         ])
 
 
-- 
cgit 1.4.1


From c1cdd7954d7cc411a5ec926148b9060e59b8a7bd Mon Sep 17 00:00:00 2001
From: "Paul \"LeoNerd\" Evans" <paul@matrix.org>
Date: Tue, 10 Mar 2015 15:54:16 +0000
Subject: Add an .inc_by() method to CounterMetric; implement
 DistributionMetric a neater way

---
 synapse/metrics/metric.py    | 37 ++++++++++++++-----------------------
 tests/metrics/test_metric.py |  5 ++---
 2 files changed, 16 insertions(+), 26 deletions(-)

(limited to 'tests')

diff --git a/synapse/metrics/metric.py b/synapse/metrics/metric.py
index 45d2752a20..12460c99c3 100644
--- a/synapse/metrics/metric.py
+++ b/synapse/metrics/metric.py
@@ -64,7 +64,7 @@ class CounterMetric(BaseMetric):
         if self.is_scalar():
             self.counts[()] = 0
 
-    def inc(self, *values):
+    def inc_by(self, incr, *values):
         if len(values) != self.dimension():
             raise ValueError("Expected as many values to inc() as labels (%d)" %
                 (self.dimension())
@@ -73,9 +73,12 @@ class CounterMetric(BaseMetric):
         # TODO: should assert that the tag values are all strings
 
         if values not in self.counts:
-            self.counts[values] = 1
+            self.counts[values] = incr
         else:
-            self.counts[values] += 1
+            self.counts[values] += incr
+
+    def inc(self, *values):
+        self.inc_by(1, *values)
 
     def render_item(self, k):
         return ["%s%s %d" % (self.name, self._render_key(k), self.counts[k])]
@@ -101,7 +104,7 @@ class CallbackMetric(BaseMetric):
                 for k in sorted(value.keys())]
 
 
-class DistributionMetric(CounterMetric):
+class DistributionMetric(object):
     """A combination of an event counter and an accumulator, which counts
     both the number of events and accumulates the total value. Typically this
     could be used to keep track of method-running times, or other distributions
@@ -110,28 +113,16 @@ class DistributionMetric(CounterMetric):
     TODO(paul): Try to export some heatmap-style stats?
     """
 
-    def __init__(self, *args, **kwargs):
-        super(DistributionMetric, self).__init__(*args, **kwargs)
-
-        self.totals = {}
-
-        # Scalar metrics are never empty
-        if self.is_scalar():
-            self.totals[()] = 0
+    def __init__(self, name, *args, **kwargs):
+        self.counts = CounterMetric(name + ":count", **kwargs)
+        self.totals = CounterMetric(name + ":total", **kwargs)
 
     def inc_by(self, inc, *values):
-        self.inc(*values)
-
-        if values not in self.totals:
-            self.totals[values] = inc
-        else:
-            self.totals[values] += inc
+        self.counts.inc(*values)
+        self.totals.inc_by(inc, *values)
 
-    def render_item(self, k):
-        keystr = self._render_key(k)
-
-        return ["%s:count%s %d" % (self.name, keystr, self.counts[k]),
-                "%s:total%s %d" % (self.name, keystr, self.totals[k])]
+    def render(self):
+        return self.counts.render() + self.totals.render()
 
 
 class CacheMetric(object):
diff --git a/tests/metrics/test_metric.py b/tests/metrics/test_metric.py
index 1ca3e45a26..6009014297 100644
--- a/tests/metrics/test_metric.py
+++ b/tests/metrics/test_metric.py
@@ -35,8 +35,7 @@ class CounterMetricTestCase(unittest.TestCase):
             'scalar 1',
         ])
 
-        counter.inc()
-        counter.inc()
+        counter.inc_by(2)
 
         self.assertEquals(counter.render(), [
             'scalar 3'
@@ -125,8 +124,8 @@ class DistributionMetricTestCase(unittest.TestCase):
 
         self.assertEquals(metric.render(), [
             'queries:count{verb="INSERT"} 1',
-            'queries:total{verb="INSERT"} 800',
             'queries:count{verb="SELECT"} 2',
+            'queries:total{verb="INSERT"} 800',
             'queries:total{verb="SELECT"} 500',
         ])
 
-- 
cgit 1.4.1