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 index 81dddb7c1f97..f2d318877a65 100644 --- 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 @@ -3,7 +3,7 @@ import * as path from 'path'; import { getAPIResponse, runServer } from '../../../../utils/index'; import { TestAPIResponse } from '../server'; -test('Should assign `baggage` header which contains 3rd party trace baggage data of an outgoing request.', async () => { +test('Should assign `baggage` header which contains 3rd party trace baggage data to an outgoing request.', async () => { const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`); const response = (await getAPIResponse(new URL(`${url}/express`), { @@ -14,39 +14,39 @@ test('Should assign `baggage` header which contains 3rd party trace baggage data expect(response).toMatchObject({ test_data: { host: 'somewhere.not.sentry', - baggage: expect.stringContaining('foo=bar,bar=baz'), + baggage: 'foo=bar,bar=baz,sentry-environment=prod,sentry-release=1.0', }, }); }); -test('Should assign `baggage` header which contains sentry trace baggage data of an outgoing request.', async () => { +test('Should not overwrite baggage if the incoming request already has Sentry baggage data.', async () => { const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`); const response = (await getAPIResponse(new URL(`${url}/express`), { - baggage: 'sentry-version=1.0.0,sentry-environment=production', + baggage: 'sentry-version=2.0.0,sentry-environment=myEnv', })) as TestAPIResponse; expect(response).toBeDefined(); expect(response).toMatchObject({ test_data: { host: 'somewhere.not.sentry', - baggage: expect.stringContaining('sentry-version=1.0.0,sentry-environment=production'), + baggage: 'sentry-version=2.0.0,sentry-environment=myEnv', }, }); }); -test('Should assign `baggage` header which contains sentry and 3rd party trace baggage data of an outgoing request.', async () => { +test('Should pass along sentry and 3rd party trace baggage data from an incoming to an outgoing request.', async () => { const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`); const response = (await getAPIResponse(new URL(`${url}/express`), { - baggage: 'sentry-version=1.0.0,sentry-environment=production,dogs=great', + baggage: 'sentry-version=2.0.0,sentry-environment=myEnv,dogs=great', })) as TestAPIResponse; expect(response).toBeDefined(); expect(response).toMatchObject({ test_data: { host: 'somewhere.not.sentry', - baggage: expect.stringContaining('dogs=great,sentry-version=1.0.0,sentry-environment=production'), + baggage: expect.stringContaining('dogs=great,sentry-version=2.0.0,sentry-environment=myEnv'), }, }); }); diff --git a/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts b/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts index 70b6cc19edee..822e85105d0e 100644 --- a/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts +++ b/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts @@ -12,8 +12,7 @@ test('should attach a `baggage` header to an outgoing request.', async () => { expect(response).toMatchObject({ test_data: { host: 'somewhere.not.sentry', - // TODO this is currently still empty but eventually it should contain sentry data - baggage: expect.stringMatching(''), + baggage: expect.stringMatching('sentry-environment=prod,sentry-release=1.0'), }, }); }); diff --git a/packages/node-integration-tests/suites/express/sentry-trace/server.ts b/packages/node-integration-tests/suites/express/sentry-trace/server.ts index 632d9b8338fb..25238008efcf 100644 --- a/packages/node-integration-tests/suites/express/sentry-trace/server.ts +++ b/packages/node-integration-tests/suites/express/sentry-trace/server.ts @@ -11,6 +11,7 @@ export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': strin Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', + environment: 'prod', integrations: [new Sentry.Integrations.Http({ tracing: true }), new Tracing.Integrations.Express({ app })], tracesSampleRate: 1.0, }); diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index 50d26e92452a..254e371aaa35 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -21,6 +21,8 @@ describe('tracing', () => { dsn: 'https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012', tracesSampleRate: 1.0, integrations: [new HttpIntegration({ tracing: true })], + release: '1.0.0', + environment: 'production', }); const hub = new Hub(new NodeClient(options)); addExtensionMethods(); @@ -40,7 +42,7 @@ describe('tracing', () => { nock('http://dogs.are.great').get('/').reply(200); const transaction = createTransactionOnScope(); - const spans = (transaction as Span).spanRecorder?.spans as Span[]; + const spans = (transaction as unknown as Span).spanRecorder?.spans as Span[]; http.get('http://dogs.are.great/'); @@ -55,7 +57,7 @@ describe('tracing', () => { nock('http://squirrelchasers.ingest.sentry.io').get('/api/12312012/store/').reply(200); const transaction = createTransactionOnScope(); - const spans = (transaction as Span).spanRecorder?.spans as Span[]; + const spans = (transaction as unknown as Span).spanRecorder?.spans as Span[]; http.get('http://squirrelchasers.ingest.sentry.io/api/12312012/store/'); @@ -96,8 +98,7 @@ describe('tracing', () => { const baggageHeader = request.getHeader('baggage') as string; expect(baggageHeader).toBeDefined(); - // this might change once we actually add our baggage data to the header - expect(baggageHeader).toEqual(''); + expect(baggageHeader).toEqual('sentry-environment=production,sentry-release=1.0.0'); }); it('propagates 3rd party baggage header data to outgoing non-sentry requests', async () => { @@ -109,8 +110,7 @@ describe('tracing', () => { const baggageHeader = request.getHeader('baggage') as string; expect(baggageHeader).toBeDefined(); - // this might change once we actually add our baggage data to the header - expect(baggageHeader).toEqual('dog=great'); + expect(baggageHeader).toEqual('dog=great,sentry-environment=production,sentry-release=1.0.0'); }); it("doesn't attach the sentry-trace header to outgoing sentry requests", () => { diff --git a/packages/tracing/src/browser/request.ts b/packages/tracing/src/browser/request.ts index b45775edd05f..0d72c96a2a30 100644 --- a/packages/tracing/src/browser/request.ts +++ b/packages/tracing/src/browser/request.ts @@ -1,4 +1,5 @@ /* eslint-disable max-lines */ +import type { Span } from '@sentry/types'; import { addInstrumentationHandler, BAGGAGE_HEADER_NAME, @@ -7,7 +8,6 @@ import { mergeAndSerializeBaggage, } from '@sentry/utils'; -import { Span } from '../span'; import { getActiveTransaction, hasTracingEnabled } from '../utils'; export const DEFAULT_TRACING_ORIGINS = ['localhost', /^\//]; diff --git a/packages/tracing/src/span.ts b/packages/tracing/src/span.ts index 68cba79eb7cc..f6755972b576 100644 --- a/packages/tracing/src/span.ts +++ b/packages/tracing/src/span.ts @@ -1,6 +1,15 @@ /* eslint-disable max-lines */ -import { Baggage, Primitive, Span as SpanInterface, SpanContext, Transaction } from '@sentry/types'; -import { dropUndefinedKeys, timestampWithMs, uuid4 } from '@sentry/utils'; +import { getCurrentHub } from '@sentry/hub'; +import { Baggage, Hub, Primitive, Span as SpanInterface, SpanContext, Transaction } from '@sentry/types'; +import { + createBaggage, + dropUndefinedKeys, + isBaggageEmpty, + isSentryBaggageEmpty, + setBaggageValue, + timestampWithMs, + uuid4, +} from '@sentry/utils'; /** * Keeps track of finished spans for a given transaction @@ -302,7 +311,14 @@ export class Span implements SpanInterface { * @inheritdoc */ public getBaggage(): Baggage | undefined { - return this.transaction && this.transaction.metadata.baggage; + const existingBaggage = this.transaction && this.transaction.metadata.baggage; + + const finalBaggage = + !existingBaggage || isSentryBaggageEmpty(existingBaggage) + ? this._getBaggageWithSentryValues(existingBaggage) + : existingBaggage; + + return isBaggageEmpty(finalBaggage) ? undefined : finalBaggage; } /** @@ -334,6 +350,24 @@ export class Span implements SpanInterface { trace_id: this.traceId, }); } + + /** + * + * @param baggage + * @returns + */ + 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(); + + const { environment, release } = (client && client.getOptions()) || {}; + + environment && setBaggageValue(baggage, 'environment', environment); + release && setBaggageValue(baggage, 'release', release); + + return baggage; + } } export type SpanStatusType = diff --git a/packages/tracing/src/transaction.ts b/packages/tracing/src/transaction.ts index e9f388516555..d51f4578717a 100644 --- a/packages/tracing/src/transaction.ts +++ b/packages/tracing/src/transaction.ts @@ -16,12 +16,12 @@ export class Transaction extends SpanClass implements TransactionInterface { public metadata: TransactionMetadata; - private _measurements: Measurements = {}; - /** * The reference to the current hub. */ - private readonly _hub: Hub; + protected readonly _hub: Hub; + + private _measurements: Measurements = {}; private _trimEnd?: boolean; diff --git a/packages/tracing/test/browser/browsertracing.test.ts b/packages/tracing/test/browser/browsertracing.test.ts index 6fbcc5b6f6d8..90587b88e255 100644 --- 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 { BaggageObj } from '@sentry/types'; +import type { BaggageObj, BaseTransportOptions, ClientOptions } from '@sentry/types'; import { getGlobalObject, InstrumentHandlerCallback, InstrumentHandlerType } from '@sentry/utils'; import { JSDOM } from 'jsdom'; @@ -415,7 +415,16 @@ describe('BrowserTracing', () => { }); }); - describe('using the data', () => { + describe('using the tag data', () => { + beforeEach(() => { + hub.getClient()!.getOptions = () => { + return { + release: '1.0.0', + environment: 'production', + } as ClientOptions; + }; + }); + it('uses the tracing data for pageload transactions', () => { // make sampled false here, so we can see that it's being used rather than the tracesSampleRate-dictated one document.head.innerHTML = @@ -439,11 +448,34 @@ describe('BrowserTracing', () => { expect(baggage[1]).toEqual('foo=bar'); }); + it('adds Sentry baggage data to pageload transactions if not present in meta tags', () => { + // make sampled false here, so we can see that it's being used rather than the tracesSampleRate-dictated one + document.head.innerHTML = + '' + + ''; + + // pageload transactions are created as part of the BrowserTracing integration's initialization + createBrowserTracing(true); + const transaction = getActiveTransaction(hub) as IdleTransaction; + const baggage = transaction.getBaggage()!; + + expect(transaction).toBeDefined(); + expect(transaction.op).toBe('pageload'); + expect(transaction.traceId).toEqual('12312012123120121231201212312012'); + expect(transaction.parentSpanId).toEqual('1121201211212012'); + expect(transaction.sampled).toBe(false); + expect(baggage).toBeDefined(); + expect(baggage[0]).toBeDefined(); + expect(baggage[0]).toEqual({ environment: 'production', release: '1.0.0' }); + expect(baggage[1]).toBeDefined(); + expect(baggage[1]).toEqual('foo=bar'); + }); + it('ignores the data for navigation transactions', () => { mockChangeHistory = () => undefined; document.head.innerHTML = '' + - ''; + ''; createBrowserTracing(true); @@ -455,7 +487,11 @@ describe('BrowserTracing', () => { expect(transaction.op).toBe('navigation'); expect(transaction.traceId).not.toEqual('12312012123120121231201212312012'); expect(transaction.parentSpanId).toBeUndefined(); - expect(baggage).toBeUndefined(); + expect(baggage).toBeDefined(); + expect(baggage[0]).toBeDefined(); + expect(baggage[0]).toEqual({ release: '1.0.0', environment: 'production' }); + expect(baggage[1]).toBeDefined(); + expect(baggage[1]).toEqual(''); }); }); }); diff --git a/packages/tracing/test/span.test.ts b/packages/tracing/test/span.test.ts index 27df400ffc9e..0c35b6c04765 100644 --- a/packages/tracing/test/span.test.ts +++ b/packages/tracing/test/span.test.ts @@ -1,5 +1,7 @@ import { BrowserClient } from '@sentry/browser'; import { Hub, makeMain, Scope } from '@sentry/hub'; +import { BaseTransportOptions, ClientOptions } from '@sentry/types'; +import { createBaggage, getSentryBaggageItems, getThirdPartyBaggage, isSentryBaggageEmpty } from '@sentry/utils'; import { Span, Transaction } from '../src'; import { TRACEPARENT_REGEXP } from '../src/utils'; @@ -390,4 +392,57 @@ describe('Span', () => { expect(span.data).toStrictEqual({ data0: 'foo', data1: 'bar' }); }); }); + + describe('getBaggage and _getBaggageWithSentryValues', () => { + beforeEach(() => { + hub.getClient()!.getOptions = () => { + return { + release: '1.0.1', + environment: 'production', + } as ClientOptions; + }; + }); + + test('leave baggage content untouched and just return baggage if there already is Sentry content in it', () => { + const transaction = new Transaction( + { + name: 'tx', + metadata: { baggage: createBaggage({ environment: 'myEnv' }, '') }, + }, + hub, + ); + + const hubSpy = jest.spyOn(hub.getClient()!, 'getOptions'); + + const span = transaction.startChild(); + + const baggage = span.getBaggage(); + + expect(hubSpy).toHaveBeenCalledTimes(0); + expect(baggage && isSentryBaggageEmpty(baggage)).toBeFalsy(); + expect(baggage && getSentryBaggageItems(baggage)).toStrictEqual({ environment: 'myEnv' }); + expect(baggage && getThirdPartyBaggage(baggage)).toStrictEqual(''); + }); + + test('add Sentry baggage data to baggage if Sentry content is empty', () => { + const transaction = new Transaction( + { + name: 'tx', + metadata: { baggage: createBaggage({}, '') }, + }, + hub, + ); + + const hubSpy = jest.spyOn(hub.getClient()!, 'getOptions'); + + const span = transaction.startChild(); + + const baggage = span.getBaggage(); + + expect(hubSpy).toHaveBeenCalledTimes(1); + expect(baggage && isSentryBaggageEmpty(baggage)).toBeFalsy(); + expect(baggage && getSentryBaggageItems(baggage)).toStrictEqual({ release: '1.0.1', environment: 'production' }); + expect(baggage && getThirdPartyBaggage(baggage)).toStrictEqual(''); + }); + }); }); diff --git a/packages/utils/src/baggage.ts b/packages/utils/src/baggage.ts index fc8cc10ba69a..9e4faa2a025e 100644 --- a/packages/utils/src/baggage.ts +++ b/packages/utils/src/baggage.ts @@ -30,11 +30,17 @@ export function setBaggageValue(baggage: Baggage, key: keyof BaggageObj, value: baggage[0][key] = value; } -/** Check if the baggage object (i.e. the first element in the tuple) is empty */ -export function isBaggageEmpty(baggage: Baggage): boolean { +/** Check if the Sentry part of the passed baggage (i.e. the first element in the tuple) is empty */ +export function isSentryBaggageEmpty(baggage: Baggage): boolean { return Object.keys(baggage[0]).length === 0; } +/** Check if the Sentry part of the passed baggage (i.e. the first element in the tuple) is empty */ +export function isBaggageEmpty(baggage: Baggage): boolean { + const thirdPartyBaggage = getThirdPartyBaggage(baggage); + return isSentryBaggageEmpty(baggage) && (thirdPartyBaggage == undefined || thirdPartyBaggage.length === 0); +} + /** Returns Sentry specific baggage values */ export function getSentryBaggageItems(baggage: Baggage): BaggageObj { return baggage[0]; diff --git a/packages/utils/test/baggage.test.ts b/packages/utils/test/baggage.test.ts index 07ccdaaedd48..380b6cd076ce 100644 --- a/packages/utils/test/baggage.test.ts +++ b/packages/utils/test/baggage.test.ts @@ -1,7 +1,7 @@ import { createBaggage, getBaggageValue, - isBaggageEmpty, + isSentryBaggageEmpty, mergeAndSerializeBaggage, parseBaggageString, serializeBaggage, @@ -98,12 +98,12 @@ describe('Baggage', () => { }); }); - describe('isBaggageEmpty', () => { + describe('isSentryBaggageEmpty', () => { it.each([ ['returns true if the modifyable part of baggage is empty', createBaggage({}), true], ['returns false if the modifyable part of baggage is not empty', createBaggage({ release: '10.0.2' }), false], ])('%s', (_: string, baggage, outcome) => { - expect(isBaggageEmpty(baggage)).toEqual(outcome); + expect(isSentryBaggageEmpty(baggage)).toEqual(outcome); }); });