yaming-github commented on PR #54433: URL: https://github.com/apache/airflow/pull/54433#issuecomment-3190656963
> Thank you for picking this up, but this approach is not right. > > This is combining capturing and presentation into the capture phase which is not right. This needs to be re-worked to a) not be a performance hit, and b) To correctly take advantage of having structured logs. > > In short: `log_format` should be a presentation time that the UI deals with, not something that actually affects how the JSON logs are written. Hi @ashb thanks so much for taking a look at this! Instead of replying all comments, I'll reply you here: 1. Totally agree on all your comments. I think I make the logic over complicated here and didn't consider the performance. We should not relate this change with log_format. That's actually why I need to overwrite LOG_FORMAT in tests to pass the tests. I've removed all of them. 2. I changed the pr to apply only a simple processor to add filename and lineno. 3. This updated change only apply for std log and warnings but not stdout and structlog itself. I think for stdout it's fine because even though we added filename, they all come from 1 specific place which is supervisor.py. For structlog, if end user uses structlog in their dag, we also didn't log the callsite here. However, that's doable, please let me know if you think we should also consider that. Thanks! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
