[Monitoring] Get ready for hits total format change #26442
Conversation
|
Pinging @elastic/kibana-monitoring |
💔 Build Failed |
|
Jenkins, test this - Change should now be on master snapshot |
💔 Build Failed |
|
@timroes I see the PR as still open though? |
|
Sorry for the confusion, the change, that allows that parameter empty (elastic/elasticsearch#36051) should now be there, and that would prevent CI from failing that it doesn't know the parameter. not the PR that actually changes the behavior. But it seems our CI has troubles picking up the ES changes. |
💔 Build Failed |
💚 Build Succeeded |
|
@mattapperson @justinkambic Feel free to review this if you have the time. No rush |
chrisronline
force-pushed
the
hits_total_format
branch
from
1d19fff
to
689456e
Compare
|
@justinkambic Thanks for the review. I've gone ahead and changed approach and only updated the few, specific api calls that need |
💔 Build Failed |
|
retest |
| @@ -56,6 +56,7 @@ export function getLastRecovery(req, esIndexPattern) { | |||
| const params = { | |||
| index: esIndexPattern, | |||
| size: 1, | |||
| rest_total_hits_as_int: true, | |||
There was a problem hiding this comment.
For this one, we can change the handler code to just see if hits.hits.length === 1 (we only request size: 1), then do the work. This would remove the need for this parameter.
There was a problem hiding this comment.
Sounds good, will adjust
| @@ -55,6 +55,7 @@ export async function getNodes(req, esIndexPattern, clusterStats, shardStats) { | |||
| index: esIndexPattern, | |||
| size: config.get('xpack.monitoring.max_bucket_size'), | |||
| ignoreUnavailable: true, | |||
| rest_total_hits_as_int: true, | |||
There was a problem hiding this comment.
We're using hits.total here, but we could also change to checking hits.hits.length > 0 again. In this particular case, we are using field collapsing, so hits.total is inherently incorrect anyway (an overestimate), so it was really just a matter of simplicity to check hits.total for not being 0.
There was a problem hiding this comment.
Sounds good, will adjust
| @@ -43,6 +43,7 @@ export function getShardStats(req, esIndexPattern, cluster, { includeNodes = fal | |||
| index: esIndexPattern, | |||
| size: 0, | |||
| ignoreUnavailable: true, | |||
| rest_total_hits_as_int: true, | |||
There was a problem hiding this comment.
In this case we're actually interested in the hits.total being non-zero, but we cannot rely on hits.hits.length because we use size: 0.
In this case though, we can just check the indices.buckets to see if it has a length > 0, then use it that way. Alternatively, we could just drop the parameter and rely on the by-default inaccurate count coming back to still be >= 1 by changing it to hits.total.value from hits.total.
There was a problem hiding this comment.
Sounds good, will adjust
💚 Build Succeeded |
💔 Build Failed |
💔 Build Failed |
|
retest |
💔 Build Failed |
|
retest |
💚 Build Succeeded |
Relates to #26356
This PR prepares the Monitoring UI codebase for the incoming changes to
hits.total.