feat(tools/mysql): add show-query-stats and list-all-locks tools for MySQL and Cloud SQL MySQL source#2954
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces two new MySQL tools, mysql-list-all-locks and mysql-show-query-stats, along with their corresponding documentation and configuration updates. I have identified several issues: the mysql-list-all-locks tool implementation is missing the extraction of the table_name parameter, the JSON examples in the documentation contain invalid trailing commas, and the test file for mysql-list-all-locks incorrectly references the wrong tool type. Please address these bugs to ensure the new tools function correctly and the documentation is valid.
| table_schema, ok := paramsMap["table_schema"].(string) | ||
| if !ok { | ||
| return nil, util.NewAgentError("invalid 'table_schema' parameter; expected a string", nil) | ||
| } | ||
| limit, ok := paramsMap["limit"].(int) | ||
| if !ok { | ||
| return nil, util.NewAgentError("invalid 'limit' parameter; expected an integer", nil) | ||
| } |
There was a problem hiding this comment.
For consistency with the project's coding standards, use direct type assertions for parameter values instead of ok checks.
table_schema := paramsMap["table_schema"].(string)
limit := paramsMap["limit"].(int)References
- Within this project's parameter parsing framework, direct type assertions on parameter values are acceptable without a comma-ok check. The framework's typed constructors (e.g., NewIntParameter) already guarantee type safety, and this is an established pattern for consistency.
| it.TRX_OPERATION_STATE AS 'current_operation', | ||
| substring(it.TRX_QUERY, 1, 100) AS 'query' | ||
| FROM | ||
| performance_schema.data_locks dl |
There was a problem hiding this comment.
this is 8.0+ only, right?
| sum_no_index_used AS 'full_table_scan_count', | ||
| sum_no_good_index_used AS 'inefficient_index_used_count', | ||
| last_seen AS 'last_executed' | ||
| FROM performance_schema.events_statements_summary_by_digest |
There was a problem hiding this comment.
is there a concern that the instance does not have p_s enabled?
| } | ||
|
|
||
| // Generating stats for query | ||
| selectStmt := fmt.Sprintf("SELECT * FROM %s WHERE id = 1 FOR UPDATE", testTableName) |
There was a problem hiding this comment.
is the query going to be "long" enough to facilitate the tests?
Yuan325
added
the
release candidate
3 participants
Description
Add the show-query-stats and list-all-locks tools for MySQL. The tools will help in performance monitoring and troubleshooting.
PR Checklist
CONTRIBUTING.md
bug/issue
before writing your code! That way we can discuss the change, evaluate
designs, and agree on the general idea
!if this involve a breaking change🛠️ Fixes #2953