Skip to content

Commit 210ce5d

Browse files
deshsidddeshsidd
authored
Add fetch top queries by id API (#195)
Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>
1 parent 47dc89e commit 210ce5d

File tree

13 files changed

+156
-65
lines changed

13 files changed

+156
-65
lines changed

‎src/main/java/org/opensearch/plugin/insights/core/reader/LocalIndexReader.java

+9-5
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@
2525
import org.opensearch.core.xcontent.NamedXContentRegistry;
2626
import org.opensearch.core.xcontent.XContentParser;
2727
import org.opensearch.index.IndexNotFoundException;
28+
import org.opensearch.index.query.BoolQueryBuilder;
2829
import org.opensearch.index.query.MatchQueryBuilder;
29-
import org.opensearch.index.query.QueryBuilder;
3030
import org.opensearch.index.query.QueryBuilders;
3131
import org.opensearch.index.query.RangeQueryBuilder;
3232
import org.opensearch.plugin.insights.core.metrics.OperationalMetric;
@@ -84,11 +84,12 @@ public LocalIndexReader setIndexPattern(DateTimeFormatter indexPattern) {
8484
* Export a list of SearchQueryRecord from local index
8585
*
8686
* @param from start timestamp
87-
* @param to end timestamp
87+
* @param to end timestamp
88+
* @param id query/group id
8889
* @return list of SearchQueryRecords whose timestamps fall between from and to
8990
*/
9091
@Override
91-
public List<SearchQueryRecord> read(final String from, final String to) {
92+
public List<SearchQueryRecord> read(final String from, final String to, String id) {
9293
List<SearchQueryRecord> records = new ArrayList<>();
9394
if (from == null || to == null) {
9495
return records;
@@ -108,7 +109,11 @@ public List<SearchQueryRecord> read(final String from, final String to) {
108109
RangeQueryBuilder rangeQuery = QueryBuilders.rangeQuery("timestamp")
109110
.from(start.toInstant().toEpochMilli())
110111
.to(end.toInstant().toEpochMilli());
111-
QueryBuilder query = QueryBuilders.boolQuery().must(rangeQuery).mustNot(excludeQuery);
112+
BoolQueryBuilder query = QueryBuilders.boolQuery().must(rangeQuery).mustNot(excludeQuery);
113+
114+
if (id != null) {
115+
query.must(QueryBuilders.matchQuery("id", id));
116+
}
112117
searchSourceBuilder.query(query);
113118
searchRequest.source(searchSourceBuilder);
114119
try {
@@ -124,7 +129,6 @@ public List<SearchQueryRecord> read(final String from, final String to) {
124129
logger.error("Unable to parse search hit: ", e);
125130
}
126131
curr = curr.plusDays(1);
127-
128132
}
129133
return records;
130134
}

‎src/main/java/org/opensearch/plugin/insights/core/reader/QueryInsightsReader.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,9 @@ public interface QueryInsightsReader extends Closeable {
2020
* Reader a list of SearchQueryRecord
2121
*
2222
* @param from string
23-
* @param to string
23+
* @param to string
24+
* @param id query/group id
2425
* @return List of SearchQueryRecord
2526
*/
26-
List<SearchQueryRecord> read(final String from, final String to);
27+
List<SearchQueryRecord> read(final String from, final String to, final String id);
2728
}

‎src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesService.java

+20-5
Original file line numberDiff line numberDiff line change
@@ -357,11 +357,16 @@ public void validateExporterAndReaderConfig(Settings settings) {
357357
* @param includeLastWindow if the top N queries from the last window should be included
358358
* @param from start timestamp
359359
* @param to end timestamp
360+
* @param id unique identifier for query/query group
360361
* @return List of the records that are in the query insight store
361362
* @throws IllegalArgumentException if query insights is disabled in the cluster
362363
*/
363-
public List<SearchQueryRecord> getTopQueriesRecords(final boolean includeLastWindow, final String from, final String to)
364-
throws IllegalArgumentException {
364+
public List<SearchQueryRecord> getTopQueriesRecords(
365+
final boolean includeLastWindow,
366+
final String from,
367+
final String to,
368+
final String id
369+
) throws IllegalArgumentException {
365370
OperationalMetricsCounter.getInstance()
366371
.incrementCounter(
367372
OperationalMetric.TOP_N_QUERIES_USAGE_COUNT,
@@ -380,13 +385,21 @@ public List<SearchQueryRecord> getTopQueriesRecords(final boolean includeLastWin
380385
queries.addAll(topQueriesHistorySnapshot.get());
381386
}
382387
List<SearchQueryRecord> filterQueries = queries;
388+
389+
// Time-based filtering
383390
if (from != null && to != null) {
384391
final ZonedDateTime start = ZonedDateTime.parse(from);
385392
final ZonedDateTime end = ZonedDateTime.parse(to);
386393
Predicate<SearchQueryRecord> timeFilter = element -> start.toInstant().toEpochMilli() <= element.getTimestamp()
387394
&& element.getTimestamp() <= end.toInstant().toEpochMilli();
388395
filterQueries = queries.stream().filter(checkIfInternal.and(timeFilter)).collect(Collectors.toList());
389396
}
397+
398+
// Filter based on the id, if provided
399+
if (id != null) {
400+
filterQueries = filterQueries.stream().filter(record -> record.getId().equals(id)).collect(Collectors.toList());
401+
}
402+
390403
return Stream.of(filterQueries)
391404
.flatMap(Collection::stream)
392405
.sorted((a, b) -> SearchQueryRecord.compare(a, b, metricType) * -1)
@@ -399,11 +412,13 @@ public List<SearchQueryRecord> getTopQueriesRecords(final boolean includeLastWin
399412
* By default, return the records in sorted order.
400413
*
401414
* @param from start timestamp
402-
* @param to end timestamp
415+
* @param to end timestamp
416+
* @param id search query record id
403417
* @return List of the records that are in local index (if enabled) with timestamps between from and to
404418
* @throws IllegalArgumentException if query insights is disabled in the cluster
405419
*/
406-
public List<SearchQueryRecord> getTopQueriesRecordsFromIndex(final String from, final String to) throws IllegalArgumentException {
420+
public List<SearchQueryRecord> getTopQueriesRecordsFromIndex(final String from, final String to, final String id)
421+
throws IllegalArgumentException {
407422
if (!enabled) {
408423
throw new IllegalArgumentException(
409424
String.format(Locale.ROOT, "Cannot get top n queries for [%s] when it is not enabled.", metricType.toString())
@@ -415,7 +430,7 @@ public List<SearchQueryRecord> getTopQueriesRecordsFromIndex(final String from,
415430
try {
416431
final ZonedDateTime start = ZonedDateTime.parse(from);
417432
final ZonedDateTime end = ZonedDateTime.parse(to);
418-
List<SearchQueryRecord> records = reader.read(from, to);
433+
List<SearchQueryRecord> records = reader.read(from, to, id);
419434
Predicate<SearchQueryRecord> timeFilter = element -> start.toInstant().toEpochMilli() <= element.getTimestamp()
420435
&& element.getTimestamp() <= end.toInstant().toEpochMilli();
421436
List<SearchQueryRecord> filteredRecords = records.stream()

‎src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java

+12-1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ public class TopQueriesRequest extends BaseNodesRequest<TopQueriesRequest> {
2222
final MetricType metricType;
2323
final String from;
2424
final String to;
25+
final String id;
2526

2627
/**
2728
* Constructor for TopQueriesRequest
@@ -34,6 +35,7 @@ public TopQueriesRequest(final StreamInput in) throws IOException {
3435
this.metricType = MetricType.readFromStream(in);
3536
this.from = null;
3637
this.to = null;
38+
this.id = null;
3739
}
3840

3941
/**
@@ -45,11 +47,12 @@ public TopQueriesRequest(final StreamInput in) throws IOException {
4547
* @param to end timestamp
4648
* @param nodesIds the nodeIds specified in the request
4749
*/
48-
public TopQueriesRequest(final MetricType metricType, final String from, final String to, final String... nodesIds) {
50+
public TopQueriesRequest(final MetricType metricType, final String from, final String to, final String id, final String... nodesIds) {
4951
super(nodesIds);
5052
this.metricType = metricType;
5153
this.from = from;
5254
this.to = to;
55+
this.id = id;
5356
}
5457

5558
/**
@@ -76,6 +79,14 @@ public String getTo() {
7679
return to;
7780
}
7881

82+
/**
83+
* Get id which is the query_id and query_group_id
84+
* @return String of to timestamp
85+
*/
86+
public String getId() {
87+
return id;
88+
}
89+
7990
@Override
8091
public void writeTo(final StreamOutput out) throws IOException {
8192
super.writeTo(out);

‎src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java

+9
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,15 @@ public long getTimestamp() {
306306
return timestamp;
307307
}
308308

309+
/**
310+
* Returns the id.
311+
*
312+
* @return the id
313+
*/
314+
public String getId() {
315+
return id;
316+
}
317+
309318
/**
310319
* Returns the measurement associated with the specified name.
311320
*

‎src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java

+33-27
Original file line numberDiff line numberDiff line change
@@ -79,40 +79,46 @@ static TopQueriesRequest prepareRequest(final RestRequest request) {
7979
final String metricType = request.param("type", MetricType.LATENCY.toString());
8080
final String from = request.param("from", null);
8181
final String to = request.param("to", null);
82+
final String id = request.param("id", null);
8283
if (!ALLOWED_METRICS.contains(metricType)) {
8384
throw new IllegalArgumentException(
8485
String.format(Locale.ROOT, "request [%s] contains invalid metric type [%s]", request.path(), metricType)
8586
);
8687
}
87-
if (from != null || to != null) {
88-
if (from != null ^ to != null) {
89-
throw new IllegalArgumentException(
90-
String.format(Locale.ROOT, "request [%s] is missing one of the time parameters. Both must be provided", request.path())
91-
);
92-
}
93-
if (isNotISODate(from)) {
94-
throw new IllegalArgumentException(
95-
String.format(
96-
Locale.ROOT,
97-
"request [%s] contains invalid 'from' date format. Expected ISO8601 format string (YYYY-MM-DD'T'HH:mm:ss.SSSZ): [%s]",
98-
request.path(),
99-
from
100-
)
101-
);
102-
}
103-
if (isNotISODate(to)) {
104-
throw new IllegalArgumentException(
105-
String.format(
106-
Locale.ROOT,
107-
"request [%s] contains invalid 'to' date format. Expected ISO8601 format string (YYYY-MM-DD'T'HH:mm:ss.SSSZ): [%s]",
108-
request.path(),
109-
to
110-
)
111-
);
112-
}
88+
boolean isTimeRangeProvided = from != null || to != null;
89+
if (isTimeRangeProvided) {
90+
validateTimeRange(request, from, to);
11391
}
11492

115-
return new TopQueriesRequest(MetricType.fromString(metricType), from, to, nodesIds);
93+
return new TopQueriesRequest(MetricType.fromString(metricType), from, to, id, nodesIds);
94+
}
95+
96+
private static void validateTimeRange(RestRequest request, String from, String to) {
97+
if (from != null ^ to != null) {
98+
throw new IllegalArgumentException(
99+
String.format(Locale.ROOT, "request [%s] is missing one of the time parameters. Both must be provided", request.path())
100+
);
101+
}
102+
if (isNotISODate(from)) {
103+
throw new IllegalArgumentException(
104+
String.format(
105+
Locale.ROOT,
106+
"request [%s] contains invalid 'from' date format. Expected ISO8601 format string (YYYY-MM-DD'T'HH:mm:ss.SSSZ): [%s]",
107+
request.path(),
108+
from
109+
)
110+
);
111+
}
112+
if (isNotISODate(to)) {
113+
throw new IllegalArgumentException(
114+
String.format(
115+
Locale.ROOT,
116+
"request [%s] contains invalid 'to' date format. Expected ISO8601 format string (YYYY-MM-DD'T'HH:mm:ss.SSSZ): [%s]",
117+
request.path(),
118+
to
119+
)
120+
);
121+
}
116122
}
117123

118124
@Override

‎src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,12 @@ protected TopQueriesResponse newResponse(
8888
}
8989
final String from = topQueriesRequest.getFrom();
9090
final String to = topQueriesRequest.getTo();
91+
final String id = topQueriesRequest.getId();
9192
if (from != null && to != null) {
9293
responses.add(
9394
new TopQueries(
9495
clusterService.localNode(),
95-
queryInsightsService.getTopQueriesService(topQueriesRequest.getMetricType()).getTopQueriesRecordsFromIndex(from, to)
96+
queryInsightsService.getTopQueriesService(topQueriesRequest.getMetricType()).getTopQueriesRecordsFromIndex(from, to, id)
9697
)
9798
);
9899
}
@@ -114,9 +115,10 @@ protected TopQueries nodeOperation(final NodeRequest nodeRequest) {
114115
final TopQueriesRequest request = nodeRequest.request;
115116
final String from = request.getFrom();
116117
final String to = request.getTo();
118+
final String id = request.getId();
117119
return new TopQueries(
118120
clusterService.localNode(),
119-
queryInsightsService.getTopQueriesService(request.getMetricType()).getTopQueriesRecords(true, from, to)
121+
queryInsightsService.getTopQueriesService(request.getMetricType()).getTopQueriesRecords(true, from, to, id)
120122
);
121123
}
122124

‎src/test/java/org/opensearch/plugin/insights/QueryInsightsTestUtils.java

+20-6
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import java.util.Objects;
3333
import java.util.Set;
3434
import java.util.TreeSet;
35+
import java.util.UUID;
3536
import org.opensearch.action.search.SearchType;
3637
import org.opensearch.cluster.node.DiscoveryNode;
3738
import org.opensearch.common.settings.ClusterSettings;
@@ -54,15 +55,26 @@
5455

5556
final public class QueryInsightsTestUtils {
5657

58+
static String randomId = UUID.randomUUID().toString();
59+
5760
public QueryInsightsTestUtils() {}
5861

62+
/**
63+
* Returns list of randomly generated search query records with a specific id
64+
* @param count number of records
65+
* @return List of records
66+
*/
67+
public static List<SearchQueryRecord> generateQueryInsightRecords(int count, String id) {
68+
return generateQueryInsightRecords(count, count, System.currentTimeMillis(), 0, AggregationType.DEFAULT_AGGREGATION_TYPE, id);
69+
}
70+
5971
/**
6072
* Returns list of randomly generated search query records.
6173
* @param count number of records
6274
* @return List of records
6375
*/
6476
public static List<SearchQueryRecord> generateQueryInsightRecords(int count) {
65-
return generateQueryInsightRecords(count, count, System.currentTimeMillis(), 0, AggregationType.DEFAULT_AGGREGATION_TYPE);
77+
return generateQueryInsightRecords(count, count, System.currentTimeMillis(), 0, AggregationType.DEFAULT_AGGREGATION_TYPE, randomId);
6678
}
6779

6880
/**
@@ -77,7 +89,8 @@ public static List<SearchQueryRecord> generateQueryInsightRecords(int count, Sea
7789
count,
7890
System.currentTimeMillis(),
7991
0,
80-
AggregationType.DEFAULT_AGGREGATION_TYPE
92+
AggregationType.DEFAULT_AGGREGATION_TYPE,
93+
randomId
8194
);
8295
for (SearchQueryRecord record : records) {
8396
record.getAttributes().put(Attribute.SOURCE, searchSourceBuilder);
@@ -92,14 +105,14 @@ public static List<SearchQueryRecord> generateQueryInsightRecords(int count, Sea
92105
* @return List of records
93106
*/
94107
public static List<SearchQueryRecord> generateQueryInsightRecords(int count, AggregationType aggregationType) {
95-
return generateQueryInsightRecords(count, count, System.currentTimeMillis(), 0, aggregationType);
108+
return generateQueryInsightRecords(count, count, System.currentTimeMillis(), 0, aggregationType, randomId);
96109
}
97110

98111
/**
99112
* Creates a List of random Query Insight Records for testing purpose
100113
*/
101114
public static List<SearchQueryRecord> generateQueryInsightRecords(int lower, int upper, long startTimeStamp, long interval) {
102-
return generateQueryInsightRecords(lower, upper, startTimeStamp, interval, AggregationType.NONE);
115+
return generateQueryInsightRecords(lower, upper, startTimeStamp, interval, AggregationType.NONE, randomId);
103116
}
104117

105118
/**
@@ -110,7 +123,8 @@ public static List<SearchQueryRecord> generateQueryInsightRecords(
110123
int upper,
111124
long startTimeStamp,
112125
long interval,
113-
AggregationType aggregationType
126+
AggregationType aggregationType,
127+
String id
114128
) {
115129
List<SearchQueryRecord> records = new ArrayList<>();
116130
int countOfRecords = randomIntBetween(lower, upper);
@@ -161,7 +175,7 @@ public static List<SearchQueryRecord> generateQueryInsightRecords(
161175
)
162176
);
163177

164-
records.add(new SearchQueryRecord(timestamp, measurements, attributes));
178+
records.add(new SearchQueryRecord(timestamp, measurements, attributes, id));
165179
timestamp += interval;
166180
}
167181
return records;

‎src/test/java/org/opensearch/plugin/insights/core/reader/LocalIndexReaderTests.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,10 @@ public void testReadRecords() {
8484
when(responseActionFuture.actionGet()).thenReturn(searchResponse);
8585
when(client.search(any(SearchRequest.class))).thenReturn(responseActionFuture);
8686
String time = ZonedDateTime.now(ZoneOffset.UTC).format(DateTimeFormatter.ISO_DATE_TIME);
87+
String id = "example-hashcode";
8788
List<SearchQueryRecord> records = List.of();
8889
try {
89-
records = localIndexReader.read(time, time);
90+
records = localIndexReader.read(time, time, id);
9091
} catch (Exception e) {
9192
fail("No exception should be thrown when reading query insights data");
9293
}

0 commit comments

Comments
 (0)