Honor json_logs config in Celery worker startup#68916
Conversation
Add two-level config lookup for JSON logging in the Celery worker: - Check [celery] json_logs first (celery-specific override) - Fall back to [logging] json_logs (global setting, added in 3.2.0) - Fall back to False if neither is set This mirrors the existing CELERY_LOGGING_LEVEL / LOGGING_LEVEL fallback pattern in the same file. Also adds [celery] json_logs to provider.yaml config schema. Closes apache#68912
shahar1
left a comment
There was a problem hiding this comment.
Thanks for the contribution! Few comments are below to be addressed.
For your next contributions, please do not use prefixes in the PR's title ([ISSUE XXX] feat(...): ...) - I fixed it.
|
Celery tests currently fail |
…ad in TestWorkerJsonLogs
|
@shahar1 Updated to mock instead. The DB access isn't from a Celery backend — it's from Airflow's If you'd prefer we follow the |
shahar1
left a comment
There was a problem hiding this comment.
LGTM, I'll be happy for a second pair of eyes
* feat(celery): honor json_logs config in Celery worker startup Add two-level config lookup for JSON logging in the Celery worker: - Check [celery] json_logs first (celery-specific override) - Fall back to [logging] json_logs (global setting, added in 3.2.0) - Fall back to False if neither is set This mirrors the existing CELERY_LOGGING_LEVEL / LOGGING_LEVEL fallback pattern in the same file. Also adds [celery] json_logs to provider.yaml config schema. Closes apache#68912 * fix(celery): rename newsfragment to PR number, set version_added to undecided * fix(celery): drop unneeded backend marker, remove provider newsfragment * fix(celery): handle null schema default and avoid DB access in non-DB test mode * fix(celery): mock cli_action_loggers and restore executor_loader reload in TestWorkerJsonLogs
Closes #68912
Problem
The Celery worker ignores
[logging] json_logs/AIRFLOW__LOGGING__JSON_LOGS. It always callsconfigure_logging(output=sys.stdout.buffer)without passingjson_output, so structured JSON logging is never enabled on the worker even when configured globally.Solution
Add a two-level config lookup before calling
configure_logging:[celery] json_logs— worker-specific override, takes precedence.[logging] json_logs— global setting, used when the celery key is absent.False— default when neither key is configured.This mirrors the existing
[celery] CELERY_LOGGING_LEVEL/[logging] LOGGING_LEVELfallback pattern already present in the worker startup code.Changes
providers/celery/src/airflow/providers/celery/cli/celery_command.py: read[celery] json_logs→[logging] json_logs→Falseand passjson_outputtoconfigure_logging.providers/celery/provider.yaml+get_provider_info.py: newjson_logsconfig option under[celery].providers/celery/tests/unit/celery/cli/test_celery_command.py:TestWorkerJsonLogs— 3 tests covering (a) default is False, (b) global[logging]setting is inherited, (c)[celery]override takes precedence.providers/celery/docs/celery_executor.rst: new "Worker logging" section documenting the fallback order and Airflow 3.0/3.2 compatibility note.providers/celery/newsfragments/68912.feature.rst: changelog entry.Notes
AIRFLOW_V_3_0_PLUSguard is unchanged).conf.getboolean(..., fallback=None)is safe on 3.0/3.1 even when the[celery] json_logskey is absent.version_added: ~inprovider.yaml— version not yet decided; to be set by the release manager.