Skip to content

ref(browser): Update browser profiling to avoid deprecated APIs #11007

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 11, 2024
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
36 changes: 16 additions & 20 deletions packages/browser/src/profiling/integration.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
import { defineIntegration, getCurrentScope } from '@sentry/core';
import type { EventEnvelope, IntegrationFn, Transaction } from '@sentry/types';
import { defineIntegration, getActiveSpan, getRootSpan } from '@sentry/core';
import type { EventEnvelope, IntegrationFn, Span } from '@sentry/types';
import type { Profile } from '@sentry/types/src/profiling';
import { logger } from '@sentry/utils';

import { DEBUG_BUILD } from '../debug-build';
import { startProfileForTransaction } from './startProfileForTransaction';
import { startProfileForSpan } from './startProfileForSpan';
import type { ProfiledEvent } from './utils';
import { isAutomatedPageLoadSpan, shouldProfileSpan } from './utils';
import {
addProfilesToEnvelope,
createProfilingEvent,
findProfiledTransactionsFromEnvelope,
getActiveProfilesCount,
isAutomatedPageLoadTransaction,
shouldProfileTransaction,
takeProfileFromGlobalCache,
} from './utils';

Expand All @@ -21,22 +20,19 @@ const INTEGRATION_NAME = 'BrowserProfiling';
const _browserProfilingIntegration = (() => {
return {
name: INTEGRATION_NAME,
// TODO v8: Remove this
setup(client) {
const scope = getCurrentScope();
const activeSpan = getActiveSpan();
const rootSpan = activeSpan && getRootSpan(activeSpan);

// eslint-disable-next-line deprecation/deprecation
const transaction = scope.getTransaction();

if (transaction && isAutomatedPageLoadTransaction(transaction)) {
if (shouldProfileTransaction(transaction)) {
startProfileForTransaction(transaction);
if (rootSpan && isAutomatedPageLoadSpan(rootSpan)) {
if (shouldProfileSpan(rootSpan)) {
startProfileForSpan(rootSpan);
}
}

client.on('startTransaction', (transaction: Transaction) => {
if (shouldProfileTransaction(transaction)) {
startProfileForTransaction(transaction);
client.on('spanStart', (span: Span) => {
if (span === getRootSpan(span) && shouldProfileSpan(span)) {
startProfileForSpan(span);
}
});

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

if (typeof profile_id !== 'string') {
DEBUG_BUILD && logger.log('[Profiling] cannot find profile for a transaction without a profile context');
DEBUG_BUILD && logger.log('[Profiling] cannot find profile for a span without a profile context');
continue;
}

if (!profile_id) {
DEBUG_BUILD && logger.log('[Profiling] cannot find profile for a transaction without a profile context');
DEBUG_BUILD && logger.log('[Profiling] cannot find profile for a span without a profile context');
continue;
}

// Remove the profile from the transaction context before sending, relay will take care of the rest.
// Remove the profile from the span context before sending, relay will take care of the rest.
if (context && context['profile']) {
delete context.profile;
}

const profile = takeProfileFromGlobalCache(profile_id);
if (!profile) {
DEBUG_BUILD && logger.log(`[Profiling] Could not retrieve profile for transaction: ${profile_id}`);
DEBUG_BUILD && logger.log(`[Profiling] Could not retrieve profile for span: ${profile_id}`);
continue;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,140 +1,132 @@
import { spanToJSON } from '@sentry/core';
import type { Transaction } from '@sentry/types';
import { getCurrentScope, spanToJSON } from '@sentry/core';
import type { Span } from '@sentry/types';
import { logger, timestampInSeconds, uuid4 } from '@sentry/utils';

import { DEBUG_BUILD } from '../debug-build';
import { WINDOW } from '../helpers';
import type { JSSelfProfile } from './jsSelfProfiling';
import {
MAX_PROFILE_DURATION_MS,
addProfileToGlobalCache,
isAutomatedPageLoadTransaction,
startJSSelfProfile,
} from './utils';
import { MAX_PROFILE_DURATION_MS, addProfileToGlobalCache, isAutomatedPageLoadSpan, startJSSelfProfile } from './utils';

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

const profiler = startJSSelfProfile();

// We failed to construct the profiler, fallback to original transaction.
// We failed to construct the profiler, so we skip.
// No need to log anything as this has already been logged in startProfile.
if (!profiler) {
return transaction;
return;
}

if (DEBUG_BUILD) {
logger.log(`[Profiling] started profiling transaction: ${spanToJSON(transaction).description}`);
logger.log(`[Profiling] started profiling span: ${spanToJSON(span).description}`);
}

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

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

getCurrentScope().setContext('profile', {
profile_id: profileId,
start_timestamp: startTimestamp,
});

/**
* Idempotent handler for profile stop
*/
async function onProfileHandler(): Promise<null> {
// Check if the profile exists and return it the behavior has to be idempotent as users may call transaction.finish multiple times.
if (!transaction) {
return null;
async function onProfileHandler(): Promise<void> {
// Check if the profile exists and return it the behavior has to be idempotent as users may call span.finish multiple times.
if (!span) {
return;
}
// Satisfy the type checker, but profiler will always be defined here.
if (!profiler) {
return null;
return;
}
if (processedProfile) {
if (DEBUG_BUILD) {
logger.log('[Profiling] profile for:', spanToJSON(transaction).description, 'already exists, returning early');
logger.log('[Profiling] profile for:', spanToJSON(span).description, 'already exists, returning early');
}
return null;
return;
}

return profiler
.stop()
.then((profile: JSSelfProfile): null => {
.then((profile: JSSelfProfile): void => {
if (maxDurationTimeoutID) {
WINDOW.clearTimeout(maxDurationTimeoutID);
maxDurationTimeoutID = undefined;
}

if (DEBUG_BUILD) {
logger.log(`[Profiling] stopped profiling of transaction: ${spanToJSON(transaction).description}`);
logger.log(`[Profiling] stopped profiling of span: ${spanToJSON(span).description}`);
}

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

addProfileToGlobalCache(profileId, profile);
return null;
})
.catch(error => {
if (DEBUG_BUILD) {
logger.log('[Profiling] error while stopping profiler:', error);
}
return null;
});
}

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

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

/**
* Wraps startTransaction and stopTransaction with profiling related logic.
* startProfiling is called after the call to startTransaction in order to avoid our own code from
* being profiled. Because of that same reason, stopProfiling is called before the call to stopTransaction.
* Wraps span `end()` with profiling related logic.
* startProfiling is called after the call to spanStart in order to avoid our own code from
* being profiled. Because of that same reason, stopProfiling is called before the call to spanEnd.
*/
function profilingWrappedTransactionEnd(): Transaction {
if (!transaction) {
function profilingWrappedSpanEnd(): Span {
if (!span) {
return originalEnd();
}
// onProfileHandler should always return the same profile even if this is called multiple times.
// Always call onProfileHandler to ensure stopProfiling is called and the timeout is cleared.
void onProfileHandler().then(
() => {
// TODO: Can we rewrite this to use attributes?
// eslint-disable-next-line deprecation/deprecation
transaction.setContext('profile', { profile_id: profileId, start_timestamp: startTimestamp });
originalEnd();
},
() => {
Expand All @@ -143,9 +135,8 @@ export function startProfileForTransaction(transaction: Transaction): Transactio
},
);

return transaction;
return span;
}

transaction.end = profilingWrappedTransactionEnd;
return transaction;
span.end = profilingWrappedSpanEnd;
}
10 changes: 5 additions & 5 deletions packages/browser/src/profiling/utils.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable max-lines */

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

Expand Down Expand Up @@ -201,8 +201,8 @@ export function isProfiledTransactionEvent(event: Event): event is ProfiledEvent
/**
*
*/
export function isAutomatedPageLoadTransaction(transaction: Transaction): boolean {
return spanToJSON(transaction).op === 'pageload';
export function isAutomatedPageLoadSpan(span: Span): boolean {
return spanToJSON(span).op === 'pageload';
}

/**
Expand Down Expand Up @@ -506,7 +506,7 @@ export function startJSSelfProfile(): JSSelfProfiler | undefined {
/**
* Determine if a profile should be profiled.
*/
export function shouldProfileTransaction(transaction: Transaction): boolean {
export function shouldProfileSpan(span: Span): boolean {
// If constructor failed once, it will always fail, so we can early return.
if (PROFILING_CONSTRUCTOR_FAILED) {
if (DEBUG_BUILD) {
Expand All @@ -515,7 +515,7 @@ export function shouldProfileTransaction(transaction: Transaction): boolean {
return false;
}

if (!transaction.isRecording()) {
if (!span.isRecording()) {
if (DEBUG_BUILD) {
logger.log('[Profiling] Discarding profile because transaction was not sampled.');
}
Expand Down