Skip to content

Preserve RabbitMQ topic names in queue stats#1420

Open
abishekgiri wants to merge 2 commits into
iii-hq:mainfrom
abishekgiri:rabbitmq-topic-name-parsing
Open

Preserve RabbitMQ topic names in queue stats#1420
abishekgiri wants to merge 2 commits into
iii-hq:mainfrom
abishekgiri:rabbitmq-topic-name-parsing

Conversation

@abishekgiri

@abishekgiri abishekgiri commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • preserve the full topic name when RabbitMQ subscription keys are parsed for topic stats and topic listings
  • switch the parsing logic to split from the right so topics containing colons are counted under the correct name
  • add a regression test for a topic like orders:v1

Testing

  • cargo test -p iii subscription_topic_preserves_topics_with_colons -- --nocapture

Summary by CodeRabbit

  • Refactor

    • Improved internal topic handling logic in message queue subscription processing for greater accuracy when extracting topic information.
  • Tests

    • Added unit tests to verify topic extraction functionality handles complex topic names correctly.

@vercel

vercel Bot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

@abishekgiri is attempting to deploy a commit to the motia Team on Vercel.

A member of the Team first needs to authorize it.

@abishekgiri

Copy link
Copy Markdown
Contributor Author

Updated RabbitMQ topic reporting so subscription keys are parsed from the right instead of the left. That keeps topic stats and list-topics output correct when the topic name itself contains colons, and it adds a regression test for that shape.

@abishekgiri abishekgiri marked this pull request as ready for review April 7, 2026 00:41
@abishekgiri abishekgiri marked this pull request as draft April 7, 2026 00:43
@abishekgiri abishekgiri marked this pull request as ready for review April 7, 2026 00:57
@coderabbitai

coderabbitai Bot commented Apr 10, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

A helper function subscription_topic() was added to extract topic names from keys formatted as topic:id, handling topics that contain colons. Consumer counting and topic aggregation logic in RabbitMQ adapter were updated to use this function, with unit tests added for verification.

Changes

Cohort / File(s) Summary
RabbitMQ Topic Extraction
engine/src/workers/queue/adapters/rabbitmq/adapter.rs
Added subscription_topic helper function to extract topic from keys by taking substring before last colon, updated topic_stats and list_topics to use it for proper topic matching when topics themselves contain colons, with unit tests validating behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A topic hidden in a key, now revealed with clarity,
Before the final colon it does dance,
With colons stacked like carrots in a row,
My burrow of exchanges now gleams so fair,
One hop, two hops, the subscriptions play! 🎯

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'Preserve RabbitMQ topic names in queue stats' accurately reflects the main change: fixing topic name parsing in RabbitMQ subscription key handling to correctly preserve topic names containing colons in stats and listings.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
engine/src/workers/queue/adapters/rabbitmq/adapter.rs (1)

1238-1244: Add a regression test for colon-containing subscriber IDs.

Current test covers colons in topic only. Add one case where the suffix id includes : to lock this edge case.

🧪 Suggested test case
 #[test]
 fn subscription_topic_preserves_topics_with_colons() {
     assert_eq!(
         subscription_topic("orders:v1:123e4567-e89b-12d3-a456-426614174000"),
         Some("orders:v1")
     );
 }
+
+#[test]
+fn subscription_topic_handles_colons_in_subscriber_id() {
+    assert_eq!(
+        subscription_topic("orders:v1:fn::validate"),
+        Some("orders:v1")
+    );
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@engine/src/workers/queue/adapters/rabbitmq/adapter.rs` around lines 1238 -
1244, Add a regression test that ensures subscription_topic correctly strips the
suffix even when the subscriber id contains a colon — e.g., add a new test
(e.g., subscription_topic_handles_colon_in_suffix) that calls subscription_topic
with a value where the part after the second colon itself contains a colon (for
example "orders:v1:123e4567-e89b:12d3-a456-426614174000") and asserts it still
returns Some("orders:v1"); reference the existing subscription_topic function
and the current subscription_topic_preserves_topics_with_colons test to place
and model the new test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@engine/src/workers/queue/adapters/rabbitmq/adapter.rs`:
- Around line 1238-1244: Add a regression test that ensures subscription_topic
correctly strips the suffix even when the subscriber id contains a colon — e.g.,
add a new test (e.g., subscription_topic_handles_colon_in_suffix) that calls
subscription_topic with a value where the part after the second colon itself
contains a colon (for example "orders:v1:123e4567-e89b:12d3-a456-426614174000")
and asserts it still returns Some("orders:v1"); reference the existing
subscription_topic function and the current
subscription_topic_preserves_topics_with_colons test to place and model the new
test.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3847ed13-2fe5-4c96-8fee-fb9dd9e9bd81

📥 Commits

Reviewing files that changed from the base of the PR and between 7de5e15 and 53afd73.

📒 Files selected for processing (1)
  • engine/src/workers/queue/adapters/rabbitmq/adapter.rs

@anthonyiscoding

anthonyiscoding commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Thanks @abishekgiri The right-to-left split approach makes sense for topic names with colons. One question though if subscriber IDs ever contain colons will this splitting always work correctly? (ex. orders:v1:fn::validate --> orders:v1:fn:)

@sergiofilhowz Do you have thoughts here? I'm not very familiar with RabbitMQ and our colons are more of a suggested design choice for paths than an enforced standard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants