roshanprabu opened a new pull request, #71337:
URL: https://github.com/apache/airflow/pull/71337
## Summary
Fixes one of three bugs reported in #68240 (the one I could verify is still
present and fix with high confidence -- see scope note below).
`StackdriverRemoteLogIO.processors`' `proc` closure calls
`_transport.send(...)` with no error handling:
```python
_transport.send(record, str(msg.get("event", "")), resource=self.resource,
labels=labels)
return event
```
This processor is installed for **every supervised component** (scheduler,
dag-processor, triggerer, workers) whenever `REMOTE_TASK_LOG` routes through
Stackdriver. Any IAM permission error, gRPC connectivity failure, or quota
exception from Cloud Logging propagates straight out of the structlog processor
and takes down the whole process -- reported as a dag-processor pod stuck in
`CrashLoopBackOff` on every log emit when its Kubernetes Service Account lacked
the `logging.logEntries.create` IAM binding.
The *read* path already has exactly this guard, a few lines down in the same
file, with a comment explaining the rationale:
```python
try:
messages, end_of_log, next_page_token = self.io.read_logs(log_filter,
next_page_token, all_pages)
except Exception:
# Cloud Logging unavailable / IAM glitch / gRPC error. Without a guard,
the
# exception used to propagate up as HTTP 500 from the log viewer. ...
_logger.exception(...)
```
The *write* path was simply missing the equivalent. This PR adds it, reusing
the same `_logger` (already defined in this file specifically for
handler-internal failures) and logging a warning instead of raising, since log
delivery here is best-effort.
**Scope note:** the linked issue reports three bugs. I traced the other two
(empty labels when `record.task_instance` is unset in supervisor context, and
`read()` filtering on `logical_date`) against current `main` and found the code
has moved on since the report -- there's now a fallback in `proc()` that reads
labels from the event dict when no `task_instance` is present (see
`test_processors_fallback_to_event_labels`), and `read()`'s `ti: RuntimeTI`
already exposes `logical_date` locally without a DB round-trip. I wasn't
confident those two are still bugs as described, so this PR only covers the
third (`transport.send()` unguarded), which I could verify precisely.
## Test plan
- [x] Added `test_processors_survives_transport_send_failure`: mocks
`transport.send()` to raise, asserts the processor doesn't propagate the
exception.
- [x] Verified the test actually catches the bug: reverted the source fix
and confirmed the new test fails with `Exception: IAM permission denied`
propagating out of `proc()`; re-applied the fix and confirmed it passes.
- [x] Ran the full file: `pytest
tests/unit/google/cloud/log/test_stackdriver_task_handler.py` -- **33 passed**.
- [x] `ruff check` and `ruff format --check` pass on both changed files.
--
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]
