stantheman0128 opened a new pull request, #71286:
URL: https://github.com/apache/airflow/pull/71286
closes: #69572
## What the problem is
The triggerer relays log lines from its subprocess. structlog serialises
exceptions into a list of dicts so they survive the JSON hop, and the relay
dropped that list straight onto the log record:
```python
if exc := event.pop("exception", None):
# TODO: convert the dict back to a pretty stack trace
event["error_detail"] = exc
```
In the triggerer's own log that comes out as a wall of nested JSON rather
than a stack trace.
## What this changes
A new pure function `format_exception_dicts()` in the shared logging package
turns `ExceptionDictTransformer` output back into a traceback. It covers
`__cause__` and `__context__` chains, exception notes, `SyntaxError` with its
caret, and exception groups including nested ones.
The output is byte-identical to what CPython's `traceback.format_exception`
produces, and the tests check that directly rather than against hand-written
fixtures: they raise a real exception, run it through the real transformer, and
compare the two renderings. Compiling the test bodies from a name that is not
on disk keeps `linecache` from supplying the source lines that structlog never
captures, so the two are comparable line for line.
The triggerer then uses it for the records that go to its own log.
## Why the task log keeps the structured payload
`error_detail` is not a free-form field, and this repository has settled on
rendering it where it is consumed rather than where it is transported:
- `renderStructuredLog.tsx` gives each exception a collapsible `<details>`,
marks user code frames apart from library frames with `isUserCodeFrame`, and
runs the frame labels through i18n. All of that needs the frames, not a
finished string.
- `_format_error_detail` in the elasticsearch and opensearch task handlers
renders the list to text for their own display path.
So the split is by destination. Records written to
`{log_path}.trigger.{id}.log`, which is a task log and therefore what the
readers above exist for, keep the list exactly as it arrived. Records going to
the triggerer's own stream get the rendered trace, because a person reads that
one as text.
The check is `log is fallback_log` rather than a test on `trigger_id`, which
matters: `get_logger()` falls back to the triggerer's own logger when a trigger
has no cached factory, for instance once `logger_cache.pop` has run at the end
of a trigger's life. Keying off the destination gets those late records right
too.
## The other two readers
`renderStructuredLog.tsx` mapped over `error_detail` unconditionally, so a
string would have thrown. It now takes a string as it comes. **This is not
fixing a live crash.** Nothing in the tree hands the log view a string today,
and the triggerer records changed here go to the triggerer's own stream rather
than to a task log, so they never reach it either. The guard is there so the
field has a single honest contract, which is already what the two provider
helpers assume, and so that a future producer cannot take the log view down.
`supervisor.py` had the same TODO and the same two lines. It keeps the
structured payload, and the TODO is replaced by a comment saying why. Two
reasons: its records go to the task log, and `_get_target_loggers` fans a
single event out to both the task log and stdout, so there is no
per-destination choice available to make there in the first place.
While tracing the readers I noticed that both copies of
`_format_error_detail` are unreachable: each is called only from its own unit
tests. That looks worth a look on its own, but it has nothing to do with this
issue, so both are left exactly as they are.
## Known gaps
Three places where the output cannot match CPython, all because the
information is missing from the payload rather than discarded here. Each has a
test pinning the behaviour and saying why.
- Exceptions outside `builtins` lose their module: `socket.gaierror` renders
as `gaierror`. structlog records the bare class name and nothing else. This one
is worth knowing about because `airflow.exceptions.*` are all non-builtin, so
it applies to most real triggerer tracebacks.
- `SyntaxError` gets a single caret. CPython underlines the whole offending
token using `end_offset`, which structlog's `SyntaxError_` does not carry.
- Repeated frames are not collapsed into `[Previous line repeated N more
times]`. structlog truncates at `max_frames` and leaves a placeholder frame
instead, which this renders as `[Skipped frames: N]`.
## Testing
`format_exception_dicts` is a pure function, so it is covered by unit tests
in `shared/logging/tests/logging/test_tracebacks.py`: 44 tests over the chain,
group, note, syntax-error and truncation paths, plus the round trips against
CPython.
The payload arrives as JSON from another process, so a good part of the
suite is about surviving whatever turns up. Anything unrecognisable renders as
`None` and the caller keeps the raw payload, on the grounds that a missing
stack trace costs less than a log pipeline that raises.
Two tests in `airflow-core/tests/unit/jobs/test_triggerer_job.py` cover the
destination split, and two in `renderStructuredLog.test.tsx` cover the string
branch in both rendering modes.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
Generated-by: Claude Code following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
