-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Implement top_metrics agg #51155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Implement top_metrics agg #51155
Changes from all commits
Commits
Show all changes
71 commits
Select commit
Hold shift + click to select a range
0fb37b5
Implement top_metrics agg
nik9000 a105a63
Moar tests
nik9000 4d2557d
Better
nik9000 5146563
Checkstyle
nik9000 12abffd
Fixup docs compile
nik9000 a685e29
Explain
nik9000 0f130d4
Sneaky sneaky
nik9000 eacddb5
Merge branch 'master' into top_metrics
nik9000 2b7a62e
Throw
nik9000 caf1004
Merge branch 'master' into top_metrics
nik9000 9f12a5e
Factory time
nik9000 845a8e7
Move
nik9000 390c4c5
Use fancy test methods
nik9000 21a2387
Bucketed sort
nik9000 feeca63
Squish nocommits
nik9000 228ccc9
Merge branch 'master' into top_metrics
nik9000 5f71b84
Merge branch 'master' into top_metrics
nik9000 aa33abb
Merge branch 'master' into top_metrics
nik9000 a2569de
Move SortValue to core
nik9000 e2925e3
moar sort
nik9000 98091fa
Rename
nik9000 b9d8085
Cleanup
nik9000 935c755
Drop method from SortValue
nik9000 fb7f122
last test
nik9000 a6c69d8
Smaller collector?
nik9000 9544c15
Move to InternalAggregationTestCase
nik9000 6efdd14
high level client support
nik9000 0a44b09
A little better docs
nik9000 ef8afc3
Merge branch 'master' into top_metrics
nik9000 0b191f8
Test
nik9000 49ee3c7
Javadoc
nik9000 6d83a2a
yaml
nik9000 80bf931
Force define
nik9000 7f46898
Script
nik9000 c36f28e
geo_distance
nik9000 e846862
Fix indentation
nik9000 af3fe9d
Merge branch 'master' into top_metrics
nik9000 05c6464
WIP
nik9000 0a9e3cc
Merge branch 'master' into top_metrics
nik9000 36eb495
Fixup casting
nik9000 290cc27
Cleanup
nik9000 6dbc02b
Drop out of date comment
nik9000 5902e6c
Merge branch 'master' into top_metrics
nik9000 ca3acf2
Update docs
nik9000 1fc6a2a
numeric_type examples
nik9000 a06ca3c
Merge branch 'master' into top_metrics
nik9000 7762c4f
Ooops
nik9000 0b78715
Implement sorting by top_metrics
nik9000 ea2e997
Better shuffle
nik9000 bdd0676
Checkstyle
nik9000 09d969b
Merge branch 'master' into top_metrics
nik9000 d7429b6
Merge branch 'master' into top_metrics
nik9000 d1b90e2
Merge branch 'master' into top_metrics
nik9000 c7b6317
Docs and test
nik9000 ba6e21f
Tests
nik9000 fdada3c
Merge branch 'master' into top_metrics
nik9000 e47d6c6
Merge branch 'master' into top_metrics
nik9000 abfc012
Fix javadoc
nik9000 03ff2b5
Javadoc
nik9000 04c2fcd
Merge!
nik9000 9752139
Merge branch 'master' into top_metrics
nik9000 4240a53
Merge branch 'master' into top_metrics
nik9000 ca6e52d
Test
nik9000 792da85
Merge branch 'master' into top_metrics
nik9000 6577077
Merge branch 'master' into top_metrics
nik9000 43c705b
Merge tests
nik9000 5f46829
Flip example
nik9000 2b5dca1
No tie breaking
nik9000 e6fd601
Error message
nik9000 134d99b
Word
nik9000 896e08a
Another word
nik9000 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
134 changes: 134 additions & 0 deletions
134
...nt/rest-high-level/src/main/java/org/elasticsearch/client/analytics/ParsedTopMetrics.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| /* | ||
| * Licensed to Elasticsearch under one or more contributor | ||
| * license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright | ||
| * ownership. Elasticsearch licenses this file to you under | ||
| * the Apache License, Version 2.0 (the "License"); you may | ||
| * not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
|
|
||
| package org.elasticsearch.client.analytics; | ||
|
|
||
| import org.elasticsearch.common.ParseField; | ||
| import org.elasticsearch.common.xcontent.ConstructingObjectParser; | ||
| import org.elasticsearch.common.xcontent.ObjectParser; | ||
| import org.elasticsearch.common.xcontent.ToXContent; | ||
| import org.elasticsearch.common.xcontent.XContentBuilder; | ||
| import org.elasticsearch.common.xcontent.XContentParser; | ||
| import org.elasticsearch.common.xcontent.XContentParserUtils; | ||
| import org.elasticsearch.search.aggregations.ParsedAggregation; | ||
|
|
||
| import java.io.IOException; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
||
| import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg; | ||
|
|
||
| /** | ||
| * Results of the {@code top_metrics} aggregation. | ||
| */ | ||
| public class ParsedTopMetrics extends ParsedAggregation { | ||
| private static final ParseField TOP_FIELD = new ParseField("top"); | ||
|
|
||
| private final List<TopMetrics> topMetrics; | ||
|
|
||
| private ParsedTopMetrics(String name, List<TopMetrics> topMetrics) { | ||
| setName(name); | ||
| this.topMetrics = topMetrics; | ||
| } | ||
|
|
||
| /** | ||
| * The list of top metrics, in sorted order. | ||
| */ | ||
| public List<TopMetrics> getTopMetrics() { | ||
| return topMetrics; | ||
| } | ||
|
|
||
| @Override | ||
| public String getType() { | ||
| return TopMetricsAggregationBuilder.NAME; | ||
| } | ||
|
|
||
| @Override | ||
| protected XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException { | ||
| builder.startArray(TOP_FIELD.getPreferredName()); | ||
| for (TopMetrics top : topMetrics) { | ||
| top.toXContent(builder, params); | ||
| } | ||
| return builder.endArray(); | ||
| } | ||
|
|
||
| public static final ConstructingObjectParser<ParsedTopMetrics, String> PARSER = new ConstructingObjectParser<>( | ||
| TopMetricsAggregationBuilder.NAME, true, (args, name) -> { | ||
| @SuppressWarnings("unchecked") | ||
| List<TopMetrics> topMetrics = (List<TopMetrics>) args[0]; | ||
| return new ParsedTopMetrics(name, topMetrics); | ||
| }); | ||
| static { | ||
| PARSER.declareObjectArray(constructorArg(), (p, c) -> TopMetrics.PARSER.parse(p, null), TOP_FIELD); | ||
| ParsedAggregation.declareAggregationFields(PARSER); | ||
| } | ||
|
|
||
| /** | ||
| * The metrics belonging to the document with the "top" sort key. | ||
| */ | ||
| public static class TopMetrics implements ToXContent { | ||
| private static final ParseField SORT_FIELD = new ParseField("sort"); | ||
| private static final ParseField METRICS_FIELD = new ParseField("metrics"); | ||
|
|
||
| private final List<Object> sort; | ||
| private final Map<String, Double> metrics; | ||
|
|
||
| private TopMetrics(List<Object> sort, Map<String, Double> metrics) { | ||
| this.sort = sort; | ||
| this.metrics = metrics; | ||
| } | ||
|
|
||
| /** | ||
| * The sort key for these top metrics. | ||
| */ | ||
| public List<Object> getSort() { | ||
| return sort; | ||
| } | ||
|
|
||
| /** | ||
| * The top metric values returned by the aggregation. | ||
| */ | ||
| public Map<String, Double> getMetrics() { | ||
| return metrics; | ||
| } | ||
|
|
||
| private static final ConstructingObjectParser<TopMetrics, Void> PARSER = new ConstructingObjectParser<>("top", true, | ||
| (args, name) -> { | ||
| @SuppressWarnings("unchecked") | ||
| List<Object> sort = (List<Object>) args[0]; | ||
| @SuppressWarnings("unchecked") | ||
| Map<String, Double> metrics = (Map<String, Double>) args[1]; | ||
| return new TopMetrics(sort, metrics); | ||
| }); | ||
| static { | ||
| PARSER.declareFieldArray(constructorArg(), (p, c) -> XContentParserUtils.parseFieldsValue(p), | ||
| SORT_FIELD, ObjectParser.ValueType.VALUE_ARRAY); | ||
| PARSER.declareObject(constructorArg(), (p, c) -> p.map(HashMap::new, XContentParser::doubleValue), METRICS_FIELD); | ||
| } | ||
|
|
||
| public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException { | ||
| builder.startObject(); | ||
| builder.field(SORT_FIELD.getPreferredName(), sort); | ||
| builder.field(METRICS_FIELD.getPreferredName(), metrics); | ||
| builder.endObject(); | ||
| return builder; | ||
| }; | ||
| } | ||
| } |
97 changes: 97 additions & 0 deletions
97
...-level/src/main/java/org/elasticsearch/client/analytics/TopMetricsAggregationBuilder.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| /* | ||
| * Licensed to Elasticsearch under one or more contributor | ||
| * license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright | ||
| * ownership. Elasticsearch licenses this file to you under | ||
| * the Apache License, Version 2.0 (the "License"); you may | ||
| * not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
|
|
||
| package org.elasticsearch.client.analytics; | ||
|
|
||
| import org.elasticsearch.common.io.stream.StreamOutput; | ||
| import org.elasticsearch.common.io.stream.Writeable; | ||
| import org.elasticsearch.common.xcontent.XContentBuilder; | ||
| import org.elasticsearch.index.query.QueryRewriteContext; | ||
| import org.elasticsearch.index.query.QueryShardContext; | ||
| import org.elasticsearch.search.aggregations.AbstractAggregationBuilder; | ||
| import org.elasticsearch.search.aggregations.AggregationBuilder; | ||
| import org.elasticsearch.search.aggregations.AggregatorFactories.Builder; | ||
| import org.elasticsearch.search.aggregations.AggregatorFactory; | ||
| import org.elasticsearch.search.builder.SearchSourceBuilder; | ||
| import org.elasticsearch.search.sort.SortBuilder; | ||
|
|
||
| import java.io.IOException; | ||
| import java.util.Map; | ||
|
|
||
| /** | ||
| * Builds the Top Metrics aggregation request. | ||
| * <p> | ||
| * NOTE: This extends {@linkplain AbstractAggregationBuilder} for compatibility | ||
| * with {@link SearchSourceBuilder#aggregation(AggregationBuilder)} but it | ||
| * doesn't support any "server" side things like | ||
| * {@linkplain Writeable#writeTo(StreamOutput)}, | ||
| * {@linkplain AggregationBuilder#rewrite(QueryRewriteContext)}, or | ||
| * {@linkplain AbstractAggregationBuilder#build(QueryShardContext, AggregatorFactory)}. | ||
| */ | ||
| public class TopMetricsAggregationBuilder extends AbstractAggregationBuilder<TopMetricsAggregationBuilder> { | ||
| public static final String NAME = "top_metrics"; | ||
|
|
||
| private final SortBuilder<?> sort; | ||
| private final String metric; | ||
|
|
||
| /** | ||
| * Build the request. | ||
| * @param name the name of the metric | ||
| * @param sort the sort key used to select the top metrics | ||
| * @param metric the name of the field to select | ||
| */ | ||
| public TopMetricsAggregationBuilder(String name, SortBuilder<?> sort, String metric) { | ||
| super(name); | ||
| this.sort = sort; | ||
| this.metric = metric; | ||
| } | ||
|
|
||
| @Override | ||
| public String getType() { | ||
| return NAME; | ||
| } | ||
|
|
||
| @Override | ||
| protected XContentBuilder internalXContent(XContentBuilder builder, Params params) throws IOException { | ||
| builder.startObject(); | ||
| { | ||
| builder.startArray("sort"); | ||
| sort.toXContent(builder, params); | ||
| builder.endArray(); | ||
| builder.startObject("metric").field("field", metric).endObject(); | ||
| } | ||
| return builder.endObject(); | ||
| } | ||
|
|
||
| @Override | ||
| protected void doWriteTo(StreamOutput out) throws IOException { | ||
| throw new UnsupportedOperationException(); | ||
| } | ||
|
|
||
| @Override | ||
| protected AggregatorFactory doBuild(QueryShardContext queryShardContext, AggregatorFactory parent, Builder subfactoriesBuilder) | ||
| throws IOException { | ||
| throw new UnsupportedOperationException(); | ||
| } | ||
|
|
||
| @Override | ||
| protected AggregationBuilder shallowCopy(Builder factoriesBuilder, Map<String, Object> metaData) { | ||
| throw new UnsupportedOperationException(); | ||
| } | ||
| } | ||
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was confused by this, since usually things that descend from
AbstractAggregationBuilderare the entry points for that aggregation. In this case, it looks like we're just using theXContentBuilderaspect of this, presumably for the HLRC to build out requests? If that's the case, I think it would help to make clear in the javadoc that this is not the main builder for this aggregation, and be explicit about its intended use.I don't think I'm alone in thinking our aggregation builders do too much, and this seems symptomatic of that. Maybe at some point, we can talk about breaking up those roles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I have to descend from
AbstractAggregationBuilderto make the client happy. I can check.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also pretty sure you have to. All I'm suggesting is adding a note to the javadoc that explains what an aggregation builder is doing here, and that it's not the "main" builder for Top Metrics. It makes sense in context of this PR, but I am imagining a future where someone is looking at all classes that implement
AbstractAggregationBuilderand scratching their head.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added javadoc explaining this.