feat: add MetricsCollector and instrumentation#9
Merged
maxence2997 merged 11 commits intomainfrom Mar 25, 2026
Merged
Conversation
1.Server needs observability hooks → defined MetricsCollector interface 2.Default must be zero-cost → added NoopCollector with value receiver 3.Option wiring needed → added WithMetrics and serverConfig.metrics field Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1.Hub needs event tracking → added hooks in handleRegister and handleBroadcast 2.Session duration needed → added connectedAt field to session struct 3.Pump and backpressure visibility → instrumented readPump, writePump, enqueue Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1.Interface contract needs verification → added NoopCollector and option tests 2.Integration tests need a helper → added recordingCollector with mutex safety Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1.Metrics goroutine call sites need documentation → added call site table 2.Connection duration semantics need clarity → documented connectedAt lifecycle Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1.Metrics option missing from docs → added to options table and features list 2.Users need usage example → added Metrics section with code sample 3.Release tracking needed → added Unreleased section to CHANGELOG Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a pluggable metrics/instrumentation surface to wspulse/server so consumers can observe connection lifecycle, room allocation, throughput, backpressure, and heartbeat health without adding a hard dependency on any metrics backend.
Changes:
- Introduces
MetricsCollector+ defaultNoopCollector, and wires it into server configuration viaWithMetrics. - Instruments hub/session lifecycle and I/O paths to emit metrics (open/close, broadcast fan-out, send/receive, drops, buffer utilization, pong timeouts).
- Updates docs (README + internals) and changelog; adds unit tests for the collector/option scaffolding.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
metrics.go |
Defines MetricsCollector, NoopCollector, and WithMetrics option. |
options.go |
Extends serverConfig to carry a metrics collector and sets a noop default. |
hub.go |
Emits room/session lifecycle + broadcast metrics; tracks session lifetime via connectedAt. |
session.go |
Emits send/receive/drop/buffer utilization + pong-timeout metrics from pump goroutines. |
metrics_test.go |
Adds tests for noop collector callability and WithMetrics option behavior. |
doc/internals.md |
Documents metrics call sites and duration semantics. |
README.md |
Documents MetricsCollector and adds an example usage section. |
CHANGELOG.md |
Notes the new metrics feature under Unreleased. |
1.resumeBuffer drops were untracked → Push now returns overflow bool, enqueue emits FrameDropped 2.errors.As needed for wrapped net.Error → replaced type assertion with errors.As 3.Code organization issues → moved WithMetrics to options.go, connectedAt to struct literal
Resolve conflicts in options.go (struct fields + defaults), session.go (imports), and CHANGELOG.md (Unreleased entries). Fix hub.go reference from removed `sessions` variable to `h.scratch`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. Add DisconnectReason type with 5 constants (normal, kick, grace_expired, server_close, duplicate) so ConnectionClosed callers can distinguish disconnect causes in metrics backends 2. Emit ConnectionClosed and RoomDestroyed during hub.shutdown() — previously these metrics were silently lost on Server.Close() 3. Fix WaitGroup misuse in MessageFlow integration test — replace with channel signal to avoid Add/Wait race 4. Add select/timeout guards to all bare channel receives in metrics integration tests to prevent CI hangs 5. Add integration tests for ResumeAttempt, FrameDropped (send full + broadcast drop-oldest), PongTimeout, and shutdown metrics 6. Fix "zero cost" wording to "minimal overhead" in metrics.go, internals.md, and README (interface dispatch is not inlined) 7. Fix FrameDropped call sites documentation in internals.md 8. Add MetricsCollector GoDoc: embed pattern for forward-compat, panic behavior warning, SendBufferUtilization volume warning 9. Change ConnectionClosed duration assertion from > 0 to >= 0 10. Add TODOS.md with failed resume detection item Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. RoomCreated/RoomDestroyed now emit outside h.mu.Lock() — if a MetricsCollector implementation calls back into server APIs (e.g. GetConnections), it would deadlock on h.mu.RLock() 2. shutdown() snapshots session info under the lock, emits ConnectionClosed/RoomDestroyed after unlock 3. Fix "future minor versions" → "future versions" in GoDoc — adding interface methods is a breaking change per semver policy Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. MessageBroadcast fanOut now counts only enqueued sessions, excluding closed sessions skipped via <-target.done 2. Add non-reentrant warning to MetricsCollector GoDoc — must not call back into Server APIs to avoid hub deadlock 3. Replace time.Sleep(50ms) with polling loop in FrameDropped_BroadcastDropOldest test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The configuration snippet used `server.NewServer` / `server.WithMetrics` but the exported package name is `wspulse`, not `server`. Co-Authored-By: Claude Opus 4.6 (1M context) <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
MetricsCollectorinterface with 11 typed hooks for connection lifecycle, room state, throughput, backpressure, and heartbeatNoopCollectordefault (zero-cost, compiler-inlined)WithMetrics(mc)server optionconnectedAtfield for connection duration trackingTest plan
make checkpasses (fmt + lint + unit test with race detector)🤖 Generated with Claude Code