Repository navigation
fix(storage): track storage metrics in S3/Azure backends + export read counters to Prometheus - #497
Conversation
…d counters to Prometheus (#349) The local filesystem backend records storage read/write/error counters, but the S3/MinIO and Azure Blob backends recorded nothing — on cloud-storage deployments the counters sat at zero. Both backends now record the same counters on the same operations the local backend instruments (writes including multipart, reads, streaming reads, ranged reads, plus errors on failure paths). Unknown-size streaming uploads count the write but skip the byte counter. Also fixes a related export gap found while documenting the change: arc_storage_reads_total and arc_storage_read_bytes_total were tracked (and present in the JSON snapshot) but missing from the Prometheus text export. Adds the two stanzas and a regression test locking all five arc_storage_* counters into the export format.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds storage metrics tracking (reads, writes, bytes, and errors) to the S3 and Azure storage backends, and exposes these metrics in the Prometheus format. The reviewer suggests ensuring that partially read bytes are still recorded in the metrics even if an io.Copy operation fails halfway through, so that network egress is accurately tracked.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Code Review
This pull request implements storage metrics tracking for S3 and Azure backends, exposing counters for reads, writes, bytes, and errors, and exporting them in Prometheus format. A regression test was also added to verify these metrics. The review feedback suggests guarding the error metric increments in both S3 and Azure backends with a check for active contexts to prevent false-positive error alerts caused by client-side cancellations or timeouts.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…tion-aware error counter Round 1: count bytes delivered to the writer in ReadTo/ReadToAt even when io.Copy fails mid-stream — partial transfers are real network egress. Applied to the four flagged S3/Azure sites and to the local backend's two io.Copy sites for cross-backend parity; the operation counter still counts only completed reads. Round 2: storage operations that fail because the caller cancelled or timed out its context no longer increment the error counter — those are caller-side lifecycle events (client disconnect, shutdown), not storage failures. Implemented as recordStorageError(err) using errors.Is on the returned error (matching the query/puller/reconciler treatment of context.Canceled) rather than a racy ctx.Err() check, applied at every error site in the S3 and Azure backends. Local file ops don't fail on cancellation, so local is unchanged.
|
@gemini-code-assist Both rounds of findings are addressed in the latest commit — partial-read bytes are now counted on mid-stream copy failures (extended to the local backend for parity), and the storage-error counter is now cancellation-aware via /gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces storage metrics tracking (reads, writes, bytes, and errors) for S3 and Azure storage backends, bringing them to parity with the local filesystem backend, and exports these metrics to Prometheus. The reviewer feedback recommends enhancing the error-tracking mechanism by passing context.Context to recordStorageError and checking ctx.Err() != nil to prevent false-positive storage error increments during client disconnects or context cancellations. This robust error-handling pattern should also be adopted in the local storage backend and across all cloud backend error-recording paths.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…n guard (Gemini round 3) A disconnecting client kills a streaming io.Copy with EPIPE/connection reset — a plain network error that does not wrap context.Canceled — so the errors.Is-only check missed exactly the case the guard exists for. recordStorageError now also checks ctx.Err(), and is applied at the local backend's two streaming-read copy sites, whose destination writer can be a network connection (HTTP response stream).
|
@gemini-code-assist Round-3 findings addressed: |
There was a problem hiding this comment.
Code Review
This pull request implements storage metrics tracking for S3 and Azure Blob backends, exports storage read and byte counters to Prometheus, and adds a utility to filter out client-side cancellations and timeouts from storage error metrics. Feedback on the changes suggests improving the Read methods in both S3 and Azure backends to record partially read bytes when io.ReadAll fails mid-stream, ensuring consistency with the ReadTo and ReadToAt implementations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…i round 4) io.ReadAll (and os.ReadFile) return the data read so far alongside an error — count those bytes in the read-byte counter on mid-read failure, consistent with the partial-transfer accounting ReadTo and ReadToAt already do. Applied to S3 and Azure Read and to local Read for cross-backend parity; the operation counter still counts only completed reads.
|
@gemini-code-assist Round-4 findings addressed: the buffered /gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements storage metrics tracking for S3 and Azure Blob storage backends, aligning them with the local filesystem backend. It adds metrics for writes, reads, bytes transferred, and errors, and exports arc_storage_reads_total and arc_storage_read_bytes_total to the Prometheus endpoint. Additionally, a helper function recordStorageError is introduced to avoid incrementing error counters on client-side context cancellations or timeouts. There are no review comments, and we have no further feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
1 participant
Closes #349.
Summary
metrics.Get(), but the S3/MinIO and Azure Blob backends recorded nothing — on cloud-storage deploymentsarc_storage_*counters sat at zero. Both backends now mirror the local backend's instrumentation exactly: writes (WriteReader, S3 multipart), reads (Read,ReadTo,ReadToAt), byte counters, andIncStorageErrorson failure paths.size <= 0, AzureUploadStream) count the write but skip the byte counter; the caveat is documented at both sites.Writedelegates toWriteReaderin both backends, and no caller-level instrumentation exists (verified repo-wide).arc_storage_reads_totalandarc_storage_read_bytes_totalwere tracked (and present in the JSON metrics snapshot) but missing from the Prometheus text export — read metrics were invisible to Prometheus even for local storage. Added the two stanzas plusTestPrometheusFormat_StorageMetrics, locking all fivearc_storage_*counters into the export format.RELEASE_NOTES_2026.06.2.md.Test plan
go build ./cmd/... ./internal/...go vetandgofmt -lclean on touched filesgo test ./internal/storage/ ./internal/metrics/ -count=1passesTestPrometheusFormat_StorageMetricsasserts all five counter stanzas in the Prometheus output🤖 Generated with Claude Code