From c7078fbd42dd820040424cc880dce3c81fde3447 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Thu, 22 Jun 2023 18:00:40 -0400 Subject: [PATCH] ref: cleanup types and drop separate profile_id --- .../browser/src/profiling/hubextensions.ts | 17 +------ packages/browser/src/profiling/integration.ts | 9 ++-- .../browser/src/profiling/jsSelfProfiling.ts | 7 --- packages/browser/src/profiling/utils.ts | 48 +++++++++---------- 4 files changed, 31 insertions(+), 50 deletions(-) diff --git a/packages/browser/src/profiling/hubextensions.ts b/packages/browser/src/profiling/hubextensions.ts index c42438103b2c..250b4eb41b10 100644 --- a/packages/browser/src/profiling/hubextensions.ts +++ b/packages/browser/src/profiling/hubextensions.ts @@ -8,7 +8,6 @@ import type { JSSelfProfile, JSSelfProfiler, JSSelfProfilerConstructor, - ProcessedJSSelfProfile, } from './jsSelfProfiling'; import { addProfileToMap,isValidSampleRate } from './utils'; @@ -163,7 +162,7 @@ export function wrapTransactionWithProfiling(transaction: Transaction): Transact // event of an error or user mistake (calling transaction.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: ProcessedJSSelfProfile | null = null; + const processedProfile: JSSelfProfile | null = null; /** * Idempotent handler for profile stop @@ -211,19 +210,7 @@ export function wrapTransactionWithProfiling(transaction: Transaction): Transact return null; } - // If a profile has less than 2 samples, it is not useful and should be discarded. - if (p.samples.length < 2) { - return null; - } - - // Discard a profile if it has no frames - happens if code was idling or sampling - // did not collect any frames. - if(!p.frames.length){ - return null; - } - - - addProfileToMap({...p, profile_id: profileId }); + addProfileToMap(profileId, p); return null; }) .catch(error => { diff --git a/packages/browser/src/profiling/integration.ts b/packages/browser/src/profiling/integration.ts index d04795481662..a559cdff53ae 100644 --- a/packages/browser/src/profiling/integration.ts +++ b/packages/browser/src/profiling/integration.ts @@ -58,11 +58,12 @@ export class BrowserProfilingIntegration implements Integration { profiledTransaction.contexts['profile']['profile_id'] as string if (!profile_id) { - throw new TypeError('[Profiling] cannot find profile for a transaction without a profile context'); + __DEBUG_BUILD__ && logger.log('[Profiling] cannot find profile for a transaction without a profile context'); + continue; } // Remove the profile from the transaction context before sending, relay will take care of the rest. - if (profiledTransaction && profiledTransaction.contexts && profiledTransaction.contexts['.profile']) { + if (profiledTransaction && profiledTransaction.contexts && profiledTransaction.contexts['profile']) { delete profiledTransaction.contexts.profile; } @@ -73,8 +74,8 @@ export class BrowserProfilingIntegration implements Integration { } PROFILE_MAP.delete(profile_id); - const profileEvent = createProfilingEvent(profile, profiledTransaction as ProfiledEvent); - + const profileEvent = createProfilingEvent(profile_id, profile, profiledTransaction as ProfiledEvent); + if (profileEvent) { profilesToAddToEnvelope.push(profileEvent); } diff --git a/packages/browser/src/profiling/jsSelfProfiling.ts b/packages/browser/src/profiling/jsSelfProfiling.ts index efa4a0a0a0bc..d2991e6ddb63 100644 --- a/packages/browser/src/profiling/jsSelfProfiling.ts +++ b/packages/browser/src/profiling/jsSelfProfiling.ts @@ -26,10 +26,6 @@ export type JSSelfProfile = { samples: JSSelfProfileSample[]; }; -export interface ProcessedJSSelfProfile extends JSSelfProfile { - profile_id: string; -} - type BufferFullCallback = (trace: JSSelfProfile) => void; export interface JSSelfProfiler { @@ -50,6 +46,3 @@ declare global { } } -export interface RawThreadCpuProfile extends JSSelfProfile { - profile_id: string; -} diff --git a/packages/browser/src/profiling/utils.ts b/packages/browser/src/profiling/utils.ts index 2aa6293ef435..09c0f44e5046 100644 --- a/packages/browser/src/profiling/utils.ts +++ b/packages/browser/src/profiling/utils.ts @@ -6,7 +6,7 @@ import type { Profile, ThreadCpuProfile } from '@sentry/types/src/profiling'; import { forEachEnvelopeItem, GLOBAL_OBJ, logger, uuid4 } from '@sentry/utils'; import { WINDOW } from '../helpers'; -import type { JSSelfProfile, JSSelfProfileStack, ProcessedJSSelfProfile } from './jsSelfProfiling'; +import type { JSSelfProfile, JSSelfProfileStack } from './jsSelfProfiling'; const MS_TO_NS = 1e6; // Use 0 as main thread id which is identical to threadId in node:worker_threads @@ -64,9 +64,7 @@ if (isUserAgentData(userAgentData)) { .catch(e => void e); } -function isProcessedJSSelfProfile( - profile: ThreadCpuProfile | ProcessedJSSelfProfile, -): profile is ProcessedJSSelfProfile { +function isProcessedJSSelfProfile(profile: ThreadCpuProfile | JSSelfProfile): profile is JSSelfProfile { return !('thread_metadata' in profile); } @@ -75,7 +73,7 @@ function isProcessedJSSelfProfile( /** * */ -export function enrichWithThreadInformation(profile: ThreadCpuProfile | ProcessedJSSelfProfile): ThreadCpuProfile { +export function enrichWithThreadInformation(profile: ThreadCpuProfile | JSSelfProfile): ThreadCpuProfile { if (!isProcessedJSSelfProfile(profile)) { return profile; } @@ -87,7 +85,7 @@ export function enrichWithThreadInformation(profile: ThreadCpuProfile | Processe // by the integration before the event is processed by other integrations. export interface ProfiledEvent extends Event { sdkProcessingMetadata: { - profile?: ProcessedJSSelfProfile; + profile?: JSSelfProfile; }; } @@ -120,7 +118,11 @@ function getTraceId(event: Event): string { /** * Creates a profiling event envelope from a Sentry event. */ -export function createProfilePayload(event: ProfiledEvent, processedProfile: ProcessedJSSelfProfile): Profile { +export function createProfilePayload( + event: ProfiledEvent, + processedProfile: JSSelfProfile, + profile_id: string, +): Profile { if (event.type !== 'transaction') { // createProfilingEventEnvelope should only be called for transactions, // we type guard this behavior with isProfiledTransactionEvent. @@ -133,17 +135,13 @@ export function createProfilePayload(event: ProfiledEvent, processedProfile: Pro ); } - if (!processedProfile.profile_id) { - throw new TypeError('Profile is missing profile_id'); - } - const traceId = getTraceId(event); const enrichedThreadProfile = enrichWithThreadInformation(processedProfile); const transactionStartMs = typeof event.start_timestamp === 'number' ? event.start_timestamp * 1000 : Date.now(); const transactionEndMs = typeof event.timestamp === 'number' ? event.timestamp * 1000 : Date.now(); const profile: Profile = { - event_id: processedProfile.profile_id, + event_id: profile_id, timestamp: new Date(transactionStartMs).toISOString(), platform: 'javascript', version: '1', @@ -420,8 +418,8 @@ export function isValidSampleRate(rate: unknown): boolean { return true; } -function isValidProfile(profile: ProcessedJSSelfProfile): profile is ProcessedJSSelfProfile & { profile_id: string } { - if (profile.samples.length <= 1) { +function isValidProfile(profile: JSSelfProfile): profile is JSSelfProfile & { profile_id: string } { + if (profile.samples.length < 2) { if (__DEBUG_BUILD__) { // Log a warning if the profile has less than 2 samples so users can know why // they are not seeing any profiling data and we cant avoid the back and forth @@ -431,7 +429,10 @@ function isValidProfile(profile: ProcessedJSSelfProfile): profile is ProcessedJS return false; } - if (!profile.profile_id) { + if (!profile.frames.length) { + if(__DEBUG_BUILD__) { + logger.log('[Profiling] Discarding profile because it contains no frames'); + } return false; } @@ -443,24 +444,23 @@ function isValidProfile(profile: ProcessedJSSelfProfile): profile is ProcessedJS * @param event * @returns {Profile | null} */ -export function createProfilingEvent(profile: ProcessedJSSelfProfile, event: ProfiledEvent): Profile | null { +export function createProfilingEvent(profile_id: string, profile: JSSelfProfile, event: ProfiledEvent): Profile | null { if (!isValidProfile(profile)) { return null; } - return createProfilePayload(event, profile); + return createProfilePayload(event, profile, profile_id); } - -export const PROFILE_MAP: Map = new Map(); +export const PROFILE_MAP: Map = new Map(); /** * */ -export function addProfileToMap(profile: ProcessedJSSelfProfile): void{ - PROFILE_MAP.set(profile.profile_id, profile); +export function addProfileToMap(profile_id: string, profile: JSSelfProfile): void { + PROFILE_MAP.set(profile_id, profile); - if(PROFILE_MAP.size > 30){ - const last: string = [...PROFILE_MAP.keys()].pop() as string; + if (PROFILE_MAP.size > 30) { + const last: string = PROFILE_MAP.keys().next().value; PROFILE_MAP.delete(last); } -} \ No newline at end of file +}