fix(core): Record span client report outcomes when discarding transactions (#25006)

The [client report
spec](https://develop.sentry.dev/sdk/telemetry/client-reports/#span-outcomes)
calls for recording category `span` client outcomes, also if streaming
is disabled. This PR makes the change to record outcomes for each span,
including one for the transaction itself when a transaction is sampled
negatively, as well as when it's dropped.

The transaction/event processing pipeline had a bunch of holes for
discarded span counts, since any combination of `ignoreSpans`, event
processors and `beforeSendTransaction` could drop child spans in any of
those callbacks. So for example, a transaction that originally had 10
child spans and drops

- 2 spans in ignoreSpans
- 1 span in an event processor
- the whole transaction in beforeSendTransaction

now all together reports 11 `span`s and 1 `transaction` outcome, with
the correct reasons. No span drops are double-counted.

Related bugfix: Spans dropped by `ignoreSpans` without span streaming
are now recorded as `ignored` instead of `before_send`.

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: Lms24

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/linked-traces-static/consistent-sampling/meta-negative/test.ts

@@ -91,6 +91,11 @@ sentryTest.describe('When `consistentTraceSampling` is `true` and page contains
               quantity: 4,
               reason: 'sample_rate',
             },
+            {
+              category: 'span',
+              quantity: expect.any(Number),
+              reason: 'sample_rate',
+            },
           ],
         });
       });

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/linked-traces-static/consistent-sampling/meta-precedence/test.ts

@@ -75,6 +75,11 @@ sentryTest.describe('When `consistentTraceSampling` is `true` and page contains
               quantity: 2,
               reason: 'sample_rate',
             },
+            {
+              category: 'span',
+              quantity: expect.any(Number),
+              reason: 'sample_rate',
+            },
           ],
         });
       });

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/linked-traces-static/consistent-sampling/tracesSampler-precedence/test.ts

@@ -59,6 +59,11 @@ sentryTest.describe('When `consistentTraceSampling` is `true`', () => {
             quantity: 1,
             reason: 'sample_rate',
           },
+          {
+            category: 'span',
+            quantity: expect.any(Number),
+            reason: 'sample_rate',
+          },
         ],
       });
     });
@@ -80,6 +85,11 @@ sentryTest.describe('When `consistentTraceSampling` is `true`', () => {
             quantity: 1,
             reason: 'sample_rate',
           },
+          {
+            category: 'span',
+            quantity: expect.any(Number),
+            reason: 'sample_rate',
+          },
         ],
       });
     });

dev-packages/node-integration-tests/suites/tracing/sampling-static/test.ts

@@ -7,7 +7,13 @@ describe('negative sampling (static)', () => {
   });
 
   createEsmAndCjsTests(__dirname, 'server.mjs', 'instrument.mjs', (createRunner, test) => {
-    test('records sample_rate outcome for root span/transaction', async () => {
+    test('records sample_rate outcomes for the transaction and all of its spans', async () => {
+      // `/health` and `/ok` go through the same middleware, so the `/ok` transaction tells us how many
+      // spans the dropped `/health` transaction would have had. The count differs per runtime (e.g. Bun creates
+      // no Express spans), so we derive it instead of hardcoding it.
+      let okSpanCount: number | undefined;
+      let droppedSpanCount: number | undefined;
+
       const runner = createRunner()
         .unignore('client_report')
         // The `GET /ok` transaction is sent as soon as its span ends, while the negatively-sampled
@@ -16,19 +22,18 @@ describe('negative sampling (static)', () => {
         // match unordered instead of asserting a fixed sequence.
         .unordered()
         .expect({
-          transaction: {
-            transaction: 'GET /ok',
+          transaction: transaction => {
+            expect(transaction.transaction).toBe('GET /ok');
+            okSpanCount = 1 + (transaction.spans?.length ?? 0);
           },
         })
         .expect({
-          client_report: {
-            discarded_events: [
-              {
-                category: 'transaction',
-                quantity: 1,
-                reason: 'sample_rate',
-              },
-            ],
+          client_report: clientReport => {
+            expect(clientReport.discarded_events).toEqual([
+              { category: 'transaction', quantity: 1, reason: 'sample_rate' },
+              { category: 'span', quantity: expect.any(Number), reason: 'sample_rate' },
+            ]);
+            droppedSpanCount = clientReport.discarded_events[1]!.quantity;
           },
         })
         .start();
@@ -40,6 +45,8 @@ describe('negative sampling (static)', () => {
       expect((res2 as { status: string }).status).toBe('ok');
 
       await runner.completed();
+
+      expect(droppedSpanCount).toBe(okSpanCount);
     });
   });
 });

packages/core/src/client.ts

@@ -1532,39 +1532,59 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
     const isError = isErrorEvent(event);
     const eventType = event.type || 'error';
     const beforeSendLabel = `before send for type \`${eventType}\``;
-    let beforeSendDropReason: 'before_send' | 'callback_error' = 'before_send';
+    let beforeSendDropReason: BeforeSendDropReason = 'before_send';
+    let ignoredSpanCount = 0;
 
     // 1.0 === 100% events are sent
     // 0.0 === 0% events are sent
     // Sampling for transaction happens somewhere else
     const parsedSampleRate = typeof sampleRate === 'undefined' ? undefined : parseSampleRate(sampleRate);
     const dataCategory = getDataCategoryByType(event.type);
+    // Spans that event processors removed are already recorded in `prepareEvent`, so all later
+    // span outcomes are relative to the span count of the prepared event.
+    let preparedSpanCount = 0;
 
     return this._prepareEvent(event, hint, currentScope, isolationScope)
       .then(prepared => {
         if (prepared === null) {
           throw _makeDoNotSendEventError('An event processor returned `null`, will not send event.');
         }
 
+        preparedSpanCount = prepared.spans?.length || 0;
+
         if (isInternalException(hint)) {
           return prepared;
         }
 
-        const result = processBeforeSend(this, options, prepared, hint, () => {
-          beforeSendDropReason = 'callback_error';
-        });
+        const result = processBeforeSend(
+          options,
+          prepared,
+          hint,
+          reason => {
+            beforeSendDropReason = reason;
+          },
+          count => {
+            ignoredSpanCount = count;
+          },
+        );
         return _validateBeforeSendResult(result, beforeSendLabel);
       })
       .then(processedEvent => {
+        if (ignoredSpanCount) {
+          this.recordDroppedEvent('ignored', 'span', ignoredSpanCount);
+        }
+
         if (processedEvent === null) {
           this.recordDroppedEvent(beforeSendDropReason, dataCategory);
           if (isTransaction) {
-            const spans = event.spans || [];
-            // the transaction itself counts as one span, plus all the child spans that are added
-            this.recordDroppedEvent(beforeSendDropReason, 'span', 1 + spans.length);
+            // the transaction itself counts as one span, plus all the child spans that weren't ignored before
+            this.recordDroppedEvent(beforeSendDropReason, 'span', 1 + preparedSpanCount - ignoredSpanCount);
           }
-          const dropMessage = beforeSendDropReason === 'callback_error' ? 'threw an error' : 'returned `null`';
-          throw _makeDoNotSendEventError(`${beforeSendLabel} ${dropMessage}, will not send event.`);
+          const dropMessage =
+            beforeSendDropReason === 'ignored'
+              ? 'Transaction matched `ignoreSpans`'
+              : `${beforeSendLabel} ${beforeSendDropReason === 'callback_error' ? 'threw an error' : 'returned `null`'}`;
+          throw _makeDoNotSendEventError(`${dropMessage}, will not send event.`);
         }
 
         const session = currentScope.getSession() || isolationScope.getSession();
@@ -1580,10 +1600,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
         }
 
         if (isTransaction) {
-          const spanCountBefore = processedEvent.sdkProcessingMetadata?.spanCountBeforeProcessing || 0;
-          const spanCountAfter = processedEvent.spans ? processedEvent.spans.length : 0;
-
-          const droppedSpanCount = spanCountBefore - spanCountAfter;
+          const droppedSpanCount = preparedSpanCount - ignoredSpanCount - (processedEvent.spans?.length || 0);
           if (droppedSpanCount > 0) {
             this.recordDroppedEvent('before_send', 'span', droppedSpanCount);
           }
@@ -1733,15 +1750,17 @@ function _validateBeforeSendResult(
   return beforeSendResult;
 }
 
+type BeforeSendDropReason = 'before_send' | 'callback_error' | 'ignored';
+
 /**
  * Process the matching `beforeSendXXX` callback.
  */
 function processBeforeSend(
-  client: Client,
   options: ClientOptions,
   event: Event,
   hint: EventHint,
-  onCallbackError: () => void,
+  onDrop: (reason: Exclude<BeforeSendDropReason, 'before_send'>) => void,
+  onIgnoredSpans: (count: number) => void,
 ): PromiseLike<Event | null> | Event | null {
   const {
     beforeSend,
@@ -1759,7 +1778,7 @@ function processBeforeSend(
       DEBUG_BUILD ? 'The `beforeSend` callback threw an error, dropping the event:' : '',
       () => beforeSend(errorEvent, hint),
       () => {
-        onCallbackError();
+        onDrop('callback_error');
         return null;
       },
     );
@@ -1779,7 +1798,7 @@ function processBeforeSend(
           ignoreSpans,
         )
       ) {
-        // dropping the whole transaction!
+        onDrop('ignored');
         return null;
       }
 
@@ -1814,30 +1833,17 @@ function processBeforeSend(
           }
         }
 
-        const droppedSpans = processedEvent.spans.length - processedSpans.length;
-        if (droppedSpans) {
-          client.recordDroppedEvent('before_send', 'span', droppedSpans);
-        }
-
+        onIgnoredSpans(initialSpans.length - processedSpans.length);
         processedEvent.spans = processedSpans;
       }
     }
 
     if (beforeSendTransaction) {
-      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 = processedEvent.spans.length;
-        processedEvent.sdkProcessingMetadata = {
-          ...event.sdkProcessingMetadata,
-          spanCountBeforeProcessing: spanCountBefore,
-        };
-      }
       return safeCallback(
         DEBUG_BUILD ? 'The `beforeSendTransaction` callback threw an error, dropping the event:' : '',
         () => beforeSendTransaction(processedEvent as TransactionEvent, hint),
         () => {
-          onCallbackError();
+          onDrop('callback_error');
           return null;
         },
       );

packages/core/src/scope.ts

@@ -62,6 +62,9 @@ export interface SdkProcessingMetadata {
   dynamicSamplingContext?: Partial<DynamicSamplingContext>;
   capturedSpanScope?: Scope;
   capturedSpanIsolationScope?: Scope;
+  /**
+   * @deprecated This is no longer set or read by the SDK and will be removed in the next major version.
+   */
   spanCountBeforeProcessing?: number;
   ipAddress?: string;
 }

packages/core/src/tracing/sentrySpan.ts

@@ -392,8 +392,6 @@ export class SentrySpan implements Span {
       }
 
       DEBUG_BUILD && debug.log('[Tracing] Discarding standalone span because its trace was not chosen to be sampled.');
-      client.recordDroppedEvent('sample_rate', 'span');
-
       return;
     }
 

packages/core/src/tracing/trace.ts

@@ -308,7 +308,11 @@ export function startNewTrace<T>(callback: () => T): T {
  */
 function startMissingRequiredParentSpan(scope: Scope, client: Client | undefined): SentryNonRecordingSpan {
   client?.recordDroppedEvent('no_parent_span', 'span');
-  const span = new SentryNonRecordingSpan({ traceId: scope.getPropagationContext().traceId });
+  // Spans nested inside the placeholder inherit its drop reason in `_startChildSpan`
+  const span = new SentryNonRecordingSpan({
+    dropReason: 'no_parent_span',
+    traceId: scope.getPropagationContext().traceId,
+  });
   setCapturedScopesOnSpan(span, scope, getIsolationScope());
   return span;
 }
@@ -523,7 +527,14 @@ function _startRootSpan(
 
   if (!sampled && client && !_isTracingSuppressed) {
     DEBUG_BUILD && debug.log('[Tracing] Discarding root span because its trace was not chosen to be sampled.');
-    client.recordDroppedEvent(dropReason || 'sample_rate', hasSpanStreamingEnabled(client) ? 'span' : 'transaction');
+    const outcomeReason = dropReason || 'sample_rate';
+    // A standalone span is sent on its own and never becomes a transaction.
+    // TODO(v12): Drop the `isStandalone` check once the static trace lifecycle is gone.
+    if (!hasSpanStreamingEnabled(client) && !spanArguments.isStandalone) {
+      client.recordDroppedEvent(outcomeReason, 'transaction');
+    }
+    // Child spans of this root record their own `span` outcome in `_startChildSpan`.
+    client.recordDroppedEvent(outcomeReason, 'span');
   }
 
   setCapturedScopesOnSpan(rootSpan, scope, isolationScope);
@@ -568,11 +579,10 @@ function _startChildSpan(
     return childSpan;
   }
 
-  if (hasSpanStreamingEnabled(client) && spanIsNonRecordingSpan(childSpan)) {
+  if (spanIsNonRecordingSpan(childSpan)) {
     if (spanIsNonRecordingSpan(parentSpan) && parentSpan.dropReason) {
-      // We land here if the parent span was a segment span that was ignored (`ignoreSpans`).
-      // In this case, the child was also ignored (see `sampled` above) but we need to
-      // record a client outcome for the child.
+      // The parent was dropped for a reason other than sampling (e.g. an ignored segment span or
+      // an `onlyIfParent` placeholder), so the child is dropped for the same reason.
       childSpan.dropReason = parentSpan.dropReason;
       client.recordDroppedEvent(parentSpan.dropReason, 'span');
     } else if (!_isTracingSuppressed) {

packages/core/src/utils/prepareEvent.ts

@@ -109,6 +109,10 @@ export function prepareEvent(
   ];
 
   // Skip event processors for internal exceptions to prevent recursion
+  const isTransaction = event.type === 'transaction';
+  // Snapshot the count rather than reading `event.spans` later: processors get a shallow copy of the
+  // event, so one that mutates `spans` in place also mutates the original array.
+  const spanCountBeforeProcessing = event.spans?.length || 0;
   const result: PromiseLike<Event | null> = isInternalException(hint)
     ? resolvedSyncPromise(prepared)
     : notifyEventProcessors(eventProcessors, prepared, hint, 0, reason => {
@@ -117,8 +121,8 @@ export function prepareEvent(
         }
 
         client.recordDroppedEvent(reason, getDataCategoryByType(event.type));
-        if (event.type === 'transaction') {
-          client.recordDroppedEvent(reason, 'span', 1 + (event.spans || []).length);
+        if (isTransaction) {
+          client.recordDroppedEvent(reason, 'span', 1 + spanCountBeforeProcessing);
         }
       });
 
@@ -127,6 +131,13 @@ export function prepareEvent(
       return null;
     }
 
+    if (isTransaction && client) {
+      const droppedSpanCount = spanCountBeforeProcessing - (evt.spans?.length || 0);
+      if (droppedSpanCount > 0) {
+        client.recordDroppedEvent('event_processor', 'span', droppedSpanCount);
+      }
+    }
+
     // We apply the debug_meta field only after all event processors have ran, so that if any event processors modified
     // file names (e.g.the RewriteFrames integration) the filename -> debug ID relationship isn't destroyed.
     // This should not cause any PII issues, since we're only moving data that is already on the event and not adding