Skip to content

feat: add search tool for profile and explain #28

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

getsolaris
Copy link
Contributor

@getsolaris getsolaris commented Apr 5, 2025

This change adds profile and explain parameters to the existing search functionality, allowing users to get profiling data and query explanations in a single request.

Changes

  • Modified search tool parameters:
    • Added profile: boolean - Enable query profiling
    • Added explain: boolean - Enable query explanation
  • Deprecated separate profile_query tool

@getsolaris getsolaris requested a review from a team as a code owner April 5, 2025 09:46
@getsolaris
Copy link
Contributor Author

@jedrazb

I noticed that this PR includes code from PR #25 . The authentication validation issue will be resolved once PR #25 is merged into main.

I apologize for not creating a separate branch from the beginning, which led to this dependency. I will be more careful with branch management in future contributions.

Next time, I will make sure to work on separate branches for future contributions.

Thank you for your understanding.

@getsolaris getsolaris changed the title Add Query Profiling Tool for Performance Analysis Add profile_query Tool for Performance Analysis Apr 7, 2025
@getsolaris getsolaris changed the title Add profile_query Tool for Performance Analysis Add profile_query tool for Performance Analysis Apr 7, 2025
Copy link
Member

@jedrazb jedrazb left a comment

Choose a reason for hiding this comment

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

Thank you for your interest in ES mcp server! I left some questions.

Also, please try to only include changes that relate to the issue/PR.

index.ts Outdated
body: {
...queryBody,
profile: true,
explain: explain,
Copy link
Member

Choose a reason for hiding this comment

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

Given that profile/explain is part of search API, isn't it sort of supported by search tool?

Would it make sense to just add new param to search tool e.g. explain: bool that would:

  • set profile and explain to true
  • process the response and include profile info in text fragmetns

Copy link
Contributor Author

@getsolaris getsolaris Apr 7, 2025

Choose a reason for hiding this comment

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

7fe9331

Oh, you're right. I was missing it.

Previously, we had a separate profile_query tool for query profiling, but I've now integrated these features into the search tool. This change adds profile and explain parameters to the existing search functionality, allowing users to get profiling data and query explanations in a single request. The integration includes performance analysis and optimization suggestions

@getsolaris getsolaris requested a review from jedrazb April 7, 2025 12:33
@getsolaris getsolaris changed the title Add profile_query tool for Performance Analysis Add search tool for profile and explain Apr 7, 2025
@getsolaris
Copy link
Contributor Author

@jedrazb please bump version to 0.1.2

@getsolaris getsolaris changed the title Add search tool for profile and explain feat: add search tool for profile and explain Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants