[Search] Add shard delay aggregation#77423
Conversation
|
So far this makes sense to me based on our discussion, though we'll need to add an expression function for this agg type as well in order for it to work with the near-future version of esaggs. (Not necessary now, but will be necessary in 7.11, so might be worth getting it out of the way at the same time) |
|
Pinging @elastic/kibana-app-arch (Team:AppArch) |
|
@lukeelmers Added the expression function, could you give this another look? |
lukeelmers
left a comment
There was a problem hiding this comment.
Have a few minor comments but overall this makes sense to me!
It would be nice if we could add a few comments either where the agg type is registered, or in the agg type definition, or both. Just in case someone runs across this later and isn't sure why it is being handled differently.
lukeelmers
left a comment
There was a problem hiding this comment.
Code changes LGTM.
Tested locally with the setting enabled. This is going to be great for testing!
lizozom
left a comment
There was a problem hiding this comment.
Looks good.
As this flag doesn't even appear in kibana.yml, I feel we need some internal documentation on this flag.
|
|
||
| const aggs = this.aggsService.setup({ registerFunction }); | ||
|
|
||
| this.initializerContext.config |
There was a problem hiding this comment.
Could we pass config in instead of the whole initializer context?
There was a problem hiding this comment.
I think the general pattern for plugins/services is to pass in the entire initializer context in the constructor. (I didn't change the signature here to add an additional parameter, just specified what the config schema should look like. I did, however, change the signature on the public service, which matches the autocomplete service.)
|
@lizozom I added a comment in the config file explaining what it does. Other than that I'm not sure exactly where we should/would document the setting. Do you have any suggestions? |
|
@lukasolson the comment is really clear, thanks! |
|
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]@kbn/optimizer bundle module count
distributable file count
page load bundle size
History
To update your PR or re-run it, just comment with: |
* [Search] [WIP] Add shard delay aggregation * Add expression functions * Register function * Fix test * Add comment Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Summary
Waiting on https://github.com/elastic/elasticsearch/pull/62082/files.
Adds support for the
shard_delayagg type for use in future manual and/or functional tests.In elastic/elasticsearch#62082, support for a new aggregation,
shard_delay, is being added. It is only available in snapshots (not in production). It allows slow queries to be simulated, which is helpful in efforts to ensure Kibana runs smoothly in environments where queries are slow. This PR adds a newAggTypefor this aggregation which allows you to specify the delay for the query to run.This new
AggTypeis only registered when the kibana.yml setting,data.search.aggs.shardDelay.enabledis set totrue. By default, it isfalse.Checklist