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

@@ -51,9 +51,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';
@@ -972,41 +974,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 {

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,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);
+}

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,81 @@ 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('does not modify existing contexts for root span in `beforeSendSpan`', () => {
+      const beforeSendSpan = jest.fn((span: SpanJSON) => {
+        return {
+          ...span,
+          data: {
+            modified: 'true',
+          },
+        };
+      });
+      const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendSpan });
+      const client = new TestClient(options);
+
+      const transaction: Event = {
+        transaction: '/animals/are/great',
+        type: 'transaction',
+        spans: [],
+        breadcrumbs: [
+          {
+            type: 'ui.click',
+          },
+        ],
+        contexts: {
+          trace: {
+            data: {
+              modified: 'false',
+              dropMe: 'true',
+            },
+            span_id: '9e15bf99fbe4bc80',
+            trace_id: '86f39e84263a4de99c326acab3bfe3bd',
+          },
+          app: {
+            data: {
+              modified: 'false',
+            },
+          },
+        },
+      };
+      client.captureEvent(transaction);
+
+      expect(beforeSendSpan).toHaveBeenCalledTimes(1);
+      const capturedEvent = TestClient.instance!.event!;
+      expect(capturedEvent).toEqual({
+        transaction: '/animals/are/great',
+        breadcrumbs: [
+          {
+            type: 'ui.click',
+          },
+        ],
+        type: 'transaction',
+        spans: [],
+        environment: 'production',
+        event_id: '12312012123120121231201212312012',
+        start_timestamp: 0,
+        timestamp: 2020,
+        contexts: {
+          trace: {
+            data: {
+              modified: 'true',
+            },
+            span_id: '9e15bf99fbe4bc80',
+            trace_id: '86f39e84263a4de99c326acab3bfe3bd',
+          },
+          app: {
+            data: {
+              modified: 'false',
+            },
+          },
+        },
+      });
     });
 
     test('calls `beforeSend` and uses the modified event', () => {
@@ -1084,7 +1157,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 +1167,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 +1189,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 +1236,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 +1264,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();
     });