fix: recover USD cost from log files when events.jsonl has no cost data#28954
fix: recover USD cost from log files when events.jsonl has no cost data#28954Copilot wants to merge 2 commits into
Conversation
When events.jsonl is used as the primary metrics source, turns/tokens/tool calls are accurate but EstimatedCost is never set (events.jsonl only carries integer premium-request counts, not USD amounts). This caused summary.total_cost = 0 in `gh aw logs --json`, which in turn made the copilot-token-audit report show "0 total cost" in its executive summary. Fix: after a successful events.jsonl parse, if EstimatedCost is still 0, walk the .log files solely to accumulate cost from Copilot API response JSON (the same source the log-file walk has always used). Turns, tokens, and tool calls continue to come from events.jsonl, which is more accurate. Add a regression test that asserts cost is recovered from a log file containing `total_cost_usd` even when events.jsonl is present and provides turns/tokens. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/7e6d90c6-7be8-4941-b0d1-16f2c3852546 Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/7e6d90c6-7be8-4941-b0d1-16f2c3852546 Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
🧪 Test Quality Sentinel ReportTest Quality Score: 100/100✅ Excellent test quality
Test Classification Details
Language SupportTests analyzed: Verdict
The new subtest verifies a clear behavioral contract: when 📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators. References:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR restores non-zero USD cost reporting when events.jsonl is present (and used for turns/tokens) but does not include cost fields, by recovering EstimatedCost from Copilot API JSON embedded in .log files.
Changes:
- Add a cost-only log-file walk that runs when
events.jsonlparses successfully butEstimatedCostis still0. - Log failures from the cost recovery walk (instead of silently discarding them).
- Add a regression test ensuring cost is recovered from
.logfiles while turns/tokens remain sourced fromevents.jsonl.
Show a summary per file
| File | Description |
|---|---|
| pkg/cli/logs_metrics.go | Adds a cost-recovery log walk when events.jsonl yields no cost. |
| pkg/cli/copilot_events_jsonl_test.go | Adds coverage for recovering cost from .log files even when events.jsonl is present. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
pkg/cli/logs_metrics.go:189
fileErrfromparseLogFileWithEngineis only surfaced whenverboseis true; whenverboseis false it is silently ignored, despite the PR intent to log cost-walk errors. Also, cost is still added even whenfileErr != nil(using whatever metrics value was returned). Consider always logging parse/read failures tologsMetricsLog(and optionally stderr when verbose) and skipping accumulation for that file when an error occurs.
fileMetrics, fileErr := parseLogFileWithEngine(path, detectedEngine, isGitHubCopilotCodingAgent, verbose)
if fileErr != nil && verbose {
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to parse log file %s for cost: %v", path, fileErr)))
return nil
}
metrics.EstimatedCost += fileMetrics.EstimatedCost
}
- Files reviewed: 2/2 changed files
- Comments generated: 2
| fileName := strings.ToLower(info.Name()) | ||
| if (strings.HasSuffix(fileName, ".log") || | ||
| (strings.HasSuffix(fileName, ".txt") && strings.Contains(fileName, "log"))) && | ||
| !strings.Contains(fileName, "aw_output") && | ||
| fileName != constants.AgentOutputFilename { | ||
| fileMetrics, fileErr := parseLogFileWithEngine(path, detectedEngine, isGitHubCopilotCodingAgent, verbose) | ||
| if fileErr != nil && verbose { | ||
| fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to parse log file %s for cost: %v", path, fileErr))) | ||
| return nil | ||
| } | ||
| metrics.EstimatedCost += fileMetrics.EstimatedCost | ||
| } | ||
| return nil |
There was a problem hiding this comment.
walkLogFilesForCost duplicates the file filtering and traversal logic used in the main log walk below. To reduce the chance these two paths drift over time (e.g., different skip rules), consider extracting a shared helper that iterates relevant log files and lets callers decide what to aggregate (full metrics vs cost-only).
| walkLogFilesForCost := func() { | ||
| if walkErr := filepath.Walk(logDir, func(path string, info os.FileInfo, err error) error { | ||
| if err != nil { | ||
| return err |
There was a problem hiding this comment.
In the cost-recovery walk, returning the incoming err from the filepath.Walk callback will abort the entire walk on the first filesystem traversal error (e.g., unreadable file/dir), which can prevent recovering cost from remaining log files. Since this path is intended to be best-effort, consider logging the traversal error and returning nil to continue walking (or SkipDir where appropriate).
This issue also appears on line 183 of the same file.
| return err | |
| logsMetricsLog.Printf("Skipping path during cost recovery walk %s: %v", path, err) | |
| if verbose { | |
| fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Skipping path during cost recovery walk %s: %v", path, err))) | |
| } | |
| if info != nil && info.IsDir() { | |
| return filepath.SkipDir | |
| } | |
| return nil |
When
events.jsonlbecame the primary metrics source,EstimatedCoststopped being populated —events.jsonlonly carries an integer premium-request count, not a USD amount. This causedsummary.total_cost = 0ingh aw logs --json, propagating to $0 totals in copilot-token-audit reports.Changes
pkg/cli/logs_metrics.go: After a successfulevents.jsonlparse (turns/tokens/tool calls), ifEstimatedCost == 0, perform a targeted.logfile walk solely to accumulate cost from Copilot API response JSON — the same extraction path that worked beforeevents.jsonltook priority. Errors from that walk are now logged rather than silently discarded.pkg/cli/copilot_events_jsonl_test.go: Regression test —recovers_cost_from_log_files_even_when_events.jsonl_is_present— verifies that a directory with a validevents.jsonl(no cost) and a.logcontaining"total_cost_usd": 0.042producesEstimatedCost > 0while preserving events.jsonl-sourced turns/tokens.