Prevent scheduler crash when process/thread missing from log format#69402
Prevent scheduler crash when process/thread missing from log format#69402anxkhn wants to merge 2 commits into
Conversation
When a record reaches the percent formatter without callsite information, for example a stdlib warning routed through the logging bridge, the process and thread fields are absent. The formatter fell back to the "(unknown)" string for them, so a format string using the numeric "%(process)d" or "%(thread)d" specifiers raised "TypeError: %d format: a real number is required" and could take down the scheduler at startup. Give those two numeric callsite parameters a numeric fallback of 0, the same way lineno is already handled, so the format never receives a string where a number is expected.
|
Nice! |
|
Confirmed the root cause: _LazyLogRecordDict.getitem fell back to the "(unknown)" string (missing key) or None (key present but None) for callsite params, so a format using %(process)d/%(thread)d raised "TypeError: %d format: a real number is required" when callsite info was absent (e.g. a stdlib warning routed through the logging bridge before CallsiteParameterAdder runs). Returning self.event.get(key) or 0 mirrors the lineno branch directly above it, and since CallsiteParameter.PROCESS.value/THREAD.value equal "process"/"thread", get(key) reads the same keys the callsite adder populates -- equivalent to the generic lookup below, just with a numeric default. The "(unknown)" fallback is correctly kept for the %s text params. The parametrized regression test covers both the missing-keys repro and the explicit-None case and is red before / green after. This also cleanly supersedes the now-closed #66230, which took a heavier route (live os.getpid()/threading.get_ident() plus a warning-bridge change in structlog.py). LGTM. |
When a log record reaches the percent formatter without callsite information, for
example a standard-library warning routed through Airflow's logging bridge, the
processandthreadfields are absent. The formatter fell back to the"(unknown)"string for every missing callsite field, so a log format usingthe numeric
%(process)dor%(thread)dspecifiers (the default consoleformat renders a process-id column) raised
TypeError: %d format: a real number is required, not strand could take downthe scheduler at startup.
This gives those two numeric callsite parameters a numeric fallback of
0, thesame way
linenois already handled a few lines above, so the format stringnever receives a string where a number is expected. The
"(unknown)"stringfallback is left intact for the text callsite parameters
(
pathname/module/threadName/processName), which are rendered with%s.A parametrized regression test formats
%(process)d:%(thread)dfor a recordwith no callsite info (the exact issue reproduction) and for one where those keys
are explicitly
None, asserting it renders0:0instead of raising; it failswith the
TypeErrorbefore the change and passes after.closes: #66195
Was generative AI tooling used to co-author this PR?
Final diff (2 files, +24/-0)
shared/logging/src/airflow_shared/logging/percent_formatter.py(+6): in_LazyLogRecordDict.__getitem__, addif key == "process" or key == "thread": return self.event.get(key) or 0immediately after the existing
linenonumeric-fallback case, with a shortcomment explaining why (numeric
%dparams must not fall back to the"(unknown)"string used for text params).shared/logging/tests/logging/test_percent_formatter.py(+18): addtest_numeric_callsite_without_process_or_thread, parametrizedmissing(no callsite keys, the issue repro) and
none(keys present butNone), inthe existing
TestPercentFormatRenderclass next totest_no_callsite; addsthe
import pytestit needs.