Skip to content

Guard StackdriverRemoteLogIO's transport.send() against errors - #71337

Open
roshanprabu wants to merge 1 commit into
apache:mainfrom
roshanprabu:fix-stackdriver-transport-send-guard
Open

Guard StackdriverRemoteLogIO's transport.send() against errors#71337
roshanprabu wants to merge 1 commit into
apache:mainfrom
roshanprabu:fix-stackdriver-transport-send-guard

Conversation

@roshanprabu

Copy link
Copy Markdown

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:

_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:

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

  • Added test_processors_survives_transport_send_failure: mocks transport.send() to raise, asserts the processor doesn't propagate the exception.
  • 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.
  • Ran the full file: pytest tests/unit/google/cloud/log/test_stackdriver_task_handler.py -- 33 passed.
  • ruff check and ruff format --check pass on both changed files.

The log processor's transport.send() call ran unguarded. This
processor is installed for every supervised component (scheduler,
dag-processor, triggerer, workers) whenever REMOTE_TASK_LOG routes
through Stackdriver, so any IAM permission error, gRPC connectivity
failure, or quota exception from Cloud Logging propagated straight out
of the structlog processor and crashed the entire process -- observed
as a dag-processor stuck in CrashLoopBackOff from a missing
logging.logEntries.create IAM binding.

The read path already has this exact guard one function down (see the
try/except around read_logs() a few lines below, with the same
rationale in its comment); the write path was simply missing the
equivalent. Log delivery is best-effort, so this logs a warning via
the handler's own logger instead of raising.
@roshanprabu
roshanprabu requested a review from shahar1 as a code owner August 8, 2026 20:16
@boring-cyborg boring-cyborg Bot added area:logging area:providers provider:google Google (including GCP) related issues labels Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:logging area:providers provider:google Google (including GCP) related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant