Opened 3 months ago

Closed 3 months ago

#35364 closed Cleanup/optimization (fixed)

AdminEmailHandler wastes work when ADMINS isn’t set

Reported by: Adam Johnson Owned by: Adam Johnson
Component: Error reporting Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

AdminEmailHandler.emit() does a lot of work to assemble the message it passes to mail_admins. If settings.ADMINS is empty, mail_admins() returns instantly, wasting all the message-creation work. It’s quite common to not configure ADMINS, whether in lieu of more advanced tools like Sentry, or during tests.

In a quick benchmark on my M1 Mac Pro on Python 3.11, the overhead is ~2.5ms:

In [1]: import logging

In [2]: logger = logging.getLogger('django')

In [3]: %timeit logger.error("Yada")
...
2.78 ms ± 75.4 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

In [4]: logger = logging.getLogger('example')

In [5]: %timeit logger.error("Yada")
...
8.37 µs ± 38.9 ns per loop (mean ± std. dev. of 7 runs, 100,000 loops each)

This can be avoided by adding an initial check to AdminEmailHandler.emit().

Change History (6)

comment:1 by Adam Johnson, 3 months ago

Owner: set to Adam Johnson
Status: newassigned

comment:2 by Natalia Bidart, 3 months ago

Triage Stage: UnreviewedAccepted

Makes sense, thank you!

comment:3 by Adam Johnson, 3 months ago

Has patch: set

comment:4 by Mariusz Felisiak, 3 months ago

Triage Stage: AcceptedReady for checkin

comment:5 by Sarah Boyce <42296566+sarahboyce@…>, 3 months ago

In b0f2289:

Refs #35364 -- Tested AdminEmailHandler with empty ADMINS.

comment:6 by Sarah Boyce <42296566+sarahboyce@…>, 3 months ago

Resolution: fixed
Status: assignedclosed

In 50a702f:

Fixed #35364 -- Stopped AdminEmailHandler rendering email unnecessarily.

Note: See TracTickets for help on using tickets.
Back to Top