-
Notifications
You must be signed in to change notification settings - Fork 2
📈 server: enrich proposal queue span instrumentation #715
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 8b9feb6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
PR SummaryMedium Risk Overview Enriches Sentry span instrumentation for Redis-backed proposal/withdraw queues in Adds a Written by Cursor Bugbot for commit 8b9feb6. This will update automatically on new commits. Configure here. |
Summary of ChangesHello @cruzdanilo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Sentry instrumentation for the server's proposal and withdraw queue processing. It introduces more granular tracing details, ensures proper trace context propagation across asynchronous operations, and adds retry count tracking for improved error monitoring and debugging. These changes provide a clearer picture of message lifecycle and potential issues within the queue system. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
WalkthroughAdds per-account OpenTelemetry spans and messaging metadata to proposal and withdrawal scheduling and processing in block hooks; introduces Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Hook as Block Hook
participant Tracer as OpenTelemetry
participant Redis as Redis (message queue)
participant Worker as Proposal/Withdraw Processor
participant Chain as Contract Executor
participant ENS as ENS/Token Resolver
participant Notifier as Notification Sender
Hook->>Tracer: start span (withScope, set user/tag)
Hook->>Redis: enqueue message (withdraw/proposal)
Hook->>Tracer: add messaging.* attrs (system, op, dest, id, body.size)
Redis->>Worker: deliver message
Worker->>Tracer: start processing span (messaging.receive.latency, retryCount)
Worker->>Chain: execute proposal/withdraw
alt success
Chain-->>Worker: tx result
Worker->>ENS: resolve receiver, token metadata
Worker->>Notifier: send notification
Worker->>Tracer: record success attributes
else NotNext error
Chain-->>Worker: NotNext
Worker->>Worker: fetch pending proposals / create idle proposal / reschedule
Worker->>Tracer: record NotNext handling
else ContractFunctionExecutionError
Chain-->>Worker: execution error
Worker->>Worker: adjust nonce / retry logic
Worker->>Tracer: record execution error
else PreExecHookReverted
Chain-->>Worker: PreExecHookReverted
Worker->>Worker: mark aborted, prune queue
Worker->>Tracer: record aborted
end
Worker->>Tracer: end span
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request enhances the Sentry instrumentation for proposal and withdraw queue processing. It introduces a retryCount for better tracking, adds detailed messaging attributes to spans, and refactors the scheduling logic for improved readability and tracing. These changes significantly improve the observability of the queue processing. I have one suggestion to improve error handling in scheduleWithdraw to make it more robust and consistent with similar logic elsewhere in the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #715 +/- ##
===========================================
+ Coverage 44.77% 65.67% +20.90%
===========================================
Files 176 190 +14
Lines 5244 6005 +761
Branches 1624 1734 +110
===========================================
+ Hits 2348 3944 +1596
+ Misses 2783 1881 -902
- Partials 113 180 +67
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
5e45503 to
5644b6e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@server/hooks/block.ts`:
- Around line 439-452: The ENS lookup can throw and will reject the entire
Promise.all, so wrap the ensClient.getEnsName call to never throw (e.g., change
the Promise.all entry to ensClient.getEnsName({ address: receiver }).catch(() =>
null)) so Promise.all still resolves and sendPushNotification runs; reference
the Promise.all invocation, ensClient.getEnsName, and sendPushNotification in
your change to mirror processProposal's behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Summary by CodeRabbit
Bug Fixes
New Features
Chores