Skip to content

Clarify logging_config_class and deprecate ES/OS implicit REMOTE_TASK_LOG registration#66994

Closed
jason810496 wants to merge 7 commits into
apache:mainfrom
jason810496:refactor/logging/clarify-logging-config-class
Closed

Clarify logging_config_class and deprecate ES/OS implicit REMOTE_TASK_LOG registration#66994
jason810496 wants to merge 7 commits into
apache:mainfrom
jason810496:refactor/logging/clarify-logging-config-class

Conversation

@jason810496
Copy link
Copy Markdown
Member

@jason810496 jason810496 commented May 15, 2026

Why

[logging] logging_config_class is documented as a "Logging class" but actually resolves to a logging.config.dictConfig dict, and the REMOTE_TASK_LOG / DEFAULT_REMOTE_CONN_ID side channel that powers remote log read-back was undocumented.

So custom configs silently lost UI log read-back, and ElasticsearchTaskHandler / OpensearchTaskHandler papered over it by self-registering from inside __init__.

How

  • Document the real contract for logging_config_class (dict, not class) and the REMOTE_TASK_LOG / DEFAULT_REMOTE_CONN_ID module-level attributes in the config option help, advanced-logging-configuration.rst, and discover_remote_log_handler docstring.
  • Deprecate the ES/OS handler self-registration with AirflowProviderDeprecationWarning (stacklevel=1 so module-based filters match through dictConfig) and update the OpenSearch logging guide to set REMOTE_TASK_LOG at module scope.
  • Add warning for the case that "user module is missing REMOTE_TASK_LOG while remote_logging is on".

Was generative AI tooling used to co-author this PR?

The warning previously fired inside ``discover_remote_log_handler`` during
``load_logging_config``, before ``dictConfig`` had constructed handlers.
That misfired for the deprecated self-registration path in the
Elasticsearch and OpenSearch task handlers, which populate
``_ActiveLoggingConfig.remote_task_log`` from inside ``__init__``.

Drop the ``remote_logging_enabled`` parameter from
``discover_remote_log_handler`` and emit the warning from
``configure_logging`` after ``dictConfig`` runs, so the check sees the
final state. Extract the duplicated test fixture into provider
``conftest.py`` files and switch the deprecation warnings to
``stacklevel=1`` so module-based deprecation filters still match.
@jason810496 jason810496 force-pushed the refactor/logging/clarify-logging-config-class branch from 517e225 to ef217fc Compare May 16, 2026 03:33
Changelog
---------

``ElasticsearchTaskHandler`` no longer silently registers itself as the remote
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not sure do I need to update the Changelog myself at this stage?

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@jason810496 jason810496 marked this pull request as ready for review May 16, 2026 09:32
@jason810496 jason810496 requested review from Lee-W, jscheffl and shahar1 May 18, 2026 05:16
@eladkal
Copy link
Copy Markdown
Contributor

eladkal commented May 18, 2026

Is it possible to split it to two PRs: core and providers?

@jason810496
Copy link
Copy Markdown
Member Author

Is it possible to split it to two PRs: core and providers?

Sure, then I will separate them into core, ES and OS. I will raise PR soon.

@jason810496 jason810496 marked this pull request as draft May 18, 2026 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

3 participants