🥅 server: bind account address to sentry instrumentation scopes#832
🥅 server: bind account address to sentry instrumentation scopes#832cruzdanilo merged 1 commit intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 6d5d477 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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughBinds account addresses into Sentry scopes across server hooks and utilities by adding withScope/setUser calls, replacing per-operation tags, adjusting error capture to include user context, and updating tests and a changeset for a patch release of Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
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 server's Sentry integration by ensuring that the relevant account address is consistently associated with Sentry spans and error reports. This provides crucial user context for debugging and monitoring, allowing for quicker identification and resolution of account-specific issues. The changes involve updating Sentry calls in activity, block, and panda hooks, as well as the keeper utility, alongside corresponding test updates. Highlights
Changelog
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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #832 +/- ##
==========================================
+ Coverage 69.09% 69.51% +0.41%
==========================================
Files 211 211
Lines 7503 7662 +159
Branches 2406 2471 +65
==========================================
+ Hits 5184 5326 +142
- Misses 2098 2113 +15
- Partials 221 223 +2
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:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/test/utils/keeper.test.ts (1)
9-9:⚠️ Potential issue | 🟠 MajorAdd missing
beforeEachimport from vitest.
beforeEachis used on line 340 but not imported, causing a compile error.Suggested fix
-import { afterEach, describe, expect, inject, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, inject, it, vi } from "vitest";
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
.changeset/weak-dove-play.mdserver/hooks/activity.tsserver/hooks/block.tsserver/hooks/panda.tsserver/test/hooks/activity.test.tsserver/test/hooks/block.test.tsserver/test/utils/keeper.test.tsserver/utils/keeper.ts
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
♻️ Duplicate comments (4)
server/test/utils/keeper.test.ts (1)
353-367:⚠️ Potential issue | 🟡 MinorType the setUser spy to avoid implicit any.
vi.fn()defaults toany, which weakens the test type safety and can drift from Sentry’s signature. Consider typing it to the scope method directly.♻️ Suggested fix
- const setUser = vi.fn(); + const setUser = vi.fn<Parameters<Parameters<typeof withScope>[0]>[0]["setUser"]>();What is the TypeScript signature of Scope#setUser in `@sentry/node` v10.35.0?As per coding guidelines "Avoid using the
anytype; leverage TypeScript's type system for type safety".server/test/hooks/block.test.ts (1)
2220-2234:⚠️ Potential issue | 🟡 MinorType the setUser spy to avoid implicit any.
vi.fn()defaults toany, which weakens the test type safety and can drift from Sentry’s signature. Consider typing it to the scope method directly.♻️ Suggested fix
- const setUser = vi.fn(); + const setUser = vi.fn<Parameters<Parameters<typeof withScope>[0]>[0]["setUser"]>();What is the TypeScript signature of Scope#setUser in `@sentry/node` v10.35.0?As per coding guidelines "Avoid using the
anytype; leverage TypeScript's type system for type safety".server/hooks/activity.ts (2)
112-112: 🧹 Nitpick | 🔵 TrivialAvoid calling Object.keys twice on the same value.
This repeats the computation on a single line and violates the guideline that values used in multiple places should be stored once.
♻️ Suggested fix
- if (Object.keys(accounts).length === 1) setUser({ id: v.parse(Address, Object.keys(accounts)[0]) }); + const accountIds = Object.keys(accounts); + if (accountIds.length === 1) setUser({ id: v.parse(Address, accountIds[0]) });As per coding guidelines "Do not extract a value into a variable or logic into a function unless it is used in two or more places; keep single-use values and functions inline".
139-143: 🧹 Nitpick | 🔵 TrivialPass trace data as an object instead of destructuring fields.
You only destructure to rewrap the same fields. Keeping the object avoids a guideline violation and is more direct.
♻️ Suggested fix
- const { "sentry-trace": sentryTrace, baggage } = getTraceData(); + const traceData = getTraceData(); @@ - continueTrace({ sentryTrace, baggage }, () => + continueTrace(traceData, () =>As per coding guidelines "Do not destructure object fields only to pass them individually; use the object directly (e.g.,
await db.insert(accounts).values({ id: crypto.randomUUID(), email: c.req.valid("json").email }))".
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
.changeset/weak-dove-play.mdserver/hooks/activity.tsserver/hooks/block.tsserver/hooks/panda.tsserver/test/hooks/activity.test.tsserver/test/hooks/block.test.tsserver/test/utils/keeper.test.tsserver/utils/keeper.ts
There was a problem hiding this comment.
♻️ Duplicate comments (2)
server/test/utils/keeper.test.ts (1)
5-5:⚠️ Potential issue | 🟡 MinorType the setUser spy to match Sentry’s Scope#setUser signature.
Untyped
vi.fn()defaults toanyand hides theUser | nullparameter /Scopereturn.🔧 Proposed fix
- const setUser = vi.fn(); + const setUser = vi.fn<Parameters<Parameters<typeof withScope>[0]>[0]["setUser"]>();What is the TypeScript signature of Scope#setUser in `@sentry/node` v10?As per coding guidelines "Avoid using the
anytype; leverage TypeScript's type system for type safety".Also applies to: 16-16, 353-366
server/test/hooks/block.test.ts (1)
7-7:⚠️ Potential issue | 🟡 MinorType the setUser spy to match Sentry’s Scope#setUser signature.
Untyped
vi.fn()defaults toanyand hides theUser | nullparameter /Scopereturn.🔧 Proposed fix
- const setUser = vi.fn(); + const setUser = vi.fn<Parameters<Parameters<typeof withScope>[0]>[0]["setUser"]>();What is the TypeScript signature of Scope#setUser in `@sentry/node` v10?As per coding guidelines "Avoid using the
anytype; leverage TypeScript's type system for type safety".Also applies to: 59-59, 2220-2233
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
.changeset/weak-dove-play.mdserver/hooks/activity.tsserver/hooks/block.tsserver/hooks/panda.tsserver/test/hooks/activity.test.tsserver/test/hooks/block.test.tsserver/test/utils/keeper.test.tsserver/utils/keeper.ts
1 participant
Summary by CodeRabbit
Improvements
Bug Fixes
Tests
Chores
Greptile Summary
improved sentry error tracking by consistently binding account addresses to instrumentation scopes across all per-account server operations. previously, account context was inconsistently attached via tags; now user identity is set uniformly via
setUser, enabling account-scoped diagnostics in sentry.key changes
server/utils/keeper.ts: automatically extracts and validates account from span attributes, sets user context for all keeper operationsserver/hooks/activity.ts: sets user context at request level (single account) and per-account operation level, adds user binding in error capture scopes, removes redundantexa.accounttagserver/hooks/block.ts: wraps async operations with user context binding for proposals and withdrawals, ensures account context in error handlersserver/hooks/panda.ts: reorders validation to set user context before throwing errorsConfidence Score: 5/5
activity.tssimplifies status logic without changing behaviorImportant Files Changed
exa.accounttag, refactors status logicexa.accounttagLast reviewed commit: 6d5d477