ref(profiling): move profiling under browser package

package.json

@@ -49,7 +49,6 @@
     "packages/node",
     "packages/node-integration-tests",
     "packages/opentelemetry-node",
-    "packages/profiling-browser",
     "packages/react",
     "packages/remix",
     "packages/replay",

packages/browser/src/index.ts

@@ -32,3 +32,8 @@ export { Replay } from '@sentry/replay';
 // __ROLLUP_EXCLUDE_OFFLINE_FROM_BUNDLES_BEGIN__
 export { makeBrowserOfflineTransport } from './transports/offline';
 // __ROLLUP_EXCLUDE_OFFLINE_FROM_BUNDLES_END__
+
+// __ROLLUP_EXCLUDE_BROWSER_PROFILING_FROM_BUNDLES_BEGIN__
+export { wrapTransactionWithProfiling } from './profiling/hubextensions';
+export { BrowserProfilingIntegration } from './profiling/browserProfiling';
+// __ROLLUP_EXCLUDE_BROWSER_PROFILING_FROM_BUNDLES_END__

packages/profiling-browser/src/integration.ts → packages/browser/src/profiling/browserProfiling.ts

@@ -1,17 +1,82 @@
 import { getCurrentHub } from '@sentry/core';
 import type { Event, EventProcessor, Hub, Integration } from '@sentry/types';
 import { logger } from '@sentry/utils';
-import { LRUMap } from 'lru_map';
 
+import { addExtensionMethods } from './hubextensions';
 import type { ProcessedJSSelfProfile } from './jsSelfProfiling';
 import type { ProfiledEvent } from './utils';
 import { createProfilingEventEnvelope } from './utils';
 
-// We need this integration in order to actually send data to Sentry. We hook into the event processor
-// and inspect each event to see if it is a transaction event and if that transaction event
-// contains a profile on it's metadata. If that is the case, we create a profiling event envelope
-// and delete the profile from the transaction metadata.
-export const PROFILING_EVENT_CACHE = new LRUMap<string, Event>(20);
+/**
+ * Creates a simple cache that evicts keys in fifo size
+ * @param size {Number}
+ */
+export function makeProfilingCache<Key extends string, Value extends Event>(
+  size: number,
+): {
+  get: (key: Key) => Value | undefined;
+  add: (key: Key, value: Value) => void;
+  delete: (key: Key) => boolean;
+  clear: () => void;
+  size: () => number;
+} {
+  // Maintain a fifo queue of keys, we cannot rely on Object.keys as the browser may not support it.
+  let evictionOrder: Key[] = [];
+  let cache: Record<string, Value> = {};
+
+  return {
+    add(key: Key, value: Value) {
+      while (evictionOrder.length >= size) {
+        // shift is O(n) but this is small size and only happens if we are
+        // exceeding the cache size so it should be fine.
+        const evictCandidate = evictionOrder.shift();
+
+        if (evictCandidate !== undefined) {
+          // eslint-disable-next-line @typescript-eslint/no-dynamic-delete
+          delete cache[evictCandidate];
+        }
+      }
+
+      // in case we have a collision, delete the old key.
+      if (cache[key]) {
+        this.delete(key);
+      }
+
+      evictionOrder.push(key);
+      cache[key] = value;
+    },
+    clear() {
+      cache = {};
+      evictionOrder = [];
+    },
+    get(key: Key): Value | undefined {
+      return cache[key];
+    },
+    size(){
+      return evictionOrder.length
+    },
+    // Delete cache key and return true if it existed, false otherwise.
+    delete(key: Key): boolean {
+      if (!cache[key]) {
+        return false;
+      }
+
+      // eslint-disable-next-line @typescript-eslint/no-dynamic-delete
+      delete cache[key];
+
+      for (let i = 0; i < evictionOrder.length; i++) {
+        if (evictionOrder[i] === key) {
+          evictionOrder.splice(i, 1);
+          break;
+        }
+      }
+
+      return true;
+    },
+  };
+}
+
+export const PROFILING_EVENT_CACHE = makeProfilingCache<string, Event>(20);
 /**
  * Browser profiling integration. Stores any event that has contexts["profile"]["profile_id"]
  * This exists because we do not want to await async profiler.stop calls as transaction.finish is called
@@ -21,27 +86,32 @@ export const PROFILING_EVENT_CACHE = new LRUMap<string, Event>(20);
  */
 export class BrowserProfilingIntegration implements Integration {
   public readonly name: string = 'BrowserProfilingIntegration';
-  public getCurrentHub?: () => Hub = undefined;
 
   /**
    * @inheritDoc
    */
-  public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
-    this.getCurrentHub = getCurrentHub;
+  public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, _getCurrentHub: () => Hub): void {
+    // Patching the hub to add the extension methods.
+    // Warning: we have an implicit dependency on import order and we will fail patching if the constructor of
+    // BrowserProfilingIntegration is called before @sentry/tracing is imported. This is because we need to patch
+    // the methods of @sentry/tracing which are patched as a side effect of importing @sentry/tracing.
+    addExtensionMethods();
+
+    // Add our event processor
     addGlobalEventProcessor(this.handleGlobalEvent.bind(this));
   }
 
   /**
    * @inheritDoc
    */
   public handleGlobalEvent(event: Event): Event {
-    const profile_id = event.contexts && event.contexts['profile'] && event.contexts['profile']['profile_id'];
+    const profileId = event.contexts && event.contexts['profile'] && event.contexts['profile']['profile_id'];
 
-    if (profile_id && typeof profile_id === 'string') {
+    if (profileId && typeof profileId === 'string') {
       if (__DEBUG_BUILD__) {
         logger.log('[Profiling] Profiling event found, caching it.');
       }
-      PROFILING_EVENT_CACHE.set(profile_id, event);
+      PROFILING_EVENT_CACHE.add(profileId, event);
     }
 
     return event;
@@ -53,8 +123,8 @@ export class BrowserProfilingIntegration implements Integration {
  * If the profiled transaction event is found, we use the profiled transaction event and profile
  * to construct a profile type envelope and send it to Sentry.
  */
-export function sendProfile(profile_id: string, profile: ProcessedJSSelfProfile): void {
-  const event = PROFILING_EVENT_CACHE.get(profile_id);
+export function sendProfile(profileId: string, profile: ProcessedJSSelfProfile): void {
+  const event = PROFILING_EVENT_CACHE.get(profileId);
 
   if (!event) {
     // We could not find a corresponding transaction event for this profile.
@@ -112,7 +182,7 @@ export function sendProfile(profile_id: string, profile: ProcessedJSSelfProfile)
   const envelope = createProfilingEventEnvelope(event as ProfiledEvent, dsn);
 
   // Evict event from the cache - we want to prevent the LRU cache from prioritizing already sent events over new ones.
-  PROFILING_EVENT_CACHE.delete(profile_id);
+  PROFILING_EVENT_CACHE.delete(profileId);
 
   if (!envelope) {
     if (__DEBUG_BUILD__) {

packages/profiling-browser/src/hubextensions.ts → packages/browser/src/profiling/hubextensions.ts

@@ -1,9 +1,9 @@
-import { WINDOW } from '@sentry/browser';
 import { getMainCarrier } from '@sentry/core';
 import type { CustomSamplingContext, Hub, Transaction, TransactionContext } from '@sentry/types';
 import { logger, uuid4 } from '@sentry/utils';
 
-import { sendProfile } from './integration';
+import { WINDOW } from '../helpers';
+import { sendProfile } from './browserProfiling';
 import type { JSSelfProfile, JSSelfProfiler, ProcessedJSSelfProfile } from './jsSelfProfiling';
 
 // Max profile duration.
@@ -34,11 +34,18 @@ export function wrapTransactionWithProfiling(
   transactionContext: TransactionContext,
   getCurrentHub: () => Hub,
 ): Transaction | undefined {
-  // 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
-  // calling the profiler methods. Note: we log the original name to the user to avoid confusion.
-  const profile_id = uuid4();
+  // Feature support check first
+  const JSProfiler = WINDOW.Profiler;
+  if (!isJSProfilerSupported(JSProfiler)) {
+    if (__DEBUG_BUILD__) {
+      logger.log(
+        '[Profiling] Profiling is not supported by this browser, Profiler interface missing on window object.',
+      );
+    }
+    return transaction;
+  }
 
+  // Check if we have a valid
   if (!transaction) {
     if (__DEBUG_BUILD__) {
       logger.log(`[Profiling] transaction not found, skipping profiling for: ${transactionContext.name}`);
@@ -57,18 +64,6 @@ export function wrapTransactionWithProfiling(
   const client = getCurrentHub().getClient();
   const options = client && client.getOptions();
 
-  const JSProfiler = WINDOW.Profiler;
-
-  // Feature support check
-  if (!isJSProfilerSupported(JSProfiler)) {
-    if (__DEBUG_BUILD__) {
-      logger.log(
-        '[Profiling] Profiling is not supported by this browser, Profiler interface missing on window object.',
-      );
-    }
-    return transaction;
-  }
-
   // @ts-ignore not part of the browser options yet
   const profilesSampleRate = (options && options.profilesSampleRate) || 0;
   if (profilesSampleRate === undefined) {
@@ -86,16 +81,20 @@ export function wrapTransactionWithProfiling(
     return transaction;
   }
 
-  // Defer any profilesSamplingInterval validation to the profiler API
-  // @ts-ignore not part of the browser options yet and might never be, but useful to control during poc stage
-  const samplingInterval = options.profilesSamplingInterval || 10;
+  // 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 / samplingInterval);
-  const profiler = new JSProfiler({ sampleInterval: samplingInterval, maxBufferSize: maxSamples });
+  const maxSamples = Math.floor(MAX_PROFILE_DURATION_MS / samplingIntervalMS);
+  const profiler = new JSProfiler({ sampleInterval: samplingIntervalMS, maxBufferSize: maxSamples });
   if (__DEBUG_BUILD__) {
     logger.log(`[Profiling] started profiling transaction: ${transaction.name || transaction.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
+  // 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
@@ -152,8 +151,8 @@ export function wrapTransactionWithProfiling(
           return;
         }
 
-        processedProfile = { ...p, profile_id: profile_id };
-        sendProfile(profile_id, processedProfile);
+        processedProfile = { ...p, profile_id: profileId };
+        sendProfile(profileId, processedProfile);
       })
       .catch(error => {
         if (__DEBUG_BUILD__) {
@@ -191,7 +190,7 @@ export function wrapTransactionWithProfiling(
     onProfileHandler();
 
     // Set profile context
-    transaction.setContext('profile', { profile_id });
+    transaction.setContext('profile', { profile_id: profileId });
 
     return originalFinish();
   }
@@ -201,9 +200,9 @@ export function wrapTransactionWithProfiling(
 }
 
 /**
- *
+ * Wraps startTransaction with profiling logic. This is done automatically by the profiling integration.
  */
-export function __PRIVATE__wrapStartTransactionWithProfiling(startTransaction: StartTransaction): StartTransaction {
+function __PRIVATE__wrapStartTransactionWithProfiling(startTransaction: StartTransaction): StartTransaction {
   return function wrappedStartTransaction(
     this: Hub,
     transactionContext: TransactionContext,
@@ -240,6 +239,7 @@ function _addProfilingExtensionMethods(): void {
   if (__DEBUG_BUILD__) {
     logger.log('[Profiling] startTransaction exists, patching it with profiling functionality...');
   }
+
   carrier.__SENTRY__.extensions['startTransaction'] = __PRIVATE__wrapStartTransactionWithProfiling(
     // This is already patched by sentry/tracing, we are going to re-patch it...
     carrier.__SENTRY__.extensions['startTransaction'] as StartTransaction,

packages/profiling-browser/src/utils.ts → packages/browser/src/profiling/utils.ts

@@ -1,4 +1,3 @@
-import { WINDOW } from '@sentry/browser';
 import type {
   DsnComponents,
   DynamicSamplingContext,
@@ -11,16 +10,20 @@ import type {
 } from '@sentry/types';
 import { createEnvelope, dropUndefinedKeys, dsnToString, logger, uuid4 } from '@sentry/utils';
 
+import { WINDOW } from '../helpers';
 import type { JSSelfProfile, JSSelfProfileStack, RawThreadCpuProfile, ThreadCpuProfile } from './jsSelfProfiling';
 
+const MS_TO_NS = 1e6;
+// Use 0 as main thread id which is identical to threadId in node:worker_threads
+// where main logs 0 and workers seem to log in increments of 1
 const THREAD_ID_STRING = String(0);
 const THREAD_NAME = 'main';
 
 // Machine properties (eval only once)
 let OS_PLATFORM = ''; // macos
 let OS_PLATFORM_VERSION = ''; // 13.2
 let OS_ARCH = ''; // arm64
-let OS_BROWSER = WINDOW.navigator && WINDOW.navigator.userAgent || '';
+let OS_BROWSER = (WINDOW.navigator && WINDOW.navigator.userAgent) || '';
 let OS_MODEL = '';
 const OS_LOCALE =
   (WINDOW.navigator && WINDOW.navigator.language) || (WINDOW.navigator && WINDOW.navigator.languages[0]) || '';
@@ -203,9 +206,10 @@ function getTraceId(event: Event): string {
  * @param tunnel
  * @returns {EventEnvelope | null}
  */
-// We will live dangerously and disable complexity here, time is of the essence.
-// Onwards to the next refactor my fellow engineers!
-// eslint-disable-next-line complexity
+
+/**
+ * Creates a profiling event envelope from a Sentry event.
+ */
 export function createProfilingEventEnvelope(
   event: ProfiledEvent,
   dsn: DsnComponents,
@@ -257,7 +261,7 @@ export function createProfilingEventEnvelope(
     environment: event.environment || '',
     runtime: {
       name: 'javascript',
-      version: WINDOW.navigator.userAgent
+      version: WINDOW.navigator.userAgent,
     },
     os: {
       name: OS_PLATFORM,
@@ -325,6 +329,7 @@ export function convertJSSelfProfileToSampledFormat(input: JSSelfProfile): Threa
   let EMPTY_STACK_ID: undefined | number = undefined;
   let STACK_ID = 0;
 
+  // Initialize the profile that we will fill with data
   const profile: ThreadCpuProfile = {
     samples: [],
     stacks: [],
@@ -338,9 +343,8 @@ export function convertJSSelfProfileToSampledFormat(input: JSSelfProfile): Threa
     return profile;
   }
 
-  // We assert samples.length > 0 above
+  // We assert samples.length > 0 above and timestamp should always be present
   const start = input.samples[0].timestamp;
-  profile.stacks = [];
 
   for (let i = 0; i < input.samples.length; i++) {
     const jsSample = input.samples[i];
@@ -354,7 +358,8 @@ export function convertJSSelfProfileToSampledFormat(input: JSSelfProfile): Threa
       }
 
       profile['samples'][i] = {
-        elapsed_since_start_ns: ((jsSample.timestamp - start) * 1e6).toFixed(0),
+        // convert ms timestamp to ns
+        elapsed_since_start_ns: ((jsSample.timestamp - start) * MS_TO_NS).toFixed(0),
         stack_id: EMPTY_STACK_ID,
         thread_id: THREAD_ID_STRING,
       };
@@ -386,7 +391,8 @@ export function convertJSSelfProfileToSampledFormat(input: JSSelfProfile): Threa
     }
 
     const sample: ThreadCpuProfile['samples'][0] = {
-      elapsed_since_start_ns: ((jsSample.timestamp - start) * 1e6).toFixed(0),
+      // convert ms timestamp to ns
+      elapsed_since_start_ns: ((jsSample.timestamp - start) * MS_TO_NS).toFixed(0),
       stack_id: STACK_ID,
       thread_id: THREAD_ID_STRING,
     };