From ae447c132400d67d222f25a2d189c68b6a5d54fb Mon Sep 17 00:00:00 2001 From: Cody Lee Date: Wed, 11 Feb 2026 08:03:20 -0600 Subject: [PATCH] fix(rum): convert relative time strings in sessions commands Fix bug where `pup rum sessions list` and `pup rum sessions search` passed raw relative time strings (e.g., "1h") directly to the Datadog RUM API, causing 400 errors. The RUM API expects either date math format ("now-1h"), ISO 8601 timestamps, or Unix milliseconds. Changes: - Convert relative times to Unix milliseconds using parseTimeString() - Validate time parameters before creating API client for faster feedback - Add comprehensive error messages with format hints and troubleshooting - Include request details in error output for better debugging Testing: - Add TestRumSessionsList_TimeConversion with 4 test cases - Add TestRumSessionsSearch_TimeConversion with 4 test cases - All tests pass with race detection enabled - Behavior now matches logs commands (cmd/logs_simple.go:645-653) Fixes #37 Co-Authored-By: Claude Sonnet 4.5 --- cmd/rum.go | 42 +++++++++-- cmd/rum_test.go | 185 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 221 insertions(+), 6 deletions(-) diff --git a/cmd/rum.go b/cmd/rum.go index 3eaabd66..16c9fc55 100644 --- a/cmd/rum.go +++ b/cmd/rum.go @@ -657,6 +657,21 @@ func runRumRetentionFiltersDelete(cmd *cobra.Command, args []string) error { // RUM Sessions Implementation func runRumSessionsList(cmd *cobra.Command, args []string) error { + // Convert relative time strings to absolute timestamps (validate input first) + fromTime, err := parseTimeString(rumFrom) + if err != nil { + return fmt.Errorf("invalid --from time: %w\n\nSupported formats:\n- Relative: 1h, 30m, 7d, 1w (hour, minute, day, week)\n- Absolute: Unix timestamp in milliseconds\n- now: Current time", err) + } + + toTime, err := parseTimeString(rumTo) + if err != nil { + return fmt.Errorf("invalid --to time: %w\n\nSupported formats:\n- Relative: 1h, 30m, 7d, 1w (hour, minute, day, week)\n- Absolute: Unix timestamp in milliseconds\n- now: Current time", err) + } + + // Convert timestamps to strings for RUM API + from := fmt.Sprintf("%d", fromTime) + to := fmt.Sprintf("%d", toTime) + client, err := getClient() if err != nil { return err @@ -665,8 +680,8 @@ func runRumSessionsList(cmd *cobra.Command, args []string) error { api := datadogV2.NewRUMApi(client.V2()) body := datadogV2.RUMSearchEventsRequest{ Filter: &datadogV2.RUMQueryFilter{ - From: &rumFrom, - To: &rumTo, + From: &from, + To: &to, }, Page: &datadogV2.RUMQueryPageOptions{ Limit: datadog.PtrInt32(int32(rumLimit)), @@ -676,7 +691,7 @@ func runRumSessionsList(cmd *cobra.Command, args []string) error { resp, r, err := api.SearchRUMEvents(client.Context(), body) if err != nil { if r != nil { - return fmt.Errorf("failed to list RUM sessions: %w (status: %d)", err, r.StatusCode) + return fmt.Errorf("failed to list RUM sessions: %w (status: %d)\n\nRequest Details:\n- From: %s (parsed from: %s)\n- To: %s (parsed from: %s)\n- Limit: %d\n\nTroubleshooting:\n- Verify your time range is valid and --from is before --to\n- Ensure you have proper permissions for RUM data access\n- Check that RUM data exists for your selected time range", err, r.StatusCode, from, rumFrom, to, rumTo, rumLimit) } return fmt.Errorf("failed to list RUM sessions: %w", err) } @@ -690,6 +705,21 @@ func runRumSessionsList(cmd *cobra.Command, args []string) error { } func runRumSessionsSearch(cmd *cobra.Command, args []string) error { + // Convert relative time strings to absolute timestamps (validate input first) + fromTime, err := parseTimeString(rumFrom) + if err != nil { + return fmt.Errorf("invalid --from time: %w\n\nSupported formats:\n- Relative: 1h, 30m, 7d, 1w (hour, minute, day, week)\n- Absolute: Unix timestamp in milliseconds\n- now: Current time", err) + } + + toTime, err := parseTimeString(rumTo) + if err != nil { + return fmt.Errorf("invalid --to time: %w\n\nSupported formats:\n- Relative: 1h, 30m, 7d, 1w (hour, minute, day, week)\n- Absolute: Unix timestamp in milliseconds\n- now: Current time", err) + } + + // Convert timestamps to strings for RUM API + from := fmt.Sprintf("%d", fromTime) + to := fmt.Sprintf("%d", toTime) + client, err := getClient() if err != nil { return err @@ -699,8 +729,8 @@ func runRumSessionsSearch(cmd *cobra.Command, args []string) error { body := datadogV2.RUMSearchEventsRequest{ Filter: &datadogV2.RUMQueryFilter{ Query: &rumQuery, - From: &rumFrom, - To: &rumTo, + From: &from, + To: &to, }, Page: &datadogV2.RUMQueryPageOptions{ Limit: datadog.PtrInt32(int32(rumLimit)), @@ -710,7 +740,7 @@ func runRumSessionsSearch(cmd *cobra.Command, args []string) error { resp, r, err := api.SearchRUMEvents(client.Context(), body) if err != nil { if r != nil { - return fmt.Errorf("failed to search RUM sessions: %w (status: %d)", err, r.StatusCode) + return fmt.Errorf("failed to search RUM sessions: %w (status: %d)\n\nRequest Details:\n- Query: %s\n- From: %s (parsed from: %s)\n- To: %s (parsed from: %s)\n- Limit: %d\n\nTroubleshooting:\n- Verify your time range is valid and --from is before --to\n- Check that your query syntax is correct\n- Ensure you have proper permissions for RUM data access\n- Check that RUM data exists for your selected time range and query", err, r.StatusCode, rumQuery, from, rumFrom, to, rumTo, rumLimit) } return fmt.Errorf("failed to search RUM sessions: %w", err) } diff --git a/cmd/rum_test.go b/cmd/rum_test.go index 02742524..b1548b2e 100644 --- a/cmd/rum_test.go +++ b/cmd/rum_test.go @@ -8,6 +8,7 @@ package cmd import ( "testing" + "github.com/DataDog/pup/pkg/config" "github.com/spf13/cobra" ) @@ -207,3 +208,187 @@ func TestRumAppsListCmd(t *testing.T) { t.Error("RunE is nil") } } + +// TestRumSessionsList_TimeConversion tests that relative time strings are converted properly +func TestRumSessionsList_TimeConversion(t *testing.T) { + // Save originals + origCfg := cfg + origClient := ddClient + origFrom := rumFrom + origTo := rumTo + origLimit := rumLimit + + // Cleanup + defer func() { + cfg = origCfg + ddClient = origClient + rumFrom = origFrom + rumTo = origTo + rumLimit = origLimit + }() + + tests := []struct { + name string + from string + to string + wantError bool + errorMsg string + }{ + { + name: "valid relative time 1h", + from: "1h", + to: "now", + wantError: true, // Will fail on getClient, but time parsing should work + }, + { + name: "valid relative time 30m", + from: "30m", + to: "now", + wantError: true, // Will fail on getClient, but time parsing should work + }, + { + name: "valid relative time 7d", + from: "7d", + to: "now", + wantError: true, // Will fail on getClient, but time parsing should work + }, + { + name: "invalid time format", + from: "invalid", + to: "now", + wantError: true, + errorMsg: "invalid --from time", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Setup: set test values + cfg = &config.Config{} + ddClient = nil // Don't create real client (avoids keychain) + rumFrom = tt.from + rumTo = tt.to + rumLimit = 10 + + // Run the command + err := runRumSessionsList(nil, nil) + + // Verify error behavior + if tt.wantError && err == nil { + t.Errorf("Expected error but got none") + } + if !tt.wantError && err != nil { + t.Errorf("Unexpected error: %v", err) + } + if tt.errorMsg != "" && err != nil { + if !containsString(err.Error(), tt.errorMsg) { + t.Errorf("Error message %q does not contain %q", err.Error(), tt.errorMsg) + } + } + }) + } +} + +// TestRumSessionsSearch_TimeConversion tests that relative time strings are converted properly +func TestRumSessionsSearch_TimeConversion(t *testing.T) { + // Save originals + origCfg := cfg + origClient := ddClient + origFrom := rumFrom + origTo := rumTo + origLimit := rumLimit + origQuery := rumQuery + + // Cleanup + defer func() { + cfg = origCfg + ddClient = origClient + rumFrom = origFrom + rumTo = origTo + rumLimit = origLimit + rumQuery = origQuery + }() + + tests := []struct { + name string + query string + from string + to string + wantError bool + errorMsg string + }{ + { + name: "valid relative time 1h", + query: "@type:view", + from: "1h", + to: "now", + wantError: true, // Will fail on getClient, but time parsing should work + }, + { + name: "valid relative time 2h", + query: "status:error", + from: "2h", + to: "1h", + wantError: true, // Will fail on getClient, but time parsing should work + }, + { + name: "invalid from time format", + query: "@type:view", + from: "invalid", + to: "now", + wantError: true, + errorMsg: "invalid --from time", + }, + { + name: "invalid to time format", + query: "@type:view", + from: "1h", + to: "bad-time", + wantError: true, + errorMsg: "invalid --to time", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Setup: set test values + cfg = &config.Config{} + ddClient = nil // Don't create real client (avoids keychain) + rumQuery = tt.query + rumFrom = tt.from + rumTo = tt.to + rumLimit = 10 + + // Run the command + err := runRumSessionsSearch(nil, nil) + + // Verify error behavior + if tt.wantError && err == nil { + t.Errorf("Expected error but got none") + } + if !tt.wantError && err != nil { + t.Errorf("Unexpected error: %v", err) + } + if tt.errorMsg != "" && err != nil { + if !containsString(err.Error(), tt.errorMsg) { + t.Errorf("Error message %q does not contain %q", err.Error(), tt.errorMsg) + } + } + }) + } +} + +// Helper function for string contains check +func containsString(s, substr string) bool { + return len(s) >= len(substr) && (s == substr || len(substr) == 0 || + (len(s) > 0 && len(substr) > 0 && findSubstring(s, substr))) +} + +func findSubstring(s, substr string) bool { + for i := 0; i <= len(s)-len(substr); i++ { + if s[i:i+len(substr)] == substr { + return true + } + } + return false +}