[ISSUE #3452]🚀Add ConsumerAttr struct and builder for metrics tracking #3453
Conversation
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ConsumerAttrBuilder
participant ConsumerAttr
User->>ConsumerAttrBuilder: set group, language, version, consume_mode
ConsumerAttrBuilder->>ConsumerAttrBuilder: validate fields
alt All fields present
ConsumerAttrBuilder->>ConsumerAttr: build ConsumerAttr instance
ConsumerAttr-->>User: return ConsumerAttr
else Missing field(s)
ConsumerAttrBuilder-->>User: return ConsumerAttrError
end
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (9)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
🔊@mxsm 🚀Thanks for your contribution🎉！ 💡CodeRabbit(AI) will review your code first🔥！ Note 🚨The code review suggestions from CodeRabbit are to be used as a reference only, and the PR submitter can decide whether to make changes based on their own judgment. Ultimately, the project management personnel will conduct the final code review💥. |
rocketmq-rust-bot
added
auto merge
ready to review
waiting-review
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new metrics module to support consumer metrics tracking for RocketMQ Broker by adding a ConsumerAttr struct and its associated builder.
- Added a new file (metrics.rs) which includes the consumer_attr submodule for metrics tracking.
- Modified the library's module exposure by including the new metrics module in lib.rs.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| rocketmq-broker/src/metrics.rs | New metrics module setup with consumer_attr |
| rocketmq-broker/src/lib.rs | Exposes the newly added metrics module |
Comments suppressed due to low confidence (1)
rocketmq-broker/src/metrics.rs:17
- Consider adding module-level documentation for 'consumer_attr' to outline its purpose and usage for metrics tracking.
mod consumer_attr;
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
rocketmq-broker/src/metrics.rs (1)
17-18: Consider tightening the module’s visibility or adding a re-export
mod consumer_attr;keeps the sub-module private tocrate::metrics.
If other crates / modules are expected to useConsumerAttrdirectly you either needpub use consumer_attr::ConsumerAttr;or to mark the sub-module
pub(crate).
Decide now to avoid a follow-up public-API change.rocketmq-broker/src/metrics/consumer_attr.rs (1)
121-142: ManualHashimpl can be replaced by#[derive(Hash)]All four fields already implement
Hash, so a custom implementation is unnecessary boilerplate and risks divergence if the struct changes.-#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize, Hash)] … -impl Hash for ConsumerAttr { - fn hash<H: Hasher>(&self, state: &mut H) { - self.group.hash(state); - self.language.hash(state); - self.version.hash(state); - self.consume_mode.hash(state); - } -} +// Custom impl no longer required
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
rocketmq-broker/src/lib.rs(1 hunks)rocketmq-broker/src/metrics.rs(1 hunks)rocketmq-broker/src/metrics/consumer_attr.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: build (ubuntu-latest, stable)
- GitHub Check: build (macos-latest, stable)
- GitHub Check: build (windows-latest, stable)
- GitHub Check: build (macos-latest, nightly)
- GitHub Check: auto-approve
- GitHub Check: build (ubuntu-latest, nightly)
- GitHub Check: build (windows-latest, nightly)
- GitHub Check: build
- GitHub Check: test
🔇 Additional comments (1)
rocketmq-broker/src/lib.rs (1)
42-42: Nice integration pointThe new
metricsmodule is wired into the crate root in the same style as the existing internal modules. No concerns here.
…and string representation
|
🔊@mxsm 🚀Thanks for your contribution🎉！ 💡CodeRabbit(AI) will review your code first🔥！ Note 🚨The code review suggestions from CodeRabbit are to be used as a reference only, and the PR submitter can decide whether to make changes based on their own judgment. Ultimately, the project management personnel will conduct the final code review💥. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3453 +/- ##
==========================================
+ Coverage 25.84% 26.09% +0.24%
==========================================
Files 542 543 +1
Lines 77055 77329 +274
==========================================
+ Hits 19912 20176 +264
- Misses 57143 57153 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
rocketmq-rust-bot
added
approved
Which Issue(s) This PR Fixes(Closes)
Fixes #3452
Brief Description
How Did You Test This Change?
Summary by CodeRabbit
New Features
Tests