Add metrics commands as experimental#175
Conversation
These commands can be used to get either summaries (default) of metrics, or the full series of data points. We introduce a TIGER_EXPERIMENTAL env var to enable this since the REST endpoint is currently marked as preview. After some testing, we can graduate this.
|
|
| type MetricSummary struct { | ||
| Name string `json:"name"` | ||
| Labels map[string]string `json:"labels,omitempty"` | ||
| From time.Time `json:"from"` | ||
| To time.Time `json:"to"` | ||
| Count int `json:"count"` | ||
| Min float64 `json:"min"` | ||
| MinTime time.Time `json:"min_time"` | ||
| Max float64 `json:"max"` | ||
| MaxTime time.Time `json:"max_time"` | ||
| Avg float64 `json:"avg"` | ||
| P50 float64 `json:"p50"` | ||
| P95 float64 `json:"p95"` | ||
| } |
There was a problem hiding this comment.
I would just return the data points and let the agent do its own calculations.
We already have the AggFn parameter and everything, it's just not exposed in the gateway yet so if we want aggregations I would expose that instead of doing it here.
Doing average and percentiles over last value aggregations (what the API does currently) is not accurate because there is already a big lose of data points since we are only taking last_value from the samples.
There was a problem hiding this comment.
After discussing with @RobAtticus we agreed to remove this. I pushed a commit doing that
The handler returns either ServiceMetricsSummaryOutput or ServiceMetricsFullOutput depending on the requested mode, but the tool was registered with only the summary schema. Full-mode responses failed validation with `unexpected additional properties ["series"]`. Register an anyOf union of both shapes. Top-level `Type: "object"` is required by the MCP SDK.
- MCP: drop Mode input, drop ServiceMetricsSummaryOutput and the union output schema, rename ServiceMetricsFullOutput to ServiceMetricsSeriesOutput. Handler always returns raw series. - CLI: drop --mode flag, drop renderMetricsSummary and summarizeSeries, rename renderMetricsFull to renderMetricSeries. - Drop client-side DefaultBucketSeconds — the server auto-selects a default bucket size based on the time window. bucket_seconds/ --bucket-seconds remain as optional overrides. - Delete now-empty internal/tiger/util/metrics.go.
- CLI: use cobra.MarkFlagRequired for --metric, --from, --to instead of manual empty-string checks; drops the "(required)" suffix from flag help since cobra now surfaces it automatically. - MCP: drop timescale_cloud_database_qps and timescale_cloud_database_num_connections from metric_name examples and tool description.
AdrianLC
left a comment
AdrianLC
left a comment
There was a problem hiding this comment.
LGTM
Aside from removing the mode=summary I made some minor adjustments to match recent changes in the API.
For context on why we are removing mode=summary the metric data points at the DB layer are currently aggregated with LAST_VALUE within the bucket. Doing Avg or Percentiles over Last values is wrong and doest not result in accurate values.
The right way to solve this is to another API endpoint that can summarize or aggregate correctly.
Mirrors timescale/savannah-gateway#1851: the metrics-api service now owns the bucket-per-window defaulting policy. Bumps minimum to 60s, updates the description to describe the new tiered auto-select (1m ≤1h, 1h ≤30d, 1d beyond), and propagates the constraint to the MCP tool schema and CLI --bucket-seconds help text.
…st of data points
|
Claude flagged a few minor things that I pushed in a latest commit. It had one more one bigger refactor to remove some of the code duplication, but we can see how this evolves a bit first before doing that. I'll leave this for you to merge in your AM since I'm about to sign off for the day |
3 participants
These commands can be used to get the full series of metric data points for a service. We introduce a TIGER_EXPERIMENTAL env var to enable this since the REST endpoint is currently marked as preview. After some testing, we can graduate this.