-
Notifications
You must be signed in to change notification settings - Fork 12.1k
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
Explore: Adds Loki explore query editor #21497
Conversation
…h explore mode enum
…back to 0 lines on negative or invalid input instead of datasource maxLines
…nting invalid/negative line limits
…te slice of a result instead of an entire one
… to use options.maxDataPoints in dashboards
public/app/plugins/datasource/loki/components/LokiExploreQueryEditor.test.tsx
Outdated
Show resolved
Hide resolved
public/app/plugins/datasource/loki/components/LokiExploreQueryEditor.tsx
Outdated
Show resolved
Hide resolved
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.
@Estrax I am not sure if you have decided to change the default behaviour, but having line limit 0 results in returning default range of logs(set in datasource). This is confusing to me, as I would expect to see 0 logs for that line). However, this is my personal opinion.
However, blocking issue - if the Line limit is 0, then we don't receive the message about returned log lines:
@ivanahuckova it's already fixed, just didn't push changes due to having language providers failing on initialization. |
… logic to be moved into a component with new form styles
Same issue as in the other PR. Need to check what's the reason the test fails. Looks like the problem is with slate plugin that doesn't see the |
@@ -467,7 +467,7 @@ export function processRangeQueryResponse( | |||
switch (response.data.resultType) { | |||
case LokiResultType.Stream: | |||
return of({ | |||
data: lokiStreamsToDataframes(response.data.result, target, limit, config, reverse), | |||
data: lokiStreamsToDataframes(limit > 0 ? response.data.result : [], target, limit, config, reverse), |
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.
Gonna change it with an implementation of new form styles - with limit == 0
request isn't going to be sent, we'll just clear the result of this query and display a validation error.
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.
Looks good. There still the issue though that when I try to enter some non numeric chars (like e
) into the input it gets cleared out. Not a big issue so if it should take long to fix we can do it some other time.
Yep, that's a known issue due to our preprocessing, it's going to be fixed with new forms and their validation. |
Cool! Checked it and it works as expected. 🙂 |
What this PR does / why we need it:
Adds LokiExploreQueryEditor - a wrapper on top of LokiQueryField allowing to change lines limit in logs mode. Also, fixes Loki input overflows on long queries and moves
ExploreMode
to grafana-data.Which issue(s) this PR fixes:
Fixes #19544
Continuation of #21089 as something went wrong during rebase.