Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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';
Comment thread
cursor[bot] marked this conversation as resolved.

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);
Comment thread
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');
},
);
8 changes: 8 additions & 0 deletions dev-packages/browser-integration-tests/utils/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,14 @@ export function shouldSkipFeedbackTest(): boolean {
* @returns `true` if we should skip the feature flags test
*/
export function shouldSkipFeatureFlagsTest(): boolean {
return shouldSkipCdnBundleTest();
}

/**
* Returns true if we're running in a CDN bundle environment (not ESM/CJS).
* Use this to skip tests for integrations that are only available via npm, not CDN bundles.
*/
export function shouldSkipCdnBundleTest(): boolean {
const bundle = process.env.PW_BUNDLE;
return bundle != null && !bundle.includes('esm') && !bundle.includes('cjs');
}
Expand Down
25 changes: 7 additions & 18 deletions packages/browser/src/integrations/spotlight.ts
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';
Expand All @@ -14,6 +14,8 @@ export type SpotlightConnectionOptions = {

export const INTEGRATION_NAME = 'SpotlightBrowser';

export const SPOTLIGHT_IGNORE_SPANS = [{ op: 'ui.interaction.click', name: '#sentry-spotlight' }];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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)?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The 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

Filtering entire segments based on data that is unknown prior to segment start.


const _spotlightIntegration = ((options: Partial<SpotlightConnectionOptions> = {}) => {
const sidecarUrl = options.sidecarUrl || 'http://localhost:8969/stream';

Expand All @@ -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);
},
Expand Down Expand Up @@ -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')),
);
}
51 changes: 51 additions & 0 deletions packages/browser/test/integrations/spotlight.test.ts
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);
});
});
});
Loading