Merge pull request #1407 from mozilla-services/feat/1400b

feat: fixup metrics + tags

autopush/metrics.py

@@ -8,8 +8,7 @@
 
 from twisted.internet import reactor
 
-import datadog
-from datadog import ThreadStats
+import markus
 
 from autopush import logging
 
@@ -65,56 +64,55 @@ def make_tags(base=None, **kwargs):
     return tags
 
 
-class DatadogMetrics(object):
-    """DataDog Metric backend"""
-    def __init__(self, api_key=None, app_key=None,
-                 hostname=None, statsd_host=None, statsd_port=None,
-                 flush_interval=10, namespace="autopush"):
-
-        datadog.initialize(
-            api_key=api_key,
-            app_key=app_key,
-            host_name=hostname,
-            statsd_host=statsd_host,
-            statsd_port=statsd_port,
-        )
-        self._client = ThreadStats()
-        self._flush_interval = flush_interval
+class TaggedMetrics(IMetrics):
+    """DataDog like tagged Metric backend"""
+    def __init__(self, hostname, statsd_host=None, statsd_port=None,
+                 namespace="autopush"):
+        markus.configure(
+            backends=[{
+                'class': 'markus.backends.datadog.DatadogMetrics',
+                'options': {
+                    'statsd_host': statsd_host,
+                    'statsd_port': statsd_port,
+                }}])
+        self._client = markus.get_metrics(namespace)
         self._host = hostname
         self._namespace = namespace
 
     def _prefix_name(self, name):
-        return "%s.%s" % (self._namespace, name)
+        return name
 
     def start(self):
-        self._client.start(flush_interval=self._flush_interval,
-                           roll_up_interval=self._flush_interval)
+        pass
 
-    def increment(self, name, count=1, **kwargs):
-        self._client.increment(self._prefix_name(name), count, host=self._host,
-                               **kwargs)
+    def _make_tags(self, tags):
+        if tags is None:
+            tags = []
+        tags.append('host:%s' % self._host)
+        return tags
 
-    def gauge(self, name, count, **kwargs):
-        self._client.gauge(self._prefix_name(name), count, host=self._host,
-                           **kwargs)
+    def increment(self, name, count=1, tags=None, **kwargs):
+        self._client.incr(self._prefix_name(name), count,
+                          tags=self._make_tags(tags))
 
-    def timing(self, name, duration, **kwargs):
+    def gauge(self, name, count, tags=None, **kwargs):
+        self._client.gauge(self._prefix_name(name), count,
+                           tags=self._make_tags(tags))
+
+    def timing(self, name, duration, tags=None, **kwargs):
         self._client.timing(self._prefix_name(name), value=duration,
-                            host=self._host, **kwargs)
+                            tags=self._make_tags(tags))
 
 
 def from_config(conf):
     # type: (AutopushConfig) -> IMetrics
     """Create an IMetrics from the given config"""
     if conf.statsd_host:
-        return DatadogMetrics(
+        return TaggedMetrics(
             hostname=logging.instance_id_or_hostname if conf.ami_id else
             conf.hostname,
-            api_key=conf.datadog_api_key,
-            app_key=conf.datadog_app_key,
             statsd_host=conf.statsd_host,
             statsd_port=conf.statsd_port,
-            flush_interval=conf.datadog_flush_interval,
         )
     else:
         return SinkMetrics()

autopush/tests/test_integration.py

@@ -49,7 +49,7 @@
 from autopush.logging import begin_or_register
 from autopush.main import ConnectionApplication, EndpointApplication
 from autopush.utils import base64url_encode, normalize_id
-from autopush.metrics import SinkMetrics, DatadogMetrics
+from autopush.metrics import SinkMetrics, TaggedMetrics
 import autopush.tests
 from autopush.tests.support import _TestingLogObserver
 from autopush.websocket import PushServerFactory
@@ -538,7 +538,7 @@ def test_legacy_simplepush_record(self):
         db.Message.all_channels = safe
         yield self.shut_down(client)
 
-    @patch("autopush.metrics.datadog")
+    @patch("autopush.metrics.markus")
     @inlineCallbacks
     def test_webpush_data_delivery_to_disconnected_client(self, m_ddog):
         tests = {
@@ -553,8 +553,8 @@ def test_webpush_data_delivery_to_disconnected_client(self, m_ddog):
                 data=b"\xc3\x28\xa0\xa1\xe2\x28\xa1", result="wyigoeIooQ"),
         }
         # Piggy back a check for stored source metrics
-        self.conn.db.metrics = DatadogMetrics(
-            "someapikey", "someappkey", namespace="testpush",
+        self.conn.db.metrics = TaggedMetrics(
+            namespace="testpush",
             hostname="localhost")
         self.conn.db.metrics._client = Mock()
 
@@ -584,8 +584,8 @@ def test_webpush_data_delivery_to_disconnected_client(self, m_ddog):
             yield client.ack(chan, result["version"])
 
         assert self.logs.logged_ci(lambda ci: 'message_size' in ci)
-        inc_call = self.conn.db.metrics._client.increment.call_args_list[5]
-        assert inc_call[1]['tags'] == ['source:Stored']
+        inc_call = self.conn.db.metrics._client.incr.call_args_list[5]
+        assert inc_call[1]['tags'] == ['source:Stored', 'host:localhost']
         yield self.shut_down(client)
 
     @inlineCallbacks

autopush/tests/test_metrics.py

@@ -5,7 +5,7 @@
 
 from autopush.metrics import (
     IMetrics,
-    DatadogMetrics,
+    TaggedMetrics,
     SinkMetrics,
     periodic_reporter,
 )
@@ -32,27 +32,23 @@ def test_passing(self):
         assert sm.timing("test", 10) is None
 
 
-class DatadogMetricsTestCase(unittest.TestCase):
-    @patch("autopush.metrics.datadog")
-    def test_basic(self, mock_dog):
-        hostname = "localhost"
-
-        m = DatadogMetrics("someapikey", "someappkey", namespace="testpush",
-                           hostname="localhost")
-        assert len(mock_dog.mock_calls) > 0
+class TaggedMetricsTestCase(unittest.TestCase):
+    @patch("autopush.metrics.markus")
+    def test_basic(self, mock_tag):
+        m = TaggedMetrics(namespace="testpush", hostname="localhost")
+        assert len(mock_tag.mock_calls) > 0
         m._client = Mock()
         m.start()
-        m._client.start.assert_called_with(flush_interval=10,
-                                           roll_up_interval=10)
         m.increment("test", 5)
-        m._client.increment.assert_called_with("testpush.test", 5,
-                                               host=hostname)
+        # Namespace is now auto-prefixed by the underlying markus lib
+        m._client.incr.assert_called_with("test", 5,
+                                          tags=['host:localhost'])
         m.gauge("connection_count", 200)
-        m._client.gauge.assert_called_with("testpush.connection_count", 200,
-                                           host=hostname)
+        m._client.gauge.assert_called_with("connection_count", 200,
+                                           tags=['host:localhost'])
         m.timing("lifespan", 113)
-        m._client.timing.assert_called_with("testpush.lifespan", value=113,
-                                            host=hostname)
+        m._client.timing.assert_called_with("lifespan", value=113,
+                                            tags=['host:localhost'])
 
 
 class PeriodicReporterTestCase(unittest.TestCase):

docs/api/metrics.rst

@@ -21,7 +21,7 @@ Implementations
     :special-members: __init__
     :member-order: bysource
 
-.. autoclass:: DatadogMetrics
+.. autoclass:: TaggedMetrics
     :members:
     :special-members: __init__
     :member-order: bysource

requirements.in

@@ -2,7 +2,7 @@ apns
 attrs
 autobahn[twisted]
 boto3
-cffi
+cffi ; platform_python_implementation == "CPython"
 click
 configargparse
 cryptography
@@ -13,6 +13,7 @@ firebase-admin
 hyper
 marshmallow
 marshmallow-polyfield
+markus
 oauth2client
 objgraph
 pyasn1
@@ -25,7 +26,6 @@ service-identity
 simplejson
 treq
 twisted
--e git+https://github.com/habnabit/txstatsd.git@157ef85fbdeafe23865c7c4e176237ffcb3c3f1f#egg=txStatsD-master
 typing
 ua-parser
 wsaccel ; platform_python_implementation == "CPython"

requirements.txt

@@ -1,4 +1,3 @@
--e git+https://github.com/habnabit/txstatsd.git@157ef85fbdeafe23865c7c4e176237ffcb3c3f1f#egg=txStatsD-master
 apns==2.0.1
 attrs==19.3.0
 autobahn[twisted]==19.11.2
@@ -8,15 +7,16 @@ botocore==1.14.11         # via boto3, s3transfer
 cachecontrol==0.12.6      # via firebase-admin
 cachetools==3.1.1         # via google-auth
 certifi==2019.11.28       # via requests
-cffi==1.13.2
+cffi==1.13.2 ; platform_python_implementation == "CPython"
 chardet==3.0.4            # via requests
 click==7.0
 configargparse==1.0
+configparser==4.0.2       # via datadog
 constantly==15.1.0        # via twisted
 contextlib2==0.6.0.post1  # via raven
 cryptography==2.8
 cyclone==1.2
-datadog==0.34.0
+datadog==0.37.1
 decorator==4.4.1          # via datadog
 docutils==0.15.2          # via botocore
 ecdsa==0.15               # via python-jose
@@ -45,6 +45,7 @@ idna==2.8                 # via hyperlink, requests, twisted
 incremental==17.5.0       # via treq, twisted
 ipaddress==1.0.23         # via cryptography, service-identity
 jmespath==0.9.4           # via boto3, botocore
+markus==2.2.0
 marshmallow-polyfield==4.2
 marshmallow==2.19.5
 msgpack==0.6.2            # via cachecontrol