Added tags management in WSGI interface. closes #3

README.rst

@@ -284,23 +284,31 @@ You can group several metrics together by "tagging" them::
     {'group1': set(['test1', 'test3'])}
     >>> metrics.metrics_by_tag("group1")
     {'test1': {'arithmetic_mean': 0.0, 'skewness': 0.0, 'harmonic_mean': 0.0, 'min': 0, 'standard_deviation': 0.0, 'median': 0.0, 'histogram': [(0, 0)], 'percentile': [(50, 0.0), (75, 0.0), (90, 0.0), (95, 0.0), (99, 0.0), (99.9, 0.0)], 'n': 0, 'max': 0, 'variance': 0.0, 'geometric_mean': 0.0, 'kurtosis': 0.0}, 'test3': {'count': 0, 'five': 0.0, 'mean': 0.0, 'fifteen': 0.0, 'day': 0.0, 'one': 0.0}}
+    >>> metrics.untag('test1', 'group1')
+    True
+    >>> metrics.untag('test1', 'group1')
+    False
+
 
-As you can see above, three functions are available:
+As you can see above, four functions are available:
 
  * ``metrics.tag(metric_name, tag_name)``: tag the metric named ``<metric_name>`` with ``<tag_name>``.
    Raise ``InvalidMetricError`` if ``<metric_name>`` does not exist.
  * ``metrics.tags()``: return the currently defined tags.
  * ``metrics.metrics_by_tag(tag_name)``: return a dictionary with metric names as keys
    and metric values as returned by ``<metric_object>.get()``. Return an empty dictionary if ``tag_name`` does
    not exist.
+ * ``metrics.untag(metric_name, tag_name)``: remove the tag named ``<metric_name>`` from the metric named
+ ``<metric_name>``. Return True if the tag was removed, False if either the metric or the tag did not exist. When a
+   tag is no longer used, it gets implicitly removed.
 
 
 External access
 ---------------
 
 You can access the metrics provided by ``AppMetrics`` externally by the ``WSGI``
 middleware found in ``appmetrics.wsgi.AppMetricsMiddleware``. It is a standard ``WSGI``
-middleware without external dependencies and it can be plugged in any framework supporting
+middleware with only ``werkzeug`` as external dependency and it can be plugged in any framework supporting
 the ``WSGI`` standard, for example in a ``Flask`` application::
 
     from flask import Flask
@@ -343,9 +351,9 @@ As usual, instantiate the middleware with the wrapped ``WSGI`` application; it l
 request paths starting with ``"/_app-metrics"``: if not found, the wrapped application
 is called. The following resources are defined:
 
-``/_app-metrics``
+``/_app-metrics/metrics``
   - **GET**: return the list of the registered metrics
-``/_app-metrics/<name>``
+``/_app-metrics/metrics/<name>``
   - **GET**: return the value of the given metric or ``404``.
   - **PUT**: create a new metric with the given name. The body must be a ``JSON`` object with a
     mandatory attribute named ``"type"`` which must be one of the metrics types allowed,
@@ -356,8 +364,22 @@ is called. The following resources are defined:
     attribute named ``"value"``: the notify method will be called with the given value.
     Other attributes are ignored.
     Request's ``content-type`` must be ``"application/json"``.
-
-
+  - **DELETE**: remove the metric with the given name. Return "deleted" or "not deleted".
+``/_app-metrics/tags``
+  - **GET**: return the list of registered tags
+``/_app-metrics/tags/<name>``
+  - **GET**: return the metrics tagged with the given tag. If the value of the ``GET`` parameter ``"expand"``
+    is ``"true"``, a JSON object is returned, with the name of each tagged metric as keys and corresponding values.
+    If it is ``"false"`` or not provided, the list of metric names is returned.
+    Return a ``404`` if the tag does not exist
+``/_app-metrics/tags/<tag_name>/<metric_name>``
+  - **PUT**: tag the metric named ``<metric_name>`` with ``<tag_name>``. Return a ``400`` if the given metric
+    does not exist.
+  - **DELETE**: remove the tag ``<tag_name>`` from ``<metric_name>``. Return "deleted" or "not deleted". If
+    ``<tag_name>`` is no longer used, it gets implicitly removed.
+
+
+The response body is always encoded in JSON, and the ``Content-Type`` is ``application/json``.
 The root doesn't have to be ``"/_app-metrics"``, you can customize it by providing your own to
 the middleware constructor.
 

appmetrics/metrics.py

@@ -264,6 +264,28 @@ def metrics_by_tag(tag_name):
     return metrics_by_name_list(names)
 
 
+def untag(name, tag_name):
+    """
+    Remove the given tag from the given metric.
+    Return True if the metric was tagged, False otherwise
+    """
+
+    with LOCK:
+        by_tag = TAGS.get(tag_name, None)
+        if not by_tag:
+            return False
+        try:
+            by_tag.remove(name)
+
+            # remove the tag if no associations left
+            if not by_tag:
+                TAGS.pop(tag_name)
+
+            return True
+        except KeyError:
+            return False
+
+
 def metrics_by_name_list(names):
     """
     Return a dictionary with {metric name: metric value} for all the metrics with the given names.

appmetrics/tests/test_metrics.py

@@ -1,5 +1,5 @@
 import mock
-from nose.tools import assert_equal, assert_in, raises, assert_is, assert_is_instance, assert_false
+from nose.tools import assert_equal, assert_in, raises, assert_is, assert_is_instance, assert_false, assert_true
 
 from .. import metrics as mm, exceptions, histogram, simple_metrics as simple, meter
 
@@ -394,6 +394,31 @@ def test_tags(self):
         assert_equal(mm.tags(), mm.TAGS)
         assert_false(mm.tags() is mm.TAGS)
 
+    def test_untag_bad_tag(self):
+        mm.TAGS = {"1": {"test1", "test3"}, "2": {"test2"}}
+
+        assert_false(mm.untag("test1", "xxx"))
+
+    def test_untag_bad_metric(self):
+        mm.TAGS = {"1": {"test1", "test3"}, "2": {"test2"}}
+
+        assert_false(mm.untag("xxx", "1"))
+
+    def test_untag(self):
+        mm.TAGS = {"1": {"test1", "test3"}, "2": {"test2"}}
+
+        assert_true(mm.untag("test1", "1"))
+
+        assert_equal(mm.TAGS, {"1": {"test3"}, "2": {"test2"}})
+
+    def test_untag_last_group(self):
+        mm.TAGS = {"1": {"test1", "test3"}, "2": {"test2"}}
+
+        assert_true(mm.untag("test1", "1"))
+        assert_true(mm.untag("test3", "1"))
+
+        assert_equal(mm.TAGS, {"2": {"test2"}})
+
     def test_metrics_by_tag_invalid_tag(self):
         mm.TAGS = {"1": {"test1", "test3"}, "2": {"test2"}}
 

appmetrics/tests/test_wsgi.py

@@ -34,15 +34,15 @@ def check_dispatching(mw, url, method, expected):
 
 def test_dispatching():
     tests = [
-        ("/_app-metrics", 'GET', werkzeug.routing.RequestRedirect),
-        ("/_app-metrics/", 'GET', wsgi.handle_metrics_list),
-        ("/_app-metrics/", 'POST', werkzeug.exceptions.MethodNotAllowed),
-        ("/_app-metrics/test", 'GET', wsgi.handle_metric_show),
-        ("/_app-metrics/test", 'PUT', wsgi.handle_metric_new),
-        ("/_app-metrics/test", 'POST', wsgi.handle_metric_update),
-        ("/_app-metrics/test", 'DELETE', wsgi.handle_metric_delete),
-        ("/_app-metrics/test", 'OPTIONS', werkzeug.exceptions.MethodNotAllowed),
-        ("/_app-metrics/test/sub", 'GET', werkzeug.routing.NotFound),
+        ("/_app-metrics", 'GET', werkzeug.exceptions.NotFound),
+        ("/_app-metrics/metrics", 'GET', wsgi.handle_metrics_list),
+        ("/_app-metrics/metrics", 'POST', werkzeug.exceptions.MethodNotAllowed),
+        ("/_app-metrics/metrics/test", 'GET', wsgi.handle_metric_show),
+        ("/_app-metrics/metrics/test", 'PUT', wsgi.handle_metric_new),
+        ("/_app-metrics/metrics/test", 'POST', wsgi.handle_metric_update),
+        ("/_app-metrics/metrics/test", 'DELETE', wsgi.handle_metric_delete),
+        ("/_app-metrics/metrics/test", 'OPTIONS', werkzeug.exceptions.MethodNotAllowed),
+        ("/_app-metrics/metrics/test/sub", 'GET', werkzeug.routing.NotFound),
     ]
 
     mw = wsgi.AppMetricsMiddleware(None)
@@ -53,14 +53,14 @@ def test_dispatching():
 
 def test_dispatching_root():
     tests = [
-        ("/", 'GET', wsgi.handle_metrics_list),
-        ("/", 'POST', werkzeug.exceptions.MethodNotAllowed),
-        ("/test", 'GET', wsgi.handle_metric_show),
-        ("/test", 'PUT', wsgi.handle_metric_new),
-        ("/test", 'POST', wsgi.handle_metric_update),
-        ("/test", 'DELETE', wsgi.handle_metric_delete),
-        ("/test", 'OPTIONS', werkzeug.exceptions.MethodNotAllowed),
-        ("/test/sub", 'GET', werkzeug.routing.NotFound),
+        ("/metrics", 'GET', wsgi.handle_metrics_list),
+        ("/metrics", 'POST', werkzeug.exceptions.MethodNotAllowed),
+        ("/metrics/test", 'GET', wsgi.handle_metric_show),
+        ("/metrics/test", 'PUT', wsgi.handle_metric_new),
+        ("/metrics/test", 'POST', wsgi.handle_metric_update),
+        ("/metrics/test", 'DELETE', wsgi.handle_metric_delete),
+        ("/metrics/test", 'OPTIONS', werkzeug.exceptions.MethodNotAllowed),
+        ("/metrics/test/sub", 'GET', werkzeug.routing.NotFound),
         ]
 
     mw = wsgi.AppMetricsMiddleware(None, "")
@@ -105,20 +105,10 @@ def test_call_not_matching_3(self):
             self.app.call_args_list,
             [mock.call(env("/_app-metrics/test/sub"), self.start_response)])
 
-    def test_call_without_trailing_slash(self):
-        self.handler.side_effect = ValueError()
-
-        self.mw(env("/_app-metrics", REQUEST_METHOD='GET'), self.start_response)
-
-        assert_equal(
-            self.start_response.call_args_list,
-            [mock.call("301 MOVED PERMANENTLY", mock.ANY)]
-        )
-
     def test_call_with_invalid_status(self):
         self.handler.side_effect = ValueError()
 
-        self.mw(env("/_app-metrics/", REQUEST_METHOD='GET'), self.start_response)
+        self.mw(env("/_app-metrics/metrics", REQUEST_METHOD='GET'), self.start_response)
 
         assert_equal(
             self.start_response.call_args_list,
@@ -128,7 +118,7 @@ def test_call_with_invalid_status(self):
     def test_call_with_error_implicit(self):
         self.handler.side_effect = werkzeug.exceptions.BadRequest()
 
-        body = self.mw(env("/_app-metrics/", REQUEST_METHOD='GET'), self.start_response)
+        body = self.mw(env("/_app-metrics/metrics", REQUEST_METHOD='GET'), self.start_response)
 
         expected_body = json.dumps(werkzeug.exceptions.BadRequest.description)
         assert_equal(b"".join(body), expected_body.encode('utf8'))
@@ -145,7 +135,7 @@ def test_call_with_error_implicit(self):
     def test_call_with_error_explicit(self):
         self.handler.side_effect = werkzeug.exceptions.BadRequest(description="bad request received")
 
-        body = self.mw(env("/_app-metrics/", REQUEST_METHOD='GET'), self.start_response)
+        body = self.mw(env("/_app-metrics/metrics", REQUEST_METHOD='GET'), self.start_response)
 
         expected_body = json.dumps("bad request received")
 
@@ -160,10 +150,28 @@ def test_call_with_error_explicit(self):
             [mock.call("400 BAD REQUEST", expected_headers)]
         )
 
+    def test_call_with_invalid_method(self):
+        self.handler.side_effect = werkzeug.exceptions.BadRequest()
+
+        body = self.mw(env("/_app-metrics/metrics", REQUEST_METHOD='POST'), self.start_response)
+
+        expected_body = json.dumps(werkzeug.exceptions.MethodNotAllowed.description)
+        assert_equal(b"".join(body), expected_body.encode('utf8'))
+
+        expected_headers = [
+            ('Content-Type', 'application/json'),
+            ('Allow', 'HEAD, GET'),
+            ('Content-Length', str(len(expected_body)))
+        ]
+        assert_equal(
+            self.start_response.call_args_list,
+            [mock.call("405 METHOD NOT ALLOWED", expected_headers)]
+        )
+
     def test_call_ok(self):
         self.handler.return_value = json.dumps("results")
 
-        body = self.mw(env("/_app-metrics/", REQUEST_METHOD='GET'), self.start_response)
+        body = self.mw(env("/_app-metrics/metrics", REQUEST_METHOD='GET'), self.start_response)
 
         expected_headers = [
             ('Content-Type', 'application/json'),
@@ -185,7 +193,7 @@ def test_call_with_unicode(self):
         else:
             self.handler.return_value = json.dumps("results").decode('utf8')
 
-        body = self.mw(env("/_app-metrics/", REQUEST_METHOD='GET'), self.start_response)
+        body = self.mw(env("/_app-metrics/metrics", REQUEST_METHOD='GET'), self.start_response)
 
         expected_body = json.dumps("results")
         assert_equal(b"".join(body), expected_body.encode('utf8'))
@@ -205,9 +213,16 @@ def setUp(self):
         self.original_registry = metrics.REGISTRY
         metrics.REGISTRY.clear()
 
+        self.original_tags = metrics.TAGS
+        metrics.TAGS.clear()
+
     def tearDown(self):
+        metrics.REGISTRY.clear()
         metrics.REGISTRY.update(self.original_registry)
 
+        metrics.TAGS.clear()
+        metrics.TAGS.update(self.original_tags)
+
     @mock.patch('appmetrics.wsgi.metrics.metrics')
     def test_handle_metrics_list(self, metrics):
         metrics.return_value = ["test1", "test2"]
@@ -243,6 +258,64 @@ def test_handle_metric_delete_not_found(self):
             assert_equal(res, "not deleted")
             assert_equal(metrics.REGISTRY, dict(none="test"))
 
+    @mock.patch('appmetrics.wsgi.metrics.tags')
+    def test_handle_tags_list(self, tags):
+        tags.return_value = dict(tag1=["test1", "test2"], tag2=["test3"])
+
+        assert_equal(wsgi.handle_tags_list(mock.Mock()), '["tag1", "tag2"]')
+
+    def test_handle_tag_add(self):
+        metrics.REGISTRY["test1"] = mock.Mock()
+
+        res = wsgi.handle_tag_add(mock.Mock(), "tag1", "test1")
+
+        assert_equal(res, "")
+        assert_equal(metrics.TAGS, {"tag1": {"test1"}})
+
+    @raises(werkzeug.exceptions.BadRequest)
+    def test_handle_tag_add_invalid(self):
+        res = wsgi.handle_tag_add(mock.Mock(), "tag1", "test1")
+
+        assert_equal(res, "")
+        assert_equal(metrics.TAGS, {"tag1": {"test1"}})
+
+    def test_handle_untag_not_existing(self):
+        res = wsgi.handle_untag(mock.Mock(), "tag1", "test1")
+        assert_equal(res, "not deleted")
+
+    def test_handle_untag(self):
+        metrics.TAGS["tag1"] = {"test1"}
+
+        res = wsgi.handle_untag(mock.Mock(), "tag1", "test1")
+        assert_equal(res, "deleted")
+
+    @raises(werkzeug.exceptions.NotFound)
+    def test_handle_tag_show_not_found(self):
+        wsgi.handle_tag_show(mock.Mock(), "tag1")
+
+    def test_handle_tag_show(self):
+        metrics.new_histogram("test1")
+        metrics.tag("test1", "tag1")
+
+        res = wsgi.handle_tag_show(mock.Mock(), "tag1")
+        assert_equal(res, '["test1"]')
+
+    def test_handle_tag_show_no_expand(self):
+        metrics.new_histogram("test1")
+        metrics.tag("test1", "tag1")
+
+        res = wsgi.handle_tag_show(mock.Mock(args={"expand": 'false'}), "tag1")
+        assert_equal(res, '["test1"]')
+
+    @mock.patch('appmetrics.metrics.metrics_by_tag')
+    def test_handle_tag_show_no_expand(self, mbt):
+        mbt.return_value = "this is a test"
+        metrics.new_histogram("test1")
+        metrics.tag("test1", "tag1")
+
+        res = wsgi.handle_tag_show(mock.Mock(args={"expand": 'true'}), "tag1")
+        assert_equal(res, '"this is a test"')
+
     @raises(werkzeug.exceptions.UnsupportedMediaType)
     def test_get_body_no_content_type(self):
         request = werkzeug.wrappers.Request(dict(CONTENT_LENGTH=10))