feat(core): Ensure normalizedRequest on sdkProcessingMetadata is merged

.size-limit.js

@@ -107,7 +107,7 @@ module.exports = [
     path: 'packages/browser/build/npm/esm/index.js',
     import: createImport('init', 'feedbackAsyncIntegration'),
     gzip: true,
-    limit: '33 KB',
+    limit: '34 KB',
   },
   // React SDK (ESM)
   {
@@ -160,7 +160,7 @@ module.exports = [
     name: 'CDN Bundle (incl. Tracing)',
     path: createCDNPath('bundle.tracing.min.js'),
     gzip: true,
-    limit: '38 KB',
+    limit: '39 KB',
   },
   {
     name: 'CDN Bundle (incl. Tracing, Replay)',

packages/core/src/scope.ts

@@ -24,6 +24,7 @@ import type {
 import { dateTimestampInSeconds, generatePropagationContext, isPlainObject, logger, uuid4 } from '@sentry/utils';
 
 import { updateSession } from './session';
+import { merge } from './utils/merge';
 import { _getSpanForScope, _setSpanForScope } from './utils/spanOnScope';
 
 /**
@@ -479,8 +480,7 @@ class ScopeClass implements ScopeInterface {
    * @inheritDoc
    */
   public setSDKProcessingMetadata(newData: { [key: string]: unknown }): this {
-    this._sdkProcessingMetadata = { ...this._sdkProcessingMetadata, ...newData };
-
+    this._sdkProcessingMetadata = merge(this._sdkProcessingMetadata, newData, 2);
     return this;
   }
 

packages/core/src/utils/applyScopeDataToEvent.ts

@@ -1,6 +1,7 @@
 import type { Breadcrumb, Event, ScopeData, Span } from '@sentry/types';
 import { arrayify, dropUndefinedKeys } from '@sentry/utils';
 import { getDynamicSamplingContextFromSpan } from '../tracing/dynamicSamplingContext';
+import { merge } from './merge';
 import { getRootSpan, spanToJSON, spanToTraceContext } from './spanUtils';
 
 /**
@@ -46,7 +47,8 @@ export function mergeScopeData(data: ScopeData, mergeData: ScopeData): void {
   mergeAndOverwriteScopeData(data, 'tags', tags);
   mergeAndOverwriteScopeData(data, 'user', user);
   mergeAndOverwriteScopeData(data, 'contexts', contexts);
-  mergeAndOverwriteScopeData(data, 'sdkProcessingMetadata', sdkProcessingMetadata);
+
+  data.sdkProcessingMetadata = merge(data.sdkProcessingMetadata, sdkProcessingMetadata, 2);
 
   if (level) {
     data.level = level;
@@ -87,15 +89,7 @@ export function mergeAndOverwriteScopeData<
   Prop extends 'extra' | 'tags' | 'user' | 'contexts' | 'sdkProcessingMetadata',
   Data extends ScopeData,
 >(data: Data, prop: Prop, mergeVal: Data[Prop]): void {
-  if (mergeVal && Object.keys(mergeVal).length) {
-    // Clone object
-    data[prop] = { ...data[prop] };
-    for (const key in mergeVal) {
-      if (Object.prototype.hasOwnProperty.call(mergeVal, key)) {
-        data[prop][key] = mergeVal[key];
-      }
-    }
-  }
+  data[prop] = merge(data[prop], mergeVal, 1);
 }
 
 /** Exported only for tests */

packages/core/src/utils/merge.ts

@@ -0,0 +1,31 @@
+/**
+ * Shallow merge two objects.
+ * Does not mutate the passed in objects.
+ * Undefined/empty values in the merge object will overwrite existing values.
+ *
+ * By default, this merges 2 levels deep.
+ */
+export function merge<T>(initialObj: T, mergeObj: T, levels = 2): T {
+  // If the merge value is not an object, or we have no merge levels left,
+  // we just set the value to the merge value
+  if (!mergeObj || typeof mergeObj !== 'object' || levels <= 0) {
+    return mergeObj;
+  }
+
+  // If the merge object is an empty object, and the initial object is not undefined, we return the initial object
+  if (initialObj && mergeObj && Object.keys(mergeObj).length === 0) {
+    return initialObj;
+  }
+
+  // Clone object
+  const output = { ...initialObj };
+
+  // Merge values into output, resursively
+  for (const key in mergeObj) {
+    if (Object.prototype.hasOwnProperty.call(mergeObj, key)) {
+      output[key] = merge(output[key], mergeObj[key], levels - 1);
+    }
+  }
+
+  return output;
+}

packages/core/test/lib/scope.test.ts

@@ -204,10 +204,27 @@ describe('Scope', () => {
       expect(scope['_user']).toEqual({});
     });
 
-    test('setProcessingMetadata', () => {
-      const scope = new Scope();
-      scope.setSDKProcessingMetadata({ dogs: 'are great!' });
-      expect(scope['_sdkProcessingMetadata'].dogs).toEqual('are great!');
+    describe('setProcessingMetadata', () => {
+      test('it works with no initial data', () => {
+        const scope = new Scope();
+        scope.setSDKProcessingMetadata({ dogs: 'are great!' });
+        expect(scope['_sdkProcessingMetadata'].dogs).toEqual('are great!');
+      });
+
+      test('it overwrites data', () => {
+        const scope = new Scope();
+        scope.setSDKProcessingMetadata({ dogs: 'are great!' });
+        scope.setSDKProcessingMetadata({ dogs: 'are really great!' });
+        scope.setSDKProcessingMetadata({ cats: 'are also great!' });
+        scope.setSDKProcessingMetadata({ obj: { nested1: 'value1', nested: 'value1' } });
+        scope.setSDKProcessingMetadata({ obj: { nested2: 'value2', nested: 'value2' } });
+
+        expect(scope['_sdkProcessingMetadata']).toEqual({
+          dogs: 'are really great!',
+          cats: 'are also great!',
+          obj: { nested2: 'value2', nested: 'value2', nested1: 'value1' },
+        });
+      });
     });
 
     test('set and get propagation context', () => {

packages/core/test/lib/utils/applyScopeDataToEvent.test.ts

@@ -134,7 +134,15 @@ describe('mergeScopeData', () => {
       contexts: { os: { name: 'os1' }, culture: { display_name: 'name1' } },
       attachments: [attachment1],
       propagationContext: { spanId: '1', traceId: '1' },
-      sdkProcessingMetadata: { aa: 'aa', bb: 'aa' },
+      sdkProcessingMetadata: {
+        aa: 'aa',
+        bb: 'aa',
+        obj: { key: 'value' },
+        normalizedRequest: {
+          url: 'oldUrl',
+          method: 'oldMethod',
+        },
+      },
       fingerprint: ['aa', 'bb'],
     };
     const data2: ScopeData = {
@@ -146,7 +154,15 @@ describe('mergeScopeData', () => {
       contexts: { os: { name: 'os2' } },
       attachments: [attachment2, attachment3],
       propagationContext: { spanId: '2', traceId: '2' },
-      sdkProcessingMetadata: { bb: 'bb', cc: 'bb' },
+      sdkProcessingMetadata: {
+        bb: 'bb',
+        cc: 'bb',
+        obj: { key2: 'value2' },
+        normalizedRequest: {
+          url: 'newUrl',
+          headers: {},
+        },
+      },
       fingerprint: ['cc'],
     };
     mergeScopeData(data1, data2);
@@ -159,7 +175,17 @@ describe('mergeScopeData', () => {
       contexts: { os: { name: 'os2' }, culture: { display_name: 'name1' } },
       attachments: [attachment1, attachment2, attachment3],
       propagationContext: { spanId: '2', traceId: '2' },
-      sdkProcessingMetadata: { aa: 'aa', bb: 'bb', cc: 'bb' },
+      sdkProcessingMetadata: {
+        aa: 'aa',
+        bb: 'bb',
+        cc: 'bb',
+        obj: { key: 'value', key2: 'value2' },
+        normalizedRequest: {
+          url: 'newUrl',
+          method: 'oldMethod',
+          headers: {},
+        },
+      },
       fingerprint: ['aa', 'bb', 'cc'],
     });
   });

packages/core/test/lib/utils/merge.test.ts

@@ -0,0 +1,75 @@
+import { merge } from '../../../src/utils/merge';
+
+describe('merge', () => {
+  it('works with empty objects', () => {
+    const oldData = {};
+    const newData = {};
+
+    const actual = merge(oldData, newData);
+
+    expect(actual).toEqual({});
+    expect(actual).toBe(oldData);
+    expect(actual).not.toBe(newData);
+  });
+
+  it('works with empty merge object', () => {
+    const oldData = { aa: 'aha' };
+    const newData = {};
+
+    const actual = merge(oldData, newData);
+
+    expect(actual).toEqual({ aa: 'aha' });
+    expect(actual).toBe(oldData);
+    expect(actual).not.toBe(newData);
+  });
+
+  it('works with arbitrary data', () => {
+    const oldData = {
+      old1: 'old1',
+      old2: 'old2',
+      obj: { key: 'value1', key1: 'value1', deep: { key: 'value' } },
+    } as any;
+    const newData = {
+      new1: 'new1',
+      old2: 'new2',
+      obj: { key2: 'value2', key: 'value2', deep: { key2: 'value2' } },
+    } as any;
+
+    const actual = merge(oldData, newData);
+
+    expect(actual).toEqual({
+      old1: 'old1',
+      old2: 'new2',
+      new1: 'new1',
+      obj: {
+        key2: 'value2',
+        key: 'value2',
+        key1: 'value1',
+        deep: { key2: 'value2' },
+      },
+    });
+    expect(actual).not.toBe(oldData);
+    expect(actual).not.toBe(newData);
+  });
+
+  it.each([
+    [undefined, { a: 'aa' }, { a: 'aa' }],
+    [{ a: 'aa' }, undefined, undefined],
+    [{ a: 'aa' }, null, null],
+    [{ a: 'aa' }, { a: undefined }, { a: undefined }],
+    [{ a: 'aa' }, { a: null }, { a: null }],
+    [{ a: 'aa' }, { a: '' }, { a: '' }],
+    [
+      { a0: { a1: { a2: { a3: { a4: 'a4a' }, a3a: 'a3a' }, a2a: 'a2a' }, a1a: 'a1a' }, a0a: 'a0a' },
+      { a0: { a1: { a2: { a3: { a4: 'a4b' }, a3b: 'a3b' }, a2b: 'a2b' }, a1b: 'a1b' }, a0b: 'a0b' },
+      {
+        a0: { a1: { a2: { a3: { a4: 'a4b' }, a3b: 'a3b' }, a2b: 'a2b' }, a1b: 'a1b', a1a: 'a1a' },
+        a0b: 'a0b',
+        a0a: 'a0a',
+      },
+    ],
+  ])('works with %p and %p', (oldData, newData, expected) => {
+    const actual = merge(oldData, newData as any);
+    expect(actual).toEqual(expected);
+  });
+});

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

@@ -6,7 +6,7 @@ import type { InstrumentationConfig } from '@opentelemetry/instrumentation';
 import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation';
 import { getRequestInfo } from '@opentelemetry/instrumentation-http';
 import { addBreadcrumb, getClient, getIsolationScope, withIsolationScope } from '@sentry/core';
-import type { PolymorphicRequest, RequestEventData, SanitizedRequestData } from '@sentry/types';
+import type { PolymorphicRequest, RequestEventData, SanitizedRequestData, Scope } from '@sentry/types';
 import {
   getBreadcrumbLogLevelFromHttpStatusCode,
   getSanitizedUrlString,
@@ -150,10 +150,14 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns
           cookies,
         };
 
-        patchRequestToCaptureBody(request, normalizedRequest);
+        patchRequestToCaptureBody(request, isolationScope);
 
         // Update the isolation scope, isolate this request
-        isolationScope.setSDKProcessingMetadata({ request, normalizedRequest });
+        // TODO(v9): Stop setting `request`, we only rely on normalizedRequest anymore
+        isolationScope.setSDKProcessingMetadata({
+          request,
+          normalizedRequest,
+        });
 
         const client = getClient<NodeClient>();
         if (client && client.getOptions().autoSessionTracking) {
@@ -347,7 +351,7 @@ function getBreadcrumbData(request: http.ClientRequest): Partial<SanitizedReques
  * we monkey patch `req.on('data')` to intercept the body chunks.
  * This way, we only read the body if the user also consumes the body, ensuring we do not change any behavior in unexpected ways.
  */
-function patchRequestToCaptureBody(req: IncomingMessage, normalizedRequest: RequestEventData): void {
+function patchRequestToCaptureBody(req: IncomingMessage, isolationScope: Scope): void {
   const chunks: Buffer[] = [];
 
   function getChunksSize(): number {
@@ -396,9 +400,9 @@ function patchRequestToCaptureBody(req: IncomingMessage, normalizedRequest: Requ
               try {
                 const body = Buffer.concat(chunks).toString('utf-8');
 
-                // We mutate the passed in normalizedRequest and add the body to it
                 if (body) {
-                  normalizedRequest.data = body;
+                  const normalizedRequest = { data: body } satisfies RequestEventData;
+                  isolationScope.setSDKProcessingMetadata({ normalizedRequest });
                 }
               } catch {
                 // ignore errors here

packages/types/src/scope.ts

@@ -217,7 +217,9 @@ export interface Scope {
   clearAttachments(): this;
 
   /**
-   * Add data which will be accessible during event processing but won't get sent to Sentry
+   * Add data which will be accessible during event processing but won't get sent to Sentry.
+   *
+   * TODO(v9): We should type this stricter, so that e.g. `normalizedRequest` is strictly typed.
    */
   setSDKProcessingMetadata(newData: { [key: string]: unknown }): this;