-
Notifications
You must be signed in to change notification settings - Fork 14
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
Patterns: Logs sample #430
Patterns: Logs sample #430
Conversation
…9/patterns-table-log-samples
…terns sample query
…9/patterns-table-log-samples
…ple query, help text
I would not apply line filters for showing sample of patterns ! |
…9/patterns-table-log-samples
Will be taking a look at this one today. Apologies for taking so long. |
…9/patterns-table-log-samples
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this tooltip coming from the UI library?
I initially thought there was a missing loading state, so I was going to suggest one, but it turns out with these many patterns (300) the performance is awful. Adding the logs panel takes a significant amount of time. Not sure if there's anything we can do now, maybe something to follow up.
Test.mov
Besides these comments and other nitpicks, it works very well, great job!
src/Components/ServiceScene/Breakdowns/Patterns/PatternsViewTableScene.tsx
Outdated
Show resolved
Hide resolved
src/Components/ServiceScene/Breakdowns/PatternsLogsSampleScene.tsx
Outdated
Show resolved
Hide resolved
src/Components/ServiceScene/Breakdowns/PatternsLogsSampleScene.tsx
Outdated
Show resolved
Hide resolved
src/Components/ServiceScene/Breakdowns/PatternsLogsSampleScene.tsx
Outdated
Show resolved
Hide resolved
src/Components/ServiceScene/Breakdowns/PatternsLogsSampleScene.tsx
Outdated
Show resolved
Hide resolved
@matyax thanks for taking a look, the tooltip is indeed in the library, it's pretty obnoxious, but it should be relatively quick to update the base component to allow passing in of a custom title tag. For performance, it would be nice to build our own scenes table that can be virtualized, but I agree that should be addressed in another PR I looked again into why we didn't have a loading state, and it's because we need to render the logs panel to subscribe to the query runner, which executes the query, so the panel is the loading state, it just takes a while to render. Maybe worth revisiting in the future. |
I'll merge this in on Monday. CC: @svennergr @matyax |
This cannot be released until 11.1.1 is released, and the dependencies have been updated due to conflict in InteractiveTable with the logs panel.
Users running < 11.1.1 will see the logs table all ****ed up, so we might want to wait to deploy this until we can deprecate those versions of Grafana
Aug 01, 2024 Update: Looks like all cloud instances are on some version of 11.1.1 or higher now, so we should be good to release this now.
Functionality:
This got a bit complicated to try and work around the fact that patterns only filter on indexed filters in the users query context.
We first attempt to run the pattern sample query with all of the line filters and field filters. If that comes back with no results, we show a warning message that the patterns being displayed do not intersect with your current query results, and we remove the ability to include/exclude by that pattern.
We also prompt the user to clear their current filters, in case they do want to filter by that pattern. Unfortunately this will probably route the user away from the patterns view (unless we are just clearing the line filter), as right now we always route to the logs tab after change of variables. We should deal with this in another issue/PR: (#640).
Ideally we would not show patterns that don't apply to the users current filters, but that needs to be addressed in the loki API: #641.
If something else goes wrong: we show an actionless error state, and log the request and traceIds to console.error (
Pattern sample query returns no results
), so we can use faro logs to later debug what's going on. We probably want to add a panel to our tracking dashboard for this error.If the active filters do not exclude all results, and the first query comes back successful, we allow users to filter by this pattern, and the results are filtered by the users current query context.