From 736562de5168bf3462dd2b93da85b9c11e425e0c Mon Sep 17 00:00:00 2001
From: Nicolas ROCHE <nroche@entrouvert.com>
Date: Wed, 13 Nov 2019 15:05:34 +0100
Subject: [PATCH] logging: append connector response to ressource log (#31774)

---
 passerelle/base/models.py      | 18 ++++++++++--------
 passerelle/views.py            | 20 +++++++++++++++++++-
 tests/test_generic_endpoint.py | 21 +++++++++++++++++++++
 3 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/passerelle/base/models.py b/passerelle/base/models.py
index 5a9b3db5..a0014ce0 100644
--- a/passerelle/base/models.py
+++ b/passerelle/base/models.py
@@ -770,6 +770,7 @@ class ProxyLogger(object):
 
     def _log(self, levelname, message, *args, **kwargs):
         force = kwargs.pop('force', False)
+        ressource_log = None
         if self.connector.down() and not force:
             # don't log if the connector is known to be down
             return
@@ -825,7 +826,7 @@ class ProxyLogger(object):
                 (exc_type, exc_value, tb) = sys.exc_info()
                 attr['extra']['error_summary'] = traceback.format_exception_only(exc_type, exc_value)
 
-            ResourceLog.objects.create(**attr)
+            ressource_log = ResourceLog.objects.create(**attr)
 
         admins = settings.ADMINS
         logging_parameters = self.connector.logging_parameters
@@ -833,28 +834,29 @@ class ProxyLogger(object):
             admins = [('', x) for x in logging_parameters.trace_emails.splitlines()]
         with override_settings(ADMINS=admins):
             getattr(self._logger, levelname.lower())(message, *args, **kwargs)
+        return ressource_log
 
     def exception(self, message, *args, **kwargs):
         kwargs['exc_info'] = 1
-        self._log('ERROR', message, *args, **kwargs)
+        return self._log('ERROR', message, *args, **kwargs)
 
     def debug(self, message, *args, **kwargs):
-        self._log('DEBUG', message, *args, **kwargs)
+        return self._log('DEBUG', message, *args, **kwargs)
 
     def info(self, message, *args, **kwargs):
-        self._log('INFO', message, *args, **kwargs)
+        return self._log('INFO', message, *args, **kwargs)
 
     def warning(self, message, *args, **kwargs):
-        self._log('WARNING', message, *args, **kwargs)
+        return self._log('WARNING', message, *args, **kwargs)
 
     def critical(self, message, *args, **kwargs):
-        self._log('CRITICAL', message, *args, **kwargs)
+        return self._log('CRITICAL', message, *args, **kwargs)
 
     def error(self, message, *args, **kwargs):
-        self._log('ERROR', message, *args, **kwargs)
+        return self._log('ERROR', message, *args, **kwargs)
 
     def fatal(self, message, *args, **kwargs):
-        self._log('FATAL', message, *args, **kwargs)
+        return self._log('FATAL', message, *args, **kwargs)
 
 
 class HTTPResource(models.Model):
diff --git a/passerelle/views.py b/passerelle/views.py
index 657693f4..58df88a0 100644
--- a/passerelle/views.py
+++ b/passerelle/views.py
@@ -18,6 +18,7 @@ import datetime
 import hashlib
 import inspect
 import json
+from exceptions import UnicodeDecodeError
 
 from django.apps import apps
 from django.conf.urls import url
@@ -26,6 +27,7 @@ from django.core.exceptions import PermissionDenied
 from django.contrib.auth import logout as auth_logout
 from django.contrib.auth import views as auth_views
 from django.db import transaction
+from django.db.utils import DataError
 from django.http import HttpResponse, HttpResponseRedirect, Http404
 from django.views.decorators.csrf import csrf_exempt
 from django.views.generic import (
@@ -421,7 +423,7 @@ class GenericEndpointView(GenericConnectorMixin, SingleObjectMixin, View):
             payload.decode('utf-8')
         except UnicodeDecodeError:
             payload = '<BINARY PAYLOAD>'
-        connector.logger.info('endpoint %s %s (%r) ' %
+        ressource_log = connector.logger.info('endpoint %s %s (%r) ' %
                               (request.method, url, payload),
                               extra={
                                   'request': request,
@@ -432,17 +434,33 @@ class GenericEndpointView(GenericConnectorMixin, SingleObjectMixin, View):
                                   'connector_payload': payload
                               })
 
+        def log_connector_response(value, from_cache=False):
+            if not ressource_log:
+                return
+            if from_cache:
+                ressource_log.extra['from_cache'] = True
+            response = str(value)[:connector.logging_parameters.responses_max_size]
+            ressource_log.extra['response'] = force_text(response, errors='ignore')
+            try:
+                with transaction.atomic():
+                    ressource_log.save()
+            except DataError:
+                ressource_log.extra['response'] = '<binary>'
+                ressource_log.save()
+
         params = self.get_params(request, *args, **kwargs)
         if request.method == 'GET' and self.endpoint.endpoint_info.cache_duration:
             cache_key = hashlib.md5(
                 repr(self.get_object().slug) + repr(self.endpoint) + repr(params)).hexdigest()
             result = cache.get(cache_key)
             if result is not None:
+                log_connector_response(result, from_cache=True)
                 return result
 
         result = self.endpoint(request, **params)
         if request.method == 'GET' and self.endpoint.endpoint_info.cache_duration:
             cache.set(cache_key, result, self.endpoint.endpoint_info.cache_duration)
+        log_connector_response(result)
         return result
 
     def get(self, request, *args, **kwargs):
diff --git a/tests/test_generic_endpoint.py b/tests/test_generic_endpoint.py
index c841b3a1..ff49110d 100644
--- a/tests/test_generic_endpoint.py
+++ b/tests/test_generic_endpoint.py
@@ -115,6 +115,7 @@ def test_proxy_logger(mocked_get, caplog, app, arcgis):
     assert log.extra['connector'] == 'arcgis'
     assert log.extra['connector_endpoint'] == 'district'
     assert log.extra['connector_endpoint_method'] == 'GET'
+    assert "'text': u'HAUSSONVILLE / BLANDAN / MON DESERT / SAURUPT'" in log.extra['response']
     assert '/arcgis/test/district?' in log.extra['connector_endpoint_url']
 
     # Resource Generic Logger
@@ -142,6 +143,26 @@ def test_proxy_logger(mocked_get, caplog, app, arcgis):
     assert ResourceLog.objects.last().message == 'first warning'
     assert ResourceLog.objects.last().levelno == 30
 
+    # troncate connector response on Resource Custom DB Logger
+    ResourceLog.objects.all().delete()
+    parameters = arcgis.logging_parameters
+    parameters.responses_max_size = 20
+    parameters.save()
+    app.get('/arcgis/test/district', params={'lon': 6.172122, 'lat': 48.673836}, status=200)
+    assert ResourceLog.objects.count() == 1
+    log = ResourceLog.objects.filter(appname='arcgis', slug='test').first()
+    assert len(log.extra['response']) == 20
+    assert log.extra.get('from_cache') == None
+
+    # check connector response returned by cache
+    ResourceLog.objects.all().delete()
+    arcgis.district.endpoint_info.cache_duration = 5
+    app.get('/arcgis/test/district', params={'lon': 6.172122, 'lat': 48.673836}, status=200)
+    app.get('/arcgis/test/district', params={'lon': 6.172122, 'lat': 48.673836}, status=200)
+    assert ResourceLog.objects.count() == 2
+    log = ResourceLog.objects.filter(appname='arcgis', slug='test').all()[1]
+    assert len(log.extra['response']) == 20
+    assert log.extra['from_cache'] == True
 
 class FakeConnectorBase(object):
     slug = 'connector'
-- 
2.24.0

