Skip to content

Commit

Permalink
feat(tracing): Propagate environment and release values in baggage Ht…
Browse files Browse the repository at this point in the history
…tp headers (#5193)

Add the sentry-environment and sentry-release values to the baggage HTTP headers of outgoing requests. We add these values to baggage as late as possible with the rationale that other baggage values (to be added in the future) will not be available right away, e.g. when intercepting baggage from incoming requests.

As a result, the flow described below happens, when the first call to span.getBaggage is made (e.g. in callbacks of instrumented outgoing requests):

1. In span.getBaggage, check if there is baggage present in the span (in which case it was intercepted from incoming headers/meta tags) and it is empty (or only includes 3rd party data) OR if there is no baggage yet
2. In both of these cases, populate the baggage with Sentry data (and leave 3rd party content untouched). Else do nothing
3. Add this baggage to outgoing headers (and merge with possible 3rd party header)

Additionally, add and improve tests to check correct handling and propagation of baggage
  • Loading branch information
Lms24 committed Jun 7, 2022
1 parent 5791c49 commit 1456b9c
Show file tree
Hide file tree
Showing 11 changed files with 163 additions and 32 deletions.
Expand Up @@ -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`), {
Expand All @@ -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'),
},
});
});
Expand Up @@ -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'),
},
});
});
Expand Up @@ -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,
});
Expand Down
12 changes: 6 additions & 6 deletions packages/node/test/integrations/http.test.ts
Expand Up @@ -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();
Expand All @@ -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/');

Expand All @@ -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/');

Expand Down Expand Up @@ -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 () => {
Expand All @@ -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", () => {
Expand Down
2 changes: 1 addition & 1 deletion 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,
Expand All @@ -7,7 +8,6 @@ import {
mergeAndSerializeBaggage,
} from '@sentry/utils';

import { Span } from '../span';
import { getActiveTransaction, hasTracingEnabled } from '../utils';

export const DEFAULT_TRACING_ORIGINS = ['localhost', /^\//];
Expand Down
40 changes: 37 additions & 3 deletions 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
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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 =
Expand Down
6 changes: 3 additions & 3 deletions packages/tracing/src/transaction.ts
Expand Up @@ -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;

Expand Down
44 changes: 40 additions & 4 deletions 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';

Expand Down Expand Up @@ -415,7 +415,16 @@ describe('BrowserTracing', () => {
});
});

describe('using the data', () => {
describe('using the <meta> tag data', () => {
beforeEach(() => {
hub.getClient()!.getOptions = () => {
return {
release: '1.0.0',
environment: 'production',
} as ClientOptions<BaseTransportOptions>;
};
});

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 =
Expand All @@ -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 =
'<meta name="sentry-trace" content="12312012123120121231201212312012-1121201211212012-0">' +
'<meta name="baggage" content="foo=bar">';

// 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 =
'<meta name="sentry-trace" content="12312012123120121231201212312012-1121201211212012-0">' +
'<meta name="baggage" content="sentry-release=2.1.14,foo=bar">';
'<meta name="baggage" content="sentry-release=2.1.14">';

createBrowserTracing(true);

Expand All @@ -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('');
});
});
});
Expand Down
55 changes: 55 additions & 0 deletions 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';
Expand Down Expand Up @@ -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<BaseTransportOptions>;
};
});

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('');
});
});
});
10 changes: 8 additions & 2 deletions packages/utils/src/baggage.ts
Expand Up @@ -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];
Expand Down
6 changes: 3 additions & 3 deletions packages/utils/test/baggage.test.ts
@@ -1,7 +1,7 @@
import {
createBaggage,
getBaggageValue,
isBaggageEmpty,
isSentryBaggageEmpty,
mergeAndSerializeBaggage,
parseBaggageString,
serializeBaggage,
Expand Down Expand Up @@ -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);
});
});

Expand Down

0 comments on commit 1456b9c

Please sign in to comment.