Skip to content

Commit b08a3c8

Browse files
committed
ref(browser): Update browser profiling to avoid deprecated APIs
Esp. migrate away from `startTransaction` to `spanStart` hooks.
1 parent ccde98b commit b08a3c8

File tree

3 files changed

+62
-75
lines changed

3 files changed

+62
-75
lines changed

packages/browser/src/profiling/integration.ts

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,17 @@
1-
import { defineIntegration, getCurrentScope } from '@sentry/core';
2-
import type { EventEnvelope, IntegrationFn, Transaction } from '@sentry/types';
1+
import { defineIntegration, getActiveSpan, getRootSpan } from '@sentry/core';
2+
import type { EventEnvelope, IntegrationFn, Span } from '@sentry/types';
33
import type { Profile } from '@sentry/types/src/profiling';
44
import { logger } from '@sentry/utils';
55

66
import { DEBUG_BUILD } from '../debug-build';
7-
import { startProfileForTransaction } from './startProfileForTransaction';
7+
import { startProfileForSpan } from './startProfileForSpan';
88
import type { ProfiledEvent } from './utils';
9+
import { isAutomatedPageLoadSpan, shouldProfileSpan } from './utils';
910
import {
1011
addProfilesToEnvelope,
1112
createProfilingEvent,
1213
findProfiledTransactionsFromEnvelope,
1314
getActiveProfilesCount,
14-
isAutomatedPageLoadTransaction,
15-
shouldProfileTransaction,
1615
takeProfileFromGlobalCache,
1716
} from './utils';
1817

@@ -21,22 +20,19 @@ const INTEGRATION_NAME = 'BrowserProfiling';
2120
const _browserProfilingIntegration = (() => {
2221
return {
2322
name: INTEGRATION_NAME,
24-
// TODO v8: Remove this
2523
setup(client) {
26-
const scope = getCurrentScope();
24+
const activeSpan = getActiveSpan();
25+
const rootSpan = activeSpan && getRootSpan(activeSpan);
2726

28-
// eslint-disable-next-line deprecation/deprecation
29-
const transaction = scope.getTransaction();
30-
31-
if (transaction && isAutomatedPageLoadTransaction(transaction)) {
32-
if (shouldProfileTransaction(transaction)) {
33-
startProfileForTransaction(transaction);
27+
if (rootSpan && isAutomatedPageLoadSpan(rootSpan)) {
28+
if (shouldProfileSpan(rootSpan)) {
29+
startProfileForSpan(rootSpan);
3430
}
3531
}
3632

37-
client.on('startTransaction', (transaction: Transaction) => {
38-
if (shouldProfileTransaction(transaction)) {
39-
startProfileForTransaction(transaction);
33+
client.on('spanStart', (span: Span) => {
34+
if (span === getRootSpan(span) && shouldProfileSpan(span)) {
35+
startProfileForSpan(span);
4036
}
4137
});
4238

@@ -59,23 +55,23 @@ const _browserProfilingIntegration = (() => {
5955
const start_timestamp = context && context['profile'] && context['profile']['start_timestamp'];
6056

6157
if (typeof profile_id !== 'string') {
62-
DEBUG_BUILD && logger.log('[Profiling] cannot find profile for a transaction without a profile context');
58+
DEBUG_BUILD && logger.log('[Profiling] cannot find profile for a span without a profile context');
6359
continue;
6460
}
6561

6662
if (!profile_id) {
67-
DEBUG_BUILD && logger.log('[Profiling] cannot find profile for a transaction without a profile context');
63+
DEBUG_BUILD && logger.log('[Profiling] cannot find profile for a span without a profile context');
6864
continue;
6965
}
7066

71-
// Remove the profile from the transaction context before sending, relay will take care of the rest.
67+
// Remove the profile from the span context before sending, relay will take care of the rest.
7268
if (context && context['profile']) {
7369
delete context.profile;
7470
}
7571

7672
const profile = takeProfileFromGlobalCache(profile_id);
7773
if (!profile) {
78-
DEBUG_BUILD && logger.log(`[Profiling] Could not retrieve profile for transaction: ${profile_id}`);
74+
DEBUG_BUILD && logger.log(`[Profiling] Could not retrieve profile for span: ${profile_id}`);
7975
continue;
8076
}
8177

Original file line numberDiff line numberDiff line change
@@ -1,140 +1,132 @@
1-
import { spanToJSON } from '@sentry/core';
2-
import type { Transaction } from '@sentry/types';
1+
import { getCurrentScope, spanToJSON } from '@sentry/core';
2+
import type { Span } from '@sentry/types';
33
import { logger, timestampInSeconds, uuid4 } from '@sentry/utils';
44

55
import { DEBUG_BUILD } from '../debug-build';
66
import { WINDOW } from '../helpers';
77
import type { JSSelfProfile } from './jsSelfProfiling';
8-
import {
9-
MAX_PROFILE_DURATION_MS,
10-
addProfileToGlobalCache,
11-
isAutomatedPageLoadTransaction,
12-
startJSSelfProfile,
13-
} from './utils';
8+
import { MAX_PROFILE_DURATION_MS, addProfileToGlobalCache, isAutomatedPageLoadSpan, startJSSelfProfile } from './utils';
149

1510
/**
1611
* Wraps startTransaction and stopTransaction with profiling related logic.
1712
* startProfileForTransaction is called after the call to startTransaction in order to avoid our own code from
1813
* being profiled. Because of that same reason, stopProfiling is called before the call to stopTransaction.
1914
*/
20-
export function startProfileForTransaction(transaction: Transaction): Transaction {
15+
export function startProfileForSpan(span: Span): void {
2116
// Start the profiler and get the profiler instance.
2217
let startTimestamp: number | undefined;
23-
if (isAutomatedPageLoadTransaction(transaction)) {
18+
if (isAutomatedPageLoadSpan(span)) {
2419
startTimestamp = timestampInSeconds() * 1000;
2520
}
2621

2722
const profiler = startJSSelfProfile();
2823

29-
// We failed to construct the profiler, fallback to original transaction.
24+
// We failed to construct the profiler, so we skip.
3025
// No need to log anything as this has already been logged in startProfile.
3126
if (!profiler) {
32-
return transaction;
27+
return;
3328
}
3429

3530
if (DEBUG_BUILD) {
36-
logger.log(`[Profiling] started profiling transaction: ${spanToJSON(transaction).description}`);
31+
logger.log(`[Profiling] started profiling span: ${spanToJSON(span).description}`);
3732
}
3833

39-
// We create "unique" transaction names to avoid concurrent transactions with same names
40-
// from being ignored by the profiler. From here on, only this transaction name should be used when
34+
// We create "unique" span names to avoid concurrent spans with same names
35+
// from being ignored by the profiler. From here on, only this span name should be used when
4136
// calling the profiler methods. Note: we log the original name to the user to avoid confusion.
4237
const profileId = uuid4();
4338

4439
// A couple of important things to note here:
4540
// `CpuProfilerBindings.stopProfiling` will be scheduled to run in 30seconds in order to exceed max profile duration.
46-
// Whichever of the two (transaction.finish/timeout) is first to run, the profiling will be stopped and the gathered profile
47-
// will be processed when the original transaction is finished. Since onProfileHandler can be invoked multiple times in the
48-
// event of an error or user mistake (calling transaction.finish multiple times), it is important that the behavior of onProfileHandler
41+
// Whichever of the two (span.finish/timeout) is first to run, the profiling will be stopped and the gathered profile
42+
// will be processed when the original span is finished. Since onProfileHandler can be invoked multiple times in the
43+
// event of an error or user mistake (calling span.finish multiple times), it is important that the behavior of onProfileHandler
4944
// is idempotent as we do not want any timings or profiles to be overriden by the last call to onProfileHandler.
5045
// After the original finish method is called, the event will be reported through the integration and delegated to transport.
5146
const processedProfile: JSSelfProfile | null = null;
5247

48+
getCurrentScope().setContext('profile', {
49+
profile_id: profileId,
50+
start_timestamp: startTimestamp,
51+
});
52+
5353
/**
5454
* Idempotent handler for profile stop
5555
*/
56-
async function onProfileHandler(): Promise<null> {
57-
// Check if the profile exists and return it the behavior has to be idempotent as users may call transaction.finish multiple times.
58-
if (!transaction) {
59-
return null;
56+
async function onProfileHandler(): Promise<void> {
57+
// Check if the profile exists and return it the behavior has to be idempotent as users may call span.finish multiple times.
58+
if (!span) {
59+
return;
6060
}
6161
// Satisfy the type checker, but profiler will always be defined here.
6262
if (!profiler) {
63-
return null;
63+
return;
6464
}
6565
if (processedProfile) {
6666
if (DEBUG_BUILD) {
67-
logger.log('[Profiling] profile for:', spanToJSON(transaction).description, 'already exists, returning early');
67+
logger.log('[Profiling] profile for:', spanToJSON(span).description, 'already exists, returning early');
6868
}
69-
return null;
69+
return;
7070
}
7171

7272
return profiler
7373
.stop()
74-
.then((profile: JSSelfProfile): null => {
74+
.then((profile: JSSelfProfile): void => {
7575
if (maxDurationTimeoutID) {
7676
WINDOW.clearTimeout(maxDurationTimeoutID);
7777
maxDurationTimeoutID = undefined;
7878
}
7979

8080
if (DEBUG_BUILD) {
81-
logger.log(`[Profiling] stopped profiling of transaction: ${spanToJSON(transaction).description}`);
81+
logger.log(`[Profiling] stopped profiling of span: ${spanToJSON(span).description}`);
8282
}
8383

84-
// In case of an overlapping transaction, stopProfiling may return null and silently ignore the overlapping profile.
84+
// In case of an overlapping span, stopProfiling may return null and silently ignore the overlapping profile.
8585
if (!profile) {
8686
if (DEBUG_BUILD) {
8787
logger.log(
88-
`[Profiling] profiler returned null profile for: ${spanToJSON(transaction).description}`,
89-
'this may indicate an overlapping transaction or a call to stopProfiling with a profile title that was never started',
88+
`[Profiling] profiler returned null profile for: ${spanToJSON(span).description}`,
89+
'this may indicate an overlapping span or a call to stopProfiling with a profile title that was never started',
9090
);
9191
}
92-
return null;
92+
return;
9393
}
9494

9595
addProfileToGlobalCache(profileId, profile);
96-
return null;
9796
})
9897
.catch(error => {
9998
if (DEBUG_BUILD) {
10099
logger.log('[Profiling] error while stopping profiler:', error);
101100
}
102-
return null;
103101
});
104102
}
105103

106104
// Enqueue a timeout to prevent profiles from running over max duration.
107105
let maxDurationTimeoutID: number | undefined = WINDOW.setTimeout(() => {
108106
if (DEBUG_BUILD) {
109-
logger.log(
110-
'[Profiling] max profile duration elapsed, stopping profiling for:',
111-
spanToJSON(transaction).description,
112-
);
107+
logger.log('[Profiling] max profile duration elapsed, stopping profiling for:', spanToJSON(span).description);
113108
}
114-
// If the timeout exceeds, we want to stop profiling, but not finish the transaction
109+
// If the timeout exceeds, we want to stop profiling, but not finish the span
115110
// eslint-disable-next-line @typescript-eslint/no-floating-promises
116111
onProfileHandler();
117112
}, MAX_PROFILE_DURATION_MS);
118113

119114
// We need to reference the original end call to avoid creating an infinite loop
120-
const originalEnd = transaction.end.bind(transaction);
115+
const originalEnd = span.end.bind(span);
121116

122117
/**
123-
* Wraps startTransaction and stopTransaction with profiling related logic.
124-
* startProfiling is called after the call to startTransaction in order to avoid our own code from
125-
* being profiled. Because of that same reason, stopProfiling is called before the call to stopTransaction.
118+
* Wraps span `end()` with profiling related logic.
119+
* startProfiling is called after the call to spanStart in order to avoid our own code from
120+
* being profiled. Because of that same reason, stopProfiling is called before the call to spanEnd.
126121
*/
127-
function profilingWrappedTransactionEnd(): Transaction {
128-
if (!transaction) {
122+
function profilingWrappedSpanEnd(): Span {
123+
if (!span) {
129124
return originalEnd();
130125
}
131126
// onProfileHandler should always return the same profile even if this is called multiple times.
132127
// Always call onProfileHandler to ensure stopProfiling is called and the timeout is cleared.
133128
void onProfileHandler().then(
134129
() => {
135-
// TODO: Can we rewrite this to use attributes?
136-
// eslint-disable-next-line deprecation/deprecation
137-
transaction.setContext('profile', { profile_id: profileId, start_timestamp: startTimestamp });
138130
originalEnd();
139131
},
140132
() => {
@@ -143,9 +135,8 @@ export function startProfileForTransaction(transaction: Transaction): Transactio
143135
},
144136
);
145137

146-
return transaction;
138+
return span;
147139
}
148140

149-
transaction.end = profilingWrappedTransactionEnd;
150-
return transaction;
141+
span.end = profilingWrappedSpanEnd;
151142
}

packages/browser/src/profiling/utils.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/* eslint-disable max-lines */
22

33
import { DEFAULT_ENVIRONMENT, getClient, spanToJSON } from '@sentry/core';
4-
import type { DebugImage, Envelope, Event, EventEnvelope, StackFrame, StackParser, Transaction } from '@sentry/types';
4+
import type { DebugImage, Envelope, Event, EventEnvelope, Span, StackFrame, StackParser } from '@sentry/types';
55
import type { Profile, ThreadCpuProfile } from '@sentry/types/src/profiling';
66
import { GLOBAL_OBJ, browserPerformanceTimeOrigin, forEachEnvelopeItem, logger, uuid4 } from '@sentry/utils';
77

@@ -201,8 +201,8 @@ export function isProfiledTransactionEvent(event: Event): event is ProfiledEvent
201201
/**
202202
*
203203
*/
204-
export function isAutomatedPageLoadTransaction(transaction: Transaction): boolean {
205-
return spanToJSON(transaction).op === 'pageload';
204+
export function isAutomatedPageLoadSpan(span: Span): boolean {
205+
return spanToJSON(span).op === 'pageload';
206206
}
207207

208208
/**
@@ -506,7 +506,7 @@ export function startJSSelfProfile(): JSSelfProfiler | undefined {
506506
/**
507507
* Determine if a profile should be profiled.
508508
*/
509-
export function shouldProfileTransaction(transaction: Transaction): boolean {
509+
export function shouldProfileSpan(span: Span): boolean {
510510
// If constructor failed once, it will always fail, so we can early return.
511511
if (PROFILING_CONSTRUCTOR_FAILED) {
512512
if (DEBUG_BUILD) {
@@ -515,7 +515,7 @@ export function shouldProfileTransaction(transaction: Transaction): boolean {
515515
return false;
516516
}
517517

518-
if (!transaction.isRecording()) {
518+
if (!span.isRecording()) {
519519
if (DEBUG_BUILD) {
520520
logger.log('[Profiling] Discarding profile because transaction was not sampled.');
521521
}

0 commit comments

Comments
 (0)