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

[Discover][ES|QL] Show loading indicator when DataView fields are fetched #179703

Open
kertal opened this issue Mar 29, 2024 · 7 comments
Open

[Discover][ES|QL] Show loading indicator when DataView fields are fetched #179703

kertal opened this issue Mar 29, 2024 · 7 comments
Labels
Feature:Discover Discover Application impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Graph) Team:ESQL ES|QL related features in Kibana usability

Comments

@kertal
Copy link
Member

kertal commented Mar 29, 2024

Currently, when users are changing their ES|QL query, this triggers a fetching of fields. Due to the way Discover works we currently can't make use of the upcoming DataViewLazy to just fetch e.g. only the date field. This is a separate issue #178927

However, what we can do is show a loading indicator. Because else the interface just appears to be blocked, when the fields request takes a long time.

The relevant code can be found in the fetchQuery function: Before the query is sent to Elasticsearch, in case of ES|QL the query is checked for an existing index patterns. In the case there's a new/changed index pattern, a new data view is initialized, when then leads to a fetching of data view fields. This happens before the data state is reset and a loading indicator is displayed. Here's the code

const fetchQuery = async (resetQuery?: boolean) => {
const query = getAppState().query;
const currentDataView = getSavedSearch().searchSource.getField('index');
if (isTextBasedQuery(query)) {
// this is where the dataView gets set to state, making it difficult to swap with dataViewLazy
const nextDataView = await getDataViewByTextBasedQueryLang(query, currentDataView, services);
if (nextDataView !== currentDataView) {
setDataView(nextDataView);
}
}
if (resetQuery) {
refetch$.next('reset');
} else {
refetch$.next(undefined);
}
return refetch$;
};

What we should aim for is to send the data state reset msg earlier (with sendResetMsg), this will show the loading indicator before a field_caps request can be triggered

    if (isTextBasedQuery(query)) {
      sendResetMsg(dataSubjects, getInitialFetchStatus(), RecordRawType.PLAIN);
      const nextDataView = await getDataViewByTextBasedQueryLang(query, currentDataView, services);
      if (nextDataView !== currentDataView) {
        setDataView(nextDataView);
      }
    }

The prevent's the impression of a blocked interface without any reason

Originally posted by @kertal in #179120 (comment)

@botelastic botelastic bot added the needs-team Issues missing a team label label Mar 29, 2024
@kertal kertal added Feature:Discover Discover Application Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Graph) labels Mar 29, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@botelastic botelastic bot removed the needs-team Issues missing a team label label Mar 29, 2024
@kertal kertal added needs-team Issues missing a team label Team:ESQL ES|QL related features in Kibana labels Mar 29, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-esql (Team:ESQL)

@stratoula
Copy link
Contributor

@kertal could we not request the fields from dataview and use ES|QL instead. Specifically we can add a limit 0 to the user's query to get the columns and check if there is an @timestamp field. I think that this might have a better performance.

@kertal
Copy link
Member Author

kertal commented Apr 3, 2024

@stratoula so this would need a separate query, right? I think this is a direction worth to evaluate now that DataViewLazy arrived and we could also use the ES|QL result to add fields to a data view that didn't fetch any fields.

However, still I think showing a loading indicator before this, no matter if it's powered by field_caps or ES|QL should be displayed, because the request can be slow in both cases.

@jughosta
Copy link
Contributor

jughosta commented Apr 3, 2024

Where do we need fields list in ES|QL mode in Discover? Not for the sidebar as it gets fields from the text based columns coming together with the main results request.

@kertal
Copy link
Member Author

kertal commented Apr 3, 2024

Where do we need fields list in ES|QL mode in Discover? Not for the sidebar as it gets fields from the text based columns coming together with the main results request.

Ah I see, so the fields + types of the field list are already provided by the textBasedQueryColumns returned by the ES|QL query, correct? so it's the time field that's needed upfront, but then, in theory no fields would be necessary , right?

@stratoula
Copy link
Contributor

Exactly! Also the autocomplete works with ES|QL query and not the dataview fields

@kertal kertal added impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:small Small Level of Effort labels Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Discover Discover Application impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Graph) Team:ESQL ES|QL related features in Kibana usability
Projects
None yet
Development

No branches or pull requests

4 participants