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: Display error message when logs fail to load #1079

Merged
merged 4 commits into from
Feb 18, 2025

Conversation

matyax
Copy link
Contributor

@matyax matyax commented Feb 17, 2025

This PR subscribes to the logs query and displays an error component when the log query fails, with the objective of improving how we surface errors to the user, making it clear that the query failed and providing a escape hatch to clear all filters.

Before:

After:

@matyax matyax requested a review from a team as a code owner February 17, 2025 11:51
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 it should replace the whole logs tab, or even higher level.
There's not really a need to show log volume histogram, when the query expression errors out.

image

src/Components/ServiceScene/LogsPanelError.tsx Outdated Show resolved Hide resolved
@matyax
Copy link
Contributor Author

matyax commented Feb 17, 2025

Definitely. I will also look for a couple other possible error responses to map to a readable message. Feel free to suggest if you remember any in particular.

@matyax matyax force-pushed the matyax/logs-panel-error branch from e727e4e to b312ef5 Compare February 17, 2025 14:52
@matyax
Copy link
Contributor Author

matyax commented Feb 17, 2025

@svennergr What do you think about collapsing Volume on error?

@matyax matyax requested review from svennergr and a team February 17, 2025 16:18
@zizzpudding
Copy link
Collaborator

Is there a way to make the make a suggestion to recover from this error without clearing all? "Invalid filter parameters" is pretty cryptic.

I'm unsure if this is an issue with the line filter? or if it's returning an empty set because the line filter was invalid.

In the first case. a simple error message "there are not logs that match your filter, clear all"

In the second case,
if there is a way to indicate which filter caused the erro ,highlight it (red border) and give a message to fix or clear that one (something like "contains invalid characters" etc.

Also link to the some documentation for creating a Regex expression for Loki would be helpful.

@gtk-grafana
Copy link
Contributor

gtk-grafana commented Feb 18, 2025

@zizzpudding Loki doesn't really tell us what part of the query failed, it just tells us that it was unable to parse the query. Also this PR is only addressing when we get an error back from Loki, not when there are no results.

We can follow up on client-side regex errors in line filters on this issue: Line filter validation. Updating the docs is good call out, I'll add a line-item to that epic.

Copy link
Contributor

@gtk-grafana gtk-grafana left a comment

Choose a reason for hiding this comment

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

Looks good, the only unexpected thing is that when I have the volume collapsed before running into an error, it gets uncollapsed after fixing the error.

Still a lot we can do here, but still worth merging

@matyax
Copy link
Contributor Author

matyax commented Feb 18, 2025

Looks good, the only unexpected thing is that when I have the volume collapsed before running into an error, it gets uncollapsed after fixing the error.

You read my mind. I was just about to do that.

Still a lot we can do here, but still worth merging

Agree. This is just the initial step to better handle and surface errors to the user.

@matyax matyax force-pushed the matyax/logs-panel-error branch from c03150d to 2ed72c4 Compare February 18, 2025 15:01
@matyax matyax merged commit adc3c8f into main Feb 18, 2025
4 checks passed
@matyax matyax deleted the matyax/logs-panel-error branch February 18, 2025 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants