From 4fbcbeeea772d0a436dd05cb19b4e2fb156a78d7 Mon Sep 17 00:00:00 2001
From: Tim Abbott <tabbott@zulipchat.com>
Date: Thu, 13 Feb 2020 23:47:41 -0800
Subject: [PATCH] settings: Disable django.request logging at WARNING log
 level.

The comment explains this issue, but effectively, the upgrade to
Django 2.x means that Django's built-in django.request logger was
writing to our errors logs WARNING-level data for every 404 and 400
error.  We don't consider user errors to be a problem worth
highlighting in that log file.
---
 zerver/lib/logging_util.py | 33 ---------------------------------
 zproject/settings.py       | 14 ++++++++------
 2 files changed, 8 insertions(+), 39 deletions(-)

diff --git a/zerver/lib/logging_util.py b/zerver/lib/logging_util.py
index 6e2ca49ae3..94df744c89 100644
--- a/zerver/lib/logging_util.py
+++ b/zerver/lib/logging_util.py
@@ -5,7 +5,6 @@ from django.utils.timezone import utc as timezone_utc
 
 import hashlib
 import logging
-import re
 import threading
 import traceback
 from typing import Optional, Tuple
@@ -125,38 +124,6 @@ def skip_200_and_304(record: logging.LogRecord) -> bool:
 
     return True
 
-IGNORABLE_404_URLS = [
-    re.compile(r'^/apple-touch-icon.*\.png$'),
-    re.compile(r'^/favicon\.ico$'),
-    re.compile(r'^/robots\.txt$'),
-    re.compile(r'^/django_static_404.html$'),
-    re.compile(r'^/wp-login.php$'),
-]
-
-def skip_boring_404s(record: logging.LogRecord) -> bool:
-    """Prevents Django's 'Not Found' warnings from being logged for common
-    404 errors that don't reflect a problem in Zulip.  The overall
-    result is to keep the Zulip error logs cleaner than they would
-    otherwise be.
-
-    Assumes that its input is a django.request log record.
-    """
-    # Apparently, `status_code` is added by Django and is not an actual
-    # attribute of LogRecord; as a result, mypy throws an error if we
-    # access the `status_code` attribute directly.
-    if getattr(record, 'status_code') != 404:
-        return True
-
-    # We're only interested in filtering the "Not Found" errors.
-    if getattr(record, 'msg') != 'Not Found: %s':
-        return True
-
-    path = getattr(record, 'args', [''])[0]
-    for pattern in IGNORABLE_404_URLS:
-        if re.match(pattern, path):
-            return False
-    return True
-
 def skip_site_packages_logs(record: logging.LogRecord) -> bool:
     # This skips the log records that are generated from libraries
     # installed in site packages.
diff --git a/zproject/settings.py b/zproject/settings.py
index ea177197a8..41b8da2ca0 100644
--- a/zproject/settings.py
+++ b/zproject/settings.py
@@ -727,10 +727,6 @@ LOGGING = {
             '()': 'django.utils.log.CallbackFilter',
             'callback': zerver.lib.logging_util.skip_200_and_304,
         },
-        'skip_boring_404s': {
-            '()': 'django.utils.log.CallbackFilter',
-            'callback': zerver.lib.logging_util.skip_boring_404s,
-        },
         'skip_site_packages_logs': {
             '()': 'django.utils.log.CallbackFilter',
             'callback': zerver.lib.logging_util.skip_site_packages_logs,
@@ -807,8 +803,14 @@ LOGGING = {
             # configured; which is what we want for it.
         },
         'django.request': {
-            'level': 'WARNING',
-            'filters': ['skip_boring_404s'],
+            # We set this to ERROR to prevent Django's default
+            # low-value logs with lines like "Not Found: /robots.txt"
+            # from being logged for every HTTP 4xx error at WARNING
+            # level, which would otherwise end up spamming our
+            # errors.log.  We'll still get logs in errors.log
+            # including tracebacks for 5xx errors (i.e. Python
+            # exceptions).
+            'level': 'ERROR',
         },
         'django.security.DisallowedHost': {
             'handlers': ['file'],