proxy: fix metrics capture for v1/responses - #586
Conversation
properly parse anthropic compatible usage data from streaming responses. closes: #577
WalkthroughThe PR modifies metrics monitoring to handle nested usage structures in SSE streaming responses. When parsing streaming data, if usage is not found at the top level, the code now checks for usage under Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can enforce grammar and style rules using `languagetool`.Configure the |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@proxy/metrics_monitor.go`:
- Around line 513-515: The code extracts the encoding token with encoding, _, _
:= strings.Cut(strings.TrimSpace(part), ";") but doesn't trim encoding
afterwards, so values like "gzip " fail the supported[strings.ToLower(encoding)]
check; fix by trimming encoding (e.g., encoding = strings.TrimSpace(encoding))
before lowercasing and checking the supported map in the same block where part,
encoding and filtered are handled (referencing the variables encoding, part,
supported and the strings.Cut call).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 20900357-3e65-4ecf-9afb-eb30a080abe5
📒 Files selected for processing (2)
proxy/metrics_monitor.goproxy/metrics_monitor_test.go
| encoding, _, _ := strings.Cut(strings.TrimSpace(part), ";") | ||
| if supported[strings.ToLower(encoding)] { | ||
| filtered = append(filtered, strings.TrimSpace(part)) |
There was a problem hiding this comment.
Trim the encoding token after strings.Cut to handle valid OWS before ;q=.
At Line 513, headers like gzip ;q=1.0 leave encoding as "gzip " and fail the supported check.
💡 Proposed fix
- encoding, _, _ := strings.Cut(strings.TrimSpace(part), ";")
+ encoding, _, _ := strings.Cut(strings.TrimSpace(part), ";")
+ encoding = strings.TrimSpace(encoding)
if supported[strings.ToLower(encoding)] {
filtered = append(filtered, strings.TrimSpace(part))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| encoding, _, _ := strings.Cut(strings.TrimSpace(part), ";") | |
| if supported[strings.ToLower(encoding)] { | |
| filtered = append(filtered, strings.TrimSpace(part)) | |
| encoding, _, _ := strings.Cut(strings.TrimSpace(part), ";") | |
| encoding = strings.TrimSpace(encoding) | |
| if supported[strings.ToLower(encoding)] { | |
| filtered = append(filtered, strings.TrimSpace(part)) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@proxy/metrics_monitor.go` around lines 513 - 515, The code extracts the
encoding token with encoding, _, _ := strings.Cut(strings.TrimSpace(part), ";")
but doesn't trim encoding afterwards, so values like "gzip " fail the
supported[strings.ToLower(encoding)] check; fix by trimming encoding (e.g.,
encoding = strings.TrimSpace(encoding)) before lowercasing and checking the
supported map in the same block where part, encoding and filtered are handled
(referencing the variables encoding, part, supported and the strings.Cut call).
properly parse anthropic compatible usage data from streaming responses. closes: #577
properly parse anthropic compatible usage data from streaming responses.
closes: #577