Skip to content

ref(tracing): Sync baggage data in Http and envelope headers #5218

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

Merged
merged 9 commits into from
Jun 17, 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
3 changes: 1 addition & 2 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -504,8 +504,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
// For now the decision is to skip normalization of Transactions and Spans,
// so this block overwrites the normalized event to add back the original
// Transaction information prior to normalization.
if (event.contexts && event.contexts.trace) {
normalized.contexts = {};
if (event.contexts && event.contexts.trace && normalized.contexts) {
normalized.contexts.trace = event.contexts.trace;

// event.contexts.trace.data may contain circular/dangerous data so we need to normalize it
Expand Down
28 changes: 18 additions & 10 deletions packages/core/src/envelope.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import {
BaggageObj,
DsnComponents,
Event,
EventEnvelope,
EventEnvelopeHeaders,
EventItem,
EventTraceContext,
SdkInfo,
SdkMetadata,
Session,
Expand Down Expand Up @@ -120,28 +122,34 @@ function createEventEnvelopeHeaders(
tunnel: string | undefined,
dsn: DsnComponents,
): EventEnvelopeHeaders {
const baggage = event.contexts && (event.contexts.baggage as BaggageObj);
const { environment, release, transaction, userid, usersegment } = baggage || {};

return {
event_id: event.event_id as string,
sent_at: new Date().toISOString(),
...(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 && {
// TODO: Grab this from 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.trace_id as string,
environment: event.environment,
release: event.release,
transaction: event.transaction,
user: event.user && {
id: event.user.id,
segment: event.user.segment,
},
trace_id: (event.contexts!.trace as Record<string, unknown>).trace_id as string,
public_key: dsn.publicKey,
}),
environment: environment,
release: release,
transaction: transaction,
...((userid || usersegment) && {
user: {
id: userid,
segment: usersegment,
},
}),
}) as EventTraceContext,
}),
};
}
25 changes: 16 additions & 9 deletions packages/core/test/lib/envelope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,16 @@ describe('createEventEnvelope', () => {

const testTable: Array<[string, Event, EventTraceContext]> = [
[
'adds only baggage item',
'adds one baggage item',
{
type: 'transaction',
release: '1.0.0',
contexts: {
trace: {
trace_id: '1234',
},
baggage: {
release: '1.0.0',
},
},
},
{ release: '1.0.0', trace_id: '1234', public_key: 'pubKey123' },
Expand All @@ -47,28 +49,33 @@ describe('createEventEnvelope', () => {
'adds two baggage items',
{
type: 'transaction',
release: '1.0.0',
environment: 'prod',
contexts: {
trace: {
trace_id: '1234',
},
baggage: {
environment: 'prod',
release: '1.0.0',
},
},
},
{ release: '1.0.0', environment: 'prod', trace_id: '1234', public_key: 'pubKey123' },
],
[
'adds all baggageitems',
'adds all baggage items',
{
type: 'transaction',
release: '1.0.0',
environment: 'prod',
user: { id: 'bob', segment: 'segmentA' },
transaction: 'TX',
contexts: {
trace: {
trace_id: '1234',
},
baggage: {
environment: 'prod',
release: '1.0.0',
userid: 'bob',
usersegment: 'segmentA',
transaction: 'TX',
},
},
},
{
Expand Down
11 changes: 6 additions & 5 deletions packages/integration-tests/suites/tracing/baggage/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,12 @@ sentryTest('should send trace context data in transaction envelope header', asyn
expect(envHeader.trace).toBeDefined();
expect(envHeader.trace).toMatchObject({
environment: 'production',
transaction: 'testTransactionBaggage',
user: {
id: 'user123',
segment: 'segmentB',
},
// TODO comment back in once we properly support transaction and user data in Baggage
// transaction: 'testTransactionBaggage',
// user: {
// id: 'user123',
// segment: 'segmentB',
// },
public_key: 'public',
trace_id: expect.any(String),
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import * as Sentry from '@sentry/browser';
import { Integrations } from '@sentry/tracing';

window.Sentry = Sentry;

Sentry.init({
dsn: 'https://[email protected]/1337',
integrations: [new Integrations.BrowserTracing()],
tracesSampleRate: 1,
environment: 'staging',
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
<head>
<meta charset="utf-8" />
<meta name="sentry-trace" content="12312012123120121231201212312012-1121201211212012-1" />
<meta name="baggage" content="sentry-version=2.1.12" />
<meta name="baggage" content="sentry-release=2.1.12" />
</head>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,19 @@ sentryTest(
},
);

// TODO this we can't really test until we actually propagate sentry- entries in baggage
// skipping for now but this must be adjusted later on
sentryTest.skip(
'should pick up `baggage` <meta> tag and propagate the content in transaction',
sentryTest(
'should pick up `baggage` <meta> tag, propagate the content in transaction and not add own data',
async ({ getLocalTestPath, page }) => {
const url = await getLocalTestPath({ testDir: __dirname });

const envHeader = await getFirstSentryEnvelopeRequest<EventEnvelopeHeaders>(page, url, envelopeHeaderRequestParser);

expect(envHeader.trace).toBeDefined();
expect(envHeader.trace).toEqual('{version:2.1.12}');
expect(envHeader.trace).toEqual({
public_key: 'public',
trace_id: expect.any(String),
release: '2.1.12',
});
},
);

Expand Down
7 changes: 3 additions & 4 deletions packages/tracing/src/span.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { Baggage, Hub, Primitive, Span as SpanInterface, SpanContext, Transactio
import {
createBaggage,
dropUndefinedKeys,
isBaggageEmpty,
isBaggageMutable,
isSentryBaggageEmpty,
setBaggageImmutable,
Expand Down Expand Up @@ -312,7 +311,7 @@ export class Span implements SpanInterface {
/**
* @inheritdoc
*/
public getBaggage(): Baggage | undefined {
public getBaggage(): Baggage {
const existingBaggage = this.transaction && this.transaction.metadata.baggage;

// Only add Sentry baggage items to baggage, if baggage does not exist yet or it is still
Expand All @@ -322,7 +321,7 @@ export class Span implements SpanInterface {
? this._getBaggageWithSentryValues(existingBaggage)
: existingBaggage;

return isBaggageEmpty(finalBaggage) ? undefined : finalBaggage;
return finalBaggage;
}

/**
Expand Down Expand Up @@ -366,7 +365,7 @@ export class Span implements SpanInterface {
*
* @param baggage
*
* @returns modified and immutable maggage object
* @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
Expand Down
3 changes: 2 additions & 1 deletion packages/tracing/src/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
TransactionContext,
TransactionMetadata,
} from '@sentry/types';
import { dropUndefinedKeys, logger } from '@sentry/utils';
import { dropUndefinedKeys, getSentryBaggageItems, logger } from '@sentry/utils';

import { Span as SpanClass, SpanRecorder } from './span';

Expand Down Expand Up @@ -122,6 +122,7 @@ export class Transaction extends SpanClass implements TransactionInterface {
const transaction: Event = {
contexts: {
trace: this.getTraceContext(),
baggage: getSentryBaggageItems(this.getBaggage()),
},
spans: finishedSpans,
start_timestamp: this.startTimestamp,
Expand Down
6 changes: 3 additions & 3 deletions packages/utils/src/baggage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,14 +151,14 @@ export function mergeAndSerializeBaggage(incomingBaggage?: Baggage, headerBaggag
* Extracted this logic to a function because it's duplicated in a lot of places.
*
* @param rawBaggageString
* @param sentryTraceData
* @param sentryTraceHeader
*/
export function parseBaggageSetMutability(
rawBaggageString: string | false | undefined | null,
sentryTraceData: TraceparentData | string | false | undefined | null,
sentryTraceHeader: TraceparentData | string | false | undefined | null,
): Baggage {
const baggage = parseBaggageString(rawBaggageString || '');
if (!isSentryBaggageEmpty(baggage) || (sentryTraceData && isSentryBaggageEmpty(baggage))) {
if (!isSentryBaggageEmpty(baggage) || (sentryTraceHeader && isSentryBaggageEmpty(baggage))) {
setBaggageImmutable(baggage);
}
return baggage;
Expand Down