From c02b26ee529be4a66416c7820dba635ee2aac6c7 Mon Sep 17 00:00:00 2001 From: Michael Vessia Date: Tue, 10 Feb 2026 22:41:20 -0500 Subject: [PATCH 1/2] feat(metrics): add v1 metrics search, fix v2 query request body The v2 QueryTimeseriesData request was missing the required Formulas array and Name field, causing 400 Bad Request for all queries. Fix the request body to match the official SDK example and add a v1 metrics search subcommand using QueryMetrics (same convention as logs search/query). Co-Authored-By: Claude Sonnet 4.5 --- README.md | 15 ++++++- cmd/metrics.go | 106 +++++++++++++++++++++++++++++++++++++++----- cmd/metrics_test.go | 65 ++++++++++++++++++++++++--- docs/COMMANDS.md | 3 +- docs/EXAMPLES.md | 19 ++++---- 5 files changed, 180 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 6e32118c..295a1cf0 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,7 @@ See [docs/COMMANDS.md](docs/COMMANDS.md) for detailed command reference. | API Domain | Status | Pup Commands | Notes | |------------|--------|--------------|-------| -| Metrics | ✅ | `metrics query`, `metrics list`, `metrics get`, `metrics search` | Full query and metadata support | +| Metrics | ✅ | `metrics search`, `metrics query`, `metrics list`, `metrics get` | V1 and V2 APIs supported | | Logs | ✅ | `logs search`, `logs list`, `logs aggregate` | V1 and V2 APIs supported | | Traces | ✅ | `traces search`, `traces list`, `traces aggregate` | APM traces support | | Events | ✅ | `events list`, `events search`, `events get` | Infrastructure event management | @@ -247,6 +247,19 @@ pup monitors get 12345678 pup monitors delete 12345678 --yes ``` +### Metrics + +```bash +# Search metrics using classic query syntax (v1 API) +pup metrics search --query="avg:system.cpu.user{*}" --from="1h" + +# Query time-series data (v2 API) +pup metrics query --query="avg:system.cpu.user{*}" --from="1h" + +# List available metrics +pup metrics list --filter="system.*" +``` + ### Dashboards ```bash diff --git a/cmd/metrics.go b/cmd/metrics.go index 7607306c..f7622223 100644 --- a/cmd/metrics.go +++ b/cmd/metrics.go @@ -12,6 +12,7 @@ import ( "strings" "time" + "github.com/DataDog/datadog-api-client-go/v2/api/datadog" "github.com/DataDog/datadog-api-client-go/v2/api/datadogV1" "github.com/DataDog/datadog-api-client-go/v2/api/datadogV2" "github.com/DataDog/pup/pkg/formatter" @@ -81,7 +82,7 @@ AUTHENTICATION: // Query command var metricsQueryCmd = &cobra.Command{ Use: "query", - Short: "Query time-series metrics data", + Short: "Query time-series metrics data (v2 API)", Long: `Query time-series metrics data with flexible aggregation and filtering. This command queries metrics data from Datadog using the metrics query language. @@ -132,6 +133,31 @@ OUTPUT: RunE: runMetricsQuery, } +// Search command (v1 API) +var metricsSearchCmd = &cobra.Command{ + Use: "search", + Short: "Search metrics (v1 API)", + Long: `Search metrics using the v1 QueryMetrics API with classic query syntax. + +This command uses the v1 metrics query endpoint which accepts the traditional +Datadog query string format directly. Use this when you want straightforward +metric queries without v2 timeseries formula semantics. + +QUERY SYNTAX: + :{} [by {}] + +EXAMPLES: + # Query CPU usage for the last hour + pup metrics search --query="avg:system.cpu.user{*}" --from="1h" + + # Query request count by service + pup metrics search --query="sum:app.requests{env:prod} by {service}" --from="4h" + + # Query with absolute time range + pup metrics search --query="avg:system.load.1{*}" --from="1704067200" --to="1704153600"`, + RunE: runMetricsSearch, +} + // List command var metricsListCmd = &cobra.Command{ Use: "list", @@ -436,6 +462,14 @@ func init() { panic(fmt.Errorf("failed to mark flag as required: %w", err)) } + // Search command flags + metricsSearchCmd.Flags().StringVar(&queryString, "query", "", "Metric query string (required)") + metricsSearchCmd.Flags().StringVar(&fromTime, "from", "1h", "Start time (e.g., 1h, 30m, 7d, now, unix timestamp)") + metricsSearchCmd.Flags().StringVar(&toTime, "to", "now", "End time (e.g., now, unix timestamp)") + if err := metricsSearchCmd.MarkFlagRequired("query"); err != nil { + panic(fmt.Errorf("failed to mark flag as required: %w", err)) + } + // List command flags metricsListCmd.Flags().StringVar(&filterPattern, "filter", "", "Filter metrics by pattern (e.g., system.*)") @@ -474,6 +508,7 @@ func init() { // Add subcommands to metrics metricsCmd.AddCommand(metricsQueryCmd) + metricsCmd.AddCommand(metricsSearchCmd) metricsCmd.AddCommand(metricsListCmd) metricsCmd.AddCommand(metricsMetadataCmd) metricsCmd.AddCommand(metricsSubmitCmd) @@ -498,19 +533,24 @@ func runMetricsQuery(cmd *cobra.Command, args []string) error { return fmt.Errorf("invalid --to time: %w", err) } - // Use v2 API for timeseries query (more flexible) + // Use v2 API for timeseries query api := datadogV2.NewMetricsApi(client.V2()) - // Build query request - metricsQuery := datadogV2.NewMetricsTimeseriesQuery(datadogV2.METRICSDATASOURCE_METRICS, queryString) - timeseriesQuery := datadogV2.MetricsTimeseriesQueryAsTimeseriesQuery(metricsQuery) - body := datadogV2.TimeseriesFormulaQueryRequest{ Data: datadogV2.TimeseriesFormulaRequest{ Attributes: datadogV2.TimeseriesFormulaRequestAttributes{ - From: from.UTC().UnixMilli(), - To: to.UTC().UnixMilli(), - Queries: []datadogV2.TimeseriesQuery{timeseriesQuery}, + Formulas: []datadogV2.QueryFormula{ + {Formula: "a"}, + }, + Queries: []datadogV2.TimeseriesQuery{{ + MetricsTimeseriesQuery: &datadogV2.MetricsTimeseriesQuery{ + DataSource: datadogV2.METRICSDATASOURCE_METRICS, + Query: queryString, + Name: datadog.PtrString("a"), + }, + }}, + From: from.UTC().UnixMilli(), + To: to.UTC().UnixMilli(), }, Type: datadogV2.TIMESERIESFORMULAREQUESTTYPE_TIMESERIES_REQUEST, }, @@ -541,6 +581,48 @@ func runMetricsQuery(cmd *cobra.Command, args []string) error { return nil } +// runMetricsSearch executes the metrics search command using the v1 API +func runMetricsSearch(cmd *cobra.Command, args []string) error { + client, err := getClient() + if err != nil { + return err + } + + // Parse time ranges + from, err := parseTimeParam(fromTime) + if err != nil { + return fmt.Errorf("invalid --from time: %w", err) + } + + to, err := parseTimeParam(toTime) + if err != nil { + return fmt.Errorf("invalid --to time: %w", err) + } + + api := datadogV1.NewMetricsApi(client.V1()) + + resp, r, err := api.QueryMetrics(client.Context(), from.Unix(), to.Unix(), queryString) + if err != nil { + if r != nil && r.Body != nil { + bodyBytes, readErr := io.ReadAll(r.Body) + if readErr == nil && len(bodyBytes) > 0 { + return fmt.Errorf("failed to search metrics: %w\nStatus: %d\nAPI Response: %s", + err, r.StatusCode, string(bodyBytes)) + } + return fmt.Errorf("failed to search metrics: %w (status: %d)", err, r.StatusCode) + } + return fmt.Errorf("failed to search metrics: %w", err) + } + + output, err := formatter.FormatOutput(resp, formatter.OutputFormat(outputFormat)) + if err != nil { + return err + } + + printOutput("%s\n", output) + return nil +} + // runMetricsList executes the metrics list command func runMetricsList(cmd *cobra.Command, args []string) error { client, err := getClient() @@ -727,9 +809,9 @@ func runMetricsSubmit(cmd *cobra.Command, args []string) error { } series := datadogV2.MetricSeries{ - Metric: submitName, - Type: &metricType, - Points: []datadogV2.MetricPoint{point}, + Metric: submitName, + Type: &metricType, + Points: []datadogV2.MetricPoint{point}, Resources: []datadogV2.MetricResource{resource}, } diff --git a/cmd/metrics_test.go b/cmd/metrics_test.go index 26af1ec8..0b9b0813 100644 --- a/cmd/metrics_test.go +++ b/cmd/metrics_test.go @@ -59,7 +59,7 @@ func setupMetricsTestClient(t *testing.T) func() { } } -func TestRunMetricsQuery(t *testing.T) { +func TestRunMetricsSearch(t *testing.T) { cleanup := setupMetricsTestClient(t) defer cleanup() @@ -97,6 +97,45 @@ func TestRunMetricsQuery(t *testing.T) { outputWriter = &buf defer func() { outputWriter = os.Stdout }() + err := runMetricsSearch(metricsSearchCmd, []string{}) + + if (err != nil) != tt.wantErr { + t.Errorf("runMetricsSearch() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func TestRunMetricsQuery(t *testing.T) { + cleanup := setupMetricsTestClient(t) + defer cleanup() + + tests := []struct { + name string + query string + from string + to string + wantErr bool + }{ + { + name: "valid query", + query: "avg:system.cpu.user{*}", + from: "1h", + to: "now", + wantErr: true, // Mock client error + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + queryString = tt.query + fromTime = tt.from + toTime = tt.to + + var buf bytes.Buffer + outputWriter = &buf + defer func() { outputWriter = os.Stdout }() + err := runMetricsQuery(metricsQueryCmd, []string{}) if (err != nil) != tt.wantErr { @@ -106,6 +145,20 @@ func TestRunMetricsQuery(t *testing.T) { } } +func TestMetricsSearchCmd(t *testing.T) { + if metricsSearchCmd == nil { + t.Fatal("metricsSearchCmd is nil") + } + + if metricsSearchCmd.Use != "search" { + t.Errorf("Use = %s, want search", metricsSearchCmd.Use) + } + + if metricsSearchCmd.Short != "Search metrics (v1 API)" { + t.Errorf("Short = %s, want 'Search metrics (v1 API)'", metricsSearchCmd.Short) + } +} + func TestRunMetricsList(t *testing.T) { cleanup := setupMetricsTestClient(t) defer cleanup() @@ -239,13 +292,13 @@ func TestRunMetricsSubmit(t *testing.T) { defer cleanup() tests := []struct { - name string + name string metricName string - value float64 - timestamp string - tags string + value float64 + timestamp string + tags string metricType string - wantErr bool + wantErr bool }{ { name: "submit gauge", diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index ce33daea..fdb20f57 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -81,7 +81,8 @@ pup slos get abc-123-def ### Search/Query ```bash pup logs search --query="status:error" --from="1h" -pup metrics query --query="avg:system.cpu.user{*}" +pup metrics search --query="avg:system.cpu.user{*}" --from="1h" +pup metrics query --query="avg:system.cpu.user{*}" --from="1h" pup events search --query="@user.id:12345" ``` diff --git a/docs/EXAMPLES.md b/docs/EXAMPLES.md index 36db3e0f..96e79679 100644 --- a/docs/EXAMPLES.md +++ b/docs/EXAMPLES.md @@ -38,19 +38,22 @@ pup metrics list --filter="system.*" pup metrics list --filter="custom.app.*" ``` -### Query Metrics +### Search Metrics (v1 API) ```bash -# Simple query +# Classic query syntax +pup metrics search --query="avg:system.cpu.user{*}" --from="1h" + +# Search with aggregation and grouping +pup metrics search --query="sum:app.requests{env:prod} by {service}" --from="4h" +``` + +### Query Metrics (v2 API) +```bash +# Timeseries formula query pup metrics query --query="avg:system.cpu.user{*}" --from="1h" --to="now" # Query with aggregation pup metrics query --query="sum:app.requests{env:prod} by {service}" --from="4h" - -# Query with multiple metrics -pup metrics query \ - --query="avg:system.cpu.user{*}" \ - --query="avg:system.mem.used{*}" \ - --from="1h" ``` ## Monitors From 5592d305635f32dece0139ca5254d6fad65291b8 Mon Sep 17 00:00:00 2001 From: Michael Vessia Date: Tue, 10 Feb 2026 23:14:13 -0500 Subject: [PATCH 2/2] refactor(metrics): improve test quality and assertions - Remove duplicate test case in TestRunMetricsSearch - Add wantErrContains assertions to verify error message content - Replace brittle string-literal checks in TestMetricsSearchCmd with behavioral assertions (subcommand registration, flag existence) - Add TestMetricsCmd_Subcommands verifying all 6 subcommands registered Co-Authored-By: Claude Opus 4.6 --- cmd/metrics_test.go | 205 +++++++++++++++++++++++++++++--------------- 1 file changed, 134 insertions(+), 71 deletions(-) diff --git a/cmd/metrics_test.go b/cmd/metrics_test.go index 0b9b0813..bfd002e6 100644 --- a/cmd/metrics_test.go +++ b/cmd/metrics_test.go @@ -64,31 +64,25 @@ func TestRunMetricsSearch(t *testing.T) { defer cleanup() tests := []struct { - name string - query string - from string - to string - wantErr bool + name string + query string + from string + to string + wantErr bool + wantErrContains string }{ { - name: "valid query", - query: "avg:system.cpu.user{*}", - from: "1h", - to: "now", - wantErr: true, // Mock client error - }, - { - name: "fails on client creation", - query: "avg:system.cpu.user{*}", - from: "1h", - to: "now", - wantErr: true, + name: "fails on client creation", + query: "avg:system.cpu.user{*}", + from: "1h", + to: "now", + wantErr: true, + wantErrContains: "mock client", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Set command flags queryString = tt.query fromTime = tt.from toTime = tt.to @@ -102,6 +96,10 @@ func TestRunMetricsSearch(t *testing.T) { if (err != nil) != tt.wantErr { t.Errorf("runMetricsSearch() error = %v, wantErr %v", err, tt.wantErr) } + + if tt.wantErrContains != "" && err != nil && !strings.Contains(err.Error(), tt.wantErrContains) { + t.Errorf("runMetricsSearch() error = %v, want error containing %q", err, tt.wantErrContains) + } }) } } @@ -111,18 +109,20 @@ func TestRunMetricsQuery(t *testing.T) { defer cleanup() tests := []struct { - name string - query string - from string - to string - wantErr bool + name string + query string + from string + to string + wantErr bool + wantErrContains string }{ { - name: "valid query", - query: "avg:system.cpu.user{*}", - from: "1h", - to: "now", - wantErr: true, // Mock client error + name: "fails on client creation", + query: "avg:system.cpu.user{*}", + from: "1h", + to: "now", + wantErr: true, + wantErrContains: "mock client", }, } @@ -141,6 +141,10 @@ func TestRunMetricsQuery(t *testing.T) { if (err != nil) != tt.wantErr { t.Errorf("runMetricsQuery() error = %v, wantErr %v", err, tt.wantErr) } + + if tt.wantErrContains != "" && err != nil && !strings.Contains(err.Error(), tt.wantErrContains) { + t.Errorf("runMetricsQuery() error = %v, want error containing %q", err, tt.wantErrContains) + } }) } } @@ -150,12 +154,50 @@ func TestMetricsSearchCmd(t *testing.T) { t.Fatal("metricsSearchCmd is nil") } - if metricsSearchCmd.Use != "search" { - t.Errorf("Use = %s, want search", metricsSearchCmd.Use) + if metricsSearchCmd.Short == "" { + t.Error("Short description is empty") + } + + if metricsSearchCmd.RunE == nil { + t.Error("RunE is nil") + } + + // Verify search is registered as a subcommand of metricsCmd + commands := metricsCmd.Commands() + commandMap := make(map[string]bool) + for _, cmd := range commands { + commandMap[cmd.Name()] = true + } + if !commandMap["search"] { + t.Error("search not registered as subcommand of metricsCmd") } - if metricsSearchCmd.Short != "Search metrics (v1 API)" { - t.Errorf("Short = %s, want 'Search metrics (v1 API)'", metricsSearchCmd.Short) + // Verify required and optional flags + flags := metricsSearchCmd.Flags() + if flags.Lookup("query") == nil { + t.Error("Missing --query flag") + } + if flags.Lookup("from") == nil { + t.Error("Missing --from flag") + } + if flags.Lookup("to") == nil { + t.Error("Missing --to flag") + } +} + +func TestMetricsCmd_Subcommands(t *testing.T) { + expectedCommands := []string{"query", "search", "list", "metadata", "submit", "tags"} + + commands := metricsCmd.Commands() + commandMap := make(map[string]bool) + for _, cmd := range commands { + commandMap[cmd.Name()] = true + } + + for _, expected := range expectedCommands { + if !commandMap[expected] { + t.Errorf("Missing subcommand: %s", expected) + } } } @@ -164,19 +206,22 @@ func TestRunMetricsList(t *testing.T) { defer cleanup() tests := []struct { - name string - filter string - wantErr bool + name string + filter string + wantErr bool + wantErrContains string }{ { - name: "no filter", - filter: "", - wantErr: true, // Mock client error + name: "no filter", + filter: "", + wantErr: true, + wantErrContains: "mock client", }, { - name: "with filter", - filter: "system.*", - wantErr: true, + name: "with filter", + filter: "system.*", + wantErr: true, + wantErrContains: "mock client", }, } @@ -193,6 +238,10 @@ func TestRunMetricsList(t *testing.T) { if (err != nil) != tt.wantErr { t.Errorf("runMetricsList() error = %v, wantErr %v", err, tt.wantErr) } + + if tt.wantErrContains != "" && err != nil && !strings.Contains(err.Error(), tt.wantErrContains) { + t.Errorf("runMetricsList() error = %v, want error containing %q", err, tt.wantErrContains) + } }) } } @@ -202,14 +251,16 @@ func TestRunMetricsMetadataGet(t *testing.T) { defer cleanup() tests := []struct { - name string - metricName string - wantErr bool + name string + metricName string + wantErr bool + wantErrContains string }{ { - name: "valid metric name", - metricName: "system.cpu.user", - wantErr: true, // Mock client error + name: "fails on client creation", + metricName: "system.cpu.user", + wantErr: true, + wantErrContains: "mock client", }, } @@ -224,6 +275,10 @@ func TestRunMetricsMetadataGet(t *testing.T) { if (err != nil) != tt.wantErr { t.Errorf("runMetricsMetadataGet() error = %v, wantErr %v", err, tt.wantErr) } + + if tt.wantErrContains != "" && err != nil && !strings.Contains(err.Error(), tt.wantErrContains) { + t.Errorf("runMetricsMetadataGet() error = %v, want error containing %q", err, tt.wantErrContains) + } }) } } @@ -233,36 +288,40 @@ func TestRunMetricsMetadataUpdate(t *testing.T) { defer cleanup() tests := []struct { - name string - metricName string - description string - unit string - metricType string - wantErr bool + name string + metricName string + description string + unit string + metricType string + wantErr bool + wantErrContains string }{ { - name: "update description", - metricName: "system.cpu.user", - description: "CPU user time", - unit: "", - metricType: "", - wantErr: true, // Mock client error + name: "update description", + metricName: "system.cpu.user", + description: "CPU user time", + unit: "", + metricType: "", + wantErr: true, + wantErrContains: "mock client", }, { - name: "update multiple fields", - metricName: "system.cpu.user", - description: "CPU user time", - unit: "percent", - metricType: "gauge", - wantErr: true, + name: "update multiple fields", + metricName: "system.cpu.user", + description: "CPU user time", + unit: "percent", + metricType: "gauge", + wantErr: true, + wantErrContains: "mock client", }, { - name: "no fields specified", - metricName: "system.cpu.user", - description: "", - unit: "", - metricType: "", - wantErr: true, // Should error: no fields specified + name: "no fields specified hits client error first", + metricName: "system.cpu.user", + description: "", + unit: "", + metricType: "", + wantErr: true, + wantErrContains: "mock client", }, } @@ -283,6 +342,10 @@ func TestRunMetricsMetadataUpdate(t *testing.T) { if (err != nil) != tt.wantErr { t.Errorf("runMetricsMetadataUpdate() error = %v, wantErr %v", err, tt.wantErr) } + + if tt.wantErrContains != "" && err != nil && !strings.Contains(err.Error(), tt.wantErrContains) { + t.Errorf("runMetricsMetadataUpdate() error = %v, want error containing %q", err, tt.wantErrContains) + } }) } }