Merge ~cjwatson/launchpad:logging-context-oops into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 3729a7eca89722612aa683ee01334989f8cc5706
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:logging-context-oops
Merge into: launchpad:master
Diff against target: 86 lines (+10/-1)
2 files modified
lib/lp/services/webapp/errorlog.py (+3/-0)
lib/lp/services/webapp/tests/test_errorlog.py (+7/-1)
Reviewer Review Type Date Requested Status
Ioana Lasc (community) Approve
Review via email: mp+407602@code.launchpad.net

Commit message

Add any OOPS ID to the Talisker logging context

Description of the change

This is helpful when trying to track down problems in logs.

To post a comment you must log in.
Revision history for this message
Ioana Lasc (ilasc) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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
21import pytz21import pytz
22import six22import six
23from six.moves.urllib.parse import urlparse23from six.moves.urllib.parse import urlparse
24from talisker.logs import logging_context
24from zope.component import getUtility25from zope.component import getUtility
25from zope.component.interfaces import ObjectEvent26from zope.component.interfaces import ObjectEvent
26from zope.error.interfaces import IErrorReportingUtility27from zope.error.interfaces import IErrorReportingUtility
@@ -174,6 +175,7 @@ def attach_http_request(report, context):
174 info[1], '__lazr_webservice_error__', 500)175 info[1], '__lazr_webservice_error__', 500)
175 if webservice_error / 100 != 5:176 if webservice_error / 100 != 5:
176 request.oopsid = None177 request.oopsid = None
178 logging_context.push(oopsid=None)
177 # Tell the oops machinery to ignore this error179 # Tell the oops machinery to ignore this error
178 report['ignore'] = True180 report['ignore'] = True
179181
@@ -383,6 +385,7 @@ class ErrorReportingUtility:
383 if request:385 if request:
384 request.oopsid = report.get('id')386 request.oopsid = report.get('id')
385 request.oops = report387 request.oops = report
388 logging_context.push(oopsid=report.get('id'))
386 return report389 return report
387390
388 def _filter_bad_urls_by_referer(self, report):391 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 @@
1# Copyright 2009-2019 Canonical Ltd. This software is licensed under the1# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Tests for error logging & OOPS reporting."""4"""Tests for error logging & OOPS reporting."""
@@ -16,6 +16,7 @@ import oops_amqp
16import pytz16import pytz
17import six17import six
18from six.moves import http_client18from six.moves import http_client
19from talisker.logs import logging_context
19import testtools20import testtools
20from timeline.timeline import Timeline21from timeline.timeline import Timeline
21from zope.interface import directlyProvides22from zope.interface import directlyProvides
@@ -115,6 +116,7 @@ class TestErrorReportingUtility(TestCaseWithFactory):
115 report = utility.raising(sys.exc_info(), request)116 report = utility.raising(sys.exc_info(), request)
116117
117 self.assertFalse('last_oops' in report)118 self.assertFalse('last_oops' in report)
119 self.assertEqual(report['id'], logging_context.flat['oopsid'])
118 last_oopsid = request.oopsid120 last_oopsid = request.oopsid
119 try:121 try:
120 raise ArbitraryException('foo')122 raise ArbitraryException('foo')
@@ -123,6 +125,7 @@ class TestErrorReportingUtility(TestCaseWithFactory):
123125
124 self.assertTrue('last_oops' in report)126 self.assertTrue('last_oops' in report)
125 self.assertEqual(report['last_oops'], last_oopsid)127 self.assertEqual(report['last_oops'], last_oopsid)
128 self.assertEqual(report['id'], logging_context.flat['oopsid'])
126129
127 def test_raising_with_request(self):130 def test_raising_with_request(self):
128 """Test ErrorReportingUtility.raising() with a request"""131 """Test ErrorReportingUtility.raising() with a request"""
@@ -167,6 +170,7 @@ class TestErrorReportingUtility(TestCaseWithFactory):
167 # verify that the oopsid was set on the request170 # verify that the oopsid was set on the request
168 self.assertEqual(request.oopsid, report['id'])171 self.assertEqual(request.oopsid, report['id'])
169 self.assertEqual(request.oops, report)172 self.assertEqual(request.oops, report)
173 self.assertEqual(report['id'], logging_context.flat['oopsid'])
170174
171 def test_raising_request_with_principal_person(self):175 def test_raising_request_with_principal_person(self):
172 utility = ErrorReportingUtility()176 utility = ErrorReportingUtility()
@@ -187,6 +191,7 @@ class TestErrorReportingUtility(TestCaseWithFactory):
187 self.assertEqual(191 self.assertEqual(
188 u'my-username, 42, account-name, description |\u25a0|',192 u'my-username, 42, account-name, description |\u25a0|',
189 report['username'])193 report['username'])
194 self.assertEqual(report['id'], logging_context.flat['oopsid'])
190195
191 def test_raising_request_with_principal_person_set_to_none(self):196 def test_raising_request_with_principal_person_set_to_none(self):
192 """197 """
@@ -211,6 +216,7 @@ class TestErrorReportingUtility(TestCaseWithFactory):
211 self.assertEqual(216 self.assertEqual(
212 u'account-name, 42, account-name, description |\u25a0|',217 u'account-name, 42, account-name, description |\u25a0|',
213 report['username'])218 report['username'])
219 self.assertEqual(report['id'], logging_context.flat['oopsid'])
214220
215 def test_raising_with_xmlrpc_request(self):221 def test_raising_with_xmlrpc_request(self):
216 # Test ErrorReportingUtility.raising() with an XML-RPC request.222 # Test ErrorReportingUtility.raising() with an XML-RPC request.

Subscribers

People subscribed via source and target branches

to status/vote changes: