elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.76k stars 8.16k forks source link

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

Open kertal opened 6 months ago

kertal commented 6 months ago

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

https://github.com/elastic/kibana/blob/edd384a141dff680c17f2d4346e65a971b9fef16/src/plugins/discover/public/application/main/services/discover_data_state_container.ts#L299-L318

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 https://github.com/elastic/kibana/pull/179120#discussion_r1533772341_

elasticmachine commented 6 months ago

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

elasticmachine commented 6 months ago

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

stratoula commented 6 months ago

@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 commented 6 months ago

@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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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