From d80ee32f97a788ebc5807198de374a5c43ace7da Mon Sep 17 00:00:00 2001 From: k-fish Date: Tue, 28 Jul 2020 11:28:19 -0700 Subject: [PATCH 01/16] feat: Add router instrumentation for @sentry/ember This will add routing instrumentation for Ember transitions to @sentry/ember. The import will be dynamically loading depending on configuration settings with to not increase bundle size. --- packages/ember/addon/index.ts | 5 +- .../sentry-performance.ts | 74 +++++++++++++++++++ .../sentry-performance.js | 1 + packages/ember/index.js | 7 +- packages/ember/package.json | 1 + packages/ember/tests/dummy/app/app.js | 4 +- .../dummy/app/controllers/application.js | 62 +--------------- .../tests/dummy/app/controllers/index.js | 63 ++++++++++++++++ packages/ember/tests/dummy/app/router.js | 1 + packages/ember/tests/dummy/app/styles/app.css | 47 +++++++++--- .../tests/dummy/app/templates/application.hbs | 43 ++--------- .../ember/tests/dummy/app/templates/index.hbs | 11 +++ .../tests/dummy/app/templates/tracing.hbs | 0 .../ember/tests/dummy/config/environment.js | 3 +- packages/ember/tsconfig.json | 2 +- 15 files changed, 208 insertions(+), 116 deletions(-) create mode 100644 packages/ember/addon/instance-initializers/sentry-performance.ts create mode 100644 packages/ember/app/instance-initializers/sentry-performance.js create mode 100644 packages/ember/tests/dummy/app/controllers/index.js create mode 100644 packages/ember/tests/dummy/app/templates/index.hbs create mode 100644 packages/ember/tests/dummy/app/templates/tracing.hbs diff --git a/packages/ember/addon/index.ts b/packages/ember/addon/index.ts index babe2de8fd73..5b1a1689c023 100644 --- a/packages/ember/addon/index.ts +++ b/packages/ember/addon/index.ts @@ -21,7 +21,7 @@ export function InitSentryForEmber(_runtimeConfig: BrowserOptions | undefined) { if (config.ignoreEmberOnErrorWarning) { return; } - next(null, function () { + next(null, function() { warn( 'Ember.onerror found. Using Ember.onerror can hide some errors (such as flushed runloop errors) from Sentry. Use Sentry.captureException to capture errors within Ember.onError or remove it to have errors caught by Sentry directly. This error can be silenced via addon configuration.', !Ember.onerror, @@ -35,7 +35,7 @@ export function InitSentryForEmber(_runtimeConfig: BrowserOptions | undefined) { function createEmberEventProcessor(): void { if (addGlobalEventProcessor) { - addGlobalEventProcessor((event) => { + addGlobalEventProcessor(event => { event.sdk = { ...event.sdk, name: 'sentry.javascript.ember', @@ -54,4 +54,5 @@ function createEmberEventProcessor(): void { } } +export default Sentry; export * from '@sentry/browser'; diff --git a/packages/ember/addon/instance-initializers/sentry-performance.ts b/packages/ember/addon/instance-initializers/sentry-performance.ts new file mode 100644 index 000000000000..3a4b515d1202 --- /dev/null +++ b/packages/ember/addon/instance-initializers/sentry-performance.ts @@ -0,0 +1,74 @@ +import ApplicationInstance from '@ember/application/instance'; +import environmentConfig from 'ember-get-config'; +import Sentry from '@sentry/ember'; + +export function initialize(appInstance: ApplicationInstance): void { + const config = environmentConfig['@sentry/ember']; + if (config['disablePerformance']) { + return; + } + instrumentForPerformance(appInstance); +} + +function getTransitionInformation(transition: any, router: any) { + const fromRoute = transition?.from?.name; + const toRoute = transition ? transition.to.name : router.currentRouteName; + return { + fromRoute, + toRoute, + }; +} + +export async function instrumentForPerformance(appInstance: ApplicationInstance) { + const config = environmentConfig['@sentry/ember']; + const sentryConfig = config.sentry; + const tracing = await import('@sentry/tracing'); + + const router = appInstance.lookup('service:router'); + + sentryConfig['integrations'] = [ + new tracing.Integrations.BrowserTracing({ + routingInstrumentation: (startTransaction, startTransactionOnPageLoad) => { + let activeTransaction: any; + if (startTransactionOnPageLoad && window.location) { + const routeInfo = router.recognize(window.location.pathname); + activeTransaction = startTransaction({ + name: `route:${routeInfo.name}`, + op: 'pageload', + tags: { + url: window.location.pathname, + toRoute: routeInfo.name, + 'routing.instrumentation': '@sentry/ember', + }, + }); + } + + router.on('routeWillChange', (transition: any) => { + const { fromRoute, toRoute } = getTransitionInformation(transition, router); + activeTransaction = startTransaction({ + name: `route:${toRoute}`, + op: 'navigation', + tags: { + fromRoute, + toRoute, + 'routing.instrumentation': '@sentry/ember', + }, + }); + }); + router.on('routeDidChange', () => { + if (activeTransaction) { + activeTransaction.finish(); + } + }); + }, + idleTimeout: 2000, + }), + ]; + + const browserClient = new Sentry.BrowserClient(sentryConfig); + Sentry.getCurrentHub().bindClient(browserClient); +} + +export default { + initialize, +}; diff --git a/packages/ember/app/instance-initializers/sentry-performance.js b/packages/ember/app/instance-initializers/sentry-performance.js new file mode 100644 index 000000000000..3137198dc7c2 --- /dev/null +++ b/packages/ember/app/instance-initializers/sentry-performance.js @@ -0,0 +1 @@ +export { default, initialize } from '@sentry/ember/instance-initializers/sentry-performance'; diff --git a/packages/ember/index.js b/packages/ember/index.js index 2e1d1d8d5fa0..967d490abc24 100644 --- a/packages/ember/index.js +++ b/packages/ember/index.js @@ -1,5 +1,10 @@ 'use strict'; module.exports = { - name: require('./package').name + name: require('./package').name, + options: { + babel: { + plugins: [ require.resolve('ember-auto-import/babel-plugin') ] + } + } }; diff --git a/packages/ember/package.json b/packages/ember/package.json index 395b2fd93855..4abf450f3784 100644 --- a/packages/ember/package.json +++ b/packages/ember/package.json @@ -27,6 +27,7 @@ }, "dependencies": { "@sentry/browser": "5.20.1", + "@sentry/tracing": "5.20.1", "@sentry/types": "5.20.1", "@sentry/utils": "5.20.1", "ember-get-config": "^0.2.4", diff --git a/packages/ember/tests/dummy/app/app.js b/packages/ember/tests/dummy/app/app.js index 6081cb2b951f..8beb74a67fc2 100644 --- a/packages/ember/tests/dummy/app/app.js +++ b/packages/ember/tests/dummy/app/app.js @@ -20,7 +20,9 @@ class TestFetchTransport extends Transports.FetchTransport { } } -InitSentryForEmber({ transport: TestFetchTransport }); +InitSentryForEmber({ + transport: TestFetchTransport, +}); export default class App extends Application { modulePrefix = config.modulePrefix; diff --git a/packages/ember/tests/dummy/app/controllers/application.js b/packages/ember/tests/dummy/app/controllers/application.js index 49ebe3e6cf45..304707936f81 100644 --- a/packages/ember/tests/dummy/app/controllers/application.js +++ b/packages/ember/tests/dummy/app/controllers/application.js @@ -1,63 +1,3 @@ import Controller from '@ember/controller'; -import { tracked } from '@glimmer/tracking'; -import { action } from '@ember/object'; -import EmberError from '@ember/error'; -import { scheduleOnce } from '@ember/runloop'; -import RSVP from 'rsvp'; -export default class ApplicationController extends Controller { - @tracked showComponents; - - @action - createError() { - this.nonExistentFunction(); - } - - @action - createEmberError() { - throw new EmberError('Whoops, looks like you have an EmberError'); - } - - @action - createCaughtEmberError() { - try { - throw new EmberError('Looks like you have a caught EmberError'); - } catch(e) { - console.log(e); - } - } - - @action - createFetchError() { - fetch('http://doesntexist.example'); - } - - @action - createAfterRenderError() { - function throwAfterRender() { - throw new Error('After Render Error'); - } - scheduleOnce('afterRender', throwAfterRender); - } - - @action - createRSVPRejection() { - const promise = new RSVP.Promise((resolve, reject) => { - reject('Promise rejected'); - }); - return promise; - } - - @action - createRSVPError() { - const promise = new RSVP.Promise(() => { - throw new Error('Error within RSVP Promise'); - }); - return promise; - } - - @action - toggleShowComponents() { - this.showComponents = !this.showComponents; - } -} +export default class ApplicationController extends Controller {} diff --git a/packages/ember/tests/dummy/app/controllers/index.js b/packages/ember/tests/dummy/app/controllers/index.js new file mode 100644 index 000000000000..a1e01de99a8c --- /dev/null +++ b/packages/ember/tests/dummy/app/controllers/index.js @@ -0,0 +1,63 @@ +import Controller from '@ember/controller'; +import { tracked } from '@glimmer/tracking'; +import { action } from '@ember/object'; +import EmberError from '@ember/error'; +import { scheduleOnce } from '@ember/runloop'; +import RSVP from 'rsvp'; + +export default class IndexController extends Controller { + @tracked showComponents; + + @action + createError() { + this.nonExistentFunction(); + } + + @action + createEmberError() { + throw new EmberError('Whoops, looks like you have an EmberError'); + } + + @action + createCaughtEmberError() { + try { + throw new EmberError('Looks like you have a caught EmberError'); + } catch (e) { + console.log(e); + } + } + + @action + createFetchError() { + fetch('http://doesntexist.example'); + } + + @action + createAfterRenderError() { + function throwAfterRender() { + throw new Error('After Render Error'); + } + scheduleOnce('afterRender', throwAfterRender); + } + + @action + createRSVPRejection() { + const promise = new RSVP.Promise((resolve, reject) => { + reject('Promise rejected'); + }); + return promise; + } + + @action + createRSVPError() { + const promise = new RSVP.Promise(() => { + throw new Error('Error within RSVP Promise'); + }); + return promise; + } + + @action + toggleShowComponents() { + this.showComponents = !this.showComponents; + } +} diff --git a/packages/ember/tests/dummy/app/router.js b/packages/ember/tests/dummy/app/router.js index 224ca426a85e..3ad8a7479c4b 100644 --- a/packages/ember/tests/dummy/app/router.js +++ b/packages/ember/tests/dummy/app/router.js @@ -7,4 +7,5 @@ export default class Router extends EmberRouter { } Router.map(function() { + this.route('tracing'); }); diff --git a/packages/ember/tests/dummy/app/styles/app.css b/packages/ember/tests/dummy/app/styles/app.css index 1f8593255fdf..4c69b62ced41 100644 --- a/packages/ember/tests/dummy/app/styles/app.css +++ b/packages/ember/tests/dummy/app/styles/app.css @@ -17,7 +17,7 @@ body { background-repeat: repeat; height: 100%; margin: 0; - font-family: Rubik,Avenir Next,Helvetica Neue,sans-serif; + font-family: Rubik, Avenir Next, Helvetica Neue, sans-serif; font-size: 16px; line-height: 24px; color: var(--foreground-color); @@ -43,7 +43,7 @@ body { .box { background-color: #fff; border: 0; - box-shadow: 0 0 0 1px rgba(0,0,0,.08),0 1px 4px rgba(0,0,0,.1); + box-shadow: 0 0 0 1px rgba(0, 0, 0, 0.08), 0 1px 4px rgba(0, 0, 0, 0.1); border-radius: 4px; display: flex; width: 100%; @@ -54,8 +54,8 @@ body { padding-top: 20px; width: 60px; background: #564f64; - background-image: linear-gradient(-180deg,rgba(52,44,62,0),rgba(52,44,62,.5)); - box-shadow: 0 2px 0 0 rgba(0,0,0,.1); + background-image: linear-gradient(-180deg, rgba(52, 44, 62, 0), rgba(52, 44, 62, 0.5)); + box-shadow: 0 2px 0 0 rgba(0, 0, 0, 0.1); border-radius: 4px 0 0 4px; margin-top: -1px; margin-bottom: -1px; @@ -73,12 +73,37 @@ body { background-image: url('/assets/images/sentry-logo.svg'); } +.nav { + display: flex; + justify-content: center; + padding: 10px; + padding-top: 20px; + padding-bottom: 0px; +} + +.nav a { + padding-left: 10px; + padding-right: 10px; + font-weight: 500; + text-decoration: none; + color: var(--foreground-color); +} + +.nav a.active { + border-bottom: 4px solid #6c5fc7; +} + section.content { flex: 1; padding-bottom: 40px; } -h1, h2, h3, h4, h5, h6 { +h1, +h2, +h3, +h4, +h5, +h6 { font-weight: 600; } @@ -101,7 +126,8 @@ div.section { padding-top: 20px; } -.content-container h3, .content-container h4 { +.content-container h3, +.content-container h4 { margin-top: 0px; } @@ -113,7 +139,7 @@ button { border-radius: 3px; font-weight: 600; padding: 8px 16px; - transition: all .1s; + transition: all 0.1s; border: 1px solid transparent; border-radius: 3px; @@ -139,8 +165,8 @@ button.primary { display: inline-block; - text-shadow: 0 -1px 0 rgba(0,0,0,.15); - box-shadow: 0 2px 0 rgba(0,0,0,.08); + text-shadow: 0 -1px 0 rgba(0, 0, 0, 0.15); + box-shadow: 0 2px 0 rgba(0, 0, 0, 0.08); text-transform: none; overflow: visible; @@ -154,7 +180,6 @@ button.primary:hover { button.primary:focus { background: #5b4cc0; border-color: #3a2f87; - box-shadow: inset 0 2px 0 rgba(0,0,0,.12); + box-shadow: inset 0 2px 0 rgba(0, 0, 0, 0.12); outline: none; } - diff --git a/packages/ember/tests/dummy/app/templates/application.hbs b/packages/ember/tests/dummy/app/templates/application.hbs index 6dac270780f8..4e41d992dc3c 100644 --- a/packages/ember/tests/dummy/app/templates/application.hbs +++ b/packages/ember/tests/dummy/app/templates/application.hbs @@ -1,4 +1,3 @@ -
@@ -9,46 +8,14 @@

Sentry Instrumented Ember Application

+
+ Errors + Tracing +
- - - - - - - + {{outlet}}
- -{{outlet}} diff --git a/packages/ember/tests/dummy/app/templates/index.hbs b/packages/ember/tests/dummy/app/templates/index.hbs new file mode 100644 index 000000000000..b39ffce5c0e8 --- /dev/null +++ b/packages/ember/tests/dummy/app/templates/index.hbs @@ -0,0 +1,11 @@ + + + + + + + +{{outlet}} diff --git a/packages/ember/tests/dummy/app/templates/tracing.hbs b/packages/ember/tests/dummy/app/templates/tracing.hbs new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/packages/ember/tests/dummy/config/environment.js b/packages/ember/tests/dummy/config/environment.js index bb0f5c2608c6..e8588c35c079 100644 --- a/packages/ember/tests/dummy/config/environment.js +++ b/packages/ember/tests/dummy/config/environment.js @@ -1,6 +1,6 @@ 'use strict'; -module.exports = function (environment) { +module.exports = function(environment) { let ENV = { modulePrefix: 'dummy', environment, @@ -25,6 +25,7 @@ module.exports = function (environment) { ENV['@sentry/ember'] = { sentry: { + tracesSampleRate: 1, dsn: process.env.SENTRY_DSN, }, ignoreEmberOnErrorWarning: true, diff --git a/packages/ember/tsconfig.json b/packages/ember/tsconfig.json index 71c52a3c72d9..2011ce8f8735 100644 --- a/packages/ember/tsconfig.json +++ b/packages/ember/tsconfig.json @@ -12,7 +12,7 @@ "noEmitOnError": false, "noEmit": true, "baseUrl": ".", - "module": "es6", + "module": "esnext", "experimentalDecorators": true, "paths": { "dummy/tests/*": ["tests/*"], From 02ec96c63df764cc043ba0ebc63a765688f28f48 Mon Sep 17 00:00:00 2001 From: k-fish Date: Wed, 26 Aug 2020 08:46:51 -0700 Subject: [PATCH 02/16] Fix integrations overriding integrations specified by the user --- packages/ember/addon/instance-initializers/sentry-performance.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/ember/addon/instance-initializers/sentry-performance.ts b/packages/ember/addon/instance-initializers/sentry-performance.ts index 3a4b515d1202..caeef417b876 100644 --- a/packages/ember/addon/instance-initializers/sentry-performance.ts +++ b/packages/ember/addon/instance-initializers/sentry-performance.ts @@ -27,6 +27,7 @@ export async function instrumentForPerformance(appInstance: ApplicationInstance) const router = appInstance.lookup('service:router'); sentryConfig['integrations'] = [ + ...(sentryConfig['integrations'] || []), new tracing.Integrations.BrowserTracing({ routingInstrumentation: (startTransaction, startTransactionOnPageLoad) => { let activeTransaction: any; From d896ff5ca16344ceabdf6f0452cf254d75a680c8 Mon Sep 17 00:00:00 2001 From: k-fish Date: Thu, 27 Aug 2020 20:46:52 -0700 Subject: [PATCH 03/16] Add example of slow loading route due to `model` --- .../addon/instance-initializers/sentry-performance.ts | 4 +++- .../tests/dummy/app/controllers/slow-loading-route.js | 9 +++++++++ packages/ember/tests/dummy/app/controllers/tracing.js | 9 +++++++++ packages/ember/tests/dummy/app/helpers/utils.js | 3 +++ packages/ember/tests/dummy/app/router.js | 1 + .../ember/tests/dummy/app/routes/slow-loading-route.js | 8 ++++++++ .../tests/dummy/app/templates/slow-loading-route.hbs | 4 ++++ packages/ember/tests/dummy/app/templates/tracing.hbs | 2 ++ 8 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 packages/ember/tests/dummy/app/controllers/slow-loading-route.js create mode 100644 packages/ember/tests/dummy/app/controllers/tracing.js create mode 100644 packages/ember/tests/dummy/app/helpers/utils.js create mode 100644 packages/ember/tests/dummy/app/routes/slow-loading-route.js create mode 100644 packages/ember/tests/dummy/app/templates/slow-loading-route.hbs diff --git a/packages/ember/addon/instance-initializers/sentry-performance.ts b/packages/ember/addon/instance-initializers/sentry-performance.ts index caeef417b876..176e5cf6fb44 100644 --- a/packages/ember/addon/instance-initializers/sentry-performance.ts +++ b/packages/ember/addon/instance-initializers/sentry-performance.ts @@ -24,6 +24,8 @@ export async function instrumentForPerformance(appInstance: ApplicationInstance) const sentryConfig = config.sentry; const tracing = await import('@sentry/tracing'); + const idleTimeout = config.transitionTimeout || 15000; + const router = appInstance.lookup('service:router'); sentryConfig['integrations'] = [ @@ -62,7 +64,7 @@ export async function instrumentForPerformance(appInstance: ApplicationInstance) } }); }, - idleTimeout: 2000, + idleTimeout, }), ]; diff --git a/packages/ember/tests/dummy/app/controllers/slow-loading-route.js b/packages/ember/tests/dummy/app/controllers/slow-loading-route.js new file mode 100644 index 000000000000..afe644bffd41 --- /dev/null +++ b/packages/ember/tests/dummy/app/controllers/slow-loading-route.js @@ -0,0 +1,9 @@ +import Controller from '@ember/controller'; +import { action } from '@ember/object'; + +export default class SlowLoadingRouteController extends Controller { + @action + back() { + this.transitionToRoute('tracing'); + } +} diff --git a/packages/ember/tests/dummy/app/controllers/tracing.js b/packages/ember/tests/dummy/app/controllers/tracing.js new file mode 100644 index 000000000000..539a989ac829 --- /dev/null +++ b/packages/ember/tests/dummy/app/controllers/tracing.js @@ -0,0 +1,9 @@ +import Controller from '@ember/controller'; +import { action } from '@ember/object'; + +export default class TracingController extends Controller { + @action + navigateToSlowRoute() { + return this.transitionToRoute('slow-loading-route'); + } +} diff --git a/packages/ember/tests/dummy/app/helpers/utils.js b/packages/ember/tests/dummy/app/helpers/utils.js new file mode 100644 index 000000000000..7aa94999942c --- /dev/null +++ b/packages/ember/tests/dummy/app/helpers/utils.js @@ -0,0 +1,3 @@ +export default function timeout(time) { + return new Promise(resolve => setTimeout(resolve, time)); +} diff --git a/packages/ember/tests/dummy/app/router.js b/packages/ember/tests/dummy/app/router.js index 3ad8a7479c4b..1eb50a98d851 100644 --- a/packages/ember/tests/dummy/app/router.js +++ b/packages/ember/tests/dummy/app/router.js @@ -8,4 +8,5 @@ export default class Router extends EmberRouter { Router.map(function() { this.route('tracing'); + this.route('slow-loading-route'); }); diff --git a/packages/ember/tests/dummy/app/routes/slow-loading-route.js b/packages/ember/tests/dummy/app/routes/slow-loading-route.js new file mode 100644 index 000000000000..5fc9c444cb70 --- /dev/null +++ b/packages/ember/tests/dummy/app/routes/slow-loading-route.js @@ -0,0 +1,8 @@ +import Route from '@ember/routing/route'; +import timeout from '../helpers/utils'; + +export default class SlowLoadingRoute extends Route { + model() { + return timeout(3000); + } +} diff --git a/packages/ember/tests/dummy/app/templates/slow-loading-route.hbs b/packages/ember/tests/dummy/app/templates/slow-loading-route.hbs new file mode 100644 index 000000000000..d85ac939e59a --- /dev/null +++ b/packages/ember/tests/dummy/app/templates/slow-loading-route.hbs @@ -0,0 +1,4 @@ +

Intentionally Slow Route

+ diff --git a/packages/ember/tests/dummy/app/templates/tracing.hbs b/packages/ember/tests/dummy/app/templates/tracing.hbs index e69de29bb2d1..b2e5087f9f3e 100644 --- a/packages/ember/tests/dummy/app/templates/tracing.hbs +++ b/packages/ember/tests/dummy/app/templates/tracing.hbs @@ -0,0 +1,2 @@ + From d958bbf960301d7c3f9b1a1bb7b536f9ff04edf1 Mon Sep 17 00:00:00 2001 From: k-fish Date: Thu, 27 Aug 2020 21:48:49 -0700 Subject: [PATCH 04/16] Fix error tests after adding performance --- .../sentry-performance.ts | 11 ++-- packages/ember/package.json | 1 + .../tests/acceptance/sentry-errors-test.js | 52 ++++++++++--------- packages/ember/tests/dummy/app/app.js | 20 +------ packages/ember/tests/test-helper.js | 18 +++++++ 5 files changed, 54 insertions(+), 48 deletions(-) diff --git a/packages/ember/addon/instance-initializers/sentry-performance.ts b/packages/ember/addon/instance-initializers/sentry-performance.ts index 176e5cf6fb44..7c3cc9c8d09d 100644 --- a/packages/ember/addon/instance-initializers/sentry-performance.ts +++ b/packages/ember/addon/instance-initializers/sentry-performance.ts @@ -26,20 +26,21 @@ export async function instrumentForPerformance(appInstance: ApplicationInstance) const idleTimeout = config.transitionTimeout || 15000; - const router = appInstance.lookup('service:router'); - sentryConfig['integrations'] = [ ...(sentryConfig['integrations'] || []), new tracing.Integrations.BrowserTracing({ routingInstrumentation: (startTransaction, startTransactionOnPageLoad) => { + const location = appInstance.lookup('router:main').location; + const router = appInstance.lookup('service:router'); let activeTransaction: any; - if (startTransactionOnPageLoad && window.location) { - const routeInfo = router.recognize(window.location.pathname); + const url = location && location.getURL && location.getURL(); + if (startTransactionOnPageLoad && url) { + const routeInfo = router.recognize(url); activeTransaction = startTransaction({ name: `route:${routeInfo.name}`, op: 'pageload', tags: { - url: window.location.pathname, + url, toRoute: routeInfo.name, 'routing.instrumentation': '@sentry/ember', }, diff --git a/packages/ember/package.json b/packages/ember/package.json index 8ae6106fbc3f..3fae300f8fb6 100644 --- a/packages/ember/package.json +++ b/packages/ember/package.json @@ -70,6 +70,7 @@ "ember-template-lint": "^2.9.1", "ember-test-selectors": "^4.1.0", "ember-try": "^1.4.0", + "ember-window-mock": "^0.7.1", "eslint": "7.6.0", "eslint-plugin-ember": "^8.6.0", "eslint-plugin-node": "^11.1.0", diff --git a/packages/ember/tests/acceptance/sentry-errors-test.js b/packages/ember/tests/acceptance/sentry-errors-test.js index 2c60b9c8d894..3fdf855b6461 100644 --- a/packages/ember/tests/acceptance/sentry-errors-test.js +++ b/packages/ember/tests/acceptance/sentry-errors-test.js @@ -10,12 +10,16 @@ const defaultAssertOptions = { errorBodyContains: [], }; -function assertSentryEventCount(assert, count) { - assert.equal(window._sentryTestEvents.length, count, 'Check correct number of Sentry events were sent'); +function getTestSentryErrors() { + return window._sentryTestEvents.filter(event => event['type'] !== 'transaction'); +} + +function assertSentryErrorCount(assert, count) { + assert.equal(getTestSentryErrors().length, count, 'Check correct number of Sentry events were sent'); } function assertSentryCall(assert, callNumber, options) { - const sentryTestEvents = window._sentryTestEvents; + const sentryTestEvents = getTestSentryErrors(); const assertOptions = Object.assign({}, defaultAssertOptions, options); const event = sentryTestEvents[callNumber]; @@ -26,15 +30,15 @@ function assertSentryCall(assert, callNumber, options) { */ assert.ok(assertOptions.errorBodyContains.length, 'Must pass strings to check against error body'); const errorBody = JSON.stringify(event); - assertOptions.errorBodyContains.forEach((bodyContent) => { + assertOptions.errorBodyContains.forEach(bodyContent => { assert.ok(errorBody.includes(bodyContent), `Checking that error body includes ${bodyContent}`); }); } -module('Acceptance | Sentry Errors', function (hooks) { +module('Acceptance | Sentry Errors', function(hooks) { setupApplicationTest(hooks); - hooks.beforeEach(function () { + hooks.beforeEach(function() { window._sentryTestEvents = []; const errorMessages = []; this.errorMessages = errorMessages; @@ -49,12 +53,12 @@ module('Acceptance | Sentry Errors', function (hooks) { */ this.qunitOnUnhandledRejection = sinon.stub(QUnit, 'onUnhandledRejection'); - QUnit.onError = function ({ message }) { + QUnit.onError = function({ message }) { errorMessages.push(message.split('Error: ')[1]); return true; }; - Ember.onerror = function (...args) { + Ember.onerror = function(...args) { const [error] = args; errorMessages.push(error.message); throw error; @@ -65,7 +69,7 @@ module('Acceptance | Sentry Errors', function (hooks) { * Will collect errors when run via testem in cli */ - window.onerror = function (error, ...args) { + window.onerror = function(error, ...args) { errorMessages.push(error.split('Error: ')[1]); if (this._windowOnError) { return this._windowOnError(error, ...args); @@ -73,42 +77,42 @@ module('Acceptance | Sentry Errors', function (hooks) { }; }); - hooks.afterEach(function () { + hooks.afterEach(function() { this.fetchStub.restore(); this.qunitOnUnhandledRejection.restore(); window.onerror = this._windowOnError; }); - test('Check "Throw Generic Javascript Error"', async function (assert) { + test('Check "Throw Generic Javascript Error"', async function(assert) { await visit('/'); const button = find('[data-test-button="Throw Generic Javascript Error"]'); await click(button); - assertSentryEventCount(assert, 1); + assertSentryErrorCount(assert, 1); assertSentryCall(assert, 0, { errorBodyContains: [...this.errorMessages] }); }); - test('Check "Throw EmberError"', async function (assert) { + test('Check "Throw EmberError"', async function(assert) { await visit('/'); const button = find('[data-test-button="Throw EmberError"]'); await click(button); - assertSentryEventCount(assert, 1); + assertSentryErrorCount(assert, 1); assertSentryCall(assert, 0, { errorBodyContains: [...this.errorMessages] }); }); - test('Check "Caught Thrown EmberError"', async function (assert) { + test('Check "Caught Thrown EmberError"', async function(assert) { await visit('/'); const button = find('[data-test-button="Caught Thrown EmberError"]'); await click(button); - assertSentryEventCount(assert, 0); + assertSentryErrorCount(assert, 0); }); - test('Check "Error From Fetch"', async function (assert) { + test('Check "Error From Fetch"', async function(assert) { this.fetchStub.onFirstCall().callsFake((...args) => { return this.fetchStub.callsThrough(args); }); @@ -120,41 +124,41 @@ module('Acceptance | Sentry Errors', function (hooks) { const done = assert.async(); run.next(() => { - assertSentryEventCount(assert, 1); + assertSentryErrorCount(assert, 1); assertSentryCall(assert, 0, { errorBodyContains: [...this.errorMessages] }); done(); }); }); - test('Check "Error in AfterRender"', async function (assert) { + test('Check "Error in AfterRender"', async function(assert) { await visit('/'); const button = find('[data-test-button="Error in AfterRender"]'); await click(button); - assertSentryEventCount(assert, 1); + assertSentryErrorCount(assert, 1); assert.ok(this.qunitOnUnhandledRejection.calledOnce, 'Uncaught rejection should only be called once'); assertSentryCall(assert, 0, { errorBodyContains: [...this.errorMessages] }); }); - test('Check "RSVP Rejection"', async function (assert) { + test('Check "RSVP Rejection"', async function(assert) { await visit('/'); const button = find('[data-test-button="RSVP Rejection"]'); await click(button); - assertSentryEventCount(assert, 1); + assertSentryErrorCount(assert, 1); assert.ok(this.qunitOnUnhandledRejection.calledOnce, 'Uncaught rejection should only be called once'); assertSentryCall(assert, 0, { errorBodyContains: [this.qunitOnUnhandledRejection.getCall(0).args[0]] }); }); - test('Check "Error inside RSVP"', async function (assert) { + test('Check "Error inside RSVP"', async function(assert) { await visit('/'); const button = find('[data-test-button="Error inside RSVP"]'); await click(button); - assertSentryEventCount(assert, 1); + assertSentryErrorCount(assert, 1); assert.ok(this.qunitOnUnhandledRejection.calledOnce, 'Uncaught rejection should only be called once'); assertSentryCall(assert, 0, { errorBodyContains: [...this.errorMessages] }); }); diff --git a/packages/ember/tests/dummy/app/app.js b/packages/ember/tests/dummy/app/app.js index 8beb74a67fc2..db99b12dcb43 100644 --- a/packages/ember/tests/dummy/app/app.js +++ b/packages/ember/tests/dummy/app/app.js @@ -4,25 +4,7 @@ import loadInitializers from 'ember-load-initializers'; import config from './config/environment'; import { InitSentryForEmber } from '@sentry/ember'; -import { Transports } from '@sentry/browser'; -import Ember from 'ember'; - -class TestFetchTransport extends Transports.FetchTransport { - sendEvent(event) { - if (Ember.testing) { - if (!window._sentryTestEvents) { - window._sentryTestEvents = []; - } - window._sentryTestEvents.push(event); - return Promise.resolve(); - } - return super.sendEvent(event); - } -} - -InitSentryForEmber({ - transport: TestFetchTransport, -}); +InitSentryForEmber(); export default class App extends Application { modulePrefix = config.modulePrefix; diff --git a/packages/ember/tests/test-helper.js b/packages/ember/tests/test-helper.js index 2a1ea0da4641..69387ded60fb 100644 --- a/packages/ember/tests/test-helper.js +++ b/packages/ember/tests/test-helper.js @@ -1,5 +1,6 @@ import sinon from 'sinon'; import * as Sentry from '@sentry/browser'; +import environmentConfig from 'ember-get-config'; /** * Stub Sentry init function before application is imported to avoid actually setting up Sentry and needing a DSN @@ -10,6 +11,23 @@ import Application from '../app'; import config from '../config/environment'; import { setApplication } from '@ember/test-helpers'; import { start } from 'ember-qunit'; +import { Transports } from '@sentry/browser'; +import Ember from 'ember'; + +export class TestFetchTransport extends Transports.FetchTransport { + sendEvent(event) { + if (Ember.testing) { + if (!window._sentryTestEvents) { + window._sentryTestEvents = []; + } + window._sentryTestEvents.push(event); + return Promise.resolve(); + } + return super.sendEvent(event); + } +} + +environmentConfig['@sentry/ember'].sentry['transport'] = TestFetchTransport; setApplication(Application.create(config.APP)); From fed373d59e6c5943217e465543cad2b32673d80f Mon Sep 17 00:00:00 2001 From: k-fish Date: Thu, 27 Aug 2020 22:27:43 -0700 Subject: [PATCH 05/16] Fix some other issues with existing error tests and change how the performance integration gets bound as otherwise default integrations won't be included --- .../sentry-performance.ts | 8 +++++--- .../tests/acceptance/sentry-errors-test.js | 3 ++- packages/ember/tests/dummy/app/app.js | 20 ++++++++++++++++++- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/packages/ember/addon/instance-initializers/sentry-performance.ts b/packages/ember/addon/instance-initializers/sentry-performance.ts index 7c3cc9c8d09d..17e0fe960ccd 100644 --- a/packages/ember/addon/instance-initializers/sentry-performance.ts +++ b/packages/ember/addon/instance-initializers/sentry-performance.ts @@ -7,7 +7,10 @@ export function initialize(appInstance: ApplicationInstance): void { if (config['disablePerformance']) { return; } - instrumentForPerformance(appInstance); + const performancePromise = instrumentForPerformance(appInstance); + if (Ember.Testing) { + window._sentryPerformanceLoad = performancePromise; + } } function getTransitionInformation(transition: any, router: any) { @@ -69,8 +72,7 @@ export async function instrumentForPerformance(appInstance: ApplicationInstance) }), ]; - const browserClient = new Sentry.BrowserClient(sentryConfig); - Sentry.getCurrentHub().bindClient(browserClient); + Sentry.init(sentryConfig); // Call init again to rebind client with new integration list in addition to the defaults } export default { diff --git a/packages/ember/tests/acceptance/sentry-errors-test.js b/packages/ember/tests/acceptance/sentry-errors-test.js index 3fdf855b6461..00647bf9288f 100644 --- a/packages/ember/tests/acceptance/sentry-errors-test.js +++ b/packages/ember/tests/acceptance/sentry-errors-test.js @@ -38,7 +38,8 @@ function assertSentryCall(assert, callNumber, options) { module('Acceptance | Sentry Errors', function(hooks) { setupApplicationTest(hooks); - hooks.beforeEach(function() { + hooks.beforeEach(async function() { + await window._sentryPerformanceLoad; window._sentryTestEvents = []; const errorMessages = []; this.errorMessages = errorMessages; diff --git a/packages/ember/tests/dummy/app/app.js b/packages/ember/tests/dummy/app/app.js index db99b12dcb43..8beb74a67fc2 100644 --- a/packages/ember/tests/dummy/app/app.js +++ b/packages/ember/tests/dummy/app/app.js @@ -4,7 +4,25 @@ import loadInitializers from 'ember-load-initializers'; import config from './config/environment'; import { InitSentryForEmber } from '@sentry/ember'; -InitSentryForEmber(); +import { Transports } from '@sentry/browser'; +import Ember from 'ember'; + +class TestFetchTransport extends Transports.FetchTransport { + sendEvent(event) { + if (Ember.testing) { + if (!window._sentryTestEvents) { + window._sentryTestEvents = []; + } + window._sentryTestEvents.push(event); + return Promise.resolve(); + } + return super.sendEvent(event); + } +} + +InitSentryForEmber({ + transport: TestFetchTransport, +}); export default class App extends Application { modulePrefix = config.modulePrefix; From 2fd13f895302c009f91375ed2b56196e62d45606 Mon Sep 17 00:00:00 2001 From: k-fish Date: Thu, 27 Aug 2020 22:40:38 -0700 Subject: [PATCH 06/16] Run ember tests in github actions --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e03b8b044bfc..8178178f3683 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -87,7 +87,7 @@ jobs: key: ${{ runner.os }}-${{ github.sha }} - run: yarn install - name: Unit Tests - run: yarn test --ignore="@sentry/ember" + run: yarn test - uses: codecov/codecov-action@v1 job_browserstack_test: From 4b8d2dec2199784732afba8b8198001e7abced30 Mon Sep 17 00:00:00 2001 From: k-fish Date: Thu, 27 Aug 2020 22:53:26 -0700 Subject: [PATCH 07/16] Re-introduce ember into scripts/test --- scripts/test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/test.sh b/scripts/test.sh index 723d87641ece..15c2a1f1c9b0 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -20,5 +20,5 @@ elif [[ "$(cut -d. -f1 <<< "$TRAVIS_NODE_VERSION")" -le 8 ]]; then else yarn install yarn build - yarn test --ignore="@sentry/ember" + yarn test fi From 2e4109ca2054928e2fe1be9fb32d868c8932cb48 Mon Sep 17 00:00:00 2001 From: k-fish Date: Wed, 2 Sep 2020 09:51:43 -0700 Subject: [PATCH 08/16] Add render runloop post transition as a child span --- packages/ember/addon/config.d.ts | 13 ++++++ packages/ember/addon/declarations.d.ts | 3 -- .../sentry-performance.ts | 40 ++++++++++++++++--- .../dummy/app/components/slow-loading-list.ts | 12 ++++++ .../components/slow-loading-list.hbs | 10 +++++ .../app/templates/slow-loading-route.hbs | 2 + 6 files changed, 71 insertions(+), 9 deletions(-) create mode 100644 packages/ember/addon/config.d.ts delete mode 100644 packages/ember/addon/declarations.d.ts create mode 100644 packages/ember/tests/dummy/app/components/slow-loading-list.ts create mode 100644 packages/ember/tests/dummy/app/templates/components/slow-loading-list.hbs diff --git a/packages/ember/addon/config.d.ts b/packages/ember/addon/config.d.ts new file mode 100644 index 000000000000..fcd365b38c73 --- /dev/null +++ b/packages/ember/addon/config.d.ts @@ -0,0 +1,13 @@ +declare module 'ember-get-config' { + import { BrowserOptions } from '@sentry/browser'; + type EmberSentryConfig = { + sentry: BrowserOptions; + disablePerformance: boolean; + transitionTimeout: number; + ignoreEmberOnErrorWarning: boolean; + }; + const config: { + '@sentry/ember': EmberSentryConfig; + }; + export default config; +} diff --git a/packages/ember/addon/declarations.d.ts b/packages/ember/addon/declarations.d.ts deleted file mode 100644 index fa430c829328..000000000000 --- a/packages/ember/addon/declarations.d.ts +++ /dev/null @@ -1,3 +0,0 @@ -declare module 'ember-get-config' { - export default object; -} \ No newline at end of file diff --git a/packages/ember/addon/instance-initializers/sentry-performance.ts b/packages/ember/addon/instance-initializers/sentry-performance.ts index 17e0fe960ccd..60fe08906f91 100644 --- a/packages/ember/addon/instance-initializers/sentry-performance.ts +++ b/packages/ember/addon/instance-initializers/sentry-performance.ts @@ -1,6 +1,9 @@ import ApplicationInstance from '@ember/application/instance'; +import Ember from 'ember'; +import { scheduleOnce } from '@ember/runloop'; import environmentConfig from 'ember-get-config'; import Sentry from '@sentry/ember'; +import { Integration } from '@sentry/types'; export function initialize(appInstance: ApplicationInstance): void { const config = environmentConfig['@sentry/ember']; @@ -8,8 +11,8 @@ export function initialize(appInstance: ApplicationInstance): void { return; } const performancePromise = instrumentForPerformance(appInstance); - if (Ember.Testing) { - window._sentryPerformanceLoad = performancePromise; + if (Ember.testing) { + (window)._sentryPerformanceLoad = performancePromise; } } @@ -27,15 +30,18 @@ export async function instrumentForPerformance(appInstance: ApplicationInstance) const sentryConfig = config.sentry; const tracing = await import('@sentry/tracing'); - const idleTimeout = config.transitionTimeout || 15000; + const idleTimeout = config.transitionTimeout || 5000; + + const existingIntegrations = (sentryConfig['integrations'] || []) as Integration[]; sentryConfig['integrations'] = [ - ...(sentryConfig['integrations'] || []), + ...existingIntegrations, new tracing.Integrations.BrowserTracing({ routingInstrumentation: (startTransaction, startTransactionOnPageLoad) => { const location = appInstance.lookup('router:main').location; const router = appInstance.lookup('service:router'); let activeTransaction: any; + let transitionSpan: any; const url = location && location.getURL && location.getURL(); if (startTransactionOnPageLoad && url) { const routeInfo = router.recognize(url); @@ -61,11 +67,33 @@ export async function instrumentForPerformance(appInstance: ApplicationInstance) 'routing.instrumentation': '@sentry/ember', }, }); + transitionSpan = activeTransaction.startChild({ + op: 'ember.transition', + description: `route:${fromRoute} -> route:${toRoute}`, + }); }); - router.on('routeDidChange', () => { - if (activeTransaction) { + router.on('routeDidChange', (transition: any) => { + const { toRoute } = getTransitionInformation(transition, router); + let renderSpan: any; + if (!transitionSpan || !activeTransaction) { + return; + } + transitionSpan.finish(); + + function startRenderSpan() { + renderSpan = activeTransaction.startChild({ + op: 'ember.runloop.render', + description: `Post transition render for route:${toRoute}`, + }); + } + + function finishRenderSpan() { + renderSpan.finish(); activeTransaction.finish(); } + + scheduleOnce('routerTransitions', null, startRenderSpan); + scheduleOnce('afterRender', null, finishRenderSpan); }); }, idleTimeout, diff --git a/packages/ember/tests/dummy/app/components/slow-loading-list.ts b/packages/ember/tests/dummy/app/components/slow-loading-list.ts new file mode 100644 index 000000000000..cd30d34acea0 --- /dev/null +++ b/packages/ember/tests/dummy/app/components/slow-loading-list.ts @@ -0,0 +1,12 @@ +import Component from '@ember/component'; +import { computed } from '@ember/object'; + +export default Component.extend({ + rowItems: computed('items', function() { + return new Array(parseInt(this.items)).fill(0).map((_, index) => { + return { + index: index + 1, + }; + }); + }), +}); diff --git a/packages/ember/tests/dummy/app/templates/components/slow-loading-list.hbs b/packages/ember/tests/dummy/app/templates/components/slow-loading-list.hbs new file mode 100644 index 000000000000..8dce1ce0c319 --- /dev/null +++ b/packages/ember/tests/dummy/app/templates/components/slow-loading-list.hbs @@ -0,0 +1,10 @@ +
+

Slow Loading List

+
+ {{#each this.rowItems as |rowItem|}} +
+ {{rowItem.index}} +
+ {{/each}} +
+
diff --git a/packages/ember/tests/dummy/app/templates/slow-loading-route.hbs b/packages/ember/tests/dummy/app/templates/slow-loading-route.hbs index d85ac939e59a..d59d6baccf12 100644 --- a/packages/ember/tests/dummy/app/templates/slow-loading-route.hbs +++ b/packages/ember/tests/dummy/app/templates/slow-loading-route.hbs @@ -2,3 +2,5 @@ + + From db396fa0fb3cf28ff4bcbd31e518acda1071048c Mon Sep 17 00:00:00 2001 From: k-fish Date: Wed, 2 Sep 2020 12:28:45 -0700 Subject: [PATCH 09/16] Add basic transaction tests to ensure router instrumentation works properly. Split off router instrumentation to allow for use in testing --- packages/ember/addon/config.d.ts | 2 +- .../sentry-performance.ts | 126 ++++++++++-------- .../acceptance/sentry-performance-test.js | 119 +++++++++++++++++ packages/ember/tests/constants.js | 1 + .../dummy/app/routes/slow-loading-route.js | 3 +- 5 files changed, 192 insertions(+), 59 deletions(-) create mode 100644 packages/ember/tests/acceptance/sentry-performance-test.js create mode 100644 packages/ember/tests/constants.js diff --git a/packages/ember/addon/config.d.ts b/packages/ember/addon/config.d.ts index fcd365b38c73..09cb21edbae2 100644 --- a/packages/ember/addon/config.d.ts +++ b/packages/ember/addon/config.d.ts @@ -2,9 +2,9 @@ declare module 'ember-get-config' { import { BrowserOptions } from '@sentry/browser'; type EmberSentryConfig = { sentry: BrowserOptions; - disablePerformance: boolean; transitionTimeout: number; ignoreEmberOnErrorWarning: boolean; + disablePerformance: boolean; }; const config: { '@sentry/ember': EmberSentryConfig; diff --git a/packages/ember/addon/instance-initializers/sentry-performance.ts b/packages/ember/addon/instance-initializers/sentry-performance.ts index 60fe08906f91..7c3a57cff57f 100644 --- a/packages/ember/addon/instance-initializers/sentry-performance.ts +++ b/packages/ember/addon/instance-initializers/sentry-performance.ts @@ -25,6 +25,72 @@ function getTransitionInformation(transition: any, router: any) { }; } +export function _instrumentEmberRouter(routerService: any, routerMain: any, config: typeof environmentConfig['@sentry/ember'], startTransaction: Function, startTransactionOnPageLoad?: boolean) { + const location = routerMain.location; + let activeTransaction: any; + let transitionSpan: any; + + const url = location && location.getURL && location.getURL(); + + if (Ember.testing) { + routerService._sentryInstrumented = true; + } + + if (startTransactionOnPageLoad && url) { + const routeInfo = routerService.recognize(url); + activeTransaction = startTransaction({ + name: `route:${routeInfo.name}`, + op: 'pageload', + tags: { + url, + toRoute: routeInfo.name, + 'routing.instrumentation': '@sentry/ember', + }, + }); + } + + routerService.on('routeWillChange', (transition: any) => { + const { fromRoute, toRoute } = getTransitionInformation(transition, routerService); + activeTransaction = startTransaction({ + name: `route:${toRoute}`, + op: 'navigation', + tags: { + fromRoute, + toRoute, + 'routing.instrumentation': '@sentry/ember', + }, + }); + transitionSpan = activeTransaction.startChild({ + op: 'ember.transition', + description: `route:${fromRoute} -> route:${toRoute}`, + }); + }); + + routerService.on('routeDidChange', (transition: any) => { + const { toRoute } = getTransitionInformation(transition, routerService); + let renderSpan: any; + if (!transitionSpan || !activeTransaction) { + return; + } + transitionSpan.finish(); + + function startRenderSpan() { + renderSpan = activeTransaction.startChild({ + op: 'ember.runloop.render', + description: `Post transition render for route:${toRoute}`, + }); + } + + function finishRenderSpan() { + renderSpan.finish(); + activeTransaction.finish(); + } + + scheduleOnce('routerTransitions', null, startRenderSpan); + scheduleOnce('afterRender', null, finishRenderSpan); + }); +} + export async function instrumentForPerformance(appInstance: ApplicationInstance) { const config = environmentConfig['@sentry/ember']; const sentryConfig = config.sentry; @@ -38,63 +104,9 @@ export async function instrumentForPerformance(appInstance: ApplicationInstance) ...existingIntegrations, new tracing.Integrations.BrowserTracing({ routingInstrumentation: (startTransaction, startTransactionOnPageLoad) => { - const location = appInstance.lookup('router:main').location; - const router = appInstance.lookup('service:router'); - let activeTransaction: any; - let transitionSpan: any; - const url = location && location.getURL && location.getURL(); - if (startTransactionOnPageLoad && url) { - const routeInfo = router.recognize(url); - activeTransaction = startTransaction({ - name: `route:${routeInfo.name}`, - op: 'pageload', - tags: { - url, - toRoute: routeInfo.name, - 'routing.instrumentation': '@sentry/ember', - }, - }); - } - - router.on('routeWillChange', (transition: any) => { - const { fromRoute, toRoute } = getTransitionInformation(transition, router); - activeTransaction = startTransaction({ - name: `route:${toRoute}`, - op: 'navigation', - tags: { - fromRoute, - toRoute, - 'routing.instrumentation': '@sentry/ember', - }, - }); - transitionSpan = activeTransaction.startChild({ - op: 'ember.transition', - description: `route:${fromRoute} -> route:${toRoute}`, - }); - }); - router.on('routeDidChange', (transition: any) => { - const { toRoute } = getTransitionInformation(transition, router); - let renderSpan: any; - if (!transitionSpan || !activeTransaction) { - return; - } - transitionSpan.finish(); - - function startRenderSpan() { - renderSpan = activeTransaction.startChild({ - op: 'ember.runloop.render', - description: `Post transition render for route:${toRoute}`, - }); - } - - function finishRenderSpan() { - renderSpan.finish(); - activeTransaction.finish(); - } - - scheduleOnce('routerTransitions', null, startRenderSpan); - scheduleOnce('afterRender', null, finishRenderSpan); - }); + const routerMain = appInstance.lookup('router:main'); + const routerService = appInstance.lookup('service:router'); + _instrumentEmberRouter(routerService, routerMain, config, startTransaction, startTransactionOnPageLoad) }, idleTimeout, }), diff --git a/packages/ember/tests/acceptance/sentry-performance-test.js b/packages/ember/tests/acceptance/sentry-performance-test.js new file mode 100644 index 000000000000..091f19760ef3 --- /dev/null +++ b/packages/ember/tests/acceptance/sentry-performance-test.js @@ -0,0 +1,119 @@ +import { test, module } from 'qunit'; +import { setupApplicationTest } from 'ember-qunit'; +import { find, click, visit } from '@ember/test-helpers'; +import Ember from 'ember'; +import sinon from 'sinon'; +import { _instrumentEmberRouter } from '@sentry/ember/instance-initializers/sentry-performance'; +import { startTransaction } from '@sentry/browser'; +import { SLOW_TRANSITION_WAIT } from '../constants'; + +function getTestSentryTransactions() { + return window._sentryTestEvents.filter(event => event['type'] === 'transaction'); +} + +function assertSentryTransactionCount(assert, count) { + assert.equal(getTestSentryTransactions().length, count, 'Check correct number of Sentry events were sent'); +} + +function assertSentryCall(assert, callNumber, options) { + const sentryTestEvents = getTestSentryTransactions(); + + const event = sentryTestEvents[callNumber]; + assert.equal(event.spans.length, options.spanCount); + assert.equal(event.transaction, options.transaction); + assert.equal(event.tags.fromRoute, options.tags.fromRoute); + assert.equal(event.tags.toRoute, options.tags.toRoute); + + if (options.durationCheck) { + const duration = (event.timestamp - event.start_timestamp) * 1000; + assert.ok(options.durationCheck(duration), `duration (${duration}ms) didn't pass duration check`); + } +} + +module('Acceptance | Sentry Transactions', function(hooks) { + setupApplicationTest(hooks); + + hooks.beforeEach(async function() { + await window._sentryPerformanceLoad; + window._sentryTestEvents = []; + const errorMessages = []; + this.errorMessages = errorMessages; + + const routerMain = this.owner.lookup('router:main'); + const routerService = this.owner.lookup('service:router'); + + if (!routerService._sentryInstrumented) { + _instrumentEmberRouter(routerService, routerMain, {}, startTransaction); + } + + /** + * Stub out fetch function to assert on Sentry calls. + */ + this.fetchStub = sinon.stub(window, 'fetch'); + + /** + * Stops global test suite failures from unhandled rejections and allows assertion on them + */ + this.qunitOnUnhandledRejection = sinon.stub(QUnit, 'onUnhandledRejection'); + + QUnit.onError = function({ message }) { + errorMessages.push(message.split('Error: ')[1]); + return true; + }; + + Ember.onerror = function(...args) { + const [error] = args; + errorMessages.push(error.message); + throw error; + }; + + this._windowOnError = window.onerror; + + /** + * Will collect errors when run via testem in cli + */ + window.onerror = function(error, ...args) { + errorMessages.push(error.split('Error: ')[1]); + if (this._windowOnError) { + return this._windowOnError(error, ...args); + } + }; + }); + + hooks.afterEach(function() { + this.fetchStub.restore(); + this.qunitOnUnhandledRejection.restore(); + window.onerror = this._windowOnError; + }); + + test('Test transaction', async function(assert) { + await visit('/tracing'); + assertSentryTransactionCount(assert, 1); + assertSentryCall(assert, 0, { + spanCount: 2, + transaction: 'route:tracing', + tags: { + fromRoute: undefined, + toRoute: 'tracing', + }, + }); + }); + + test('Test navigating to slow route', async function(assert) { + await visit('/tracing'); + const button = find('[data-test-button="Transition to slow loading route"]'); + + await click(button); + + assertSentryTransactionCount(assert, 2); + assertSentryCall(assert, 1, { + spanCount: 2, + transaction: 'route:slow-loading-route', + durationCheck: duration => duration > SLOW_TRANSITION_WAIT, + tags: { + fromRoute: 'tracing', + toRoute: 'slow-loading-route', + }, + }); + }); +}); diff --git a/packages/ember/tests/constants.js b/packages/ember/tests/constants.js new file mode 100644 index 000000000000..5b1d70dc2722 --- /dev/null +++ b/packages/ember/tests/constants.js @@ -0,0 +1 @@ +export const SLOW_TRANSITION_WAIT = 3000; // Make dummy route wait 3000ms diff --git a/packages/ember/tests/dummy/app/routes/slow-loading-route.js b/packages/ember/tests/dummy/app/routes/slow-loading-route.js index 5fc9c444cb70..7b52de1ef0e7 100644 --- a/packages/ember/tests/dummy/app/routes/slow-loading-route.js +++ b/packages/ember/tests/dummy/app/routes/slow-loading-route.js @@ -1,8 +1,9 @@ import Route from '@ember/routing/route'; import timeout from '../helpers/utils'; +import { SLOW_TRANSITION_WAIT } from 'dummy/tests/constants'; export default class SlowLoadingRoute extends Route { model() { - return timeout(3000); + return timeout(SLOW_TRANSITION_WAIT); } } From afc6a8cd31826d81355f8dfb2d73e552cebfb289 Mon Sep 17 00:00:00 2001 From: k-fish Date: Wed, 2 Sep 2020 14:46:57 -0700 Subject: [PATCH 10/16] Add route instrumentation --- packages/ember/addon/config.d.ts | 1 + packages/ember/addon/index.ts | 35 +++++++++++++++++++ .../sentry-performance.ts | 5 +++ .../acceptance/sentry-performance-test.js | 2 +- .../dummy/app/routes/slow-loading-route.js | 28 +++++++++++---- packages/ember/tests/{ => dummy}/constants.js | 0 6 files changed, 64 insertions(+), 7 deletions(-) rename packages/ember/tests/{ => dummy}/constants.js (100%) diff --git a/packages/ember/addon/config.d.ts b/packages/ember/addon/config.d.ts index 09cb21edbae2..193d08f2db93 100644 --- a/packages/ember/addon/config.d.ts +++ b/packages/ember/addon/config.d.ts @@ -5,6 +5,7 @@ declare module 'ember-get-config' { transitionTimeout: number; ignoreEmberOnErrorWarning: boolean; disablePerformance: boolean; + disablePostTransitionRender: boolean; }; const config: { '@sentry/ember': EmberSentryConfig; diff --git a/packages/ember/addon/index.ts b/packages/ember/addon/index.ts index 953c3ef56b53..cc799441278a 100644 --- a/packages/ember/addon/index.ts +++ b/packages/ember/addon/index.ts @@ -3,6 +3,7 @@ import { addGlobalEventProcessor, SDK_VERSION, BrowserOptions } from '@sentry/br import environmentConfig from 'ember-get-config'; import { next } from '@ember/runloop'; +import Route from '@ember/routing/route'; import { assert, warn, runInDebug } from '@ember/debug'; import Ember from 'ember'; @@ -37,6 +38,40 @@ export function InitSentryForEmber(_runtimeConfig: BrowserOptions | undefined) { }); } +const getCurrentTransaction = () => { + return Sentry.getCurrentHub() + ?.getScope() + ?.getTransaction(); +} + +const instrumentFunction = async (op: string, description: string, fn: Function, args: any) => { + const currentTransaction = getCurrentTransaction(); + const span = currentTransaction?.startChild({ op, description }); + const result = await fn(...args); + span?.finish(); + return result; +}; + +export const InstrumentRoutePerformance = (BaseRoute: typeof Route) => { + return class InstrumentedRoute extends BaseRoute { + beforeModel(...args: any[]) { + return instrumentFunction('ember.route.beforeModel', (this).fullRouteName, super.beforeModel, args); + } + + async model(...args: any[]) { + return instrumentFunction('ember.route.model', (this).fullRouteName, super.model, args); + } + + async afterModel(...args: any[]) { + return instrumentFunction('ember.route.afterModel', (this).fullRouteName, super.afterModel, args); + } + + async setupController(...args: any[]) { + return instrumentFunction('ember.route.setupController', (this).fullRouteName, super.setupController, args); + } + }; +}; + function createEmberEventProcessor(): void { if (addGlobalEventProcessor) { addGlobalEventProcessor(event => { diff --git a/packages/ember/addon/instance-initializers/sentry-performance.ts b/packages/ember/addon/instance-initializers/sentry-performance.ts index 7c3a57cff57f..4842b6b727aa 100644 --- a/packages/ember/addon/instance-initializers/sentry-performance.ts +++ b/packages/ember/addon/instance-initializers/sentry-performance.ts @@ -26,6 +26,7 @@ function getTransitionInformation(transition: any, router: any) { } export function _instrumentEmberRouter(routerService: any, routerMain: any, config: typeof environmentConfig['@sentry/ember'], startTransaction: Function, startTransactionOnPageLoad?: boolean) { + const { disablePostTransitionRender } = config; const location = routerMain.location; let activeTransaction: any; let transitionSpan: any; @@ -74,6 +75,10 @@ export function _instrumentEmberRouter(routerService: any, routerMain: any, conf } transitionSpan.finish(); + if (disablePostTransitionRender) { + activeTransaction.finish(); + } + function startRenderSpan() { renderSpan = activeTransaction.startChild({ op: 'ember.runloop.render', diff --git a/packages/ember/tests/acceptance/sentry-performance-test.js b/packages/ember/tests/acceptance/sentry-performance-test.js index 091f19760ef3..8b1450b18f5e 100644 --- a/packages/ember/tests/acceptance/sentry-performance-test.js +++ b/packages/ember/tests/acceptance/sentry-performance-test.js @@ -5,7 +5,7 @@ import Ember from 'ember'; import sinon from 'sinon'; import { _instrumentEmberRouter } from '@sentry/ember/instance-initializers/sentry-performance'; import { startTransaction } from '@sentry/browser'; -import { SLOW_TRANSITION_WAIT } from '../constants'; +import { SLOW_TRANSITION_WAIT } from '../dummy/constants'; function getTestSentryTransactions() { return window._sentryTestEvents.filter(event => event['type'] === 'transaction'); diff --git a/packages/ember/tests/dummy/app/routes/slow-loading-route.js b/packages/ember/tests/dummy/app/routes/slow-loading-route.js index 7b52de1ef0e7..9dcbdb365443 100644 --- a/packages/ember/tests/dummy/app/routes/slow-loading-route.js +++ b/packages/ember/tests/dummy/app/routes/slow-loading-route.js @@ -1,9 +1,25 @@ import Route from '@ember/routing/route'; import timeout from '../helpers/utils'; -import { SLOW_TRANSITION_WAIT } from 'dummy/tests/constants'; +import { InstrumentRoutePerformance } from '@sentry/ember'; -export default class SlowLoadingRoute extends Route { - model() { - return timeout(SLOW_TRANSITION_WAIT); - } -} +const SLOW_TRANSITION_WAIT = 3000; + +export default InstrumentRoutePerformance( + class _SlowLoadingRoute extends Route { + beforeModel() { + return timeout(SLOW_TRANSITION_WAIT / 3); + } + + model() { + return timeout(SLOW_TRANSITION_WAIT / 3); + } + + afterModel() { + return timeout(SLOW_TRANSITION_WAIT / 3); + } + + setupController() { + super.setupController(); + } + }, +); diff --git a/packages/ember/tests/constants.js b/packages/ember/tests/dummy/constants.js similarity index 100% rename from packages/ember/tests/constants.js rename to packages/ember/tests/dummy/constants.js From 161c364c02095c38d0c9af293d17be67c6e8ab93 Mon Sep 17 00:00:00 2001 From: k-fish Date: Wed, 2 Sep 2020 14:55:02 -0700 Subject: [PATCH 11/16] Fix description --- .../ember/addon/instance-initializers/sentry-performance.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ember/addon/instance-initializers/sentry-performance.ts b/packages/ember/addon/instance-initializers/sentry-performance.ts index 4842b6b727aa..da80bf1fb4bb 100644 --- a/packages/ember/addon/instance-initializers/sentry-performance.ts +++ b/packages/ember/addon/instance-initializers/sentry-performance.ts @@ -82,7 +82,7 @@ export function _instrumentEmberRouter(routerService: any, routerMain: any, conf function startRenderSpan() { renderSpan = activeTransaction.startChild({ op: 'ember.runloop.render', - description: `Post transition render for route:${toRoute}`, + description: `post-transition render route:${toRoute}`, }); } From 56bf8d2434b2205ef1ebafc9a31c0b5a604754eb Mon Sep 17 00:00:00 2001 From: k-fish Date: Wed, 2 Sep 2020 15:36:52 -0700 Subject: [PATCH 12/16] Update readme and cleanup --- packages/ember/README.md | 28 +++++++++++++++++++ packages/ember/addon/index.ts | 3 +- .../dummy/app/routes/slow-loading-route.js | 6 ++-- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/packages/ember/README.md b/packages/ember/README.md index 4cfb1f3f77cf..60e3dd09e041 100644 --- a/packages/ember/README.md +++ b/packages/ember/README.md @@ -62,6 +62,34 @@ Aside from configuration passed from this addon into `@sentry/browser` via the ` sentry: ... // See sentry-javascript configuration https://docs.sentry.io/error-reporting/configuration/?platform=javascript }; ``` +#### Disabling Performance + +`@sentry/ember` captures performance by default, if you would like to disable the automatic performance instrumentation, you can add the following to your `config/environment.js`: + +```javascript + ENV['@sentry/ember'] = { + disablePerformance: true, // Will disable automatic instrumentation of performance. Manual instrumentation will still be sent. + sentry: ... // See sentry-javascript configuration https://docs.sentry.io/error-reporting/configuration/?platform=javascript + }; +``` + +### Performance +#### Routes +If you would like to capture `beforeModel`, `model`, `afterModel` and `setupController` times for one of your routes, +you can import `instrumentRoutePerformance` and wrap your route with it. + +```javascript +import Route from '@ember/routing/route'; +import { instrumentRoutePerformance } from '@sentry/ember'; + +export default instrumentRoutePerformance( + class MyRoute extends Route { + model() { + //... + } + } +); +``` ### Supported Versions diff --git a/packages/ember/addon/index.ts b/packages/ember/addon/index.ts index cc799441278a..e6544ee71a52 100644 --- a/packages/ember/addon/index.ts +++ b/packages/ember/addon/index.ts @@ -3,7 +3,6 @@ import { addGlobalEventProcessor, SDK_VERSION, BrowserOptions } from '@sentry/br import environmentConfig from 'ember-get-config'; import { next } from '@ember/runloop'; -import Route from '@ember/routing/route'; import { assert, warn, runInDebug } from '@ember/debug'; import Ember from 'ember'; @@ -52,7 +51,7 @@ const instrumentFunction = async (op: string, description: string, fn: Function, return result; }; -export const InstrumentRoutePerformance = (BaseRoute: typeof Route) => { +export const instrumentRoutePerformance = (BaseRoute: any) => { return class InstrumentedRoute extends BaseRoute { beforeModel(...args: any[]) { return instrumentFunction('ember.route.beforeModel', (this).fullRouteName, super.beforeModel, args); diff --git a/packages/ember/tests/dummy/app/routes/slow-loading-route.js b/packages/ember/tests/dummy/app/routes/slow-loading-route.js index 9dcbdb365443..678520c9c202 100644 --- a/packages/ember/tests/dummy/app/routes/slow-loading-route.js +++ b/packages/ember/tests/dummy/app/routes/slow-loading-route.js @@ -1,11 +1,11 @@ import Route from '@ember/routing/route'; import timeout from '../helpers/utils'; -import { InstrumentRoutePerformance } from '@sentry/ember'; +import { instrumentRoutePerformance } from '@sentry/ember'; const SLOW_TRANSITION_WAIT = 3000; -export default InstrumentRoutePerformance( - class _SlowLoadingRoute extends Route { +export default instrumentRoutePerformance( + class SlowLoadingRoute extends Route { beforeModel() { return timeout(SLOW_TRANSITION_WAIT / 3); } From 7048f9619409cabbdb415c64329148d42f8d4668 Mon Sep 17 00:00:00 2001 From: k-fish Date: Wed, 2 Sep 2020 15:38:11 -0700 Subject: [PATCH 13/16] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cfde2793af1..e5047247ad1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +- [ember] feat: Add performance instrumentation for routes (#2784) ## 5.22.3 From bc9b41d0b7062755b2685c3d982bbbd3684f206a Mon Sep 17 00:00:00 2001 From: k-fish Date: Wed, 2 Sep 2020 15:39:06 -0700 Subject: [PATCH 14/16] Fix bad sed replacement --- packages/ember/package.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/ember/package.json b/packages/ember/package.json index 895b03eb0e8c..7bf3b7868ead 100644 --- a/packages/ember/package.json +++ b/packages/ember/package.json @@ -30,10 +30,10 @@ "pack": "npm pack" }, "dependencies": { - "@sentry/browser": "5.32.3", - "@sentry/tracing": "5.32.3", - "@sentry/types": "5.32.3", - "@sentry/utils": "5.32.3", + "@sentry/browser": "5.22.3", + "@sentry/tracing": "5.22.3", + "@sentry/types": "5.22.3", + "@sentry/utils": "5.22.3", "ember-auto-import": "^1.6.0", "ember-cli-babel": "^7.20.5", "ember-cli-htmlbars": "^5.1.2", From e0b31cc9440a1bffff39d27f24f03280e9a5bb9a Mon Sep 17 00:00:00 2001 From: k-fish Date: Wed, 2 Sep 2020 18:09:53 -0700 Subject: [PATCH 15/16] Clean up tests and lint --- packages/ember/tests/acceptance/sentry-performance-test.js | 3 ++- .../ember/tests/dummy/app/templates/slow-loading-route.hbs | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/ember/tests/acceptance/sentry-performance-test.js b/packages/ember/tests/acceptance/sentry-performance-test.js index 8b1450b18f5e..312fc1a8d3cb 100644 --- a/packages/ember/tests/acceptance/sentry-performance-test.js +++ b/packages/ember/tests/acceptance/sentry-performance-test.js @@ -5,7 +5,8 @@ import Ember from 'ember'; import sinon from 'sinon'; import { _instrumentEmberRouter } from '@sentry/ember/instance-initializers/sentry-performance'; import { startTransaction } from '@sentry/browser'; -import { SLOW_TRANSITION_WAIT } from '../dummy/constants'; + +const SLOW_TRANSITION_WAIT = 3000; function getTestSentryTransactions() { return window._sentryTestEvents.filter(event => event['type'] === 'transaction'); diff --git a/packages/ember/tests/dummy/app/templates/slow-loading-route.hbs b/packages/ember/tests/dummy/app/templates/slow-loading-route.hbs index d59d6baccf12..a1177d563500 100644 --- a/packages/ember/tests/dummy/app/templates/slow-loading-route.hbs +++ b/packages/ember/tests/dummy/app/templates/slow-loading-route.hbs @@ -3,4 +3,4 @@ Go Back - + From 2b72278425a95b85975ccd2ee6f4c614e47c9699 Mon Sep 17 00:00:00 2001 From: k-fish Date: Tue, 8 Sep 2020 10:19:00 -0700 Subject: [PATCH 16/16] Remove default import of Sentry --- packages/ember/addon/index.ts | 7 +++---- .../instance-initializers/sentry-performance.ts | 12 +++++++++--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/ember/addon/index.ts b/packages/ember/addon/index.ts index e6544ee71a52..1069225c4934 100644 --- a/packages/ember/addon/index.ts +++ b/packages/ember/addon/index.ts @@ -39,9 +39,9 @@ export function InitSentryForEmber(_runtimeConfig: BrowserOptions | undefined) { const getCurrentTransaction = () => { return Sentry.getCurrentHub() - ?.getScope() - ?.getTransaction(); -} + ?.getScope() + ?.getTransaction(); +}; const instrumentFunction = async (op: string, description: string, fn: Function, args: any) => { const currentTransaction = getCurrentTransaction(); @@ -92,5 +92,4 @@ function createEmberEventProcessor(): void { } } -export default Sentry; export * from '@sentry/browser'; diff --git a/packages/ember/addon/instance-initializers/sentry-performance.ts b/packages/ember/addon/instance-initializers/sentry-performance.ts index da80bf1fb4bb..f76b605a8fb3 100644 --- a/packages/ember/addon/instance-initializers/sentry-performance.ts +++ b/packages/ember/addon/instance-initializers/sentry-performance.ts @@ -2,7 +2,7 @@ import ApplicationInstance from '@ember/application/instance'; import Ember from 'ember'; import { scheduleOnce } from '@ember/runloop'; import environmentConfig from 'ember-get-config'; -import Sentry from '@sentry/ember'; +import Sentry from '@sentry/browser'; import { Integration } from '@sentry/types'; export function initialize(appInstance: ApplicationInstance): void { @@ -25,7 +25,13 @@ function getTransitionInformation(transition: any, router: any) { }; } -export function _instrumentEmberRouter(routerService: any, routerMain: any, config: typeof environmentConfig['@sentry/ember'], startTransaction: Function, startTransactionOnPageLoad?: boolean) { +export function _instrumentEmberRouter( + routerService: any, + routerMain: any, + config: typeof environmentConfig['@sentry/ember'], + startTransaction: Function, + startTransactionOnPageLoad?: boolean, +) { const { disablePostTransitionRender } = config; const location = routerMain.location; let activeTransaction: any; @@ -111,7 +117,7 @@ export async function instrumentForPerformance(appInstance: ApplicationInstance) routingInstrumentation: (startTransaction, startTransactionOnPageLoad) => { const routerMain = appInstance.lookup('router:main'); const routerService = appInstance.lookup('service:router'); - _instrumentEmberRouter(routerService, routerMain, config, startTransaction, startTransactionOnPageLoad) + _instrumentEmberRouter(routerService, routerMain, config, startTransaction, startTransactionOnPageLoad); }, idleTimeout, }),