fix(tracing): Discard invalid navigation transactions via event processor

CHANGELOG.md

@@ -16,6 +16,7 @@
 ### Fixes
 
 - Stop the Hermes sampling profiler on React instance teardown to prevent `pthread_kill` SIGABRT when the JS thread is torn down with profiling active ([#6035](https://github.com/getsentry/sentry-react-native/pull/6035))
+- Discard invalid navigation/interaction transactions via an event processor instead of mutating the internal `_sampled` flag, removing misleading "dropped due to sampling" debug logs ([#6051](https://github.com/getsentry/sentry-react-native/pull/6051))
 
 ### Dependencies
 

packages/core/src/js/tracing/integrations/appStart.ts

@@ -23,6 +23,7 @@ import {
 } from '../../measurements';
 import { convertSpanToTransaction, isRootSpan, setEndTimeValue } from '../../utils/span';
 import { NATIVE } from '../../wrapper';
+import { getRootSpanDiscardReason, getTransactionEventDiscardReason } from '../onSpanEndUtils';
 import {
   APP_START_COLD as APP_START_COLD_OP,
   APP_START_WARM as APP_START_WARM_OP,
@@ -353,14 +354,14 @@ export const appStartIntegration = ({
   const recordFirstStartedActiveRootSpanId = (rootSpan: Span): void => {
     if (firstStartedActiveRootSpanId) {
       // Check if the previously locked span was dropped after it ended (e.g., by
-      // ignoreEmptyRouteChangeTransactions or ignoreEmptyBackNavigation setting
-      // _sampled = false during spanEnd). If so, reset and allow this new span.
+      // ignoreEmptyRouteChangeTransactions or ignoreEmptyBackNavigation marking
+      // it for discard during spanEnd). If so, reset and allow this new span.
       // We check here (at the next spanStart) rather than at spanEnd because
       // the discard listeners run after the app start listener in registration order,
-      // so _sampled is not yet false when our own spanEnd listener would fire.
-      if (firstStartedActiveRootSpan && !spanIsSampled(firstStartedActiveRootSpan)) {
+      // so the discard attribute is not yet set when our own spanEnd listener would fire.
+      if (firstStartedActiveRootSpan && getRootSpanDiscardReason(firstStartedActiveRootSpan) !== undefined) {
         debug.log(
-          '[AppStart] Previously locked root span was unsampled after ending. Resetting to allow next transaction.',
+          '[AppStart] Previously locked root span was marked for discard after ending. Resetting to allow next transaction.',
         );
         resetFirstStartedActiveRootSpanId();
         // Fall through to lock to this new span
@@ -468,6 +469,16 @@ export const appStartIntegration = ({
       return;
     }
 
+    // Don't attach (and don't flip the flushed flag) for transactions the
+    // tracing integration is about to drop — e.g. empty back-navigations,
+    // route-change spans that never received route info, or childless idle
+    // spans. Otherwise the next real transaction would be left without app
+    // start data because `appStartDataFlushed` would already be `true`.
+    if (getTransactionEventDiscardReason(event)) {
+      debug.log('[AppStart] Skipping app start attach for transaction marked for discard.');
+      return;
+    }
+
     if (!event.contexts?.trace) {
       debug.warn('[AppStart] Transaction event is missing trace context. Can not attach app start.');
       return;

packages/core/src/js/tracing/onSpanEndUtils.ts

@@ -1,11 +1,55 @@
-import type { Client, Span } from '@sentry/core';
+import type { Client, Event, Span } from '@sentry/core';
 import type { AppStateStatus } from 'react-native';
 
 import { debug, getSpanDescendants, SPAN_STATUS_ERROR, spanToJSON, timestampInSeconds } from '@sentry/core';
 import { AppState, Platform } from 'react-native';
 
 import { isRootSpan, isSentrySpan } from '../utils/span';
 
+/**
+ * Attribute used to mark root spans whose corresponding transaction event
+ * should be dropped by the React Native tracing event processor instead of
+ * being treated as sampled-out by `@sentry/core`.
+ *
+ * The value is a stable string identifier describing why the SDK chose to
+ * discard the transaction (e.g. an empty back-navigation, a route-change
+ * transaction that never received route information, or a childless idle
+ * transaction).
+ */
+export const SENTRY_DISCARD_REASON_ATTRIBUTE = 'sentry.rn.discard_reason';
+
+export type SentryDiscardReason =
+  | 'empty_back_navigation'
+  | 'no_route_info'
+  | 'no_child_spans'
+  | 'discarded_latest_navigation';
+
+/**
+ * Marks a root span so its transaction event will be filtered out by the
+ * tracing integration's event processor. The span itself is left with its
+ * original sampling decision so debug logs no longer report "dropped due to
+ * sampling" for SDK-side discards.
+ */
+export function markRootSpanForDiscard(span: Span, reason: SentryDiscardReason): void {
+  span.setAttribute(SENTRY_DISCARD_REASON_ATTRIBUTE, reason);
+}
+
+/**
+ * Returns the SDK-side discard reason recorded on a root span, if any.
+ */
+export function getRootSpanDiscardReason(span: Span): SentryDiscardReason | undefined {
+  const value = spanToJSON(span).data?.[SENTRY_DISCARD_REASON_ATTRIBUTE];
+  return typeof value === 'string' ? (value as SentryDiscardReason) : undefined;
+}
+
+/**
+ * Returns the SDK-side discard reason from a transaction event, if any.
+ */
+export function getTransactionEventDiscardReason(event: Event): SentryDiscardReason | undefined {
+  const value = event.contexts?.trace?.data?.[SENTRY_DISCARD_REASON_ATTRIBUTE];
+  return typeof value === 'string' ? (value as SentryDiscardReason) : undefined;
+}
+
 /**
  * The time to wait after the app enters the `inactive` state on iOS before
  * cancelling the span.
@@ -104,7 +148,6 @@ function discardEmptyNavigationSpan(
     const meaningfulChildren = getMeaningfulChildSpans(span);
     if (meaningfulChildren.length <= 0) {
       onDiscardFn(span);
-      span['_sampled'] = false;
     }
   });
 }
@@ -115,11 +158,12 @@ export const ignoreEmptyBackNavigation = (client: Client | undefined, span: Span
     span,
     // Only discard if route has been seen before
     span => spanToJSON(span).data?.['route.has_been_seen'] === true,
-    // Log message when discarding
-    () => {
+    // Log message and mark the span for discard via the event processor.
+    span => {
       debug.log(
         'Not sampling transaction as route has been seen before. Pass ignoreEmptyBackNavigationTransactions = false to disable this feature.',
       );
+      markRootSpanForDiscard(span, 'empty_back_navigation');
     },
   );
 };
@@ -151,10 +195,13 @@ export const ignoreEmptyRouteChangeTransactions = (
         spanJSON.description === defaultNavigationSpanName && !spanJSON.data?.['route.name'] && isSpanStillTracked()
       );
     },
-    // Log and record dropped event
-    _span => {
+    // Log and mark the span for discard. The actual `recordDroppedEvent` call
+    // happens in the tracing integration's event processor with the correct
+    // `event_processor` reason, so we no longer record it here as a sampling
+    // drop.
+    span => {
       debug.log(`Discarding empty "${defaultNavigationSpanName}" transaction that never received route information.`);
-      client?.recordDroppedEvent('sample_rate', 'transaction');
+      markRootSpanForDiscard(span, 'no_route_info');
     },
   );
 };
@@ -180,7 +227,7 @@ export const onlySampleIfChildSpans = (client: Client, span: Span): void => {
     if (children.length <= 1) {
       // Span always has at lest one child, itself
       debug.log(`Not sampling as ${spanToJSON(span).op} transaction has no child spans.`);
-      span['_sampled'] = false;
+      markRootSpanForDiscard(span, 'no_child_spans');
     }
   });
 };

packages/core/src/js/tracing/reactnativenavigation.ts

@@ -13,7 +13,11 @@ import type { EmitterSubscription } from '../utils/rnlibrariesinterface';
 import type { ReactNativeTracingIntegration } from './reactnativetracing';
 
 import { isSentrySpan } from '../utils/span';
-import { ignoreEmptyBackNavigation, ignoreEmptyRouteChangeTransactions } from './onSpanEndUtils';
+import {
+  ignoreEmptyBackNavigation,
+  ignoreEmptyRouteChangeTransactions,
+  markRootSpanForDiscard,
+} from './onSpanEndUtils';
 import { SPAN_ORIGIN_AUTO_NAVIGATION_REACT_NATIVE_NAVIGATION } from './origin';
 import { getReactNativeTracingIntegration } from './reactnativetracing';
 import {
@@ -221,7 +225,7 @@ export const reactNativeNavigationIntegration = ({
   const discardLatestNavigationSpan = (): void => {
     if (latestNavigationSpan) {
       if (isSentrySpan(latestNavigationSpan)) {
-        latestNavigationSpan['_sampled'] = false;
+        markRootSpanForDiscard(latestNavigationSpan, 'discarded_latest_navigation');
       }
       // TODO: What if it's not SentrySpan?
       latestNavigationSpan.end();

packages/core/src/js/tracing/reactnativetracing.ts

@@ -1,10 +1,11 @@
-import type { Client, Event, Integration, StartSpanOptions } from '@sentry/core';
+import type { Client, Event, EventHint, Integration, StartSpanOptions } from '@sentry/core';
 
 import { instrumentOutgoingRequests } from '@sentry/browser';
-import { getClient } from '@sentry/core';
+import { debug, getClient } from '@sentry/core';
 
 import { isWeb } from '../utils/environment';
 import { getDevServer } from './../integrations/debugsymbolicatorutils';
+import { getTransactionEventDiscardReason } from './onSpanEndUtils';
 import { addDefaultOpForSpanFrom, addThreadInfoToSpan, defaultIdleOptions } from './span';
 
 export const INTEGRATION_NAME = 'ReactNativeTracing';
@@ -136,7 +137,18 @@ export const reactNativeTracingIntegration = (
     });
   };
 
-  const processEvent = (event: Event): Event => {
+  const processEvent = (event: Event, _hint: EventHint, _client: Client): Event | null => {
+    if (event.type === 'transaction') {
+      const discardReason = getTransactionEventDiscardReason(event);
+      if (discardReason) {
+        debug.log(`[ReactNativeTracing] Dropping transaction marked for discard (reason: ${discardReason}).`);
+        // `@sentry/core` automatically records a dropped event with reason
+        // `event_processor` when a processor returns `null`, so we don't call
+        // `recordDroppedEvent` here to avoid double-counting in client reports.
+        return null;
+      }
+    }
+
     if (event.contexts && state.currentRoute) {
       event.contexts.app = { view_names: [state.currentRoute], ...event.contexts.app };
     }

packages/core/src/js/tracing/reactnavigation.ts

@@ -21,7 +21,11 @@ import { getAppRegistryIntegration } from '../integrations/appRegistry';
 import { isSentrySpan } from '../utils/span';
 import { RN_GLOBAL_OBJ } from '../utils/worldwide';
 import { NATIVE } from '../wrapper';
-import { ignoreEmptyBackNavigation, ignoreEmptyRouteChangeTransactions } from './onSpanEndUtils';
+import {
+  ignoreEmptyBackNavigation,
+  ignoreEmptyRouteChangeTransactions,
+  markRootSpanForDiscard,
+} from './onSpanEndUtils';
 import { SPAN_ORIGIN_AUTO_NAVIGATION_REACT_NAVIGATION } from './origin';
 import { getReactNativeTracingIntegration } from './reactnativetracing';
 import { SEMANTIC_ATTRIBUTE_NAVIGATION_ACTION_TYPE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from './semanticAttributes';
@@ -535,7 +539,7 @@ export const reactNavigationIntegration = ({
   const _discardLatestTransaction = (): void => {
     if (latestNavigationSpan) {
       if (isSentrySpan(latestNavigationSpan)) {
-        latestNavigationSpan['_sampled'] = false;
+        markRootSpanForDiscard(latestNavigationSpan, 'discarded_latest_navigation');
       }
       // TODO: What if it's not SentrySpan?
       latestNavigationSpan.end();

packages/core/test/tracing/integrations/appStart.test.ts

@@ -491,10 +491,11 @@ describe('App Start Integration', () => {
       });
 
       // Simulate the span being dropped (e.g., ignoreEmptyRouteChangeTransactions
-      // sets _sampled = false during spanEnd processing).
-      // Note: _sampled is a @sentry/core SentrySpan internal — this couples to the
-      // same mechanism used by onSpanEndUtils.ts (discardEmptyNavigationSpan).
-      (firstSpan as any)['_sampled'] = false;
+      // marking the root span via the `sentry.rn.discard_reason` attribute during
+      // spanEnd processing). The tracing integration's event processor drops the
+      // resulting transaction; appStart detects the marker on the next spanStart
+      // and resets the lock so the next root span can attach app start data.
+      firstSpan.setAttribute('sentry.rn.discard_reason', 'no_route_info');
 
       // Second root span starts — recordFirstStartedActiveRootSpanId detects
       // the previously locked span is no longer sampled and resets the lock
@@ -524,6 +525,61 @@ describe('App Start Integration', () => {
       expect((actualEvent as TransactionEvent)?.measurements?.[APP_START_COLD_MEASUREMENT]).toBeDefined();
     });
 
+    it('Skips app start attachment for discarded transactions and preserves it for the next one', async () => {
+      mockAppStart({ cold: true });
+
+      const integration = appStartIntegration();
+      const client = new TestClient({
+        ...getDefaultTestClientOptions(),
+        enableAppStartTracking: true,
+        tracesSampleRate: 1.0,
+      });
+      setCurrentClient(client);
+      integration.setup(client);
+      integration.afterAllSetup(client);
+
+      // First root span — gets marked for discard before its transaction event
+      // reaches the app start integration's processEvent.
+      const firstSpan = startInactiveSpan({ name: 'First Navigation', forceTransaction: true });
+      const firstSpanId = firstSpan.spanContext().spanId;
+      firstSpan.setAttribute('sentry.rn.discard_reason', 'no_route_info');
+
+      // Simulate `appStartIntegration` (registered before `reactNativeTracingIntegration`)
+      // running its processEvent on the discarded transaction. It must not
+      // attach app start data nor flip `appStartDataFlushed = true`, otherwise
+      // the next real transaction would lose app start.
+      const discardedEvent = getMinimalTransactionEvent();
+      discardedEvent.contexts!.trace!.span_id = firstSpanId;
+      discardedEvent.contexts!.trace!.data = { 'sentry.rn.discard_reason': 'no_route_info' };
+
+      const processedDiscarded = await processEventWithIntegration(integration, discardedEvent);
+
+      // Event passes through unchanged — no app start span attached.
+      const discardedColdStartSpan = (processedDiscarded as TransactionEvent)?.spans?.find(
+        ({ description }) => description === 'Cold Start',
+      );
+      expect(discardedColdStartSpan).toBeUndefined();
+      expect((processedDiscarded as TransactionEvent)?.measurements?.[APP_START_COLD_MEASUREMENT]).toBeUndefined();
+
+      // Next real root span starts — its discard marker on the previous span
+      // resets the lock and the new span gets locked.
+      const secondSpan = startInactiveSpan({ name: 'Second Navigation', forceTransaction: true });
+      const secondSpanId = secondSpan.spanContext().spanId;
+
+      const realEvent = getMinimalTransactionEvent();
+      realEvent.contexts!.trace!.span_id = secondSpanId;
+
+      const actualEvent = await processEventWithIntegration(integration, realEvent);
+
+      // App start data is still available because the discarded event did not
+      // flip `appStartDataFlushed`.
+      const appStartSpan = (actualEvent as TransactionEvent)?.spans?.find(
+        ({ description }) => description === 'Cold Start',
+      );
+      expect(appStartSpan).toBeDefined();
+      expect((actualEvent as TransactionEvent)?.measurements?.[APP_START_COLD_MEASUREMENT]).toBeDefined();
+    });
+
     it('Does not lock firstStartedActiveRootSpanId to unsampled root span', async () => {
       mockAppStart({ cold: true });