Skip to content

Register Stackdriver remote task logging handler - #70549

Merged
potiuk merged 1 commit into
apache:mainfrom
fat-catTW:fix-stackdriver-remote-logging-dispatch
Aug 1, 2026
Merged

Register Stackdriver remote task logging handler#70549
potiuk merged 1 commit into
apache:mainfrom
fat-catTW:fix-stackdriver-remote-logging-dispatch

Conversation

@fat-catTW

Copy link
Copy Markdown
Contributor

Register Stackdriver task remote logging with the provider remote logging dispatch registry.

This lets remote_base_log_folder = stackdriver:///... resolve through the ProvidersManager scheme dispatch path to StackdriverRemoteLogIO.from_config(), instead of relying on the legacy logging config fallback.

The change also teaches StackdriverRemoteLogIO.from_config() to build itself from Airflow logging configuration, including the Stackdriver log name derived from remote_base_log_folder.

related: #70267

Was generative AI tooling used to co-author this PR?
  • [X] Yes (please specify the tool below)

Generated-by: [Codex] following the guidelines


  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Faithful on all four fields, including the urlsplit(...).path[1:] derivation that turns stackdriver:///airflow-tasks into airflow-tasks, and the | io_kwargs merge that legacy applies for this backend.

The guard is a genuine improvement rather than just a port:

if not log_name:
    raise ValueError(
        "Cannot derive a Stackdriver log name from "
        f"logging/remote_base_log_folder: {remote_base_log_folder!r}"
    )

Legacy had nothing here, so remote_base_log_folder = stackdriver:// with no path silently produced an empty gcp_log_name and a handler that could not work. Turning that into a startup error that names the offending config value is the right call, and parametrizing the rejection test over the bad forms is the right way to pin it.

Reviewing this alongside the sibling ports, the cluster made opposite choices on superficially similar "config is empty" cases — #70525 falls back to a class default host, this one raises. Both are correct for their situation: an empty Elasticsearch host has a sensible default, an empty Stackdriver log name does not. Worth noting only because the divergence is deliberate rather than accidental.

Adding a second entry under the existing remote-logging block alongside gs also confirms multiple schemes per provider work, which is useful to have demonstrated.

Rebased onto main before merging.


Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting

@potiuk
potiuk force-pushed the fix-stackdriver-remote-logging-dispatch branch from a75cb43 to 19b8ed1 Compare August 1, 2026 03:31
@potiuk
potiuk merged commit 251a928 into apache:main Aug 1, 2026
87 checks passed
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 ready for maintainer review Set after triaging when all criteria pass.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants