Custom logging condition (clean history) (#60)

* Move logging condition to a dedicated method

* Add test for custom logging check method

* Fix #52: implement custom mixin that logs only errors

rest_framework_tracking/mixins.py

@@ -4,18 +4,11 @@
 import traceback
 
 
-class LoggingMixin(object):
+class BaseLoggingMixin(object):
     logging_methods = '__all__'
 
     """Mixin to log requests"""
     def initial(self, request, *args, **kwargs):
-        """Set current time on request"""
-
-        # check if request method is being logged
-        if self.logging_methods != '__all__' and request.method not in self.logging_methods:
-            super(LoggingMixin, self).initial(request, *args, **kwargs)
-            return None
-
         # get IP
         ipaddr = request.META.get("HTTP_X_FORWARDED_FOR", None)
         if ipaddr:
@@ -53,7 +46,7 @@ def initial(self, request, *args, **kwargs):
         )
 
         # regular initial, including auth check
-        super(LoggingMixin, self).initial(request, *args, **kwargs)
+        super(BaseLoggingMixin, self).initial(request, *args, **kwargs)
 
         # add user to log after auth
         user = request.user
@@ -73,7 +66,7 @@ def initial(self, request, *args, **kwargs):
 
     def handle_exception(self, exc):
         # basic handling
-        response = super(LoggingMixin, self).handle_exception(exc)
+        response = super(BaseLoggingMixin, self).handle_exception(exc)
 
         # log error
         if hasattr(self.request, 'log'):
@@ -84,11 +77,9 @@ def handle_exception(self, exc):
 
     def finalize_response(self, request, response, *args, **kwargs):
         # regular finalize response
-        response = super(LoggingMixin, self).finalize_response(request, response, *args, **kwargs)
+        response = super(BaseLoggingMixin, self).finalize_response(request, response, *args, **kwargs)
 
-        # check if request method is being logged
-        if self.logging_methods != '__all__' and request.method not in self.logging_methods:
-            return response
+        # check if request is being logged
         if not hasattr(self.request, 'log'):
             return response
 
@@ -97,18 +88,38 @@ def finalize_response(self, request, response, *args, **kwargs):
         response_ms = int(response_timedelta.total_seconds() * 1000)
 
         # save to log
-        self.request.log.response = response.rendered_content
-        self.request.log.status_code = response.status_code
-        self.request.log.response_ms = response_ms
-        try:
-            self.request.log.save()
-        except:
-            # ensure that a DB error doesn't prevent API call to continue as expected
-            pass
+        if (self._should_log(request, response)):
+            self.request.log.response = response.rendered_content
+            self.request.log.status_code = response.status_code
+            self.request.log.response_ms = response_ms
+            try:
+                self.request.log.save()
+            except:
+                # ensure that a DB error doesn't prevent API call to continue as expected
+                pass
 
         # return
         return response
 
+    def _should_log(self, request, response):
+        """
+        Method that should return True if this request should be logged.
+        By default, check if the request method is in logging_methods.
+        """
+        return self.logging_methods == '__all__' or request.method in self.logging_methods
+
+
+class LoggingMixin(BaseLoggingMixin):
+    pass
+
+
+class LoggingErrorsMixin(BaseLoggingMixin):
+    """
+    Log only errors
+    """
+    def _should_log(self, request, response):
+        return response.status_code >= 400
+
 
 def _clean_data(data):
     """

tests/test_mixins.py

@@ -94,6 +94,16 @@ def test_logging_explicit(self):
         self.client.post('/explicit-logging')
         self.assertEqual(APIRequestLog.objects.all().count(), 1)
 
+    def test_custom_check_logging(self):
+        self.client.get('/custom-check-logging')
+        self.client.post('/custom-check-logging')
+        self.assertEqual(APIRequestLog.objects.all().count(), 1)
+
+    def test_errors_logging(self):
+        self.client.get('/errors-logging')
+        self.client.post('/errors-logging')
+        self.assertEqual(APIRequestLog.objects.all().count(), 1)
+
     def test_log_anon_user(self):
         self.client.get('/logging')
         log = APIRequestLog.objects.first()

tests/urls.py

@@ -10,6 +10,8 @@
     url(r'^logging$', test_views.MockLoggingView.as_view()),
     url(r'^slow-logging$', test_views.MockSlowLoggingView.as_view()),
     url(r'^explicit-logging$', test_views.MockExplicitLoggingView.as_view()),
+    url(r'^custom-check-logging$', test_views.MockCustomCheckLoggingView.as_view()),
+    url(r'^errors-logging$', test_views.MockLoggingErrorsView.as_view()),
     url(r'^session-auth-logging$', test_views.MockSessionAuthLoggingView.as_view()),
     url(r'^token-auth-logging$', test_views.MockTokenAuthLoggingView.as_view()),
     url(r'^json-logging$', test_views.MockJSONLoggingView.as_view()),

tests/views.py

@@ -5,7 +5,7 @@
 from rest_framework.views import APIView
 from rest_framework import serializers, viewsets, mixins
 from rest_framework.exceptions import APIException
-from rest_framework_tracking.mixins import LoggingMixin
+from rest_framework_tracking.mixins import LoggingErrorsMixin, LoggingMixin
 from rest_framework_tracking.models import APIRequestLog
 from tests.test_serializers import ApiRequestLogSerializer
 import time
@@ -37,6 +37,28 @@ def post(self, request):
         return Response('with logging')
 
 
+class MockCustomCheckLoggingView(LoggingMixin, APIView):
+    def _should_log(self, request, response):
+        """
+        Log only if response contains 'log'
+        """
+        return 'log' in response.data
+
+    def get(self, request):
+        return Response('with logging')
+
+    def post(self, request):
+        return Response('no recording')
+
+
+class MockLoggingErrorsView(LoggingErrorsMixin, APIView):
+    def get(self, request):
+        raise APIException('with logging')
+
+    def post(self, request):
+        return Response('no logging')
+
+
 class MockSessionAuthLoggingView(LoggingMixin, APIView):
     authentication_classes = (SessionAuthentication,)
     permission_classes = (IsAuthenticated,)