Skip to content

Commit ae8ff88

Browse files
chargomechargome
authored
feat(browser): Migrate spotlight event processor to ignoreSpans (#20595)
Added some integration tests too, might be an overkill but they are quite simple. closes #20363
1 parent d58f82a commit ae8ff88

11 files changed

Lines changed: 264 additions & 18 deletions

File tree

‎dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/spotlight-interaction-filter-streamed/init.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
7+
integrations: [
8+
Sentry.browserTracingIntegration({
9+
enableLongTask: false,
10+
_experiments: {
11+
enableInteractions: true,
12+
},
13+
}),
14+
Sentry.spanStreamingIntegration(),
15+
Sentry.spotlightBrowserIntegration(),
16+
],
17+
tracesSampleRate: 1,
18+
});

‎dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/spotlight-interaction-filter-streamed/subject.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Block the main thread for 70ms so the PerformanceObserver registers
2+
// a click event entry, which triggers `ui.interaction.click` child spans.
3+
const simulateSlowClick = e => {
4+
const startTime = Date.now();
5+
while (Date.now() - startTime < 70) {
6+
//
7+
}
8+
e.target.classList.add('clicked');
9+
};
10+
11+
document.querySelector('[data-test-id=spotlight-button]').addEventListener('click', simulateSlowClick);
12+
document.querySelector('[data-test-id=regular-button]').addEventListener('click', simulateSlowClick);

‎dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/spotlight-interaction-filter-streamed/template.html‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<!doctype html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8" />
5+
</head>
6+
<body>
7+
<div id="sentry-spotlight">
8+
<button data-test-id="spotlight-button">Spotlight Button</button>
9+
</div>
10+
<button data-test-id="regular-button">Regular Button</button>
11+
</body>
12+
</html>

‎dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/spotlight-interaction-filter-streamed/test.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import { expect } from '@playwright/test';
2+
import { sentryTest } from '../../../../utils/fixtures';
3+
import { shouldSkipCdnBundleTest, shouldSkipTracingTest } from '../../../../utils/helpers';
4+
import { getSpanOp, observeStreamedSpan, waitForStreamedSpan, waitForStreamedSpans } from '../../../../utils/spanUtils';
5+
6+
sentryTest(
7+
'filters ui.interaction.click spans for spotlight elements via ignoreSpans in streaming mode',
8+
async ({ getLocalTestUrl, page }) => {
9+
// spotlightBrowserIntegration is not available in CDN bundles
10+
if (shouldSkipTracingTest() || shouldSkipCdnBundleTest()) {
11+
sentryTest.skip();
12+
}
13+
14+
const url = await getLocalTestUrl({ testDir: __dirname });
15+
16+
// Set up an observer that fails if a spotlight interaction span is ever sent
17+
let sawSpotlightInteractionSpan = false;
18+
await observeStreamedSpan(page, span => {
19+
if (getSpanOp(span) === 'ui.interaction.click' && span.name?.includes('#sentry-spotlight')) {
20+
sawSpotlightInteractionSpan = true;
21+
return true;
22+
}
23+
return false;
24+
});
25+
26+
await page.goto(url);
27+
28+
// Wait for pageload to finish before clicking
29+
await waitForStreamedSpan(page, span => getSpanOp(span) === 'pageload');
30+
31+
// Click on the spotlight element — its ui.interaction.click child should be filtered
32+
await page.locator('[data-test-id=spotlight-button]').click();
33+
await page.locator('.clicked[data-test-id=spotlight-button]').isVisible();
34+
35+
// Wait for the spotlight click's segment span to arrive
36+
await waitForStreamedSpans(page, spans =>
37+
spans.some(span => span.is_segment && getSpanOp(span) === 'ui.action.click'),
38+
);
39+
40+
// Click on the regular button — its ui.interaction.click child should be kept
41+
const regularInteractionSpansPromise = waitForStreamedSpans(page, spans =>
42+
spans.some(span => getSpanOp(span) === 'ui.interaction.click' && !span.name?.includes('#sentry-spotlight')),
43+
);
44+
45+
await page.locator('[data-test-id=regular-button]').click();
46+
await page.locator('.clicked[data-test-id=regular-button]').isVisible();
47+
48+
const regularSpans = await regularInteractionSpansPromise;
49+
const regularInteractionSpan = regularSpans.find(
50+
span => getSpanOp(span) === 'ui.interaction.click' && !span.name?.includes('#sentry-spotlight'),
51+
);
52+
expect(regularInteractionSpan).toBeDefined();
53+
expect(regularInteractionSpan!.name).toContain('button');
54+
55+
// Verify no spotlight interaction span was ever sent
56+
expect(sawSpotlightInteractionSpan).toBe(false);
57+
},
58+
);

‎dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/spotlight-interaction-filter/init.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
7+
integrations: [
8+
Sentry.browserTracingIntegration({
9+
enableLongTask: false,
10+
_experiments: {
11+
enableInteractions: true,
12+
},
13+
}),
14+
Sentry.spotlightBrowserIntegration(),
15+
],
16+
tracesSampleRate: 1,
17+
});

‎dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/spotlight-interaction-filter/subject.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Block the main thread for 70ms so the PerformanceObserver registers
2+
// a click event entry, which triggers `ui.interaction.click` child spans.
3+
const simulateSlowClick = e => {
4+
const startTime = Date.now();
5+
while (Date.now() - startTime < 70) {
6+
//
7+
}
8+
e.target.classList.add('clicked');
9+
};
10+
11+
document.querySelector('[data-test-id=spotlight-button]').addEventListener('click', simulateSlowClick);
12+
document.querySelector('[data-test-id=regular-button]').addEventListener('click', simulateSlowClick);

‎dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/spotlight-interaction-filter/template.html‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<!doctype html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8" />
5+
</head>
6+
<body>
7+
<div id="sentry-spotlight">
8+
<button data-test-id="spotlight-button">Spotlight Button</button>
9+
</div>
10+
<button data-test-id="regular-button">Regular Button</button>
11+
</body>
12+
</html>

‎dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/spotlight-interaction-filter/test.ts‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import { expect } from '@playwright/test';
2+
import type { TransactionEvent } from '@sentry/core';
3+
import { sentryTest } from '../../../../utils/fixtures';
4+
import {
5+
envelopeRequestParser,
6+
shouldSkipCdnBundleTest,
7+
shouldSkipTracingTest,
8+
waitForTransactionRequest,
9+
} from '../../../../utils/helpers';
10+
11+
sentryTest(
12+
'filters ui.interaction.click spans for spotlight elements via ignoreSpans',
13+
async ({ getLocalTestUrl, page }) => {
14+
// spotlightBrowserIntegration is not available in CDN bundles
15+
if (shouldSkipTracingTest() || shouldSkipCdnBundleTest()) {
16+
sentryTest.skip();
17+
}
18+
19+
const url = await getLocalTestUrl({ testDir: __dirname });
20+
await page.goto(url);
21+
22+
// Wait for the pageload transaction to complete
23+
await waitForTransactionRequest(page);
24+
25+
// Click on the spotlight element — interaction span should be filtered
26+
const spotlightTxnPromise = waitForTransactionRequest(page, txn => txn.contexts?.trace?.op === 'ui.action.click');
27+
await page.locator('[data-test-id=spotlight-button]').click();
28+
await page.locator('.clicked[data-test-id=spotlight-button]').isVisible();
29+
const spotlightTransaction = envelopeRequestParser<TransactionEvent>(await spotlightTxnPromise);
30+
31+
expect(spotlightTransaction.contexts?.trace?.op).toBe('ui.action.click');
32+
33+
const spotlightInteractionSpans = spotlightTransaction.spans?.filter(span => span.op === 'ui.interaction.click');
34+
expect(spotlightInteractionSpans).toHaveLength(0);
35+
36+
// Let the first idle span fully settle before clicking again
37+
await page.waitForTimeout(1000);
38+
39+
// Click on the regular button — wait specifically for a transaction that contains
40+
// a ui.interaction.click child span, since the PerformanceObserver may deliver
41+
// the event entry asynchronously
42+
const regularTxnPromise = waitForTransactionRequest(
43+
page,
44+
txn =>
45+
txn.contexts?.trace?.op === 'ui.action.click' &&
46+
(txn.spans?.some(span => span.op === 'ui.interaction.click') ?? false),
47+
);
48+
await page.locator('[data-test-id=regular-button]').click();
49+
await page.locator('.clicked[data-test-id=regular-button]').isVisible();
50+
const regularTransaction = envelopeRequestParser<TransactionEvent>(await regularTxnPromise);
51+
52+
const regularInteractionSpans = regularTransaction.spans?.filter(span => span.op === 'ui.interaction.click');
53+
expect(regularInteractionSpans?.length).toBeGreaterThanOrEqual(1);
54+
expect(regularInteractionSpans![0]!.description).toContain('button');
55+
expect(regularInteractionSpans![0]!.description).not.toContain('#sentry-spotlight');
56+
},
57+
);

‎dev-packages/browser-integration-tests/utils/helpers.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,14 @@ export function shouldSkipFeedbackTest(): boolean {
421421
* @returns `true` if we should skip the feature flags test
422422
*/
423423
export function shouldSkipFeatureFlagsTest(): boolean {
424+
return shouldSkipCdnBundleTest();
425+
}
426+
427+
/**
428+
* Returns true if we're running in a CDN bundle environment (not ESM/CJS).
429+
* Use this to skip tests for integrations that are only available via npm, not CDN bundles.
430+
*/
431+
export function shouldSkipCdnBundleTest(): boolean {
424432
const bundle = process.env.PW_BUNDLE;
425433
return bundle != null && !bundle.includes('esm') && !bundle.includes('cjs');
426434
}

‎packages/browser/src/integrations/spotlight.ts‎

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { Client, Envelope, Event, IntegrationFn } from '@sentry/core';
1+
import type { Client, Envelope, IntegrationFn } from '@sentry/core';
22
import { debug, defineIntegration, serializeEnvelope } from '@sentry/core';
33
import { getNativeImplementation } from '@sentry-internal/browser-utils';
44
import { DEBUG_BUILD } from '../debug-build';
@@ -14,6 +14,8 @@ export type SpotlightConnectionOptions = {
1414

1515
export const INTEGRATION_NAME = 'SpotlightBrowser';
1616

17+
export const SPOTLIGHT_IGNORE_SPANS = [{ op: 'ui.interaction.click', name: '#sentry-spotlight' }];
18+
1719
const _spotlightIntegration = ((options: Partial<SpotlightConnectionOptions> = {}) => {
1820
const sidecarUrl = options.sidecarUrl || 'http://localhost:8969/stream';
1921

@@ -22,10 +24,10 @@ const _spotlightIntegration = ((options: Partial<SpotlightConnectionOptions> = {
2224
setup: () => {
2325
DEBUG_BUILD && debug.log('Using Sidecar URL', sidecarUrl);
2426
},
25-
// We don't want to send interaction transactions/root spans created from
26-
// clicks within Spotlight to Sentry. Neither do we want them to be sent to
27-
// spotlight.
28-
processEvent: event => (isSpotlightInteraction(event) ? null : event),
27+
beforeSetup(client: Client) {
28+
const opts = client.getOptions();
29+
opts.ignoreSpans = [...(opts.ignoreSpans || []), ...SPOTLIGHT_IGNORE_SPANS];
30+
},
2931
afterAllSetup: (client: Client) => {
3032
setupSidecarForwarding(client, sidecarUrl);
3133
},
@@ -73,16 +75,3 @@ function setupSidecarForwarding(client: Client, sidecarUrl: string): void {
7375
* Learn more about spotlight at https://spotlightjs.com
7476
*/
7577
export const spotlightBrowserIntegration = defineIntegration(_spotlightIntegration);
76-
77-
/**
78-
* Flags if the event is a transaction created from an interaction with the spotlight UI.
79-
*/
80-
export function isSpotlightInteraction(event: Event): boolean {
81-
return Boolean(
82-
event.type === 'transaction' &&
83-
event.spans &&
84-
event.contexts?.trace &&
85-
event.contexts.trace.op === 'ui.action.click' &&
86-
event.spans.some(({ description }) => description?.includes('#sentry-spotlight')),
87-
);
88-
}

0 commit comments

Comments
 (0)