diff --git a/dev-packages/e2e-tests/test-applications/ember-classic/app/app.ts b/dev-packages/e2e-tests/test-applications/ember-classic/app/app.ts index e03c77ddc1a4..5835dac94c19 100644 --- a/dev-packages/e2e-tests/test-applications/ember-classic/app/app.ts +++ b/dev-packages/e2e-tests/test-applications/ember-classic/app/app.ts @@ -7,7 +7,6 @@ import config from './config/environment'; Sentry.init({ dsn: config.sentryDsn, - traceLifecycle: 'static', tracesSampleRate: 1, replaysSessionSampleRate: 1, replaysOnErrorSampleRate: 1, diff --git a/dev-packages/e2e-tests/test-applications/ember-classic/app/instance-initializers/sentry-performance.ts b/dev-packages/e2e-tests/test-applications/ember-classic/app/instance-initializers/sentry-performance.ts index b7c3f70b1e30..25c66e15d10d 100644 --- a/dev-packages/e2e-tests/test-applications/ember-classic/app/instance-initializers/sentry-performance.ts +++ b/dev-packages/e2e-tests/test-applications/ember-classic/app/instance-initializers/sentry-performance.ts @@ -5,6 +5,8 @@ export function initialize(appInstance: ApplicationInstance): void { instrumentAppInstancePerformance(appInstance, { minimumRunloopQueueDuration: 0, minimumComponentRenderDuration: 0, + // Opt in so the E2E suite covers the `ui.resolve` spans, which are off by default. + enableComponentDefinitions: true, }); } diff --git a/dev-packages/e2e-tests/test-applications/ember-classic/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/ember-classic/tests/errors.test.ts index a34194d4fd30..be2ccc732168 100644 --- a/dev-packages/e2e-tests/test-applications/ember-classic/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/ember-classic/tests/errors.test.ts @@ -1,5 +1,5 @@ import { expect, test } from '@playwright/test'; -import { waitForError, waitForTransaction } from '@sentry-internal/test-utils'; +import { getSpanOp, waitForError, waitForStreamedSpan } from '@sentry-internal/test-utils'; test('sends an error', async ({ page }) => { const errorPromise = waitForError('ember-classic', async errorEvent => { @@ -30,16 +30,17 @@ test('sends an error', async ({ page }) => { }); test('assigns the correct transaction value after a navigation', async ({ page }) => { - const pageloadTxnPromise = waitForTransaction('ember-classic', async transactionEvent => { - return !!transactionEvent.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; - }); + const pageloadSpanPromise = waitForStreamedSpan( + 'ember-classic', + span => span.is_segment && getSpanOp(span) === 'pageload', + ); const errorPromise = waitForError('ember-classic', async errorEvent => { return !errorEvent.type; }); await page.goto(`/tracing`); - await pageloadTxnPromise; + await pageloadSpanPromise; await page.getByText('Errors').click(); 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 141eb8cec968..030a6086cabc 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 @@ -1,181 +1,206 @@ import { expect, test } from '@playwright/test'; -import { waitForTransaction } from '@sentry-internal/test-utils'; - -test('sends a pageload transaction with a parameterized URL', async ({ page }) => { - const transactionPromise = waitForTransaction('ember-classic', async transactionEvent => { - return !!transactionEvent.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; +import type { SerializedStreamedSpan } from '@sentry-internal/test-utils'; +import { collectStreamedSpansUntilSegment, getSpanOp, waitForStreamedSpan } from '@sentry-internal/test-utils'; + +// This app runs on the default `traceLifecycle: 'stream'`, so it emits spans, not transactions. +// `ember-embroider` and `ember-vite` cover the same routes on the static lifecycle. + +// The SDK stamps these onto every streamed span, whatever produced it. Stripping them lets the +// instrumentation's own attributes be asserted exactly, the way the static suites assert `span.data`. +const SDK_ATTRIBUTES = [ + 'sentry.trace_lifecycle', + 'sentry.segment.name', + 'sentry.segment.id', + 'sentry.sdk.name', + 'sentry.sdk.version', + 'sentry.sdk.integrations', + 'sentry.environment', + 'sentry.release', + 'sentry.sample_rate', + 'user.id', + 'user.email', + 'user.ip_address', + 'user.username', + // `httpContextIntegration` sets this on every span; replay is enabled in this app, so it tags + // every span too. + 'user_agent.original', + 'sentry.replay_id', + 'sentry._internal.replay_is_buffering', +]; + +function instrumentationAttributes(span: SerializedStreamedSpan): Record { + return Object.fromEntries( + Object.entries(span.attributes) + .filter(([key]) => !SDK_ATTRIBUTES.includes(key)) + .map(([key, attribute]) => [key, attribute.value]), + ); +} + +function expectChildSpan( + span: SerializedStreamedSpan | undefined, + segmentSpan: SerializedStreamedSpan, + expected: { name: string; attributes: Record }, +): void { + expect(span).toMatchObject({ + name: expected.name, + is_segment: false, + status: 'ok', + parent_span_id: segmentSpan.span_id, + span_id: expect.stringMatching(/^[a-f0-9]{16}$/), + trace_id: segmentSpan.trace_id, + start_timestamp: expect.any(Number), + end_timestamp: expect.any(Number), }); + expect(instrumentationAttributes(span!)).toEqual(expected.attributes); +} + +test('sends a pageload span with a parameterized URL', async ({ page }) => { + const pageloadSpanPromise = waitForStreamedSpan( + 'ember-classic', + span => span.is_segment && getSpanOp(span) === 'pageload', + ); await page.goto(`/`); - const rootSpan = await transactionPromise; - - expect(rootSpan).toMatchObject({ - contexts: { - trace: { - op: 'pageload', - origin: 'auto.pageload.ember', - data: { - 'sentry.origin': 'auto.pageload.ember', - 'sentry.segment.name.source': 'route', - 'url.template': '/', - 'url.path': '/', - 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/$/), - }, - }, - }, - transaction: 'route:index', - transaction_info: { - source: 'route', + const pageloadSpan = await pageloadSpanPromise; + + expect(pageloadSpan).toMatchObject({ + name: 'route:index', + is_segment: true, + attributes: { + 'sentry.op': { type: 'string', value: 'pageload' }, + 'sentry.origin': { type: 'string', value: 'auto.pageload.ember' }, + 'sentry.segment.name.source': { type: 'string', value: 'route' }, + 'url.template': { type: 'string', value: '/' }, + 'url.path': { type: 'string', value: '/' }, + 'url.full': { type: 'string', value: expect.stringMatching(/^https?:\/\/localhost:\d+\/$/) }, }, }); }); -test('sends a navigation transaction with a parameterized URL', async ({ page }) => { - const pageloadTxnPromise = waitForTransaction('ember-classic', async transactionEvent => { - return !!transactionEvent.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; - }); +test('sends a navigation span with a parameterized URL', async ({ page }) => { + const pageloadSpanPromise = waitForStreamedSpan( + 'ember-classic', + span => span.is_segment && getSpanOp(span) === 'pageload', + ); - const navigationTxnPromise = waitForTransaction('ember-classic', async transactionEvent => { - return !!transactionEvent.transaction && transactionEvent.contexts?.trace?.op === 'navigation'; - }); + const navigationSpanPromise = waitForStreamedSpan( + 'ember-classic', + span => span.is_segment && getSpanOp(span) === 'navigation', + ); await page.goto(`/`); - await pageloadTxnPromise; - - const [_, navigationTxn] = await Promise.all([page.getByText('Tracing').click(), navigationTxnPromise]); - - expect(navigationTxn).toMatchObject({ - contexts: { - trace: { - op: 'navigation', - origin: 'auto.navigation.ember', - data: { - 'sentry.origin': 'auto.navigation.ember', - 'sentry.segment.name.source': 'route', - 'url.template': '/tracing', - 'url.path': '/tracing', - 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/tracing$/), - }, - }, - }, - transaction: 'route:tracing', - transaction_info: { - source: 'route', + await pageloadSpanPromise; + + const [_, navigationSpan] = await Promise.all([page.getByText('Tracing').click(), navigationSpanPromise]); + + expect(navigationSpan).toMatchObject({ + name: 'route:tracing', + is_segment: true, + attributes: { + 'sentry.op': { type: 'string', value: 'navigation' }, + 'sentry.origin': { type: 'string', value: 'auto.navigation.ember' }, + 'sentry.segment.name.source': { type: 'string', value: 'route' }, + 'url.template': { type: 'string', value: '/tracing' }, + 'url.path': { type: 'string', value: '/tracing' }, + 'url.full': { type: 'string', value: expect.stringMatching(/^https?:\/\/localhost:\d+\/tracing$/) }, }, }); }); -test('sends a navigation transaction even if the pageload span is still active', async ({ page }) => { - const pageloadTxnPromise = waitForTransaction('ember-classic', async transactionEvent => { - return !!transactionEvent.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; - }); +test('sends a navigation span even if the pageload span is still active', async ({ page }) => { + const pageloadSpanPromise = waitForStreamedSpan( + 'ember-classic', + span => span.is_segment && getSpanOp(span) === 'pageload', + ); - const navigationTxnPromise = waitForTransaction('ember-classic', async transactionEvent => { - return !!transactionEvent.transaction && transactionEvent.contexts?.trace?.op === 'navigation'; - }); + const navigationSpanPromise = waitForStreamedSpan( + 'ember-classic', + span => span.is_segment && getSpanOp(span) === 'navigation', + ); await page.goto(`/`); // immediately navigate to a different route - const [_, pageloadTxn, navigationTxn] = await Promise.all([ + const [_, pageloadSpan, navigationSpan] = await Promise.all([ page.getByText('Tracing').click(), - pageloadTxnPromise, - navigationTxnPromise, + pageloadSpanPromise, + navigationSpanPromise, ]); - expect(pageloadTxn).toMatchObject({ - contexts: { - trace: { - op: 'pageload', - origin: 'auto.pageload.ember', - data: { - 'sentry.origin': 'auto.pageload.ember', - 'sentry.segment.name.source': 'route', - 'url.template': '/', - 'url.path': '/', - 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/$/), - }, - }, - }, - transaction: 'route:index', - transaction_info: { - source: 'route', + expect(pageloadSpan).toMatchObject({ + name: 'route:index', + is_segment: true, + attributes: { + 'sentry.op': { type: 'string', value: 'pageload' }, + 'sentry.origin': { type: 'string', value: 'auto.pageload.ember' }, + 'sentry.segment.name.source': { type: 'string', value: 'route' }, + 'url.template': { type: 'string', value: '/' }, + 'url.path': { type: 'string', value: '/' }, + 'url.full': { type: 'string', value: expect.stringMatching(/^https?:\/\/localhost:\d+\/$/) }, }, }); - expect(navigationTxn).toMatchObject({ - contexts: { - trace: { - op: 'navigation', - origin: 'auto.navigation.ember', - data: { - 'sentry.origin': 'auto.navigation.ember', - 'sentry.segment.name.source': 'route', - 'url.template': '/tracing', - 'url.path': '/tracing', - 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/tracing$/), - }, - }, - }, - transaction: 'route:tracing', - transaction_info: { - source: 'route', + expect(navigationSpan).toMatchObject({ + name: 'route:tracing', + is_segment: true, + attributes: { + 'sentry.op': { type: 'string', value: 'navigation' }, + 'sentry.origin': { type: 'string', value: 'auto.navigation.ember' }, + 'sentry.segment.name.source': { type: 'string', value: 'route' }, + 'url.template': { type: 'string', value: '/tracing' }, + 'url.path': { type: 'string', value: '/tracing' }, + 'url.full': { type: 'string', value: expect.stringMatching(/^https?:\/\/localhost:\d+\/tracing$/) }, }, }); }); test('captures correct spans for navigation', async ({ page }) => { - const pageloadTxnPromise = waitForTransaction('ember-classic', async transactionEvent => { - return !!transactionEvent.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; - }); - - const navigationTxnPromise = waitForTransaction('ember-classic', async transactionEvent => { - return !!transactionEvent.transaction && transactionEvent.contexts?.trace?.op === 'navigation'; - }); + const pageloadSpanPromise = waitForStreamedSpan( + 'ember-classic', + span => span.is_segment && getSpanOp(span) === 'pageload', + ); await page.goto(`/tracing`); - await pageloadTxnPromise; - - const [_, navigationTxn] = await Promise.all([page.getByText('Measure Things!').click(), navigationTxnPromise]); - - const traceId = navigationTxn.contexts?.trace?.trace_id; - const spanId = navigationTxn.contexts?.trace?.span_id; - - expect(traceId).toBeDefined(); - expect(spanId).toBeDefined(); - - const spans = navigationTxn.spans || []; - - expect(navigationTxn).toMatchObject({ - contexts: { - trace: { - op: 'navigation', - origin: 'auto.navigation.ember', - data: { - 'sentry.origin': 'auto.navigation.ember', - 'sentry.segment.name.source': 'route', - 'url.template': '/slow-loading-route', - 'url.path': '/slow-loading-route', - 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/slow-loading-route$/), - }, - }, - }, - transaction: 'route:slow-loading-route.index', - transaction_info: { - source: 'route', + await pageloadSpanPromise; + + const spansPromise = collectStreamedSpansUntilSegment('ember-classic', 'route:slow-loading-route.index'); + await page.getByText('Measure Things!').click(); + const spans = await spansPromise; + + const navigationSpan = spans.find(span => span.is_segment)!; + + expect(navigationSpan.trace_id).toEqual(expect.stringMatching(/^[a-f0-9]{32}$/)); + expect(navigationSpan.span_id).toEqual(expect.stringMatching(/^[a-f0-9]{16}$/)); + + expect(navigationSpan).toMatchObject({ + name: 'route:slow-loading-route.index', + is_segment: true, + status: 'ok', + attributes: { + 'sentry.op': { type: 'string', value: 'navigation' }, + 'sentry.origin': { type: 'string', value: 'auto.navigation.ember' }, + 'sentry.segment.name.source': { type: 'string', value: 'route' }, + 'url.template': { type: 'string', value: '/slow-loading-route' }, + 'url.path': { type: 'string', value: '/slow-loading-route' }, + 'url.full': { type: 'string', value: expect.stringMatching(/^https?:\/\/localhost:\d+\/slow-loading-route$/) }, }, }); - const transitionSpans = spans.filter(span => span.op === 'router'); - const beforeModelSpans = spans.filter( - span => span.op === 'function' && span.data?.['code.function.name'] === 'beforeModel', - ); - const modelSpans = spans.filter(span => span.op === 'function' && span.data?.['code.function.name'] === 'model'); - const afterModelSpans = spans.filter( - span => span.op === 'function' && span.data?.['code.function.name'] === 'afterModel', - ); - const renderSpans = spans.filter(span => span.op === 'ui.task' && span.data?.['ember.runloop.queue'] === 'render'); + const childrenOf = (op: string, functionName?: string): SerializedStreamedSpan[] => + spans.filter( + span => + !span.is_segment && + span.parent_span_id === navigationSpan.span_id && + getSpanOp(span) === op && + (functionName === undefined || span.attributes['code.function.name']?.value === functionName), + ); + + const transitionSpans = childrenOf('router'); + const beforeModelSpans = childrenOf('function', 'beforeModel'); + const modelSpans = childrenOf('function', 'model'); + const afterModelSpans = childrenOf('function', 'afterModel'); + const renderSpans = childrenOf('ui.task').filter(span => span.attributes['ember.runloop.queue']?.value === 'render'); expect(transitionSpans).toHaveLength(1); @@ -187,141 +212,87 @@ test('captures correct spans for navigation', async ({ page }) => { // There may be many render spans... expect(renderSpans.length).toBeGreaterThan(1); - expect(transitionSpans[0]).toEqual({ - data: { + // Ember has no route template for the transition itself, so a streamed router span takes the + // static fallback rather than the `route:a -> route:b` pair it uses on the static lifecycle. + expectChildSpan(transitionSpans[0], navigationSpan, { + name: 'Router', + attributes: { 'sentry.op': 'router', 'sentry.origin': 'auto.ui.ember', }, - description: 'route:tracing -> route:slow-loading-route.index', - op: 'router', - origin: 'auto.ui.ember', - status: 'ok', - parent_span_id: spanId, - span_id: expect.stringMatching(/[a-f0-9]{16}/), - start_timestamp: expect.any(Number), - timestamp: expect.any(Number), - trace_id: traceId, }); - expect(beforeModelSpans).toEqual([ - { - data: { - 'code.function.name': 'beforeModel', + // Route hook spans are named after the hook (matching `code.function.name`), which is the + // `function` op's name template. The route stays on the span as its description. + for (const [hookName, hookSpans] of [ + ['beforeModel', beforeModelSpans], + ['model', modelSpans], + ['afterModel', afterModelSpans], + ] as const) { + expectChildSpan(hookSpans[0], navigationSpan, { + name: hookName, + attributes: { + 'code.function.name': hookName, + 'sentry.description': 'slow-loading-route', 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', }, - description: 'slow-loading-route', - op: 'function', - origin: 'auto.ui.ember', - status: 'ok', - parent_span_id: spanId, - span_id: expect.stringMatching(/[a-f0-9]{16}/), - start_timestamp: expect.any(Number), - timestamp: expect.any(Number), - trace_id: traceId, - }, - { - data: { - 'code.function.name': 'beforeModel', - 'sentry.op': 'function', - 'sentry.origin': 'auto.ui.ember', - }, - description: 'slow-loading-route.index', - op: 'function', - origin: 'auto.ui.ember', - status: 'ok', - parent_span_id: spanId, - span_id: expect.stringMatching(/[a-f0-9]{16}/), - start_timestamp: expect.any(Number), - timestamp: expect.any(Number), - trace_id: traceId, - }, - ]); + }); - expect(modelSpans).toEqual([ - { - data: { - 'code.function.name': 'model', - 'sentry.op': 'function', - 'sentry.origin': 'auto.ui.ember', - }, - description: 'slow-loading-route', - op: 'function', - origin: 'auto.ui.ember', - status: 'ok', - parent_span_id: spanId, - span_id: expect.stringMatching(/[a-f0-9]{16}/), - start_timestamp: expect.any(Number), - timestamp: expect.any(Number), - trace_id: traceId, - }, - { - data: { - 'code.function.name': 'model', + expectChildSpan(hookSpans[1], navigationSpan, { + name: hookName, + attributes: { + 'code.function.name': hookName, + 'sentry.description': 'slow-loading-route.index', 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', }, - description: 'slow-loading-route.index', - op: 'function', - origin: 'auto.ui.ember', - status: 'ok', - parent_span_id: spanId, - span_id: expect.stringMatching(/[a-f0-9]{16}/), - start_timestamp: expect.any(Number), - timestamp: expect.any(Number), - trace_id: traceId, - }, - ]); + }); + } - expect(afterModelSpans).toEqual([ - { - data: { - 'code.function.name': 'afterModel', - 'sentry.op': 'function', - 'sentry.origin': 'auto.ui.ember', - }, - description: 'slow-loading-route', - op: 'function', - origin: 'auto.ui.ember', - status: 'ok', - parent_span_id: spanId, - span_id: expect.stringMatching(/[a-f0-9]{16}/), - start_timestamp: expect.any(Number), - timestamp: expect.any(Number), - trace_id: traceId, + expectChildSpan(renderSpans[0], navigationSpan, { + name: 'runloop', + attributes: { + 'ember.runloop.queue': 'render', + 'sentry.op': 'ui.task', + 'sentry.origin': 'auto.ui.ember', }, - { - data: { - 'code.function.name': 'afterModel', - 'sentry.op': 'function', - 'sentry.origin': 'auto.ui.ember', - }, - description: 'slow-loading-route.index', - op: 'function', - origin: 'auto.ui.ember', - status: 'ok', - parent_span_id: spanId, - span_id: expect.stringMatching(/[a-f0-9]{16}/), - start_timestamp: expect.any(Number), - timestamp: expect.any(Number), - trace_id: traceId, + }); +}); + +test('captures a `ui.resolve` span alongside the `ui.render` span for a component', async ({ page }) => { + const spansPromise = collectStreamedSpansUntilSegment('ember-classic', 'route:tracing'); + + await page.goto(`/tracing`); + + const spans = await spansPromise; + + const pageloadSpan = spans.find(span => span.is_segment)!; + const resolveSpans = spans.filter(span => getSpanOp(span) === 'ui.resolve'); + const renderSpans = spans.filter(span => getSpanOp(span) === 'ui.render'); + + expect(resolveSpans.length).toBeGreaterThan(0); + expect(renderSpans.length).toBeGreaterThan(0); + + // Resolving a component definition and rendering it are separate steps, so they must not share + // an op even though they carry the same component name. + const resolveSpan = resolveSpans[0]!; + expectChildSpan(resolveSpan, pageloadSpan, { + name: resolveSpan.name, + attributes: { + 'sentry.op': 'ui.resolve', + 'sentry.origin': 'auto.ui.ember', + 'ui.component_name': resolveSpan.name, }, - ]); + }); - expect(renderSpans).toContainEqual({ - data: { - 'ember.runloop.queue': 'render', - 'sentry.op': 'ui.task', + const renderSpan = renderSpans[0]!; + expectChildSpan(renderSpan, pageloadSpan, { + name: renderSpan.name, + attributes: { + 'sentry.op': 'ui.render', 'sentry.origin': 'auto.ui.ember', + 'ui.component_name': renderSpan.name, }, - description: 'runloop', - op: 'ui.task', - origin: 'auto.ui.ember', - status: 'ok', - parent_span_id: spanId, - span_id: expect.stringMatching(/[a-f0-9]{16}/), - start_timestamp: expect.any(Number), - timestamp: expect.any(Number), - trace_id: traceId, }); }); 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 4043b52dc04b..b6ccaccbde6a 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 @@ -209,6 +209,7 @@ test('captures correct spans for navigation', async ({ page }) => { 'code.function.name': 'beforeModel', 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', + 'sentry.description': 'slow-loading-route', }, description: 'slow-loading-route', op: 'function', @@ -225,6 +226,7 @@ test('captures correct spans for navigation', async ({ page }) => { 'code.function.name': 'beforeModel', 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', + 'sentry.description': 'slow-loading-route.index', }, description: 'slow-loading-route.index', op: 'function', @@ -244,6 +246,7 @@ test('captures correct spans for navigation', async ({ page }) => { 'code.function.name': 'model', 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', + 'sentry.description': 'slow-loading-route', }, description: 'slow-loading-route', op: 'function', @@ -260,6 +263,7 @@ test('captures correct spans for navigation', async ({ page }) => { 'code.function.name': 'model', 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', + 'sentry.description': 'slow-loading-route.index', }, description: 'slow-loading-route.index', op: 'function', @@ -279,6 +283,7 @@ test('captures correct spans for navigation', async ({ page }) => { 'code.function.name': 'afterModel', 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', + 'sentry.description': 'slow-loading-route', }, description: 'slow-loading-route', op: 'function', @@ -295,6 +300,7 @@ test('captures correct spans for navigation', async ({ page }) => { 'code.function.name': 'afterModel', 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', + 'sentry.description': 'slow-loading-route.index', }, description: 'slow-loading-route.index', op: 'function', diff --git a/dev-packages/e2e-tests/test-applications/ember-strict-resolver/tests/sentry-performance.test.ts b/dev-packages/e2e-tests/test-applications/ember-strict-resolver/tests/sentry-performance.test.ts index 57d9a6c26972..f2641d316534 100644 --- a/dev-packages/e2e-tests/test-applications/ember-strict-resolver/tests/sentry-performance.test.ts +++ b/dev-packages/e2e-tests/test-applications/ember-strict-resolver/tests/sentry-performance.test.ts @@ -181,6 +181,7 @@ test('captures correct spans for navigation', async ({ page }) => { 'code.function.name': 'beforeModel', 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', + 'sentry.description': 'slow-loading-route', }, description: 'slow-loading-route', op: 'function', @@ -197,6 +198,7 @@ test('captures correct spans for navigation', async ({ page }) => { 'code.function.name': 'beforeModel', 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', + 'sentry.description': 'slow-loading-route.index', }, description: 'slow-loading-route.index', op: 'function', @@ -216,6 +218,7 @@ test('captures correct spans for navigation', async ({ page }) => { 'code.function.name': 'model', 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', + 'sentry.description': 'slow-loading-route', }, description: 'slow-loading-route', op: 'function', @@ -232,6 +235,7 @@ test('captures correct spans for navigation', async ({ page }) => { 'code.function.name': 'model', 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', + 'sentry.description': 'slow-loading-route.index', }, description: 'slow-loading-route.index', op: 'function', @@ -251,6 +255,7 @@ test('captures correct spans for navigation', async ({ page }) => { 'code.function.name': 'afterModel', 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', + 'sentry.description': 'slow-loading-route', }, description: 'slow-loading-route', op: 'function', @@ -267,6 +272,7 @@ test('captures correct spans for navigation', async ({ page }) => { 'code.function.name': 'afterModel', 'sentry.op': 'function', 'sentry.origin': 'auto.ui.ember', + 'sentry.description': 'slow-loading-route.index', }, description: 'slow-loading-route.index', op: 'function', @@ -327,56 +333,80 @@ test('handles slow loading route', async ({ page }) => { expect.objectContaining({ op: 'function', description: 'slow-loading-route', - data: expect.objectContaining({ 'code.function.name': 'beforeModel' }), + data: expect.objectContaining({ + 'code.function.name': 'beforeModel', + 'sentry.description': 'slow-loading-route', + }), }), ); expect(transaction.spans).toContainEqual( expect.objectContaining({ op: 'function', description: 'slow-loading-route.index', - data: expect.objectContaining({ 'code.function.name': 'beforeModel' }), + data: expect.objectContaining({ + 'code.function.name': 'beforeModel', + 'sentry.description': 'slow-loading-route.index', + }), }), ); expect(transaction.spans).toContainEqual( expect.objectContaining({ op: 'function', description: 'slow-loading-route.index', - data: expect.objectContaining({ 'code.function.name': 'model' }), + data: expect.objectContaining({ + 'code.function.name': 'model', + 'sentry.description': 'slow-loading-route.index', + }), }), ); expect(transaction.spans).toContainEqual( expect.objectContaining({ op: 'function', description: 'slow-loading-route', - data: expect.objectContaining({ 'code.function.name': 'model' }), + data: expect.objectContaining({ + 'code.function.name': 'model', + 'sentry.description': 'slow-loading-route', + }), }), ); expect(transaction.spans).toContainEqual( expect.objectContaining({ op: 'function', description: 'slow-loading-route', - data: expect.objectContaining({ 'code.function.name': 'afterModel' }), + data: expect.objectContaining({ + 'code.function.name': 'afterModel', + 'sentry.description': 'slow-loading-route', + }), }), ); expect(transaction.spans).toContainEqual( expect.objectContaining({ op: 'function', description: 'slow-loading-route.index', - data: expect.objectContaining({ 'code.function.name': 'afterModel' }), + data: expect.objectContaining({ + 'code.function.name': 'afterModel', + 'sentry.description': 'slow-loading-route.index', + }), }), ); expect(transaction.spans).toContainEqual( expect.objectContaining({ op: 'function', description: 'slow-loading-route', - data: expect.objectContaining({ 'code.function.name': 'setupController' }), + data: expect.objectContaining({ + 'code.function.name': 'setupController', + 'sentry.description': 'slow-loading-route', + }), }), ); expect(transaction.spans).toContainEqual( expect.objectContaining({ op: 'function', description: 'slow-loading-route.index', - data: expect.objectContaining({ 'code.function.name': 'setupController' }), + data: expect.objectContaining({ + 'code.function.name': 'setupController', + 'sentry.description': 'slow-loading-route.index', + }), }), ); }); diff --git a/dev-packages/e2e-tests/test-applications/ember-strict-resolver/tests/streamed-performance.test.ts b/dev-packages/e2e-tests/test-applications/ember-strict-resolver/tests/streamed-performance.test.ts index bd6405989474..6f814f55c52f 100644 --- a/dev-packages/e2e-tests/test-applications/ember-strict-resolver/tests/streamed-performance.test.ts +++ b/dev-packages/e2e-tests/test-applications/ember-strict-resolver/tests/streamed-performance.test.ts @@ -17,3 +17,26 @@ test('names the transition span with the low cardinality fallback', async ({ pag expect(transitionSpan.name).toBe('Router'); expect(transitionSpan.attributes['sentry.origin']).toEqual({ type: 'string', value: 'auto.ui.ember' }); }); + +test('names route hook spans after the hook and keeps the route as the description', async ({ page }) => { + const modelSpanPromise = waitForStreamedSpan( + 'ember-strict-resolver', + span => + getSpanOp(span) === 'function' && + span.attributes['code.function.name']?.value === 'model' && + span.attributes['sentry.description']?.value === 'slow-loading-route.index', + ); + + await page.goto('/tracing'); + await page.getByText('Transition to slow loading route').click(); + + const modelSpan = await modelSpanPromise; + + // `slow-loading-route.index` is an Ember route name, not one of the `function` op's name templates, + // so a streamed span is named after the hook (matching `code.function.name`) and carries the route + // as its description instead. + expect(modelSpan.name).toBe('model'); + expect(modelSpan.attributes['code.function.name']).toEqual({ type: 'string', value: 'model' }); + expect(modelSpan.attributes['sentry.description']).toEqual({ type: 'string', value: 'slow-loading-route.index' }); + expect(modelSpan.attributes['sentry.origin']).toEqual({ type: 'string', value: 'auto.ui.ember' }); +}); diff --git a/packages/ember/src/utils/instrumentEmberGlobals.ts b/packages/ember/src/utils/instrumentEmberGlobals.ts index 6d19479af04c..a7b6fe761a30 100644 --- a/packages/ember/src/utils/instrumentEmberGlobals.ts +++ b/packages/ember/src/utils/instrumentEmberGlobals.ts @@ -1,7 +1,7 @@ import { subscribe } from '@ember/instrumentation'; import { scheduleOnce } from '@ember/runloop'; import { SENTRY_OP, UI_COMPONENT_NAME } from '@sentry/conventions/attributes'; -import { UI_MOUNT, UI_RENDER, UI_TASK, FUNCTION } from '@sentry/conventions/op'; +import { UI_MOUNT, UI_RENDER, UI_TASK } 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'; @@ -193,7 +193,9 @@ function _instrumentComponents(config: { }, after(_name: string, _timestamp: number, payload: object) { - _processComponentRenderAfter(payload as Payload, beforeComponentDefinitionEntries, FUNCTION, 0); + // TODO: Use the `UI_RESOLVE` const from `@sentry/conventions/op` once the op is released. + // See https://github.com/getsentry/sentry-conventions/pull/633 + _processComponentRenderAfter(payload as Payload, beforeComponentDefinitionEntries, 'ui.resolve', 0); }, }); } diff --git a/packages/ember/src/utils/instrumentRoutePerformance.ts b/packages/ember/src/utils/instrumentRoutePerformance.ts index 080326bab3ce..7704f9d2462c 100644 --- a/packages/ember/src/utils/instrumentRoutePerformance.ts +++ b/packages/ember/src/utils/instrumentRoutePerformance.ts @@ -1,7 +1,7 @@ import { startSpan } from '@sentry/browser'; -import { CODE_FUNCTION_NAME, SENTRY_OP } from '@sentry/conventions/attributes'; +import { CODE_FUNCTION_NAME, SENTRY_DESCRIPTION, SENTRY_OP } from '@sentry/conventions/attributes'; import { FUNCTION } from '@sentry/conventions/op'; -import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; +import { getClient, hasSpanStreamingEnabled, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import type Route from '@ember/routing/route'; @@ -33,19 +33,21 @@ type RouteConstructor = new (...args: ConstructorParameters) => Ro export function instrumentRoutePerformance(BaseRoute: T): T { const instrumentFunction = async ( hookName: string, - name: string, + fullRouteName: string, // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Route hooks have varied signatures that can't be unified with unknown fn: (...args: any[]) => any, args: unknown[], ): Promise => { + const client = getClient(); return startSpan( { attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.ember', [SENTRY_OP]: FUNCTION, [CODE_FUNCTION_NAME]: hookName, + [SENTRY_DESCRIPTION]: fullRouteName, }, - name, + name: client && hasSpanStreamingEnabled(client) ? hookName : fullRouteName, onlyIfParent: true, }, () => { diff --git a/packages/ember/tests/instrument-ember-globals.test.ts b/packages/ember/tests/instrument-ember-globals.test.ts new file mode 100644 index 000000000000..8651fbec41ab --- /dev/null +++ b/packages/ember/tests/instrument-ember-globals.test.ts @@ -0,0 +1,88 @@ +import { subscribe } from '@ember/instrumentation'; +import { startInactiveSpan } from '@sentry/browser'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +type Subscriber = { + before: (name: string, timestamp: number, payload: object) => void; + after: (name: string, timestamp: number, payload: object) => void; +}; + +vi.mock('@ember/instrumentation', () => ({ subscribe: vi.fn() })); +vi.mock('@ember/runloop', () => ({ scheduleOnce: vi.fn(), _backburner: undefined, run: {} })); +vi.mock('@sentry/browser', () => ({ + getActiveSpan: vi.fn(() => undefined), + startInactiveSpan: vi.fn(() => ({ end: vi.fn() })), + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN: 'sentry.origin', +})); + +function getSubscriber(eventName: string): Subscriber | undefined { + const call = vi.mocked(subscribe).mock.calls.find(([name]) => name === eventName); + return call?.[1] as Subscriber | undefined; +} + +async function instrumentComponents(enableComponentDefinitions: boolean): Promise { + const { instrumentGlobalsForPerformance } = await import('../src/utils/instrumentEmberGlobals.ts'); + instrumentGlobalsForPerformance({ + disableRunloopPerformance: true, + disableInitialLoadInstrumentation: true, + // The instrumented renders below take ~0ms, so drop the threshold that would skip them. + minimumComponentRenderDuration: 0, + enableComponentDefinitions, + }); +} + +describe('component instrumentation', () => { + const payload = { containerKey: 'component:test-component', initialRender: true as const, object: '' }; + + beforeEach(() => { + vi.clearAllMocks(); + vi.resetModules(); + }); + + it('starts a `ui.render` span for a component render', async () => { + await instrumentComponents(false); + + const subscriber = getSubscriber('render.component'); + subscriber?.before('render.component', 0, payload); + subscriber?.after('render.component', 0, payload); + + expect(startInactiveSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'component:test-component', + attributes: expect.objectContaining({ + 'sentry.op': 'ui.render', + 'sentry.origin': 'auto.ui.ember', + 'ui.component_name': 'component:test-component', + }), + }), + ); + }); + + it('starts a `ui.resolve` span for a component definition lookup', async () => { + await instrumentComponents(true); + + const subscriber = getSubscriber('render.getComponentDefinition'); + subscriber?.before('render.getComponentDefinition', 0, payload); + subscriber?.after('render.getComponentDefinition', 0, payload); + + // Resolving a component definition is not a render, so it must not collide with the + // `ui.render` span that follows it for the same component. + expect(startInactiveSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'component:test-component', + attributes: expect.objectContaining({ + 'sentry.op': 'ui.resolve', + 'sentry.origin': 'auto.ui.ember', + 'ui.component_name': 'component:test-component', + }), + }), + ); + }); + + it('does not subscribe to component definition lookups unless they are enabled', async () => { + await instrumentComponents(false); + + expect(getSubscriber('render.getComponentDefinition')).toBeUndefined(); + expect(getSubscriber('render.component')).toBeDefined(); + }); +}); diff --git a/packages/ember/tests/instrument-route-performance.test.ts b/packages/ember/tests/instrument-route-performance.test.ts index 3f0bee0321fb..89a71b702cc2 100644 --- a/packages/ember/tests/instrument-route-performance.test.ts +++ b/packages/ember/tests/instrument-route-performance.test.ts @@ -1,48 +1,48 @@ import type Route from '@ember/routing/route'; import { startSpan } from '@sentry/browser'; -import { describe, expect, it, vi } from 'vitest'; +import { hasSpanStreamingEnabled } from '@sentry/core'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; vi.mock('@sentry/browser', () => ({ startSpan: vi.fn((_options: unknown, callback: () => unknown) => callback()), })); vi.mock('@sentry/core', () => ({ SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN: 'sentry.origin', + getClient: vi.fn(() => ({})), + hasSpanStreamingEnabled: vi.fn(() => false), })); -describe('instrumentRoutePerformance', () => { - it('wrapped Route hooks maintain the current context', async () => { - const { instrumentRoutePerformance } = await import('../src/utils/instrumentRoutePerformance.ts'); +class DummyRoute { + public fullRouteName = 'dummy'; - const beforeModel = vi.fn(); - const model = vi.fn(); - const afterModel = vi.fn(); - const setupController = vi.fn(); + public beforeModel(..._args: unknown[]): void {} - class DummyRoute { - public fullRouteName = 'dummy'; + public model(..._args: unknown[]): void {} - public beforeModel(...args: unknown[]): void { - beforeModel.apply(this, args); - } + public afterModel(..._args: unknown[]): void {} - public model(...args: unknown[]): void { - model.apply(this, args); - } + public setupController(..._args: unknown[]): void {} +} - public afterModel(...args: unknown[]): void { - afterModel.apply(this, args); - } +async function createInstrumentedRoute(): Promise { + const { instrumentRoutePerformance } = await import('../src/utils/instrumentRoutePerformance.ts'); + const InstrumentedDummyRoute = instrumentRoutePerformance(DummyRoute as unknown as new (...args: unknown[]) => Route); + return new InstrumentedDummyRoute(); +} - public setupController(...args: unknown[]): void { - setupController.apply(this, args); - } - } +describe('instrumentRoutePerformance', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(hasSpanStreamingEnabled).mockReturnValue(false); + }); - const InstrumentedDummyRoute = instrumentRoutePerformance( - DummyRoute as unknown as new (...args: unknown[]) => Route, - ); + it('wrapped Route hooks maintain the current context', async () => { + const beforeModel = vi.spyOn(DummyRoute.prototype, 'beforeModel'); + const model = vi.spyOn(DummyRoute.prototype, 'model'); + const afterModel = vi.spyOn(DummyRoute.prototype, 'afterModel'); + const setupController = vi.spyOn(DummyRoute.prototype, 'setupController'); - const route = new InstrumentedDummyRoute(); + const route = await createInstrumentedRoute(); await route.beforeModel('foo'); expect(beforeModel).toHaveBeenCalledWith('foo'); @@ -62,4 +62,59 @@ describe('instrumentRoutePerformance', () => { expect(startSpan).toHaveBeenCalledTimes(4); }); + + it('names the span after the route and describes it with the route when span streaming is disabled', async () => { + const route = await createInstrumentedRoute(); + + await route.model(); + + expect(startSpan).toHaveBeenCalledWith( + { + attributes: { + 'sentry.origin': 'auto.ui.ember', + 'sentry.op': 'function', + 'code.function.name': 'model', + 'sentry.description': 'dummy', + }, + name: 'dummy', + onlyIfParent: true, + }, + expect.any(Function), + ); + }); + + it('names the span after the hook when span streaming is enabled, keeping the route as the description', async () => { + vi.mocked(hasSpanStreamingEnabled).mockReturnValue(true); + + const route = await createInstrumentedRoute(); + + await route.model(); + + expect(startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'model', + attributes: expect.objectContaining({ + 'code.function.name': 'model', + 'sentry.description': 'dummy', + }), + }), + expect.any(Function), + ); + }); + + it.each(['beforeModel', 'model', 'afterModel', 'setupController'] as const)( + 'sets `code.function.name` to the %s hook name', + async hookName => { + const route = await createInstrumentedRoute(); + + await (route[hookName] as () => Promise)(); + + expect(startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + attributes: expect.objectContaining({ 'code.function.name': hookName }), + }), + expect.any(Function), + ); + }, + ); });