Fix wrong display of multiline messages in the log after filtering

[jason810496]

closes: #41265

Issue Context

After research, the issue is caused by airflow/www/static/js/dag/details/taskInstance/Logs/utils.ts, the API just return the full raw logs.
Additionally, incorrect display behavior also occurs when filtering by file source.

Main Fix Idea

The currentLogLevel and currentFileSource should remain same until a new logLevel or fileSource is encountered.

Screenshot After Fix

Without any filters

Filter with level

Filter with file source

Filter with level and file source

[jason810496]

Hi @Lee-W, I think this issue should be back ported to 2.9.x, 2.8.x, and 2.7.x and also fixed on the latest main branch once it is merged. Does that sound correct?

[pierrejeambrun]

Hi @Lee-W, I think this issue should be back ported to 2.9.x, 2.8.x, and 2.7.x

We do not backport to released versions. (unless critical security fixes but I haven't seen that happen before).

This will most likely be in the next patch release of 2.10.x, people will have to upgrade to get it.

[Lee-W]

Thanks @pierrejeambrun !

@jason810496 To create a "backport" PR to 2.10.x, you'll need to branch out from v2-10-test. In case you've not yet tried it 🙂

[potiuk]

@jason810496 To create a "backport" PR to 2.10.x, you'll need to branch out from v2-10-test. In case you've not yet tried it 🙂

Actually we now have cherry-picker - and we marked it with "backport" label, so the backport PR should be created automatically and if it is not possible, instructions what to do will be posted automatically here as comment :D

[pierrejeambrun]

Main is too different now and do not have this issue because of structured logs. (json)

I noticed that the PR directly targets v2-10-test. I think it's fine in this case.

@potiuk@Lee-W are we fine with that ?

[pierrejeambrun]

Tested, working as expected.

Just a few questions.

I think we should also consider applying the colors to those lines. (only certain lines appear red.)

[potiuk]

I noticed that the PR directly targets v2-10-test. I think it's fine in this case.

I think yes, but - we need to make sure this one will be forward-ported (for lack of a better word) or determined not to be needed in main :D

[jason810496]

Refactored the test cases to include error cases with traceback.

I think we should also consider applying the colors to those lines. (only certain lines appear red.)

This has been addressed. Screenshot:

[jason810496]

Rebased to the latest v2-10-test. Looking forward to any advice or feedback before merging.
cc @Lee-W , @pierrejeambrun

[pierrejeambrun]

Thanks for the update @jason810496 I was really busy today, I'll do a review tomorrow. (but last time I checked it was looking nice :))

[pierrejeambrun]

Looks good thanks.

Some of the questions of the previous review are left un answered. Just for my comprehension, if do you mind giving me a few hints.

Stale

[jason810496]

Looks good thanks.

Some of the questions of the previous review are left un answered. Just for my comprehension, if do you mind giving me a few hints.

Sure!

1. The mockExtraLog in the previous version was just a variable to reuse in the regex instead of directly adding ----... to the regex. In the refactored version, I removed the regex and matched the expected result with .toContain, which is more readable than handling multiple different matches in a single regex.

2. There was an empty newline in the previous version of the test cases, causing an empty line in parsedLogs. This issue is resolved in the current version:

   // Previous version with an empty line:constmockLog=`line 1line2`;// Current version without an empty line:constmockLog=`line 1line 2`;

3. Updated the comments for the new regex in the current version.

[pierrejeambrun]

Great thanks @jason810496 for the details.