Conversation
Define metrics catalog (14 counters), HTTP endpoint design, architecture overview, CLI flag (-metrics-addr), security considerations, and usage examples. Zero external dependencies — uses stdlib expvar + net/http only.
Task breakdown: T001 metrics package, T002 CLI flag/HTTP listener, T003-T007 instrumentation points (connections, registry, publisher disconnect, media logger, relay), T008 integration test. Includes dependency graph, acceptance criteria, and test strategy.
Declare 14 expvar.Int counters for connections, streams, publishers, subscribers, media throughput, and relay stats. Register expvar.Func for uptime_seconds and server_info. All counters use atomic int64.
Add -metrics-addr flag (default empty = disabled). When set, starts an HTTP server on the given address serving expvar data at /debug/vars. Blank-import expvar and metrics packages to register handlers.
Increment ConnectionsActive and ConnectionsTotal on successful handshake in acceptLoop. Decrement ConnectionsActive in RemoveConnection on disconnect.
…ribers) Increment/decrement StreamsActive on CreateStream/DeleteStream. Increment PublishersActive and PublishersTotal on SetPublisher. Increment/decrement SubscribersActive on AddSubscriber/RemoveSubscriber. Increment SubscribersTotal on AddSubscriber.
Decrement PublishersActive when a publisher disconnects and its reference is cleared from the stream. Only fires when the disconnecting connection matches the current publisher.
Increment MessagesAudio/MessagesVideo on each media packet. Accumulate BytesIngested from payload sizes. Counters are updated inside the existing mu.Lock scope alongside per-connection counters.
Increment RelayMessagesSent and RelayBytesSent on successful relay. Increment RelayMessagesDropped on connection-down and send-error paths. These are global aggregates across all destinations.
Start RTMP server + HTTP metrics server on ephemeral ports, query /debug/vars, and verify all 15 rtmp_* keys are present in the JSON response. Assert uptime >= 0 and server_info contains go_version.
…ss-package comment
…ations writeChunk() previously allocated a new buffer on every call via make([]byte, 0, len(header)+len(payload)). For 30fps video with 4096-byte chunks this created ~1000 allocations/sec. Move writeChunk to a method on Writer and reuse a scratch buffer that grows as needed but is never shrunk, eliminating steady-state allocations.
The bufpool package (sync.Pool-backed byte slice recycler) was implemented but never imported by any production or test code. Removing dead code to reduce maintenance surface.
Only allocate the Extra map when there are actually extra fields in the connect object. In the common case (no extra fields), this avoids one map allocation per connect command.
Replace panic(err) with t.Fatalf in number_test.go and string_test.go golden helpers. Add t.Helper() to array_test.go helper. Remove duplicate goldenDir constants (goldenDirBoolean, goldenDirNull, goldenDirString) that shadowed the canonical constant from number_test.go.
Move stubConn, capturingConn, stubPublisher, buildPublishMessage, and buildPlayMessage into a shared helpers_test.go file. These types were duplicated across publish_handler_test.go and play_handler_test.go.
TestNumber_EdgeCases_RoundTrip and TestDecodeValue_UnsupportedMarkers used bare loops without t.Run, producing unnamed subtest output on failure. Wrap each iteration in t.Run with descriptive names.
Add TestWriter_WriteMessage_BoundaryChunkSizes covering payloads at chunkSize-1, chunkSize, and chunkSize+1 bytes for both 128 and 4096 byte chunk sizes. Round-trips through Reader to verify correctness. Add TestHandlePublishNilArgs verifying that HandlePublish returns errors (not panics) for nil registry, nil connection, and nil message.
decodeValueWithMarker previously reconstructed an io.MultiReader for every nested AMF value (3 heap allocations per value: []byte, Reader, MultiReader). For deeply nested objects this produced significant GC pressure. Rewrite decodeValueWithMarker to decode each type's payload directly after the marker has been consumed. Extract decodeObjectPayload, decodeStrictArrayPayload, and decodeStringPayload internal helpers. DecodeObject and DecodeStrictArray now delegate to these helpers after reading their marker byte.
Add t.Run with named subtests to TestParseAudioMessage_Errors and TestParseVideoMessage_ErrorCases for individually identifiable failures. Remove duplicate _tVidFatalf helper from video_test.go — reuse _tFatalf from audio_test.go since both are in the same package.
TriggerEvent allocated a hooks slice then used append for the stdio hook, causing a reallocation when stdio was enabled. Pre-allocate with capacity for the extra element to avoid the wasted allocation.
- amf.go: update DecodeValue comment to reflect that decodeValueWithMarker now decodes payloads directly (no longer uses io.MultiReader) - amf.go: clarify unsupported AMF0 marker comment with specific marker names - docs/implementation.md: remove bufpool from package structure tree - docs/architecture.md: remove bufpool from package map table - docs/testing-guide.md: remove bufpool test command and test matrix entry - CHANGELOG.md: remove bufpool from server infrastructure list
Add complete changelog section covering all 21 commits on the branch: - Expvar metrics feature (counters, CLI flag, HTTP endpoint) - Performance optimizations (AMF MultiReader, chunk writer buffer, RPC lazy-init, hook manager pre-alloc) - Dead code removal (bufpool package, ErrForbidden sentinel) - Test improvements (golden helpers, t.Run subtests, edge cases, consolidated test doubles) - Update feature branch table and comparison links
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
1 participant
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.
This pull request introduces a new metrics subsystem using Go's
expvarpackage, providing live server counters via an optional HTTP endpoint. It also removes the unused buffer pool code and updates documentation and CLI flags to reflect these changes. The most important changes are grouped below: