fix(logs): parse compute string for aggregate metrics#40
Merged
Conversation
Fixes #35 The `pup logs aggregate --compute` flag now properly parses metric-based aggregation functions like `avg(@duration)`, `sum(@bytes)`, etc. Previously, the entire compute string was passed as-is to the aggregation function field, causing 400 errors from the API. The metric field was also hardcoded to "*" instead of being extracted from the compute string. Changes: - Add parseComputeString() to extract aggregation function and metric - Support formats: "count", "avg(@duration)", "percentile(@duration, 99)" - Validate aggregation functions against supported list - Improve error messages with detailed request info and troubleshooting - Add comprehensive test coverage for all compute string formats The fix enables proper usage: pup logs aggregate --query="*" --from="1h" --compute="avg(@duration)" --group-by="service" pup logs aggregate --query="*" --from="1h" --compute="cardinality(@user.id)" pup logs aggregate --query="*" --from="1h" --compute="percentile(@Latency, 99)" Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
📊 Test Coverage ReportThreshold: 80% ✅ Coverage by Package📈 Coverage Status: ✅ PASSED - Coverage meets minimum threshold Updated for commit b96c9bb |
The Datadog API expects percentile aggregations in "pcNN" format (e.g., pc99, pc95, pc50) not "percentile". Updated parseComputeString to automatically convert user-friendly "percentile(@field, NN)" syntax to the API's "pcNN" format. Changes: - Convert percentile(@Latency, 99) -> aggregation="pc99", metric="@Latency" - Add percentileValue extraction from regex - Validate that percentile has a value parameter - Update error messages to show supported percentile formats - Add tests for pc99, pc95, pc50 conversions - Add test for missing percentile value error Examples now working: pup logs aggregate --compute="percentile(@Latency, 99)" # -> pc99 pup logs aggregate --compute="percentile(@duration, 95)" # -> pc95 pup logs aggregate --compute="percentile(@response_time, 50)" # -> pc50 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Changed timestamp handling to use UTC consistently throughout logs commands. This ensures timestamps are displayed and processed in UTC regardless of the local system timezone. Changes: - parseTimeString now uses time.Now().UTC() for relative time calculations - Error messages display timestamps in UTC with "UTC" label - Both runLogsSearch and runLogsAggregate use UTC formatting Before: From: 2026-02-11T08:20:07-06:00 (CST) After: From: 2026-02-11T14:20:07Z UTC This makes timestamps consistent and timezone-independent for API operations. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #35
The
pup logs aggregate --computeflag now properly parses metric-based aggregation functions likeavg(@duration),sum(@bytes), etc. Previously, the entire compute string was passed as-is to the aggregation function field, causing 400 errors from the Datadog API.Changes
Core Fix (cmd/logs_simple.go:641-690)
parseComputeString()helper to extract aggregation function and metric from compute strings"count","avg(@duration)","percentile(@duration, 99)"Updated runLogsAggregate (cmd/logs_simple.go:954-1035)
Test Coverage (cmd/logs_simple_test.go:84-228)
Examples
Now working:
Testing
All tests pass with race detection:
go test -race ./cmd/...Specific test for this fix:
go test -v -run TestParseComputeString ./cmd/Error Message Improvements
Enhanced error messages now include:
🤖 Generated with Claude Code