Skip to content

Commit

Permalink
ref(tracing): Unify DSC key names in envelope and baggage headers (#5302
Browse files Browse the repository at this point in the history
)

Unify the key names in the envelope `trace` header and  the `baggage` Http header. As a result, we get rid of the mapping that was necessary before, thereby loosing a little JS bundle weight ;) 
Furthermore, the PR renames the `EventTraceContext` interface and aligns the `Baggage` type so that we can ensure that both DSC transport mechanisms are based on the same keys.
  • Loading branch information
Lms24 authored Jun 24, 2022
1 parent b52900f commit b9f2122
Show file tree
Hide file tree
Showing 12 changed files with 80 additions and 105 deletions.
25 changes: 6 additions & 19 deletions packages/core/src/envelope.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import {
Baggage,
DsnComponents,
DynamicSamplingContext,
Event,
EventEnvelope,
EventEnvelopeHeaders,
EventItem,
EventTraceContext,
SdkInfo,
SdkMetadata,
Session,
Expand Down Expand Up @@ -100,31 +101,17 @@ function createEventEnvelopeHeaders(
tunnel: string | undefined,
dsn: DsnComponents,
): EventEnvelopeHeaders {
const baggage = event.sdkProcessingMetadata && event.sdkProcessingMetadata.baggage;
const { environment, release, transaction, userid, usersegment, samplerate, publickey, traceid } =
(baggage && getSentryBaggageItems(baggage)) || {};
const baggage: Baggage | undefined = event.sdkProcessingMetadata && event.sdkProcessingMetadata.baggage;
const dynamicSamplingContext = baggage && getSentryBaggageItems(baggage);

return {
event_id: event.event_id as string,
sent_at: new Date().toISOString(),
...(sdkInfo && { sdk: sdkInfo }),
...(!!tunnel && { dsn: dsnToString(dsn) }),
...(event.type === 'transaction' &&
baggage && {
trace: dropUndefinedKeys({
trace_id: traceid,
public_key: publickey,
sample_rate: samplerate,
environment,
release,
transaction,
...((userid || usersegment) && {
user: {
id: userid,
segment: usersegment,
},
}),
}) as EventTraceContext,
dynamicSamplingContext && {
trace: dropUndefinedKeys({ ...dynamicSamplingContext }) as DynamicSamplingContext,
}),
};
}
27 changes: 14 additions & 13 deletions packages/core/test/lib/envelope.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { DsnComponents, Event, EventTraceContext } from '@sentry/types';
import { DsnComponents, DynamicSamplingContext, Event } from '@sentry/types';

import { createEventEnvelope } from '../../src/envelope';

Expand All @@ -14,13 +14,13 @@ describe('createEventEnvelope', () => {
expect(envelopeHeaders.trace).toBeUndefined();
});

const testTable: Array<[string, Event, EventTraceContext]> = [
const testTable: Array<[string, Event, DynamicSamplingContext]> = [
[
'adds minimal baggage items',
{
type: 'transaction',
sdkProcessingMetadata: {
baggage: [{ traceid: '1234', publickey: 'pubKey123' }, '', false],
baggage: [{ trace_id: '1234', public_key: 'pubKey123' }, '', false],
},
},
{ trace_id: '1234', public_key: 'pubKey123' },
Expand All @@ -30,7 +30,7 @@ describe('createEventEnvelope', () => {
{
type: 'transaction',
sdkProcessingMetadata: {
baggage: [{ environment: 'prod', release: '1.0.0', publickey: 'pubKey123', traceid: '1234' }, '', false],
baggage: [{ environment: 'prod', release: '1.0.0', public_key: 'pubKey123', trace_id: '1234' }, '', false],
},
},
{ release: '1.0.0', environment: 'prod', trace_id: '1234', public_key: 'pubKey123' },
Expand All @@ -44,26 +44,27 @@ describe('createEventEnvelope', () => {
{
environment: 'prod',
release: '1.0.0',
userid: 'bob',
usersegment: 'segmentA',
transaction: 'TX',
samplerate: '0.95',
publickey: 'pubKey123',
traceid: '1234',
user_id: 'bob',
user_segment: 'segmentA',
sample_rate: '0.95',
public_key: 'pubKey123',
trace_id: '1234',
},
'',
false,
],
},
},
{
release: '1.0.0',
environment: 'prod',
user: { id: 'bob', segment: 'segmentA' },
release: '1.0.0',
transaction: 'TX',
trace_id: '1234',
public_key: 'pubKey123',
user_id: 'bob',
user_segment: 'segmentA',
sample_rate: '0.95',
public_key: 'pubKey123',
trace_id: '1234',
},
],
];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<meta name="sentry-trace" content="12312012123120121231201212312012-1121201211212012-1" />
<meta
name="baggage"
content="sentry-release=2.1.12,sentry-publickey=public,sentry-traceid=123,sentry-samplerate=0.3232"
content="sentry-release=2.1.12,sentry-public_key=public,sentry-trace_id=123,sentry-sample_rate=0.3232"
/>
</head>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { sentryTest } from '../../../utils/fixtures';
import { envelopeHeaderRequestParser, getFirstSentryEnvelopeRequest } from '../../../utils/helpers';

sentryTest(
'should send dynamic sampling context data in transaction envelope header',
'should send dynamic sampling context data in trace envelope header of a transaction envelope',
async ({ getLocalTestPath, page }) => {
const url = await getLocalTestPath({ testDir: __dirname });

Expand All @@ -15,10 +15,8 @@ sentryTest(
expect(envHeader.trace).toEqual({
environment: 'production',
transaction: expect.stringContaining('index.html'),
user: {
id: 'user123',
segment: 'segmentB',
},
user_id: 'user123',
user_segment: 'segmentB',
sample_rate: '1',
trace_id: expect.any(String),
public_key: 'public',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ test('Should populate and propagate sentry baggage if sentry-trace header does n
test_data: {
host: 'somewhere.not.sentry',
baggage: expect.stringContaining(
'sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,sentry-samplerate=1,' +
'sentry-publickey=public,sentry-traceid=',
'sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,' +
'sentry-public_key=public,sentry-trace_id=',
),
},
});
Expand All @@ -99,7 +99,7 @@ test('Should populate Sentry and propagate 3rd party content if sentry-trace hea
// 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=',
'sentry-public_key=public,sentry-trace_id=',
),
},
});
Expand Down
12 changes: 7 additions & 5 deletions packages/node/test/integrations/http.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,11 @@ describe('tracing', () => {
const baggageHeader = request.getHeader('baggage') as string;

expect(baggageHeader).toBeDefined();
expect(typeof baggageHeader).toEqual('string');
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',
'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,sentry-user_id=uid123,' +
'sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,' +
'sentry-trace_id=12312012123120121231201212312012,sentry-sample_rate=1',
);
});

Expand All @@ -124,10 +125,11 @@ describe('tracing', () => {
const baggageHeader = request.getHeader('baggage') as string;

expect(baggageHeader).toBeDefined();
expect(typeof baggageHeader).toEqual('string');
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',
'sentry-user_id=uid123,sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,' +
'sentry-trace_id=12312012123120121231201212312012,sentry-sample_rate=1',
);
});

Expand Down
69 changes: 32 additions & 37 deletions packages/tracing/src/transaction.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { getCurrentHub, Hub } from '@sentry/hub';
import {
Baggage,
BaggageObj,
Event,
Measurements,
Transaction as TransactionInterface,
Expand All @@ -10,11 +11,11 @@ import {
import {
createBaggage,
dropUndefinedKeys,
getSentryBaggageItems,
getThirdPartyBaggage,
isBaggageMutable,
isSentryBaggageEmpty,
logger,
setBaggageImmutable,
setBaggageValue,
} from '@sentry/utils';

import { Span as SpanClass, SpanRecorder } from './span';
Expand Down Expand Up @@ -228,40 +229,34 @@ export class Transaction extends SpanClass implements TransactionInterface {
const hub: Hub = this._hub || getCurrentHub();
const client = hub && hub.getClient();

const { environment, release } = (client && client.getOptions()) || {};
const { publicKey } = (client && client.getDsn()) || {};

const sampleRate = this.metadata && this.metadata.transactionSampling && this.metadata.transactionSampling.rate;
const traceId = this.traceId;
const transactionName = this.name;

let userId, userSegment;
hub.configureScope(scope => {
const { id, segment } = scope.getUser() || {};
userId = id;
userSegment = 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);
sampleRate &&
setBaggageValue(
baggage,
'samplerate',
// This will make sure that expnent notation (e.g. 1.45e-14) is converted to simple decimal representation
// Another edge case would be something like Number.NEGATIVE_INFINITY in which case we could still
// add something like .replace(/-?∞/, '0'). For the sake of saving bytes, I'll not add this until
// it becomes a problem
sampleRate.toLocaleString('fullwide', { useGrouping: false, maximumFractionDigits: 16 }),
);
publicKey && setBaggageValue(baggage, 'publickey', publicKey);
traceId && setBaggageValue(baggage, 'traceid', traceId);

setBaggageImmutable(baggage);

return baggage;
if (!client) return baggage;

const { environment, release } = client.getOptions() || {};
const { publicKey: public_key } = client.getDsn() || {};

const rate = this.metadata && this.metadata.transactionSampling && this.metadata.transactionSampling.rate;
const sample_rate =
rate !== undefined
? rate.toLocaleString('fullwide', { useGrouping: false, maximumFractionDigits: 16 })
: undefined;

const scope = hub.getScope();
const { id: user_id, segment: user_segment } = (scope && scope.getUser()) || {};

return createBaggage(
dropUndefinedKeys({
environment,
release,
transaction: this.name,
user_id,
user_segment,
public_key,
trace_id: this.traceId,
sample_rate,
...getSentryBaggageItems(baggage), // keep user-added values
} as BaggageObj),
getThirdPartyBaggage(baggage), // TODO: remove once we ignore 3rd party baggage
false, // set baggage immutable
);
}
}
4 changes: 2 additions & 2 deletions packages/tracing/test/browser/browsertracing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -507,9 +507,9 @@ describe('BrowserTracing', () => {
expect(baggage[0]).toEqual({
release: '1.0.0',
environment: 'production',
publickey: 'pubKey',
traceid: expect.any(String),
transaction: 'blank',
public_key: 'pubKey',
trace_id: expect.not.stringMatching('12312012123120121231201212312012'),
});
expect(baggage[1]).toBeDefined();
expect(baggage[1]).toEqual('');
Expand Down
8 changes: 4 additions & 4 deletions packages/tracing/test/span.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -444,8 +444,8 @@ describe('Span', () => {
release: '1.0.1',
environment: 'production',
transaction: 'tx',
samplerate: '0.56',
traceid: expect.any(String),
sample_rate: '0.56',
trace_id: expect.any(String),
});
expect(baggage && getThirdPartyBaggage(baggage)).toStrictEqual('');
});
Expand All @@ -468,8 +468,8 @@ describe('Span', () => {
release: '1.0.1',
environment: 'production',
transaction: 'tx',
samplerate: '0.0000000000000145',
traceid: expect.any(String),
sample_rate: '0.0000000000000145',
trace_id: expect.any(String),
});
expect(baggage && getThirdPartyBaggage(baggage)).toStrictEqual('');
});
Expand Down
12 changes: 3 additions & 9 deletions packages/types/src/baggage.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
export type AllowedBaggageKeys =
| 'environment'
| 'release'
| 'transaction'
| 'userid'
| 'usersegment'
| 'samplerate'
| 'traceid'
| 'publickey';
import { DynamicSamplingContext } from './envelope';

export type AllowedBaggageKeys = keyof DynamicSamplingContext;

export type BaggageObj = Partial<Record<AllowedBaggageKeys, string> & Record<string, string>>;

Expand Down
10 changes: 4 additions & 6 deletions packages/types/src/envelope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,15 @@ import { UserFeedback } from './user';
// Based on: https://develop.sentry.dev/sdk/envelopes/

// Based on https://github.com/getsentry/relay/blob/b23b8d3b2360a54aaa4d19ecae0231201f31df5e/relay-sampling/src/lib.rs#L685-L707
export type EventTraceContext = {
export type DynamicSamplingContext = {
trace_id: Transaction['traceId'];
public_key: DsnComponents['publicKey'];
sample_rate?: string;
release?: string;
user?: {
id?: string;
segment?: string;
};
environment?: string;
transaction?: string;
user_id?: string;
user_segment?: string;
};

export type EnvelopeItemType =
Expand Down Expand Up @@ -74,7 +72,7 @@ export type SessionItem =
| BaseEnvelopeItem<SessionAggregatesItemHeaders, SessionAggregates>;
export type ClientReportItem = BaseEnvelopeItem<ClientReportItemHeaders, ClientReport>;

export type EventEnvelopeHeaders = { event_id: string; sent_at: string; trace?: EventTraceContext };
export type EventEnvelopeHeaders = { event_id: string; sent_at: string; trace?: DynamicSamplingContext };
type SessionEnvelopeHeaders = { sent_at: string };
type ClientReportEnvelopeHeaders = BaseEnvelopeHeaders;

Expand Down
2 changes: 1 addition & 1 deletion packages/types/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ export type {
BaseEnvelopeItemHeaders,
ClientReportEnvelope,
ClientReportItem,
DynamicSamplingContext,
Envelope,
EnvelopeItemType,
EnvelopeItem,
EventEnvelope,
EventEnvelopeHeaders,
EventItem,
EventTraceContext,
SessionEnvelope,
SessionItem,
UserFeedbackItem,
Expand Down

0 comments on commit b9f2122

Please sign in to comment.