Skip to content
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

Move query categorization changes to plugin #16

Merged

Conversation

deshsidd
Copy link
Collaborator

@deshsidd deshsidd commented Jul 15, 2024

Original PR : opensearch-project/OpenSearch#14528

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 opensearch-project/OpenSearch#14527
Addresses opensearch-project/OpenSearch#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.

@deshsidd deshsidd changed the title [Draft] Move query categorization changes to plugin Move query categorization changes to plugin Jul 15, 2024
@deshsidd deshsidd force-pushed the sid/move-query-categorization branch from 691838d to c332bd9 Compare July 16, 2024 21:44
Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>
Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>
Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>
Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>
Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>
Copy link
Member

@ansjcy ansjcy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for refactoring the enable functions and adding the tests!

@deshsidd deshsidd merged commit 811f4a5 into opensearch-project:main Jul 17, 2024
12 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 17, 2024
* Move query categorization changes to plugin

Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>

* Fix SearchSourceBuilder read/write test failures

Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>

* Fix tests

Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>

* Fix starting and stopping query insights service

Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>

* Unit tests for feature enable/disable and refactor logic

Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>

---------

Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>
(cherry picked from commit 811f4a5)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 17, 2024
* Move query categorization changes to plugin

Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>

* Fix SearchSourceBuilder read/write test failures

Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>

* Fix tests

Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>

* Fix starting and stopping query insights service

Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>

* Unit tests for feature enable/disable and refactor logic

Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>

---------

Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>
(cherry picked from commit 811f4a5)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
deshsidd pushed a commit that referenced this pull request Jul 18, 2024
(cherry picked from commit 811f4a5)

Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
deshsidd pushed a commit that referenced this pull request Jul 18, 2024
(cherry picked from commit 811f4a5)

Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Move query categorization to query insights plugin
2 participants