server: add lifetime Prometheus counters#31
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends the server’s Prometheus /metrics output with process-lifetime counters for accepted/completed connections and cumulative bytes sent/received, complementing the existing per-active-connection gauges.
Changes:
- Add server-wide atomic accumulators for completed connections and lifetime byte totals, updated when a relay finishes.
- Extend Prometheus exposition to include
*_totalcounters for accepted/completed connections and bytes sent/received. - Add tests covering the
/metricsendpoint behavior and basic counter/escaping expectations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/server/server.go | Adds lifetime counters, updates relay completion accounting, and writes additional Prometheus counter metrics. |
| pkg/server/server_test.go | Adds /metrics endpoint tests and label escaping/unit expectations for the new metrics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sentBytes := info.SentBytes.Load() | ||
| receivedBytes := info.ReceivedBytes.Load() | ||
| s.completedConns.Add(1) | ||
| s.sentBytesTotal.Add(sentBytes) | ||
| s.recvBytesTotal.Add(receivedBytes) |
There was a problem hiding this comment.
Fixed in 82cb742. The lifetime counters now wait for both copy goroutines to finish before sampling byte totals, so the counters and access log use the completed transfer state instead of one half of the relay.
|
|
||
| func (s *Server) writePrometheusMetrics(w io.Writer, now time.Time) { | ||
| connections := s.ListConnectionInfo() | ||
| acceptedConnections := s.connIndex.Load() |
There was a problem hiding this comment.
Fixed in 82cb742. rsync_proxy_accepted_connections_total now uses a dedicated 64-bit counter, so it cannot wrap at uint32 connection index boundaries.
yaoge123
force-pushed
the
add-prometheus-counters
branch
from
e82cf90 to
82cb742
Compare
Add accepted/completed connection counters and total byte counters that persist across connection lifetimes. Wait for both io.Copy directions to finish before reading final byte counts to avoid undercounting. Close the opposite connection when one direction finishes first to prevent deadlock. Co-Authored-By: Copilot
yaoge123
force-pushed
the
add-prometheus-counters
branch
from
82cb742 to
a6dc511
Compare
| acceptedConnTotal atomic.Uint64 | ||
| completedConns atomic.Int64 | ||
| sentBytesTotal atomic.Int64 | ||
| recvBytesTotal atomic.Int64 |
There was a problem hiding this comment.
These variables are missing some consistency. Namely:
acceptedConnTotalis not a sum of other values, but an incremental counter. It is thus better renamed toacceptedConnCountto align withactiveConnCount. Same forcompletedConns.- These 4 variables are using a mix of
int64anduint64, but since they're all monotonically increasing, it'd be good to have them all useuint64.
There was a problem hiding this comment.
Renamed acceptedConnTotal to acceptedConnCount and completedConns to completedConnCount for consistency with activeConnCount. Changed all four lifetime counters to atomic.Uint64.
Rename acceptedConnTotal to acceptedConnCount and completedConns to completedConnCount for consistency with activeConnCount. Change completedConnCount, sentBytesTotal, and recvBytesTotal to atomic.Uint64 since all values are monotonically increasing.
This is stacked on #30 and adds lifetime counters on top of the Prometheus
/metricsendpoint.Summary
Notes
rsync_proxy_accepted_connections_totalis based on the existing monotonic connection index.rsync_proxy_completed_connections_total,rsync_proxy_sent_bytes_total, andrsync_proxy_received_bytes_totalare updated when a connection reaches and finishes upstream relay.Testing
go test ./pkg/server -run TestMetricsIncludesLifetimeCounters -count=1go test ./pkg/server -run "TestMetrics|TestStatusIncludesSelectedUpstream|TestPrometheusLabelValueEscaping" -count=1go test ./...passed in a Go container withrsyncandnetcat-openbsdinstalled.