Skip to content

Commit

Permalink
feat(browser): Allow collecting of pageload profiles (#9317)
Browse files Browse the repository at this point in the history 
Profiling is currently unable to profile page loads which creates a gap
in the product as performance instrumentation seemingly can (via
synthetic transactions)

In order to bridge this gap, the plan is to provide a small JS snippet
that users can insert into their document which will initialize and
store a reference to the pageload profile on the sentry carrier. By
doing so we can hook back into our finishTransaction codepath and send a
profile associated to a transaction.

Would love to hear early thoughts on this approach from SDK maintainers
before I start adding test coverage.
  • Loading branch information
@JonasBa
JonasBa authored Oct 24, 2023
1 parent f77bb6e commit 81c9651
Show file tree
Hide file tree
Showing 5 changed files with 264 additions and 132 deletions.
136 changes: 21 additions & 115 deletions packages/browser/src/profiling/hubextensions.ts
View file
Original file line number Diff line number Diff line change
@@ -1,24 +1,15 @@
/* eslint-disable complexity */
import { getCurrentHub } from '@sentry/core';
import type { Transaction } from '@sentry/types';
import { logger, uuid4 } from '@sentry/utils';

import { WINDOW } from '../helpers';
import type { JSSelfProfile, JSSelfProfiler, JSSelfProfilerConstructor } from './jsSelfProfiling';
import { addProfileToMap, isValidSampleRate } from './utils';

export const MAX_PROFILE_DURATION_MS = 30_000;
// Keep a flag value to avoid re-initializing the profiler constructor. If it fails
// once, it will always fail and this allows us to early return.
let PROFILING_CONSTRUCTOR_FAILED = false;

/**
* Check if profiler constructor is available.
* @param maybeProfiler
*/
function isJSProfilerSupported(maybeProfiler: unknown): maybeProfiler is typeof JSSelfProfilerConstructor {
return typeof maybeProfiler === 'function';
}
import type { JSSelfProfile } from './jsSelfProfiling';
import {
addProfileToGlobalCache,
MAX_PROFILE_DURATION_MS,
shouldProfileTransaction,
startJSSelfProfile,
} from './utils';

/**
* Safety wrapper for startTransaction for the unlikely case that transaction starts before tracing is imported -
Expand All @@ -35,98 +26,24 @@ export function onProfilingStartRouteTransaction(transaction: Transaction | unde
return transaction;
}

return wrapTransactionWithProfiling(transaction);
if (shouldProfileTransaction(transaction)) {
return startProfileForTransaction(transaction);
}

return transaction;
}

/**
* Wraps startTransaction and stopTransaction with profiling related logic.
* startProfiling is called after the call to startTransaction in order to avoid our own code from
* 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 wrapTransactionWithProfiling(transaction: Transaction): Transaction {
// Feature support check first
const JSProfilerConstructor = WINDOW.Profiler;

if (!isJSProfilerSupported(JSProfilerConstructor)) {
if (__DEBUG_BUILD__) {
logger.log(
'[Profiling] Profiling is not supported by this browser, Profiler interface missing on window object.',
);
}
return transaction;
}

// If constructor failed once, it will always fail, so we can early return.
if (PROFILING_CONSTRUCTOR_FAILED) {
if (__DEBUG_BUILD__) {
logger.log('[Profiling] Profiling has been disabled for the duration of the current user session.');
}
return transaction;
}

const client = getCurrentHub().getClient();
const options = client && client.getOptions();
if (!options) {
__DEBUG_BUILD__ && logger.log('[Profiling] Profiling disabled, no options found.');
return transaction;
}
export function startProfileForTransaction(transaction: Transaction): Transaction {
// Start the profiler and get the profiler instance.
const profiler = startJSSelfProfile();

// @ts-expect-error profilesSampleRate is not part of the browser options yet
const profilesSampleRate: number | boolean | undefined = options.profilesSampleRate;

// Since this is coming from the user (or from a function provided by the user), who knows what we might get. (The
// only valid values are booleans or numbers between 0 and 1.)
if (!isValidSampleRate(profilesSampleRate)) {
__DEBUG_BUILD__ && logger.warn('[Profiling] Discarding profile because of invalid sample rate.');
return transaction;
}

// if the function returned 0 (or false), or if `profileSampleRate` is 0, it's a sign the profile should be dropped
if (!profilesSampleRate) {
__DEBUG_BUILD__ &&
logger.log(
'[Profiling] Discarding profile because a negative sampling decision was inherited or profileSampleRate is set to 0',
);
return transaction;
}

// Now we roll the dice. Math.random is inclusive of 0, but not of 1, so strict < is safe here. In case sampleRate is
// a boolean, the < comparison will cause it to be automatically cast to 1 if it's true and 0 if it's false.
const sampled = profilesSampleRate === true ? true : Math.random() < profilesSampleRate;
// Check if we should sample this profile
if (!sampled) {
__DEBUG_BUILD__ &&
logger.log(
`[Profiling] Discarding profile because it's not included in the random sample (sampling rate = ${Number(
profilesSampleRate,
)})`,
);
return transaction;
}

// From initial testing, it seems that the minimum value for sampleInterval is 10ms.
const samplingIntervalMS = 10;
// Start the profiler
const maxSamples = Math.floor(MAX_PROFILE_DURATION_MS / samplingIntervalMS);
let profiler: JSSelfProfiler | undefined;

// Attempt to initialize the profiler constructor, if it fails, we disable profiling for the current user session.
// This is likely due to a missing 'Document-Policy': 'js-profiling' header. We do not want to throw an error if this happens
// as we risk breaking the user's application, so just disable profiling and log an error.
try {
profiler = new JSProfilerConstructor({ sampleInterval: samplingIntervalMS, maxBufferSize: maxSamples });
} catch (e) {
if (__DEBUG_BUILD__) {
logger.log(
"[Profiling] Failed to initialize the Profiling constructor, this is likely due to a missing 'Document-Policy': 'js-profiling' header.",
);
logger.log('[Profiling] Disabling profiling for current user session.');
}
PROFILING_CONSTRUCTOR_FAILED = true;
}

// We failed to construct the profiler, fallback to original transaction - there is no need to log
// anything as we already did that in the try/catch block.
// We failed to construct the profiler, fallback to original transaction.
// No need to log anything as this has already been logged in startProfile.
if (!profiler) {
return transaction;
}
Expand Down Expand Up @@ -172,19 +89,9 @@ export function wrapTransactionWithProfiling(transaction: Transaction): Transact
return null;
}

// This is temporary - we will use the collected span data to evaluate
// if deferring txn.finish until profiler resolves is a viable approach.
const stopProfilerSpan = transaction.startChild({
description: 'profiler.stop',
op: 'profiler',
origin: 'auto.profiler.browser',
});

return profiler
.stop()
.then((p: JSSelfProfile): null => {
stopProfilerSpan.finish();

.then((profile: JSSelfProfile): null => {
if (maxDurationTimeoutID) {
WINDOW.clearTimeout(maxDurationTimeoutID);
maxDurationTimeoutID = undefined;
Expand All @@ -195,7 +102,7 @@ export function wrapTransactionWithProfiling(transaction: Transaction): Transact
}

// In case of an overlapping transaction, stopProfiling may return null and silently ignore the overlapping profile.
if (!p) {
if (!profile) {
if (__DEBUG_BUILD__) {
logger.log(
`[Profiling] profiler returned null profile for: ${transaction.name || transaction.description}`,
Expand All @@ -205,11 +112,10 @@ export function wrapTransactionWithProfiling(transaction: Transaction): Transact
return null;
}

addProfileToMap(profileId, p);
addProfileToGlobalCache(profileId, profile);
return null;
})
.catch(error => {
stopProfilerSpan.finish();
if (__DEBUG_BUILD__) {
logger.log('[Profiling] error while stopping profiler:', error);
}
Expand Down
39 changes: 29 additions & 10 deletions packages/browser/src/profiling/integration.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@ import type { EventProcessor, Hub, Integration, Transaction } from '@sentry/type
import type { Profile } from '@sentry/types/src/profiling';
import { logger } from '@sentry/utils';

import type { BrowserClient } from './../client';
import { wrapTransactionWithProfiling } from './hubextensions';
import { startProfileForTransaction } from './hubextensions';
import type { ProfiledEvent } from './utils';
import {
addProfilesToEnvelope,
createProfilingEvent,
findProfiledTransactionsFromEnvelope,
PROFILE_MAP,
getActiveProfilesCount,
isAutomatedPageLoadTransaction,
shouldProfileTransaction,
takeProfileFromGlobalCache,
} from './utils';

/**
Expand Down Expand Up @@ -37,16 +39,29 @@ export class BrowserProfilingIntegration implements Integration {
*/
public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
this.getCurrentHub = getCurrentHub;
const client = this.getCurrentHub().getClient() as BrowserClient;

const hub = this.getCurrentHub();
const client = hub.getClient();
const scope = hub.getScope();

const transaction = scope.getTransaction();

if (transaction && isAutomatedPageLoadTransaction(transaction)) {
if (shouldProfileTransaction(transaction)) {
startProfileForTransaction(transaction);
}
}

if (client && typeof client.on === 'function') {
client.on('startTransaction', (transaction: Transaction) => {
wrapTransactionWithProfiling(transaction);
if (shouldProfileTransaction(transaction)) {
startProfileForTransaction(transaction);
}
});

client.on('beforeEnvelope', (envelope): void => {
// if not profiles are in queue, there is nothing to add to the envelope.
if (!PROFILE_MAP['size']) {
if (!getActiveProfilesCount()) {
return;
}

Expand All @@ -59,7 +74,13 @@ export class BrowserProfilingIntegration implements Integration {

for (const profiledTransaction of profiledTransactionEvents) {
const context = profiledTransaction && profiledTransaction.contexts;
const profile_id = context && context['profile'] && (context['profile']['profile_id'] as string);
const profile_id = context && context['profile'] && context['profile']['profile_id'];

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

if (!profile_id) {
__DEBUG_BUILD__ &&
Expand All @@ -72,15 +93,13 @@ export class BrowserProfilingIntegration implements Integration {
delete context.profile;
}

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

PROFILE_MAP.delete(profile_id);
const profileEvent = createProfilingEvent(profile_id, profile, profiledTransaction as ProfiledEvent);

if (profileEvent) {
profilesToAddToEnvelope.push(profileEvent);
}
Expand Down
4 changes: 1 addition & 3 deletions packages/browser/src/profiling/jsSelfProfiling.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,12 @@ export type JSSelfProfile = {
samples: JSSelfProfileSample[];
};

type BufferFullCallback = (trace: JSSelfProfile) => void;

export interface JSSelfProfiler {
sampleInterval: number;
stopped: boolean;

stop: () => Promise<JSSelfProfile>;
addEventListener(event: 'samplebufferfull', callback: BufferFullCallback): void;
addEventListener(event: 'samplebufferfull', callback: (trace: JSSelfProfile) => void): void;
}

export declare const JSSelfProfilerConstructor: {
Expand Down
Loading

0 comments on commit 81c9651

Please sign in to comment.