Finished tweaking the metric submission with the help of zas

brainzutils/metrics.py

@@ -29,28 +29,51 @@ def decorated(*args, **kwargs):
 
 @cache.init_required
 @metrics_init_required
-def set(metric_name: str, tags: Dict[str,str]=None, timestamp: int=None, **fields):
+def set(metric_name: str, tags: Dict[str,str]={}, timestamp: int=None, **fields):
     """
         Submit a metric to the MetaBrainz influx datastore for graphing/monitoring
-        purposes. TBC.
+        purposes.
+
+        Args:
+          metric_name: The name of the metric to record.
+          tags: Additional influx tags to write with the metric. (optional)
+          timestamp: A nanosecond timestamp to use for this metric. If not provided
+                     the current time is used.
+          fields: The key, value pairs to store with this metric.
     """
 
-    hostid = os.environ['PRIVATE_IP']
-    if not hostid:
-        hostid = socket.gethostname()
+    # Add types to influx data
+    try:
+        host = os.environ['PRIVATE_IP']
+    except KeyError:
+        host = socket.gethostname()
 
     tags["dc"] = "hetzner"
-    tags["server"] = hostid
+    tags["server"] = host
+    tags["project"] = _metrics_project_name
     tag_string = ",".join([ "%s=%s" % (k, tags[k]) for k in tags ])
 
-    fields = " ".join([ "%s=%s" % (k, fields[k]) for k in fields ])
+    fields_list = []
+    for k in fields:
+        if type(fields[k]) == int:
+            fields_list.append("%s=%di" % (k, fields[k]))
+        elif type(fields[k]) == bool and fields[k] == True:
+            fields_list.append("%s=t" % (k))
+        elif type(fields[k]) == bool and fields[k] == False:
+            fields_list.append("%s=f" % (k))
+        elif type(fields[k]) == str:
+            fields_list.append('%s="%s"' % (k, fields[k]))
+        else:
+            fields_list.append("%s=%s" % (k, str(fields[k])))
+
+    fields = " ".join(fields_list)
 
     if timestamp is None:
         timestamp = time_ns()
 
     metric = "%s,%s %s %d" % (metric_name, tag_string, fields, timestamp)
     try:
-        cache.rpush(REDIS_METRICS_KEY, metric)
+        cache._r.rpush(REDIS_METRICS_KEY, metric)
     except Exception:
         # If we fail to push the metric to redis, so be it.
         pass

brainzutils/test/test_metrics.py

@@ -1,3 +1,4 @@
+import os
 from unittest import mock, TestCase
 from freezegun import freeze_time
 from redis import ResponseError
@@ -14,107 +15,10 @@ def setUp(self):
     def tearDown(self):
         metrics._metrics_project_name = None
 
-    @mock.patch('brainzutils.metrics.cache.hincrby')
-    def test_increment(self, hincrby):
+    @mock.patch('brainzutils.metrics.cache._r.rpush')
+    def test_set(self, rpush):
         metrics.init('listenbrainz.org')
-        metrics.increment('test_m', 2)
-        hincrby.assert_called_with('listenbrainz.org', 'test_m', 2, namespace='metrics')
-
-    @mock.patch('brainzutils.metrics.cache.hincrby')
-    def test_increment_default(self, hincrby):
-        metrics.init('listenbrainz.org')
-        metrics.increment('test_m')
-        hincrby.assert_called_with('listenbrainz.org', 'test_m', 1, namespace='metrics')
-
-    def test_increment_negative(self):
-        metrics.init('listenbrainz.org')
-        with self.assertRaises(ValueError):
-            metrics.increment('test_m', -2)
-
-    def test_increment_badname(self):
-        metrics.init('listenbrainz.org')
-        with self.assertRaises(ValueError):
-            metrics.increment('tag')
-
-    def test_increment_noinit(self):
-        with self.assertRaises(RuntimeError):
-            metrics.increment('test_m')
-
-    @mock.patch('brainzutils.metrics.cache.hincrby')
-    @mock.patch('brainzutils.metrics.cache.hset')
-    def test_increment_overflow(self, hset, hincrby):
-        hincrby.side_effect = [ResponseError("increment or decrement would overflow"), 10]
-        metrics.init('listenbrainz.org')
-        metrics.increment('test_m', 10)
-
-        hincrby.assert_has_calls([mock.call('listenbrainz.org', 'test_m', 10, namespace='metrics'),
-                                  mock.call('listenbrainz.org', 'test_m', 10, namespace='metrics')])
-        hset.assert_called_with('listenbrainz.org', 'test_m', 0, namespace='metrics')
-
-    @mock.patch('brainzutils.metrics.cache.hdel')
-    def test_remove(self, hdel):
-        metrics.init('listenbrainz.org')
-        metrics.remove('test_m')
-        hdel.assert_called_with('listenbrainz.org', ['test_m'], namespace='metrics')
-
-    @mock.patch('brainzutils.metrics.cache.hgetall')
-    def test_stats(self, hgetall):
-        metrics.init('listenbrainz.org')
-        hgetall.return_value = {'valueone': b'1',
-                                'valuetwo': b'20',
-                                'somethingelse': b'8'}
-
-        stats = metrics.stats()
-        hgetall.assert_called_with('listenbrainz.org', namespace='metrics')
-
-        expected = {'valueone': 1,
-                    'valuetwo': 20,
-                    'somethingelse': 8,
-                    'tag': 'listenbrainz.org'}
-
-        self.assertEqual(stats, expected)
-
-    @freeze_time('2021-02-15T10:22:00')
-    @mock.patch('brainzutils.metrics.cache.hset')
-    @mock.patch('brainzutils.metrics.cache.delete')
-    def test_set_count(self, mock_del, hset):
-        metrics.init('listenbrainz.org')
-        metrics.set_count('dataimport', artists=10, recordings=2)
-
-        mock_del.assert_called_with('listenbrainz.org:dataimport', namespace='metrics')
-        hset.assert_has_calls([mock.call('listenbrainz.org:dataimport', 'artists', 10, namespace='metrics'),
-                               mock.call('listenbrainz.org:dataimport', 'recordings', 2, namespace='metrics'),
-                               mock.call('listenbrainz.org:dataimport', 'date', '2021-02-15T10:22:00', namespace='metrics')],
-                              any_order=True)
-
-    def test_set_count_invalid_values(self):
-        metrics.init('listenbrainz.org')
-        with self.assertRaises(ValueError):
-            metrics.set_count('dataimport', date=1)
-
-        with self.assertRaises(ValueError):
-            metrics.set_count('dataimport', artists='not-an-int')
-
-    @mock.patch('brainzutils.metrics.cache.delete')
-    def test_remove_count(self, mock_del):
-        metrics.init('listenbrainz.org')
-        metrics.remove_count('dataimport')
-        mock_del.assert_called_with('listenbrainz.org:dataimport', namespace='metrics')
-
-    @mock.patch('brainzutils.metrics.cache.hgetall')
-    def test_stats_count(self, hgetall):
-        metrics.init('listenbrainz.org')
-        hgetall.return_value = {'valueone': b'1',
-                                'valuetwo': b'20',
-                                'date': b'2021-02-12T13:02:18'}
-
-        stats = metrics.stats_count('dataimport')
-        hgetall.assert_called_with('listenbrainz.org:dataimport', namespace='metrics')
-
-        expected = {'valueone': 1,
-                    'valuetwo': 20,
-                    'date': '2021-02-12T13:02:18',
-                    'metric': 'dataimport',
-                    'tag': 'listenbrainz.org'}
-
-        self.assertEqual(stats, expected)
+        os.environ["PRIVATE_IP"] = "127.0.0.1"
+        metrics.set("my_metric", timestamp=1619629462352960742, test_i=2, test_fl=.3, test_t=True, test_f=False, test_s="gobble")
+        rpush.assert_called_with(metrics.REDIS_METRICS_KEY, 
+            'my_metric,dc=hetzner,server=127.0.0.1,project=listenbrainz.org test_i=2i test_fl=0.3 test_t=t test_f=f test_s="gobble" 1619629462352960742')