use generic merge instead

Author: mydea

packages/core/src/index.ts

@@ -68,7 +68,6 @@ export { hasTracingEnabled } from './utils/hasTracingEnabled';
 export { isSentryRequestUrl } from './utils/isSentryRequestUrl';
 export { handleCallbackErrors } from './utils/handleCallbackErrors';
 export { parameterize } from './utils/parameterize';
-export { setRequestEventData } from './utils/sdkProcessingMetadata';
 export {
   spanToTraceHeader,
   spanToJSON,

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,7 +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,7 +1,7 @@
 import type { Breadcrumb, Event, ScopeData, Span } from '@sentry/types';
 import { arrayify, dropUndefinedKeys } from '@sentry/utils';
 import { getDynamicSamplingContextFromSpan } from '../tracing/dynamicSamplingContext';
-import { mergeSdkProcessingMetadata } from './sdkProcessingMetadata';
+import { merge } from './merge';
 import { getRootSpan, spanToJSON, spanToTraceContext } from './spanUtils';
 
 /**
@@ -48,7 +48,7 @@ export function mergeScopeData(data: ScopeData, mergeData: ScopeData): void {
   mergeAndOverwriteScopeData(data, 'user', user);
   mergeAndOverwriteScopeData(data, 'contexts', contexts);
 
-  data.sdkProcessingMetadata = mergeSdkProcessingMetadata(data.sdkProcessingMetadata, sdkProcessingMetadata);
+  data.sdkProcessingMetadata = merge(data.sdkProcessingMetadata, sdkProcessingMetadata, 2);
 
   if (level) {
     data.level = level;
@@ -89,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,29 @@
+/**
+ * Shallow merge two objects.
+ * Does not mutate the passed in objects.
+ * 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 (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/src/utils/sdkProcessingMetadata.ts

@@ -216,13 +216,13 @@ describe('Scope', () => {
         scope.setSDKProcessingMetadata({ dogs: 'are great!' });
         scope.setSDKProcessingMetadata({ dogs: 'are really great!' });
         scope.setSDKProcessingMetadata({ cats: 'are also great!' });
-        scope.setSDKProcessingMetadata({ obj: { nested: 'value1' } });
-        scope.setSDKProcessingMetadata({ obj: { nested2: 'value2' } });
+        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' },
+          obj: { nested2: 'value2', nested: 'value2', nested1: 'value1' },
         });
       });
     });

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

@@ -157,9 +157,7 @@ describe('mergeScopeData', () => {
       sdkProcessingMetadata: {
         bb: 'bb',
         cc: 'bb',
-        // Regular objects are not deep merged
         obj: { key2: 'value2' },
-        // Only normalizedRequest is deep merged
         normalizedRequest: {
           url: 'newUrl',
           headers: {},
@@ -181,7 +179,7 @@ describe('mergeScopeData', () => {
         aa: 'aa',
         bb: 'bb',
         cc: 'bb',
-        obj: { key2: 'value2' },
+        obj: { key: 'value', key2: 'value2' },
         normalizedRequest: {
           url: 'newUrl',
           method: 'oldMethod',

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

@@ -0,0 +1,54 @@
+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);
+  });
+});

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

@@ -5,7 +5,7 @@ import { VERSION } from '@opentelemetry/core';
 import type { InstrumentationConfig } from '@opentelemetry/instrumentation';
 import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation';
 import { getRequestInfo } from '@opentelemetry/instrumentation-http';
-import { addBreadcrumb, getClient, getIsolationScope, setRequestEventData, withIsolationScope } from '@sentry/core';
+import { addBreadcrumb, getClient, getIsolationScope, withIsolationScope } from '@sentry/core';
 import type { PolymorphicRequest, RequestEventData, SanitizedRequestData, Scope } from '@sentry/types';
 import {
   getBreadcrumbLogLevelFromHttpStatusCode,
@@ -154,8 +154,10 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns
 
         // Update the isolation scope, isolate this request
         // TODO(v9): Stop setting `request`, we only rely on normalizedRequest anymore
-        isolationScope.setSDKProcessingMetadata({ request });
-        setRequestEventData(normalizedRequest, isolationScope);
+        isolationScope.setSDKProcessingMetadata({
+          request,
+          normalizedRequest,
+        });
 
         const client = getClient<NodeClient>();
         if (client && client.getOptions().autoSessionTracking) {
@@ -399,7 +401,8 @@ function patchRequestToCaptureBody(req: IncomingMessage, isolationScope: Scope):
                 const body = Buffer.concat(chunks).toString('utf-8');
 
                 if (body) {
-                  setRequestEventData({ data: body }, isolationScope);
+                  const normalizedRequest = { data: body } satisfies RequestEventData;
+                  isolationScope.setSDKProcessingMetadata({ normalizedRequest });
                 }
               } catch {
                 // ignore errors here