Skip to content

Commit

Permalink
ref: cleanup types and drop separate profile_id
Browse files Browse the repository at this point in the history
  • Loading branch information
JonasBa committed Jun 22, 2023
1 parent 25d30ae commit c7078fb
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 50 deletions.
17 changes: 2 additions & 15 deletions packages/browser/src/profiling/hubextensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import type {
JSSelfProfile,
JSSelfProfiler,
JSSelfProfilerConstructor,
ProcessedJSSelfProfile,
} from './jsSelfProfiling';
import { addProfileToMap,isValidSampleRate } from './utils';

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 => {
Expand Down
9 changes: 5 additions & 4 deletions packages/browser/src/profiling/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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);
}
Expand Down
7 changes: 0 additions & 7 deletions packages/browser/src/profiling/jsSelfProfiling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -50,6 +46,3 @@ declare global {
}
}

export interface RawThreadCpuProfile extends JSSelfProfile {
profile_id: string;
}
48 changes: 24 additions & 24 deletions packages/browser/src/profiling/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

Expand All @@ -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;
}
Expand All @@ -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;
};
}

Expand Down Expand Up @@ -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.
Expand All @@ -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',
Expand Down Expand Up @@ -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
Expand All @@ -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;
}

Expand All @@ -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<string, ProcessedJSSelfProfile> = new Map();
export const PROFILE_MAP: Map<string, JSSelfProfile> = 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);
}
}
}

0 comments on commit c7078fb

Please sign in to comment.