[Kibana App] Adapt to new hits.total format in Elasticsearch

[timroes]

Once elastic/elasticsearch#35849 is merged the format of hits.total as returned by Elasticsearch will change. We need to cater for that, since we read out hits.total in some places.

The way we are planning to do this is the following:

• Step 1: Add rest_total_hits_as_int=true to all requests, that we need to get hits.total from in the end. This will cause the API to return the old format for requests, where nothing changed. Since this parameter is available since 6.6, we can backport that change to the 6.x branch.
• Step 2: Once we don't have much work going on on the 6.x branch anymore (after 7.0 release), adopt all places that are consuming hits.total to the new format, and switch over the parameter to use track_total_hits=true instead to get the exact result in all those requests.
• Step 3: Analyze if there are any places where we don't need the exact matches and could potentially replace this by showing the estimated hits as with the new behavior.

We are going this way, since that allows us to have master and 6.x branch in sync before the 7.0 release. If we would directly switch to the new format, we can't backport this change to 6.x and thus master and 6.x will have a different state in all the places we are accessing hits.total and are executing a query. Since this will make backporting future changes to 6.x more difficult we won't change to the new format as long as we are still actively developing features on 6.x.

Since the rest_total_hits_as_int parameter is scheduled for removal in 8.0, I will directly mark this issue as a blocker for 8.0 as a reminder, that step 2 needs to be implemented till then.

When this is completed, we can remove the temporary shim that was introduced as part of #61565.

Related issues/PRs

• Infra UI ([Infra UI] Adapt to Elasticsearch total hit count response format change #26351)
• APM ([APM] Add rest_total_hits_as_int to all ES requests #26462)
• Kibana App (Add rest_total_hits_as_int into Kibana App #26404)
• Platform (Add rest_total_hits_as_int into kibana platform #26432)
• ML ([ML] Add rest_total_hit_as_int where total hits is required #26421)
• Monitoring ([Monitoring] Get ready for hits total format change #26442)
• Management ?

CC

Since I did a quick search through the repository for hits.total it seems the following teams are effected and might establish a similar progress for moving over.

cc @elastic/kibana-platform used in es_archiver and saved objects
cc @elastic/apm-ui Used in a couple of places
cc @elastic/ml-ui Used in a couple of places
cc @elastic/kibana-management (I couldn't find a handle for elasticsearch UI yet): used in watcher
cc whoever manages Stack monitoring

[elasticmachine]

Pinging @elastic/kibana-app

[stacey-gammon]

The plan is for that Elasticsearch PR to be merged on Monday which means in order to prevent ci breakage, Step 1 should be complete before then. And in order to verify that ci will continue to run once that PR is merged, we should test ci ahead of time against that branch.

@spalger - do you know the best way to run ci against an elasticsearch branch? I don't want to run it locally. I could create a kibana branch called total_hits_as_an_object but that wouldn't work because Jim's branch with the same name isn't on ES, just his fork, I believe. Do you know a way to accomplish this?

cc @LeeDr

[stacey-gammon]

never mind that question @spalger, @timroes knows how to do it and is going to put up a PR so we can all see what failures will happen and see how we are doing at completing Step 1.

[epixa]

Is the rest_total_hits_as_int parameter going to spam deprecation logs in 7.0? If so, it seems like a reasonable approach for 6.x, but switching to the non-deprecated format needs to be a 7.0 blocker.

[timroes]

@jimczi Could you clarify on the deprecation log warning for 7.0 above?

[timroes]

I have setup a simple PR, that just builds against that ES PR, so everyone can see around what tests will fail master from Monday on, once the PR is merged: #26360

[LeeDr]

We'll need to test Kibana N.M -> Elasticsearch N.M+1
K6.5 with E6.6
K6.6 with E6.7

and Kibana N.M -> Elasticsearch N+1 .0
K6.7 with E7.0 (maybe also K6.6 with E7.0)

because users can upgrade their Elasticsearch before their Kibana and everything needs to work.

[cjcenizal]

@timroes I just created @elastic/es-ui to represent our team, thanks for the nudge.

[jimczi]

Is the rest_total_hits_as_int parameter going to spam deprecation logs in 7.0?

Setting rest_total_hits_as_int will not emit deprecation log warnings (at least not in 7.0).

[chrisronline]

Thanks @timroes. The handle is @elastic/stack-monitoring

[peterschretlen]

@jimczi I assume rest_total_hits_as_int is going to be removed in 8.0? I couldn't find a corresponding issue in the elasticsearch repo, but it will take some effort to remove it from Kibana.

A quick search shows ~50 places the rest_total_hits_as_int parameter is used, and ~120 references to hits.total. So we'll need to coordinate making the change, and depending on the timing we'll need to start the effort on the Kibana side soon...

[peterschretlen]

Discussed with @jimczi and support for rest_total_hits_as_int is likely past 8.0. There area a few factors:

• there's desire to have a versioned Elasticsearch API, which (if available in 8.0) would allow rest_total_hits_as_int to be used in 8.0
• the usage of the flag in the stack is another consideration, there are other tools besides Kibana (Watcher for instances) that must also move off rest_total_hits_as_int.

This does not change the fact that eventually rest_total_hits_as_int goes away eventually and parts of Kibana can benefit from the optimization it provides - this work still needs to be done.

I am going to leave the 'blocker' label for now until this is confirmed, but given the situation I don't think we need to migrate away from rest_total_hits_as_int urgently.

[stratoula]

This has been addressed