From 8369f43ab6f7e8a687f584234538f5046acb039c Mon Sep 17 00:00:00 2001 From: Alberto Leal Date: Thu, 14 May 2020 07:09:31 -0400 Subject: [PATCH 1/6] fix(apm): Set the transaction name for JavaScript transactions before they are flushed Set the transaction name for JavaScript transactions based on latest window.location object just before the transaction is flushed. The window.location object is translated based on the app react-router routes using the react-router utility functions. --- src/sentry/static/sentry/app/bootstrap.tsx | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/sentry/static/sentry/app/bootstrap.tsx b/src/sentry/static/sentry/app/bootstrap.tsx index c987adf99abb95..200e0a9ea7116a 100644 --- a/src/sentry/static/sentry/app/bootstrap.tsx +++ b/src/sentry/static/sentry/app/bootstrap.tsx @@ -26,6 +26,8 @@ import ConfigStore from 'app/stores/configStore'; import Main from 'app/main'; import ajaxCsrfSetup from 'app/utils/ajaxCsrfSetup'; import plugins from 'app/plugins'; +import routes from 'app/routes'; +import getRouteStringFromRoutes from 'app/utils/getRouteStringFromRoutes'; function getSentryIntegrations(hasReplays: boolean = false) { const integrations = [ @@ -74,11 +76,39 @@ const tracesSampleRate = config ? config.apmSampling : 0; const hasReplays = window.__SENTRY__USER && window.__SENTRY__USER.isStaff && !!process.env.DISABLE_RR_WEB; +const appRoutes = Router.createRoutes(routes()); + Sentry.init({ ...window.__SENTRY__OPTIONS, integrations: getSentryIntegrations(hasReplays), tracesSampleRate, _experiments: {useEnvelope: true}, + async beforeSend(event) { + if (event.type === 'transaction') { + // for JavaScript transactions, set the transaction name based on the current window.location + // object against the application react-router routes + + const transactionName: string | undefined = await new Promise(function(resolve) { + Router.match( + {routes: appRoutes, location: window.location}, + (error, _redirectLocation, renderProps) => { + if (error) { + return resolve(undefined); + } + + const routePath = getRouteStringFromRoutes(renderProps.routes ?? []); + return resolve(routePath); + } + ); + }); + + if (typeof transactionName === 'string' && transactionName.length) { + event.transaction = transactionName; + } + } + + return event; + }, }); if (window.__SENTRY__USER) { From 579cacca40da6d3bafe235f00bdb140e335a1e96 Mon Sep 17 00:00:00 2001 From: Alberto Leal Date: Thu, 14 May 2020 10:16:37 -0400 Subject: [PATCH 2/6] translate transaction name if it exists and doesn't start with / --- src/sentry/static/sentry/app/bootstrap.tsx | 25 +++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/sentry/static/sentry/app/bootstrap.tsx b/src/sentry/static/sentry/app/bootstrap.tsx index 200e0a9ea7116a..1226e8a8880f6e 100644 --- a/src/sentry/static/sentry/app/bootstrap.tsx +++ b/src/sentry/static/sentry/app/bootstrap.tsx @@ -12,6 +12,7 @@ import React from 'react'; import ReactDOM from 'react-dom'; import Reflux from 'reflux'; import * as Router from 'react-router'; +import {createMemoryHistory} from 'history'; import * as Sentry from '@sentry/browser'; import {ExtraErrorData} from '@sentry/integrations'; import {Integrations} from '@sentry/apm'; @@ -77,6 +78,7 @@ const hasReplays = window.__SENTRY__USER && window.__SENTRY__USER.isStaff && !!process.env.DISABLE_RR_WEB; const appRoutes = Router.createRoutes(routes()); +const createLocation = createMemoryHistory().createLocation; Sentry.init({ ...window.__SENTRY__OPTIONS, @@ -88,9 +90,22 @@ Sentry.init({ // for JavaScript transactions, set the transaction name based on the current window.location // object against the application react-router routes + let prevTransactionName = event.transaction; + + if (typeof prevTransactionName === 'string') { + if (prevTransactionName.startsWith('/')) { + return event; + } + } else { + prevTransactionName = window.location.pathname; + } + const transactionName: string | undefined = await new Promise(function(resolve) { Router.match( - {routes: appRoutes, location: window.location}, + { + routes: appRoutes, + location: createLocation(prevTransactionName), + }, (error, _redirectLocation, renderProps) => { if (error) { return resolve(undefined); @@ -104,6 +119,14 @@ Sentry.init({ if (typeof transactionName === 'string' && transactionName.length) { event.transaction = transactionName; + + if (event.tags) { + event.tags['ui.route'] = transactionName; + } else { + event.tags = { + 'ui.route': transactionName, + }; + } } } From bb11e491f0cb508cbf27c944a0e83bd6ccb1e33e Mon Sep 17 00:00:00 2001 From: Alberto Leal Date: Thu, 14 May 2020 13:14:46 -0400 Subject: [PATCH 3/6] docs --- src/sentry/static/sentry/app/bootstrap.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/sentry/static/sentry/app/bootstrap.tsx b/src/sentry/static/sentry/app/bootstrap.tsx index 1226e8a8880f6e..b82809d7b44f0e 100644 --- a/src/sentry/static/sentry/app/bootstrap.tsx +++ b/src/sentry/static/sentry/app/bootstrap.tsx @@ -87,8 +87,9 @@ Sentry.init({ _experiments: {useEnvelope: true}, async beforeSend(event) { if (event.type === 'transaction') { - // for JavaScript transactions, set the transaction name based on the current window.location - // object against the application react-router routes + // For JavaScript transactions, translate the transaction name if it exists and doesn't start with / + // using the app's react-router routes. If the transaction name doesn't exist, use the window.location.pathname + // as the fallback. let prevTransactionName = event.transaction; From 542bce13d84cd6de1a6db688fec8156c58803c58 Mon Sep 17 00:00:00 2001 From: Alberto Leal Date: Tue, 19 May 2020 19:23:24 -0400 Subject: [PATCH 4/6] consolidate code --- src/sentry/static/sentry/app/bootstrap.tsx | 54 +------------------- src/sentry/static/sentry/app/utils/apm.tsx | 59 ++++++++++++++++++++++ 2 files changed, 61 insertions(+), 52 deletions(-) diff --git a/src/sentry/static/sentry/app/bootstrap.tsx b/src/sentry/static/sentry/app/bootstrap.tsx index b82809d7b44f0e..25b5f8749a4b2d 100644 --- a/src/sentry/static/sentry/app/bootstrap.tsx +++ b/src/sentry/static/sentry/app/bootstrap.tsx @@ -12,7 +12,6 @@ import React from 'react'; import ReactDOM from 'react-dom'; import Reflux from 'reflux'; import * as Router from 'react-router'; -import {createMemoryHistory} from 'history'; import * as Sentry from '@sentry/browser'; import {ExtraErrorData} from '@sentry/integrations'; import {Integrations} from '@sentry/apm'; @@ -27,8 +26,7 @@ import ConfigStore from 'app/stores/configStore'; import Main from 'app/main'; import ajaxCsrfSetup from 'app/utils/ajaxCsrfSetup'; import plugins from 'app/plugins'; -import routes from 'app/routes'; -import getRouteStringFromRoutes from 'app/utils/getRouteStringFromRoutes'; +import {normalizeTransactionName} from 'app/utils/apm'; function getSentryIntegrations(hasReplays: boolean = false) { const integrations = [ @@ -77,61 +75,13 @@ const tracesSampleRate = config ? config.apmSampling : 0; const hasReplays = window.__SENTRY__USER && window.__SENTRY__USER.isStaff && !!process.env.DISABLE_RR_WEB; -const appRoutes = Router.createRoutes(routes()); -const createLocation = createMemoryHistory().createLocation; - Sentry.init({ ...window.__SENTRY__OPTIONS, integrations: getSentryIntegrations(hasReplays), tracesSampleRate, _experiments: {useEnvelope: true}, async beforeSend(event) { - if (event.type === 'transaction') { - // For JavaScript transactions, translate the transaction name if it exists and doesn't start with / - // using the app's react-router routes. If the transaction name doesn't exist, use the window.location.pathname - // as the fallback. - - let prevTransactionName = event.transaction; - - if (typeof prevTransactionName === 'string') { - if (prevTransactionName.startsWith('/')) { - return event; - } - } else { - prevTransactionName = window.location.pathname; - } - - const transactionName: string | undefined = await new Promise(function(resolve) { - Router.match( - { - routes: appRoutes, - location: createLocation(prevTransactionName), - }, - (error, _redirectLocation, renderProps) => { - if (error) { - return resolve(undefined); - } - - const routePath = getRouteStringFromRoutes(renderProps.routes ?? []); - return resolve(routePath); - } - ); - }); - - if (typeof transactionName === 'string' && transactionName.length) { - event.transaction = transactionName; - - if (event.tags) { - event.tags['ui.route'] = transactionName; - } else { - event.tags = { - 'ui.route': transactionName, - }; - } - } - } - - return event; + return normalizeTransactionName(event); }, }); diff --git a/src/sentry/static/sentry/app/utils/apm.tsx b/src/sentry/static/sentry/app/utils/apm.tsx index d2a5a3e6989edf..0349f08eaf9903 100644 --- a/src/sentry/static/sentry/app/utils/apm.tsx +++ b/src/sentry/static/sentry/app/utils/apm.tsx @@ -1,4 +1,12 @@ import * as Sentry from '@sentry/browser'; +import {createMemoryHistory} from 'history'; +import * as Router from 'react-router'; + +import routes from 'app/routes'; +import getRouteStringFromRoutes from 'app/utils/getRouteStringFromRoutes'; + +const appRoutes = Router.createRoutes(routes()); +const createLocation = createMemoryHistory().createLocation; /** * Sets the transaction name @@ -9,3 +17,54 @@ export function setTransactionName(name: string) { scope.setTag('ui.route', name); }); } + +export async function normalizeTransactionName( + event: Sentry.Event +): Promise { + if (event.type === 'transaction') { + // For JavaScript transactions, translate the transaction name if it exists and doesn't start with / + // using the app's react-router routes. If the transaction name doesn't exist, use the window.location.pathname + // as the fallback. + + let prevTransactionName = event.transaction; + + if (typeof prevTransactionName === 'string') { + if (prevTransactionName.startsWith('/')) { + return event; + } + } else { + prevTransactionName = window.location.pathname; + } + + const transactionName: string | undefined = await new Promise(function(resolve) { + Router.match( + { + routes: appRoutes, + location: createLocation(prevTransactionName), + }, + (error, _redirectLocation, renderProps) => { + if (error) { + return resolve(undefined); + } + + const routePath = getRouteStringFromRoutes(renderProps.routes ?? []); + return resolve(routePath); + } + ); + }); + + if (typeof transactionName === 'string' && transactionName.length) { + event.transaction = transactionName; + + if (event.tags) { + event.tags['ui.route'] = transactionName; + } else { + event.tags = { + 'ui.route': transactionName, + }; + } + } + } + + return event; +} From 194338d51dd216121e3b4d7b0d8981917dbff0e7 Mon Sep 17 00:00:00 2001 From: Alberto Leal Date: Tue, 19 May 2020 20:31:42 -0400 Subject: [PATCH 5/6] fix --- src/sentry/static/sentry/app/bootstrap.tsx | 5 ++++- src/sentry/static/sentry/app/utils/apm.tsx | 5 ++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/sentry/static/sentry/app/bootstrap.tsx b/src/sentry/static/sentry/app/bootstrap.tsx index 25b5f8749a4b2d..b850c07f7c40bb 100644 --- a/src/sentry/static/sentry/app/bootstrap.tsx +++ b/src/sentry/static/sentry/app/bootstrap.tsx @@ -26,6 +26,7 @@ import ConfigStore from 'app/stores/configStore'; import Main from 'app/main'; import ajaxCsrfSetup from 'app/utils/ajaxCsrfSetup'; import plugins from 'app/plugins'; +import routes from 'app/routes'; import {normalizeTransactionName} from 'app/utils/apm'; function getSentryIntegrations(hasReplays: boolean = false) { @@ -75,13 +76,15 @@ const tracesSampleRate = config ? config.apmSampling : 0; const hasReplays = window.__SENTRY__USER && window.__SENTRY__USER.isStaff && !!process.env.DISABLE_RR_WEB; +const appRoutes = Router.createRoutes(routes()); + Sentry.init({ ...window.__SENTRY__OPTIONS, integrations: getSentryIntegrations(hasReplays), tracesSampleRate, _experiments: {useEnvelope: true}, async beforeSend(event) { - return normalizeTransactionName(event); + return normalizeTransactionName(appRoutes, event); }, }); diff --git a/src/sentry/static/sentry/app/utils/apm.tsx b/src/sentry/static/sentry/app/utils/apm.tsx index 0349f08eaf9903..80ff87b2f93bdc 100644 --- a/src/sentry/static/sentry/app/utils/apm.tsx +++ b/src/sentry/static/sentry/app/utils/apm.tsx @@ -1,11 +1,9 @@ import * as Sentry from '@sentry/browser'; -import {createMemoryHistory} from 'history'; import * as Router from 'react-router'; +import {createMemoryHistory} from 'history'; -import routes from 'app/routes'; import getRouteStringFromRoutes from 'app/utils/getRouteStringFromRoutes'; -const appRoutes = Router.createRoutes(routes()); const createLocation = createMemoryHistory().createLocation; /** @@ -19,6 +17,7 @@ export function setTransactionName(name: string) { } export async function normalizeTransactionName( + appRoutes: Router.PlainRoute[], event: Sentry.Event ): Promise { if (event.type === 'transaction') { From 5249c9b42098856888c4675f06faca2db7dc1775 Mon Sep 17 00:00:00 2001 From: Alberto Leal Date: Tue, 19 May 2020 20:34:09 -0400 Subject: [PATCH 6/6] add tags to track the transaction rename --- src/sentry/static/sentry/app/utils/apm.tsx | 81 ++++++++++++---------- 1 file changed, 44 insertions(+), 37 deletions(-) diff --git a/src/sentry/static/sentry/app/utils/apm.tsx b/src/sentry/static/sentry/app/utils/apm.tsx index 80ff87b2f93bdc..34f543353982eb 100644 --- a/src/sentry/static/sentry/app/utils/apm.tsx +++ b/src/sentry/static/sentry/app/utils/apm.tsx @@ -1,6 +1,7 @@ import * as Sentry from '@sentry/browser'; import * as Router from 'react-router'; import {createMemoryHistory} from 'history'; +import set from 'lodash/set'; import getRouteStringFromRoutes from 'app/utils/getRouteStringFromRoutes'; @@ -20,49 +21,55 @@ export async function normalizeTransactionName( appRoutes: Router.PlainRoute[], event: Sentry.Event ): Promise { - if (event.type === 'transaction') { - // For JavaScript transactions, translate the transaction name if it exists and doesn't start with / - // using the app's react-router routes. If the transaction name doesn't exist, use the window.location.pathname - // as the fallback. + if (event.type !== 'transaction') { + return event; + } - let prevTransactionName = event.transaction; + // For JavaScript transactions, translate the transaction name if it exists and doesn't start with / + // using the app's react-router routes. If the transaction name doesn't exist, use the window.location.pathname + // as the fallback. - if (typeof prevTransactionName === 'string') { - if (prevTransactionName.startsWith('/')) { - return event; - } - } else { - prevTransactionName = window.location.pathname; + let prevTransactionName = event.transaction; + + if (typeof prevTransactionName === 'string') { + if (prevTransactionName.startsWith('/')) { + return event; } - const transactionName: string | undefined = await new Promise(function(resolve) { - Router.match( - { - routes: appRoutes, - location: createLocation(prevTransactionName), - }, - (error, _redirectLocation, renderProps) => { - if (error) { - return resolve(undefined); - } - - const routePath = getRouteStringFromRoutes(renderProps.routes ?? []); - return resolve(routePath); + set(event, ['tags', 'transaction.rename.source'], 'existing transaction name'); + } else { + set(event, ['tags', 'transaction.rename.source'], 'window.location.pathname'); + + prevTransactionName = window.location.pathname; + } + + const transactionName: string | undefined = await new Promise(function(resolve) { + Router.match( + { + routes: appRoutes, + location: createLocation(prevTransactionName), + }, + (error, _redirectLocation, renderProps) => { + if (error) { + set(event, ['tags', 'transaction.rename.react-router-match'], 'error'); + return resolve(undefined); } - ); - }); - - if (typeof transactionName === 'string' && transactionName.length) { - event.transaction = transactionName; - - if (event.tags) { - event.tags['ui.route'] = transactionName; - } else { - event.tags = { - 'ui.route': transactionName, - }; + + set(event, ['tags', 'transaction.rename.react-router-match'], 'success'); + + const routePath = getRouteStringFromRoutes(renderProps.routes ?? []); + return resolve(routePath); } - } + ); + }); + + if (typeof transactionName === 'string' && transactionName.length) { + event.transaction = transactionName; + + set(event, ['tags', 'transaction.rename.before'], prevTransactionName); + set(event, ['tags', 'transaction.rename.after'], transactionName); + + set(event, ['tags', 'ui.route'], transactionName); } return event;