richat: add metric channel_events_received#153
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new metric channel_events_received to track the number of received messages by source and type. The implementation threads source names through the subscription system to enable per-source metrics.
Key changes:
- Added source name tracking throughout the subscription chain (
&'static str) - Implemented validation for unique source names and consistent parsers at deserialization time
- Added new counter metric
CHANNEL_EVENTS_RECEIVEDwith source and type labels
Reviewed Changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| richat/src/source.rs | Extended Subscription and stream types to carry source name (&'static str) through the streaming pipeline |
| richat/src/metrics.rs | Added new CHANNEL_EVENTS_RECEIVED metric constant and description |
| richat/src/config.rs | Added custom deserializer to validate unique source names and parser consistency; simplified get_messages_parser() |
| richat/src/channel.rs | Added as_str_type() method and integrated metric recording in Sender::push() |
| richat/src/bin/richat.rs | Updated message handling to thread source name through to messages.push() |
| richat/Cargo.toml | Version bump to 6.1.0 |
| Cargo.lock | Updated version lock for richat package |
| CHANGELOG.md | Documented new feature release |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) -> anyhow::Result<BoxStream<'static, Result<(usize, Message), ReceiveError>>> { | ||
| ) -> anyhow::Result<BoxStream<'static, Result<(usize, &'static str, Message), ReceiveError>>> { | ||
| let (subscription_config, mut config) = SubscriptionConfig::new(config); | ||
| let name: &'static str = config.name.clone().leak(); |
There was a problem hiding this comment.
Memory leak: String::leak() allocates memory that is never freed. This is called every time a subscription is created. Consider using Arc<str> or &'static str from a pre-allocated string pool instead, especially since source names are now validated to be unique during deserialization and could be allocated once during config loading.
| }); | ||
| }; | ||
| } | ||
| unreachable!("deserialize should check sources") |
There was a problem hiding this comment.
Error message should be more descriptive. Consider: unreachable!('sources cannot be empty due to deserialize validation') to clarify the invariant being relied upon.
| unreachable!("deserialize should check sources") | |
| unreachable!("sources cannot be empty due to deserialize validation") |
example: