From 20ab7a969088d24aa11b38057802a4c1691c73f1 Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Tue, 18 Feb 2020 14:22:47 +0100 Subject: [PATCH] misc: find logs corresponding to the same API call (#38157) --- passerelle/base/models.py | 9 +++++++-- passerelle/templates/passerelle/manage/log.html | 4 +++- passerelle/views.py | 16 +++++++--------- tests/test_generic_endpoint.py | 13 +++++++++++++ tests/test_manager.py | 4 ++++ tests/utils.py | 1 + 6 files changed, 35 insertions(+), 12 deletions(-) diff --git a/passerelle/base/models.py b/passerelle/base/models.py index 19eded49..03998c74 100644 --- a/passerelle/base/models.py +++ b/passerelle/base/models.py @@ -9,6 +9,7 @@ import sys import traceback import base64 import itertools +import uuid from django.apps import apps from django.conf import settings @@ -131,7 +132,7 @@ class BaseResource(models.Model): def __init__(self, *args, **kwargs): super(BaseResource, self).__init__(*args, **kwargs) - self.logger = ProxyLogger(connector=self) + self.logger = ProxyLogger(connector=self, transaction_id=str(uuid.uuid4())) def __str__(self): return self.title @@ -762,12 +763,13 @@ class ResourceStatus(models.Model): class ProxyLogger(object): - def __init__(self, connector, extra=None): + def __init__(self, connector, extra=None, transaction_id=None): self.connector = connector self.appname = connector.get_connector_slug() self.slug = connector.slug self.extra = extra or {} logger_name = 'passerelle.resource.%s.%s' % (self.appname, self.slug) + self.transaction_id = transaction_id self._logger = logging.getLogger(logger_name) self._logger.setLevel(connector.log_level) @@ -823,6 +825,9 @@ class ProxyLogger(object): return isinstance(value, (list, dict, bool) + six.integer_types + six.string_types) attr['extra'] = {key: value for key, value in extra.items() if is_json_serializable(value)} + if self.transaction_id: + attr['extra']['transaction_id'] = self.transaction_id + if getattr(request, 'META', None): if 'HTTP_X_FORWARDED_FOR' in request.META: sourceip = request.META.get('HTTP_X_FORWARDED_FOR', '').split(",")[0].strip() diff --git a/passerelle/templates/passerelle/manage/log.html b/passerelle/templates/passerelle/manage/log.html index 0dcf112b..c320ffa5 100644 --- a/passerelle/templates/passerelle/manage/log.html +++ b/passerelle/templates/passerelle/manage/log.html @@ -1,4 +1,4 @@ -{% load passerelle %} +{% load passerelle i18n %}
{% for key, value in logline.extra.items %} @@ -10,6 +10,8 @@ {% with val=value|censor %} {% if key == 'connector_endpoint_url' %} {{val}} + {% elif key == 'transaction_id' %} + {{val}} ({% trans "search for logs from the same call" %}) {% else %} {{val|linebreaksbr|urlize}} {% endif %} diff --git a/passerelle/views.py b/passerelle/views.py index 0fbdf379..207fc923 100644 --- a/passerelle/views.py +++ b/passerelle/views.py @@ -378,9 +378,9 @@ class GenericEndpointView(GenericConnectorMixin, SingleObjectMixin, View): @csrf_exempt def dispatch(self, request, *args, **kwargs): self.init_stuff(request, *args, **kwargs) - connector = self.get_object() + self.connector = self.get_object() self.endpoint = None - for name, method in inspect.getmembers(connector, inspect.ismethod): + for name, method in inspect.getmembers(self.connector, inspect.ismethod): if not hasattr(method, 'endpoint_info'): continue if not method.endpoint_info.name == kwargs.get('endpoint'): @@ -395,7 +395,7 @@ class GenericEndpointView(GenericConnectorMixin, SingleObjectMixin, View): self.endpoint = method if not self.endpoint: raise Http404() - if kwargs.get('endpoint') == 'up' and hasattr(connector.check_status, 'not_implemented'): + if kwargs.get('endpoint') == 'up' and hasattr(self.connector.check_status, 'not_implemented'): # hide automatic up endpoint if check_status method is not implemented raise Http404() return super(GenericEndpointView, self).dispatch(request, *args, **kwargs) @@ -407,7 +407,7 @@ class GenericEndpointView(GenericConnectorMixin, SingleObjectMixin, View): perm = self.endpoint.endpoint_info.perm if not perm: return True - return is_authorized(request, self.get_object(), perm) + return is_authorized(request, self.connector, perm) def perform(self, request, *args, **kwargs): if request.method.lower() not in self.endpoint.endpoint_info.methods: @@ -437,14 +437,13 @@ class GenericEndpointView(GenericConnectorMixin, SingleObjectMixin, View): # auto log request's inputs connector_name, endpoint_name = kwargs['connector'], kwargs['endpoint'] - connector = self.get_object() url = request.get_full_path() - payload = request.body[:connector.logging_parameters.requests_max_size] + payload = request.body[:self.connector.logging_parameters.requests_max_size] try: payload = payload.decode('utf-8') except UnicodeDecodeError: payload = '' - connector.logger.info('endpoint %s %s (%r) ' % + self.connector.logger.info('endpoint %s %s (%r) ' % (request.method, url, payload), extra={ 'request': request, @@ -478,8 +477,7 @@ class GenericEndpointView(GenericConnectorMixin, SingleObjectMixin, View): kwargs['other_params'] = match.kwargs elif kwargs.get('rest'): raise Http404() - connector = self.get_object() - return to_json(logger=connector.logger)(self.perform)(request, *args, **kwargs) + return to_json(logger=self.connector.logger)(self.perform)(request, *args, **kwargs) def post(self, request, *args, **kwargs): return self.get(request, *args, **kwargs) diff --git a/tests/test_generic_endpoint.py b/tests/test_generic_endpoint.py index 13e1440c..8939633e 100644 --- a/tests/test_generic_endpoint.py +++ b/tests/test_generic_endpoint.py @@ -147,6 +147,19 @@ def test_proxy_logger(mocked_get, caplog, app, arcgis): assert ResourceLog.objects.last().levelno == 30 +@mock.patch('requests.Session.send') +def test_proxy_logger_transaction_id(mocked_send, app, arcgis): + payload = open(os.path.join(os.path.dirname(__file__), 'data', 'nancy_arcgis', 'sigresponse.json')).read() + mocked_send.return_value = utils.FakedResponse(content=payload, status_code=200) + arcgis.log_evel = 'DEBUG' + arcgis.base_url = 'https://example.net/' + arcgis.save() + resp = app.get('/arcgis/test/district', params={'lon': 6.172122, 'lat': 48.673836}, status=200) + + log1, log2 = ResourceLog.objects.filter(appname='arcgis', slug='test').all() + assert log1.extra['transaction_id'] == log2.extra['transaction_id'] + + class FakeConnectorBase(object): slug = 'connector' diff --git a/tests/test_manager.py b/tests/test_manager.py index 01c81533..308c0850 100644 --- a/tests/test_manager.py +++ b/tests/test_manager.py @@ -188,9 +188,13 @@ def test_logs(app, admin_user): resp.form['q'] = '' resp = resp.form.submit() assert resp.text.count('
') == 4 + log_pk = re.findall(r'data-pk="(.*)"', resp.text)[0] base_url = re.findall(r'data-log-base-url="(.*)"', resp.text)[0] resp = app.get(base_url + log_pk + '/') + resp = resp.click('search for logs from the same call') + assert resp.text.count('') == 2 + resp = app.get(base_url + '12345' + '/', status=404) def test_logging_parameters(app, admin_user): diff --git a/tests/utils.py b/tests/utils.py index d8e56df6..62fc82bc 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -26,6 +26,7 @@ def setup_access_rights(obj): class FakedResponse(mock.Mock): + headers = {} def json(self): return json_loads(self.content) -- 2.20.1