From 38f2efba69fd8860e02d022995eeb13281b8dcb6 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Wed, 27 Aug 2025 12:23:27 +0200 Subject: [PATCH 1/8] test(profiling): Add tests for current state of profiling --- .../suites/profiling/legacyMode/subject.js | 25 +++++ .../suites/profiling/legacyMode/test.ts | 98 +++++++++++++++++++ 2 files changed, 123 insertions(+) create mode 100644 dev-packages/browser-integration-tests/suites/profiling/legacyMode/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts diff --git a/dev-packages/browser-integration-tests/suites/profiling/legacyMode/subject.js b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/subject.js new file mode 100644 index 000000000000..4a5077b299e0 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/subject.js @@ -0,0 +1,25 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserProfilingIntegration()], + tracesSampleRate: 1, + profilesSampleRate: 1, +}); + +function fibonacci(n) { + if (n <= 1) { + return n; + } + return fibonacci(n - 1) + fibonacci(n - 2); +} + +await Sentry.startSpanManual({ name: 'root-fibonacci-2', parentSpan: null, forceTransaction: true }, async span => { + fibonacci(30); + + // Timeout to prevent flaky tests. Integration samples every 20ms, if function is too fast it might not get sampled + await new Promise(resolve => setTimeout(resolve, 21)); + span.end(); +}); diff --git a/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts new file mode 100644 index 000000000000..d133ab2be8bd --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts @@ -0,0 +1,98 @@ +import { expect } from '@playwright/test'; +import type { Event, Profile } from '@sentry/core'; +import { sentryTest } from '../../../utils/fixtures'; +import { properEnvelopeRequestParser, waitForTransactionRequestOnUrl } from '../../../utils/helpers'; + +sentryTest('does not send profile envelope when document-policy is not set', async ({ page, getLocalTestUrl }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + await page.goto(url); + + const req = await waitForTransactionRequestOnUrl(page, url); + const transactionEvent = properEnvelopeRequestParser(req, 0); + const profileEvent = properEnvelopeRequestParser(req, 1); + + expect(transactionEvent).toBeDefined(); + + expect(profileEvent).toBeUndefined(); +}); + +sentryTest('sends profile envelope in legacy mode', async ({ page, getLocalTestUrl }) => { + const url = await getLocalTestUrl({ testDir: __dirname, responseHeaders: { 'Document-Policy': 'js-profiling' } }); + await page.goto(url); + + const req = await waitForTransactionRequestOnUrl(page, url); + const profileEvent = properEnvelopeRequestParser(req, 1); + + const profile = profileEvent.profile; + + expect(profileEvent).toBeDefined(); + expect(profileEvent.profile).toBeDefined(); + + expect(profile.samples).toBeDefined(); + expect(profile.stacks).toBeDefined(); + expect(profile.frames).toBeDefined(); + expect(profile.thread_metadata).toBeDefined(); + + // Samples + expect(profile.samples.length).toBeGreaterThanOrEqual(2); + for (const sample of profile.samples) { + expect(typeof sample.elapsed_since_start_ns).toBe('string'); + expect(sample.elapsed_since_start_ns).toMatch(/^\d+$/); // Numeric string + expect(parseInt(sample.elapsed_since_start_ns, 10)).toBeGreaterThanOrEqual(0); + + expect(typeof sample.stack_id).toBe('number'); + expect(sample.stack_id).toBeGreaterThanOrEqual(0); + expect(sample.thread_id).toBe('0'); // Should be main thread + } + + // Stacks + expect(profile.stacks.length).toBeGreaterThan(0); + for (const stack of profile.stacks) { + expect(Array.isArray(stack)).toBe(true); + for (const frameIndex of stack) { + expect(typeof frameIndex).toBe('number'); + expect(frameIndex).toBeGreaterThanOrEqual(0); + expect(frameIndex).toBeLessThan(profile.frames.length); + } + } + + // Frames + expect(profile.frames.length).toBeGreaterThan(0); + for (const frame of profile.frames) { + expect(frame).toHaveProperty('function'); + expect(frame).toHaveProperty('abs_path'); + expect(frame).toHaveProperty('lineno'); + expect(frame).toHaveProperty('colno'); + + expect(typeof frame.function).toBe('string'); + expect(typeof frame.abs_path).toBe('string'); + expect(typeof frame.lineno).toBe('number'); + expect(typeof frame.colno).toBe('number'); + } + + const functionNames = profile.frames.map(frame => frame.function).filter(name => name !== ''); + + expect(functionNames).toEqual( + expect.arrayContaining([ + '_startRootSpan', + 'withScope', + 'createChildOrRootSpan', + 'startSpanManual', + 'startProfileForSpan', + 'startJSSelfProfile', + ]), + ); + + expect(profile.thread_metadata).toHaveProperty('0'); + expect(profile.thread_metadata['0']).toHaveProperty('name'); + expect(profile.thread_metadata['0'].name).toBe('main'); + + // Test that profile duration makes sense (should be > 20ms based on test setup) + const startTime = parseInt(profile.samples[0].elapsed_since_start_ns, 10); + const endTime = parseInt(profile.samples[profile.samples.length - 1].elapsed_since_start_ns, 10); + const durationNs = endTime - startTime; + const durationMs = durationNs / 1_000_000; // Convert ns to ms + + // Should be at least 20ms based on our setTimeout(21) in the test + expect(durationMs).toBeGreaterThan(20); +}); From 8e121c456ae3ec9857fd16bb7d924f932726ecad Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Wed, 27 Aug 2025 13:05:54 +0200 Subject: [PATCH 2/8] change order --- .../suites/profiling/legacyMode/test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts index d133ab2be8bd..d6f50beee442 100644 --- a/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts +++ b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts @@ -22,10 +22,9 @@ sentryTest('sends profile envelope in legacy mode', async ({ page, getLocalTestU const req = await waitForTransactionRequestOnUrl(page, url); const profileEvent = properEnvelopeRequestParser(req, 1); + expect(profileEvent).toBeDefined(); const profile = profileEvent.profile; - - expect(profileEvent).toBeDefined(); expect(profileEvent.profile).toBeDefined(); expect(profile.samples).toBeDefined(); From 2a52c2474c7986da34344c4e32b6d28c41abdaca Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Wed, 27 Aug 2025 13:18:31 +0200 Subject: [PATCH 3/8] Add helpers for tests --- .../browser-integration-tests/utils/fixtures.ts | 13 +++++++++++-- .../browser-integration-tests/utils/helpers.ts | 9 +++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/dev-packages/browser-integration-tests/utils/fixtures.ts b/dev-packages/browser-integration-tests/utils/fixtures.ts index adebce59edef..7cedc1e4001a 100644 --- a/dev-packages/browser-integration-tests/utils/fixtures.ts +++ b/dev-packages/browser-integration-tests/utils/fixtures.ts @@ -35,6 +35,7 @@ export type TestFixtures = { skipRouteHandler?: boolean; skipDsnRouteHandler?: boolean; handleLazyLoadedFeedback?: boolean; + responseHeaders?: Record; }) => Promise; forceFlushReplay: () => Promise; enableConsole: () => void; @@ -59,7 +60,13 @@ const sentryTest = base.extend({ getLocalTestUrl: ({ page }, use) => { return use( - async ({ testDir, skipRouteHandler = false, skipDsnRouteHandler = false, handleLazyLoadedFeedback = false }) => { + async ({ + testDir, + skipRouteHandler = false, + skipDsnRouteHandler = false, + handleLazyLoadedFeedback = false, + responseHeaders = {}, + }) => { const pagePath = `${TEST_HOST}/index.html`; const tmpDir = path.join(testDir, 'dist', crypto.randomUUID()); @@ -86,7 +93,9 @@ const sentryTest = base.extend({ const file = route.request().url().split('/').pop(); const filePath = path.resolve(tmpDir, `./${file}`); - return fs.existsSync(filePath) ? route.fulfill({ path: filePath }) : route.continue(); + return fs.existsSync(filePath) + ? route.fulfill({ path: filePath, headers: responseHeaders }) + : route.continue(); }); if (handleLazyLoadedFeedback) { diff --git a/dev-packages/browser-integration-tests/utils/helpers.ts b/dev-packages/browser-integration-tests/utils/helpers.ts index 5a9d8a351449..42657ab14731 100644 --- a/dev-packages/browser-integration-tests/utils/helpers.ts +++ b/dev-packages/browser-integration-tests/utils/helpers.ts @@ -29,6 +29,7 @@ export const envelopeParser = (request: Request | null): unknown[] => { }); }; +// Rather use the `properEnvelopeRequestParser`, as the `envelopeParser` does not follow the envelope spec. export const envelopeRequestParser = (request: Request | null, envelopeIndex = 2): T => { return envelopeParser(request)[envelopeIndex] as T; }; @@ -79,8 +80,12 @@ function getEventAndTraceHeader(envelope: EventEnvelope): EventAndTraceHeader { return [event, trace]; } -export const properEnvelopeRequestParser = (request: Request | null, envelopeIndex = 1): T => { - return properEnvelopeParser(request)[0]?.[envelopeIndex] as T; +export const properEnvelopeRequestParser = ( + request: Request | null, + envelopeItemIndex: number, + envelopeIndex = 1, // 1 is usually the payload of the envelope (0 is the header) +): T => { + return properEnvelopeParser(request)[envelopeItemIndex]?.[envelopeIndex] as T; }; export const properFullEnvelopeRequestParser = (request: Request | null): T => { From b078eea51a5ebe201ab3f8177ce924bf73abb0fb Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Wed, 27 Aug 2025 13:36:14 +0200 Subject: [PATCH 4/8] remove goto url --- .../suites/profiling/legacyMode/test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts index d6f50beee442..f8d3ef589357 100644 --- a/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts +++ b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts @@ -5,7 +5,6 @@ import { properEnvelopeRequestParser, waitForTransactionRequestOnUrl } from '../ sentryTest('does not send profile envelope when document-policy is not set', async ({ page, getLocalTestUrl }) => { const url = await getLocalTestUrl({ testDir: __dirname }); - await page.goto(url); const req = await waitForTransactionRequestOnUrl(page, url); const transactionEvent = properEnvelopeRequestParser(req, 0); @@ -18,7 +17,6 @@ sentryTest('does not send profile envelope when document-policy is not set', asy sentryTest('sends profile envelope in legacy mode', async ({ page, getLocalTestUrl }) => { const url = await getLocalTestUrl({ testDir: __dirname, responseHeaders: { 'Document-Policy': 'js-profiling' } }); - await page.goto(url); const req = await waitForTransactionRequestOnUrl(page, url); const profileEvent = properEnvelopeRequestParser(req, 1); From e905ec4eaef9d95922bbaac0992c685397dc13f0 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Wed, 27 Aug 2025 13:44:26 +0200 Subject: [PATCH 5/8] skip tests for non-tracing bundles --- .../suites/profiling/legacyMode/test.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts index f8d3ef589357..9ad4c1fd214e 100644 --- a/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts +++ b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts @@ -1,9 +1,18 @@ import { expect } from '@playwright/test'; import type { Event, Profile } from '@sentry/core'; import { sentryTest } from '../../../utils/fixtures'; -import { properEnvelopeRequestParser, waitForTransactionRequestOnUrl } from '../../../utils/helpers'; +import { + properEnvelopeRequestParser, + shouldSkipTracingTest, + waitForTransactionRequestOnUrl, +} from '../../../utils/helpers'; sentryTest('does not send profile envelope when document-policy is not set', async ({ page, getLocalTestUrl }) => { + if (shouldSkipTracingTest()) { + // Profiling only works when tracing is enabled + sentryTest.skip(); + } + const url = await getLocalTestUrl({ testDir: __dirname }); const req = await waitForTransactionRequestOnUrl(page, url); @@ -16,6 +25,11 @@ sentryTest('does not send profile envelope when document-policy is not set', asy }); sentryTest('sends profile envelope in legacy mode', async ({ page, getLocalTestUrl }) => { + if (shouldSkipTracingTest()) { + // Profiling only works when tracing is enabled + sentryTest.skip(); + } + const url = await getLocalTestUrl({ testDir: __dirname, responseHeaders: { 'Document-Policy': 'js-profiling' } }); const req = await waitForTransactionRequestOnUrl(page, url); From f1ffb359cba883a7a61a231a7d044b4fecd94eb9 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Wed, 27 Aug 2025 16:31:24 +0200 Subject: [PATCH 6/8] add profiling integration in test --- .../suites/profiling/legacyMode/subject.js | 3 ++- .../suites/profiling/legacyMode/test.ts | 26 ++++++++++++------- .../utils/generatePlugin.ts | 1 + 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/profiling/legacyMode/subject.js b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/subject.js index 4a5077b299e0..230e9ee1fb9e 100644 --- a/dev-packages/browser-integration-tests/suites/profiling/legacyMode/subject.js +++ b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/subject.js @@ -1,10 +1,11 @@ import * as Sentry from '@sentry/browser'; +import { browserProfilingIntegration } from '@sentry/browser'; window.Sentry = Sentry; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', - integrations: [Sentry.browserProfilingIntegration()], + integrations: [browserProfilingIntegration()], tracesSampleRate: 1, profilesSampleRate: 1, }); diff --git a/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts index 9ad4c1fd214e..73fd712eca8f 100644 --- a/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts +++ b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts @@ -83,16 +83,22 @@ sentryTest('sends profile envelope in legacy mode', async ({ page, getLocalTestU const functionNames = profile.frames.map(frame => frame.function).filter(name => name !== ''); - expect(functionNames).toEqual( - expect.arrayContaining([ - '_startRootSpan', - 'withScope', - 'createChildOrRootSpan', - 'startSpanManual', - 'startProfileForSpan', - 'startJSSelfProfile', - ]), - ); + if (!(process.env.PW_BUNDLE || '').startsWith('bundle')) { + // In bundled mode, function names are minified + expect(functionNames.length).toBeGreaterThan(0); + expect((functionNames as string[]).every(name => name?.length > 0)).toBe(true); // Just make sure they're not empty strings + } else { + expect(functionNames).toEqual( + expect.arrayContaining([ + '_startRootSpan', + 'withScope', + 'createChildOrRootSpan', + 'startSpanManual', + 'startProfileForSpan', + 'startJSSelfProfile', + ]), + ); + } expect(profile.thread_metadata).toHaveProperty('0'); expect(profile.thread_metadata['0']).toHaveProperty('name'); diff --git a/dev-packages/browser-integration-tests/utils/generatePlugin.ts b/dev-packages/browser-integration-tests/utils/generatePlugin.ts index ff25097ccf50..bd505473f9b7 100644 --- a/dev-packages/browser-integration-tests/utils/generatePlugin.ts +++ b/dev-packages/browser-integration-tests/utils/generatePlugin.ts @@ -36,6 +36,7 @@ const IMPORTED_INTEGRATION_CDN_BUNDLE_PATHS: Record = { feedbackIntegration: 'feedback', moduleMetadataIntegration: 'modulemetadata', graphqlClientIntegration: 'graphqlclient', + browserProfilingIntegration: 'browserprofiling', // technically, this is not an integration, but let's add it anyway for simplicity makeMultiplexedTransport: 'multiplexedtransport', }; From ffccdc18fb246d17ffcb8bf714dd9cb3fc206716 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Wed, 27 Aug 2025 17:33:53 +0200 Subject: [PATCH 7/8] check for chromium --- .../suites/profiling/legacyMode/test.ts | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts index 73fd712eca8f..6e6d1ae26de3 100644 --- a/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts +++ b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts @@ -7,25 +7,27 @@ import { waitForTransactionRequestOnUrl, } from '../../../utils/helpers'; -sentryTest('does not send profile envelope when document-policy is not set', async ({ page, getLocalTestUrl }) => { - if (shouldSkipTracingTest()) { - // Profiling only works when tracing is enabled - sentryTest.skip(); - } - - const url = await getLocalTestUrl({ testDir: __dirname }); +sentryTest( + 'does not send profile envelope when document-policy is not set', + async ({ page, getLocalTestUrl, browserName }) => { + if (shouldSkipTracingTest() || browserName !== 'chromium') { + // Profiling only works when tracing is enabled + sentryTest.skip(); + } - const req = await waitForTransactionRequestOnUrl(page, url); - const transactionEvent = properEnvelopeRequestParser(req, 0); - const profileEvent = properEnvelopeRequestParser(req, 1); + const url = await getLocalTestUrl({ testDir: __dirname }); - expect(transactionEvent).toBeDefined(); + const req = await waitForTransactionRequestOnUrl(page, url); + const transactionEvent = properEnvelopeRequestParser(req, 0); + const profileEvent = properEnvelopeRequestParser(req, 1); - expect(profileEvent).toBeUndefined(); -}); + expect(transactionEvent).toBeDefined(); + expect(profileEvent).toBeUndefined(); + }, +); -sentryTest('sends profile envelope in legacy mode', async ({ page, getLocalTestUrl }) => { - if (shouldSkipTracingTest()) { +sentryTest('sends profile envelope in legacy mode', async ({ page, getLocalTestUrl, browserName }) => { + if (shouldSkipTracingTest() || browserName !== 'chromium') { // Profiling only works when tracing is enabled sentryTest.skip(); } From d7df494306fef90197f3a47f0e9cf3e392b6fc18 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Thu, 28 Aug 2025 08:58:30 +0200 Subject: [PATCH 8/8] check for minified bundles --- .../suites/profiling/legacyMode/test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts index 6e6d1ae26de3..35f4e17bec0a 100644 --- a/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts +++ b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts @@ -85,8 +85,8 @@ sentryTest('sends profile envelope in legacy mode', async ({ page, getLocalTestU const functionNames = profile.frames.map(frame => frame.function).filter(name => name !== ''); - if (!(process.env.PW_BUNDLE || '').startsWith('bundle')) { - // In bundled mode, function names are minified + if ((process.env.PW_BUNDLE || '').endsWith('min')) { + // Function names are minified in minified bundles expect(functionNames.length).toBeGreaterThan(0); expect((functionNames as string[]).every(name => name?.length > 0)).toBe(true); // Just make sure they're not empty strings } else {