Decision for broken task log prefix feature

[abhishekbhakat]

Apache Airflow version

2.9.0

What happened?

There is a part of code that may have been dead since airflow 2.0.0 for task logs prefix. The code is extending a StreamHandler which is not being used anywhere. The current local or remote task logs are extending FileTaskHandler.

What you think should happen instead?

Either this should be completely removed from Airflow or fix the feature with FileTaskHandler

How to reproduce

Set AIRFLOW__LOGGING__TASK_LOG_PREFIX_TEMPLATE with an example provided in the docs.

Operating System

Any

Versions of Apache Airflow Providers

Not Applicable.

Deployment

Docker-Compose

Deployment details

No response

Anything else?

I have raised a PR #38709 in an attempt to make this feature working with FileTaskHandler. Though I understand there might not be any point in bring a dead feature back. So, I'm proposing to remove the config task_log_prefix_template all together residing in below code: https://github.com/apache/airflow/blob/f8104325b7a66d4e98ff3a6c3555f90c796071c6/airflow/utils/log/task_handler_with_custom_formatter.py#L36

Are you willing to submit PR?

• [X] Yes I am willing to submit a PR!

Code of Conduct

• [X] I agree to follow this project's Code of Conduct

[abhishekbhakat]

We can close the PR #38709 if we decide to strip this feature completely.

[abhishekbhakat]

cc @dstandish

[jedcunningham]

I also commented in the PR, but I think we should just keep it scoped to TaskHandlerWithCustomFormatter (it's already documented that way fwiw). My 2c.

[jedcunningham]

So the question is whether we can cut it now or not. It'd be possible for a user to swap in TaskHandlerWithCustomFormatter in airflow_local_settings, right?

I say we mark it as deprecated. Not sure on the removal though.

[dstandish]

So the question is whether we can cut it now or not. It'd be possible for a user to swap in TaskHandlerWithCustomFormatter in airflow_local_settings, right?

sorry didn't understand @jedcunningham can you clarify?

[abhishekbhakat]

I think we can remove it cause anyway it is unreachable code. So we can be sure no one is using it.

[abhishekbhakat]

It'd be possible for a user to swap in TaskHandlerWithCustomFormatter in airflow_local_settings, right?

Nah, it's inheriting StreamHandler which is not at all used for Task logs. Task logs are only FileHandlers. So no matter what settings we use in airflow_local_settings this will never work.

[jedcunningham]

So the question is whether we can cut it now or not. It'd be possible for a user to swap in TaskHandlerWithCustomFormatter in airflow_local_settings, right?

sorry didn't understand @jedcunningham can you clarify?

If a user has replaced FileTaskHandler with TaskHandlerWithCustomFormatter for the task handler in airflow_local_settings, does it work?

[dstandish]

i was more curious about "the only question is whether we can cut it now or not"

i guess what you mean is, whether we can remove it immediately, or we simply deprecate it and leave it

[jedcunningham]

Yep exactly.

[dstandish]

I think we probably could just remove it. It would be sorta not too bad if someone was actually using it because the cluster would no run I think so hopefully they'd discover very early in pre-upgrade testing BUT it probably also doesn't hurt too much to just mark deprecated. @jedcunningham wdyt

[shahar1]

@abhishekbhakat Would you still like to work on this issue? I'd also suggest to rename the title to "Deprecate task lop prefix", as it seems that a decision has been made :)

[abhishekbhakat]

Yeah sure. Renaming now. Will PR this later this week.