Allow browsing skipped logfile content with chunk navigation#5326
Allow browsing skipped logfile content with chunk navigation#5326vetri15 wants to merge 3 commits intocodecentric:masterfrom
Conversation
Add byte-range logfile chunk navigation for skipped content, with a compact follow/page up/page down toolbar that switches between live tailing and manual browsing. Also parse logfile window metadata, show skipped bytes for manual chunks, and add localized navigation labels.
|
@SteKoe can we get this reviewed please? |
|
Hi @vetri15. I tested this a bit, by changing the servlet sample logging configuration to logging:
level:
root: trace # even debug is fineand I found the following bugs:
-- the previous chunk and throw away everything that comes before the last new line. If it's the first chunk, nothing will be thrown away.
|
| row.appendChild(col); | ||
| document.querySelector('.log-viewer > table')?.appendChild(row); | ||
| }); | ||
| this.renderedLines = [...this.renderedLines, ...lines]; |
There was a problem hiding this comment.
Does this imply we're keeping potentially the full log content in memory?
If it's so, that's not ideal at all.
There was a problem hiding this comment.
Hi @cdprete
I had the same thought. However, in the previous implementation, when navigating to the logfile view, it initially fetched around 300 KB, and while staying on the page it continued accumulating newly received log lines. The loaded content was only reset when the page was refreshed or when the user left the logfile view and opened it again.
Should I change this behavior so the logfile view always maintains a 300 KB sliding window instead?
There was a problem hiding this comment.
Should I change this behavior so the logfile view always maintains a 300 KB sliding window instead?
Mmm I don't know.
@SteKoe @ulischulte your thoughts?
I want to avoid that the memory explodes with big log files.
However, in the previous implementation, when navigating to the logfile view, it initially fetched around 300 KB, and while staying on the page it continued accumulating newly received log lines.
But, if I'm not mistaken, before they were kept only in the dom as table rows and columns, while now we're also storming them as data.
There was a problem hiding this comment.
Hi @cdprete
Yes, that makes sense. Keeping the log lines both in Vue state and in the rendered DOM does mean the content is effectively retained in two places.
However, directly appending rows to the DOM and then clearing the Vue state would make the DOM diverge from Vue’s source of truth, which seems risky and harder to maintain. Also, the expensive part is usually rendering and managing a large number of DOM nodes, so clearing only the JavaScript array would not fully solve the memory/performance concern.
I think the cleaner approach is to keep the rendered content itself bounded to 300kb sliding window limit.
There was a problem hiding this comment.
Having such an hard limit would be risky, because it highly depends on how many lines can be rendered.
On large or high-density screens, it may not be enough.
Maybe the best would be to use something like https://github.com/hilongjw/vue-recyclerview or https://github.com/Akryum/vue-virtual-scroller (which is highly used in Android, for example, https://developer.android.com/develop/ui/views/layout/recyclerview) to reuse the existing DOM nodes rather than always creating and adding new ones.
This, together with proactively dropping a batch of non visible lines from the component state when a new chunk is fetched should keep the memory quite under control even with very big log files.
There was a problem hiding this comment.
shall i implement this kind of solution
I can't say, because it would be a breaking change.
@SteKoe @ulischulte?
I think that, if you can show a notification to the user indicating that the content has been reloaded because the log file has been rotated, it's then safe enough.
There was a problem hiding this comment.
okay thanks @cdprete , I understand now.
-
I will implement such that we are rendering only a small portion on the screen (~300kb) and a higher limit on the javascript (~2MB) so scrolling few chunks would be faster will leverage the vue libraries you provided, both will be constants so we can set them for what's apt in prod.
-
I will show a notification after reloading logview table.
There was a problem hiding this comment.
Hi @cdprete
Regarding the table shrinking issue you mentioned, the current implementation always renders the exact chunk returned by the requested range, without overlapping it with the previously rendered chunk.
For example, if the current view shows lines 1–100 and the next chunk only contains lines 101–120 because we are near the start or end of the file, only those available lines are rendered.
I kept it this way so that each previous/next chunk action consistently moves to a new byte range without mixing content from the previous chunk. If we append or prepend part of the old chunk to keep the table height stable, the navigation would no longer represent a clean chunk progression.
So the trade-off is between:
- preserving consistent chunk progression
- preserving consistent visual table height
The current implementation favors consistent chunk progression.
if you prefer table consistency , please let me know , I will update the code
There was a problem hiding this comment.
will leverage the vue libraries you provided, both will be constants so we can set them for what's apt in prod
They're not mine :)
Moreover, the recycle view pattern/idea is used a lot in Android, but I don't know if there are better options in Vue.
I let you and the team decide on it.
There was a problem hiding this comment.
if you prefer table consistency , please let me know , I will update the code
If also the existing implementation presents the same behavior, then so be it.
| if (this.atBottom) { | ||
| this.scrollToBottom(); | ||
| if (shouldKeepAtBottom) { | ||
| this.$nextTick(() => this.scrollToBottom()); |
There was a problem hiding this comment.
Why do we need to use $nextTick now?
There was a problem hiding this comment.
We need that $nextTick because the log rows are now rendered from renderedLines via Vue instead of being appended to the DOM manually. Updating renderedLines queues the DOM patch, so calling scrollToBottom() synchronously would read the previous scrollHeight. Waiting for the next tick ensures the new <tr> rows are in the DOM before we scroll.
replaced plain strings with Enum
|
@vetri15 by the way, let me know if I should test it again ;) |



This PR implements the #4342 enhancement ,
other minor tweaks: