Skip to content

ref(tracing): Move getBaggage() from Span to Transaction class #5299

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 2 commits into from
Jun 23, 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: 2 additions & 1 deletion packages/node/src/integrations/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,13 @@ function _createWrappedRequestMethodFactory(
`[Tracing] Adding sentry-trace header ${sentryTraceHeader} to outgoing request to ${requestUrl}: `,
);

const baggage = parentSpan.transaction && parentSpan.transaction.getBaggage();
const headerBaggageString = requestOptions.headers && requestOptions.headers.baggage;

requestOptions.headers = {
...requestOptions.headers,
'sentry-trace': sentryTraceHeader,
baggage: mergeAndSerializeBaggage(span.getBaggage(), headerBaggageString),
baggage: mergeAndSerializeBaggage(baggage, headerBaggageString),
};
}
}
Expand Down
8 changes: 4 additions & 4 deletions packages/tracing/src/browser/request.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable max-lines */
import type { Span } from '@sentry/types';
import type { Baggage, Span } from '@sentry/types';
import {
addInstrumentationHandler,
BAGGAGE_HEADER_NAME,
Expand Down Expand Up @@ -198,12 +198,13 @@ export function fetchCallback(
const request = (handlerData.args[0] = handlerData.args[0] as string | Request);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const options = (handlerData.args[1] = (handlerData.args[1] as { [key: string]: any }) || {});
options.headers = addTracingHeaders(request, span, options);
options.headers = addTracingHeaders(request, activeTransaction.getBaggage(), span, options);
}
}

function addTracingHeaders(
request: string | Request,
incomingBaggage: Baggage | undefined,
span: Span,
options: { [key: string]: any },
): PolymorphicRequestHeaders {
Expand All @@ -212,7 +213,6 @@ function addTracingHeaders(
if (isInstanceOf(request, Request)) {
headers = (request as Request).headers;
}
const incomingBaggage = span.getBaggage();

if (headers) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
Expand Down Expand Up @@ -302,7 +302,7 @@ export function xhrCallback(

handlerData.xhr.setRequestHeader(
BAGGAGE_HEADER_NAME,
mergeAndSerializeBaggage(span.getBaggage(), headerBaggageString),
mergeAndSerializeBaggage(activeTransaction.getBaggage(), headerBaggageString),
);
} catch (_) {
// Error: InvalidStateError: Failed to execute 'setRequestHeader' on 'XMLHttpRequest': The object's state must be OPENED.
Expand Down
96 changes: 2 additions & 94 deletions packages/tracing/src/span.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,6 @@
/* eslint-disable max-lines */
import { getCurrentHub } from '@sentry/hub';
import { Baggage, Hub, Primitive, Span as SpanInterface, SpanContext, Transaction } from '@sentry/types';
import {
createBaggage,
dropUndefinedKeys,
isBaggageMutable,
isSentryBaggageEmpty,
setBaggageImmutable,
setBaggageValue,
timestampWithMs,
uuid4,
} from '@sentry/utils';
import { Primitive, Span as SpanInterface, SpanContext, Transaction } from '@sentry/types';
import { dropUndefinedKeys, timestampWithMs, uuid4 } from '@sentry/utils';

/**
* Keeps track of finished spans for a given transaction
Expand Down Expand Up @@ -308,29 +298,6 @@ export class Span implements SpanInterface {
});
}

/**
* @inheritdoc
*/
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
// empty and mutable
// TODO: we might want to ditch the isSentryBaggageEmpty condition because it prevents
// custom sentry-values in DSC (added by users in the future)
const finalBaggage =
!existingBaggage || (isBaggageMutable(existingBaggage) && isSentryBaggageEmpty(existingBaggage))
? this._populateBaggageWithSentryValues(existingBaggage)
: existingBaggage;

// In case, we poulated the DSC, we have update the stored one on the transaction.
if (existingBaggage !== finalBaggage && this.transaction) {
this.transaction.metadata.baggage = finalBaggage;
}

return finalBaggage;
}

/**
* @inheritDoc
*/
Expand Down Expand Up @@ -360,65 +327,6 @@ export class Span implements SpanInterface {
trace_id: this.traceId,
});
}

/**
* Collects and adds data to the passed baggage object.
*
* Note: This function does not explicitly check if the passed baggage object is allowed
* to be modified. Implicitly, `setBaggageValue` will not make modification to the object
* if it was already set immutable.
*
* After adding the data, the baggage object is set immutable to prevent further modifications.
*
* @param baggage
*
* @returns modified and immutable baggage object
*/
private _populateBaggageWithSentryValues(baggage: Baggage = createBaggage({})): Baggage {
// Because of a cicular dependency, we cannot import the Transaction class here, hence the type casts
// 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 && hub.getClient();

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

const metadata = this.transaction && this.transaction.metadata;
const sampleRate = metadata && metadata.transactionSampling && metadata.transactionSampling.rate;

const traceId = this.traceId;

const transactionName = this.transaction && this.transaction.name;

let userId, userSegment;
hub.withScope(scope => {
const user = scope.getUser();
userId = user && user.id;
userSegment = user && user.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;
}
}

export type SpanStatusType =
Expand Down
90 changes: 89 additions & 1 deletion packages/tracing/src/transaction.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,21 @@
import { getCurrentHub, Hub } from '@sentry/hub';
import {
Baggage,
Event,
Measurements,
Transaction as TransactionInterface,
TransactionContext,
TransactionMetadata,
} from '@sentry/types';
import { dropUndefinedKeys, logger } from '@sentry/utils';
import {
createBaggage,
dropUndefinedKeys,
isBaggageMutable,
isSentryBaggageEmpty,
logger,
setBaggageImmutable,
setBaggageValue,
} from '@sentry/utils';

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

Expand Down Expand Up @@ -176,4 +185,83 @@ export class Transaction extends SpanClass implements TransactionInterface {

return this;
}

/**
* @inheritdoc
*
* @experimental
*/
public getBaggage(): Baggage {
const existingBaggage = this.metadata.baggage;

// Only add Sentry baggage items to baggage, if baggage does not exist yet or it is still
// empty and mutable
// TODO: we might want to ditch the isSentryBaggageEmpty condition because it prevents
// custom sentry-values in DSC (added by users in the future)
const finalBaggage =
!existingBaggage || (isBaggageMutable(existingBaggage) && isSentryBaggageEmpty(existingBaggage))
? this._populateBaggageWithSentryValues(existingBaggage)
: existingBaggage;

// In case, we poulated the DSC, we have update the stored one on the transaction.
if (existingBaggage !== finalBaggage) {
this.metadata.baggage = finalBaggage;
}

return finalBaggage;
}

/**
* Collects and adds data to the passed baggage object.
*
* Note: This function does not explicitly check if the passed baggage object is allowed
* to be modified. Implicitly, `setBaggageValue` will not make modification to the object
* if it was already set immutable.
*
* After adding the data, the baggage object is set immutable to prevent further modifications.
*
* @param baggage
*
* @returns modified and immutable baggage object
*/
private _populateBaggageWithSentryValues(baggage: Baggage = createBaggage({})): Baggage {
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;
}
}
8 changes: 2 additions & 6 deletions packages/tracing/test/span.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -414,9 +414,7 @@ describe('Span', () => {

const hubSpy = jest.spyOn(hub.getClient()!, 'getOptions');

const span = transaction.startChild();

const baggage = span.getBaggage();
const baggage = transaction.getBaggage();

expect(hubSpy).toHaveBeenCalledTimes(0);
expect(baggage && isSentryBaggageEmpty(baggage)).toBe(false);
Expand All @@ -438,9 +436,7 @@ describe('Span', () => {

const hubSpy = jest.spyOn(hub.getClient()!, 'getOptions');

const span = transaction.startChild();

const baggage = span.getBaggage();
const baggage = transaction.getBaggage();

expect(hubSpy).toHaveBeenCalledTimes(1);
expect(baggage && isSentryBaggageEmpty(baggage)).toBe(false);
Expand Down
5 changes: 1 addition & 4 deletions packages/types/src/span.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { Baggage } from './baggage';
import { Primitive } from './misc';
import { Transaction } from './transaction';

Expand Down Expand Up @@ -162,6 +161,7 @@ export interface Span extends SpanContext {
tags?: { [key: string]: Primitive };
trace_id: string;
};

/** Convert the object to JSON */
toJSON(): {
data?: { [key: string]: any };
Expand All @@ -175,7 +175,4 @@ export interface Span extends SpanContext {
timestamp?: number;
trace_id: string;
};

/** return the baggage for dynamic sampling and trace propagation */
getBaggage(): Baggage | undefined;
}
3 changes: 3 additions & 0 deletions packages/types/src/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ export interface Transaction extends TransactionContext, Span {

/** Updates the current transaction with a new `TransactionContext` */
updateWithContext(transactionContext: TransactionContext): this;

/** return the baggage for dynamic sampling and trace propagation */
getBaggage(): Baggage;
}

/**
Expand Down
3 changes: 1 addition & 2 deletions packages/utils/src/baggage.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Baggage, BaggageObj, TraceparentData } from '@sentry/types';
import { HttpHeaderValue } from '@sentry/types';
import { Baggage, BaggageObj, HttpHeaderValue, TraceparentData } from '@sentry/types';

import { isString } from './is';
import { logger } from './logger';
Expand Down