1
\$\begingroup\$

I have this middleware to log the number of queries, the time spent on each view and some errors and tracebacks.

This will be called on each request, so I had to put a 75ms timeout. I don't like it, but I feel like using async request for this is unnecessary, I don't care about the host response.

What do you think? Can I improve this in any way?

import sys
import traceback
import os
import json
import time
import requests
from django.conf import settings

CUSTOM_LOG_AGENT_HOST = 'http://localhost:9875'
CUSTOM_LOG_AGENT_HEADERS = {}
CUSTOM_LOG_AGENT_TIMEOUT = 0.0075

class SimpleMiddleware(object):
    def __init__(self, get_response):
        self.get_response = get_response
        # One-time configuration and initialization.

    def __call__(self, request):
        self.customLogAgent = {}

        request.start_time = time.time()
        # Code to be executed for each request before
        # the view (and later middleware) are called.
        response = self.get_response(request)
         # Code to be executed for each request/response after
        # the view is called.

        total = time.time() - request.start_time
        self.customLogAgent['queries'] = connection.queries
        self.customLogAgent['time'] = int(total * 1000)
        try:
            url = getattr(settings, 'CUSTOM_LOG_AGENT_HOST', CUSTOM_LOG_AGENT_HOST)
            headers = getattr(settings, 'CUSTOM_LOG_AGENT_HEADERS', CUSTOM_LOG_AGENT_HEADERS)
            timeout = getattr(settings, 'CUSTOM_LOG_AGENT_TIMEOUT', CUSTOM_LOG_AGENT_TIMEOUT)

            requests.post(url, headers=headers, json=self.customLogAgent, timeout=timeout)
        except requests.exceptions.RequestException as e:
            print(e)
        print(json.dumps(self.customLogAgent, indent=4))

        return response

    def process_view(self, request, view_func, view_args, view_kwargs):
        self.customLogAgent['view_func'] = view_func.__name__
        self.customLogAgent['view_kwargs'] = view_kwargs
        self.customLogAgent['url'] = request.get_full_path()


    def process_exception(self, request, exception):
        ltype, lvalue, ltb = sys.exc_info()
        error = {}
        error['type'] = ltype.__name__
        error['message'] = str(lvalue)
        error['line'] = ltb.tb_lineno
        error['traceback'] = ''.join(traceback.format_tb(ltb))
        self.customLogAgent['error'] = error
        return None
\$\endgroup\$
2
\$\begingroup\$

I believe that performance is extremely important on websites. I would consider any delay at all due to requests to a logging server unacceptable. I think you should move the logging call to a background thread, and return the response to the user as quickly as possible.

As usual, it's probably a good idea to put a cap on the maximum number of background threads. It could be even just one thread doing all the logging tasks in sequence.

Other than this, the code looks fine to me. It's simple and not easy to pick on.

\$\endgroup\$
  • 1
    \$\begingroup\$ Agreed. Probably keep an in-memory queue of logging messages, then dedicate a separate thread to pushing those messages to the logging server. \$\endgroup\$ – scnerd May 11 '18 at 17:04
1
\$\begingroup\$

When using Django, using Django timezone is pretty good practice. If your log agent is a custom build, put it inside the same VPC to decrease latency. Removing unused libraries is also good practice.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.