Fixed tests and added a new metric (#96)

paweldudzinski · korfuri · commit 6fd72b5031f3 · 2019-06-13T15:13:08.000+09:00
* Would be good to have those additional data in response

* Fix tests

* Fix end2end tests

* Bump version

* Eliminate flake8 errors

* More updates

* extend buckets in django_http_requests_latency_seconds_by_view_method
diff --git a/django_prometheus/__init__.py b/django_prometheus/__init__.py
@@ -2,12 +2,13 @@
 
 https://github.com/korfuri/django-prometheus
 """
-
 # Import all files that define metrics. This has the effect that
 # `import django_prometheus` will always instanciate all metric
 # objects right away.
-import django_prometheus.middleware
-import django_prometheus.models
+from django_prometheus import middleware
+from django_prometheus import models
+
+__all__ = ['middleware', 'models', 'pip_prometheus']
 
 # Import pip_prometheus to export the pip metrics automatically.
 try:
diff --git a/django_prometheus/apps.py b/django_prometheus/apps.py
@@ -1,13 +1,13 @@
 from django.apps import AppConfig
 from django.conf import settings
+
+import django_prometheus
 from django_prometheus.exports import SetupPrometheusExportsFromConfig
 from django_prometheus.migrations import ExportMigrations
-# unused import to force instantiating the metric objects at startup.
-import django_prometheus
 
 
 class DjangoPrometheusConfig(AppConfig):
-    name = 'django_prometheus'
+    name = django_prometheus.__name__
     verbose_name = 'Django-Prometheus'
 
     def ready(self):
diff --git a/django_prometheus/cache/backends/django_memcached_consul.py b/django_prometheus/cache/backends/django_memcached_consul.py
@@ -1,7 +1,11 @@
 from __future__ import absolute_import
-from django_memcached_consul import memcached
+
 from django_prometheus.cache.metrics import (
-    django_cache_get_total, django_cache_hits_total, django_cache_misses_total)
+    django_cache_get_total,
+    django_cache_hits_total,
+    django_cache_misses_total)
+
+from django_memcached_consul import memcached
 
 
 class MemcachedCache(memcached.MemcachedCache):
diff --git a/django_prometheus/cache/backends/locmem.py b/django_prometheus/cache/backends/locmem.py
@@ -1,6 +1,9 @@
 from django.core.cache.backends import locmem
+
 from django_prometheus.cache.metrics import (
-    django_cache_get_total, django_cache_hits_total, django_cache_misses_total)
+    django_cache_get_total,
+    django_cache_hits_total,
+    django_cache_misses_total)
 
 
 class LocMemCache(locmem.LocMemCache):
diff --git a/django_prometheus/cache/backends/memcached.py b/django_prometheus/cache/backends/memcached.py
@@ -1,6 +1,9 @@
 from django.core.cache.backends import memcached
+
 from django_prometheus.cache.metrics import (
-    django_cache_get_total, django_cache_hits_total, django_cache_misses_total)
+    django_cache_get_total,
+    django_cache_hits_total,
+    django_cache_misses_total)
 
 
 class MemcachedCache(memcached.MemcachedCache):
diff --git a/django_prometheus/cache/backends/redis.py b/django_prometheus/cache/backends/redis.py
@@ -1,12 +1,12 @@
-from django_redis import cache, exceptions
-
 from django_prometheus.cache.metrics import (
     django_cache_get_fail_total,
     django_cache_get_total,
     django_cache_hits_total,
     django_cache_misses_total,
 )
 
+from django_redis import cache, exceptions
+
 
 class RedisCache(cache.RedisCache):
     """Inherit redis to add metrics about hit/miss/interruption ratio"""
diff --git a/django_prometheus/db/__init__.py b/django_prometheus/db/__init__.py
@@ -1,2 +1,15 @@
 # Import all metrics
-from django_prometheus.db.metrics import *
+from django_prometheus.db.metrics import (
+    Counter,
+    connection_errors_total,
+    connections_total,
+    errors_total,
+    execute_many_total,
+    execute_total)
+__all__ = [
+    'Counter',
+    'connection_errors_total',
+    'connections_total',
+    'errors_total',
+    'execute_many_total',
+    'execute_total']
diff --git a/django_prometheus/db/backends/mysql/base.py b/django_prometheus/db/backends/mysql/base.py
@@ -1,7 +1,9 @@
-from django_prometheus.db.common import (
-    DatabaseWrapperMixin, ExportingCursorWrapper)
 from django.db.backends.mysql import base
 
+from django_prometheus.db.common import (
+    DatabaseWrapperMixin,
+    ExportingCursorWrapper)
+
 
 class DatabaseFeatures(base.DatabaseFeatures):
     """Our database has the exact same features as the base one."""
diff --git a/django_prometheus/db/backends/postgresql/base.py b/django_prometheus/db/backends/postgresql/base.py
@@ -1,8 +1,11 @@
 import django
-import psycopg2.extensions
 
-from django_prometheus.db.common import DatabaseWrapperMixin, \
+from django_prometheus.db.common import (
+    DatabaseWrapperMixin,
     ExportingCursorWrapper
+)
+
+import psycopg2.extensions
 
 if django.VERSION >= (1, 9):
     from django.db.backends.postgresql import base
diff --git a/django_prometheus/db/backends/sqlite3/base.py b/django_prometheus/db/backends/sqlite3/base.py
@@ -1,6 +1,7 @@
-from django_prometheus.db.common import DatabaseWrapperMixin
 from django.db.backends.sqlite3 import base
 
+from django_prometheus.db.common import DatabaseWrapperMixin
+
 
 class DatabaseFeatures(base.DatabaseFeatures):
     """Our database has the exact same features as the base one."""
diff --git a/django_prometheus/db/common.py b/django_prometheus/db/common.py
@@ -1,6 +1,10 @@
 from django_prometheus.db import (
-    connections_total, execute_total, execute_many_total, errors_total,
-    connection_errors_total)
+    connection_errors_total,
+    connections_total,
+    execute_total,
+    execute_many_total,
+    errors_total
+)
 
 
 class ExceptionCounterByType(object):
@@ -39,7 +43,7 @@ def get_new_connection(self, *args, **kwargs):
         try:
             return super(DatabaseWrapperMixin, self).get_new_connection(
                 *args, **kwargs)
-        except:
+        except Exception:
             connection_errors_total.labels(self.alias, self.vendor).inc()
             raise
 
diff --git a/django_prometheus/db/metrics.py b/django_prometheus/db/metrics.py
@@ -1,6 +1,5 @@
 from prometheus_client import Counter
 
-
 connections_total = Counter(
     'django_db_new_connections_total',
     'Counter of created connections by database and by vendor.',
diff --git a/django_prometheus/exports.py b/django_prometheus/exports.py
@@ -1,5 +1,11 @@
-from django.http import HttpResponse
+import socket
+import logging
+import os
+import prometheus_client
+import threading
+
 from django.conf import settings
+from django.http import HttpResponse
 from prometheus_client import multiprocess
 
 try:
@@ -8,11 +14,6 @@
 except ImportError:
     # Python 3
     from http.server import HTTPServer
-import socket
-import logging
-import os
-import prometheus_client
-import threading
 
 
 logger = logging.getLogger(__name__)
diff --git a/django_prometheus/middleware.py b/django_prometheus/middleware.py
@@ -1,6 +1,8 @@
+import django
 from prometheus_client import Counter, Histogram
+from prometheus_client.utils import INF
+
 from django_prometheus.utils import Time, TimeSince, PowersOf
-import django
 
 if django.VERSION >= (1, 10, 0):
     from django.utils.deprecation import MiddlewareMixin
@@ -43,7 +45,11 @@ def process_response(self, request, response):
 requests_latency_by_view_method = Histogram(
     'django_http_requests_latency_seconds_by_view_method',
     'Histogram of request processing time labelled by view.',
-    ['view', 'method'])
+    ['view', 'method'],
+    buckets=(.01, .025, .05, .075,
+             .1, .25, .5, .75,
+             1.0, 2.5, 5.0, 7.5,
+             10.0, 25.0, 50.0, 75.0, INF))
 requests_unknown_latency = Counter(
     'django_http_requests_unknown_latency_total',
     'Count of requests for which the latency was unknown.')
@@ -78,6 +84,10 @@ def process_response(self, request, response):
     'django_http_responses_total_by_status',
     'Count of responses by status.',
     ['status'])
+responses_by_status_view_method = Counter(
+    'django_http_responses_total_by_status_view_method',
+    'Count of responses by status, view, method.',
+    ['status', 'view', 'method'])
 responses_body_bytes = Histogram(
     'django_http_responses_body_total_bytes',
     'Histogram of responses by body size.',
@@ -147,7 +157,12 @@ def process_template_response(self, request, response):
         return response
 
     def process_response(self, request, response):
+        method = self._method(request)
+        name = self._get_view_name(request)
+
         responses_by_status.labels(str(response.status_code)).inc()
+        responses_by_status_view_method.labels(
+            response.status_code, name, method).inc()
         if hasattr(response, 'charset'):
             responses_by_charset.labels(str(response.charset)).inc()
         if hasattr(response, 'streaming') and response.streaming:
diff --git a/django_prometheus/migrations.py b/django_prometheus/migrations.py
@@ -1,5 +1,6 @@
 from django.db import connections
 from django.db.backends.dummy.base import DatabaseWrapper
+
 from prometheus_client import Gauge
 
 unapplied_migrations = Gauge(
diff --git a/django_prometheus/tests/end2end/testapp/models.py b/django_prometheus/tests/end2end/testapp/models.py
@@ -1,4 +1,5 @@
 from django.db.models import Model, CharField, PositiveIntegerField
+
 from django_prometheus.models import ExportModelOperationsMixin
 
 
diff --git a/django_prometheus/tests/end2end/testapp/settings.py b/django_prometheus/tests/end2end/testapp/settings.py
@@ -10,10 +10,11 @@
 https://docs.djangoproject.com/en/1.8/ref/settings/
 """
 
-import django
 # Build paths inside the project like this: os.path.join(BASE_DIR, ...)
 import os
 
+import django
+
 BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 
 
diff --git a/django_prometheus/tests/end2end/testapp/test_caches.py b/django_prometheus/tests/end2end/testapp/test_caches.py
@@ -1,9 +1,10 @@
 from django.core.cache import caches
 from django.test import TestCase
-from redis import RedisError
 
 from django_prometheus.testutils import PrometheusTestCaseMixin
 
+from redis import RedisError
+
 
 class TestCachesMetrics(PrometheusTestCaseMixin, TestCase):
     """Test django_prometheus.caches metrics."""
diff --git a/django_prometheus/tests/end2end/testapp/test_db.py b/django_prometheus/tests/end2end/testapp/test_db.py
@@ -1,8 +1,9 @@
 from unittest import skipUnless
 
-from django_prometheus.testutils import PrometheusTestCaseMixin
-from django.test import TestCase
 from django.db import connections
+from django.test import TestCase
+
+from django_prometheus.testutils import PrometheusTestCaseMixin
 
 
 @skipUnless(connections['test_db_1'].vendor == 'sqlite',
@@ -32,7 +33,7 @@ def testCounters(self):
         cursor_db1.execute('SELECT 3')
         try:
             cursor_db1.execute('this is clearly not valid SQL')
-        except:
+        except Exception:
             pass
 
         self.assertMetricEquals(
diff --git a/django_prometheus/tests/end2end/testapp/test_middleware.py b/django_prometheus/tests/end2end/testapp/test_middleware.py
@@ -1,19 +1,26 @@
-import django
+from django.test import SimpleTestCase
+
 from django_prometheus.testutils import PrometheusTestCaseMixin
+
 from testapp.views import ObjectionException
-from django.test import SimpleTestCase
-import unittest
 
 
 def M(metric_name):
-    """Make a full metric name from a short metric name.
+    """Makes a full metric name from a short metric name.
 
     This is just intended to help keep the lines shorter in test
     cases.
     """
     return 'django_http_%s' % metric_name
 
 
+def T(metric_name):
+    """Makes a full metric name from a short metric name like M(metric_name)
+
+    This method adds a '_total' postfix for metrics."""
+    return '%s_total' % M(metric_name)
+
+
 class TestMiddlewareMetrics(PrometheusTestCaseMixin, SimpleTestCase):
     """Test django_prometheus.middleware.
 
@@ -31,19 +38,22 @@ def test_request_counters(self):
         self.assertMetricDiff(r, 4, M('requests_before_middlewares_total'))
         self.assertMetricDiff(r, 4, M('responses_before_middlewares_total'))
         self.assertMetricDiff(
-            r, 3, M('requests_total_by_method'), method='GET')
+            r, 3, T('requests_total_by_method'),
+            method='GET')
         self.assertMetricDiff(
-            r, 1, M('requests_total_by_method'), method='POST')
+            r, 1, T('requests_total_by_method'),
+            method='POST')
         self.assertMetricDiff(
-            r, 4, M('requests_total_by_transport'), transport='http')
+            r, 4, T('requests_total_by_transport'),
+            transport='http')
         self.assertMetricDiff(
-            r, 2, M('requests_total_by_view_transport_method'),
+            r, 2, T('requests_total_by_view_transport_method'),
             view='testapp.views.index', transport='http', method='GET')
         self.assertMetricDiff(
-            r, 1, M('requests_total_by_view_transport_method'),
+            r, 1, T('requests_total_by_view_transport_method'),
             view='testapp.views.help', transport='http', method='GET')
         self.assertMetricDiff(
-            r, 1, M('requests_total_by_view_transport_method'),
+            r, 1, T('requests_total_by_view_transport_method'),
             view='testapp.views.index', transport='http', method='POST')
         # We have 3 requests with no post body, and one with a few
         # bytes, but buckets are cumulative so that is 4 requests with
@@ -56,18 +66,18 @@ def test_request_counters(self):
             None, M('responses_total_by_templatename'),
             templatename='help.html')
         self.assertMetricDiff(
-            r, 3, M('responses_total_by_templatename'),
+            r, 3, T('responses_total_by_templatename'),
             templatename='index.html')
         self.assertMetricDiff(
-            r, 4, M('responses_total_by_status'), status='200')
+            r, 4, T('responses_total_by_status'), status='200')
         self.assertMetricDiff(
             r, 0, M('responses_body_total_bytes_bucket'), le='0.0')
         self.assertMetricDiff(
             r, 3, M('responses_body_total_bytes_bucket'), le='128.0')
         self.assertMetricDiff(
             r, 4, M('responses_body_total_bytes_bucket'), le='8192.0')
         self.assertMetricDiff(
-            r, 4, M('responses_total_by_charset'), charset='utf-8')
+            r, 4, T('responses_total_by_charset'), charset='utf-8')
         self.assertMetricDiff(r, 0, M('responses_streaming_total'))
 
     def test_latency_histograms(self):
diff --git a/django_prometheus/tests/end2end/testapp/test_migrations.py b/django_prometheus/tests/end2end/testapp/test_migrations.py
@@ -1,8 +1,9 @@
-import django
-from django_prometheus.testutils import PrometheusTestCaseMixin
-from django.test import SimpleTestCase
-import unittest
 import sys
+
+from django.test import SimpleTestCase
+
+from django_prometheus.testutils import PrometheusTestCaseMixin
+
 if sys.version_info[:2] >= (3, 0):
     from unittest.mock import MagicMock
 else:
diff --git a/django_prometheus/tests/end2end/testapp/test_models.py b/django_prometheus/tests/end2end/testapp/test_models.py
diff --git a/django_prometheus/tests/end2end/testapp/urls.py b/django_prometheus/tests/end2end/testapp/urls.py
diff --git a/django_prometheus/tests/end2end/testapp/views.py b/django_prometheus/tests/end2end/testapp/views.py
diff --git a/django_prometheus/tests/test_django_prometheus.py b/django_prometheus/tests/test_django_prometheus.py
diff --git a/django_prometheus/tests/test_exports.py b/django_prometheus/tests/test_exports.py
diff --git a/django_prometheus/tests/test_testutils.py b/django_prometheus/tests/test_testutils.py
diff --git a/django_prometheus/testutils.py b/django_prometheus/testutils.py
diff --git a/django_prometheus/urls.py b/django_prometheus/urls.py
diff --git a/examples/prometheus/django.rules b/examples/prometheus/django.rules
diff --git a/setup.py b/setup.py
