You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "tirkarthi (via GitHub)" <gi...@apache.org> on 2025/03/07 16:26:33 UTC
[PR] Highlight log lines by keyword. [airflow]
tirkarthi opened a new pull request, #47507:
URL: https://github.com/apache/airflow/pull/47507
Update API to return `color_log_error_keywords` and `color_log_warning_keywords` from airflow.cfg through config API. Then in frontend parse the log message to add color attribute to span tag based on the keyword.
Notes to self and review :
1. The semantic tokens in light theme appear to be hard to read. I guess it could be due to the Code block adding some more background color.
2. For task log preview I have disabled highlighting since it didn't make sense and it was already filtered by error/critical level.
airflow.cfg values to test as per the screenshot
```ini
color_log_error_keywords = error,exception,group
color_log_warning_keywords = utils
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Highlight log lines by keyword. [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #47507:
URL: https://github.com/apache/airflow/pull/47507#discussion_r1990127320
##########
airflow/api_fastapi/core_api/routes/ui/config.py:
##########
@@ -61,6 +61,10 @@ def get_configs() -> ConfigResponse:
"audit_view_included_events": conf.get("webserver", "audit_view_included_events", fallback=""),
"audit_view_excluded_events": conf.get("webserver", "audit_view_excluded_events", fallback=""),
"test_connection": conf.get("core", "test_connection", fallback="Disabled"),
+ "color_log_error_keywords": conf.get("logging", "color_log_error_keywords", fallback="").split(","),
Review Comment:
I am mostly with Ash. I had mostly false-positives with the highlighting so far which ended-up that we disabled all keywords from default as users called because being confused.
For Error/Warning I think the highlighting should be really oriented from the log sub system and log level as we have structured data.
I'd be OK to have the highlighting with keyword, if not "Exception", "Error" and "Warning" are used per default in config (as this would be over-lapping with the log level). It might be use-ful for cases where custom words (other than error...) are to be highlighted.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Highlight log lines by keyword. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on PR #47507:
URL: https://github.com/apache/airflow/pull/47507#issuecomment-2726755020
Thanks everyone for the comments. I am closing the PR. This can be revisited if there are requests from users in Airflow 3.x .
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Highlight log lines by keyword. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #47507:
URL: https://github.com/apache/airflow/pull/47507#discussion_r1987586252
##########
airflow/api_fastapi/core_api/routes/ui/config.py:
##########
@@ -61,6 +61,10 @@ def get_configs() -> ConfigResponse:
"audit_view_included_events": conf.get("webserver", "audit_view_included_events", fallback=""),
"audit_view_excluded_events": conf.get("webserver", "audit_view_excluded_events", fallback=""),
"test_connection": conf.get("core", "test_connection", fallback="Disabled"),
+ "color_log_error_keywords": conf.get("logging", "color_log_error_keywords", fallback="").split(","),
Review Comment:
3.0 is the opportunity to decide which config to remove too.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Highlight log lines by keyword. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi closed pull request #47507: Highlight log lines by keyword.
URL: https://github.com/apache/airflow/pull/47507
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Highlight log lines by keyword. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on code in PR #47507:
URL: https://github.com/apache/airflow/pull/47507#discussion_r1985980806
##########
airflow/api_fastapi/core_api/routes/ui/config.py:
##########
@@ -61,6 +61,10 @@ def get_configs() -> ConfigResponse:
"audit_view_included_events": conf.get("webserver", "audit_view_included_events", fallback=""),
"audit_view_excluded_events": conf.get("webserver", "audit_view_excluded_events", fallback=""),
"test_connection": conf.get("core", "test_connection", fallback="Disabled"),
+ "color_log_error_keywords": conf.get("logging", "color_log_error_keywords", fallback="").split(","),
Review Comment:
This is not a new configuration. This was already present in Airflow 2.10. This is a port of the implementation from the old UI to the new UI. We have use cases where we show logs from external systems like Spark, HDFS etc that are not structured from the triggerer as a log group like "::group::stdout". The keywords help in highlighting the relevant line and improves debugging.
> Also "Task succeded without error" would get highlighted as an error. Very prone to false positives.
Agreed, this is like a simple `ctrl+f` in the browser and has been more useful than a hindrance,
Config docs : https://airflow.apache.org/docs/apache-airflow/stable/configurations-ref.html#color-log-error-keywords
Airflow 2.10 implementation : https://github.com/apache/airflow/issues/37443
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Highlight log lines by keyword. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on PR #47507:
URL: https://github.com/apache/airflow/pull/47507#issuecomment-2706978762
Yeah, we shoudl adjust those light mode colors. First, want to try a light Code background before playign with the text color?
We should also make sure this plays well with https://github.com/apache/airflow/pull/47469. Maybe log source shouldn't be by color then?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Highlight log lines by keyword. [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on code in PR #47507:
URL: https://github.com/apache/airflow/pull/47507#discussion_r1985814758
##########
airflow/api_fastapi/core_api/routes/ui/config.py:
##########
@@ -61,6 +61,10 @@ def get_configs() -> ConfigResponse:
"audit_view_included_events": conf.get("webserver", "audit_view_included_events", fallback=""),
"audit_view_excluded_events": conf.get("webserver", "audit_view_excluded_events", fallback=""),
"test_connection": conf.get("core", "test_connection", fallback="Disabled"),
+ "color_log_error_keywords": conf.get("logging", "color_log_error_keywords", fallback="").split(","),
Review Comment:
1. there's a `conf.getlist` function
2. This is a UI setting, not a logging setting
3. Now we have structured logging (where the log level comes form the "source" directly without needing to pase , I don't think we should be looking for keywords.
4. I really really really REALLY don't want us to add more config options!
Also "Task succeded without error" would get highlighted as an error. Very prone to false positives.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org