From d7bd763b3332c551a139da69e6557ff6ff013265 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 5 Sep 2022 16:42:57 +0200 Subject: [PATCH 1/6] Add transaction source for dynamic sampling --- src/js/tracing/reactnativenavigation.ts | 55 +++++++++++++--------- src/js/tracing/utils.ts | 7 ++- test/tracing/reactnativenavigation.test.ts | 4 ++ test/tracing/reactnavigation.test.ts | 5 ++ test/tracing/reactnavigationv4.test.ts | 3 ++ 5 files changed, 52 insertions(+), 22 deletions(-) diff --git a/src/js/tracing/reactnativenavigation.ts b/src/js/tracing/reactnativenavigation.ts index 8d6be1b594..4f538ca2be 100644 --- a/src/js/tracing/reactnativenavigation.ts +++ b/src/js/tracing/reactnativenavigation.ts @@ -1,4 +1,4 @@ -import { Transaction as TransactionType } from '@sentry/types'; +import { Transaction as TransactionType, TransactionContext } from '@sentry/types'; import { logger } from '@sentry/utils'; import { EmitterSubscription } from 'react-native'; @@ -8,7 +8,7 @@ import { TransactionCreator, } from './routingInstrumentation'; import { BeforeNavigate, RouteChangeContextData } from './types'; -import { getBlankTransactionContext } from './utils'; +import { defaultTransactionSource, getBlankTransactionContext } from './utils'; interface ReactNativeNavigationOptions { routeChangeTimeoutMs: number; @@ -142,6 +142,7 @@ export class ReactNativeNavigationInstrumentation extends InternalRoutingInstrum this._clearStateChangeTimeout(); const originalContext = this._latestTransaction.toContext(); + const originalMetadataSource = this._latestTransaction.metadata.source; const routeHasBeenSeen = this._recentComponentIds.includes( event.componentId ); @@ -171,29 +172,16 @@ export class ReactNativeNavigationInstrumentation extends InternalRoutingInstrum data, }; - let finalContext = this._beforeNavigate?.(updatedContext); - - // This block is to catch users not returning a transaction context - if (!finalContext) { - logger.error( - `[${ReactNativeNavigationInstrumentation.name}] beforeNavigate returned ${finalContext}, return context.sampled = false to not send transaction.` - ); - - finalContext = { - ...updatedContext, - sampled: false, - }; - } + const finalContext = this._prepareFinalContext(updatedContext); + this._latestTransaction.updateWithContext(finalContext); - if (finalContext.sampled === false) { - logger.log( - `[${ReactNativeNavigationInstrumentation.name}] Will not send transaction "${finalContext.name}" due to beforeNavigate.` - ); + const notCustomName = updatedContext.name === finalContext.name; + if (notCustomName) { + const newSource = originalMetadataSource || defaultTransactionSource; + this._latestTransaction.setName(finalContext.name, newSource); } - this._latestTransaction.updateWithContext(finalContext); this._onConfirmRoute?.(finalContext); - this._prevComponentEvent = event; } else { this._discardLatestTransaction(); @@ -203,6 +191,31 @@ export class ReactNativeNavigationInstrumentation extends InternalRoutingInstrum } } + /** Creates final transaction context before confirmation */ + private _prepareFinalContext(updatedContext: TransactionContext): TransactionContext { + let finalContext = this._beforeNavigate?.(updatedContext); + + // This block is to catch users not returning a transaction context + if (!finalContext) { + logger.error( + `[${ReactNativeNavigationInstrumentation.name}] beforeNavigate returned ${finalContext}, return context.sampled = false to not send transaction.` + ); + + finalContext = { + ...updatedContext, + sampled: false, + }; + } + + if (finalContext.sampled === false) { + logger.log( + `[${ReactNativeNavigationInstrumentation.name}] Will not send transaction "${finalContext.name}" due to beforeNavigate.` + ); + } + + return finalContext; + } + /** Cancels the latest transaction so it does not get sent to Sentry. */ private _discardLatestTransaction(): void { if (this._latestTransaction) { diff --git a/src/js/tracing/utils.ts b/src/js/tracing/utils.ts index ef933dc28c..963362acef 100644 --- a/src/js/tracing/utils.ts +++ b/src/js/tracing/utils.ts @@ -1,7 +1,9 @@ import { IdleTransaction, Span, Transaction } from '@sentry/tracing'; -import { TransactionContext } from '@sentry/types'; +import { TransactionContext, TransactionSource } from '@sentry/types'; import { timestampInSeconds } from '@sentry/utils'; +export const defaultTransactionSource: TransactionSource = 'view'; + export const getBlankTransactionContext = ( name: string ): TransactionContext => { @@ -12,6 +14,9 @@ export const getBlankTransactionContext = ( 'routing.instrumentation': name, }, data: {}, + metadata: { + source: defaultTransactionSource, + }, }; }; diff --git a/test/tracing/reactnativenavigation.test.ts b/test/tracing/reactnativenavigation.test.ts index e2a126c031..1a8a57330e 100644 --- a/test/tracing/reactnativenavigation.test.ts +++ b/test/tracing/reactnativenavigation.test.ts @@ -92,6 +92,7 @@ describe('React Native Navigation Instrumentation', () => { }, previousRoute: null, }); + expect(mockTransaction.metadata.source).toBe('view'); }); test('Transaction context is changed with beforeNavigate', () => { @@ -144,6 +145,7 @@ describe('React Native Navigation Instrumentation', () => { }, previousRoute: null, }); + expect(mockTransaction.metadata.source).toBe('view'); }); test('Transaction not sent on a cancelled route change', () => { @@ -258,6 +260,8 @@ describe('React Native Navigation Instrumentation', () => { expect(confirmedContext).toBeDefined(); if (confirmedContext) { expect(confirmedContext.name).toBe(mockEvent2.componentName); + expect(confirmedContext.metadata).toBeDefined(); + expect(confirmedContext.metadata?.source).toBe('view'); expect(confirmedContext.data).toBeDefined(); if (confirmedContext.data) { expect(confirmedContext.data.route.name).toBe( diff --git a/test/tracing/reactnavigation.test.ts b/test/tracing/reactnavigation.test.ts index 3d4b78b12a..19330ebf82 100644 --- a/test/tracing/reactnavigation.test.ts +++ b/test/tracing/reactnavigation.test.ts @@ -80,6 +80,7 @@ describe('ReactNavigationInstrumentation', () => { }, previousRoute: null, }); + expect(mockTransaction.metadata.source).toBe('view'); }); test('transaction sent on navigation', async () => { @@ -140,6 +141,7 @@ describe('ReactNavigationInstrumentation', () => { params: {}, }, }); + expect(mockTransaction.metadata.source).toBe('view'); resolve(); }, 50); @@ -192,6 +194,7 @@ describe('ReactNavigationInstrumentation', () => { expect(mockTransaction.sampled).toBe(false); expect(mockTransaction.name).toBe('New Name'); expect(mockTransaction.description).toBe('Description'); + expect(mockTransaction.metadata.source).toBe('view'); resolve(); }, 50); }); @@ -488,6 +491,8 @@ describe('ReactNavigationInstrumentation', () => { expect(confirmedContext).toBeDefined(); if (confirmedContext) { expect(confirmedContext.name).toBe(route2.name); + expect(confirmedContext.metadata).toBeDefined(); + expect(confirmedContext.metadata?.source).toBe('view'); expect(confirmedContext.data).toBeDefined(); if (confirmedContext.data) { expect(confirmedContext.data.route.name).toBe(route2.name); diff --git a/test/tracing/reactnavigationv4.test.ts b/test/tracing/reactnavigationv4.test.ts index 354442598b..47c8931bdd 100644 --- a/test/tracing/reactnavigationv4.test.ts +++ b/test/tracing/reactnavigationv4.test.ts @@ -139,6 +139,7 @@ describe('ReactNavigationV4Instrumentation', () => { previousRoute: null, }); expect(mockTransaction.sampled).toBe(true); + expect(mockTransaction.metadata.source).toBe('view'); }); test('transaction sent on navigation', () => { @@ -199,6 +200,7 @@ describe('ReactNavigationV4Instrumentation', () => { }); expect(mockTransaction.sampled).toBe(true); + expect(mockTransaction.metadata.source).toBe('view'); }); test('transaction context changed with beforeNavigate', () => { @@ -262,6 +264,7 @@ describe('ReactNavigationV4Instrumentation', () => { }); expect(mockTransaction.sampled).toBe(false); + expect(mockTransaction.metadata.source).toBe('view'); }); test('transaction not attached on a cancelled navigation', () => { From 78f9f515b611651afefeac49be934346537dcdfc Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 5 Sep 2022 21:39:16 +0200 Subject: [PATCH 2/6] Add source to react navigation, fix tests --- src/js/tracing/reactnavigation.ts | 62 +++++++++++++--------- src/js/tracing/reactnavigationv4.ts | 55 ++++++++++++------- test/tracing/reactnativenavigation.test.ts | 3 +- test/tracing/reactnavigation.test.ts | 3 +- test/tracing/reactnavigationv4.test.ts | 1 + 5 files changed, 77 insertions(+), 47 deletions(-) diff --git a/src/js/tracing/reactnavigation.ts b/src/js/tracing/reactnavigation.ts index 816776450a..9667dc7a5b 100644 --- a/src/js/tracing/reactnavigation.ts +++ b/src/js/tracing/reactnavigation.ts @@ -1,5 +1,5 @@ /* eslint-disable max-lines */ -import { Transaction as TransactionType } from '@sentry/types'; +import { Transaction as TransactionType, TransactionContext } from '@sentry/types'; import { getGlobalObject, logger } from '@sentry/utils'; import { @@ -12,7 +12,7 @@ import { ReactNavigationTransactionContext, RouteChangeContextData, } from './types'; -import { getBlankTransactionContext } from './utils'; +import { defaultTransactionSource, getBlankTransactionContext } from './utils'; export interface NavigationRoute { name: string; @@ -199,6 +199,7 @@ export class ReactNavigationInstrumentation extends InternalRoutingInstrumentati if (this._latestTransaction) { if (!previousRoute || previousRoute.key !== route.key) { const originalContext = this._latestTransaction.toContext() as typeof BLANK_TRANSACTION_CONTEXT; + const originalMetadataSource = this._latestTransaction.metadata.source; const routeHasBeenSeen = this._recentRouteKeys.includes(route.key); const data: RouteChangeContextData = { @@ -228,31 +229,15 @@ export class ReactNavigationInstrumentation extends InternalRoutingInstrumentati data, }; - let finalContext = this._beforeNavigate?.(updatedContext); - - // This block is to catch users not returning a transaction context - if (!finalContext) { - logger.error( - `[ReactNavigationInstrumentation] beforeNavigate returned ${finalContext}, return context.sampled = false to not send transaction.` - ); + const finalContext = this._prepareFinalContext(updatedContext); + this._latestTransaction.updateWithContext(finalContext); - finalContext = { - ...updatedContext, - sampled: false, - }; + const notCustomName = updatedContext.name === finalContext.name; + if (notCustomName) { + const newSource = originalMetadataSource || defaultTransactionSource; + this._latestTransaction.setName(finalContext.name, newSource); } - // Note: finalContext.sampled will be false at this point only if the user sets it to be so in beforeNavigate. - if (finalContext.sampled === false) { - logger.log( - `[ReactNavigationInstrumentation] Will not send transaction "${finalContext.name}" due to beforeNavigate.` - ); - } else { - // Clear the timeout so the transaction does not get cancelled. - this._clearStateChangeTimeout(); - } - - this._latestTransaction.updateWithContext(finalContext); this._onConfirmRoute?.(finalContext); } @@ -265,6 +250,35 @@ export class ReactNavigationInstrumentation extends InternalRoutingInstrumentati this._latestTransaction = undefined; } + /** Creates final transaction context before confirmation */ + private _prepareFinalContext(updatedContext: TransactionContext): TransactionContext { + let finalContext = this._beforeNavigate?.(updatedContext); + + // This block is to catch users not returning a transaction context + if (!finalContext) { + logger.error( + `[ReactNavigationInstrumentation] beforeNavigate returned ${finalContext}, return context.sampled = false to not send transaction.` + ); + + finalContext = { + ...updatedContext, + sampled: false, + }; + } + + // Note: finalContext.sampled will be false at this point only if the user sets it to be so in beforeNavigate. + if (finalContext.sampled === false) { + logger.log( + `[ReactNavigationInstrumentation] Will not send transaction "${finalContext.name}" due to beforeNavigate.` + ); + } else { + // Clear the timeout so the transaction does not get cancelled. + this._clearStateChangeTimeout(); + } + + return finalContext; + } + /** Pushes a recent route key, and removes earlier routes when there is greater than the max length */ private _pushRecentRouteKey = (key: string): void => { this._recentRouteKeys.push(key); diff --git a/src/js/tracing/reactnavigationv4.ts b/src/js/tracing/reactnavigationv4.ts index 755ae590a5..10f7048244 100644 --- a/src/js/tracing/reactnavigationv4.ts +++ b/src/js/tracing/reactnavigationv4.ts @@ -1,5 +1,5 @@ /* eslint-disable max-lines */ -import { Transaction } from '@sentry/types'; +import { Transaction, TransactionContext } from '@sentry/types'; import { getGlobalObject, logger } from '@sentry/utils'; import { @@ -12,6 +12,7 @@ import { ReactNavigationTransactionContext, RouteChangeContextData, } from './types'; +import { defaultTransactionSource } from './utils'; export interface NavigationRouteV4 { routeName: string; @@ -221,6 +222,7 @@ class ReactNavigationV4Instrumentation extends InternalRoutingInstrumentation { currentRoute, this._prevRoute ); + const originalMetadataSource = originalContext.metadata?.source; let mergedContext = originalContext; if (updateLatestTransaction && this._latestTransaction) { @@ -230,27 +232,16 @@ class ReactNavigationV4Instrumentation extends InternalRoutingInstrumentation { }; } - let finalContext = this._beforeNavigate?.(mergedContext); - - // This block is to catch users not returning a transaction context - if (!finalContext) { - logger.error( - `[ReactNavigationV4Instrumentation] beforeNavigate returned ${finalContext}, return context.sampled = false to not send transaction.` - ); - - finalContext = { - ...mergedContext, - sampled: false, - }; - } - - if (finalContext.sampled === false) { - this._onBeforeNavigateNotSampled(finalContext.name); - } + const finalContext = this._prepareFinalContext(mergedContext); if (updateLatestTransaction && this._latestTransaction) { // Update the latest transaction instead of calling onRouteWillChange this._latestTransaction.updateWithContext(finalContext); + const notCustomName = mergedContext.name === finalContext.name; + if (notCustomName) { + const newSource = originalMetadataSource || defaultTransactionSource; + this._latestTransaction.setName(finalContext.name, newSource); + } } else { this._latestTransaction = this.onRouteWillChange(finalContext); } @@ -262,6 +253,29 @@ class ReactNavigationV4Instrumentation extends InternalRoutingInstrumentation { } } + /** Creates final transaction context before confirmation */ + private _prepareFinalContext(mergedContext: TransactionContext): TransactionContext { + let finalContext = this._beforeNavigate?.(mergedContext); + + // This block is to catch users not returning a transaction context + if (!finalContext) { + logger.error( + `[ReactNavigationV4Instrumentation] beforeNavigate returned ${finalContext}, return context.sampled = false to not send transaction.` + ); + + finalContext = { + ...mergedContext, + sampled: false, + }; + } + + if (finalContext.sampled === false) { + this._onBeforeNavigateNotSampled(finalContext.name); + } + + return finalContext; + } + /** * Gets the transaction context for a `NavigationRouteV4` */ @@ -345,7 +359,7 @@ class ReactNavigationV4Instrumentation extends InternalRoutingInstrumentation { } } -const INITIAL_TRANSACTION_CONTEXT_V4 = { +const INITIAL_TRANSACTION_CONTEXT_V4: TransactionContext = { name: 'App Launch', op: 'navigation', tags: { @@ -353,6 +367,9 @@ const INITIAL_TRANSACTION_CONTEXT_V4 = { ReactNavigationV4Instrumentation.instrumentationName, }, data: {}, + metadata: { + source: 'view', + }, }; export { ReactNavigationV4Instrumentation, INITIAL_TRANSACTION_CONTEXT_V4 }; diff --git a/test/tracing/reactnativenavigation.test.ts b/test/tracing/reactnativenavigation.test.ts index 1a8a57330e..58b7eaf655 100644 --- a/test/tracing/reactnativenavigation.test.ts +++ b/test/tracing/reactnativenavigation.test.ts @@ -260,8 +260,7 @@ describe('React Native Navigation Instrumentation', () => { expect(confirmedContext).toBeDefined(); if (confirmedContext) { expect(confirmedContext.name).toBe(mockEvent2.componentName); - expect(confirmedContext.metadata).toBeDefined(); - expect(confirmedContext.metadata?.source).toBe('view'); + expect(confirmedContext.metadata).toBeUndefined(); expect(confirmedContext.data).toBeDefined(); if (confirmedContext.data) { expect(confirmedContext.data.route.name).toBe( diff --git a/test/tracing/reactnavigation.test.ts b/test/tracing/reactnavigation.test.ts index 19330ebf82..be2bae7ee3 100644 --- a/test/tracing/reactnavigation.test.ts +++ b/test/tracing/reactnavigation.test.ts @@ -491,8 +491,7 @@ describe('ReactNavigationInstrumentation', () => { expect(confirmedContext).toBeDefined(); if (confirmedContext) { expect(confirmedContext.name).toBe(route2.name); - expect(confirmedContext.metadata).toBeDefined(); - expect(confirmedContext.metadata?.source).toBe('view'); + expect(confirmedContext.metadata).toBeUndefined(); expect(confirmedContext.data).toBeDefined(); if (confirmedContext.data) { expect(confirmedContext.data.route.name).toBe(route2.name); diff --git a/test/tracing/reactnavigationv4.test.ts b/test/tracing/reactnavigationv4.test.ts index 47c8931bdd..da4fbfb50e 100644 --- a/test/tracing/reactnavigationv4.test.ts +++ b/test/tracing/reactnavigationv4.test.ts @@ -501,6 +501,7 @@ describe('ReactNavigationV4Instrumentation', () => { if (confirmedContext) { expect(confirmedContext.name).toBe(route2.routeName); expect(confirmedContext.data).toBeDefined(); + expect(confirmedContext.metadata).toBeUndefined(); if (confirmedContext.data) { expect(confirmedContext.data.route.name).toBe(route2.routeName); expect(confirmedContext.data.previousRoute).toBeDefined(); From f9f2468d72b6af03f46c15593f3214027df11075 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 6 Sep 2022 10:18:03 +0200 Subject: [PATCH 3/6] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 597ca2c786..fd6af97dda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Bump JavaScript SDK from v7.9.0 to v7.12.1 ([#2451](https://github.com/getsentry/sentry-react-native/pull/2451)) - [changelog](https://github.com/getsentry/sentry-javascript/blob/master/CHANGELOG.md#7121) - [diff](https://github.com/getsentry/sentry-javascript/compare/7.9.0...7.12.1) +- Add Transaction Source for Dynamic Sampling Context ([#2454](https://github.com/getsentry/sentry-react-native/pull/2454)) ## 4.2.4 From f3a8a81e5abd44c286ead0247fbca6433f3f2ad2 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 6 Sep 2022 10:18:43 +0200 Subject: [PATCH 4/6] Fix beforeNavigation name change results in custom source --- src/js/tracing/reactnativenavigation.ts | 13 ++++++------- src/js/tracing/reactnavigation.ts | 13 ++++++------- src/js/tracing/reactnavigationv4.ts | 13 ++++++------- test/tracing/reactnativenavigation.test.ts | 2 +- test/tracing/reactnavigation.test.ts | 2 +- test/tracing/reactnavigationv4.test.ts | 2 +- 6 files changed, 21 insertions(+), 24 deletions(-) diff --git a/src/js/tracing/reactnativenavigation.ts b/src/js/tracing/reactnativenavigation.ts index 4f538ca2be..127095b385 100644 --- a/src/js/tracing/reactnativenavigation.ts +++ b/src/js/tracing/reactnativenavigation.ts @@ -142,7 +142,6 @@ export class ReactNativeNavigationInstrumentation extends InternalRoutingInstrum this._clearStateChangeTimeout(); const originalContext = this._latestTransaction.toContext(); - const originalMetadataSource = this._latestTransaction.metadata.source; const routeHasBeenSeen = this._recentComponentIds.includes( event.componentId ); @@ -175,11 +174,11 @@ export class ReactNativeNavigationInstrumentation extends InternalRoutingInstrum const finalContext = this._prepareFinalContext(updatedContext); this._latestTransaction.updateWithContext(finalContext); - const notCustomName = updatedContext.name === finalContext.name; - if (notCustomName) { - const newSource = originalMetadataSource || defaultTransactionSource; - this._latestTransaction.setName(finalContext.name, newSource); - } + const isCustomName = updatedContext.name !== finalContext.name; + this._latestTransaction.setName( + finalContext.name, + isCustomName ? 'custom' : defaultTransactionSource, + ); this._onConfirmRoute?.(finalContext); this._prevComponentEvent = event; @@ -193,7 +192,7 @@ export class ReactNativeNavigationInstrumentation extends InternalRoutingInstrum /** Creates final transaction context before confirmation */ private _prepareFinalContext(updatedContext: TransactionContext): TransactionContext { - let finalContext = this._beforeNavigate?.(updatedContext); + let finalContext = this._beforeNavigate?.({ ...updatedContext }); // This block is to catch users not returning a transaction context if (!finalContext) { diff --git a/src/js/tracing/reactnavigation.ts b/src/js/tracing/reactnavigation.ts index 9667dc7a5b..7c6eba998e 100644 --- a/src/js/tracing/reactnavigation.ts +++ b/src/js/tracing/reactnavigation.ts @@ -199,7 +199,6 @@ export class ReactNavigationInstrumentation extends InternalRoutingInstrumentati if (this._latestTransaction) { if (!previousRoute || previousRoute.key !== route.key) { const originalContext = this._latestTransaction.toContext() as typeof BLANK_TRANSACTION_CONTEXT; - const originalMetadataSource = this._latestTransaction.metadata.source; const routeHasBeenSeen = this._recentRouteKeys.includes(route.key); const data: RouteChangeContextData = { @@ -232,11 +231,11 @@ export class ReactNavigationInstrumentation extends InternalRoutingInstrumentati const finalContext = this._prepareFinalContext(updatedContext); this._latestTransaction.updateWithContext(finalContext); - const notCustomName = updatedContext.name === finalContext.name; - if (notCustomName) { - const newSource = originalMetadataSource || defaultTransactionSource; - this._latestTransaction.setName(finalContext.name, newSource); - } + const isCustomName = updatedContext.name !== finalContext.name; + this._latestTransaction.setName( + finalContext.name, + isCustomName ? 'custom' : defaultTransactionSource, + ); this._onConfirmRoute?.(finalContext); } @@ -252,7 +251,7 @@ export class ReactNavigationInstrumentation extends InternalRoutingInstrumentati /** Creates final transaction context before confirmation */ private _prepareFinalContext(updatedContext: TransactionContext): TransactionContext { - let finalContext = this._beforeNavigate?.(updatedContext); + let finalContext = this._beforeNavigate?.({ ...updatedContext }); // This block is to catch users not returning a transaction context if (!finalContext) { diff --git a/src/js/tracing/reactnavigationv4.ts b/src/js/tracing/reactnavigationv4.ts index 10f7048244..200d9a49e4 100644 --- a/src/js/tracing/reactnavigationv4.ts +++ b/src/js/tracing/reactnavigationv4.ts @@ -222,7 +222,6 @@ class ReactNavigationV4Instrumentation extends InternalRoutingInstrumentation { currentRoute, this._prevRoute ); - const originalMetadataSource = originalContext.metadata?.source; let mergedContext = originalContext; if (updateLatestTransaction && this._latestTransaction) { @@ -237,11 +236,11 @@ class ReactNavigationV4Instrumentation extends InternalRoutingInstrumentation { if (updateLatestTransaction && this._latestTransaction) { // Update the latest transaction instead of calling onRouteWillChange this._latestTransaction.updateWithContext(finalContext); - const notCustomName = mergedContext.name === finalContext.name; - if (notCustomName) { - const newSource = originalMetadataSource || defaultTransactionSource; - this._latestTransaction.setName(finalContext.name, newSource); - } + const isCustomName = mergedContext.name !== finalContext.name; + this._latestTransaction.setName( + finalContext.name, + isCustomName ? 'custom' : defaultTransactionSource, + ); } else { this._latestTransaction = this.onRouteWillChange(finalContext); } @@ -255,7 +254,7 @@ class ReactNavigationV4Instrumentation extends InternalRoutingInstrumentation { /** Creates final transaction context before confirmation */ private _prepareFinalContext(mergedContext: TransactionContext): TransactionContext { - let finalContext = this._beforeNavigate?.(mergedContext); + let finalContext = this._beforeNavigate?.({ ...mergedContext }); // This block is to catch users not returning a transaction context if (!finalContext) { diff --git a/test/tracing/reactnativenavigation.test.ts b/test/tracing/reactnativenavigation.test.ts index 58b7eaf655..9670864ae7 100644 --- a/test/tracing/reactnativenavigation.test.ts +++ b/test/tracing/reactnativenavigation.test.ts @@ -145,7 +145,7 @@ describe('React Native Navigation Instrumentation', () => { }, previousRoute: null, }); - expect(mockTransaction.metadata.source).toBe('view'); + expect(mockTransaction.metadata.source).toBe('custom'); }); test('Transaction not sent on a cancelled route change', () => { diff --git a/test/tracing/reactnavigation.test.ts b/test/tracing/reactnavigation.test.ts index be2bae7ee3..60592e15fb 100644 --- a/test/tracing/reactnavigation.test.ts +++ b/test/tracing/reactnavigation.test.ts @@ -194,7 +194,7 @@ describe('ReactNavigationInstrumentation', () => { expect(mockTransaction.sampled).toBe(false); expect(mockTransaction.name).toBe('New Name'); expect(mockTransaction.description).toBe('Description'); - expect(mockTransaction.metadata.source).toBe('view'); + expect(mockTransaction.metadata.source).toBe('custom'); resolve(); }, 50); }); diff --git a/test/tracing/reactnavigationv4.test.ts b/test/tracing/reactnavigationv4.test.ts index da4fbfb50e..d145c249eb 100644 --- a/test/tracing/reactnavigationv4.test.ts +++ b/test/tracing/reactnavigationv4.test.ts @@ -264,7 +264,7 @@ describe('ReactNavigationV4Instrumentation', () => { }); expect(mockTransaction.sampled).toBe(false); - expect(mockTransaction.metadata.source).toBe('view'); + expect(mockTransaction.metadata.source).toBe('custom'); }); test('transaction not attached on a cancelled navigation', () => { From d405343baffa82a5e2bae18deb637e173836e980 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 6 Sep 2022 10:56:55 +0200 Subject: [PATCH 5/6] Change to component source, add custom const --- src/js/tracing/reactnativenavigation.ts | 4 ++-- src/js/tracing/reactnavigation.ts | 4 ++-- src/js/tracing/reactnavigationv4.ts | 4 ++-- src/js/tracing/utils.ts | 3 ++- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/js/tracing/reactnativenavigation.ts b/src/js/tracing/reactnativenavigation.ts index 127095b385..d522eb7ad9 100644 --- a/src/js/tracing/reactnativenavigation.ts +++ b/src/js/tracing/reactnativenavigation.ts @@ -8,7 +8,7 @@ import { TransactionCreator, } from './routingInstrumentation'; import { BeforeNavigate, RouteChangeContextData } from './types'; -import { defaultTransactionSource, getBlankTransactionContext } from './utils'; +import { customTransactionSource, defaultTransactionSource, getBlankTransactionContext } from './utils'; interface ReactNativeNavigationOptions { routeChangeTimeoutMs: number; @@ -177,7 +177,7 @@ export class ReactNativeNavigationInstrumentation extends InternalRoutingInstrum const isCustomName = updatedContext.name !== finalContext.name; this._latestTransaction.setName( finalContext.name, - isCustomName ? 'custom' : defaultTransactionSource, + isCustomName ? customTransactionSource : defaultTransactionSource, ); this._onConfirmRoute?.(finalContext); diff --git a/src/js/tracing/reactnavigation.ts b/src/js/tracing/reactnavigation.ts index 7c6eba998e..3dcae3a42f 100644 --- a/src/js/tracing/reactnavigation.ts +++ b/src/js/tracing/reactnavigation.ts @@ -12,7 +12,7 @@ import { ReactNavigationTransactionContext, RouteChangeContextData, } from './types'; -import { defaultTransactionSource, getBlankTransactionContext } from './utils'; +import { customTransactionSource, defaultTransactionSource, getBlankTransactionContext } from './utils'; export interface NavigationRoute { name: string; @@ -234,7 +234,7 @@ export class ReactNavigationInstrumentation extends InternalRoutingInstrumentati const isCustomName = updatedContext.name !== finalContext.name; this._latestTransaction.setName( finalContext.name, - isCustomName ? 'custom' : defaultTransactionSource, + isCustomName ? customTransactionSource : defaultTransactionSource, ); this._onConfirmRoute?.(finalContext); diff --git a/src/js/tracing/reactnavigationv4.ts b/src/js/tracing/reactnavigationv4.ts index 200d9a49e4..deae17c41f 100644 --- a/src/js/tracing/reactnavigationv4.ts +++ b/src/js/tracing/reactnavigationv4.ts @@ -12,7 +12,7 @@ import { ReactNavigationTransactionContext, RouteChangeContextData, } from './types'; -import { defaultTransactionSource } from './utils'; +import { customTransactionSource, defaultTransactionSource } from './utils'; export interface NavigationRouteV4 { routeName: string; @@ -239,7 +239,7 @@ class ReactNavigationV4Instrumentation extends InternalRoutingInstrumentation { const isCustomName = mergedContext.name !== finalContext.name; this._latestTransaction.setName( finalContext.name, - isCustomName ? 'custom' : defaultTransactionSource, + isCustomName ? customTransactionSource : defaultTransactionSource, ); } else { this._latestTransaction = this.onRouteWillChange(finalContext); diff --git a/src/js/tracing/utils.ts b/src/js/tracing/utils.ts index 963362acef..78bac70663 100644 --- a/src/js/tracing/utils.ts +++ b/src/js/tracing/utils.ts @@ -2,7 +2,8 @@ import { IdleTransaction, Span, Transaction } from '@sentry/tracing'; import { TransactionContext, TransactionSource } from '@sentry/types'; import { timestampInSeconds } from '@sentry/utils'; -export const defaultTransactionSource: TransactionSource = 'view'; +export const defaultTransactionSource: TransactionSource = 'component'; +export const customTransactionSource: TransactionSource = 'custom'; export const getBlankTransactionContext = ( name: string From c4feaaaafebc2591693bf84e80d84307c5ff263c Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 6 Sep 2022 11:11:17 +0200 Subject: [PATCH 6/6] Fix tests update to component --- test/tracing/reactnativenavigation.test.ts | 2 +- test/tracing/reactnavigation.test.ts | 4 ++-- test/tracing/reactnavigationv4.test.ts | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/tracing/reactnativenavigation.test.ts b/test/tracing/reactnativenavigation.test.ts index 9670864ae7..93c79e02e4 100644 --- a/test/tracing/reactnativenavigation.test.ts +++ b/test/tracing/reactnativenavigation.test.ts @@ -92,7 +92,7 @@ describe('React Native Navigation Instrumentation', () => { }, previousRoute: null, }); - expect(mockTransaction.metadata.source).toBe('view'); + expect(mockTransaction.metadata.source).toBe('component'); }); test('Transaction context is changed with beforeNavigate', () => { diff --git a/test/tracing/reactnavigation.test.ts b/test/tracing/reactnavigation.test.ts index 60592e15fb..66d3275888 100644 --- a/test/tracing/reactnavigation.test.ts +++ b/test/tracing/reactnavigation.test.ts @@ -80,7 +80,7 @@ describe('ReactNavigationInstrumentation', () => { }, previousRoute: null, }); - expect(mockTransaction.metadata.source).toBe('view'); + expect(mockTransaction.metadata.source).toBe('component'); }); test('transaction sent on navigation', async () => { @@ -141,7 +141,7 @@ describe('ReactNavigationInstrumentation', () => { params: {}, }, }); - expect(mockTransaction.metadata.source).toBe('view'); + expect(mockTransaction.metadata.source).toBe('component'); resolve(); }, 50); diff --git a/test/tracing/reactnavigationv4.test.ts b/test/tracing/reactnavigationv4.test.ts index d145c249eb..9aaa2095f3 100644 --- a/test/tracing/reactnavigationv4.test.ts +++ b/test/tracing/reactnavigationv4.test.ts @@ -139,7 +139,7 @@ describe('ReactNavigationV4Instrumentation', () => { previousRoute: null, }); expect(mockTransaction.sampled).toBe(true); - expect(mockTransaction.metadata.source).toBe('view'); + expect(mockTransaction.metadata.source).toBe('component'); }); test('transaction sent on navigation', () => { @@ -200,7 +200,7 @@ describe('ReactNavigationV4Instrumentation', () => { }); expect(mockTransaction.sampled).toBe(true); - expect(mockTransaction.metadata.source).toBe('view'); + expect(mockTransaction.metadata.source).toBe('component'); }); test('transaction context changed with beforeNavigate', () => {