Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logs Panel: Limit displayed characters to MAX_CHARACTERS #96997

Merged
merged 14 commits into from
Nov 26, 2024

Conversation

matyax
Copy link
Contributor

@matyax matyax commented Nov 25, 2024

Closes #96020

As a measure to improve how the logs panel responds when dealing with excessively long lines, this PR limits the displayed characters to MAX_CHARACTERS, which is currently 100.000.

Demo.mov

@matyax matyax added add to changelog no-backport Skip backport of PR labels Nov 25, 2024
@matyax matyax requested a review from a team as a code owner November 25, 2024 17:24
@github-actions github-actions bot added this to the 11.4.x milestone Nov 25, 2024
@matyax
Copy link
Contributor Author

matyax commented Nov 25, 2024

A smaller limit might be used, but that opens a whole different discussion, as this is meant to improve how the panel responds under extreme conditions.

Some background about the existing limit:

@matyax matyax requested a review from a team as a code owner November 26, 2024 10:27
@matyax matyax requested review from Clarity-89 and L-M-K-B and removed request for a team November 26, 2024 10:27
Copy link
Contributor

@svennergr svennergr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the visual should be different to the usual log line content. Also, maybe add a space before the ellipsis?

image

@matyax
Copy link
Contributor Author

matyax commented Nov 26, 2024

Good point @svennergr. I also felt that something was missing. How about:

secondary

Uses a secondary type button. I think primary is too flashy:

primary

As far as I know, spaces are not used before ellipsis, but also tried it and looks weird. Keep in mind that this is only visible after 100k characters, though.

@svennergr
Copy link
Contributor

As far as I know, spaces are not used before ellipsis, but also tried it and looks weird. Keep in mind that this is only visible after 100k characters, though.

👍

How about fill="outline":
image

Either way, you are correct. It won't be a very prominent button anyways.

@matyax
Copy link
Contributor Author

matyax commented Nov 26, 2024

Yeah! Let's go with fill.

@matyax matyax enabled auto-merge (squash) November 26, 2024 13:44
@matyax matyax merged commit d69888d into main Nov 26, 2024
13 checks passed
@matyax matyax deleted the matyax/really-long-lines branch November 26, 2024 15:45
gelicia pushed a commit that referenced this pull request Dec 4, 2024
* LogRowMessage: limit displayed characters to MAX_CHARACTERS

* LogRowMessage: update ellipsis text

* Formatting

* Revert test change

* LogRowMessage: fix conditional

* Extract translations

* LogRowMessage: use button for ellipsis

* Revert test change

* Change fill to outline

* Revert test change
@zserge zserge modified the milestones: 11.4.x, 11.4.1, 11.4.0 Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Grafana explorer - Fail fast when processing logs with extremely long lines (more than 100,000 characters)
3 participants