fix(tracing): Remove undefined tracestate data rather than setting it to null (#3373)

Author: lobsterkatie

packages/tracing/src/span.ts

@@ -378,6 +378,9 @@ export class Span implements SpanInterface {
 
     const { environment, release } = client.getOptions() || {};
 
+    // only define a `user` object if there's going to be something in it
+    const user = userId || userSegment ? { id: userId, segment: userSegment } : undefined;
+
     // TODO - the only reason we need the non-null assertion on `dsn.publicKey` (below) is because `dsn.publicKey` has
     // to be optional while we transition from `dsn.user` -> `dsn.publicKey`. Once `dsn.user` is removed, we can make
     // `dsn.publicKey` required and remove the `!`.
@@ -388,7 +391,7 @@ export class Span implements SpanInterface {
       release,
       // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
       public_key: dsn.publicKey!,
-      user: { id: userId, segment: userSegment },
+      user,
     })}`;
   }
 

packages/tracing/src/utils.ts

@@ -1,6 +1,6 @@
 import { getCurrentHub, Hub } from '@sentry/hub';
 import { Options, TraceparentData, Transaction } from '@sentry/types';
-import { SentryError, unicodeToBase64 } from '@sentry/utils';
+import { dropUndefinedKeys, SentryError, unicodeToBase64 } from '@sentry/utils';
 
 export const SENTRY_TRACE_REGEX = new RegExp(
   '^[ \\t]*' + // whitespace
@@ -146,10 +146,10 @@ export { stripUrlQueryAndFragment } from '@sentry/utils';
 
 type SentryTracestateData = {
   trace_id: string;
-  environment: string | undefined | null;
-  release: string | undefined | null;
+  environment?: string;
+  release?: string;
   public_key: string;
-  user: { id: string | undefined | null; segment: string | undefined | null };
+  user?: { id?: string; segment?: string };
 };
 
 /**
@@ -161,10 +161,6 @@ type SentryTracestateData = {
 export function computeTracestateValue(data: SentryTracestateData): string {
   // `JSON.stringify` will drop keys with undefined values, but not ones with null values, so this prevents
   // these values from being dropped if they haven't been set by `Sentry.init`
-  data.environment = data.environment || null;
-  data.release = data.release || null;
-  data.user.id = data.user.id || null;
-  data.user.segment = data.user.segment || null;
 
   // See https://www.w3.org/TR/trace-context/#tracestate-header-field-values
   // The spec for tracestate header values calls for a string of the form
@@ -175,7 +171,7 @@ export function computeTracestateValue(data: SentryTracestateData): string {
   // used to pad the end of base64 values though, so to avoid confusion, we strip them off. (Most languages' base64
   // decoding functions (including those in JS) are able to function without the padding.)
   try {
-    return unicodeToBase64(JSON.stringify(data)).replace(/={1,2}$/, '');
+    return unicodeToBase64(JSON.stringify(dropUndefinedKeys(data))).replace(/={1,2}$/, '');
   } catch (err) {
     throw new SentryError(`[Tracing] Error computing tracestate value from data: ${err}\nData: ${data}`);
   }

packages/tracing/test/httpheaders.test.ts

@@ -166,14 +166,13 @@ describe('tracestate', () => {
 
     describe('mutibility', () => {
       it("won't include data set after transaction is created if there's an inherited value", () => {
-        expect.assertions(2);
+        expect.assertions(1);
 
         const inheritedTracestate = `sentry=${computeTracestateValue({
           trace_id: '12312012090820131231201209082013',
           environment: 'dogpark',
           release: 'off.leash.trail',
           public_key: 'dogsarebadatkeepingsecrets',
-          user: { id: undefined, segment: undefined },
         })}`;
 
         const transaction = new Transaction(
@@ -194,8 +193,7 @@ describe('tracestate', () => {
           const tracestateValue = (transaction as any)._toTracestate().replace('sentry=', '');
           const reinflatedTracestate = JSON.parse(base64ToUnicode(tracestateValue));
 
-          expect(reinflatedTracestate.user.id).toBeNull();
-          expect(reinflatedTracestate.user.segment).toBeNull();
+          expect(reinflatedTracestate.user).toBeUndefined();
         });
       });
 
@@ -221,7 +219,7 @@ describe('tracestate', () => {
       });
 
       it("won't include data set after first call to `getTraceHeaders`", () => {
-        expect.assertions(2);
+        expect.assertions(1);
 
         const transaction = new Transaction(
           {
@@ -238,8 +236,7 @@ describe('tracestate', () => {
           const tracestateValue = (transaction as any)._toTracestate().replace('sentry=', '');
           const reinflatedTracestate = JSON.parse(base64ToUnicode(tracestateValue));
 
-          expect(reinflatedTracestate.user.id).toBeNull();
-          expect(reinflatedTracestate.user.segment).toBeNull();
+          expect(reinflatedTracestate.user).toBeUndefined();
         });
       });
     });