Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ref(tracing): Unify DSC key names in envelope and baggage headers #5302

Merged
merged 3 commits into from
Jun 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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