diff --git a/dev-packages/e2e-tests/test-applications/ember-classic/tests/performance.test.ts b/dev-packages/e2e-tests/test-applications/ember-classic/tests/performance.test.ts index 11006836751b..ef86faed21b8 100644 --- a/dev-packages/e2e-tests/test-applications/ember-classic/tests/performance.test.ts +++ b/dev-packages/e2e-tests/test-applications/ember-classic/tests/performance.test.ts @@ -168,10 +168,14 @@ test('captures correct spans for navigation', async ({ page }) => { }); const transitionSpans = spans.filter(span => span.op === 'ui.ember.transition'); - const beforeModelSpans = spans.filter(span => span.op === 'ui.ember.route.before_model'); - const modelSpans = spans.filter(span => span.op === 'ui.ember.route.model'); - const afterModelSpans = spans.filter(span => span.op === 'ui.ember.route.after_model'); - const renderSpans = spans.filter(span => span.op === 'ui.ember.runloop.render'); + const beforeModelSpans = spans.filter( + span => span.op === 'function' && span.data?.['ember.route.hook'] === 'beforeModel', + ); + const modelSpans = spans.filter(span => span.op === 'function' && span.data?.['ember.route.hook'] === 'model'); + const afterModelSpans = spans.filter( + span => span.op === 'function' && span.data?.['ember.route.hook'] === 'afterModel', + ); + const renderSpans = spans.filter(span => span.op === 'ui.task' && span.data?.['ember.runloop.queue'] === 'render'); expect(transitionSpans).toHaveLength(1); @@ -202,12 +206,14 @@ test('captures correct spans for navigation', async ({ page }) => { expect(beforeModelSpans).toEqual([ { data: { - 'sentry.op': 'ui.ember.route.before_model', + 'code.function.name': 'beforeModel', + 'ember.route.hook': 'beforeModel', + 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', 'sentry.source': 'custom', }, description: 'slow-loading-route', - op: 'ui.ember.route.before_model', + op: 'function', origin: 'auto.ui.ember', status: 'ok', parent_span_id: spanId, @@ -218,12 +224,14 @@ test('captures correct spans for navigation', async ({ page }) => { }, { data: { - 'sentry.op': 'ui.ember.route.before_model', + 'code.function.name': 'beforeModel', + 'ember.route.hook': 'beforeModel', + 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', 'sentry.source': 'custom', }, description: 'slow-loading-route.index', - op: 'ui.ember.route.before_model', + op: 'function', origin: 'auto.ui.ember', status: 'ok', parent_span_id: spanId, @@ -237,12 +245,14 @@ test('captures correct spans for navigation', async ({ page }) => { expect(modelSpans).toEqual([ { data: { - 'sentry.op': 'ui.ember.route.model', + 'code.function.name': 'model', + 'ember.route.hook': 'model', + 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', 'sentry.source': 'custom', }, description: 'slow-loading-route', - op: 'ui.ember.route.model', + op: 'function', origin: 'auto.ui.ember', status: 'ok', parent_span_id: spanId, @@ -253,12 +263,14 @@ test('captures correct spans for navigation', async ({ page }) => { }, { data: { - 'sentry.op': 'ui.ember.route.model', + 'code.function.name': 'model', + 'ember.route.hook': 'model', + 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', 'sentry.source': 'custom', }, description: 'slow-loading-route.index', - op: 'ui.ember.route.model', + op: 'function', origin: 'auto.ui.ember', status: 'ok', parent_span_id: spanId, @@ -272,12 +284,14 @@ test('captures correct spans for navigation', async ({ page }) => { expect(afterModelSpans).toEqual([ { data: { - 'sentry.op': 'ui.ember.route.after_model', + 'code.function.name': 'afterModel', + 'ember.route.hook': 'afterModel', + 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', 'sentry.source': 'custom', }, description: 'slow-loading-route', - op: 'ui.ember.route.after_model', + op: 'function', origin: 'auto.ui.ember', status: 'ok', parent_span_id: spanId, @@ -288,12 +302,14 @@ test('captures correct spans for navigation', async ({ page }) => { }, { data: { - 'sentry.op': 'ui.ember.route.after_model', + 'code.function.name': 'afterModel', + 'ember.route.hook': 'afterModel', + 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', 'sentry.source': 'custom', }, description: 'slow-loading-route.index', - op: 'ui.ember.route.after_model', + op: 'function', origin: 'auto.ui.ember', status: 'ok', parent_span_id: spanId, @@ -306,11 +322,12 @@ test('captures correct spans for navigation', async ({ page }) => { expect(renderSpans).toContainEqual({ data: { - 'sentry.op': 'ui.ember.runloop.render', + 'ember.runloop.queue': 'render', + 'sentry.op': 'ui.task', 'sentry.origin': 'auto.ui.ember', }, description: 'runloop', - op: 'ui.ember.runloop.render', + op: 'ui.task', origin: 'auto.ui.ember', status: 'ok', parent_span_id: spanId, diff --git a/dev-packages/e2e-tests/test-applications/ember-embroider/tests/performance.test.ts b/dev-packages/e2e-tests/test-applications/ember-embroider/tests/performance.test.ts index dbdee99717f5..cbac7d30e8c2 100644 --- a/dev-packages/e2e-tests/test-applications/ember-embroider/tests/performance.test.ts +++ b/dev-packages/e2e-tests/test-applications/ember-embroider/tests/performance.test.ts @@ -168,10 +168,14 @@ test('captures correct spans for navigation', async ({ page }) => { }); const transitionSpans = spans.filter(span => span.op === 'ui.ember.transition'); - const beforeModelSpans = spans.filter(span => span.op === 'ui.ember.route.before_model'); - const modelSpans = spans.filter(span => span.op === 'ui.ember.route.model'); - const afterModelSpans = spans.filter(span => span.op === 'ui.ember.route.after_model'); - const renderSpans = spans.filter(span => span.op === 'ui.ember.runloop.render'); + const beforeModelSpans = spans.filter( + span => span.op === 'function' && span.data?.['ember.route.hook'] === 'beforeModel', + ); + const modelSpans = spans.filter(span => span.op === 'function' && span.data?.['ember.route.hook'] === 'model'); + const afterModelSpans = spans.filter( + span => span.op === 'function' && span.data?.['ember.route.hook'] === 'afterModel', + ); + const renderSpans = spans.filter(span => span.op === 'ui.task' && span.data?.['ember.runloop.queue'] === 'render'); expect(transitionSpans).toHaveLength(1); @@ -202,12 +206,14 @@ test('captures correct spans for navigation', async ({ page }) => { expect(beforeModelSpans).toEqual([ { data: { - 'sentry.op': 'ui.ember.route.before_model', + 'code.function.name': 'beforeModel', + 'ember.route.hook': 'beforeModel', + 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', 'sentry.source': 'custom', }, description: 'slow-loading-route', - op: 'ui.ember.route.before_model', + op: 'function', origin: 'auto.ui.ember', status: 'ok', parent_span_id: spanId, @@ -218,12 +224,14 @@ test('captures correct spans for navigation', async ({ page }) => { }, { data: { - 'sentry.op': 'ui.ember.route.before_model', + 'code.function.name': 'beforeModel', + 'ember.route.hook': 'beforeModel', + 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', 'sentry.source': 'custom', }, description: 'slow-loading-route.index', - op: 'ui.ember.route.before_model', + op: 'function', origin: 'auto.ui.ember', status: 'ok', parent_span_id: spanId, @@ -237,12 +245,14 @@ test('captures correct spans for navigation', async ({ page }) => { expect(modelSpans).toEqual([ { data: { - 'sentry.op': 'ui.ember.route.model', + 'code.function.name': 'model', + 'ember.route.hook': 'model', + 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', 'sentry.source': 'custom', }, description: 'slow-loading-route', - op: 'ui.ember.route.model', + op: 'function', origin: 'auto.ui.ember', status: 'ok', parent_span_id: spanId, @@ -253,12 +263,14 @@ test('captures correct spans for navigation', async ({ page }) => { }, { data: { - 'sentry.op': 'ui.ember.route.model', + 'code.function.name': 'model', + 'ember.route.hook': 'model', + 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', 'sentry.source': 'custom', }, description: 'slow-loading-route.index', - op: 'ui.ember.route.model', + op: 'function', origin: 'auto.ui.ember', status: 'ok', parent_span_id: spanId, @@ -272,12 +284,14 @@ test('captures correct spans for navigation', async ({ page }) => { expect(afterModelSpans).toEqual([ { data: { - 'sentry.op': 'ui.ember.route.after_model', + 'code.function.name': 'afterModel', + 'ember.route.hook': 'afterModel', + 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', 'sentry.source': 'custom', }, description: 'slow-loading-route', - op: 'ui.ember.route.after_model', + op: 'function', origin: 'auto.ui.ember', status: 'ok', parent_span_id: spanId, @@ -288,12 +302,14 @@ test('captures correct spans for navigation', async ({ page }) => { }, { data: { - 'sentry.op': 'ui.ember.route.after_model', + 'code.function.name': 'afterModel', + 'ember.route.hook': 'afterModel', + 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', 'sentry.source': 'custom', }, description: 'slow-loading-route.index', - op: 'ui.ember.route.after_model', + op: 'function', origin: 'auto.ui.ember', status: 'ok', parent_span_id: spanId, @@ -306,11 +322,12 @@ test('captures correct spans for navigation', async ({ page }) => { expect(renderSpans).toContainEqual({ data: { - 'sentry.op': 'ui.ember.runloop.render', + 'ember.runloop.queue': 'render', + 'sentry.op': 'ui.task', 'sentry.origin': 'auto.ui.ember', }, description: 'runloop', - op: 'ui.ember.runloop.render', + op: 'ui.task', origin: 'auto.ui.ember', status: 'ok', parent_span_id: spanId, diff --git a/packages/ember/addon/index.ts b/packages/ember/addon/index.ts index c6da31c431f0..fedf69a0214d 100644 --- a/packages/ember/addon/index.ts +++ b/packages/ember/addon/index.ts @@ -4,6 +4,8 @@ import { assert } from '@ember/debug'; import type Route from '@ember/routing/route'; import { getOwnConfig } from '@embroider/macros'; +import { CODE_FUNCTION_NAME, SENTRY_OP } from '@sentry/conventions/attributes'; +import { GENERAL_FUNCTION_SPAN_OP } from '@sentry/conventions/op'; import type { BrowserOptions } from '@sentry/browser'; import { startSpan } from '@sentry/browser'; import * as Sentry from '@sentry/browser'; @@ -54,7 +56,7 @@ type RouteConstructor = new (...args: ConstructorParameters) => Ro export const instrumentRoutePerformance = (BaseRoute: T): T => { // eslint-disable-next-line @typescript-eslint/no-explicit-any const instrumentFunction = async any>( - op: string, + hookName: string, name: string, fn: X, args: Parameters, @@ -65,8 +67,10 @@ export const instrumentRoutePerformance = (BaseRoute attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source, [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.ember', + [SENTRY_OP]: GENERAL_FUNCTION_SPAN_OP, + [CODE_FUNCTION_NAME]: hookName, + 'ember.route.hook': hookName, }, - op, name, onlyIfParent: true, }, @@ -82,32 +86,20 @@ export const instrumentRoutePerformance = (BaseRoute // @ts-expect-error TS2545 We do not need to redefine a constructor here [routeName]: class extends BaseRoute { public beforeModel(...args: unknown[]): void | Promise { - return instrumentFunction( - 'ui.ember.route.before_model', - this.fullRouteName, - super.beforeModel.bind(this), - args, - 'custom', - ); + return instrumentFunction('beforeModel', this.fullRouteName, super.beforeModel.bind(this), args, 'custom'); } public async model(...args: unknown[]): Promise { - return instrumentFunction('ui.ember.route.model', this.fullRouteName, super.model.bind(this), args, 'custom'); + return instrumentFunction('model', this.fullRouteName, super.model.bind(this), args, 'custom'); } public afterModel(...args: unknown[]): void | Promise { - return instrumentFunction( - 'ui.ember.route.after_model', - this.fullRouteName, - super.afterModel.bind(this), - args, - 'custom', - ); + return instrumentFunction('afterModel', this.fullRouteName, super.afterModel.bind(this), args, 'custom'); } public setupController(...args: unknown[]): void | Promise { return instrumentFunction( - 'ui.ember.route.setup_controller', + 'setupController', this.fullRouteName, super.setupController.bind(this), args, diff --git a/packages/ember/addon/utils/instrumentEmberGlobals.ts b/packages/ember/addon/utils/instrumentEmberGlobals.ts index 02e365e27c23..291c18faf56b 100644 --- a/packages/ember/addon/utils/instrumentEmberGlobals.ts +++ b/packages/ember/addon/utils/instrumentEmberGlobals.ts @@ -1,6 +1,8 @@ import { subscribe } from '@ember/instrumentation'; import { scheduleOnce } from '@ember/runloop'; import type { EmberRunQueues } from '@ember/runloop/-private/types'; +import { SENTRY_OP, UI_COMPONENT_NAME } from '@sentry/conventions/attributes'; +import { BROWSER_UI_RENDER_SPAN_OP, BROWSER_UI_TASK_SPAN_OP, GENERAL_FUNCTION_SPAN_OP } from '@sentry/conventions/op'; import { getActiveSpan, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan } from '@sentry/browser'; import type { Span } from '@sentry/core'; import { browserPerformanceTimeOrigin, timestampInSeconds } from '@sentry/core'; @@ -89,10 +91,11 @@ function _instrumentEmberRunloop(config: { minimumRunloopQueueDuration?: number if ((now - currentQueueStart) * 1000 >= minQueueDuration) { startInactiveSpan({ attributes: { + [SENTRY_OP]: BROWSER_UI_TASK_SPAN_OP, [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.ember', + 'ember.runloop.queue': queue, }, name: 'runloop', - op: `ui.ember.runloop.${queue}`, startTime: currentQueueStart, onlyIfParent: true, })?.end(now); @@ -147,13 +150,16 @@ function processComponentRenderAfter( const now = timestampInSeconds(); const componentRenderDuration = now - begin.now; + const name = payload.containerKey || payload.object; + if (componentRenderDuration * 1000 >= minComponentDuration) { startInactiveSpan({ - name: payload.containerKey || payload.object, - op, + name, startTime: begin.now, attributes: { + [SENTRY_OP]: op, [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.ember', + [UI_COMPONENT_NAME]: name, }, onlyIfParent: true, })?.end(now); @@ -178,7 +184,7 @@ function _instrumentComponents(config: { }, after(_name: string, _timestamp: number, payload: Payload, _beganIndex: number) { - processComponentRenderAfter(payload, beforeEntries, 'ui.ember.component.render', minComponentDuration); + processComponentRenderAfter(payload, beforeEntries, BROWSER_UI_RENDER_SPAN_OP, minComponentDuration); }, }); if (enableComponentDefinitions) { @@ -188,7 +194,7 @@ function _instrumentComponents(config: { }, after(_name: string, _timestamp: number, payload: Payload, _beganIndex: number) { - processComponentRenderAfter(payload, beforeComponentDefinitionEntries, 'ui.ember.component.definition', 0); + processComponentRenderAfter(payload, beforeComponentDefinitionEntries, GENERAL_FUNCTION_SPAN_OP, 0); }, }); } @@ -230,9 +236,10 @@ function _instrumentInitialLoad(): void { const endTime = startTime + measure.duration / 1000; startInactiveSpan({ - op: 'ui.ember.init', name: 'init', attributes: { + // TODO(v11): Replace with the `ui.mount` constant from `@sentry/conventions/op` once it is registered there. + [SENTRY_OP]: 'ui.mount', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.ember', }, startTime, diff --git a/packages/ember/tests/acceptance/sentry-performance-test.ts b/packages/ember/tests/acceptance/sentry-performance-test.ts index 424f4663484d..9e8e19b87884 100644 --- a/packages/ember/tests/acceptance/sentry-performance-test.ts +++ b/packages/ember/tests/acceptance/sentry-performance-test.ts @@ -15,10 +15,7 @@ module('Acceptance | Sentry Performance', function (hooks) { assertSentryTransactionCount(assert, 1); assertSentryTransactions(assert, 0, { - spans: [ - 'ui.ember.transition | route:undefined -> route:tracing', - 'ui.ember.component.render | component:test-section', - ], + spans: ['ui.ember.transition | route:undefined -> route:tracing', 'ui.render | component:test-section'], transaction: 'route:tracing', attributes: { fromRoute: undefined, @@ -36,16 +33,16 @@ module('Acceptance | Sentry Performance', function (hooks) { assertSentryTransactions(assert, 1, { spans: [ 'ui.ember.transition | route:tracing -> route:slow-loading-route.index', - 'ui.ember.route.before_model | slow-loading-route', - 'ui.ember.route.model | slow-loading-route', - 'ui.ember.route.after_model | slow-loading-route', - 'ui.ember.route.before_model | slow-loading-route.index', - 'ui.ember.route.model | slow-loading-route.index', - 'ui.ember.route.after_model | slow-loading-route.index', - 'ui.ember.route.setup_controller | slow-loading-route', - 'ui.ember.route.setup_controller | slow-loading-route.index', - 'ui.ember.component.render | component:slow-loading-list', - 'ui.ember.component.render | component:slow-loading-list', + 'function:beforeModel | slow-loading-route', + 'function:model | slow-loading-route', + 'function:afterModel | slow-loading-route', + 'function:beforeModel | slow-loading-route.index', + 'function:model | slow-loading-route.index', + 'function:afterModel | slow-loading-route.index', + 'function:setupController | slow-loading-route', + 'function:setupController | slow-loading-route.index', + 'ui.render | component:slow-loading-list', + 'ui.render | component:slow-loading-list', ], transaction: 'route:slow-loading-route.index', durationCheck: duration => duration > SLOW_TRANSITION_WAIT, @@ -63,10 +60,10 @@ module('Acceptance | Sentry Performance', function (hooks) { assertSentryTransactions(assert, 0, { spans: [ 'ui.ember.transition | route:undefined -> route:with-loading.index', - 'ui.ember.route.before_model | with-loading.index', - 'ui.ember.route.model | with-loading.index', - 'ui.ember.route.after_model | with-loading.index', - 'ui.ember.route.setup_controller | with-loading.index', + 'function:beforeModel | with-loading.index', + 'function:model | with-loading.index', + 'function:afterModel | with-loading.index', + 'function:setupController | with-loading.index', ], transaction: 'route:with-loading.index', attributes: { @@ -86,8 +83,8 @@ module('Acceptance | Sentry Performance', function (hooks) { assertSentryTransactions(assert, 0, { spans: [ 'ui.ember.transition | route:undefined -> route:with-error.index', - 'ui.ember.route.before_model | with-error.index', - 'ui.ember.route.model | with-error.index', + 'function:beforeModel | with-error.index', + 'function:model | with-error.index', ], transaction: 'route:with-error.index', attributes: { diff --git a/packages/ember/tests/helpers/utils.ts b/packages/ember/tests/helpers/utils.ts index bce62a85dea1..b2f3a891111d 100644 --- a/packages/ember/tests/helpers/utils.ts +++ b/packages/ember/tests/helpers/utils.ts @@ -44,6 +44,11 @@ export function assertSentryErrors( }); } +// Runloop spans share the generic `ui.task` op, so the queue attribute is what identifies them +function isRunloopSpan(span: NonNullable[number]): boolean { + return span.op === 'ui.task' && span.data?.['ember.runloop.queue'] !== undefined; +} + export function assertSentryTransactions( assert: Assert, callNumber: number, @@ -68,20 +73,15 @@ export function assertSentryTransactions( const filteredSpans = spans .filter(span => { const op = span.op; - return ( - !op?.startsWith('ui.ember.runloop.') && - !op?.startsWith('ui.long-task') && - !op?.startsWith('ui.long-animation-frame') - ); + return !isRunloopSpan(span) && !op?.startsWith('ui.long-task') && !op?.startsWith('ui.long-animation-frame'); }) .map(spanJson => { - return `${spanJson.op} | ${spanJson.description}`; + // Route hooks all share the `function` op, so the hook name is what distinguishes them + const hook = spanJson.data?.['ember.route.hook']; + return hook ? `${spanJson.op}:${hook} | ${spanJson.description}` : `${spanJson.op} | ${spanJson.description}`; }); - assert.true( - spans.some(span => span.op?.startsWith('ui.ember.runloop.')), - 'it captures runloop spans', - ); + assert.true(spans.some(isRunloopSpan), 'it captures runloop spans'); assert.deepEqual(filteredSpans, options.spans, 'Has correct spans'); assert.equal(event.transaction, options.transaction);