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

docs/migration/v8-to-v9.md

@@ -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
@@ -223,6 +225,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:

packages/core/src/baseclient.ts

@@ -54,6 +54,7 @@ import { getPossibleEventMessages } from './utils/eventUtils';
 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';
@@ -972,41 +973,57 @@ 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 = {
+          ...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 {

packages/core/src/envelope.ts

@@ -18,7 +18,6 @@ import type {
   SessionItem,
   SpanEnvelope,
   SpanItem,
-  SpanJSON,
 } from './types-hoist';
 import { dsnToString } from './utils-hoist/dsn';
 import {
@@ -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) {

packages/core/src/types-hoist/options.ts

@@ -300,7 +300,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

packages/core/src/utils/spanUtils.ts

@@ -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;

packages/core/src/utils/transactionEvent.ts

@@ -0,0 +1,59 @@
+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,
+  });
+}
+
+/**
+ * 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 }),
+        },
+      },
+    },
+  };
+
+  if (span.measurements) {
+    event.measurements = span.measurements;
+  }
+
+  return dropUndefinedKeys(event);
+}

packages/core/test/lib/baseclient.test.ts

@@ -1,3 +1,4 @@
+import type { SpanJSON } from '@sentry/core';
 import { SentryError, SyncPromise, dsnToString } from '@sentry/core';
 import type { Client, Envelope, ErrorEvent, Event, TransactionEvent } from '../../src/types-hoist';
 
@@ -994,14 +995,14 @@ describe('BaseClient', () => {
     });
 
     test('calls `beforeSendSpan` and uses original spans without any changes', () => {
-      expect.assertions(2);
+      expect.assertions(3);
 
       const beforeSendSpan = jest.fn(span => span);
       const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendSpan });
       const client = new TestClient(options);
 
       const transaction: Event = {
-        transaction: '/cats/are/great',
+        transaction: '/dogs/are/great',
         type: 'transaction',
         spans: [
           {
@@ -1022,9 +1023,10 @@ describe('BaseClient', () => {
       };
       client.captureEvent(transaction);
 
-      expect(beforeSendSpan).toHaveBeenCalledTimes(2);
+      expect(beforeSendSpan).toHaveBeenCalledTimes(3);
       const capturedEvent = TestClient.instance!.event!;
       expect(capturedEvent.spans).toEqual(transaction.spans);
+      expect(capturedEvent.transaction).toEqual(transaction.transaction);
     });
 
     test('calls `beforeSend` and uses the modified event', () => {
@@ -1084,7 +1086,7 @@ describe('BaseClient', () => {
     });
 
     test('calls `beforeSendSpan` and uses the modified spans', () => {
-      expect.assertions(3);
+      expect.assertions(4);
 
       const beforeSendSpan = jest.fn(span => {
         span.data = { version: 'bravo' };
@@ -1094,7 +1096,7 @@ describe('BaseClient', () => {
       const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendSpan });
       const client = new TestClient(options);
       const transaction: Event = {
-        transaction: '/cats/are/great',
+        transaction: '/dogs/are/great',
         type: 'transaction',
         spans: [
           {
@@ -1116,12 +1118,13 @@ describe('BaseClient', () => {
 
       client.captureEvent(transaction);
 
-      expect(beforeSendSpan).toHaveBeenCalledTimes(2);
+      expect(beforeSendSpan).toHaveBeenCalledTimes(3);
       const capturedEvent = TestClient.instance!.event!;
       for (const [idx, span] of capturedEvent.spans!.entries()) {
         const originalSpan = transaction.spans![idx];
         expect(span).toEqual({ ...originalSpan, data: { version: 'bravo' } });
       }
+      expect(capturedEvent.contexts?.trace?.data).toEqual({ version: 'bravo' });
     });
 
     test('calls `beforeSend` and discards the event', () => {
@@ -1162,15 +1165,14 @@ describe('BaseClient', () => {
       expect(loggerWarnSpy).toBeCalledWith('before send for type `transaction` returned `null`, will not send event.');
     });
 
-    test('calls `beforeSendSpan` and discards the span', () => {
+    test('does not discard span and warn when returning null from `beforeSendSpan`', () => {
       const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => undefined);
-
-      const beforeSendSpan = jest.fn(() => null);
+      const beforeSendSpan = jest.fn(() => null as unknown as SpanJSON);
       const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendSpan });
       const client = new TestClient(options);
 
       const transaction: Event = {
-        transaction: '/cats/are/great',
+        transaction: '/dogs/are/great',
         type: 'transaction',
         spans: [
           {
@@ -1191,14 +1193,13 @@ describe('BaseClient', () => {
       };
       client.captureEvent(transaction);
 
-      expect(beforeSendSpan).toHaveBeenCalledTimes(2);
+      expect(beforeSendSpan).toHaveBeenCalledTimes(3);
       const capturedEvent = TestClient.instance!.event!;
-      expect(capturedEvent.spans).toHaveLength(0);
-      expect(client['_outcomes']).toEqual({ 'before_send:span': 2 });
+      expect(capturedEvent.spans).toHaveLength(2);
+      expect(client['_outcomes']).toEqual({});
 
-      expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
-      expect(consoleWarnSpy).toBeCalledWith(
-        '[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.',
+      expect(consoleWarnSpy).toHaveBeenCalledWith(
+        '[Sentry] Returning null from `beforeSendSpan` is disallowed. To drop certain spans, configure the respective integrations directly.',
       );
       consoleWarnSpy.mockRestore();
     });

packages/core/test/lib/tracing/sentrySpan.test.ts

@@ -1,3 +1,4 @@
+import type { SpanJSON } from '../../../src';
 import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, getCurrentScope, setCurrentClient, timestampInSeconds } from '../../../src';
 import { SentrySpan } from '../../../src/tracing/sentrySpan';
 import { SPAN_STATUS_ERROR } from '../../../src/tracing/spanstatus';
@@ -176,10 +177,10 @@ describe('SentrySpan', () => {
       expect(mockSend).toHaveBeenCalled();
     });
 
-    test('does not send the span if `beforeSendSpan` drops the span', () => {
+    test('does not drop the span if `beforeSendSpan` returns null', () => {
       const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => undefined);
 
-      const beforeSendSpan = jest.fn(() => null);
+      const beforeSendSpan = jest.fn(() => null as unknown as SpanJSON);
       const client = new TestClient(
         getDefaultTestClientOptions({
           dsn: 'https://username@domain/123',
@@ -201,12 +202,11 @@ describe('SentrySpan', () => {
       });
       span.end();
 
-      expect(mockSend).not.toHaveBeenCalled();
-      expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'span');
+      expect(mockSend).toHaveBeenCalled();
+      expect(recordDroppedEventSpy).not.toHaveBeenCalled();
 
-      expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
-      expect(consoleWarnSpy).toBeCalledWith(
-        '[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.',
+      expect(consoleWarnSpy).toHaveBeenCalledWith(
+        '[Sentry] Returning null from `beforeSendSpan` is disallowed. To drop certain spans, configure the respective integrations directly.',
       );
       consoleWarnSpy.mockRestore();
     });