-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(browser): Migrate spotlight event processor to ignoreSpans
#20595
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| import * as Sentry from '@sentry/browser'; | ||
|
|
||
| window.Sentry = Sentry; | ||
|
|
||
| Sentry.init({ | ||
| dsn: 'https://public@dsn.ingest.sentry.io/1337', | ||
| integrations: [ | ||
| Sentry.browserTracingIntegration({ | ||
| enableLongTask: false, | ||
| _experiments: { | ||
| enableInteractions: true, | ||
| }, | ||
| }), | ||
| Sentry.spanStreamingIntegration(), | ||
| Sentry.spotlightBrowserIntegration(), | ||
| ], | ||
| tracesSampleRate: 1, | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| // Block the main thread for 70ms so the PerformanceObserver registers | ||
| // a click event entry, which triggers `ui.interaction.click` child spans. | ||
| const simulateSlowClick = e => { | ||
| const startTime = Date.now(); | ||
| while (Date.now() - startTime < 70) { | ||
| // | ||
| } | ||
| e.target.classList.add('clicked'); | ||
| }; | ||
|
|
||
| document.querySelector('[data-test-id=spotlight-button]').addEventListener('click', simulateSlowClick); | ||
| document.querySelector('[data-test-id=regular-button]').addEventListener('click', simulateSlowClick); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| <!doctype html> | ||
| <html> | ||
| <head> | ||
| <meta charset="utf-8" /> | ||
| </head> | ||
| <body> | ||
| <div id="sentry-spotlight"> | ||
| <button data-test-id="spotlight-button">Spotlight Button</button> | ||
| </div> | ||
| <button data-test-id="regular-button">Regular Button</button> | ||
| </body> | ||
| </html> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| import { expect } from '@playwright/test'; | ||
| import { sentryTest } from '../../../../utils/fixtures'; | ||
| import { shouldSkipCdnBundleTest, shouldSkipTracingTest } from '../../../../utils/helpers'; | ||
| import { getSpanOp, observeStreamedSpan, waitForStreamedSpan, waitForStreamedSpans } from '../../../../utils/spanUtils'; | ||
|
|
||
| sentryTest( | ||
| 'filters ui.interaction.click spans for spotlight elements via ignoreSpans in streaming mode', | ||
| async ({ getLocalTestUrl, page }) => { | ||
| // spotlightBrowserIntegration is not available in CDN bundles | ||
| if (shouldSkipTracingTest() || shouldSkipCdnBundleTest()) { | ||
| sentryTest.skip(); | ||
| } | ||
|
|
||
| const url = await getLocalTestUrl({ testDir: __dirname }); | ||
|
|
||
| // Set up an observer that fails if a spotlight interaction span is ever sent | ||
| let sawSpotlightInteractionSpan = false; | ||
| await observeStreamedSpan(page, span => { | ||
| if (getSpanOp(span) === 'ui.interaction.click' && span.name?.includes('#sentry-spotlight')) { | ||
| sawSpotlightInteractionSpan = true; | ||
| return true; | ||
| } | ||
| return false; | ||
| }); | ||
|
|
||
| await page.goto(url); | ||
|
|
||
| // Wait for pageload to finish before clicking | ||
| await waitForStreamedSpan(page, span => getSpanOp(span) === 'pageload'); | ||
|
|
||
| // Click on the spotlight element — its ui.interaction.click child should be filtered | ||
| await page.locator('[data-test-id=spotlight-button]').click(); | ||
| await page.locator('.clicked[data-test-id=spotlight-button]').isVisible(); | ||
|
|
||
| // Wait for the spotlight click's segment span to arrive | ||
| await waitForStreamedSpans(page, spans => | ||
| spans.some(span => span.is_segment && getSpanOp(span) === 'ui.action.click'), | ||
| ); | ||
|
|
||
| // Click on the regular button — its ui.interaction.click child should be kept | ||
| const regularInteractionSpansPromise = waitForStreamedSpans(page, spans => | ||
| spans.some(span => getSpanOp(span) === 'ui.interaction.click' && !span.name?.includes('#sentry-spotlight')), | ||
| ); | ||
|
|
||
| await page.locator('[data-test-id=regular-button]').click(); | ||
| await page.locator('.clicked[data-test-id=regular-button]').isVisible(); | ||
|
|
||
| const regularSpans = await regularInteractionSpansPromise; | ||
| const regularInteractionSpan = regularSpans.find( | ||
| span => getSpanOp(span) === 'ui.interaction.click' && !span.name?.includes('#sentry-spotlight'), | ||
| ); | ||
| expect(regularInteractionSpan).toBeDefined(); | ||
| expect(regularInteractionSpan!.name).toContain('button'); | ||
|
|
||
| // Verify no spotlight interaction span was ever sent | ||
| expect(sawSpotlightInteractionSpan).toBe(false); | ||
| }, | ||
| ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import * as Sentry from '@sentry/browser'; | ||
|
|
||
| window.Sentry = Sentry; | ||
|
|
||
| Sentry.init({ | ||
| dsn: 'https://public@dsn.ingest.sentry.io/1337', | ||
| integrations: [ | ||
| Sentry.browserTracingIntegration({ | ||
| enableLongTask: false, | ||
| _experiments: { | ||
| enableInteractions: true, | ||
| }, | ||
| }), | ||
| Sentry.spotlightBrowserIntegration(), | ||
| ], | ||
| tracesSampleRate: 1, | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| // Block the main thread for 70ms so the PerformanceObserver registers | ||
| // a click event entry, which triggers `ui.interaction.click` child spans. | ||
| const simulateSlowClick = e => { | ||
| const startTime = Date.now(); | ||
| while (Date.now() - startTime < 70) { | ||
| // | ||
| } | ||
| e.target.classList.add('clicked'); | ||
| }; | ||
|
|
||
| document.querySelector('[data-test-id=spotlight-button]').addEventListener('click', simulateSlowClick); | ||
| document.querySelector('[data-test-id=regular-button]').addEventListener('click', simulateSlowClick); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| <!doctype html> | ||
| <html> | ||
| <head> | ||
| <meta charset="utf-8" /> | ||
| </head> | ||
| <body> | ||
| <div id="sentry-spotlight"> | ||
| <button data-test-id="spotlight-button">Spotlight Button</button> | ||
| </div> | ||
| <button data-test-id="regular-button">Regular Button</button> | ||
| </body> | ||
| </html> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| import { expect } from '@playwright/test'; | ||
| import type { TransactionEvent } from '@sentry/core'; | ||
| import { sentryTest } from '../../../../utils/fixtures'; | ||
| import { | ||
| envelopeRequestParser, | ||
| shouldSkipCdnBundleTest, | ||
| shouldSkipTracingTest, | ||
| waitForTransactionRequest, | ||
| } from '../../../../utils/helpers'; | ||
|
|
||
| sentryTest( | ||
| 'filters ui.interaction.click spans for spotlight elements via ignoreSpans', | ||
| async ({ getLocalTestUrl, page }) => { | ||
| // spotlightBrowserIntegration is not available in CDN bundles | ||
| if (shouldSkipTracingTest() || shouldSkipCdnBundleTest()) { | ||
| sentryTest.skip(); | ||
| } | ||
|
|
||
| const url = await getLocalTestUrl({ testDir: __dirname }); | ||
| await page.goto(url); | ||
|
|
||
| // Wait for the pageload transaction to complete | ||
| await waitForTransactionRequest(page); | ||
|
|
||
| // Click on the spotlight element — interaction span should be filtered | ||
| const spotlightTxnPromise = waitForTransactionRequest(page, txn => txn.contexts?.trace?.op === 'ui.action.click'); | ||
| await page.locator('[data-test-id=spotlight-button]').click(); | ||
| await page.locator('.clicked[data-test-id=spotlight-button]').isVisible(); | ||
| const spotlightTransaction = envelopeRequestParser<TransactionEvent>(await spotlightTxnPromise); | ||
|
|
||
| expect(spotlightTransaction.contexts?.trace?.op).toBe('ui.action.click'); | ||
|
|
||
| const spotlightInteractionSpans = spotlightTransaction.spans?.filter(span => span.op === 'ui.interaction.click'); | ||
| expect(spotlightInteractionSpans).toHaveLength(0); | ||
|
|
||
| // Let the first idle span fully settle before clicking again | ||
| await page.waitForTimeout(1000); | ||
|
chargome marked this conversation as resolved.
|
||
|
|
||
| // Click on the regular button — wait specifically for a transaction that contains | ||
| // a ui.interaction.click child span, since the PerformanceObserver may deliver | ||
| // the event entry asynchronously | ||
| const regularTxnPromise = waitForTransactionRequest( | ||
| page, | ||
| txn => | ||
| txn.contexts?.trace?.op === 'ui.action.click' && | ||
| (txn.spans?.some(span => span.op === 'ui.interaction.click') ?? false), | ||
| ); | ||
| await page.locator('[data-test-id=regular-button]').click(); | ||
| await page.locator('.clicked[data-test-id=regular-button]').isVisible(); | ||
| const regularTransaction = envelopeRequestParser<TransactionEvent>(await regularTxnPromise); | ||
|
|
||
| const regularInteractionSpans = regularTransaction.spans?.filter(span => span.op === 'ui.interaction.click'); | ||
| expect(regularInteractionSpans?.length).toBeGreaterThanOrEqual(1); | ||
| expect(regularInteractionSpans![0]!.description).toContain('button'); | ||
| expect(regularInteractionSpans![0]!.description).not.toContain('#sentry-spotlight'); | ||
| }, | ||
| ); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import type { Client, Envelope, Event, IntegrationFn } from '@sentry/core'; | ||
| import type { Client, Envelope, IntegrationFn } from '@sentry/core'; | ||
| import { debug, defineIntegration, serializeEnvelope } from '@sentry/core'; | ||
| import { getNativeImplementation } from '@sentry-internal/browser-utils'; | ||
| import { DEBUG_BUILD } from '../debug-build'; | ||
|
|
@@ -14,6 +14,8 @@ export type SpotlightConnectionOptions = { | |
|
|
||
| export const INTEGRATION_NAME = 'SpotlightBrowser'; | ||
|
|
||
| export const SPOTLIGHT_IGNORE_SPANS = [{ op: 'ui.interaction.click', name: '#sentry-spotlight' }]; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. h: Before we dropped the whole transaction if any of these spans were detected, this behaves differently now since this will only match non-segment spans as far as I understand. Should we try to detect the segment span instead (so that then the whole segment gets dropped if it matches)?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A tradeoff yes, there's no ignoreSpans pattern that can match the segment span to drop the whole thing
|
||
|
|
||
| const _spotlightIntegration = ((options: Partial<SpotlightConnectionOptions> = {}) => { | ||
| const sidecarUrl = options.sidecarUrl || 'http://localhost:8969/stream'; | ||
|
|
||
|
|
@@ -22,10 +24,10 @@ const _spotlightIntegration = ((options: Partial<SpotlightConnectionOptions> = { | |
| setup: () => { | ||
| DEBUG_BUILD && debug.log('Using Sidecar URL', sidecarUrl); | ||
| }, | ||
| // We don't want to send interaction transactions/root spans created from | ||
| // clicks within Spotlight to Sentry. Neither do we want them to be sent to | ||
| // spotlight. | ||
| processEvent: event => (isSpotlightInteraction(event) ? null : event), | ||
| beforeSetup(client: Client) { | ||
| const opts = client.getOptions(); | ||
| opts.ignoreSpans = [...(opts.ignoreSpans || []), ...SPOTLIGHT_IGNORE_SPANS]; | ||
| }, | ||
| afterAllSetup: (client: Client) => { | ||
| setupSidecarForwarding(client, sidecarUrl); | ||
| }, | ||
|
|
@@ -73,16 +75,3 @@ function setupSidecarForwarding(client: Client, sidecarUrl: string): void { | |
| * Learn more about spotlight at https://spotlightjs.com | ||
| */ | ||
| export const spotlightBrowserIntegration = defineIntegration(_spotlightIntegration); | ||
|
|
||
| /** | ||
| * Flags if the event is a transaction created from an interaction with the spotlight UI. | ||
| */ | ||
| export function isSpotlightInteraction(event: Event): boolean { | ||
| return Boolean( | ||
| event.type === 'transaction' && | ||
| event.spans && | ||
| event.contexts?.trace && | ||
| event.contexts.trace.op === 'ui.action.click' && | ||
| event.spans.some(({ description }) => description?.includes('#sentry-spotlight')), | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| import type { Client, ClientOptions } from '@sentry/core'; | ||
| import { shouldIgnoreSpan } from '@sentry/core'; | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { SPOTLIGHT_IGNORE_SPANS, spotlightBrowserIntegration } from '../../src/integrations/spotlight'; | ||
|
|
||
| function makeMockClient(initial: Partial<ClientOptions> = {}): Client { | ||
| const options = { ...initial } as ClientOptions; | ||
| return { getOptions: () => options } as Client; | ||
| } | ||
|
|
||
| function setupIntegrationAndGetIgnoreSpans(initial: Partial<ClientOptions> = {}) { | ||
| const integration = spotlightBrowserIntegration(); | ||
| const client = makeMockClient(initial); | ||
| integration.beforeSetup!(client); | ||
| return client.getOptions().ignoreSpans!; | ||
| } | ||
|
|
||
| describe('spotlightBrowserIntegration', () => { | ||
| it('appends spotlight interaction filters to ignoreSpans', () => { | ||
| expect(setupIntegrationAndGetIgnoreSpans()).toEqual(SPOTLIGHT_IGNORE_SPANS); | ||
| }); | ||
|
|
||
| it('preserves user-provided ignoreSpans entries', () => { | ||
| expect(setupIntegrationAndGetIgnoreSpans({ ignoreSpans: [/keep-me/] })).toEqual([ | ||
| /keep-me/, | ||
| ...SPOTLIGHT_IGNORE_SPANS, | ||
| ]); | ||
| }); | ||
|
|
||
| describe('drops spotlight interaction spans', () => { | ||
| it.each([ | ||
| ['click on spotlight overlay', 'body > div#sentry-spotlight > div.overlay'], | ||
| ['click on spotlight button', 'body > div > div#sentry-spotlight > button.close'], | ||
| ['click on nested spotlight element', 'html > body > aside#sentry-spotlight'], | ||
| ])('%s', (_label, name) => { | ||
| const ignoreSpans = setupIntegrationAndGetIgnoreSpans(); | ||
| expect(shouldIgnoreSpan({ description: name, op: 'ui.interaction.click' }, ignoreSpans)).toBe(true); | ||
| }); | ||
| }); | ||
|
|
||
| describe('keeps non-spotlight interaction spans', () => { | ||
| it.each([ | ||
| ['regular click', 'body > div.main > button.submit', 'ui.interaction.click'], | ||
| ['regular ui action', '/dashboard', 'ui.action.click'], | ||
| ['non-interaction span', 'GET /api/data', 'http.client'], | ||
| ])('%s', (_label, name, op) => { | ||
| const ignoreSpans = setupIntegrationAndGetIgnoreSpans(); | ||
| expect(shouldIgnoreSpan({ description: name, op }, ignoreSpans)).toBe(false); | ||
| }); | ||
| }); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.