Merge pull request #1410 from mozilla-services/bug/1408

bug: regression: wrap metric calls in thread handler
mozilla-services · Jul 7, 2020 · 4a31b1e · 4a31b1e
2 parents 5299977 + b514712
commit 4a31b1e
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 20 deletions.
diff --git a/autopush/metrics.py b/autopush/metrics.py
@@ -6,7 +6,7 @@
     Sequence
 )
 
-from twisted.internet import (reactor, threads)
+from twisted.internet import reactor
 
 import markus
 
@@ -92,21 +92,27 @@ def _make_tags(self, tags):
         return tags
 
     def increment(self, name, count=1, tags=None, **kwargs):
-        threads.deferToThread(
-            self._client.incr(self._prefix_name(name), count,
-                              tags=self._make_tags(tags))
+        reactor.callInThread(
+            self._client.incr,
+            self._prefix_name(name),
+            count,
+            tags=self._make_tags(tags)
         )
 
     def gauge(self, name, count, tags=None, **kwargs):
-        threads.deferToThread(
-            self._client.gauge(self._prefix_name(name), count,
-                               tags=self._make_tags(tags))
+        reactor.callInThread(
+            self._client.gauge,
+            self._prefix_name(name),
+            count,
+            tags=self._make_tags(tags)
         )
 
     def timing(self, name, duration, tags=None, **kwargs):
-        threads.deferToThread(
-            self._client.timing(self._prefix_name(name), value=duration,
-                                tags=self._make_tags(tags))
+        reactor.callInThread(
+            self._client.timing,
+            self._prefix_name(name),
+            value=duration,
+            tags=self._make_tags(tags)
         )
 
 

diff --git a/autopush/tests/test_metrics.py b/autopush/tests/test_metrics.py
@@ -39,10 +39,10 @@ def test_basic(self, mock_tag):
         assert len(mock_tag.mock_calls) > 0
         m._client = Mock()
         m.start()
-        m.increment("test", 5)
+        yield m.increment("test", 5)
         # Namespace is now auto-prefixed by the underlying markus lib
-        m._client.incr.assert_called_with("test", 5,
-                                          tags=['host:localhost'])
+        m._client.increment.assert_called_with(
+            "test", 5, tags=['host:localhost'])
         m.gauge("connection_count", 200)
         m._client.gauge.assert_called_with("connection_count", 200,
                                            tags=['host:localhost'])

diff --git a/autopush/web/webpush.py b/autopush/web/webpush.py
@@ -94,8 +94,9 @@ def validate_uaid_month_and_chid(self, d):
             self.context["log"].debug(format="Dropping User", code=102,
                                       uaid_hash=hasher(result["uaid"]),
                                       uaid_record=repr(result))
-            self.context["metrics"].increment("updates.drop_user",
-                                              tags=make_tags(errno=102))
+            self.context["metrics"].increment(
+                "updates.drop_user",
+                tags=make_tags(errno=102))
             self.context["db"].router.drop_user(result["uaid"])
             raise InvalidRequest("No such subscription", status_code=410,
                                  errno=106)
@@ -142,7 +143,9 @@ def _validate_webpush(self, d, result):
             log.debug(format="Dropping User", code=102,
                       uaid_hash=hasher(uaid),
                       uaid_record=repr(result))
-            metrics.increment("updates.drop_user", tags=make_tags(errno=102))
+            metrics.increment(
+                "updates.drop_user",
+                tags=make_tags(errno=102))
             db.router.drop_user(uaid)
             raise InvalidRequest("No such subscription", status_code=410,
                                  errno=106)
@@ -152,7 +155,9 @@ def _validate_webpush(self, d, result):
             log.debug(format="Dropping User", code=103,
                       uaid_hash=hasher(uaid),
                       uaid_record=repr(result))
-            metrics.increment("updates.drop_user", tags=make_tags(errno=103))
+            metrics.increment(
+                "updates.drop_user",
+                tags=make_tags(errno=103))
             db.router.drop_user(uaid)
             raise InvalidRequest("No such subscription", status_code=410,
                                  errno=106)

diff --git a/autopush/websocket.py b/autopush/websocket.py
@@ -1319,7 +1319,9 @@ def process_nack(self, data):
                        user_agent=self.ps.user_agent, message_id=str(version),
                        code=code, **self.ps.raw_agent)
         mcode = code if code in NACK_CODES else 0
-        self.metrics.increment('ua.command.nack', tags=make_tags(code=mcode))
+        self.metrics.increment(
+            'ua.command.nack',
+            tags=make_tags(code=mcode))
         self.ps.stats.nacks += 1
 
     def check_missed_notifications(self, results, resume=False):
@@ -1373,8 +1375,9 @@ def send_notification(self, update):
     def emit_send_metrics(self, notif):
         if notif.topic:
             self.metrics.increment("ua.notification.topic")
-        self.metrics.increment('ua.message_data', notif.data_length,
-                               tags=make_tags(source=notif.source))
+        self.metrics.increment(
+            'ua.message_data', notif.data_length,
+            tags=make_tags(source=notif.source))
 
 
 class PushServerFactory(WebSocketServerFactory):