Add recommendations parameter to Top Queries API#569
Conversation
ansjcy
requested review from
KishoreKicha14,
deshsidd,
dzane17 and
jainankitk
as code owners
ansjcy
force-pushed
the
feature/top-queries-recommendations
branch
2 times, most recently
from
e93143b to
7782da3
Compare
Adds ?recommendations=true query parameter to the top_queries endpoint that enriches each query record with inline recommendations generated by the recommendation service. Changes: - TopQueriesRequest: add recommendations Boolean field with serialization - TopQueries: add recommendations map (Map<String, List<Recommendation>>) with version-aware serialization, new 3-arg constructor - TopQueriesResponse: add transient recommendation enrichment context, merge pre-computed recommendations from data nodes during serialization, fall back to coordinator-side generation for historical records - RestTopQueriesAction: add recommendations query parameter - TransportTopQueriesAction: pre-compute recommendations on data nodes in nodeOperation, set recommendation context on responses in handleInMemoryDataResponse, onHistoricalDataResponse, and onHistoricalDataFailure - SearchQueryRecord: add toXContentWithRecommendations() method - Update all test files for new TopQueriesRequest constructor signature Signed-off-by: Chenyang Ji <cyji@amazon.com>
ansjcy
force-pushed
the
feature/top-queries-recommendations
branch
from
7782da3 to
498b028
Compare
ansjcy
changed the title
|
If no recommendations match the rules, do we want to return recommendations: []? It would make it clearer when recommendations is enabled but that there weren't any rule matches. |
| List<TopQueries> filteredNodes = response.getNodes().stream().map(topQueries -> { | ||
| List<SearchQueryRecord> filtered = TopQueriesRbacFilter.filterRecords(topQueries.getTopQueriesRecord(), mode, info); | ||
| return new TopQueries(topQueries.getNode(), filtered); | ||
| return new TopQueries(topQueries.getNode(), filtered, topQueries.getRecommendations()); |
There was a problem hiding this comment.
Nit: When RBAC filters out records here, the original recommendations map is still passed through unfiltered. The orphaned entries won't leak into the JSON response (since toClusterLevelResult only looks up recs for records that survived filtering), but they do get serialized over the wire unnecessarily. Consider filtering the recommendations map to only keep keys matching the filtered records:
Set<String> filteredIds = filtered.stream().map(SearchQueryRecord::getId).collect(Collectors.toSet());
Map<String, List<Recommendation>> filteredRecs = topQueries.getRecommendations().entrySet().stream()
.filter(e -> filteredIds.contains(e.getKey()))
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
return new TopQueries(topQueries.getNode(), filtered, filteredRecs);
If the recommendations map is large, this could matter?
| // Merge pre-computed recommendations from all node responses | ||
| final Map<String, List<Recommendation>> mergedRecommendations = new HashMap<>(); | ||
| for (TopQueries tq : results) { | ||
| mergedRecommendations.putAll(tq.getRecommendations()); |
There was a problem hiding this comment.
putAll here will quietly drop recommendations if two nodes happen to have the same record ID — last one wins. Probably won't happen because of dedup, but using putIfAbsent or merging lists would be safer
I have this same concern. Although if majority of the time the API invoked by a dashboard, displaying empty list does not matter as much. Otherwise this PR looks good |
Signed-off-by: Chenyang Ji <cyji@amazon.com>
ansjcy
force-pushed
the
feature/top-queries-recommendations
branch
2 times, most recently
from
c809de2 to
a7bcc50
Compare
4 participants
Description
Adds ?recommendations=true query parameter to the top_queries endpoint that enriches each query record with inline recommendations generated by the recommendation service.
Issues Resolved
List any issues this PR will resolve, e.g. Closes [...].
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.