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

Panel: Show multiple errors info in the inspector #64340

Merged
merged 3 commits into from Mar 8, 2023
Merged

Panel: Show multiple errors info in the inspector #64340

merged 3 commits into from Mar 8, 2023

Conversation

andresmgot
Copy link
Contributor

@andresmgot andresmgot commented Mar 7, 2023

What is this feature?

The goal of this PR is to show the details of the errors in case a query with multiple targets fails more than once. Before, only the first error was shown in the panel title error and in the inspector. Now, in case there are multiple errors, only that is stated in the title error. Clicking on it displays the query inspector, in the Error tab which shows information about all the errors.

This also includes information about the status code in the error message (which was missing before):

Screencast.from.07-03-23.16.51.56.webm

Since the error could have different formats (e.g. including a json). This is how it looks for the different formats:

Screenshot from 2023-03-07 16-52-49

Why do we need this feature?

To display all the information available to the user about an error.

Who is this feature for?

Grafana users.

Which issue(s) does this PR fix?:

Fixes #54665
Fixes #58637

Special notes for your reviewer:

This supersedes #64022

@andresmgot andresmgot requested review from a team as code owners March 7, 2023 16:40
@andresmgot andresmgot requested review from tskarhed, ashharrison90, axelavargas, polibb and leventebalogh and removed request for a team March 7, 2023 16:40
@andresmgot andresmgot added no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes labels Mar 7, 2023
@andresmgot andresmgot added this to the 9.5.0 milestone Mar 7, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2023

Backend code coverage report for PR #64340
No changes

@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2023

Frontend code coverage report for PR #64340

Plugin Main PR Difference
explore 86.43% 86.44% .01%

public/app/features/inspector/InspectErrorTab.test.tsx Outdated Show resolved Hide resolved
return null;
}
if (errors.length === 1) {
return renderError(errors[0]);
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason behind rendering a single error slightly differently (not wrapping it with an <Alert>)?

Copy link
Contributor Author

@andresmgot andresmgot Mar 8, 2023

Choose a reason for hiding this comment

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

I didn't want to affect the current style since for one error it's clear that all the content belong to the same error. For multiple errors, the best way I found to encasulate them separately was to show different Alerts. Happy to suggestions if there is something else I could try!

Copy link
Contributor

@leventebalogh leventebalogh left a comment

Choose a reason for hiding this comment

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

LGTM, but would be good to also wait for someone from user essentials to have a look.

@andresmgot
Copy link
Contributor Author

Also covered now the new panel header:

Screenshot from 2023-03-08 13-12-12

Copy link
Member

@axelavargas axelavargas left a comment

Choose a reason for hiding this comment

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

Great work @andresmgot 🥳

@andresmgot andresmgot merged commit 15aae5e into main Mar 8, 2023
10 checks passed
@andresmgot andresmgot deleted the errorTab branch March 8, 2023 15:11
@andresmgot andresmgot added add to changelog and removed no-changelog Skip including change in changelog/release notes labels Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants