add other missing DSC values + adjust tests

Lms24 · Lms24 · commit cd7862192b3d · 2022-06-22T14:47:34.000+02:00
diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts
@@ -101,7 +101,7 @@ function createEventEnvelopeHeaders(
   dsn: DsnComponents,
 ): EventEnvelopeHeaders {
   const baggage = event.sdkProcessingMetadata && event.sdkProcessingMetadata.baggage;
-  const { environment, release, transaction, userid, usersegment, samplerate } =
+  const { environment, release, transaction, userid, usersegment, samplerate, publickey, traceid } =
     (baggage && getSentryBaggageItems(baggage)) || {};
 
   return {
@@ -110,15 +110,10 @@ function createEventEnvelopeHeaders(
     ...(sdkInfo && { sdk: sdkInfo }),
     ...(!!tunnel && { dsn: dsnToString(dsn) }),
     ...(event.type === 'transaction' &&
-      // If we don't already have a trace context in the event, we can't get the trace id, which makes adding any other
-      // trace data pointless
-      event.contexts &&
-      event.contexts.trace && {
+      baggage && {
         trace: dropUndefinedKeys({
-          // Trace context must be defined for transactions
-          // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-          trace_id: (event.contexts!.trace as Record<string, unknown>).trace_id as string,
-          public_key: dsn.publicKey,
+          trace_id: traceid,
+          public_key: publickey,
           sample_rate: samplerate,
           environment,
           release,
diff --git a/packages/core/test/lib/envelope.test.ts b/packages/core/test/lib/envelope.test.ts
@@ -14,48 +14,23 @@ describe('createEventEnvelope', () => {
       expect(envelopeHeaders.trace).toBeUndefined();
     });
 
-    it('adds minimal trace data if event is a transaction and no other baggage-related data is available', () => {
-      const event: Event = {
-        type: 'transaction',
-        contexts: {
-          trace: {
-            trace_id: '1234',
-          },
-        },
-      };
-      const envelopeHeaders = createEventEnvelope(event, testDsn)[0];
-
-      expect(envelopeHeaders).toBeDefined();
-      expect(envelopeHeaders.trace).toEqual({ trace_id: '1234', public_key: 'pubKey123' });
-    });
-
     const testTable: Array<[string, Event, EventTraceContext]> = [
       [
-        'adds one baggage item',
+        'adds minimal baggage items',
         {
           type: 'transaction',
-          contexts: {
-            trace: {
-              trace_id: '1234',
-            },
-          },
           sdkProcessingMetadata: {
-            baggage: [{ release: '1.0.0' }, '', false],
+            baggage: [{ traceid: '1234', publickey: 'pubKey123' }, '', false],
           },
         },
-        { release: '1.0.0', trace_id: '1234', public_key: 'pubKey123' },
+        { trace_id: '1234', public_key: 'pubKey123' },
       ],
       [
-        'adds two baggage items',
+        'adds multiple baggage items',
         {
           type: 'transaction',
-          contexts: {
-            trace: {
-              trace_id: '1234',
-            },
-          },
           sdkProcessingMetadata: {
-            baggage: [{ environment: 'prod', release: '1.0.0' }, '', false],
+            baggage: [{ environment: 'prod', release: '1.0.0', publickey: 'pubKey123', traceid: '1234' }, '', false],
           },
         },
         { release: '1.0.0', environment: 'prod', trace_id: '1234', public_key: 'pubKey123' },
@@ -64,11 +39,6 @@ describe('createEventEnvelope', () => {
         'adds all baggage items',
         {
           type: 'transaction',
-          contexts: {
-            trace: {
-              trace_id: '1234',
-            },
-          },
           sdkProcessingMetadata: {
             baggage: [
               {
@@ -78,6 +48,8 @@ describe('createEventEnvelope', () => {
                 usersegment: 'segmentA',
                 transaction: 'TX',
                 samplerate: '0.95',
+                publickey: 'pubKey123',
+                traceid: '1234',
               },
               '',
               false,
diff --git a/packages/integration-tests/suites/tracing/browsertracing/meta/template.html b/packages/integration-tests/suites/tracing/browsertracing/meta/template.html
@@ -2,6 +2,9 @@
   <head>
     <meta charset="utf-8" />
     <meta name="sentry-trace" content="12312012123120121231201212312012-1121201211212012-1" />
-    <meta name="baggage" content="sentry-release=2.1.12" />
+    <meta
+      name="baggage"
+      content="sentry-release=2.1.12,sentry-publickey=public,sentry-traceid=123,sentry-samplerate=0.3232"
+    />
   </head>
 </html>
diff --git a/packages/integration-tests/suites/tracing/browsertracing/meta/test.ts b/packages/integration-tests/suites/tracing/browsertracing/meta/test.ts
@@ -30,9 +30,10 @@ sentryTest(
 
     expect(envHeader.trace).toBeDefined();
     expect(envHeader.trace).toEqual({
-      public_key: 'public',
-      trace_id: expect.any(String),
       release: '2.1.12',
+      sample_rate: '0.3232',
+      trace_id: '123',
+      public_key: 'public',
     });
   },
 );
diff --git a/packages/integration-tests/suites/tracing/envelope-header/init.js b/packages/integration-tests/suites/tracing/envelope-header/init.js
@@ -13,5 +13,5 @@ Sentry.init({
 
 Sentry.configureScope(scope => {
   scope.setUser({ id: 'user123', segment: 'segmentB' });
-  scope.setTransactionName('testTransactionBaggage');
+  scope.setTransactionName('testTransactionDSC');
 });
diff --git a/packages/integration-tests/suites/tracing/envelope-header/test.ts b/packages/integration-tests/suites/tracing/envelope-header/test.ts
@@ -14,15 +14,14 @@ sentryTest(
     expect(envHeader.trace).toBeDefined();
     expect(envHeader.trace).toEqual({
       environment: 'production',
-      // TODO comment back in once we properly support transaction and user data in Baggage
-      // transaction: 'testTransactionBaggage',
-      // user: {
-      //   id: 'user123',
-      //   segment: 'segmentB',
-      // },
+      transaction: expect.stringContaining('index.html'),
+      user: {
+        id: 'user123',
+        segment: 'segmentB',
+      },
       sample_rate: '1',
-      public_key: 'public',
       trace_id: expect.any(String),
+      public_key: 'public',
     });
   },
 );
diff --git a/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts b/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts
@@ -77,7 +77,10 @@ test('Should populate and propagate sentry baggage if sentry-trace header does n
   expect(response).toMatchObject({
     test_data: {
       host: 'somewhere.not.sentry',
-      baggage: 'sentry-environment=prod,sentry-release=1.0,sentry-samplerate=1',
+      baggage: expect.stringContaining(
+        'sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,sentry-samplerate=1,' +
+          'sentry-publickey=public,sentry-traceid=',
+      ),
     },
   });
 });
@@ -93,7 +96,11 @@ test('Should populate Sentry and propagate 3rd party content if sentry-trace hea
   expect(response).toMatchObject({
     test_data: {
       host: 'somewhere.not.sentry',
-      baggage: 'foo=bar,bar=baz,sentry-environment=prod,sentry-release=1.0,sentry-samplerate=1',
+      // TraceId changes, hence we only expect that the string contains the traceid key
+      baggage: expect.stringContaining(
+        'foo=bar,bar=baz,sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,' +
+          'sentry-samplerate=1,sentry-publickey=public,sentry-traceid=',
+      ),
     },
   });
 });
diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts
@@ -27,12 +27,22 @@ describe('tracing', () => {
     const hub = new Hub(new NodeClient(options));
     addExtensionMethods();
 
+    hub.configureScope(scope =>
+      scope.setUser({
+        id: 'uid123',
+        segment: 'segmentA',
+      }),
+    );
+
     // we need to mock both of these because the tracing handler relies on `@sentry/core` while the sampler relies on
     // `@sentry/hub`, and mocking breaks the link between the two
     jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);
     jest.spyOn(hubModule, 'getCurrentHub').mockReturnValue(hub);
 
-    const transaction = hub.startTransaction({ name: 'dogpark' });
+    const transaction = hub.startTransaction({
+      name: 'dogpark',
+      traceId: '12312012123120121231201212312012',
+    });
     hub.getScope()?.setSpan(transaction);
 
     return transaction;
@@ -98,7 +108,11 @@ describe('tracing', () => {
     const baggageHeader = request.getHeader('baggage') as string;
 
     expect(baggageHeader).toBeDefined();
-    expect(baggageHeader).toEqual('sentry-environment=production,sentry-release=1.0.0,sentry-samplerate=1');
+    expect(baggageHeader).toEqual(
+      'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,sentry-userid=uid123,' +
+        'sentry-usersegment=segmentA,sentry-samplerate=1,sentry-publickey=dogsarebadatkeepingsecrets,' +
+        'sentry-traceid=12312012123120121231201212312012',
+    );
   });
 
   it('propagates 3rd party baggage header data to outgoing non-sentry requests', async () => {
@@ -110,7 +124,11 @@ describe('tracing', () => {
     const baggageHeader = request.getHeader('baggage') as string;
 
     expect(baggageHeader).toBeDefined();
-    expect(baggageHeader).toEqual('dog=great,sentry-environment=production,sentry-release=1.0.0,sentry-samplerate=1');
+    expect(baggageHeader).toEqual(
+      'dog=great,sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,' +
+        'sentry-userid=uid123,sentry-usersegment=segmentA,sentry-samplerate=1,' +
+        'sentry-publickey=dogsarebadatkeepingsecrets,sentry-traceid=12312012123120121231201212312012',
+    );
   });
 
   it("doesn't attach the sentry-trace header to outgoing sentry requests", () => {
diff --git a/packages/tracing/src/span.ts b/packages/tracing/src/span.ts
@@ -1,6 +1,6 @@
 /* eslint-disable max-lines */
 import { getCurrentHub } from '@sentry/hub';
-import { Baggage, Hub, Primitive, Span as SpanInterface, SpanContext, Transaction } from '@sentry/types';
+import { Baggage, Hub, Primitive, Span as SpanInterface, SpanContext } from '@sentry/types';
 import {
   createBaggage,
   dropUndefinedKeys,
@@ -12,6 +12,8 @@ import {
   uuid4,
 } from '@sentry/utils';
 
+import { Transaction } from './transaction';
+
 /**
  * Keeps track of finished spans for a given transaction
  * @internal
@@ -320,7 +322,7 @@ export class Span implements SpanInterface {
     //       custom sentry-values in DSC (added by users in the future)
     const finalBaggage =
       !existingBaggage || (isBaggageMutable(existingBaggage) && isSentryBaggageEmpty(existingBaggage))
-        ? this._getBaggageWithSentryValues(existingBaggage)
+        ? this._populateBaggageWithSentryValues(existingBaggage)
         : existingBaggage;
 
     return finalBaggage;
@@ -369,19 +371,35 @@ export class Span implements SpanInterface {
    *
    * @returns modified and immutable baggage object
    */
-  private _getBaggageWithSentryValues(baggage: Baggage = createBaggage({})): Baggage {
-    // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any
-    const hub: Hub = ((this.transaction as any) && (this.transaction as any)._hub) || getCurrentHub();
-    const client = hub.getClient();
+  private _populateBaggageWithSentryValues(baggage: Baggage = createBaggage({})): Baggage {
+    const hub: Hub = (this.transaction && this.transaction._hub) || getCurrentHub();
+    const client = hub && hub.getClient();
 
     const { environment, release } = (client && client.getOptions()) || {};
+    const { publicKey } = (client && client.getDsn()) || {};
 
     const metadata = this.transaction && this.transaction.metadata;
     const sampelRate = metadata && metadata.transactionSampling && metadata.transactionSampling.rate;
 
+    const traceId = this.traceId;
+
+    const transactionName = this.transaction && this.transaction.name;
+
+    let userId, userSegment;
+    hub.withScope(scope => {
+      const user = scope.getUser();
+      userId = user && user.id;
+      userSegment = user && user.segment;
+    });
+
     environment && setBaggageValue(baggage, 'environment', environment);
     release && setBaggageValue(baggage, 'release', release);
+    transactionName && setBaggageValue(baggage, 'transaction', transactionName);
+    userId && setBaggageValue(baggage, 'userid', userId);
+    userSegment && setBaggageValue(baggage, 'usersegment', userSegment);
     sampelRate && setBaggageValue(baggage, 'samplerate', sampelRate.toString());
+    publicKey && setBaggageValue(baggage, 'publickey', publicKey);
+    traceId && setBaggageValue(baggage, 'traceid', traceId);
 
     setBaggageImmutable(baggage);
 
diff --git a/packages/tracing/src/transaction.ts b/packages/tracing/src/transaction.ts
@@ -19,7 +19,7 @@ export class Transaction extends SpanClass implements TransactionInterface {
   /**
    * The reference to the current hub.
    */
-  protected readonly _hub: Hub;
+  public readonly _hub: Hub;
 
   private _measurements: Measurements = {};
 
diff --git a/packages/tracing/test/browser/browsertracing.test.ts b/packages/tracing/test/browser/browsertracing.test.ts
@@ -1,6 +1,6 @@
 import { BrowserClient } from '@sentry/browser';
 import { Hub, makeMain } from '@sentry/hub';
-import type { Baggage, BaggageObj, BaseTransportOptions, ClientOptions } from '@sentry/types';
+import type { Baggage, BaggageObj, BaseTransportOptions, ClientOptions, DsnComponents } from '@sentry/types';
 import {
   getGlobalObject,
   InstrumentHandlerCallback,
@@ -433,6 +433,12 @@ describe('BrowserTracing', () => {
             environment: 'production',
           } as ClientOptions<BaseTransportOptions>;
         };
+
+        hub.getClient()!.getDsn = () => {
+          return {
+            publicKey: 'pubKey',
+          } as DsnComponents;
+        };
       });
 
       it('uses the tracing data for pageload transactions', () => {
@@ -498,7 +504,13 @@ describe('BrowserTracing', () => {
         expect(transaction.parentSpanId).toBeUndefined();
         expect(baggage).toBeDefined();
         expect(baggage[0]).toBeDefined();
-        expect(baggage[0]).toEqual({ release: '1.0.0', environment: 'production' });
+        expect(baggage[0]).toEqual({
+          release: '1.0.0',
+          environment: 'production',
+          publickey: 'pubKey',
+          traceid: expect.any(String),
+          transaction: 'blank',
+        });
         expect(baggage[1]).toBeDefined();
         expect(baggage[1]).toEqual('');
       });
diff --git a/packages/tracing/test/span.test.ts b/packages/tracing/test/span.test.ts
@@ -393,7 +393,7 @@ describe('Span', () => {
     });
   });
 
-  describe('getBaggage and _getBaggageWithSentryValues', () => {
+  describe('getBaggage and _populateBaggageWithSentryValues', () => {
     beforeEach(() => {
       hub.getClient()!.getOptions = () => {
         return {
@@ -428,7 +428,10 @@ describe('Span', () => {
       const transaction = new Transaction(
         {
           name: 'tx',
-          metadata: { baggage: createBaggage({}, '', true) },
+          metadata: {
+            baggage: createBaggage({}, '', true),
+            transactionSampling: { rate: 0.56, method: 'client_rate' },
+          },
         },
         hub,
       );
@@ -441,7 +444,13 @@ describe('Span', () => {
 
       expect(hubSpy).toHaveBeenCalledTimes(1);
       expect(baggage && isSentryBaggageEmpty(baggage)).toBe(false);
-      expect(baggage && getSentryBaggageItems(baggage)).toStrictEqual({ release: '1.0.1', environment: 'production' });
+      expect(baggage && getSentryBaggageItems(baggage)).toStrictEqual({
+        release: '1.0.1',
+        environment: 'production',
+        transaction: 'tx',
+        samplerate: '0.56',
+        traceid: expect.any(String),
+      });
       expect(baggage && getThirdPartyBaggage(baggage)).toStrictEqual('');
     });
   });
diff --git a/packages/types/src/baggage.ts b/packages/types/src/baggage.ts
@@ -1,4 +1,13 @@
-export type AllowedBaggageKeys = 'environment' | 'release' | 'userid' | 'transaction' | 'usersegment' | 'samplerate';
+export type AllowedBaggageKeys =
+  | 'environment'
+  | 'release'
+  | 'transaction'
+  | 'userid'
+  | 'usersegment'
+  | 'samplerate'
+  | 'traceid'
+  | 'publickey';
+
 export type BaggageObj = Partial<Record<AllowedBaggageKeys, string> & Record<string, string>>;
 
 /**
