launchpad-reviewers team mailing list archive

[Colin Watson <mp+407602@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:logging-context-oops into launchpad:master.

Commit message:
Add any OOPS ID to the Talisker logging context

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/407602

This is helpful when trying to track down problems in logs.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:logging-context-oops into launchpad:master.

diff --git a/lib/lp/services/webapp/errorlog.py b/lib/lp/services/webapp/errorlog.py
index ed25ace..35d83d2 100644
--- a/lib/lp/services/webapp/errorlog.py
+++ b/lib/lp/services/webapp/errorlog.py
@@ -21,6 +21,7 @@ import oops_timeline
 import pytz
 import six
 from six.moves.urllib.parse import urlparse
+from talisker.logs import logging_context
 from zope.component import getUtility
 from zope.component.interfaces import ObjectEvent
 from zope.error.interfaces import IErrorReportingUtility
@@ -174,6 +175,7 @@ def attach_http_request(report, context):
             info[1], '__lazr_webservice_error__', 500)
         if webservice_error / 100 != 5:
             request.oopsid = None
+            logging_context.push(oopsid=None)
             # Tell the oops machinery to ignore this error
             report['ignore'] = True
 
@@ -383,6 +385,7 @@ class ErrorReportingUtility:
         if request:
             request.oopsid = report.get('id')
             request.oops = report
+        logging_context.push(oopsid=report.get('id'))
         return report
 
     def _filter_bad_urls_by_referer(self, report):
diff --git a/lib/lp/services/webapp/tests/test_errorlog.py b/lib/lp/services/webapp/tests/test_errorlog.py
index b194a96..8f42df3 100644
--- a/lib/lp/services/webapp/tests/test_errorlog.py
+++ b/lib/lp/services/webapp/tests/test_errorlog.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for error logging & OOPS reporting."""
@@ -16,6 +16,7 @@ import oops_amqp
 import pytz
 import six
 from six.moves import http_client
+from talisker.logs import logging_context
 import testtools
 from timeline.timeline import Timeline
 from zope.interface import directlyProvides
@@ -115,6 +116,7 @@ class TestErrorReportingUtility(TestCaseWithFactory):
             report = utility.raising(sys.exc_info(), request)
 
         self.assertFalse('last_oops' in report)
+        self.assertEqual(report['id'], logging_context.flat['oopsid'])
         last_oopsid = request.oopsid
         try:
             raise ArbitraryException('foo')
@@ -123,6 +125,7 @@ class TestErrorReportingUtility(TestCaseWithFactory):
 
         self.assertTrue('last_oops' in report)
         self.assertEqual(report['last_oops'], last_oopsid)
+        self.assertEqual(report['id'], logging_context.flat['oopsid'])
 
     def test_raising_with_request(self):
         """Test ErrorReportingUtility.raising() with a request"""
@@ -167,6 +170,7 @@ class TestErrorReportingUtility(TestCaseWithFactory):
         # verify that the oopsid was set on the request
         self.assertEqual(request.oopsid, report['id'])
         self.assertEqual(request.oops, report)
+        self.assertEqual(report['id'], logging_context.flat['oopsid'])
 
     def test_raising_request_with_principal_person(self):
         utility = ErrorReportingUtility()
@@ -187,6 +191,7 @@ class TestErrorReportingUtility(TestCaseWithFactory):
         self.assertEqual(
             u'my-username, 42, account-name, description |\u25a0|',
             report['username'])
+        self.assertEqual(report['id'], logging_context.flat['oopsid'])
 
     def test_raising_request_with_principal_person_set_to_none(self):
         """
@@ -211,6 +216,7 @@ class TestErrorReportingUtility(TestCaseWithFactory):
         self.assertEqual(
             u'account-name, 42, account-name, description |\u25a0|',
             report['username'])
+        self.assertEqual(report['id'], logging_context.flat['oopsid'])
 
     def test_raising_with_xmlrpc_request(self):
         # Test ErrorReportingUtility.raising() with an XML-RPC request.