Move query categorization changes to query insights plugin #14528

deshsidd · 2024-06-24T20:37:24Z

Query categorization changes to increment counters for search query related metrics currently resides on the search path and occurs before the request.

Move these changes to the query insights plugin and make sure the incrementing of counters happens separately from the search path.

Another option is to keep query categorization changes as is. However, this will lead to additional overhead on the search path. Furthermore, we need to tie query latency, cpu, memory with the query categorization data which will only be possible if we increment the counters after the request is completed and the query latency and resource usage data resides inside the plugin.

To support the above and to prevent doing these counter increments on the search path, we need to move query categorization changes to the query insights plugin.

Related Issues

Resolves #14527
Addresses #11596

Check List

Functionality includes testing.
API changes companion pull request created, if applicable.
Public documentation issue/PR created, if applicable.

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.

github-actions · 2024-06-24T20:48:36Z

❌ Gradle check result for 9e5a09a: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2024-06-24T20:52:26Z

❌ Gradle check result for af58c5f: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2024-06-24T21:07:16Z

❌ Gradle check result for 2bd1a05: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2024-06-24T23:27:18Z

❌ Gradle check result for 68a226e: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2024-06-24T23:41:24Z

❌ Gradle check result for 8dc38c4: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2024-06-24T23:50:29Z

❌ Gradle check result for a4ad58e: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2024-06-25T02:57:17Z

❌ Gradle check result for cdce692: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Siddhant Deshmukh <[email protected]>

github-actions · 2024-06-29T02:27:48Z

❌ Gradle check result for dc1fefb: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Siddhant Deshmukh <[email protected]>

github-actions · 2024-07-01T17:42:46Z

❌ Gradle check result for 0906513: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Siddhant Deshmukh <[email protected]>

github-actions · 2024-07-01T18:56:07Z

✅ Gradle check result for be6527b: SUCCESS

codecov · 2024-07-01T18:57:08Z

Codecov Report

Attention: Patch coverage is 67.69231% with 21 lines in your changes missing coverage. Please review.

Project coverage is 71.78%. Comparing base (a34270d) to head (eb8ebd8).
Report is 5 commits behind head on main.

Files	Patch %	Lines
...in/insights/core/service/QueryInsightsService.java	46.66%	7 Missing and 1 partial ⚠️
...nsearch/plugin/insights/rules/model/Attribute.java	73.91%	3 Missing and 3 partials ⚠️
...re/service/categorizer/SearchQueryCategorizer.java	37.50%	5 Missing ⚠️
...categorizer/SearchQueryAggregationCategorizer.java	50.00%	1 Missing ⚠️
...insights/settings/QueryCategorizationSettings.java	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##               main   #14528    +/-   ##
==========================================
  Coverage     71.77%   71.78%            
+ Complexity    62302    62282    -20     
==========================================
  Files          5125     5127     +2     
  Lines        292473   292547    +74     
  Branches      42258    42269    +11     
==========================================
+ Hits         209912   209991    +79     
+ Misses        65324    65213   -111     
- Partials      17237    17343   +106

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2024-07-01T19:07:20Z

❕ Gradle check result for eb8ebd8: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

Signed-off-by: Siddhant Deshmukh <[email protected]>

github-actions · 2024-07-02T20:19:22Z

❌ Gradle check result for d419ca5: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2024-07-02T20:31:28Z

❌ Gradle check result for d6b29c7: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

...sights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java

jainankitk · 2024-07-02T20:58:52Z

...insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java

@@ -69,15 +77,27 @@ public class QueryInsightsService extends AbstractLifecycleComponent {
     */
    final QueryInsightsExporterFactory queryInsightsExporterFactory;

+    private volatile boolean searchQueryMetricsEnabled;


Why do you need a volatile boolean? Does the eventual consistency not work here?

If searchQueryMetricsEnabled is not declared as volatile, changes made by one thread to searchQueryMetricsEnabled may not be immediately visible to other threads and could cause some issues.

...insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java

jainankitk · 2024-07-02T21:03:59Z

...src/main/java/org/opensearch/plugin/insights/core/service/categorizer/QueryShapeVisitor.java

@@ -6,10 +6,12 @@
 * compatible open source license.
 */

-package org.opensearch.index.query;
+package org.opensearch.plugin.insights.core.service.categorizer;


nit: Should we really have service in the namespace?

Had added categorizer in core earlier but @ansjcy mentioned that it might make more sense to keep it under service. I am okay either way.

...ain/java/org/opensearch/plugin/insights/core/service/categorizer/SearchQueryCategorizer.java

jainankitk · 2024-07-02T21:18:05Z

...ghts/src/main/java/org/opensearch/plugin/insights/core/service/categorizer/package-info.java

+/**
+ * Query Insights search query categorizor to collect metrics related to search queries
+ */
+package org.opensearch.plugin.insights.core.service.categorizer;


Most of the QueryCategorizer logic is independent of QueryInsightsService. Hence, categorizer should not be contained within service IMO.

Signed-off-by: Siddhant Deshmukh <[email protected]>

github-actions · 2024-07-04T01:08:27Z

❌ Gradle check result for 9b80e90: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

jainankitk · 2024-07-16T22:52:04Z

@deshsidd - This PR can be discarded in favor of opensearch-project/query-insights#16, right?

deshsidd · 2024-07-16T23:14:02Z

Closing in favor of : https://github.com/opensearch-project/OpenSearch/pull/14528/files

deshsidd requested review from anasalkouz, andrross, Bukhtawar, CEHENKLE, dblock, dbwiddis, gbbafna, kotwanikunal, mch2, msfroh, nknize, owaiskazi19, reta, Rishikesh1159, sachinpkale, saratvemulapalli, shwetathareja, sohami and VachaShah as code owners June 24, 2024 20:37

deshsidd added the backport 2.x Backport to 2.x branch label Jun 24, 2024

deshsidd self-assigned this Jun 24, 2024

github-actions bot added enhancement Enhancement or improvement to existing feature or request Search:Query Insights labels Jun 24, 2024

deshsidd added 5 commits June 28, 2024 19:10

Javadoc

5c0e83e

Signed-off-by: Siddhant Deshmukh <[email protected]>

Add setting

8ff9e21

Signed-off-by: Siddhant Deshmukh <[email protected]>

Address review comments

ac7fd90

Signed-off-by: Siddhant Deshmukh <[email protected]>

Add categorization to the enabled check for insights service

32ab63a

Signed-off-by: Siddhant Deshmukh <[email protected]>

Spotless apply

dc1fefb

Signed-off-by: Siddhant Deshmukh <[email protected]>

deshsidd force-pushed the sid/query-cat-latency branch from 827085e to dc1fefb Compare June 29, 2024 02:11

Fix settings test

0906513

Signed-off-by: Siddhant Deshmukh <[email protected]>

Javadoc

be6527b

Signed-off-by: Siddhant Deshmukh <[email protected]>

deshsidd requested a review from jainankitk July 1, 2024 17:50

Java doc

eb8ebd8

Signed-off-by: Siddhant Deshmukh <[email protected]>

deshsidd added 2 commits July 2, 2024 12:56

Add unit test and change serialization

d419ca5

Signed-off-by: Siddhant Deshmukh <[email protected]>

Javadoc

d6b29c7

Signed-off-by: Siddhant Deshmukh <[email protected]>

jainankitk reviewed Jul 2, 2024

View reviewed changes

deshsidd added 2 commits July 3, 2024 16:38

Fix entrypoint check

ead3ae2

Signed-off-by: Siddhant Deshmukh <[email protected]>

Address review comments

9b80e90

Signed-off-by: Siddhant Deshmukh <[email protected]>

deshsidd mentioned this pull request Jul 16, 2024

Move query categorization changes to plugin opensearch-project/query-insights#16

Merged

deshsidd closed this Jul 16, 2024

This was referenced Sep 11, 2024

[AUTOCUT] Gradle Check Flaky Test Report for MinimumClusterManagerNodesIT #14289

Open

[AUTOCUT] Gradle Check Flaky Test Report for AzureBlobContainerRetriesTests #14318

Open

opensearch-ci-bot mentioned this pull request Oct 28, 2024

[AUTOCUT] Gradle Check Flaky Test Report for QueryInsightsServiceTests #16512

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move query categorization changes to query insights plugin #14528

Move query categorization changes to query insights plugin #14528

deshsidd commented Jun 24, 2024 •

edited

Loading

github-actions bot commented Jun 24, 2024

github-actions bot commented Jun 24, 2024

github-actions bot commented Jun 24, 2024

github-actions bot commented Jun 24, 2024

github-actions bot commented Jun 24, 2024

github-actions bot commented Jun 24, 2024

github-actions bot commented Jun 25, 2024

github-actions bot commented Jun 29, 2024

github-actions bot commented Jul 1, 2024

github-actions bot commented Jul 1, 2024

codecov bot commented Jul 1, 2024 •

edited

Loading

github-actions bot commented Jul 1, 2024

github-actions bot commented Jul 2, 2024

github-actions bot commented Jul 2, 2024

jainankitk Jul 2, 2024

deshsidd Jul 4, 2024

jainankitk Jul 2, 2024

deshsidd Jul 4, 2024

jainankitk Jul 2, 2024

github-actions bot commented Jul 4, 2024

jainankitk commented Jul 16, 2024

deshsidd commented Jul 16, 2024

Move query categorization changes to query insights plugin #14528

Move query categorization changes to query insights plugin #14528

Conversation

deshsidd commented Jun 24, 2024 • edited Loading

Related Issues

Check List

github-actions bot commented Jun 24, 2024

github-actions bot commented Jun 24, 2024

github-actions bot commented Jun 24, 2024

github-actions bot commented Jun 24, 2024

github-actions bot commented Jun 24, 2024

github-actions bot commented Jun 24, 2024

github-actions bot commented Jun 25, 2024

github-actions bot commented Jun 29, 2024

github-actions bot commented Jul 1, 2024

github-actions bot commented Jul 1, 2024

codecov bot commented Jul 1, 2024 • edited Loading

Codecov Report

github-actions bot commented Jul 1, 2024

github-actions bot commented Jul 2, 2024

github-actions bot commented Jul 2, 2024

jainankitk Jul 2, 2024

Choose a reason for hiding this comment

deshsidd Jul 4, 2024

Choose a reason for hiding this comment

jainankitk Jul 2, 2024

Choose a reason for hiding this comment

deshsidd Jul 4, 2024

Choose a reason for hiding this comment

jainankitk Jul 2, 2024

Choose a reason for hiding this comment

github-actions bot commented Jul 4, 2024

jainankitk commented Jul 16, 2024

deshsidd commented Jul 16, 2024

deshsidd commented Jun 24, 2024 •

edited

Loading

codecov bot commented Jul 1, 2024 •

edited

Loading