fix(cli): improve time parsing, interface consistency, and documentation accuracy

README.md

@@ -41,7 +41,7 @@ See [docs/COMMANDS.md](docs/COMMANDS.md) for detailed command reference.
 |------------|--------|--------------|-------|
 | 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 |
+| Traces | ❌ | - | Not yet implemented (use APM commands for trace data) |
 | Events | ✅ | `events list`, `events search`, `events get` | Infrastructure event management |
 | RUM | ✅ | `rum apps`, `rum sessions`, `rum metrics list/get`, `rum retention-filters list/get` | Apps, sessions, metrics, retention filters (create/update pending) |
 | APM Services | ✅ | `apm services`, `apm entities`, `apm dependencies`, `apm flow-map` | Services stats, operations, resources; entity queries; dependencies; flow visualization |

cmd/apm.go

@@ -428,7 +428,10 @@ var (
 
 func init() {
 	// Services list flags
-	apmServicesListCmd.Flags().StringVar(&envFilter, "env", "", "Environment filter")
+	apmServicesListCmd.Flags().StringVar(&envFilter, "env", "", "Environment filter (required)")
+	if err := apmServicesListCmd.MarkFlagRequired("env"); err != nil {
+		panic(fmt.Errorf("failed to mark flag as required: %w", err))
+	}
 	apmServicesListCmd.Flags().Int64Var(&startTime, "start", time.Now().Add(-1*time.Hour).Unix(), "Start time (Unix timestamp)")
 	apmServicesListCmd.Flags().Int64Var(&endTime, "end", time.Now().Unix(), "End time (Unix timestamp)")
 
@@ -544,24 +547,37 @@ func runAPMServicesList(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
+	// Validate that env filter is provided
+	if envFilter == "" {
+		return fmt.Errorf("--env flag is required (e.g., --env prod)")
+	}
+
+	// Set default time range if not provided (shared variables may be 0)
+	if startTime == 0 {
+		startTime = time.Now().Add(-1 * time.Hour).Unix()
+	}
+	if endTime == 0 {
+		endTime = time.Now().Unix()
+	}
+
 	// Build query parameters
 	params := url.Values{}
 	params.Add("start", strconv.FormatInt(startTime, 10))
 	params.Add("end", strconv.FormatInt(endTime, 10))
-	if envFilter != "" {
-		params.Add("env", envFilter)
-	}
+	params.Add("filter[env]", envFilter)
 
 	path := fmt.Sprintf("/api/v2/apm/services?%s", params.Encode())
 	resp, err := client.RawRequest("GET", path, nil)
 	if err != nil {
-		return fmt.Errorf("failed to list APM services: %w", err)
+		return fmt.Errorf("failed to list APM services: %w\n\nRequest: GET %s\nParameters: start=%d, end=%d, filter[env]=%s",
+			err, path, startTime, endTime, envFilter)
 	}
 	defer func() { _ = resp.Body.Close() }()
 
 	result, err := readRawResponse(resp)
 	if err != nil {
-		return fmt.Errorf("failed to list APM services: %w", err)
+		return fmt.Errorf("failed to list APM services: %w\n\nRequest: GET %s\nParameters: start=%d, end=%d, filter[env]=%s",
+			err, path, startTime, endTime, envFilter)
 	}
 
 	output, err := formatter.FormatOutput(result, formatter.OutputFormat(outputFormat))

cmd/logs_simple.go

@@ -14,6 +14,7 @@ import (
 
 	"github.com/DataDog/datadog-api-client-go/v2/api/datadogV2"
 	"github.com/DataDog/pup/pkg/formatter"
+	"github.com/DataDog/pup/pkg/util"
 	"github.com/spf13/cobra"
 )
 
@@ -53,8 +54,12 @@ LOG QUERY SYNTAX:
 
 TIME RANGES:
   Supported time formats:
-  • Relative: 1h, 30m, 7d, 1w (hour, minute, day, week)
+  • Relative short: 1h, 30m, 7d, 5s, 1w
+  • Relative long: 5min, 5minutes, 2hr, 2hours, 3days, 1week
+  • With spaces: "5 minutes", "2 hours"
+  • With minus: -5m, -2h (treated same as 5m, 2h)
   • Absolute: Unix timestamp in milliseconds
+  • RFC3339: 2024-01-01T00:00:00Z
   • now: Current time
 
 EXAMPLES:
@@ -556,33 +561,27 @@ var (
 func init() {
 	// Search command flags (v1)
 	logsSearchCmd.Flags().StringVar(&logsQuery, "query", "", "Search query (required)")
-	logsSearchCmd.Flags().StringVar(&logsFrom, "from", "", "Start time: 1h, 30m, 7d, or timestamp (required)")
-	logsSearchCmd.Flags().StringVar(&logsTo, "to", "now", "End time: 1h, 30m, now, or timestamp")
+	logsSearchCmd.Flags().StringVar(&logsFrom, "from", "1h", "Start time: 1h, 5min, 2hours, '5 minutes', RFC3339, Unix timestamp, or 'now'")
+	logsSearchCmd.Flags().StringVar(&logsTo, "to", "now", "End time: 1h, 5min, 2hours, '5 minutes', RFC3339, Unix timestamp, or 'now'")
 	logsSearchCmd.Flags().IntVar(&logsLimit, "limit", 50, "Maximum number of logs (1-1000)")
 	logsSearchCmd.Flags().StringVar(&logsSort, "sort", "desc", "Sort order: asc or desc")
 	logsSearchCmd.Flags().StringVar(&logsIndex, "index", "", "Comma-separated log indexes")
 	logsSearchCmd.Flags().StringVar(&logsStorage, "storage", "", "Storage tier: indexes, online-archives, or flex")
 	if err := logsSearchCmd.MarkFlagRequired("query"); err != nil {
 		panic(fmt.Errorf("failed to mark flag as required: %w", err))
 	}
-	if err := logsSearchCmd.MarkFlagRequired("from"); err != nil {
-		panic(fmt.Errorf("failed to mark flag as required: %w", err))
-	}
 
 	// List command flags (v2)
 	logsListCmd.Flags().StringVar(&logsQuery, "query", "*", "Search query")
-	logsListCmd.Flags().StringVar(&logsFrom, "from", "", "Start time (required)")
+	logsListCmd.Flags().StringVar(&logsFrom, "from", "1h", "Start time: 1h, 5min, 2hours, '5 minutes', RFC3339, Unix timestamp, or 'now'")
 	logsListCmd.Flags().StringVar(&logsTo, "to", "now", "End time")
 	logsListCmd.Flags().IntVar(&logsLimit, "limit", 10, "Number of logs")
 	logsListCmd.Flags().StringVar(&logsSort, "sort", "-timestamp", "Sort order")
 	logsListCmd.Flags().StringVar(&logsStorage, "storage", "", "Storage tier: indexes, online-archives, or flex")
-	if err := logsListCmd.MarkFlagRequired("from"); err != nil {
-		panic(fmt.Errorf("failed to mark flag as required: %w", err))
-	}
 
 	// Query command flags (v2)
 	logsQueryCmd.Flags().StringVar(&logsQuery, "query", "", "Log query (required)")
-	logsQueryCmd.Flags().StringVar(&logsFrom, "from", "", "Start time (required)")
+	logsQueryCmd.Flags().StringVar(&logsFrom, "from", "1h", "Start time: 1h, 5min, 2hours, '5 minutes', RFC3339, Unix timestamp, or 'now'")
 	logsQueryCmd.Flags().StringVar(&logsTo, "to", "now", "End time")
 	logsQueryCmd.Flags().IntVar(&logsLimit, "limit", 50, "Maximum results")
 	logsQueryCmd.Flags().StringVar(&logsSort, "sort", "-timestamp", "Sort order")
@@ -591,13 +590,10 @@ func init() {
 	if err := logsQueryCmd.MarkFlagRequired("query"); err != nil {
 		panic(fmt.Errorf("failed to mark flag as required: %w", err))
 	}
-	if err := logsQueryCmd.MarkFlagRequired("from"); err != nil {
-		panic(fmt.Errorf("failed to mark flag as required: %w", err))
-	}
 
 	// Aggregate command flags (v2)
 	logsAggregateCmd.Flags().StringVar(&logsQuery, "query", "", "Log query (required)")
-	logsAggregateCmd.Flags().StringVar(&logsFrom, "from", "", "Start time (required)")
+	logsAggregateCmd.Flags().StringVar(&logsFrom, "from", "1h", "Start time: 1h, 5min, 2hours, '5 minutes', RFC3339, Unix timestamp, or 'now'")
 	logsAggregateCmd.Flags().StringVar(&logsTo, "to", "now", "End time")
 	logsAggregateCmd.Flags().StringVar(&logsCompute, "compute", "count", "Metric to compute")
 	logsAggregateCmd.Flags().StringVar(&logsGroupBy, "group-by", "", "Field to group by")
@@ -606,9 +602,6 @@ func init() {
 	if err := logsAggregateCmd.MarkFlagRequired("query"); err != nil {
 		panic(fmt.Errorf("failed to mark flag as required: %w", err))
 	}
-	if err := logsAggregateCmd.MarkFlagRequired("from"); err != nil {
-		panic(fmt.Errorf("failed to mark flag as required: %w", err))
-	}
 
 	// Add subcommands
 	logsCmd.AddCommand(logsSearchCmd)
@@ -641,47 +634,6 @@ func init() {
 
 // Helper functions
 
-// parseTimeString converts relative or absolute time to Unix timestamp in milliseconds (UTC)
-func parseTimeString(timeStr string) (int64, error) {
-	if timeStr == "now" {
-		return time.Now().UTC().UnixMilli(), nil
-	}
-
-	// Try parsing as relative time (1h, 30m, 7d)
-	if len(timeStr) >= 2 {
-		unit := timeStr[len(timeStr)-1:]
-		valueStr := timeStr[:len(timeStr)-1]
-
-		var value int64
-		if _, err := fmt.Sscanf(valueStr, "%d", &value); err == nil {
-			var duration time.Duration
-			switch unit {
-			case "s":
-				duration = time.Duration(value) * time.Second
-			case "m":
-				duration = time.Duration(value) * time.Minute
-			case "h":
-				duration = time.Duration(value) * time.Hour
-			case "d":
-				duration = time.Duration(value) * 24 * time.Hour
-			case "w":
-				duration = time.Duration(value) * 7 * 24 * time.Hour
-			default:
-				return 0, fmt.Errorf("invalid time unit: %s (use s, m, h, d, or w)", unit)
-			}
-			return time.Now().UTC().Add(-duration).UnixMilli(), nil
-		}
-	}
-
-	// Try parsing as Unix timestamp (milliseconds)
-	var timestamp int64
-	if _, err := fmt.Sscanf(timeStr, "%d", &timestamp); err == nil {
-		return timestamp, nil
-	}
-
-	return 0, fmt.Errorf("invalid time format: %s (use relative like '1h' or Unix timestamp)", timeStr)
-}
-
 // validateAndConvertStorageTier validates the storage tier string and converts it to LogsStorageTier
 // Returns nil if storage is empty (which means search all tiers)
 func validateAndConvertStorageTier(storage string) (*datadogV2.LogsStorageTier, error) {
@@ -782,12 +734,12 @@ func runLogsSearch(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	fromTime, err := parseTimeString(logsFrom)
+	fromTime, err := util.ParseTimeToUnixMilli(logsFrom)
 	if err != nil {
 		return fmt.Errorf("invalid --from time: %w", err)
 	}
 
-	toTime, err := parseTimeString(logsTo)
+	toTime, err := util.ParseTimeToUnixMilli(logsTo)
 	if err != nil {
 		return fmt.Errorf("invalid --to time: %w", err)
 	}
@@ -930,12 +882,12 @@ func runLogsList(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	fromTime, err := parseTimeString(logsFrom)
+	fromTime, err := util.ParseTimeToUnixMilli(logsFrom)
 	if err != nil {
 		return fmt.Errorf("invalid --from time: %w", err)
 	}
 
-	toTime, err := parseTimeString(logsTo)
+	toTime, err := util.ParseTimeToUnixMilli(logsTo)
 	if err != nil {
 		return fmt.Errorf("invalid --to time: %w", err)
 	}
@@ -1000,12 +952,12 @@ func runLogsQuery(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	fromTime, err := parseTimeString(logsFrom)
+	fromTime, err := util.ParseTimeToUnixMilli(logsFrom)
 	if err != nil {
 		return fmt.Errorf("invalid --from time: %w", err)
 	}
 
-	toTime, err := parseTimeString(logsTo)
+	toTime, err := util.ParseTimeToUnixMilli(logsTo)
 	if err != nil {
 		return fmt.Errorf("invalid --to time: %w", err)
 	}
@@ -1072,12 +1024,12 @@ func runLogsAggregate(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	fromTime, err := parseTimeString(logsFrom)
+	fromTime, err := util.ParseTimeToUnixMilli(logsFrom)
 	if err != nil {
 		return fmt.Errorf("invalid --from time: %w", err)
 	}
 
-	toTime, err := parseTimeString(logsTo)
+	toTime, err := util.ParseTimeToUnixMilli(logsTo)
 	if err != nil {
 		return fmt.Errorf("invalid --to time: %w", err)
 	}

cmd/logs_simple_test.go

@@ -14,6 +14,7 @@ import (
 
 	"github.com/DataDog/pup/pkg/client"
 	"github.com/DataDog/pup/pkg/config"
+	"github.com/DataDog/pup/pkg/util"
 )
 
 func TestLogsCmd(t *testing.T) {
@@ -58,6 +59,15 @@ func TestParseTimeString(t *testing.T) {
 				return ts > 1000000000000 && ts < 9999999999999
 			},
 		},
+		{
+			name:  "RFC3339 timestamp",
+			input: "2024-01-01T00:00:00Z",
+			check: func(ts int64) bool {
+				// Should be Jan 1, 2024 in milliseconds
+				// 2024-01-01T00:00:00Z = 1704067200000ms
+				return ts == 1704067200000
+			},
+		},
 		{
 			name:    "invalid format",
 			input:   "invalid",
@@ -67,15 +77,15 @@ func TestParseTimeString(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := parseTimeString(tt.input)
+			got, err := util.ParseTimeToUnixMilli(tt.input)
 
 			if (err != nil) != tt.wantErr {
-				t.Errorf("parseTimeString() error = %v, wantErr %v", err, tt.wantErr)
+				t.Errorf("util.ParseTimeToUnixMilli() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
 
 			if !tt.wantErr && tt.check != nil && !tt.check(got) {
-				t.Errorf("parseTimeString() = %d, validation failed", got)
+				t.Errorf("util.ParseTimeToUnixMilli() = %d, validation failed", got)
 			}
 		})
 	}

cmd/metrics.go

@@ -16,6 +16,7 @@ import (
 	"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"
+	"github.com/DataDog/pup/pkg/util"
 	"github.com/spf13/cobra"
 )
 
@@ -523,12 +524,12 @@ func runMetricsQuery(cmd *cobra.Command, args []string) error {
 	}
 
 	// Parse time ranges
-	from, err := parseTimeParam(fromTime)
+	from, err := util.ParseTimeParam(fromTime)
 	if err != nil {
 		return fmt.Errorf("invalid --from time: %w", err)
 	}
 
-	to, err := parseTimeParam(toTime)
+	to, err := util.ParseTimeParam(toTime)
 	if err != nil {
 		return fmt.Errorf("invalid --to time: %w", err)
 	}
@@ -589,12 +590,12 @@ func runMetricsSearch(cmd *cobra.Command, args []string) error {
 	}
 
 	// Parse time ranges
-	from, err := parseTimeParam(fromTime)
+	from, err := util.ParseTimeParam(fromTime)
 	if err != nil {
 		return fmt.Errorf("invalid --from time: %w", err)
 	}
 
-	to, err := parseTimeParam(toTime)
+	to, err := util.ParseTimeParam(toTime)
 	if err != nil {
 		return fmt.Errorf("invalid --to time: %w", err)
 	}
@@ -859,48 +860,3 @@ func runMetricsTagsList(cmd *cobra.Command, args []string) error {
 	return fmt.Errorf("listing tags by metric name is not supported in the current API client version")
 }
 
-// parseTimeParam parses a time parameter (relative or absolute)
-func parseTimeParam(timeStr string) (time.Time, error) {
-	// Handle "now" keyword
-	if strings.ToLower(timeStr) == "now" {
-		return time.Now(), nil
-	}
-
-	// Try parsing as unix timestamp
-	if timestamp, err := strconv.ParseInt(timeStr, 10, 64); err == nil {
-		return time.Unix(timestamp, 0), nil
-	}
-
-	// Parse relative time (e.g., 1h, 30m, 7d, 1w)
-	if len(timeStr) < 2 {
-		return time.Time{}, fmt.Errorf("invalid time format: %s", timeStr)
-	}
-
-	valueStr := timeStr[:len(timeStr)-1]
-	unit := timeStr[len(timeStr)-1:]
-
-	value, err := strconv.ParseInt(valueStr, 10, 64)
-	if err != nil {
-		return time.Time{}, fmt.Errorf("invalid time value: %s", timeStr)
-	}
-
-	now := time.Now()
-	var duration time.Duration
-
-	switch strings.ToLower(unit) {
-	case "s":
-		duration = time.Duration(value) * time.Second
-	case "m":
-		duration = time.Duration(value) * time.Minute
-	case "h":
-		duration = time.Duration(value) * time.Hour
-	case "d":
-		duration = time.Duration(value) * 24 * time.Hour
-	case "w":
-		duration = time.Duration(value) * 7 * 24 * time.Hour
-	default:
-		return time.Time{}, fmt.Errorf("invalid time unit: %s (use s, m, h, d, or w)", unit)
-	}
-
-	return now.Add(-duration), nil
-}