Skip to content

Commit c7b8c02

Browse files
authored
fix(browser): correct timestamp on pageload profiles (#9350)
Since there is an unknown elapsed time between document response and profile start, setting profile timestamp = transaction start timestamp can significantly skew our profile and cause alignment issue in the product. This updates the pageload profiles to create independent timestamps.
1 parent f6d66b5 commit c7b8c02

File tree

4 files changed

+39
-17
lines changed

4 files changed

+39
-17
lines changed

packages/browser/src/profiling/hubextensions.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
/* eslint-disable complexity */
22
import type { Transaction } from '@sentry/types';
3-
import { logger, uuid4 } from '@sentry/utils';
3+
import { logger, timestampInSeconds, uuid4 } from '@sentry/utils';
44

55
import { WINDOW } from '../helpers';
66
import type { JSSelfProfile } from './jsSelfProfiling';
77
import {
88
addProfileToGlobalCache,
9+
isAutomatedPageLoadTransaction,
910
MAX_PROFILE_DURATION_MS,
1011
shouldProfileTransaction,
1112
startJSSelfProfile,
@@ -40,6 +41,11 @@ export function onProfilingStartRouteTransaction(transaction: Transaction | unde
4041
*/
4142
export function startProfileForTransaction(transaction: Transaction): Transaction {
4243
// Start the profiler and get the profiler instance.
44+
let startTimestamp: number | undefined;
45+
if (isAutomatedPageLoadTransaction(transaction)) {
46+
startTimestamp = timestampInSeconds() * 1000;
47+
}
48+
4349
const profiler = startJSSelfProfile();
4450

4551
// We failed to construct the profiler, fallback to original transaction.
@@ -151,7 +157,7 @@ export function startProfileForTransaction(transaction: Transaction): Transactio
151157
// Always call onProfileHandler to ensure stopProfiling is called and the timeout is cleared.
152158
void onProfileHandler().then(
153159
() => {
154-
transaction.setContext('profile', { profile_id: profileId });
160+
transaction.setContext('profile', { profile_id: profileId, start_timestamp: startTimestamp });
155161
originalFinish();
156162
},
157163
() => {

packages/browser/src/profiling/integration.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ export class BrowserProfilingIntegration implements Integration {
7575
for (const profiledTransaction of profiledTransactionEvents) {
7676
const context = profiledTransaction && profiledTransaction.contexts;
7777
const profile_id = context && context['profile'] && context['profile']['profile_id'];
78+
const start_timestamp = context && context['profile'] && context['profile']['start_timestamp'];
7879

7980
if (typeof profile_id !== 'string') {
8081
__DEBUG_BUILD__ &&
@@ -99,7 +100,12 @@ export class BrowserProfilingIntegration implements Integration {
99100
continue;
100101
}
101102

102-
const profileEvent = createProfilingEvent(profile_id, profile, profiledTransaction as ProfiledEvent);
103+
const profileEvent = createProfilingEvent(
104+
profile_id,
105+
start_timestamp as number | undefined,
106+
profile,
107+
profiledTransaction as ProfiledEvent,
108+
);
103109
if (profileEvent) {
104110
profilesToAddToEnvelope.push(profileEvent);
105111
}

packages/browser/src/profiling/utils.ts

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -119,25 +119,30 @@ function getTraceId(event: Event): string {
119119
* Creates a profiling event envelope from a Sentry event.
120120
*/
121121
export function createProfilePayload(
122-
event: ProfiledEvent,
123-
processedProfile: JSSelfProfile,
124122
profile_id: string,
123+
start_timestamp: number | undefined,
124+
processed_profile: JSSelfProfile,
125+
event: ProfiledEvent,
125126
): Profile {
126127
if (event.type !== 'transaction') {
127128
// createProfilingEventEnvelope should only be called for transactions,
128129
// we type guard this behavior with isProfiledTransactionEvent.
129130
throw new TypeError('Profiling events may only be attached to transactions, this should never occur.');
130131
}
131132

132-
if (processedProfile === undefined || processedProfile === null) {
133+
if (processed_profile === undefined || processed_profile === null) {
133134
throw new TypeError(
134-
`Cannot construct profiling event envelope without a valid profile. Got ${processedProfile} instead.`,
135+
`Cannot construct profiling event envelope without a valid profile. Got ${processed_profile} instead.`,
135136
);
136137
}
137138

138139
const traceId = getTraceId(event);
139-
const enrichedThreadProfile = enrichWithThreadInformation(processedProfile);
140-
const transactionStartMs = typeof event.start_timestamp === 'number' ? event.start_timestamp * 1000 : Date.now();
140+
const enrichedThreadProfile = enrichWithThreadInformation(processed_profile);
141+
const transactionStartMs = start_timestamp
142+
? start_timestamp
143+
: typeof event.start_timestamp === 'number'
144+
? event.start_timestamp * 1000
145+
: Date.now();
141146
const transactionEndMs = typeof event.timestamp === 'number' ? event.timestamp * 1000 : Date.now();
142147

143148
const profile: Profile = {
@@ -164,7 +169,7 @@ export function createProfilePayload(
164169
is_emulator: false,
165170
},
166171
debug_meta: {
167-
images: applyDebugMetadata(processedProfile.resources),
172+
images: applyDebugMetadata(processed_profile.resources),
168173
},
169174
profile: enrichedThreadProfile,
170175
transactions: [
@@ -575,12 +580,17 @@ export function shouldProfileTransaction(transaction: Transaction): boolean {
575580
* @param event
576581
* @returns {Profile | null}
577582
*/
578-
export function createProfilingEvent(profile_id: string, profile: JSSelfProfile, event: ProfiledEvent): Profile | null {
583+
export function createProfilingEvent(
584+
profile_id: string,
585+
start_timestamp: number | undefined,
586+
profile: JSSelfProfile,
587+
event: ProfiledEvent,
588+
): Profile | null {
579589
if (!isValidProfile(profile)) {
580590
return null;
581591
}
582592

583-
return createProfilePayload(event, profile, profile_id);
593+
return createProfilePayload(profile_id, start_timestamp, profile, event);
584594
}
585595

586596
const PROFILE_MAP: Map<string, JSSelfProfile> = new Map();

packages/browser/test/unit/profiling/integration.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,6 @@ import { BrowserProfilingIntegration } from '../../../src/profiling/integration'
55
import type { JSSelfProfile } from '../../../src/profiling/jsSelfProfiling';
66

77
describe('BrowserProfilingIntegration', () => {
8-
beforeEach(() => {
9-
jest.clearAllMocks();
10-
jest.useRealTimers();
11-
// We will mock the carrier as if it has been initialized by the SDK, else everything is short circuited
12-
});
138
it('pageload profiles follow regular transaction code path', async () => {
149
const stopProfile = jest.fn().mockImplementation((): Promise<JSSelfProfile> => {
1510
return Promise.resolve({
@@ -63,6 +58,11 @@ describe('BrowserProfilingIntegration', () => {
6358
expect(send).toHaveBeenCalledTimes(1);
6459

6560
const profile = send.mock.calls?.[0]?.[0]?.[1]?.[1]?.[1];
61+
const transaction = send.mock.calls?.[0]?.[0]?.[1]?.[0]?.[1];
62+
const profile_timestamp_ms = new Date(profile.timestamp).getTime();
63+
const transaction_timestamp_ms = new Date(transaction.start_timestamp * 1e3).getTime();
64+
65+
expect(profile_timestamp_ms).toBeGreaterThan(transaction_timestamp_ms);
6666
expect(profile.profile.frames[0]).toMatchObject({ function: 'pageload_fn', lineno: 1, colno: 1 });
6767
});
6868
});

0 commit comments

Comments
 (0)