From b33ef50a97dcb3e1e460000dced77d085c8ade26 Mon Sep 17 00:00:00 2001 From: Chenyang Ji Date: Mon, 22 Jan 2024 18:25:55 -0800 Subject: [PATCH] Increase JavaDoc coverage and update PR based comments Signed-off-by: Chenyang Ji --- gradle/missing-javadoc.gradle | 1 - .../core/exporter/QueryInsightsExporter.java | 22 ++--- .../exporter/QueryInsightsExporterType.java | 16 ++-- .../QueryInsightsLocalIndexExporter.java | 84 +++++++++++-------- .../listener/SearchQueryLatencyListener.java | 8 ++ .../core/service/QueryInsightsService.java | 42 +++++++++- .../service/TopQueriesByLatencyService.java | 61 ++++++++++++++ .../rules/action/top_queries/TopQueries.java | 10 +++ .../action/top_queries/TopQueriesAction.java | 6 ++ .../action/top_queries/TopQueriesRequest.java | 22 ++++- .../top_queries/TopQueriesResponse.java | 14 ++++ .../rules/model/SearchQueryLatencyRecord.java | 43 ++++++++-- .../rules/model/SearchQueryRecord.java | 74 ++++++++++++---- .../top_queries/RestTopQueriesAction.java | 5 +- .../TransportTopQueriesAction.java | 18 ++++ .../settings/QueryInsightsSettings.java | 8 +- .../exporter/QueryInsightsExporterTests.java | 8 +- 17 files changed, 344 insertions(+), 98 deletions(-) diff --git a/gradle/missing-javadoc.gradle b/gradle/missing-javadoc.gradle index d3fb9f82c3715..e9a6d798b8323 100644 --- a/gradle/missing-javadoc.gradle +++ b/gradle/missing-javadoc.gradle @@ -141,7 +141,6 @@ configure([ project(":plugins:mapper-annotated-text"), project(":plugins:mapper-murmur3"), project(":plugins:mapper-size"), - project(":plugins:query-insights"), project(":plugins:repository-azure"), project(":plugins:repository-gcs"), project(":plugins:repository-hdfs"), diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporter.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporter.java index 21b084e957298..5f53dbb6e39db 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporter.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporter.java @@ -20,11 +20,9 @@ * @opensearch.internal */ public abstract class QueryInsightsExporter> { - private QueryInsightsExporterType type; - private String identifier; + private final String identifier; - QueryInsightsExporter(QueryInsightsExporterType type, String identifier) { - this.type = type; + QueryInsightsExporter(String identifier) { this.identifier = identifier; } @@ -35,18 +33,10 @@ public abstract class QueryInsightsExporter> { */ public abstract void export(List records) throws Exception; - public void setType(QueryInsightsExporterType type) { - this.type = type; - } - - public QueryInsightsExporterType getType() { - return type; - } - - public void setIdentifier(String identifier) { - this.identifier = identifier; - } - + /** + * Get the identifier of this exporter + * @return identifier of this exporter + */ public String getIdentifier() { return identifier; } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterType.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterType.java index 92466411ac47c..7c2a9b86fb642 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterType.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterType.java @@ -16,15 +16,19 @@ * @opensearch.internal */ public enum QueryInsightsExporterType { - /* local index exporter */ - LOCAL_INDEX("local_index"); + /** local index exporter */ + LOCAL_INDEX; - private final String type; - - QueryInsightsExporterType(String type) { - this.type = type; + @Override + public String toString() { + return super.toString().toLowerCase(Locale.ROOT); } + /** + * Parse QueryInsightsExporterType from String + * @param type the String representation of the QueryInsightsExporterType + * @return QueryInsightsExporterType + */ public static QueryInsightsExporterType parse(String type) { return valueOf(type.toUpperCase(Locale.ROOT)); } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsLocalIndexExporter.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsLocalIndexExporter.java index b9cbfb80c325c..6a79442a9b559 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsLocalIndexExporter.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsLocalIndexExporter.java @@ -51,13 +51,20 @@ public class QueryInsightsLocalIndexExporter> ext /** The mapping for the local index that holds the data */ private final InputStream localIndexMapping; + /** + * Create a QueryInsightsLocalIndexExporter Object + * @param clusterService The clusterService of the node + * @param client The OpenSearch Client to support index operations + * @param localIndexName The local index name to export the data to + * @param localIndexMapping The mapping for the local index + */ public QueryInsightsLocalIndexExporter( ClusterService clusterService, Client client, String localIndexName, InputStream localIndexMapping ) { - super(QueryInsightsExporterType.LOCAL_INDEX, localIndexName); + super(localIndexName); this.clusterService = clusterService; this.client = client; this.localIndexMapping = localIndexMapping; @@ -70,42 +77,41 @@ public QueryInsightsLocalIndexExporter( * @throws IOException if an error occurs */ @Override - public synchronized void export(List records) throws IOException { + public void export(List records) throws IOException { if (records.size() == 0) { return; } - if (checkIfIndexExists()) { - bulkRecord(records); - } else { - // local index not exist - initLocalIndex(new ActionListener<>() { - @Override - public void onResponse(CreateIndexResponse response) { - if (response.isAcknowledged()) { - log.debug( - String.format(Locale.ROOT, "successfully initialized local index %s for query insight.", getIdentifier()) - ); - try { - bulkRecord(records); - } catch (IOException e) { - log.error(String.format(Locale.ROOT, "fail to ingest query insight data to local index, error: %s", e)); - } - } else { - log.error( - String.format( - Locale.ROOT, - "request to created local index %s for query insight not acknowledged.", - getIdentifier() - ) - ); + boolean indexExists = checkAndInitLocalIndex(new ActionListener<>() { + @Override + public void onResponse(CreateIndexResponse response) { + if (response.isAcknowledged()) { + log.debug( + String.format(Locale.ROOT, "successfully initialized local index %s for query insight.", getIdentifier()) + ); + try { + bulkRecord(records); + } catch (IOException e) { + log.error(String.format(Locale.ROOT, "fail to ingest query insight data to local index, error: %s", e)); } + } else { + log.error( + String.format( + Locale.ROOT, + "request to created local index %s for query insight not acknowledged.", + getIdentifier() + ) + ); } + } - @Override - public void onFailure(Exception e) { - log.error(String.format(Locale.ROOT, "error creating local index for query insight: %s", e)); - } - }); + @Override + public void onFailure(Exception e) { + log.error(String.format(Locale.ROOT, "error creating local index for query insight: %s", e)); + } + }); + + if (indexExists) { + bulkRecord(records); } } @@ -120,15 +126,21 @@ private boolean checkIfIndexExists() { } /** - * Initialize the local OpenSearch Index for the exporter + * Check and initialize the local OpenSearch Index for the exporter * * @param listener the listener to be notified upon completion + * @return boolean to represent if the index has already been created before calling this function * @throws IOException if an error occurs */ - private synchronized void initLocalIndex(ActionListener listener) throws IOException { - CreateIndexRequest createIndexRequest = new CreateIndexRequest(this.getIdentifier()).mapping(getIndexMappings()) - .settings(Settings.builder().put("index.hidden", false).build()); - client.admin().indices().create(createIndexRequest, listener); + private synchronized boolean checkAndInitLocalIndex(ActionListener listener) throws IOException { + if (!checkIfIndexExists()) { + CreateIndexRequest createIndexRequest = new CreateIndexRequest(this.getIdentifier()).mapping(getIndexMappings()) + .settings(Settings.builder().put("index.hidden", false).build()); + client.admin().indices().create(createIndexRequest, listener); + return false; + } else { + return true; + } } /** diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListener.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListener.java index 4a7a3ff9dc547..fe532b1496d92 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListener.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/SearchQueryLatencyListener.java @@ -16,9 +16,11 @@ import org.opensearch.action.search.SearchRequestOperationsListener; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.inject.Inject; +import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.plugin.insights.core.service.TopQueriesByLatencyService; +import java.io.IOException; import java.util.Collections; import java.util.HashMap; import java.util.Locale; @@ -43,6 +45,12 @@ public final class SearchQueryLatencyListener extends SearchRequestOperationsLis private final TopQueriesByLatencyService topQueriesByLatencyService; + /** + * Create a new SearchQueryLatencyListener object + * + * @param clusterService The Node's cluster service. + * @param topQueriesByLatencyService The topQueriesByLatencyService associated with this listener + */ @Inject public SearchQueryLatencyListener(ClusterService clusterService, TopQueriesByLatencyService topQueriesByLatencyService) { this.topQueriesByLatencyService = topQueriesByLatencyService; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java index 64e45e4327eee..bf97707c7f3bd 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java @@ -46,7 +46,7 @@ public abstract class QueryInsightsService, S ext /** enable insight data export */ private boolean enableExport; - /** The internal store that holds the query insight data */ + /** The internal thread-safe store that holds the query insight data */ @Nullable protected S store; @@ -59,8 +59,19 @@ public abstract class QueryInsightsService, S ext /** The internal OpenSearch thread pool that execute async processing and exporting tasks*/ protected final ThreadPool threadPool; + + /** + * Holds a reference to delayed operation {@link Scheduler.Cancellable} so it can be cancelled when + * the service closed concurrently. + */ protected volatile Scheduler.Cancellable scheduledFuture; + /** + * Create the Query Insights Service object + * @param threadPool The OpenSearch thread pool to run async tasks + * @param store The in memory store to keep the Query Insights data + * @param exporter The optional {@link QueryInsightsExporter} to export the Query Insights data + */ @Inject public QueryInsightsService(ThreadPool threadPool, @Nullable S store, @Nullable E exporter) { this.threadPool = threadPool; @@ -102,7 +113,13 @@ public List getQueryData() throws IllegalArgumentException { public abstract void clearOutdatedData(); /** - * Restart the exporter with new config + * Reset the exporter with new config + * + * This function can be used to enable/disable an exporter, change the type of the exporter, + * or change the identifier of the exporter. + * @param enabled the enable flag to set on the exporter + * @param type The QueryInsightsExporterType to set on the exporter + * @param identifier the Identifier to set on the exporter */ public abstract void resetExporter(boolean enabled, QueryInsightsExporterType type, String identifier); @@ -113,18 +130,34 @@ public void clearAllData() { store.clear(); } + /** + * Set flag to enable or disable Query Insights data collection + * @param enableCollect Flag to enable or disable Query Insights data collection + */ public void setEnableCollect(boolean enableCollect) { this.enableCollect = enableCollect; } + /** + * Get if the Query Insights data collection is enabled + * @return if the Query Insights data collection is enabled + */ public boolean getEnableCollect() { return this.enableCollect; } + /** + * Set flag to enable or disable Query Insights data export + * @param enableExport + */ public void setEnableExport(boolean enableExport) { this.enableExport = enableExport; } + /** + * Get if the Query Insights data export is enabled + * @return if the Query Insights data export is enabled + */ public boolean getEnableExport() { return this.enableExport; } @@ -156,7 +189,6 @@ private void doExport() { List storedData = getQueryData(); try { exporter.export(storedData); - log.debug(String.format(Locale.ROOT, "finish exporting query insight data to sink %s", storedData)); } catch (Exception e) { throw new RuntimeException(String.format(Locale.ROOT, "failed to export query insight data to sink, error: %s", e)); } @@ -165,6 +197,10 @@ private void doExport() { @Override protected void doClose() {} + /** + * Get the export interval set for the {@link QueryInsightsExporter} + * @return export interval + */ public TimeValue getExportInterval() { return exportInterval; } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyService.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyService.java index 5914239b4054e..7ce0e110886fd 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyService.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesByLatencyService.java @@ -50,6 +50,12 @@ public class TopQueriesByLatencyService extends QueryInsightsService< private final ClusterService clusterService; private final Client client; + /** + * Create the TopQueriesByLatencyService Object + * @param threadPool The OpenSearch thread pool to run async tasks + * @param clusterService The clusterService of this node + * @param client The OpenSearch Client + */ @Inject public TopQueriesByLatencyService(ThreadPool threadPool, ClusterService clusterService, Client client) { super(threadPool, new PriorityBlockingQueue<>(), null); @@ -117,10 +123,18 @@ public void clearOutdatedData() { store.removeIf(record -> record.getTimestamp() < System.currentTimeMillis() - windowSize.getMillis()); } + /** + * Set the top N size for TopQueriesByLatencyService service. + * @param size the top N size to set + */ public void setTopNSize(int size) { this.topNSize = size; } + /** + * Validate the top N size based on the internal constrains + * @param size the wanted top N size + */ public void validateTopNSize(int size) { if (size > QueryInsightsSettings.MAX_N_SIZE) { throw new IllegalArgumentException( @@ -138,14 +152,26 @@ public void validateTopNSize(int size) { } } + /** + * Get the top N size set for TopQueriesByLatencyService + * @return the top N size + */ public int getTopNSize() { return this.topNSize; } + /** + * Set the window size for TopQueriesByLatencyService + * @param windowSize window size to set + */ public void setWindowSize(TimeValue windowSize) { this.windowSize = windowSize; } + /** + * Validate if the window size is valid, based on internal constrains. + * @param windowSize the window size to validate + */ public void validateWindowSize(TimeValue windowSize) { if (windowSize.compareTo(QueryInsightsSettings.MAX_WINDOW_SIZE) > 0) { throw new IllegalArgumentException( @@ -163,10 +189,18 @@ public void validateWindowSize(TimeValue windowSize) { } } + /** + * Get the window size set for TopQueriesByLatencyService + * @return the window size + */ public TimeValue getWindowSize() { return this.windowSize; } + /** + * Set the exporter type to export data generated in TopQueriesByLatencyService + * @param type The type of exporter, defined in {@link QueryInsightsExporterType} + */ public void setExporterType(QueryInsightsExporterType type) { resetExporter( getEnableExport(), @@ -175,6 +209,10 @@ public void setExporterType(QueryInsightsExporterType type) { ); } + /** + * Set if the exporter is enabled + * @param enabled if the exporter is enabled + */ public void setExporterEnabled(boolean enabled) { super.setEnableExport(enabled); resetExporter( @@ -184,6 +222,12 @@ public void setExporterEnabled(boolean enabled) { ); } + /** + * Set the identifier of this exporter, which will be used when exporting the data + * + * For example, for local index exporter, this identifier would be used to define the index name + * @param identifier the identifier for the exporter + */ public void setExporterIdentifier(String identifier) { resetExporter( getEnableExport(), @@ -192,6 +236,10 @@ public void setExporterIdentifier(String identifier) { ); } + /** + * Set the export interval for the exporter + * @param interval export interval + */ public void setExportInterval(TimeValue interval) { super.setExportInterval(interval); resetExporter( @@ -201,6 +249,10 @@ public void setExportInterval(TimeValue interval) { ); } + /** + * Validate if the export interval is valid, based on internal constrains. + * @param exportInterval the export interval to validate + */ public void validateExportInterval(TimeValue exportInterval) { if (exportInterval.getSeconds() < MIN_EXPORT_INTERVAL.getSeconds()) { throw new IllegalArgumentException( @@ -219,6 +271,15 @@ public void validateExportInterval(TimeValue exportInterval) { } } + /** + * Reset the exporter with new config + * + * This function can be used to enable/disable an exporter, change the type of the exporter, + * or change the identifier of the exporter. + * @param enabled the enable flag to set on the exporter + * @param type The QueryInsightsExporterType to set on the exporter + * @param identifier the Identifier to set on the exporter + */ public void resetExporter(boolean enabled, QueryInsightsExporterType type, String identifier) { this.stop(); this.exporter = null; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java index 1bb250e5d57b7..138d23a365de3 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueries.java @@ -32,11 +32,21 @@ public class TopQueries extends BaseNodeResponse implements ToXContentObject { @Nullable private final List latencyRecords; + /** + * Create the TopQueries Object from StreamInput + * @param in A {@link StreamInput} object. + * @throws IOException IOException + */ public TopQueries(StreamInput in) throws IOException { super(in); latencyRecords = in.readList(SearchQueryLatencyRecord::new); } + /** + * Create the TopQueries Object + * @param node A node that is part of the cluster. + * @param latencyRecords The top queries by latency records stored on this node + */ public TopQueries(DiscoveryNode node, @Nullable List latencyRecords) { super(node); this.latencyRecords = latencyRecords; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesAction.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesAction.java index 69abf001d18d9..13dd198681ca8 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesAction.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesAction.java @@ -17,7 +17,13 @@ */ public class TopQueriesAction extends ActionType { + /** + * The TopQueriesAction Instance. + */ public static final TopQueriesAction INSTANCE = new TopQueriesAction(); + /** + * The name of this Action + */ public static final String NAME = "cluster:monitor/insights/top_queries"; private TopQueriesAction() { diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java index 435f5188a4e0b..a671e3a7fe176 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesRequest.java @@ -30,9 +30,9 @@ public class TopQueriesRequest extends BaseNodesRequest { Metric metricType = Metric.LATENCY; /** - * Create a new TopQueriesRequest from a {@link StreamInput} object. + * Constructor for TopQueriesRequest * - * @param in A stream input object. + * @param in A {@link StreamInput} object. * @throws IOException if the stream cannot be deserialized. */ public TopQueriesRequest(StreamInput in) throws IOException { @@ -43,6 +43,8 @@ public TopQueriesRequest(StreamInput in) throws IOException { /** * Get top queries from nodes based on the nodes ids specified. * If none are passed, cluster level top queries will be returned. + * + * @param nodesIds the nodeIds specified in the request */ public TopQueriesRequest(String... nodesIds) { super(nodesIds); @@ -56,7 +58,10 @@ public Metric getMetricType() { } /** - * Set the type of requested metrics + * Validate and set the metric type of requested metrics + * + * @param metricType the metric type to set to + * @return The current TopQueriesRequest */ public TopQueriesRequest setMetricType(String metricType) { metricType = metricType.toUpperCase(Locale.ROOT); @@ -77,6 +82,9 @@ public void writeTo(StreamOutput out) throws IOException { * ALl supported metrics for top queries */ public enum Metric { + /** + * Latency metric type, used by top queries by latency + */ LATENCY("LATENCY"); private final String metricName; @@ -85,10 +93,18 @@ public enum Metric { this.metricName = name; } + /** + * Get the metric name of the Metric + * @return the metric name + */ public String metricName() { return this.metricName; } + /** + * Get all valid metrics + * @return A set of String that contains all valid metrics + */ public static Set allMetrics() { return Arrays.stream(values()).map(Metric::metricName).collect(Collectors.toSet()); } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java index 17b03aa0957fc..b9afd2898ab07 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/action/top_queries/TopQueriesResponse.java @@ -36,11 +36,25 @@ public class TopQueriesResponse extends BaseNodesResponse implements private static final String CLUSTER_LEVEL_RESULTS_KEY = "top_queries"; private final int top_n_size; + /** + * Constructor for TopQueriesResponse. + * + * @param in A {@link StreamInput} object. + * @throws IOException if the stream cannot be deserialized. + */ public TopQueriesResponse(StreamInput in) throws IOException { super(in); top_n_size = in.readInt(); } + /** + * Constructor for TopQueriesResponse + * + * @param clusterName The current cluster name + * @param nodes A list that contains top queries results from all nodes + * @param failures A list that contains FailedNodeException + * @param top_n_size The top N size to return to the user + */ public TopQueriesResponse(ClusterName clusterName, List nodes, List failures, int top_n_size) { super(clusterName, nodes, failures); this.top_n_size = top_n_size; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryLatencyRecord.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryLatencyRecord.java index 6e978489b9f15..97b423e092673 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryLatencyRecord.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryLatencyRecord.java @@ -29,12 +29,36 @@ public final class SearchQueryLatencyRecord extends SearchQueryRecord { // latency info for each search phase private final Map phaseLatencyMap; + /** + * Constructor for SearchQueryLatencyRecord + * + * @param in A {@link StreamInput} object. + * @throws IOException if the stream cannot be deserialized. + */ public SearchQueryLatencyRecord(final StreamInput in) throws IOException { super(in); this.phaseLatencyMap = in.readMap(StreamInput::readString, StreamInput::readLong); this.setValue(in.readLong()); } + @Override + protected void addCustomXContent(XContentBuilder builder, Params params) throws IOException { + builder.field(PHASE_LATENCY_MAP, this.getPhaseLatencyMap()); + builder.field(TOOK, this.getValue()); + } + + /** + * Constructor of the SearchQueryLatencyRecord + * + * @param timestamp The timestamp of the query. + * @param searchType The manner at which the search operation is executed. see {@link SearchType} + * @param source The search source that was executed by the query. + * @param totalShards Total number of shards as part of the search query across all indices + * @param indices The indices involved in the search query + * @param propertyMap Extra attributes and information about a search query + * @param phaseLatencyMap A map contains per-phase latency data + * @param tookInNanos Total time took to finish this request + */ public SearchQueryLatencyRecord( final Long timestamp, final SearchType searchType, @@ -49,19 +73,15 @@ public SearchQueryLatencyRecord( this.phaseLatencyMap = phaseLatencyMap; } + /** + * Get the phase level latency map of this request record + * + * @return Map contains per-phase latency of this request record + */ public Map getPhaseLatencyMap() { return phaseLatencyMap; } - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(); - super.toXContent(builder, params); - builder.field(PHASE_LATENCY_MAP, this.getPhaseLatencyMap()); - builder.field(TOOK, this.getValue()); - return builder.endObject(); - } - @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); @@ -69,6 +89,11 @@ public void writeTo(StreamOutput out) throws IOException { out.writeLong(getValue()); } + /** + * Compare if two SearchQueryLatencyRecord are equal + * @param other The Other SearchQueryLatencyRecord to compare to + * @return boolean + */ public boolean equals(SearchQueryLatencyRecord other) { if (!super.equals(other)) { return false; diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java index bb6b2ae23e01e..f086a9e38c4cd 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java @@ -34,13 +34,13 @@ public abstract class SearchQueryRecord> ToXContentObject { private static final Logger log = LogManager.getLogger(SearchQueryRecord.class); - protected static final String TIMESTAMP = "timestamp"; - protected static final String SEARCH_TYPE = "searchType"; - protected static final String SOURCE = "source"; - protected static final String TOTAL_SHARDS = "totalShards"; - protected static final String INDICES = "indices"; - protected static final String PROPERTY_MAP = "propertyMap"; - protected static final String VALUE = "value"; + private static final String TIMESTAMP = "timestamp"; + private static final String SEARCH_TYPE = "searchType"; + private static final String SOURCE = "source"; + private static final String TOTAL_SHARDS = "totalShards"; + private static final String INDICES = "indices"; + private static final String PROPERTY_MAP = "propertyMap"; + private static final String VALUE = "value"; private final Long timestamp; @@ -57,6 +57,13 @@ public abstract class SearchQueryRecord> private T value; + /** + * Constructor of the SearchQueryRecord + * + * @param in A {@link StreamInput} object. + * @throws IOException if the stream cannot be deserialized. + * @throws ClassCastException ClassCastException + */ public SearchQueryRecord(final StreamInput in) throws IOException, ClassCastException { this.timestamp = in.readLong(); this.searchType = SearchType.fromString(in.readString().toLowerCase(Locale.ROOT)); @@ -66,6 +73,17 @@ public SearchQueryRecord(final StreamInput in) throws IOException, ClassCastExce this.propertyMap = in.readMap(); } + /** + * Constructor of the SearchQueryRecord + * + * @param timestamp The timestamp of the query. + * @param searchType The manner at which the search operation is executed. see {@link SearchType} + * @param source The search source that was executed by the query. + * @param totalShards Total number of shards as part of the search query across all indices + * @param indices The indices involved in the search query + * @param propertyMap Extra attributes and information about a search query + * @param value The value on this SearchQueryRecord + */ public SearchQueryRecord( final Long timestamp, final SearchType searchType, @@ -79,6 +97,16 @@ public SearchQueryRecord( this.value = value; } + /** + * Constructor of the SearchQueryRecord + * + * @param timestamp The timestamp of the query. + * @param searchType The manner at which the search operation is executed. see {@link SearchType} + * @param source The search source that was executed by the query. + * @param totalShards Total number of shards as part of the search query across all indices + * @param indices The indices involved in the search query + * @param propertyMap Extra attributes and information about a search query + */ public SearchQueryRecord( final Long timestamp, final SearchType searchType, @@ -139,6 +167,7 @@ public T getValue() { /** * Set the value of the query metric record + * @param value The value to set on the record */ public void setValue(T value) { this.value = value; @@ -156,14 +185,19 @@ public int compareTo(SearchQueryRecord otherRecord) { return value.compareTo(otherRecord.getValue()); } + /** + * Compare if two SearchQueryRecord are equal + * @param other The Other SearchQueryRecord to compare to + * @return boolean + */ public boolean equals(SearchQueryRecord other) { - if (false == this.timestamp.equals(other.getTimestamp()) - && this.searchType.equals(other.getSearchType()) - && this.source.equals(other.getSource()) - && this.totalShards == other.getTotalShards() - && this.indices.length == other.getIndices().length - && this.propertyMap.size() == other.getPropertyMap().size() - && this.value.equals(other.getValue())) { + if (!this.timestamp.equals(other.getTimestamp()) + || !this.searchType.equals(other.getSearchType()) + || !this.source.equals(other.getSource()) + || this.totalShards != other.getTotalShards() + || this.indices.length != other.getIndices().length + || this.propertyMap.size() != other.getPropertyMap().size() + || !this.value.equals(other.getValue())) { return false; } for (int i = 0; i < indices.length; i++) { @@ -192,15 +226,25 @@ private T castToValue(Object obj) throws ClassCastException { } } + /** + * Add custom XContent fields to the record + * @param builder XContent builder + * @param params XContent parameters + * @throws IOException IOException + */ + protected abstract void addCustomXContent(XContentBuilder builder, Params params) throws IOException; + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); builder.field(TIMESTAMP, timestamp); builder.field(SEARCH_TYPE, searchType); builder.field(SOURCE, source); builder.field(TOTAL_SHARDS, totalShards); builder.field(INDICES, indices); builder.field(PROPERTY_MAP, propertyMap); - return builder; + addCustomXContent(builder, params); + return builder.endObject(); } @Override diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java index ca540f295a3f5..9c0271842372d 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java @@ -39,6 +39,9 @@ public class RestTopQueriesAction extends BaseRestHandler { /** The metric types that are allowed in top N queries */ static final Set ALLOWED_METRICS = TopQueriesRequest.Metric.allMetrics(); + /** + * Constructor for RestTopQueriesAction + */ public RestTopQueriesAction() {} @Override @@ -51,7 +54,7 @@ public List routes() { @Override public String getName() { - return "top_queries_action"; + return "query_insights_top_queries_action"; } @Override diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java index fa7996cc6f3f7..66ba350f281b2 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java @@ -43,6 +43,15 @@ public class TransportTopQueriesAction extends TransportNodesAction< private final TopQueriesByLatencyService topQueriesByLatencyService; + /** + * Create the TransportTopQueriesAction Object + + * @param threadPool The OpenSearch thread pool to run async tasks + * @param clusterService The clusterService of this node + * @param transportService The TransportService of this node + * @param topQueriesByLatencyService The topQueriesByLatencyService associated with this Transport Action + * @param actionFilters the action filters + */ @Inject public TransportTopQueriesAction( ThreadPool threadPool, @@ -113,11 +122,20 @@ public static class NodeRequest extends TransportRequest { TopQueriesRequest request; + /** + * Create the NodeResponse object from StreamInput + * @param in the StreamInput to read the object + * @throws IOException IOException + */ public NodeRequest(StreamInput in) throws IOException { super(in); request = new TopQueriesRequest(in); } + /** + * Create the NodeResponse object from a TopQueriesRequest + * @param request the TopQueriesRequest object + */ public NodeRequest(TopQueriesRequest request) { this.request = request; } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryInsightsSettings.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryInsightsSettings.java index a741daff76516..818959bf7f12a 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryInsightsSettings.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryInsightsSettings.java @@ -26,8 +26,11 @@ public class QueryInsightsSettings { * Default Values and Settings */ public static final TimeValue MAX_WINDOW_SIZE = new TimeValue(21600, TimeUnit.SECONDS); + /** Default N size for top N queries */ public static final int MAX_N_SIZE = 100; - public static final TimeValue MIN_EXPORT_INTERVAL = new TimeValue(21600, TimeUnit.SECONDS); + /** Default min export interval for Query Insights exporters */ + public static final TimeValue MIN_EXPORT_INTERVAL = new TimeValue(1, TimeUnit.SECONDS); + /** Default local index mapping for top n queries records */ public static final String DEFAULT_LOCAL_INDEX_MAPPING = "mappings/top_n_queries_record.json"; /** Default window size in seconds to keep the top N queries with latency data in query insight store */ public static final int DEFAULT_WINDOW_SIZE = 60; @@ -43,8 +46,9 @@ public class QueryInsightsSettings { * */ public static final String TOP_QUERIES_BASE_URI = PLUGINS_BASE_URI + "/top_queries"; + /** Default prefix for top N queries feature */ public static final String TOP_N_QUERIES_SETTING_PREFIX = "search.top_n_queries"; - + /** Default prefix for top N queries by latency feature */ public static final String TOP_N_LATENCY_QUERIES_PREFIX = TOP_N_QUERIES_SETTING_PREFIX + ".latency"; /** * Boolean setting for enabling top queries by latency. diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterTests.java index da7857fe3214d..64d296e501519 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterTests.java @@ -20,20 +20,16 @@ public class QueryInsightsExporterTests extends OpenSearchTestCase { public class DummyExporter> extends QueryInsightsExporter { DummyExporter(String identifier) { - super(QueryInsightsExporterType.LOCAL_INDEX, identifier); + super(identifier); } @Override public void export(List records) {} } - public void testSetAndGetType() { + public void testGetType() { DummyExporter exporter = new DummyExporter<>("test-index"); - exporter.setType(QueryInsightsExporterType.LOCAL_INDEX); - exporter.setIdentifier("test-index"); - QueryInsightsExporterType type = exporter.getType(); String identifier = exporter.getIdentifier(); - assertEquals(QueryInsightsExporterType.LOCAL_INDEX, type); assertEquals("test-index", identifier); } }