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,26 @@
import * as Sentry from '@sentry/browser';
import { browserProfilingIntegration } from '@sentry/browser';

window.Sentry = Sentry;

Sentry.init({
dsn: 'https://[email protected]/1337',
integrations: [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();
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
import { expect } from '@playwright/test';
import type { Event, Profile } from '@sentry/core';
import { sentryTest } from '../../../utils/fixtures';
import {
properEnvelopeRequestParser,
shouldSkipTracingTest,
waitForTransactionRequestOnUrl,
} from '../../../utils/helpers';

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 url = await getLocalTestUrl({ testDir: __dirname });

const req = await waitForTransactionRequestOnUrl(page, url);
const transactionEvent = properEnvelopeRequestParser<Event>(req, 0);
const profileEvent = properEnvelopeRequestParser<Profile>(req, 1);

expect(transactionEvent).toBeDefined();
expect(profileEvent).toBeUndefined();
},
);

sentryTest('sends profile envelope in legacy mode', async ({ page, getLocalTestUrl, browserName }) => {
if (shouldSkipTracingTest() || browserName !== 'chromium') {
// 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);
const profileEvent = properEnvelopeRequestParser<Profile>(req, 1);
expect(profileEvent).toBeDefined();

const profile = profileEvent.profile;
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 !== '');

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 {
expect(functionNames).toEqual(
expect.arrayContaining([
'_startRootSpan',
'withScope',
'createChildOrRootSpan',
'startSpanManual',
'startProfileForSpan',
'startJSSelfProfile',
]),
);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Function Name Logic Inverted in Bundled Mode

The if/else logic for checking function names in bundled vs. non-bundled mode appears inverted. The comment "In bundled mode, function names are minified" is in the non-bundled branch, and the test's expectations for minified vs. readable function names are swapped, which could lead to incorrect assertions.

Fix in Cursor Fix in Web


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);
});
13 changes: 11 additions & 2 deletions dev-packages/browser-integration-tests/utils/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export type TestFixtures = {
skipRouteHandler?: boolean;
skipDsnRouteHandler?: boolean;
handleLazyLoadedFeedback?: boolean;
responseHeaders?: Record<string, string>;
}) => Promise<string>;
forceFlushReplay: () => Promise<string>;
enableConsole: () => void;
Expand All @@ -59,7 +60,13 @@ const sentryTest = base.extend<TestFixtures>({

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());
Expand All @@ -86,7 +93,9 @@ const sentryTest = base.extend<TestFixtures>({
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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const IMPORTED_INTEGRATION_CDN_BUNDLE_PATHS: Record<string, string> = {
feedbackIntegration: 'feedback',
moduleMetadataIntegration: 'modulemetadata',
graphqlClientIntegration: 'graphqlclient',
browserProfilingIntegration: 'browserprofiling',
// technically, this is not an integration, but let's add it anyway for simplicity
makeMultiplexedTransport: 'multiplexedtransport',
};
Expand Down
9 changes: 7 additions & 2 deletions dev-packages/browser-integration-tests/utils/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = <T = SentryEvent>(request: Request | null, envelopeIndex = 2): T => {
return envelopeParser(request)[envelopeIndex] as T;
};
Expand Down Expand Up @@ -79,8 +80,12 @@ function getEventAndTraceHeader(envelope: EventEnvelope): EventAndTraceHeader {
return [event, trace];
}

export const properEnvelopeRequestParser = <T = SentryEvent>(request: Request | null, envelopeIndex = 1): T => {
return properEnvelopeParser(request)[0]?.[envelopeIndex] as T;
export const properEnvelopeRequestParser = <T = SentryEvent>(
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;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Function Signature Change Causes Argument Mismatch

The properEnvelopeRequestParser function's signature changed, introducing a new required envelopeItemIndex parameter and shifting envelopeIndex's position. This breaks existing calls: those with only a request argument now fail, and those with two arguments misinterpret the second argument as envelopeItemIndex, leading to incorrect data access or silent failures.

Fix in Cursor Fix in Web

};

export const properFullEnvelopeRequestParser = <T extends Envelope>(request: Request | null): T => {
Expand Down
Loading