Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support of max log length for OTEL task log event #43868

Closed
2 tasks done
howardyoo opened this issue Nov 11, 2024 · 9 comments
Closed
2 tasks done

Add support of max log length for OTEL task log event #43868

howardyoo opened this issue Nov 11, 2024 · 9 comments
Assignees
Labels
area:core area:logging kind:bug This is a clearly a bug kind:feature Feature Requests needs-triage label for new issues that we didn't triage yet

Comments

@howardyoo
Copy link
Contributor

Apache Airflow version

2.10.3

If "Other Airflow 2 version" selected, which one?

No response

What happened?

When the OTEL_TASK_LOG_EVENT is enabled, scheduler job performs extraction of the task log as span event which would get added to the task span (

from airflow.utils.log.log_reader import TaskLogReader
). However, there is a risk of log entry being extremely large, that it would easily produce message that may not fit into the span event. Also, there is a potential risk of halting scheduler from running as scheduler would be busy processing it.

What you think should happen instead?

OTEL_TASK_LOG_EVENT should support MAX length of the log entry, such that it should have a safe default value of the max length (e.g. 64k characters), which will effectively trim the task log once that max limit is reached, preventing any operational risk of scheduler if the log message is too large to be practical.

How to reproduce

Generate a huge log (hundreds of megabytes?) and observe scheduler to hang (or causing delay) processing the log as span event.

Operating System

Linux (amd64)

Versions of Apache Airflow Providers

N/A

Deployment

Official Apache Airflow Helm Chart

Deployment details

No response

Anything else?

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@howardyoo howardyoo added area:core kind:bug This is a clearly a bug needs-triage label for new issues that we didn't triage yet labels Nov 11, 2024
@dosubot dosubot bot added area:logging kind:feature Feature Requests labels Nov 11, 2024
@potiuk
Copy link
Member

potiuk commented Nov 11, 2024

+1

@dstandish
Copy link
Contributor

dstandish commented Nov 12, 2024

-1

If you are talking about uploading task execution logs from the scheduler loop, this seems like still very much not a good idea. Even if just a snippet of logs, you don't want to be retrieving connections, reading from s3, etc. It's not the place to do that.

@potiuk
Copy link
Member

potiuk commented Nov 12, 2024

If you are talking about uploading task execution logs from the scheduler loop, this seems like still very much not a good idea. Even if just a snippet of logs, you don't want to be retrieving connections, reading from s3, etc. It's not the place to do that.

Who is talking about doing it from scheduler loop?

@dstandish
Copy link
Contributor

that's where the code in question is right now and that's the context for this is it not?

@dstandish
Copy link
Contributor

look in the top of this issue @potiuk
image

@dstandish
Copy link
Contributor

so it sounds like this issue is just about reducing the size of the log message in the area where it's currently added.

but it cannot be added ther.

@potiuk
Copy link
Member

potiuk commented Nov 12, 2024

that's where the code in question is right now and that's the context for this is it not?

Oh, I have not seen that - this should be definitely done from task/worker..

@dstandish
Copy link
Contributor

dstandish commented Nov 12, 2024

😌 (relieved face)

@ashb
Copy link
Member

ashb commented Nov 12, 2024

No, remove this feature entirely. Logs do not belong in Otel traces/spans. Use OTel logging and associate it with a span if that's what you want to do, but this code cannot exist in the scheduler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:core area:logging kind:bug This is a clearly a bug kind:feature Feature Requests needs-triage label for new issues that we didn't triage yet
Projects
None yet
Development

No branches or pull requests

4 participants