Conversation
📝 WalkthroughWalkthroughAdds transaction-span filtering to Sentry client: new utilities detect and remove noisy third-party telemetry spans (e.g., GA, Coinbase) and record filtered counts/keys in event tags/extra; instrumentation-client runs this filter inside beforeSendTransaction and tests cover the behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Instrumentation as InstrumentationClient
participant Filter as SentryClientFilters
participant Sentry as SentrySDK
Client->>Instrumentation: send transaction event
Instrumentation->>Filter: filterNoisyThirdPartyTransactionSpans(event)
Filter-->>Instrumentation: filtered event (spans removed, keys/count)
Instrumentation->>Sentry: sanitize & forward event (with tags/extra)
Sentry-->>Client: capture/submit event
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can customize the tone of the review comments and chat replies.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
__tests__/instrumentation-client.test.ts (1)
175-187: Assertions may be fragile ifsanitizeSentryEventmodifies tags/extra.Lines 185-186 assert that
tagsandextraareundefinedfor events with no spans. However,sanitizeSentryEvent(called in the realbeforeSendTransaction) might add its own tags or extra fields. If the sanitizer's behavior changes, these assertions could fail unexpectedly.Consider verifying the specific filtering-related fields are absent rather than asserting the entire objects are undefined:
♻️ More resilient assertions
const result = beforeSendTransaction(event); expect(result.spans).toBeUndefined(); - expect(result.tags).toBeUndefined(); - expect(result.extra).toBeUndefined(); + expect(result.tags?.["third_party_span_noise_filtered"]).toBeUndefined(); + expect(result.extra?.["filteredThirdPartySpanCount"]).toBeUndefined(); + expect(result.extra?.["filteredThirdPartySpanKeys"]).toBeUndefined();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/instrumentation-client.test.ts` around lines 175 - 187, The test's assertions that result.tags and result.extra are entirely undefined are fragile because sanitizeSentryEvent (used by beforeSendTransaction) may inject its own fields; update the assertions in the "is a no-op when the transaction has no spans" test to only check that the filtering-related fields added by beforeSendTransaction are absent (for example assert that specific keys like the span-filtering tag/extra added by beforeSendTransaction are undefined or that result does not have those properties) rather than requiring tags and extra to be wholly undefined; locate the test using loadBeforeSendTransaction and sanitizeSentryEvent references and replace the broad expect(...).toBeUndefined() checks with targeted checks for the specific keys that beforeSendTransaction would add.instrumentation-client.ts (1)
276-280: Consider removing theas anycast for better type safety.The
as anycast bypasses TypeScript's type checking. SinceSentryTransactionEventextendsSentry.Event, consider using a more precise cast or adjusting the type definitions to avoid losing type safety.♻️ Suggested improvement
beforeSendTransaction(event) { - return sanitizeSentryEvent( - filterNoisyThirdPartyTransactionSpans(event) as any - ); + return sanitizeSentryEvent( + filterNoisyThirdPartyTransactionSpans(event) as Sentry.Event + ); },Also note: The server-side (
sentry.server.config.ts) and edge (sentry.edge.config.ts)beforeSendTransactionhooks don't include this third-party span filtering. This is likely intentional since these noisy spans originate from client-side third-party scripts, but worth confirming if consistency is needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@instrumentation-client.ts` around lines 276 - 280, Replace the unsafe "as any" in beforeSendTransaction by using the correct Sentry transaction type and/or adjusting sanitizeSentryEvent/filterNoisyThirdPartyTransactionSpans signatures: cast the result of filterNoisyThirdPartyTransactionSpans to Sentry.TransactionEvent (or SentryTransactionEvent if that type is defined) instead of any, or change sanitizeSentryEvent to accept Sentry.TransactionEvent; update imports/types accordingly for type compatibility and run TS checks. Also verify whether beforeSendTransaction in sentry.server.config.ts and sentry.edge.config.ts should apply the same filter for consistency and adjust those hooks if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@__tests__/instrumentation-client.test.ts`:
- Around line 175-187: The test's assertions that result.tags and result.extra
are entirely undefined are fragile because sanitizeSentryEvent (used by
beforeSendTransaction) may inject its own fields; update the assertions in the
"is a no-op when the transaction has no spans" test to only check that the
filtering-related fields added by beforeSendTransaction are absent (for example
assert that specific keys like the span-filtering tag/extra added by
beforeSendTransaction are undefined or that result does not have those
properties) rather than requiring tags and extra to be wholly undefined; locate
the test using loadBeforeSendTransaction and sanitizeSentryEvent references and
replace the broad expect(...).toBeUndefined() checks with targeted checks for
the specific keys that beforeSendTransaction would add.
In `@instrumentation-client.ts`:
- Around line 276-280: Replace the unsafe "as any" in beforeSendTransaction by
using the correct Sentry transaction type and/or adjusting
sanitizeSentryEvent/filterNoisyThirdPartyTransactionSpans signatures: cast the
result of filterNoisyThirdPartyTransactionSpans to Sentry.TransactionEvent (or
SentryTransactionEvent if that type is defined) instead of any, or change
sanitizeSentryEvent to accept Sentry.TransactionEvent; update imports/types
accordingly for type compatibility and run TS checks. Also verify whether
beforeSendTransaction in sentry.server.config.ts and sentry.edge.config.ts
should apply the same filter for consistency and adjust those hooks if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 77bc1841-184d-40da-aaf9-40b3afc86ebe
📒 Files selected for processing (4)
__tests__/instrumentation-client.test.ts__tests__/utils/sentry-client-filters.test.tsinstrumentation-client.tsutils/sentry-client-filters.ts
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@instrumentation-client.ts`:
- Around line 278-282: In beforeSendTransaction, remove the unnecessary "as any"
cast on the result of filterNoisyThirdPartyTransactionSpans: call
sanitizeSentryEvent(filterNoisyThirdPartyTransactionSpans(event)) directly since
filterNoisyThirdPartyTransactionSpans already returns Sentry.Event and is
compatible with sanitizeSentryEvent<T extends Event>; update the return
expression inside beforeSendTransaction to drop the cast and rely on the
existing types (sanitizeSentryEvent and filterNoisyThirdPartyTransactionSpans)
to satisfy the compiler.
ragnep
approved these changes
Mar 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Tests