rename mutability helper functions

Lms24 · Lms24 · commit df74f4248b73 · 2022-06-07T11:38:40.000+02:00
diff --git a/packages/nextjs/src/utils/instrumentServer.ts b/packages/nextjs/src/utils/instrumentServer.ts
@@ -13,7 +13,7 @@ import {
   fill,
   isString,
   logger,
-  parseAndFreezeBaggageIfNecessary,
+  parseBaggageSetMutability,
   stripUrlQueryAndFragment,
 } from '@sentry/utils';
 import * as domain from 'domain';
@@ -256,7 +256,7 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler {
           }
 
           const rawBaggageString = nextReq.headers && isString(nextReq.headers.baggage) && nextReq.headers.baggage;
-          const baggage = parseAndFreezeBaggageIfNecessary(rawBaggageString, traceparentData);
+          const baggage = parseBaggageSetMutability(rawBaggageString, traceparentData);
 
           // pull off query string, if any
           const reqPath = stripUrlQueryAndFragment(nextReq.url);
diff --git a/packages/nextjs/src/utils/withSentry.ts b/packages/nextjs/src/utils/withSentry.ts
@@ -6,7 +6,7 @@ import {
   isString,
   logger,
   objectify,
-  parseAndFreezeBaggageIfNecessary,
+  parseBaggageSetMutability,
   stripUrlQueryAndFragment,
 } from '@sentry/utils';
 import * as domain from 'domain';
@@ -52,7 +52,7 @@ export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler =
           }
 
           const rawBaggageString = req.headers && isString(req.headers.baggage) && req.headers.baggage;
-          const baggage = parseAndFreezeBaggageIfNecessary(rawBaggageString, traceparentData);
+          const baggage = parseBaggageSetMutability(rawBaggageString, traceparentData);
 
           const url = `${req.url}`;
           // pull off query string, if any
diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts
@@ -11,7 +11,7 @@ import {
   extractTraceparentData,
   isString,
   logger,
-  parseAndFreezeBaggageIfNecessary,
+  parseBaggageSetMutability,
 } from '@sentry/utils';
 import * as domain from 'domain';
 import * as http from 'http';
@@ -38,7 +38,7 @@ export function tracingHandler(): (
       req.headers && isString(req.headers['sentry-trace']) && extractTraceparentData(req.headers['sentry-trace']);
     const rawBaggageString = req.headers && isString(req.headers.baggage) && req.headers.baggage;
 
-    const baggage = parseAndFreezeBaggageIfNecessary(rawBaggageString, traceparentData);
+    const baggage = parseBaggageSetMutability(rawBaggageString, traceparentData);
 
     const transaction = startTransaction(
       {
diff --git a/packages/node/test/handlers.test.ts b/packages/node/test/handlers.test.ts
@@ -3,7 +3,7 @@ import * as sentryHub from '@sentry/hub';
 import { Hub } from '@sentry/hub';
 import { Transaction } from '@sentry/tracing';
 import { Baggage, Event } from '@sentry/types';
-import { isBaggageEmpty, isBaggageFrozen, SentryError } from '@sentry/utils';
+import { isBaggageEmpty, isBaggageMutable, SentryError } from '@sentry/utils';
 import * as http from 'http';
 
 import { NodeClient } from '../src/client';
@@ -194,7 +194,7 @@ describe('tracingHandler', () => {
     expect(transaction.sampled).toEqual(false);
     expect(transaction.metadata?.baggage).toBeDefined();
     expect(isBaggageEmpty(transaction.metadata?.baggage)).toBeTruthy();
-    expect(isBaggageFrozen(transaction.metadata?.baggage)).toBeTruthy();
+    expect(isBaggageMutable(transaction.metadata?.baggage)).toBeFalsy();
   });
 
   it("pulls parent's data from tracing and baggage headers on the request", () => {
@@ -212,7 +212,11 @@ describe('tracingHandler', () => {
     expect(transaction.parentSpanId).toEqual('1121201211212012');
     expect(transaction.sampled).toEqual(false);
     expect(transaction.metadata?.baggage).toBeDefined();
-    expect(transaction.metadata?.baggage).toEqual([{ version: '1.0', environment: 'production' }, '', true] as Baggage);
+    expect(transaction.metadata?.baggage).toEqual([
+      { version: '1.0', environment: 'production' },
+      '',
+      false,
+    ] as Baggage);
   });
 
   it("pulls parent's baggage (sentry + third party entries) headers on the request", () => {
@@ -228,7 +232,7 @@ describe('tracingHandler', () => {
     expect(transaction.metadata?.baggage).toEqual([
       { version: '1.0', environment: 'production' },
       'dogs=great,cats=boring',
-      true,
+      false,
     ] as Baggage);
   });
 
diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts
@@ -11,7 +11,7 @@ import {
 } from '@sentry/node';
 import { extractTraceparentData } from '@sentry/tracing';
 import { Integration } from '@sentry/types';
-import { extensionRelayDSN, isString, logger, parseAndFreezeBaggageIfNecessary } from '@sentry/utils';
+import { extensionRelayDSN, isString, logger, parseBaggageSetMutability } from '@sentry/utils';
 // NOTE: I have no idea how to fix this right now, and don't want to waste more time, as it builds just fine — Kamil
 // eslint-disable-next-line import/no-unresolved
 import { Context, Handler } from 'aws-lambda';
@@ -285,7 +285,7 @@ export function wrapHandler<TEvent, TResult>(
 
     const rawBaggageString =
       eventWithHeaders.headers && isString(eventWithHeaders.headers.baggage) && eventWithHeaders.headers.baggage;
-    const baggage = parseAndFreezeBaggageIfNecessary(rawBaggageString, traceparentData);
+    const baggage = parseBaggageSetMutability(rawBaggageString, traceparentData);
 
     const transaction = startTransaction({
       name: context.functionName,
diff --git a/packages/serverless/src/gcpfunction/http.ts b/packages/serverless/src/gcpfunction/http.ts
@@ -7,7 +7,7 @@ import {
   startTransaction,
 } from '@sentry/node';
 import { extractTraceparentData } from '@sentry/tracing';
-import { isString, logger, parseAndFreezeBaggageIfNecessary, stripUrlQueryAndFragment } from '@sentry/utils';
+import { isString, logger, parseBaggageSetMutability, stripUrlQueryAndFragment } from '@sentry/utils';
 
 import { domainify, getActiveDomain, proxyFunction } from './../utils';
 import { HttpFunction, WrapperOptions } from './general';
@@ -75,7 +75,7 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial<HttpFunctionWr
     const rawBaggageString =
       reqWithHeaders.headers && isString(reqWithHeaders.headers.baggage) && reqWithHeaders.headers.baggage;
 
-    const baggage = parseAndFreezeBaggageIfNecessary(rawBaggageString, traceparentData);
+    const baggage = parseBaggageSetMutability(rawBaggageString, traceparentData);
 
     const transaction = startTransaction({
       name: `${reqMethod} ${reqUrl}`,
diff --git a/packages/serverless/test/awslambda.test.ts b/packages/serverless/test/awslambda.test.ts
@@ -190,7 +190,7 @@ describe('AWSLambda', () => {
       expect(Sentry.startTransaction).toBeCalledWith({
         name: 'functionName',
         op: 'awslambda.handler',
-        metadata: { baggage: [{}, '', false] },
+        metadata: { baggage: [{}, '', true] },
       });
       expectScopeSettings();
       // @ts-ignore see "Why @ts-ignore" note
@@ -213,7 +213,7 @@ describe('AWSLambda', () => {
         expect(Sentry.startTransaction).toBeCalledWith({
           name: 'functionName',
           op: 'awslambda.handler',
-          metadata: { baggage: [{}, '', false] },
+          metadata: { baggage: [{}, '', true] },
         });
         expectScopeSettings();
         expect(Sentry.captureException).toBeCalledWith(error);
@@ -257,7 +257,7 @@ describe('AWSLambda', () => {
                   release: '2.12.1',
                 },
                 'maisey=silly,charlie=goofy',
-                true,
+                false,
               ],
             },
           }),
@@ -289,7 +289,7 @@ describe('AWSLambda', () => {
           traceId: '12312012123120121231201212312012',
           parentSpanId: '1121201211212012',
           parentSampled: false,
-          metadata: { baggage: [{}, '', true] },
+          metadata: { baggage: [{}, '', false] },
         });
         expectScopeSettings();
         expect(Sentry.captureException).toBeCalledWith(e);
@@ -313,7 +313,7 @@ describe('AWSLambda', () => {
       expect(Sentry.startTransaction).toBeCalledWith({
         name: 'functionName',
         op: 'awslambda.handler',
-        metadata: { baggage: [{}, '', false] },
+        metadata: { baggage: [{}, '', true] },
       });
       expectScopeSettings();
       // @ts-ignore see "Why @ts-ignore" note
@@ -347,7 +347,7 @@ describe('AWSLambda', () => {
         expect(Sentry.startTransaction).toBeCalledWith({
           name: 'functionName',
           op: 'awslambda.handler',
-          metadata: { baggage: [{}, '', false] },
+          metadata: { baggage: [{}, '', true] },
         });
         expectScopeSettings();
         expect(Sentry.captureException).toBeCalledWith(error);
@@ -386,7 +386,7 @@ describe('AWSLambda', () => {
       expect(Sentry.startTransaction).toBeCalledWith({
         name: 'functionName',
         op: 'awslambda.handler',
-        metadata: { baggage: [{}, '', false] },
+        metadata: { baggage: [{}, '', true] },
       });
       expectScopeSettings();
       // @ts-ignore see "Why @ts-ignore" note
@@ -420,7 +420,7 @@ describe('AWSLambda', () => {
         expect(Sentry.startTransaction).toBeCalledWith({
           name: 'functionName',
           op: 'awslambda.handler',
-          metadata: { baggage: [{}, '', false] },
+          metadata: { baggage: [{}, '', true] },
         });
         expectScopeSettings();
         expect(Sentry.captureException).toBeCalledWith(error);
diff --git a/packages/serverless/test/gcpfunction.test.ts b/packages/serverless/test/gcpfunction.test.ts
@@ -113,7 +113,7 @@ describe('GCPFunction', () => {
       expect(Sentry.startTransaction).toBeCalledWith({
         name: 'POST /path',
         op: 'gcp.function.http',
-        metadata: { baggage: [{}, '', false] },
+        metadata: { baggage: [{}, '', true] },
       });
       // @ts-ignore see "Why @ts-ignore" note
       expect(Sentry.fakeScope.setSpan).toBeCalledWith(Sentry.fakeTransaction);
@@ -151,7 +151,7 @@ describe('GCPFunction', () => {
                 release: '2.12.1',
               },
               'maisey=silly,charlie=goofy',
-              true,
+              false,
             ],
           },
         }),
@@ -178,7 +178,7 @@ describe('GCPFunction', () => {
         traceId: '12312012123120121231201212312012',
         parentSpanId: '1121201211212012',
         parentSampled: false,
-        metadata: { baggage: [{}, '', true] },
+        metadata: { baggage: [{}, '', false] },
       });
       // @ts-ignore see "Why @ts-ignore" note
       expect(Sentry.fakeScope.setSpan).toBeCalledWith(Sentry.fakeTransaction);
diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts
@@ -1,6 +1,6 @@
 import { Hub } from '@sentry/hub';
 import { EventProcessor, Integration, Transaction, TransactionContext } from '@sentry/types';
-import { getGlobalObject, logger, parseAndFreezeBaggageIfNecessary } from '@sentry/utils';
+import { getGlobalObject, logger, parseBaggageSetMutability } from '@sentry/utils';
 
 import { startIdleTransaction } from '../hubextensions';
 import { DEFAULT_FINAL_TIMEOUT, DEFAULT_IDLE_TIMEOUT } from '../idletransaction';
@@ -255,7 +255,7 @@ export function extractTraceDataFromMetaTags(): Partial<TransactionContext> | un
   const baggageValue = getMetaContent('baggage');
 
   const sentrytraceData = sentrytraceValue ? extractTraceparentData(sentrytraceValue) : undefined;
-  const baggage = parseAndFreezeBaggageIfNecessary(baggageValue, sentrytraceValue);
+  const baggage = parseBaggageSetMutability(baggageValue, sentrytraceValue);
 
   // TODO more extensive checks for baggage validity/emptyness?
   if (sentrytraceData || baggage) {
diff --git a/packages/tracing/src/span.ts b/packages/tracing/src/span.ts
@@ -4,10 +4,10 @@ import { Baggage, Hub, Primitive, Span as SpanInterface, SpanContext, Transactio
 import {
   createBaggage,
   dropUndefinedKeys,
-  freezeBaggage,
   isBaggageEmpty,
-  isBaggageFrozen,
+  isBaggageMutable,
   isSentryBaggageEmpty,
+  setBaggageImmutable,
   setBaggageValue,
   timestampWithMs,
   uuid4,
@@ -316,9 +316,9 @@ export class Span implements SpanInterface {
     const existingBaggage = this.transaction && this.transaction.metadata.baggage;
 
     // Only add Sentry baggage items to baggage, if baggage does not exist yet or it is still
-    // empty and not yet set immutable
+    // empty and mutable
     const finalBaggage =
-      !existingBaggage || (!isBaggageFrozen(existingBaggage) && isSentryBaggageEmpty(existingBaggage))
+      !existingBaggage || (isBaggageMutable(existingBaggage) && isSentryBaggageEmpty(existingBaggage))
         ? this._getBaggageWithSentryValues(existingBaggage)
         : existingBaggage;
 
@@ -378,7 +378,7 @@ export class Span implements SpanInterface {
     environment && setBaggageValue(baggage, 'environment', environment);
     release && setBaggageValue(baggage, 'release', release);
 
-    freezeBaggage(baggage);
+    setBaggageImmutable(baggage);
 
     return baggage;
   }
diff --git a/packages/tracing/test/browser/browsertracing.test.ts b/packages/tracing/test/browser/browsertracing.test.ts
@@ -234,7 +234,7 @@ describe('BrowserTracing', () => {
           parentSpanId: 'b6e54397b12a2a0f',
           parentSampled: true,
           metadata: {
-            baggage: [{ release: '2.1.14' }, 'foo=bar', true],
+            baggage: [{ release: '2.1.14' }, 'foo=bar', false],
           },
         }),
         expect.any(Number),
diff --git a/packages/tracing/test/span.test.ts b/packages/tracing/test/span.test.ts
@@ -428,7 +428,7 @@ describe('Span', () => {
       const transaction = new Transaction(
         {
           name: 'tx',
-          metadata: { baggage: createBaggage({}, '', false) },
+          metadata: { baggage: createBaggage({}, '', true) },
         },
         hub,
       );
diff --git a/packages/types/src/baggage.ts b/packages/types/src/baggage.ts
@@ -13,7 +13,7 @@ export type BaggageObj = Partial<Record<AllowedBaggageKeys, string> & Record<str
  * set/used by sentry, they will be stored in an object to be easily accessed.
  * If they are not, they are kept as a string to be only accessed when serialized
  * at baggage propagation time.
- * The third tuple member controls the mutability of the baggage. If it is `true`,
- * the baggage can not be modified any longer (i.e. is immutable).
+ * The third tuple member controls the mutability of the baggage. If it is `false`,
+ * the baggage can no longer longer be modified (i.e. is immutable).
  */
 export type Baggage = [BaggageObj, string, boolean];
diff --git a/packages/utils/src/baggage.ts b/packages/utils/src/baggage.ts
@@ -16,8 +16,8 @@ export const SENTRY_BAGGAGE_KEY_PREFIX_REGEX = /^sentry-/;
 export const MAX_BAGGAGE_STRING_LENGTH = 8192;
 
 /** Create an instance of Baggage */
-export function createBaggage(initItems: BaggageObj, baggageString: string = '', frozen: boolean = false): Baggage {
-  return [{ ...initItems }, baggageString, frozen];
+export function createBaggage(initItems: BaggageObj, baggageString: string = '', mutable: boolean = true): Baggage {
+  return [{ ...initItems }, baggageString, mutable];
 }
 
 /** Get a value from baggage */
@@ -27,7 +27,7 @@ export function getBaggageValue(baggage: Baggage, key: keyof BaggageObj): Baggag
 
 /** Add a value to baggage */
 export function setBaggageValue(baggage: Baggage, key: keyof BaggageObj, value: BaggageObj[keyof BaggageObj]): void {
-  if (!isBaggageFrozen(baggage)) {
+  if (isBaggageMutable(baggage)) {
     baggage[0][key] = value;
   }
 }
@@ -57,20 +57,20 @@ export function getThirdPartyBaggage(baggage: Baggage): string {
 }
 
 /**
- * Checks if baggage is frozen (i.e. immutable)
+ * Checks if baggage is mutable
  * @param baggage
- * @returns true if baggage is frozen, else false
+ * @returns true if baggage is mutable, else false
  */
-export function isBaggageFrozen(baggage: Baggage): boolean {
+export function isBaggageMutable(baggage: Baggage): boolean {
   return baggage[2];
 }
 
 /**
- * Freezes baggage (i.e. makes it immutable)
+ * Sets the passed baggage immutable
  * @param baggage
  */
-export function freezeBaggage(baggage: Baggage): void {
-  baggage[2] = true;
+export function setBaggageImmutable(baggage: Baggage): void {
+  baggage[2] = false;
 }
 
 /** Serialize a baggage object */
@@ -102,13 +102,13 @@ export function parseBaggageString(inputBaggageString: string): Baggage {
             [baggageKey]: decodeURIComponent(val),
           },
           baggageString,
-          false,
+          true,
         ];
       } else {
-        return [baggageObj, baggageString === '' ? curr : `${baggageString},${curr}`, false];
+        return [baggageObj, baggageString === '' ? curr : `${baggageString},${curr}`, true];
       }
     },
-    [{}, '', false],
+    [{}, '', true],
   );
 }
 
@@ -145,21 +145,21 @@ export function mergeAndSerializeBaggage(incomingBaggage?: Baggage, headerBaggag
  * Helper function that takes a raw baggage string (if available) and the processed sentry-trace header
  * data (if available), parses the baggage string and creates a Baggage object
  * If there is no baggage string, it will create an empty Baggage object.
- * In a second step, this functions determines when the created Baggage object should be frozen to prevent mutation
- * of the Sentry data.
+ * In a second step, this functions determines when the created Baggage object should be set immutable
+ * to prevent mutation of the Sentry data.
  *
  * Extracted this logic to a function because it's duplicated in a lot of places.
  *
  * @param rawBaggageString
  * @param traceparentData
  */
-export function parseAndFreezeBaggageIfNecessary(
+export function parseBaggageSetMutability(
   rawBaggageString: string | false | undefined | null,
   traceparentData: TraceparentData | string | false | undefined | null,
 ): Baggage {
   const baggage = parseBaggageString(rawBaggageString || '');
   if (!isSentryBaggageEmpty(baggage) || (traceparentData && isSentryBaggageEmpty(baggage))) {
-    freezeBaggage(baggage);
+    setBaggageImmutable(baggage);
   }
   return baggage;
 }
diff --git a/packages/utils/test/baggage.test.ts b/packages/utils/test/baggage.test.ts
