Conversation
- Implement GraphiteClient for handling queries to a Graphite datasource via Grafana proxy. - Add functions for querying metrics, listing metrics, and retrieving tags. - Enhance error handling and provide hints for empty results specific to Graphite. - Include unit tests for Graphite functionalities to ensure reliability.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before autofix could start.
Reviewed by Cursor Bugbot for commit 70430c8. Configure here.
| // Primary path: fetch summarize(isNonNull(<target>), <bucket>, "sum") to | ||
| // obtain a compact per-bucket count of non-null points for each series. | ||
| // This avoids downloading full-resolution data for long windows. | ||
| bucketSize := graphiteDensityBucketSize(args.From) |
There was a problem hiding this comment.
Density bucket size ignores default when From is empty
Medium Severity
graphiteDensityBucketSize is called with raw args.From instead of the default-applied value. When args.From is empty, the query defaults to "-1h" (line 491-492), but graphiteDensityBucketSize("") returns "1h" because an empty string doesn't start with "-". If the defaulted "-1h" were passed instead, it would correctly return "5m". This means the summarize call uses a 1-hour bucket for a 1-hour window, producing ~1 data bucket instead of the targeted 12, significantly degrading density analysis accuracy.
Reviewed by Cursor Bugbot for commit 70430c8. Configure here.
|
|
1 participant
Note
Medium Risk
Adds new datasource-querying MCP tools (Graphite) and related HTTP/proxy logic, which can affect server behavior and load when enabled. Risk is moderated by read-only semantics and extensive unit tests, but it introduces new external API interactions and response parsing.
Overview
Adds a new Graphite tool suite:
list_graphite_metrics,list_graphite_tags,query_graphite, andquery_graphite_density, all executed via Grafana’s datasource proxy with auth/org-aware transport and response-size limiting.Introduces parsing/typing for Graphite render/find/tag APIs, plus a density-analysis path that optionally uses
summarize(isNonNull())for efficiency and falls back to raw datapoints when unsupported/mismatched.Wires
graphiteinto the server’s--enabled-toolsdefault and adds--disable-graphite, and extends empty-result hint generation with Graphite-specific causes/actions. Includes comprehensive unit tests for parsing, HTTP handling, and density calculations.Reviewed by Cursor Bugbot for commit 70430c8. Bugbot is set up for automated code reviews on this repo. Configure here.