Skip to content

Configure logging for Triggerer#54992

Merged
o-nikolas merged 2 commits intoapache:mainfrom
aws-mwaa:onikolas/trigger_no_colour_logging
Sep 2, 2025
Merged

Configure logging for Triggerer#54992
o-nikolas merged 2 commits intoapache:mainfrom
aws-mwaa:onikolas/trigger_no_colour_logging

Conversation

@o-nikolas
Copy link
Copy Markdown
Contributor

This will ensure logging configuration such as colored_console_log are respected by the Triggerer processes.

The way this was detected was Trigger logs not respecting the logging.colored_console_log config (see #54962). With this change the logs now respect that setting:

Screenshot from 2025-08-27 11-16-40

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

This will ensure logging configuration such as colored_console_log are
respected by the Triggerer processes.
@o-nikolas
Copy link
Copy Markdown
Contributor Author

CC @ashb

@o-nikolas o-nikolas requested review from ashb and potiuk August 27, 2025 21:53
@ashb
Copy link
Copy Markdown
Member

ashb commented Aug 28, 2025

Unless this is pressing, I'm working on a complete overhaul of logging in #52651

Copy link
Copy Markdown
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

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

Okay yes, I see. We were using an unconfigured/wrongly configured

@o-nikolas
Copy link
Copy Markdown
Contributor Author

o-nikolas commented Aug 28, 2025

Okay yes, I see. We were using an unconfigured/wrongly configured

Yupp, it was just being used unconfigured, so much of the configuration wasn't being applied to the logger.

Are you happy to approve after the test change?

@ashb

Copy link
Copy Markdown
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

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

Though now I think about it, I wonder if this should be called much earlier in the trigger_job process, not just here?

Doesn't this affect the " triggers running" log message too?

I guess not since that comes from the subprcoess. Still, I think it should be moved much earlier, like in the TriggerJobRunner.execute or similar.

@o-nikolas
Copy link
Copy Markdown
Contributor Author

So far I see all the logs I expect to be configured doing so in the Triggerer (though tracebacks in all processes still don't respect the colour configuration). Let's merge this for now since it's tested and at least solving the issues I know of in the Triggerer. We can always continue iterate further if necessary.

@o-nikolas o-nikolas merged commit 52c1abb into apache:main Sep 2, 2025
107 of 108 checks passed
@o-nikolas o-nikolas deleted the onikolas/trigger_no_colour_logging branch September 2, 2025 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants