feat(tools/mysql): add list-table-stats-tool to list table statistics in MySQL and Cloud SQL MySQL source.#2938
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 the mysql-list-table-stats tool, which provides detailed MySQL table-level statistics including size, latency, and CRUD activity. The implementation includes the tool logic, configuration for Cloud SQL and standard MySQL, documentation, and integration tests. Several issues were identified in the review: a fragile and redundant schema validation block in the tool's Invoke method should be removed, documentation headings and titles must be updated to comply with the repository's strict style guide, and the limit parameter should be capped to enforce the documented maximum of 1000. Additionally, inconsistent indentation in the Go source and a copy-pasted test function name should be corrected.
| connected_schema := os.Getenv("MYSQL_DATABASE") | ||
| if table_schema != connected_schema && table_schema != "" { | ||
| err := fmt.Errorf("error: connected schema '%s' does not match queried schema '%s'", connected_schema, table_schema) | ||
| return nil, util.NewClientServerError("error getting logger", http.StatusInternalServerError, err) | ||
| } | ||
|
|
||
| return resp, nil |
There was a problem hiding this comment.
This validation block should be removed. It relies on a hardcoded environment variable MYSQL_DATABASE which is fragile (e.g., it differs for Cloud SQL sources) and it incorrectly blocks querying other schemas even if the user has permissions, contradicting the tool's description. Furthermore, the error message on line 174 is a copy-paste error ("error getting logger"). The query already filters by table_schema if provided, making this extra check redundant. If you remove this, also remove the unused os import at line 22.
References
- Automated analysis may incorrectly report discrepancies between documentation and code; always verify its findings before acting on them.
- Assume that data extracted from a request into a structured object like baseMessage has already been validated, and does not need re-validation before being used in metrics.
| @@ -0,0 +1,75 @@ | |||
| "--- | |||
| title: ""mysql-list-table-stats"" | |||
There was a problem hiding this comment.
The tool page title must follow the repository's naming convention and end with "Tool". Also, avoid using double double quotes in the frontmatter.
| title: ""mysql-list-table-stats"" | |
| title: "MySQL List Table Stats Tool" |
References
- The YAML frontmatter title must always end with 'Tool' (e.g., title: 'Execute SQL Tool'). (link)
| ## Example | ||
|
|
||
| ```yaml | ||
| kind: tools | ||
| name: list_table_stats | ||
| type: mysql-list-table-stats | ||
| source: my-mysql-instance | ||
| description: Display table statistics including table size, total latency, rows read, rows written, read and write latency for entire instance, a specified database, or a specified table. Specifying a database name or table name filters the output to that specific db or table. Results are limited to 10 by default, with a maximum allowable limit of 1000. | ||
| ``` | ||
| The response is a json array with the following fields: | ||
| ```json | ||
| [ | ||
| { | ||
| ""table_schema"": ""The schema/database this table belongs to"", | ||
| ""table_name"": ""Name of this table"", | ||
| ""size_MB"": ""Size of the table data in MB"", | ||
| ""row_count"": ""Number of rows in the table"", | ||
| ""total_latency_secs"": ""total latency in secs"", | ||
| ""rows_fetched"": ""total number of rows fetched"", | ||
| ""rows_inserted"": ""total number of rows inserted"", | ||
| ""rows_updated"": ""total number of rows updated"", | ||
| ""rows_deleted"": ""total number of rows deleted"", | ||
| ""io_reads"": ""total number of io read requests"", | ||
| ""io_read_latency"": ""io read latency in seconds"", | ||
| ""io_write_latency"": ""io write latency in seconds"", | ||
| ""io_misc_latency"": ""io misc latency in seconds"", | ||
| } | ||
| ] | ||
| ``` | ||
|
|
||
| ## prerequisite | ||
|
|
||
| - `performance_schema` should be turned ON for this tool to work. | ||
|
|
||
| ## Reference | ||
|
|
||
| | **field** | **type** | **required** | **description** | | ||
| |-------------|:--------:|:------------:|----------------------------------------------------| | ||
| | type | string | true | Must be ""mysql-list-table-stats"". | | ||
| | source | string | true | Name of the source the SQL should execute on. | | ||
| | description | string | true | Description of the tool that is passed to the LLM. | | ||
|
|
||
|
|
||
| ## Use Cases | ||
|
|
||
| - **Finding hottest tables**: Identify tables with highest total latency, read or writes based on the `sort_by` column. | ||
| - **Finding tables with most reads**: Identify tables with highest reads by sorting on `rows_fetched`. | ||
| - **Monitoring growth**: Track `row_count` and `size_MB` of table over time to estimate growth." No newline at end of file |
There was a problem hiding this comment.
The documentation headings must follow the strict sequence and naming defined in the repository style guide. Specifically:
## prerequisiteshould be renamed to## Requirementsand placed before## Example.## Use Casesis not a standard heading and its content should be integrated into## Aboutor## Advanced Usage.- The JSON response description should ideally be under a
## Output Formatheading.
References
- Strict H2 Ordering: You must use the following H2 (##) headings in this exact sequence: About, Compatible Sources, Requirements, Parameters, Example, Output Format, Reference, Advanced Usage, Troubleshooting, Additional Resources. (link)
| func (t Tool) Invoke(ctx context.Context, resourceMgr tools.SourceProvider, params parameters.ParamValues, accessToken tools.AccessToken) (any, util.ToolboxError) { | ||
| source, err := tools.GetCompatibleSource[compatibleSource](resourceMgr, t.Source, t.Name, t.Type) | ||
| if err != nil { | ||
| return nil, util.NewClientServerError("source used is not compatible with the tool", http.StatusInternalServerError, err) | ||
| } | ||
|
|
||
| paramsMap := params.AsMap() | ||
|
|
||
| table_schema, ok := paramsMap["table_schema"].(string) | ||
| if !ok { | ||
| return nil, util.NewAgentError("invalid 'table_schema' parameter; expected a string", nil) | ||
| } | ||
| table_name, ok := paramsMap["table_name"].(string) | ||
| if !ok { | ||
| return nil, util.NewAgentError("invalid 'table_name' parameter; expected a string", nil) | ||
| } | ||
| sort_by, ok := paramsMap["sort_by"].(string) | ||
| if !ok { | ||
| return nil, util.NewAgentError("invalid 'sort_by' parameter; expected a string", nil) | ||
| } | ||
| limit, ok := paramsMap["limit"].(int) | ||
| if !ok { | ||
| return nil, util.NewAgentError("invalid 'limit' parameter; expected an integer", nil) | ||
| } | ||
|
|
||
| // Log the query executed for debugging. | ||
| logger, err := util.LoggerFromContext(ctx) | ||
| if err != nil { | ||
| return nil, util.NewClientServerError("error getting logger", http.StatusInternalServerError, err) | ||
| } | ||
| logger.DebugContext(ctx, fmt.Sprintf("executing `%s` tool query: %s", resourceType, listTableStatsStatement)) | ||
| sliceParams := []any{table_schema, table_schema, table_name, table_name, sort_by, sort_by, sort_by, sort_by, sort_by, limit} | ||
| resp, err := source.RunSQL(ctx, listTableStatsStatement, sliceParams) | ||
| if err != nil { | ||
| return nil, util.ProcessGeneralError(err) | ||
| } | ||
| connected_schema := os.Getenv("MYSQL_DATABASE") | ||
| if table_schema != connected_schema && table_schema != "" { | ||
| err := fmt.Errorf("error: connected schema '%s' does not match queried schema '%s'", connected_schema, table_schema) | ||
| return nil, util.NewClientServerError("error getting logger", http.StatusInternalServerError, err) | ||
| } | ||
|
|
||
| return resp, nil | ||
| } |
There was a problem hiding this comment.
The file has inconsistent indentation, mixing tabs and spaces. Additionally, the return statements inside if !ok blocks are using excessive indentation (3 tabs instead of 2).
| 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.
The documentation mentions a maximum allowable limit of 1000, but the code does not enforce this. It's recommended to cap the limit parameter to prevent performance issues with very large result sets.
| "github.com/googleapis/genai-toolbox/internal/tools/mysql/mysqllisttablestats" | ||
| ) | ||
|
|
||
| func TestParseFromYamlExecuteSql(t *testing.T) { |
There was a problem hiding this comment.
The test function name TestParseFromYamlExecuteSql appears to be a copy-paste error. It should be updated to reflect the tool being tested.
References
- Automated analysis may incorrectly report discrepancies between documentation and code; always verify its findings before acting on them.
| name: "invoke list_table_stats with schema other than connected to, expected log error and nil results", | ||
| requestBody: bytes.NewBufferString(fmt.Sprintf(`{"table_schema": "%s"}`, "testdb")), | ||
| wantStatusCode: http.StatusInternalServerError, | ||
| want: []tableStatsDetails(nil), | ||
| }, |
There was a problem hiding this comment.
This test case expects a failure based on the buggy schema validation in the tool's Invoke method. Once that validation is removed, this test case should be updated to expect success (e.g., http.StatusOK and an empty result if the schema doesn't exist) or removed.
References
- Automated analysis may incorrectly report discrepancies between documentation and code; always verify its findings before acting on them.
| ` | ||
|
|
||
| func init() { | ||
| if !tools.Register(resourceType, newConfig) { |
There was a problem hiding this comment.
You want to fix the indentation
| type: docs | ||
| weight: 1 | ||
| description: > | ||
| A ""mysql-list-table-stats"" tool report table statistics including table size, total latency, rows read, rows written, read and write latency for entire instance, a specified database, or a specified table. |
There was a problem hiding this comment.
AI suggested:
Provides a detailed breakdown of MySQL table resource usage (size, latency, and I/O) with optional filtering by database or table name.
| A `mysql-list-table-stats` tool provides a table level statistics by checking table size, estimated rows count, total latency, rows fetched, rows inserted, rows updated, rows deleted, number of IO reads and IO latency, number of IO writes and IO write latency, number of IO misc operations and IO misc latency, IO latency of reads and writes on the table. | ||
|
|
||
| `mysql-list-table-stats` outputs detailed overview of table level resource consumption including estimated row counts, table size, a complete breakdown of CRUD activity (rows fetched, inserted, updated, and deleted) and table level IO statistics such as total latency, read latency, write latency and misc latency. The output is a JSON formatted array of the top 10 MySQL tables ranked by total latency. | ||
|
|
There was a problem hiding this comment.
Just a personal feeling, that these 2 paragraphs above repeat each other a lot.
The mysql-list-table-stats tool provides a comprehensive overview of table-level resource consumption and performance metrics. It captures critical data including table size, estimated row counts, and a complete breakdown of CRUD activity (rows fetched, inserted, updated, and deleted)
And then the 2nd graph can enumerate all the field one by one if you'd like to.
| name: list_table_stats | ||
| type: mysql-list-table-stats | ||
| source: my-mysql-instance | ||
| description: Display table statistics including table size, total latency, rows read, rows written, read and write latency for entire instance, a specified database, or a specified table. Specifying a database name or table name filters the output to that specific db or table. Results are limited to 10 by default, with a maximum allowable limit of 1000. |
There was a problem hiding this comment.
Is 1000 the tool default?
| - get_query_metrics | ||
| - get_system_metrics | ||
| - list_table_fragmentation | ||
| - list_table_stats |
There was a problem hiding this comment.
I think the convention here is to order them alphabetically.
| ` | ||
|
|
||
| func init() { | ||
| if !tools.Register(resourceType, newConfig) { |
There was a problem hiding this comment.
You want to double check the indentation across this PR.
| executed. Check all tables visible to the current database if not specified. | ||
| - `table_name` (optional): Name of the table to be checked. Check all tables | ||
| visible to the current user if not specified. | ||
| - `sort_by` (optional): The column to sort by. Valid values are `row_count`, `rows_fetched`, `rows_inserted`, `rows_updated`, `rows_deleted`, `total_latency_secs` (defaults to `total_latency_secs`) |
There was a problem hiding this comment.
you want to mention the default (total_latency)
| parameters.NewStringParameterWithDefault("table_schema", "", "(Optional) The database where statistics is to be executed. Check all tables visible to the current user if not specified"), | ||
| parameters.NewStringParameterWithDefault("table_name", "", "(Optional) Name of the table to be checked. Check all tables visible to the current user if not specified."), | ||
| parameters.NewStringParameterWithDefault("sort_by", "", "(Optional) The column to sort by"), | ||
| parameters.NewIntParameterWithDefault("limit", 10, "(Optional) Max rows to return, default is 10"), |
There was a problem hiding this comment.
Not sure if you want to limit it to 1000 as the .md file declares. Please cross check with the tool default.
| if err != nil { | ||
| return nil, util.ProcessGeneralError(err) | ||
| } | ||
| connected_schema := os.Getenv("MYSQL_DATABASE") |
There was a problem hiding this comment.
I think this check should be done before running the actual SQL
| connected_schema := os.Getenv("MYSQL_DATABASE") | ||
| if table_schema != connected_schema && table_schema != "" { | ||
| err := fmt.Errorf("error: connected schema '%s' does not match queried schema '%s'", connected_schema, table_schema) | ||
| return nil, util.NewClientServerError("error getting logger", http.StatusInternalServerError, err) |
There was a problem hiding this comment.
It is not error getting logger, probably a copy-paste error
3 participants
Description
Add the list_table_stats tool to MySQL to list table statistics.
The tool display table level statistics for troubleshooting performance issues, finding hottest tables and monitoring table growth.
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 #2484