chore: observability audit fixes and documentation updates - #41
Merged
Conversation
- fix connected_clients in INFO to show active connections (not total) - add connections_active atomic counter to ServerContext - move metrics_enabled into ServerContext, remove parameter threading - gate Instant::now() behind metrics/slowlog check to avoid overhead - replace .expect() on slowlog mutex with graceful poison recovery - add SlowLog::is_enabled() for fast enablement checks - update README with observability features, new CLI flags, status - update Dockerfile to expose metrics port 9100 - update crate READMEs (ember-server, ember-protocol) - mark Phase 5 Week 15 items complete in CLAUDE.md
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
connected_clientsin INFO to report active connections instead of total accepted (was a monotonically increasing counter, now tracks live connections)connections_activeatomic counter toServerContext, incremented on accept and decremented on handler exitmetrics_enabledintoServerContextto reduce parameter threading throughhandle()andprocess()Instant::now()behind ametrics_enabled || slowlog.is_enabled()check so there's zero timing overhead when both are disabled.expect("slowlog lock poisoned")on all 4 mutex acquisitions inSlowLogwith graceful poison recovery (clears entries and continues rather than crashing the server)tested
cargo clippy --workspace -- -D warnings— cleancargo test --workspace— all 579 tests passconnections_activeinstead ofconnections_accepteddesign considerations
SlowLogclears entries on the write path (maybe_record) but preserves them on read paths (get,len). the slow log is best-effort observability — losing entries after a panic is acceptable, crashing the server is not.is_enabled()onSlowLogreadsconfig.enabledwhich is set at construction time and never changes, so no synchronization needed.Option<Instant>instead of a separate code path to keep the logic simple and avoid duplication.