Skip to content

Conversation

corlettb
Copy link
Contributor

@corlettb corlettb commented May 20, 2025

https://trello.com/c/btX99C0D/1085-celery-workers-not-logging-critical-errors

What

  • Remove the warnings override. Feels a bit dodgy. It only means we miss a few items on startup
  • Allow celery beat and celery worker log levels to be specified independently and from our normal config setup.
  • Update tests to include celery beat log tests
  • Add test to ensure we do not print celery worker log messages if log level is CRITICAL.

Why

Previous implementation lacked beat testing and was using a non-standard configuration

@corlettb corlettb force-pushed the BC-celery-logging-mk-2 branch 2 times, most recently from 37b1777 to 5f3499e Compare May 21, 2025 10:15
@corlettb corlettb changed the title Bc celery logging mk 2 Celery Logging Update May 21, 2025
@corlettb corlettb marked this pull request as ready for review May 21, 2025 10:56
@corlettb corlettb force-pushed the BC-celery-logging-mk-2 branch from 5f3499e to 90b74ce Compare May 21, 2025 11:26
@risicle
Copy link
Member

risicle commented May 27, 2025

Add test to ensure we do not print celery worker log messages if log level is CRITICAL

I thought CRITICAL were the ones we did want 😕

@risicle
Copy link
Member

risicle commented May 27, 2025

I thought CRITICAL were the ones we did want 😕

Or perhaps by "worker logs" you meant the boring, lower-level worker logs, which indeed we don't want.

@corlettb
Copy link
Contributor Author

I thought CRITICAL were the ones we did want 😕

Or perhaps by "worker logs" you meant the boring, lower-level worker logs, which indeed we don't want.

I added the test to ensure we don't print any non-critical messages if CRITICAL has been specified as the log level. As we know the celery worker logs can leak PII I just wanted a double check to ensure the logic is correct.

Ben Corlett added 2 commits May 27, 2025 11:32
…s a few items on startup

Allow celery beat and celery worker log levels to be specified independently and from our normal config setup.
Update tests to include celery beat log tests
Add test to ensure we do not print celery worker log messages if log level is CRITICAL.
@corlettb corlettb force-pushed the BC-celery-logging-mk-2 branch from 90b74ce to 1ac276b Compare May 27, 2025 10:35
@corlettb corlettb merged commit b6f3cf8 into main May 27, 2025
2 checks passed
@corlettb corlettb deleted the BC-celery-logging-mk-2 branch May 27, 2025 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants