From eb55f17466570d56584dab3de41a21c6856c0b6b Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 13 Nov 2024 10:53:37 +0100 Subject: [PATCH 1/5] fix(core): Set `sentry.source` attribute to `custom` when calling `span.updateName` --- .../suites/public-api/startSpan/basic/test.ts | 43 +++++++++++++------ .../pageload-update-txn-name/init.js | 10 +++++ .../pageload-update-txn-name/subject.js | 3 ++ .../pageload-update-txn-name/test.ts | 36 ++++++++++++++++ .../pageload/test.ts | 17 +++++++- packages/core/src/tracing/sentrySpan.ts | 1 + .../core/test/lib/tracing/sentrySpan.test.ts | 15 ++++++- 7 files changed, 110 insertions(+), 15 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-update-txn-name/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-update-txn-name/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-update-txn-name/test.ts diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts index f1152bde21da..9f9157b6aca5 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts @@ -6,19 +6,36 @@ import { shouldSkipTracingTest, waitForTransactionRequestOnUrl, } from '../../../../utils/helpers'; - -sentryTest('should send a transaction in an envelope', async ({ getLocalTestPath, page }) => { - if (shouldSkipTracingTest()) { - sentryTest.skip(); - } - - const url = await getLocalTestPath({ testDir: __dirname }); - const req = await waitForTransactionRequestOnUrl(page, url); - const transaction = envelopeRequestParser(req); - - expect(transaction.transaction).toBe('parent_span'); - expect(transaction.spans).toBeDefined(); -}); +import { + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, +} from '@sentry/browser'; + +sentryTest( + 'sends a transaction in an envelope with manual origin and custom source', + async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + const req = await waitForTransactionRequestOnUrl(page, url); + const transaction = envelopeRequestParser(req); + + const attributes = transaction.contexts?.trace?.data; + expect(attributes).toEqual({ + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'manual', + [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', + }); + + expect(transaction.transaction_info?.source).toBe('custom'); + + expect(transaction.transaction).toBe('parent_span'); + expect(transaction.spans).toBeDefined(); + }, +); sentryTest('should report finished spans as children of the root transaction', async ({ getLocalTestPath, page }) => { if (shouldSkipTracingTest()) { diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-update-txn-name/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-update-txn-name/init.js new file mode 100644 index 000000000000..1f0b64911a75 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-update-txn-name/init.js @@ -0,0 +1,10 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; +window._testBaseTimestamp = performance.timeOrigin / 1000; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration()], + tracesSampleRate: 1, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-update-txn-name/subject.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-update-txn-name/subject.js new file mode 100644 index 000000000000..6e93018cc063 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-update-txn-name/subject.js @@ -0,0 +1,3 @@ +const activeSpan = Sentry.getActiveSpan(); +const rootSpan = activeSpan && Sentry.getRootSpan(activeSpan); +rootSpan?.updateName('new name'); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-update-txn-name/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-update-txn-name/test.ts new file mode 100644 index 000000000000..0470ed6054cc --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-update-txn-name/test.ts @@ -0,0 +1,36 @@ +import { expect } from '@playwright/test'; +import type { Event } from '@sentry/types'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, +} from '@sentry/browser'; + +sentryTest('sets the source to custom when updating the transaction name', async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + const traceContextData = eventData.contexts?.trace?.data; + + expect(traceContextData).toMatchObject({ + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', + [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', + }); + + expect(eventData.transaction).toBe('new name'); + + expect(eventData.contexts?.trace?.op).toBe('pageload'); + expect(eventData.spans?.length).toBeGreaterThan(0); + expect(eventData.transaction_info?.source).toEqual('custom'); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload/test.ts index 6a186b63b02a..7fc2f287488a 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload/test.ts @@ -3,8 +3,14 @@ import type { Event } from '@sentry/types'; import { sentryTest } from '../../../../utils/fixtures'; import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, +} from '@sentry/browser'; -sentryTest('should create a pageload transaction', async ({ getLocalTestPath, page }) => { +sentryTest('creates a pageload transaction with url as source', async ({ getLocalTestPath, page }) => { if (shouldSkipTracingTest()) { sentryTest.skip(); } @@ -16,8 +22,17 @@ sentryTest('should create a pageload transaction', async ({ getLocalTestPath, pa const { start_timestamp: startTimestamp } = eventData; + const traceContextData = eventData.contexts?.trace?.data; + expect(startTimestamp).toBeCloseTo(timeOrigin, 1); + expect(traceContextData).toMatchObject({ + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', + [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', + }); + expect(eventData.contexts?.trace?.op).toBe('pageload'); expect(eventData.spans?.length).toBeGreaterThan(0); expect(eventData.transaction_info?.source).toEqual('url'); diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts index 54f59386ff17..4f0096b29773 100644 --- a/packages/core/src/tracing/sentrySpan.ts +++ b/packages/core/src/tracing/sentrySpan.ts @@ -193,6 +193,7 @@ export class SentrySpan implements Span { */ public updateName(name: string): this { this._name = name; + this.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'custom'); return this; } diff --git a/packages/core/test/lib/tracing/sentrySpan.test.ts b/packages/core/test/lib/tracing/sentrySpan.test.ts index 9698ab5e3398..8e43123e3f3c 100644 --- a/packages/core/test/lib/tracing/sentrySpan.test.ts +++ b/packages/core/test/lib/tracing/sentrySpan.test.ts @@ -1,5 +1,5 @@ import { timestampInSeconds } from '@sentry/utils'; -import { setCurrentClient } from '../../../src'; +import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, setCurrentClient } from '../../../src'; import { SentrySpan } from '../../../src/tracing/sentrySpan'; import { SPAN_STATUS_ERROR } from '../../../src/tracing/spanstatus'; import { TRACE_FLAG_NONE, TRACE_FLAG_SAMPLED, spanToJSON } from '../../../src/utils/spanUtils'; @@ -20,6 +20,19 @@ describe('SentrySpan', () => { expect(spanToJSON(span).description).toEqual('new name'); }); + + it('sets the source to custom when calling updateName', () => { + const span = new SentrySpan({ + name: 'original name', + attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url' }, + }); + + span.updateName('new name'); + + const spanJson = spanToJSON(span); + expect(spanJson.description).toEqual('new name'); + expect(spanJson.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]).toEqual('custom'); + }); }); describe('setters', () => { From cf1778047a7e15b152de157bb0d4ba1560a2cb57 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 13 Nov 2024 11:25:45 +0100 Subject: [PATCH 2/5] formatting --- .../suites/public-api/startSpan/basic/test.ts | 10 +++++----- .../pageload-update-txn-name/test.ts | 4 ++-- .../tracing/browserTracingIntegration/pageload/test.ts | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts index 9f9157b6aca5..3b64a1230a5b 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts @@ -1,16 +1,16 @@ import { expect } from '@playwright/test'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, +} from '@sentry/browser'; import { sentryTest } from '../../../../utils/fixtures'; import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequestOnUrl, } from '../../../../utils/helpers'; -import { - SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, - SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, -} from '@sentry/browser'; sentryTest( 'sends a transaction in an envelope with manual origin and custom source', diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-update-txn-name/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-update-txn-name/test.ts index 0470ed6054cc..ff47f1a2d238 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-update-txn-name/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-update-txn-name/test.ts @@ -1,14 +1,14 @@ import { expect } from '@playwright/test'; import type { Event } from '@sentry/types'; -import { sentryTest } from '../../../../utils/fixtures'; -import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, } from '@sentry/browser'; +import { sentryTest } from '../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers'; sentryTest('sets the source to custom when updating the transaction name', async ({ getLocalTestPath, page }) => { if (shouldSkipTracingTest()) { diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload/test.ts index 7fc2f287488a..70f719d8dbbf 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload/test.ts @@ -1,14 +1,14 @@ import { expect } from '@playwright/test'; import type { Event } from '@sentry/types'; -import { sentryTest } from '../../../../utils/fixtures'; -import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, } from '@sentry/browser'; +import { sentryTest } from '../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers'; sentryTest('creates a pageload transaction with url as source', async ({ getLocalTestPath, page }) => { if (shouldSkipTracingTest()) { From 0f12ced3e3be697728444e24de10d9b5bc4a81d0 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 13 Nov 2024 11:38:52 +0100 Subject: [PATCH 3/5] size limit increase for vue+tracing --- .size-limit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.size-limit.js b/.size-limit.js index 4903d38fef62..3f1be5b9c140 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -139,7 +139,7 @@ module.exports = [ path: 'packages/vue/build/esm/index.js', import: createImport('init', 'browserTracingIntegration'), gzip: true, - limit: '38 KB', + limit: '38.5 KB', }, // Svelte SDK (ESM) { From 22324583b4dce8a240ebb5bd5b4c43812c7abf2a Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 13 Nov 2024 16:10:23 +0100 Subject: [PATCH 4/5] add browser-specific source updates after calling updateName --- .../src/client/routing/appRouterRoutingInstrumentation.ts | 3 +++ packages/solid/src/solidrouter.ts | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts b/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts index c44ef444fdf7..4c92e0999f57 100644 --- a/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts +++ b/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts @@ -61,6 +61,7 @@ export function appRouterInstrumentNavigation(client: Client): void { WINDOW.addEventListener('popstate', () => { if (currentNavigationSpan && currentNavigationSpan.isRecording()) { currentNavigationSpan.updateName(WINDOW.location.pathname); + currentNavigationSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'url'); } else { currentNavigationSpan = startBrowserTracingNavigationSpan(client, { name: WINDOW.location.pathname, @@ -105,9 +106,11 @@ export function appRouterInstrumentNavigation(client: Client): void { if (routerFunctionName === 'push') { span?.updateName(transactionNameifyRouterArgument(argArray[0])); + span?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'url'); span?.setAttribute('navigation.type', 'router.push'); } else if (routerFunctionName === 'replace') { span?.updateName(transactionNameifyRouterArgument(argArray[0])); + span?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'url'); span?.setAttribute('navigation.type', 'router.replace'); } else if (routerFunctionName === 'back') { span?.setAttribute('navigation.type', 'router.back'); diff --git a/packages/solid/src/solidrouter.ts b/packages/solid/src/solidrouter.ts index da0391dea35e..4fc774f379f9 100644 --- a/packages/solid/src/solidrouter.ts +++ b/packages/solid/src/solidrouter.ts @@ -82,13 +82,14 @@ function withSentryRouterRoot(Root: Component): Component Date: Wed, 13 Nov 2024 16:36:04 +0100 Subject: [PATCH 5/5] simplify solid router --- packages/solid/src/solidrouter.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/solid/src/solidrouter.ts b/packages/solid/src/solidrouter.ts index 4fc774f379f9..b4c0972decff 100644 --- a/packages/solid/src/solidrouter.ts +++ b/packages/solid/src/solidrouter.ts @@ -82,14 +82,14 @@ function withSentryRouterRoot(Root: Component): Component