fix: Normalize Transaction and Span consistently

This is such that setting data on a Transaction behaves the same as
setting data on a Span.

Author: rhcarvalho

packages/core/src/baseclient.ts

@@ -300,7 +300,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
     }
 
     // tslint:disable:no-unsafe-any
-    return {
+    const normalized = {
       ...event,
       ...(event.breadcrumbs && {
         breadcrumbs: event.breadcrumbs.map(b => ({
@@ -320,6 +320,17 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
         extra: normalize(event.extra, depth),
       }),
     };
+    // event.contexts.trace stores information about a Transaction. Similarly,
+    // event.spans[] stores information about child Spans. Given that a
+    // Transaction is conceptually a Span, normalization should apply to both
+    // Transactions and Spans consistently.
+    // For now the decision is to skip normalization of Transactions and Spans,
+    // so this block overwrites the normalized event to add back the original
+    // Transaction information prior to normalization.
+    if (event.contexts && event.contexts.trace) {
+      normalized.contexts.trace = event.contexts.trace;
+    }
+    return normalized;
   }
 
   /**

packages/core/test/lib/base.test.ts

@@ -1,5 +1,5 @@
 import { Hub, Scope } from '@sentry/hub';
-import { Event, Severity } from '@sentry/types';
+import { Event, Severity, Span } from '@sentry/types';
 import { SentryError } from '@sentry/utils';
 
 import { TestBackend } from '../mocks/backend';
@@ -570,6 +570,54 @@ describe('BaseClient', () => {
       });
     });
 
+    test('normalization applies to Transaction and Span consistently', () => {
+      expect.assertions(1);
+      const client = new TestClient({ dsn: PUBLIC_DSN });
+      const transaction: Event = {
+        contexts: {
+          trace: {
+            data: { _sentry_web_vitals: { LCP: { value: 99.9 } } },
+            op: 'pageload',
+            span_id: 'a3df84a60c2e4e76',
+            trace_id: '86f39e84263a4de99c326acab3bfe3bd',
+          },
+        },
+        event_id: '972f45b826a248bba98e990878a177e1',
+        spans: [
+          ({
+            data: { _sentry_extra_metrics: { M1: { value: 1 }, M2: { value: 2 } } },
+            description: 'first-paint',
+            endTimestamp: 1591603196.637835,
+            op: 'paint',
+            parentSpanId: 'a3df84a60c2e4e76',
+            spanId: '9e15bf99fbe4bc80',
+            startTimestamp: 1591603196.637835,
+            traceId: '86f39e84263a4de99c326acab3bfe3bd',
+          } as any) as Span, // `as any` to bypass linter https://palantir.github.io/tslint/rules/no-object-literal-type-assertion/
+          ({
+            description: 'first-contentful-paint',
+            endTimestamp: 1591603196.637835,
+            op: 'paint',
+            parentSpanId: 'a3df84a60c2e4e76',
+            spanId: 'aa554c1f506b0783',
+            startTimestamp: 1591603196.637835,
+            traceId: '86f39e84263a4de99c326acab3bfe3bd',
+          } as any) as Span,
+        ],
+        start_timestamp: 1591603196.614865,
+        timestamp: 1591603196.728485,
+        transaction: '/',
+        type: 'transaction',
+      };
+      // To be consistent, normalization could apply either to both transactions
+      // and spans, or to none. So far the decision is to skip normalization for
+      // both, such that the expected normalizedTransaction is the same as the
+      // input transaction.
+      const normalizedTransaction = JSON.parse(JSON.stringify(transaction)); // deep-copy
+      client.captureEvent(transaction);
+      expect(TestBackend.instance!.event!).toEqual(normalizedTransaction);
+    });
+
     test('calls beforeSend and uses original event without any changes', () => {
       expect.assertions(1);
       const beforeSend = jest.fn(event => event);