Add %z for %(asctime)s to fix timezone for logs on UI #24811
Conversation
boring-cyborg
bot
added
area:logging
area:UI
rino0601
force-pushed
the
cherry-pick-able-23473
branch
from
684f80c to
2f539bc
Compare
| @@ -635,7 +635,7 @@ | |||
| default: "%%(asctime)s %%(levelname)s - %%(message)s" | |||
| - name: log_formatter_class | |||
| description: ~ | |||
| version_added: 2.3.3 | |||
| version_added: 2.3.4 | |||
There was a problem hiding this comment.
bump up.
#24373 didn't show up on https://github.com/apache/airflow/pull/24762/files#diff-9b07fd28838e759e05bdaa5293886d860374fe291bdc6cc43c20f951fc77eab4 and it's commits
| // above regex is a kind of duplication of 'dateRegex' | ||
| // in airflow/www/static/js/grid/details/content/taskinstance/Logs/utils.js | ||
| const dateRegex = /\d{4}[./-]\d{2}[./-]\d{2} \d{2}:\d{2}:\d{2},\d{3}/g; | ||
| const iso8601Regex = /\d{4}[./-]\d{2}[./-]\d{2}T\d{2}:\d{2}:\d{2}.\d{3}[+-]\d{4}/g; |
There was a problem hiding this comment.
Should we not do both of these checks in utils.ts too?
There was a problem hiding this comment.
The regular expression on the utils.ts side is const regExp = /\[(.*?)\] \{(.*?)\}/;, which finds the string between square brackets instead of finding the time string.
moments.js understands the ISO 8601 format, so we don't have to worry about this.
rino0601
force-pushed
the
cherry-pick-able-23473
branch
from
2f539bc to
d31f332
Compare
|
By |
|
Could you please give this PR a |
|
Should we possibly add a warning if someone does not use TimezoneAware formatter? I think it is no harm to add it (with clear instruction on what to do). I think writing it in the logs of tasks, would make it much more discoverable - you could simply print a warning "Beware, this log might contain wrong timezone and you should use this and that to fix it". There are two kinds of users - those who read the "migration guides and changelog" and those who don't and the latter kind of user will generally open an issue if they see logs with wrong timestamp. Producing a warning in the very logs that the user might complain about is as good as we can do to make the user self-serviced, especially if we are very explicit that "timestamps here might have wrong timezone" and add "this is how you fix it". |
|
I've tried to add a warnings but it didn't work at the first try. |
(cherry picked from commit 851e5ca)
follow up of #24373
related:
original issue #23796
previous pr #24373
revert previous pr #24810
reference #21942
As already addressed in #24810, #24373 has git conflict problem. This PR solves that problem and makes it able to release as a bugfix.
Also, thanks to @millin 's comment, by applying ISO 8601 format, it can drop complicated and duplicated custom parse code on UI.
This pr must be merged after #24810
^ 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, in newsfragments.