Merge branch 'develop' into cg-optimize-dropundefinedkeys

Author: chargome

packages/node/src/integrations/http/SentryHttpInstrumentationBeforeOtel.ts

@@ -9,6 +9,17 @@ import { stealthWrap } from './utils';
 type Http = typeof http;
 type Https = typeof https;
 
+// The reason this "before OTEL" integration even exists is due to timing reasons. We need to be able to register a
+// `res.on('close')` handler **after** OTEL registers its own handler (which it uses to end spans), so that we can do
+// something (ie. flush) after OTEL has ended a span for a request. If you think about it like an onion:
+//
+// (Sentry after OTEL instrumentation
+//   (OTEL instrumentation
+//     (Sentry before OTEL instrumentation
+//       (orig HTTP request handler))))
+//
+// registering an instrumentation before OTEL allows us to do this for incoming requests.
+
 /**
  * A Sentry specific http instrumentation that is applied before the otel instrumentation.
  */
@@ -70,46 +81,50 @@ export class SentryHttpInstrumentationBeforeOtel extends InstrumentationBase {
 function patchResponseToFlushOnServerlessPlatforms(res: http.OutgoingMessage): void {
   // Freely extend this function with other platforms if necessary
   if (process.env.VERCEL) {
-    let markOnEndDone = (): void => undefined;
-    const onEndDonePromise = new Promise<void>(res => {
-      markOnEndDone = res;
-    });
-
-    res.on('close', () => {
-      markOnEndDone();
-    });
-
-    // eslint-disable-next-line @typescript-eslint/unbound-method
-    res.end = new Proxy(res.end, {
-      apply(target, thisArg, argArray) {
-        vercelWaitUntil(
-          new Promise<void>(finishWaitUntil => {
-            // Define a timeout that unblocks the lambda just to be safe so we're not indefinitely keeping it alive, exploding server bills
-            const timeout = setTimeout(() => {
-              finishWaitUntil();
-            }, 2000);
-
-            onEndDonePromise
-              .then(() => {
-                DEBUG_BUILD && logger.log('Flushing events before Vercel Lambda freeze');
-                return flush(2000);
-              })
-              .then(
-                () => {
-                  clearTimeout(timeout);
-                  finishWaitUntil();
-                },
-                e => {
-                  clearTimeout(timeout);
-                  DEBUG_BUILD && logger.log('Error while flushing events for Vercel:\n', e);
-                  finishWaitUntil();
-                },
-              );
-          }),
-        );
-
-        return target.apply(thisArg, argArray);
-      },
-    });
+    // In some cases res.end does not seem to be defined leading to errors if passed to Proxy
+    // https://github.com/getsentry/sentry-javascript/issues/15759
+    if (typeof res.end === 'function') {
+      let markOnEndDone = (): void => undefined;
+      const onEndDonePromise = new Promise<void>(res => {
+        markOnEndDone = res;
+      });
+
+      res.on('close', () => {
+        markOnEndDone();
+      });
+
+      // eslint-disable-next-line @typescript-eslint/unbound-method
+      res.end = new Proxy(res.end, {
+        apply(target, thisArg, argArray) {
+          vercelWaitUntil(
+            new Promise<void>(finishWaitUntil => {
+              // Define a timeout that unblocks the lambda just to be safe so we're not indefinitely keeping it alive, exploding server bills
+              const timeout = setTimeout(() => {
+                finishWaitUntil();
+              }, 2000);
+
+              onEndDonePromise
+                .then(() => {
+                  DEBUG_BUILD && logger.log('Flushing events before Vercel Lambda freeze');
+                  return flush(2000);
+                })
+                .then(
+                  () => {
+                    clearTimeout(timeout);
+                    finishWaitUntil();
+                  },
+                  e => {
+                    clearTimeout(timeout);
+                    DEBUG_BUILD && logger.log('Error while flushing events for Vercel:\n', e);
+                    finishWaitUntil();
+                  },
+                );
+            }),
+          );
+
+          return target.apply(thisArg, argArray);
+        },
+      });
+    }
   }
 }

packages/node/src/integrations/http/index.ts

@@ -142,7 +142,17 @@ export const httpIntegration = defineIntegration((options: HttpOptions = {}) =>
   return {
     name: INTEGRATION_NAME,
     setupOnce() {
-      instrumentSentryHttpBeforeOtel();
+      // Below, we instrument the Node.js HTTP API three times. 2 times Sentry-specific, 1 time OTEL specific.
+      // Due to timing reasons, we sometimes need to apply Sentry instrumentation _before_ we apply the OTEL
+      // instrumentation (e.g. to flush on serverless platforms), and sometimes we need to apply Sentry instrumentation
+      // _after_ we apply OTEL instrumentation (e.g. for isolation scope handling and breadcrumbs).
+
+      // This is Sentry-specific instrumentation that is applied _before_ any OTEL instrumentation.
+      if (process.env.VERCEL) {
+        // Currently this instrumentation only does something when deployed on Vercel, so to save some overhead, we short circuit adding it here only for Vercel.
+        // If it's functionality is extended in the future, feel free to remove the if statement and this comment.
+        instrumentSentryHttpBeforeOtel();
+      }
 
       const instrumentSpans = _shouldInstrumentSpans(options, getClient<NodeClient>()?.getOptions());
 
@@ -152,9 +162,7 @@ export const httpIntegration = defineIntegration((options: HttpOptions = {}) =>
         instrumentOtelHttp(instrumentationConfig);
       }
 
-      // This is the Sentry-specific instrumentation that isolates requests & creates breadcrumbs
-      // Note that this _has_ to be wrapped after the OTEL instrumentation,
-      // otherwise the isolation will not work correctly
+      // This is Sentry-specific instrumentation that is applied _after_ any OTEL instrumentation.
       instrumentSentryHttp({
         ...options,
         // If spans are not instrumented, it means the HttpInstrumentation has not been added

packages/profiling-node/src/integration.ts

@@ -516,7 +516,7 @@ class ContinuousProfiler {
     const metadata = this._client.getSdkMetadata();
     const tunnel = this._client.getOptions().tunnel;
 
-    const envelope = makeProfileChunkEnvelope(chunk, metadata?.sdk, tunnel, dsn);
+    const envelope = makeProfileChunkEnvelope('node', chunk, metadata?.sdk, tunnel, dsn);
     transport.send(envelope).then(null, reason => {
       DEBUG_BUILD && logger.error('Error while sending profile chunk envelope:', reason);
     });

packages/profiling-node/src/utils.ts

@@ -400,13 +400,15 @@ export function createEventEnvelopeHeaders(
  * Creates a standalone profile_chunk envelope.
  */
 export function makeProfileChunkEnvelope(
+  platform: 'node',
   chunk: ProfileChunk,
   sdkInfo: SdkInfo | undefined,
   tunnel: string | undefined,
   dsn?: DsnComponents,
 ): ProfileChunkEnvelope {
   const profileChunkHeader: ProfileChunkItem[0] = {
     type: 'profile_chunk',
+    platform,
   };
 
   return createEnvelope<ProfileChunkEnvelope>(createEventEnvelopeHeaders(sdkInfo, tunnel, dsn), [

packages/profiling-node/test/integration.test.ts

@@ -643,7 +643,9 @@ describe('ProfilingIntegration', () => {
       Sentry.profiler.stopProfiler();
       vi.advanceTimersByTime(1000);
 
-      expect(transportSpy.mock.calls?.[0]?.[0]?.[1]?.[0]?.[0]?.type).toBe('profile_chunk');
+      const envelopeHeaders = transportSpy.mock.calls?.[0]?.[0]?.[1]?.[0]?.[0];
+      expect(envelopeHeaders?.type).toBe('profile_chunk');
+      expect(envelopeHeaders?.platform).toBe('node');
     });
 
     it('sets global profile context', async () => {