[Search source] Use response filtering to reduce date histogram response size #80740
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wylieconlon
added
Feature:Aggregations
|
Pinging @elastic/kibana-app-arch (Team:AppArch) |
lukeelmers
reviewed
Oct 16, 2020
Comment on lines
+202
to
+209
| /** | ||
| * Use ES response filtering to reduce JSON size | ||
| * @param filterPath Array of filter strings, such as ['*', '-**.key_as_string'] | ||
| */ | ||
| setFilterPath(filterPaths: string[]) { | ||
| this.filterPaths = filterPaths; | ||
| return this; | ||
| } |
There was a problem hiding this comment.
Eventually it might be nice to expose a more generic way to directly add any param to a request, but a purpose-built method makes sense to me for the time being.
There was a problem hiding this comment.
Also could we add a unit test for this method, just to validate that the params make their way into the request?
| @@ -111,6 +111,8 @@ const handleCourierRequest = async ({ | |||
| return aggs.toDsl(metricsAtAllLevels); | |||
| }); | |||
|
|
|||
| requestSearchSource.setFilterPath(['*', '-**.key_as_string']); | |||
There was a problem hiding this comment.
nit: Could we include a comment here so that its purpose is clear? Perhaps a link to the key_as_string docs or some indication that this is specific to date histogram responses.
|
@lukeelmers It turns out that it wasn't "working great" in all scenarios: I found a bug in Elasticsearch which I'm doing a client-side workaround for. |
wylieconlon
commented
Oct 16, 2020
| aggBucket, | ||
| agg.params, | ||
| respOpts?.timeRange, | ||
| agg.type.name === 'filter' |
There was a problem hiding this comment.
I'd appreciate another look at this from @lukeelmers or others: I need to identify "normal" aggregations vs unusual ones, and the filter agg is the one that was called out in docs. I think I have two options:
- Special case for
filter, like this - Add a property to
AggTypeto indicate that it's an unusual response shape
There was a problem hiding this comment.
To summarize our offline chat -- in general I would prefer to wait and see what the ES team says about elastic/elasticsearch#63842. If it's something that can be addressed shorter-term, I'd say let's just hold off on this PR.
If it can't be addressed shorter term, I would vote for addressing this as part of a wider tabify refactor... I think right now tabify knows too much about aggs, and we could probably find a way to move some of this logic over to individual agg types. (Would need to think about it, but perhaps something like a method on each agg type that can convert its response into a standardized tabify-compatible format).
So in that sense, I think an approach closer to option 2 feels like the right direction, but perhaps something more extensive than a flag that just says whether the agg response is "normal" or not.
I'm also not sure which agg types besides filter might match this condition. Would need to look into each of them individually, unless anyone else already knows (@ppisljar ?)
There was a problem hiding this comment.
Based on the latest discussion with the ES team, we're looking at handling this in ES. It's possible that response filtering is not the right mechanism for this, but I've also opened a feature request specifically for the date_histogram to have a way to remove the key_as_string parameter.
|
@elasticmachine merge upstream |
💔 Build Failed
Failed CI StepsTest FailuresChrome X-Pack UI Functional Tests.x-pack/test/functional/apps/dashboard/async_search/async_search·ts.dashboard async search dashboard with async search delayed should loadStandard OutStack TraceChrome X-Pack UI Functional Tests.x-pack/test/functional/apps/dashboard/async_search/async_search·ts.dashboard async search dashboard with async search delayed should loadStandard OutStack TraceMetrics [docs]page load bundle size
History
To update your PR or re-run it, just comment with: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR reduces the JSON payload that Kibana needs by approximately 50% by using response filtering parameters on esaggs.
Closes #80720
Checklist