Merge pull request #28 from olxbr/fix/prometheus-with-no-labels

Changing api to force a label if not set

.travis.yml

@@ -7,7 +7,7 @@ python:
 # command to install dependencies
 install:
   - make setup
-  - pip install python-coveralls  # for coveralls
+  - pip install coveralls  # for coveralls
 # command to run tests
 script:
   - make all-tests
@@ -23,7 +23,7 @@ deploy:
   on:
     branch: master
     tags: true
-    condition: $TRAVIS_PYTHON_VERSION = "3.6"
+    condition: $TRAVIS_PYTHON_VERSION = "3.7"
 
 after_success: coveralls
 

Makefile

@@ -10,10 +10,10 @@ lint:
 	@flake8 barterdude tests_unit tests_integration
 
 test:
-	@nosetests -s --exclude="tests_integration" --with-coverage --cover-erase --cover-package=barterdude
+	@pytest --ignore="tests_integration" --cov=barterdude
 
 integration:
-	@nosetests -s --exclude="tests_unit"
+	@pytest --ignore="tests_unit"
 
 all-tests: | test integration lint check-sec
 

README.md

@@ -54,7 +54,7 @@ healthcheck = Healthcheck(barterdude) # automatic and customizable healthcheck
 prometheus = Prometheus(barterdude, labels) # automatic and customizable Prometheus integration
 
 self.logger = getLogger("my_app", logging.DEBUG) # automatic json log with barterdude namespace
-# BARTERDUDE_DEFAULT_LOG_NAME is an env var to control log namespace
+# BARTERDUDE_DEFAULT_APP_NAME is an env var to control your project namespace
 # BARTERDUDE_DEFAULT_LOG_LEVEL is an env var to control loglevel by number 0, 10, 20, etc...
 
 monitor = Monitor(

barterdude/conf.py

@@ -2,8 +2,8 @@
 import os
 from pythonjsonlogger import jsonlogger
 
-BARTERDUDE_DEFAULT_LOG_NAME = os.environ.get(
-    "BARTERDUDE_DEFAULT_LOG_NAME", "barterdude"
+BARTERDUDE_DEFAULT_APP_NAME = os.environ.get(
+    "BARTERDUDE_DEFAULT_APP_NAME", "barterdude"
 )
 BARTERDUDE_DEFAULT_LOG_LEVEL = int(
     os.environ.get(
@@ -18,7 +18,7 @@
     '(levelname) (name) (pathname) (lineno)',
     timestamp=True
 ))
-default_logger = logging.getLogger(BARTERDUDE_DEFAULT_LOG_NAME)
+default_logger = logging.getLogger(BARTERDUDE_DEFAULT_APP_NAME)
 
 
 def getLogger(name, level=BARTERDUDE_DEFAULT_LOG_LEVEL):

barterdude/hooks/metrics/prometheus/__init__.py

@@ -32,7 +32,7 @@ def __init__(
         self.__labels = labels
         self.__metrics = Metrics(self.__registry)
         self.__definitions = Definitions(
-            self.__registry, self.__metrics, list(labels.keys())
+            self.__registry, self.__metrics, list(self.__labels.keys())
         )
         self._msg_start = {}
         self.__definitions.save_metrics()
@@ -44,10 +44,11 @@ def metrics(self):
 
     async def before_consume(self, message: RabbitMQMessage):
         hash_message = id(message)
-        self.metrics[self.__definitions.BEFORE_CONSUME].labels(
-            **self.__labels
-        ).inc()
         self._msg_start[hash_message] = time.time()
+        metric = self.metrics[self.__definitions.BEFORE_CONSUME]
+        if self.__labels:
+            metric = metric.labels(**self.__labels)
+        metric.inc()
 
     async def _on_complete(self,
                            message: RabbitMQMessage,
@@ -58,9 +59,8 @@ async def _on_complete(self,
         hash_message = id(message)
         labels = self.__labels.copy()
         labels["state"] = state
-        labels["error"] = str(type(error)) if (error) else None
-        self.metrics[state].labels(**labels).inc()
-        self.metrics[self.__definitions.TIME_MEASURE].labels(
+        labels["error"] = str(type(error)) if error else ""
+        self.metrics[self.__definitions.HISTOGRAM_MEASURE].labels(
             **labels).observe(
                 final_time - self._msg_start.pop(hash_message)
         )
@@ -72,9 +72,10 @@ async def on_fail(self, message: RabbitMQMessage, error: Exception):
         await self._on_complete(message, self.__definitions.FAIL, error)
 
     async def on_connection_fail(self, error: Exception, retries: int):
-        self.metrics[self.__definitions.CONNECTION_FAIL].labels(
-            **self.__labels
-        ).inc()
+        metric = self.metrics[self.__definitions.CONNECTION_FAIL]
+        if self.__labels:
+            metric = metric.labels(**self.__labels)
+        metric.inc()
 
     async def __call__(self, req: web.Request):
         return web.Response(

barterdude/hooks/metrics/prometheus/definitions.py

@@ -17,7 +17,7 @@ class Definitions:
     BEFORE_CONSUME = "before_consume"
     SUCCESS = "success"
     FAIL = "fail"
-    TIME_MEASURE = "time_measure"
+    HISTOGRAM_MEASURE = "histogram_measure"
     CONNECTION_FAIL = "connection_fail"
 
     def __init__(
@@ -38,18 +38,8 @@ def save_metrics(self):
             namespace=self.NAMESPACE,
             unit=self.MESSAGE_UNITS,
         )
-        self._prepare_on_complete(
-            self.SUCCESS,
-            namespace=self.NAMESPACE,
-            unit=self.MESSAGE_UNITS,
-        )
-        self._prepare_on_complete(
-            self.FAIL,
-            namespace=self.NAMESPACE,
-            unit=self.MESSAGE_UNITS,
-        )
-        self._prepare_time_measure(
-            self.TIME_MEASURE,
+        self._prepare_histogram_measure(
+            self.HISTOGRAM_MEASURE,
             namespace=self.NAMESPACE,
             unit=self.TIME_UNITS,
         )
@@ -60,7 +50,7 @@ def save_metrics(self):
         )
 
     def _prepare_before_consume(
-            self, name: str, namespace: str = "", unit: str = ""):
+            self, name: str, namespace: str, unit: str):
 
         self.__metrics[name] = Counter(
                 name="received_number_before_consume",
@@ -71,26 +61,13 @@ def _prepare_before_consume(
                 registry=self.__registry
             )
 
-    def _prepare_on_complete(
-            self, state: str, namespace: str = "", unit: str = ""):
-
-        self.__metrics[state] = Counter(
-                name=f"consumed_number_on_{state}",
-                documentation=(f"Messages that worker consumed with {state}"
-                               " from queue(s)"),
-                labelnames=self.__labelkeys + ["state", "error"],
-                namespace=namespace,
-                unit=unit,
-                registry=self.__registry
-            )
-
-    def _prepare_time_measure(
-            self, name: str, namespace: str = "", unit: str = ""):
+    def _prepare_histogram_measure(
+            self, name: str, namespace: str, unit: str):
 
         self.__metrics[name] = Histogram(
-                name="time_spent_processing_message",
-                documentation=("Time spent when function was "
-                               "processing a message"),
+                name="processing_message",
+                documentation=("Histogram for time and counter "
+                               "when function was processing a message"),
                 buckets=self.__time_buckets,
                 labelnames=self.__labelkeys + ["state", "error"],
                 namespace=namespace,
@@ -103,7 +80,7 @@ def _prepare_on_connection_fail(
 
         self.__metrics[state] = Counter(
                 name="connection_fail",
-                documentation=("Number of times barterdude failed "
+                documentation=("Number of times failed "
                                "to connect to the AMQP broker"),
                 labelnames=self.__labelkeys,
                 namespace=namespace,

requirements/requirements_test.txt

@@ -1,7 +1,8 @@
 -r requirements_prometheus.txt
-nose==1.3.7
-coverage<5
-bandit<2.0.0
-flake8<4.0.0
+pytest>5.0.0,<6.0.0
+pytest-cov>2.0.0,<3.0.0
+coverage>5.0,<6.0
+bandit>1.0,<2.0.0
+flake8>3.0.0,<4.0.0
 asynctest==0.13.0
 freezegun==0.3.14

tests_integration/test_barterdude.py

@@ -90,8 +90,8 @@ async def handler(message):
                 status_code = response.status
                 text = await response.text()
 
-        self.assertEquals(status_code, 200)
-        self.assertEquals(
+        self.assertEqual(status_code, 200)
+        self.assertEqual(
             text,
             '{"message": "Success rate: 1.0 (expected: 0.95)", '
             '"fail": 0, "success": 1, "status": "ok"}'
@@ -118,8 +118,8 @@ async def handler(message):
                 status_code = response.status
                 text = await response.text()
 
-        self.assertEquals(status_code, 200)
-        self.assertEquals(
+        self.assertEqual(status_code, 200)
+        self.assertEqual(
             text,
             '{"message": "Success rate: 1.0 (expected: 0.95)", '
             '"fail": 0, "success": 1, "status": "ok"}'
@@ -147,10 +147,52 @@ async def handler(message):
                 status_code = response.status
                 text = await response.text()
 
-        self.assertEquals(status_code, 200)
-        self.assertNotEquals(-1, text.find(
+        self.assertEqual(status_code, 200)
+        self.assertNotEqual(-1, text.find(
             'barterdude_received_number_before_consume_messages_total'
             '{app_name="barterdude_consumer"} 1.0'))
+        self.assertNotEqual(-1, text.find(
+            'barterdude_processing_message_seconds_bucket{app_name='
+            '"barterdude_consumer",error="",le="0.025",state="success"} 1.0'
+        ))
+        self.assertNotEqual(-1, text.find(
+            'barterdude_processing_message_seconds_count'
+            '{app_name="barterdude_consumer",error="",state="success"} 1.0'
+        ))
+
+    async def test_obtains_prometheus_metrics_without_labels(self):
+        monitor = Monitor(Prometheus(self.app))
+
+        @self.app.consume_amqp([self.input_queue], monitor)
+        async def handler(message):
+            pass
+
+        await self.app.startup()
+        await self.queue_manager.put(
+            routing_key=self.input_queue,
+            data=self.messages[0]
+        )
+        await asyncio.sleep(1)
+
+        async with aiohttp.ClientSession() as session:
+            timeout = aiohttp.ClientTimeout(total=1)
+            url = f'http://{self.barterdude_host}:8080/metrics'
+            async with session.get(url, timeout=timeout) as response:
+                status_code = response.status
+                text = await response.text()
+
+        self.assertEqual(status_code, 200)
+        self.assertNotEqual(-1, text.find(
+            'barterdude_received_number_before_consume_messages_total 1.0'
+        ))
+        self.assertNotEqual(-1, text.find(
+            'barterdude_processing_message_seconds_bucket'
+            '{error="",le="0.025",state="success"} 1.0'
+        ))
+        self.assertNotEqual(-1, text.find(
+            'barterdude_processing_message_seconds_count'
+            '{error="",state="success"} 1.0'
+        ))
 
     async def test_register_multiple_prometheus_hooks(self):
         """This test raised the following error:
@@ -171,9 +213,8 @@ async def handler(message):
             if message.body == self.messages[0]:
                 raise error
 
-        await self.app.startup()
-
         with self.assertLogs("barterdude") as cm:
+            await self.app.startup()
             await self.queue_manager.put(
                 routing_key=self.input_queue,
                 data=self.messages[0]
@@ -207,9 +248,8 @@ async def handler(message):
             if message.body == self.messages[0]:
                 raise error
 
-        await self.app.startup()
-
         with self.assertLogs("barterdude") as cm:
+            await self.app.startup()
             await self.queue_manager.put(
                 routing_key=self.input_queue,
                 data=self.messages[0]

tests_integration/test_rabbitmq_integration.py

@@ -146,7 +146,7 @@ async def handler(message):
             data=self.messages[0]
         )
         await asyncio.sleep(1)
-        self.assertEquals(received_message, self.messages[1])
+        self.assertEqual(received_message, self.messages[1])
 
     async def test_process_message_requeue_with_requeue(self):
         handler_called = 0
@@ -164,7 +164,7 @@ async def handler(messages):
             data=self.messages[0]
         )
         await asyncio.sleep(1)
-        self.assertEquals(handler_called, 2)
+        self.assertEqual(handler_called, 2)
 
     async def test_process_message_reject_with_requeue(self):
         handler_called = 0
@@ -184,7 +184,7 @@ async def handler(messages):
             data=self.messages[0]
         )
         await asyncio.sleep(2)
-        self.assertEquals(handler_called, 2)
+        self.assertEqual(handler_called, 2)
 
     async def test_process_message_reject_by_validation_with_requeue(self):
         handler_called = 0
@@ -204,7 +204,7 @@ async def handler(messages):
             data=self.messages[0]
         )
         await asyncio.sleep(2)
-        self.assertEquals(handler_called, 2)
+        self.assertEqual(handler_called, 2)
 
     async def test_process_message_reject_without_requeue(self):
         handler_called = 0
@@ -224,7 +224,7 @@ async def handler(message):
             data=self.messages[0]
         )
         await asyncio.sleep(2)
-        self.assertEquals(handler_called, 1)
+        self.assertEqual(handler_called, 1)
 
     async def test_process_message_reject_by_validation_without_requeue(self):
         handler_called = 0
@@ -243,7 +243,7 @@ async def handler(messages):
             data=self.messages[0]
         )
         await asyncio.sleep(2)
-        self.assertEquals(handler_called, 1)
+        self.assertEqual(handler_called, 1)
 
     async def test_process_message_reject_by_validation_before_handler(self):
         handler_called = 0
@@ -260,7 +260,7 @@ async def handler(messages):
             data={"wrong": "key"}
         )
         await asyncio.sleep(1)
-        self.assertEquals(handler_called, 0)
+        self.assertEqual(handler_called, 0)
 
     async def test_process_messages_and_requeue_only_one(self):
         first_read = set()

tests_unit/test_conf.py

@@ -3,14 +3,14 @@
 
 from asynctest import TestCase
 from barterdude.conf import (
-    getLogger, BARTERDUDE_DEFAULT_LOG_NAME, BARTERDUDE_DEFAULT_LOG_LEVEL
+    getLogger, BARTERDUDE_DEFAULT_APP_NAME, BARTERDUDE_DEFAULT_LOG_LEVEL
 )
 
 
 class TestConf(TestCase):
 
     def setUp(self):
-        self.log_name = BARTERDUDE_DEFAULT_LOG_NAME
+        self.log_name = BARTERDUDE_DEFAULT_APP_NAME
         self.log_level = BARTERDUDE_DEFAULT_LOG_LEVEL
 
     async def test_should_get_log_with_default_configs(self):

tests_unit/test_hooks/test_logging.py

@@ -1,7 +1,7 @@
 import logging
 from asynctest import TestCase, Mock, patch
 from barterdude.hooks import logging as hook_log
-from barterdude.conf import BARTERDUDE_DEFAULT_LOG_NAME
+from barterdude.conf import BARTERDUDE_DEFAULT_APP_NAME
 
 
 class TestLogging(TestCase):
@@ -23,7 +23,7 @@ async def test_should_access_default_logger(self):
             logging.StreamHandler
         )
         self.assertEqual(
-            logger.name, f"{BARTERDUDE_DEFAULT_LOG_NAME}.my_log")
+            logger.name, f"{BARTERDUDE_DEFAULT_APP_NAME}.my_log")
         self.assertEqual(logger.level, logging.DEBUG)
 
     @patch("barterdude.hooks.logging.Logging.logger")