Skip to content

feat(core)!: Pass root spans to beforeSendSpan and disallow returning null #14831

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 13 commits into from
Jan 8, 2025
Merged
6 changes: 6 additions & 0 deletions docs/migration/v8-to-v9.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ Sentry.init({
});
```

- Dropping spans in the `beforeSendSpan` hook is no longer possible.
- The `beforeSendSpan` hook now receives the root span as well as the child spans.
- In previous versions, we determined if tracing is enabled (for Tracing Without Performance) by checking if either `tracesSampleRate` or `traceSampler` are _defined_ at all, in `Sentry.init()`. This means that e.g. the following config would lead to tracing without performance (=tracing being enabled, even if no spans would be started):

```js
Expand Down Expand Up @@ -243,6 +245,10 @@ The following outlines deprecations that were introduced in version 8 of the SDK
## General

- **Returning `null` from `beforeSendSpan` span is deprecated.**

Returning `null` from `beforeSendSpan` will now result in a warning being logged.
In v9, dropping spans is not possible anymore within this hook.

- **Passing `undefined` to `tracesSampleRate` / `tracesSampler` / `enableTracing` will be handled differently in v9**

In v8, a setup like the following:
Expand Down
51 changes: 33 additions & 18 deletions packages/core/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,11 @@ import { logger } from './utils-hoist/logger';
import { checkOrSetAlreadyCaught, uuid4 } from './utils-hoist/misc';
import { SyncPromise, rejectedSyncPromise, resolvedSyncPromise } from './utils-hoist/syncpromise';
import { getPossibleEventMessages } from './utils/eventUtils';
import { merge } from './utils/merge';
import { parseSampleRate } from './utils/parseSampleRate';
import { prepareEvent } from './utils/prepareEvent';
import { showSpanDropWarning } from './utils/spanUtils';
import { convertSpanJsonToTransactionEvent, convertTransactionEventToSpanJson } from './utils/transactionEvent';

const ALREADY_SEEN_ERROR = "Not capturing exception because it's already been captured.";
const MISSING_RELEASE_FOR_SESSION_ERROR = 'Discarded session because of missing or non-string release';
Expand Down Expand Up @@ -1004,41 +1006,54 @@ function processBeforeSend(
hint: EventHint,
): PromiseLike<Event | null> | Event | null {
const { beforeSend, beforeSendTransaction, beforeSendSpan } = options;
let processedEvent = event;

if (isErrorEvent(event) && beforeSend) {
return beforeSend(event, hint);
if (isErrorEvent(processedEvent) && beforeSend) {
return beforeSend(processedEvent, hint);
}

if (isTransactionEvent(event)) {
if (event.spans && beforeSendSpan) {
const processedSpans: SpanJSON[] = [];
for (const span of event.spans) {
const processedSpan = beforeSendSpan(span);
if (processedSpan) {
processedSpans.push(processedSpan);
} else {
showSpanDropWarning();
client.recordDroppedEvent('before_send', 'span');
if (isTransactionEvent(processedEvent)) {
if (beforeSendSpan) {
// process root span
const processedRootSpanJson = beforeSendSpan(convertTransactionEventToSpanJson(processedEvent));
if (!processedRootSpanJson) {
showSpanDropWarning();
} else {
// update event with processed root span values
processedEvent = merge(event, convertSpanJsonToTransactionEvent(processedRootSpanJson));
}

// process child spans
if (processedEvent.spans) {
const processedSpans: SpanJSON[] = [];
for (const span of processedEvent.spans) {
const processedSpan = beforeSendSpan(span);
if (!processedSpan) {
showSpanDropWarning();
processedSpans.push(span);
} else {
processedSpans.push(processedSpan);
}
}
processedEvent.spans = processedSpans;
}
event.spans = processedSpans;
}

if (beforeSendTransaction) {
if (event.spans) {
if (processedEvent.spans) {
// We store the # of spans before processing in SDK metadata,
// so we can compare it afterwards to determine how many spans were dropped
const spanCountBefore = event.spans.length;
event.sdkProcessingMetadata = {
const spanCountBefore = processedEvent.spans.length;
processedEvent.sdkProcessingMetadata = {
...event.sdkProcessingMetadata,
spanCountBeforeProcessing: spanCountBefore,
};
}
return beforeSendTransaction(event, hint);
return beforeSendTransaction(processedEvent as TransactionEvent, hint);
}
}

return event;
return processedEvent;
}

function isErrorEvent(event: Event): event is ErrorEvent {
Expand Down
13 changes: 8 additions & 5 deletions packages/core/src/envelope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import type {
SessionItem,
SpanEnvelope,
SpanItem,
SpanJSON,
} from './types-hoist';
import { dsnToString } from './utils-hoist/dsn';
import {
Expand Down Expand Up @@ -127,13 +126,17 @@ export function createSpanEnvelope(spans: [SentrySpan, ...SentrySpan[]], client?
const beforeSendSpan = client && client.getOptions().beforeSendSpan;
const convertToSpanJSON = beforeSendSpan
? (span: SentrySpan) => {
const spanJson = beforeSendSpan(spanToJSON(span) as SpanJSON);
if (!spanJson) {
const spanJson = spanToJSON(span);
const processedSpan = beforeSendSpan(spanJson);

if (!processedSpan) {
showSpanDropWarning();
return spanJson;
}
return spanJson;

return processedSpan;
}
: (span: SentrySpan) => spanToJSON(span);
: spanToJSON;

const items: SpanItem[] = [];
for (const span of spans) {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/types-hoist/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ export interface ClientOptions<TO extends BaseTransportOptions = BaseTransportOp
*
* @returns A new span that will be sent or null if the span should not be sent.
*/
beforeSendSpan?: (span: SpanJSON) => SpanJSON | null;
beforeSendSpan?: (span: SpanJSON) => SpanJSON;

/**
* An event-processing callback for transaction events, guaranteed to be invoked after all other event
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/utils/spanUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ export function showSpanDropWarning(): void {
consoleSandbox(() => {
// eslint-disable-next-line no-console
console.warn(
'[Sentry] Deprecation warning: Returning null from `beforeSendSpan` will be disallowed from SDK version 9.0.0 onwards. The callback will only support mutating spans. To drop certain spans, configure the respective integrations directly.',
'[Sentry] Returning null from `beforeSendSpan` is disallowed. To drop certain spans, configure the respective integrations directly.',
);
});
hasShownSpanDropWarning = true;
Expand Down
57 changes: 57 additions & 0 deletions packages/core/src/utils/transactionEvent.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME, SEMANTIC_ATTRIBUTE_PROFILE_ID } from '../semanticAttributes';
import type { SpanJSON, TransactionEvent } from '../types-hoist';
import { dropUndefinedKeys } from '../utils-hoist';

/**
* Converts a transaction event to a span JSON object.
*/
export function convertTransactionEventToSpanJson(event: TransactionEvent): SpanJSON {
const { trace_id, parent_span_id, span_id, status, origin, data, op } = event.contexts?.trace ?? {};

return dropUndefinedKeys({
data: data ?? {},
description: event.transaction,
op,
parent_span_id,
span_id: span_id ?? '',
start_timestamp: event.start_timestamp ?? 0,
status,
timestamp: event.timestamp,
trace_id: trace_id ?? '',
origin,
profile_id: data?.[SEMANTIC_ATTRIBUTE_PROFILE_ID] as string | undefined,
exclusive_time: data?.[SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME] as number | undefined,
measurements: event.measurements,
is_segment: true,
});
}

/**
* Converts a span JSON object to a transaction event.
*/
export function convertSpanJsonToTransactionEvent(span: SpanJSON): TransactionEvent {
const event: TransactionEvent = {
type: 'transaction',
timestamp: span.timestamp,
start_timestamp: span.start_timestamp,
transaction: span.description,
contexts: {
trace: {
trace_id: span.trace_id,
span_id: span.span_id,
parent_span_id: span.parent_span_id,
op: span.op,
status: span.status,
origin: span.origin,
data: {
...span.data,
...(span.profile_id && { [SEMANTIC_ATTRIBUTE_PROFILE_ID]: span.profile_id }),
...(span.exclusive_time && { [SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: span.exclusive_time }),
},
},
},
measurements: span.measurements,
};

return dropUndefinedKeys(event);
}
Loading
Loading