From abd79ea75850fa24ab41be6c0e40474b97ac654f Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 15 Apr 2024 17:26:44 +0200 Subject: [PATCH 01/21] test(browser-integration-tests): Test that errors during pageload/navigation have correct trace id (#11610) Adds two tests to ensure errors thrown during pageload or navigations have the correct traceId. --------- Co-authored-by: Luca Forstner --- .../tracing/trace-lifetime/navigation/test.ts | 43 ++++++++++++++++--- .../tracing/trace-lifetime/pageload/test.ts | 30 ++++++++++++- 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts index 559fba5a8e06..bf4e4445b0af 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts @@ -1,9 +1,13 @@ import { expect } from '@playwright/test'; import type { Event } from '@sentry/types'; import { sentryTest } from '../../../../utils/fixtures'; -import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers'; +import { + getFirstSentryEnvelopeRequest, + getMultipleSentryEnvelopeRequests, + shouldSkipTracingTest, +} from '../../../../utils/helpers'; -sentryTest('should create a new trace on each navigation', async ({ getLocalTestPath, page }) => { +sentryTest('creates a new trace on each navigation', async ({ getLocalTestPath, page }) => { if (shouldSkipTracingTest()) { sentryTest.skip(); } @@ -32,13 +36,13 @@ sentryTest('error after navigation has navigation traceId', async ({ getLocalTes const url = await getLocalTestPath({ testDir: __dirname }); - // ensure navigation transaction is finished + // ensure pageload transaction is finished await getFirstSentryEnvelopeRequest(page, url); - const navigationEvent1 = await getFirstSentryEnvelopeRequest(page, `${url}#foo`); - expect(navigationEvent1.contexts?.trace?.op).toBe('navigation'); + const navigationEvent = await getFirstSentryEnvelopeRequest(page, `${url}#foo`); + expect(navigationEvent.contexts?.trace?.op).toBe('navigation'); - const navigationTraceId = navigationEvent1.contexts?.trace?.trace_id; + const navigationTraceId = navigationEvent.contexts?.trace?.trace_id; expect(navigationTraceId).toMatch(/^[0-9a-f]{32}$/); const [, errorEvent] = await Promise.all([ @@ -49,3 +53,30 @@ sentryTest('error after navigation has navigation traceId', async ({ getLocalTes const errorTraceId = errorEvent.contexts?.trace?.trace_id; expect(errorTraceId).toBe(navigationTraceId); }); + +sentryTest('error during navigation has new navigation traceId', async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + // ensure navigation transaction is finished + await getFirstSentryEnvelopeRequest(page, url); + + const envelopeRequestsPromise = getMultipleSentryEnvelopeRequests(page, 2); + await page.goto(`${url}#foo`); + await page.locator('#errorBtn').click(); + const events = await envelopeRequestsPromise; + + const navigationEvent = events.find(event => event.type === 'transaction'); + const errorEvent = events.find(event => !event.type); + + expect(navigationEvent?.contexts?.trace?.op).toBe('navigation'); + + const navigationTraceId = navigationEvent?.contexts?.trace?.trace_id; + expect(navigationTraceId).toMatch(/^[0-9a-f]{32}$/); + + const errorTraceId = errorEvent?.contexts?.trace?.trace_id; + expect(errorTraceId).toBe(navigationTraceId); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts index 16659f013dd0..d1a847e01e2c 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts @@ -1,7 +1,11 @@ import { expect } from '@playwright/test'; import type { Event } from '@sentry/types'; import { sentryTest } from '../../../../utils/fixtures'; -import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers'; +import { + getFirstSentryEnvelopeRequest, + getMultipleSentryEnvelopeRequests, + shouldSkipTracingTest, +} from '../../../../utils/helpers'; sentryTest( 'should create a new trace for a navigation after the initial pageload', @@ -48,3 +52,27 @@ sentryTest('error after pageload has pageload traceId', async ({ getLocalTestPat const errorTraceId = errorEvent.contexts?.trace?.trace_id; expect(errorTraceId).toBe(pageloadTraceId); }); + +sentryTest('error during pageload has pageload traceId', async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + const envelopeRequestsPromise = getMultipleSentryEnvelopeRequests(page, 2); + await page.goto(url); + await page.locator('#errorBtn').click(); + const events = await envelopeRequestsPromise; + + const pageloadEvent = events.find(event => event.type === 'transaction'); + const errorEvent = events.find(event => !event.type); + + expect(pageloadEvent?.contexts?.trace?.op).toBe('pageload'); + + const pageloadTraceId = pageloadEvent?.contexts?.trace?.trace_id; + expect(pageloadTraceId).toMatch(/^[0-9a-f]{32}$/); + + const errorTraceId = errorEvent?.contexts?.trace?.trace_id; + expect(errorTraceId).toBe(pageloadTraceId); +}); From 8cb70e3b4862e5d34fc41e5d6981ec0a0660eab0 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 16 Apr 2024 09:19:11 +0200 Subject: [PATCH 02/21] test(browser-integration-tests): Test trace lifetime for outgoing `fetch` requests (#11614) Add test for `sentry-trace` and `baggage` header values so that the headers contain the correct traceId in the folllowing four scenarios: * request during pageload span * request after pageload span * request during navigation span * request after navigation span --- .../suites/tracing/trace-lifetime/init.js | 1 + .../tracing/trace-lifetime/navigation/test.ts | 71 +++++++++++++++++-- .../tracing/trace-lifetime/pageload/test.ts | 58 +++++++++++++++ .../suites/tracing/trace-lifetime/subject.js | 5 ++ .../tracing/trace-lifetime/template.html | 1 + 5 files changed, 132 insertions(+), 4 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/init.js b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/init.js index 83076460599f..7cd076a052e5 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/init.js +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/init.js @@ -5,5 +5,6 @@ window.Sentry = Sentry; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', integrations: [Sentry.browserTracingIntegration()], + tracePropagationTargets: ['http://example.com'], tracesSampleRate: 1, }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts index bf4e4445b0af..0ed99e3b7ec9 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts @@ -45,10 +45,9 @@ sentryTest('error after navigation has navigation traceId', async ({ getLocalTes const navigationTraceId = navigationEvent.contexts?.trace?.trace_id; expect(navigationTraceId).toMatch(/^[0-9a-f]{32}$/); - const [, errorEvent] = await Promise.all([ - page.locator('#errorBtn').click(), - getFirstSentryEnvelopeRequest(page), - ]); + const errorEventPromise = getFirstSentryEnvelopeRequest(page); + await page.locator('#errorBtn').click(); + const errorEvent = await errorEventPromise; const errorTraceId = errorEvent.contexts?.trace?.trace_id; expect(errorTraceId).toBe(navigationTraceId); @@ -80,3 +79,67 @@ sentryTest('error during navigation has new navigation traceId', async ({ getLoc const errorTraceId = errorEvent?.contexts?.trace?.trace_id; expect(errorTraceId).toBe(navigationTraceId); }); + +sentryTest( + 'outgoing fetch request after navigation has navigation traceId in headers', + async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + // ensure navigation transaction is finished + await getFirstSentryEnvelopeRequest(page, url); + + const navigationEvent = await getFirstSentryEnvelopeRequest(page, `${url}#foo`); + expect(navigationEvent.contexts?.trace?.op).toBe('navigation'); + + const navigationTraceId = navigationEvent.contexts?.trace?.trace_id; + expect(navigationTraceId).toMatch(/^[0-9a-f]{32}$/); + + const requestPromise = page.waitForRequest('http://example.com/*'); + await page.locator('#fetchBtn').click(); + const request = await requestPromise; + const headers = request.headers(); + + // sampling decision is deferred b/c of no active span at the time of request + expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}$`)); + expect(headers['baggage']).toEqual( + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId}`, + ); + }, +); + +sentryTest( + 'outgoing fetch request during navigation has navigation traceId in headers', + async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + // ensure navigation transaction is finished + await getFirstSentryEnvelopeRequest(page, url); + + const navigationEventPromise = getFirstSentryEnvelopeRequest(page); + const requestPromise = page.waitForRequest('http://example.com/*'); + await page.goto(`${url}#foo`); + await page.locator('#fetchBtn').click(); + const [navigationEvent, request] = await Promise.all([navigationEventPromise, requestPromise]); + + expect(navigationEvent.contexts?.trace?.op).toBe('navigation'); + + const navigationTraceId = navigationEvent.contexts?.trace?.trace_id; + expect(navigationTraceId).toMatch(/^[0-9a-f]{32}$/); + + const headers = request.headers(); + + // sampling decision is propagated from active span sampling decision + expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}-1$`)); + expect(headers['baggage']).toEqual( + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sample_rate=1,sentry-sampled=true`, + ); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts index d1a847e01e2c..69141a107bbc 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts @@ -76,3 +76,61 @@ sentryTest('error during pageload has pageload traceId', async ({ getLocalTestPa const errorTraceId = errorEvent?.contexts?.trace?.trace_id; expect(errorTraceId).toBe(pageloadTraceId); }); + +sentryTest( + 'outgoing fetch request after pageload has pageload traceId in headers', + async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + const pageloadEvent = await getFirstSentryEnvelopeRequest(page, url); + expect(pageloadEvent.contexts?.trace?.op).toBe('pageload'); + + const pageloadTraceId = pageloadEvent.contexts?.trace?.trace_id; + expect(pageloadTraceId).toMatch(/^[0-9a-f]{32}$/); + + const requestPromise = page.waitForRequest('http://example.com/*'); + await page.locator('#fetchBtn').click(); + const request = await requestPromise; + const headers = request.headers(); + + // sampling decision is deferred b/c of no active span at the time of request + expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}$`)); + expect(headers['baggage']).toEqual( + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId}`, + ); + }, +); + +sentryTest( + 'outgoing fetch request during pageload has pageload traceId in headers', + async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + const pageloadEventPromise = getFirstSentryEnvelopeRequest(page); + const requestPromise = page.waitForRequest('http://example.com/*'); + await page.goto(url); + await page.locator('#fetchBtn').click(); + const [pageloadEvent, request] = await Promise.all([pageloadEventPromise, requestPromise]); + + expect(pageloadEvent.contexts?.trace?.op).toBe('pageload'); + + const navigationTraceId = pageloadEvent.contexts?.trace?.trace_id; + expect(navigationTraceId).toMatch(/^[0-9a-f]{32}$/); + + const headers = request.headers(); + + // sampling decision is propagated from active span sampling decision + expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}-1$`)); + expect(headers['baggage']).toEqual( + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sample_rate=1,sentry-sampled=true`, + ); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/subject.js b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/subject.js index 5131ea7631e9..38e475a5f216 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/subject.js +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/subject.js @@ -2,3 +2,8 @@ const errorBtn = document.getElementById('errorBtn'); errorBtn.addEventListener('click', () => { throw new Error('Sentry Test Error'); }); + +const fetchBtn = document.getElementById('fetchBtn'); +fetchBtn.addEventListener('click', async () => { + await fetch('http://example.com'); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/template.html b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/template.html index a29ad2056a45..218c42031ccf 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/template.html +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/template.html @@ -5,5 +5,6 @@ + From 39c8290ab8e96f92cb127692e09ad24d8068a00f Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 16 Apr 2024 12:00:13 +0200 Subject: [PATCH 03/21] test(browser-integration-tests): Add trace lifetime tests for `` tag pageload transactions (#11622) Add a couple of tests to our `trace-lifetime` integration test suite to test `` tag continued pageload traces. These differ from "normal" pageload traces in the sense that they should pick up and continue the trace from the service that added the meta tags to the HTML response. So in our suite: - `trace-lifetime/pageload` - pageload is head of trace - `trace-lifetime/pageload-meta` - pageload continues trace from server --- .../suites/tracing/trace-lifetime/README.md | 9 + .../pageload-meta/template.html | 13 ++ .../trace-lifetime/pageload-meta/test.ts | 159 ++++++++++++++++++ 3 files changed, 181 insertions(+) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/README.md create mode 100644 dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/template.html create mode 100644 dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/README.md b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/README.md new file mode 100644 index 000000000000..d7330f233425 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/README.md @@ -0,0 +1,9 @@ +Tests in this suite are meant to test the lifetime of a trace in the browser SDK and how different events sent are +connected to a trace. This suite distinguishes the following cases: + +1. `pageload` - Traces started on the initial pageload as head of trace +2. `pageload-meta` - Traces started on the initial pageload as a continuation of the trace on the server (via `` + tags) +3. `navigation` - Traces started during navigations on a page + +Tests scenarios should be fairly similar for all three cases but it's important we test all of them. diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/template.html b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/template.html new file mode 100644 index 000000000000..61372c8605e5 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/template.html @@ -0,0 +1,13 @@ + + + + + + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts new file mode 100644 index 000000000000..2f872b398d87 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts @@ -0,0 +1,159 @@ +import { expect } from '@playwright/test'; +import type { Event } from '@sentry/types'; +import { sentryTest } from '../../../../utils/fixtures'; +import { + getFirstSentryEnvelopeRequest, + getMultipleSentryEnvelopeRequests, + shouldSkipTracingTest, +} from '../../../../utils/helpers'; + +const META_TAG_TRACE_ID = '12345678901234567890123456789012'; +const META_TAG_PARENT_SPAN_ID = '1234567890123456'; +const META_TAG_BAGGAGE = + 'sentry-trace_id=12345678901234567890123456789012,sentry-sample_rate=0.2,sentry-transaction=my-transaction,sentry-public_key=public,sentry-release=1.0.0,sentry-environment=prod'; + +sentryTest( + 'create a new trace for a navigation after the tag pageload trace', + async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + const pageloadEvent = await getFirstSentryEnvelopeRequest(page, url); + const navigationEvent = await getFirstSentryEnvelopeRequest(page, `${url}#foo`); + + const pageloadTraceContext = pageloadEvent.contexts?.trace; + const navigationTraceContext = navigationEvent.contexts?.trace; + + expect(pageloadTraceContext).toMatchObject({ + op: 'pageload', + trace_id: META_TAG_TRACE_ID, + parent_span_id: META_TAG_PARENT_SPAN_ID, + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + expect(navigationTraceContext).toMatchObject({ + op: 'navigation', + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + // navigation span is head of trace, so there's no parent span: + expect(navigationTraceContext?.trace_id).not.toHaveProperty('parent_span_id'); + + expect(pageloadTraceContext?.trace_id).not.toEqual(navigationTraceContext?.trace_id); + }, +); + +sentryTest('error after tag pageload has pageload traceId', async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + const pageloadEvent = await getFirstSentryEnvelopeRequest(page, url); + expect(pageloadEvent.contexts?.trace).toMatchObject({ + op: 'pageload', + trace_id: META_TAG_TRACE_ID, + parent_span_id: META_TAG_PARENT_SPAN_ID, + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + + const errorEventPromise = getFirstSentryEnvelopeRequest(page); + await page.locator('#errorBtn').click(); + const errorEvent = await errorEventPromise; + + expect(errorEvent.contexts?.trace).toMatchObject({ + trace_id: META_TAG_TRACE_ID, + parent_span_id: META_TAG_PARENT_SPAN_ID, + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); +}); + +sentryTest('error during tag pageload has pageload traceId', async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + const envelopeRequestsPromise = getMultipleSentryEnvelopeRequests(page, 2); + await page.goto(url); + await page.locator('#errorBtn').click(); + const events = await envelopeRequestsPromise; + + const pageloadEvent = events.find(event => event.type === 'transaction'); + const errorEvent = events.find(event => !event.type); + + expect(pageloadEvent?.contexts?.trace).toMatchObject({ + op: 'pageload', + trace_id: META_TAG_TRACE_ID, + parent_span_id: META_TAG_PARENT_SPAN_ID, + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + + expect(errorEvent?.contexts?.trace).toMatchObject({ + trace_id: META_TAG_TRACE_ID, + parent_span_id: META_TAG_PARENT_SPAN_ID, + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); +}); + +sentryTest( + 'outgoing fetch request after tag pageload has pageload traceId in headers', + async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + const pageloadEvent = await getFirstSentryEnvelopeRequest(page, url); + expect(pageloadEvent?.contexts?.trace).toMatchObject({ + op: 'pageload', + trace_id: META_TAG_TRACE_ID, + parent_span_id: META_TAG_PARENT_SPAN_ID, + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + + const requestPromise = page.waitForRequest('http://example.com/*'); + await page.locator('#fetchBtn').click(); + const request = await requestPromise; + const headers = request.headers(); + + // sampling decision is propagated from meta tag's sentry-trace sampled flag + expect(headers['sentry-trace']).toMatch(new RegExp(`^${META_TAG_TRACE_ID}-[0-9a-f]{16}-1$`)); + expect(headers['baggage']).toBe(META_TAG_BAGGAGE); + }, +); + +sentryTest( + 'outgoing fetch request during tag pageload has pageload traceId in headers', + async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + const pageloadEventPromise = getFirstSentryEnvelopeRequest(page); + const requestPromise = page.waitForRequest('http://example.com/*'); + await page.goto(url); + await page.locator('#fetchBtn').click(); + const [pageloadEvent, request] = await Promise.all([pageloadEventPromise, requestPromise]); + + expect(pageloadEvent?.contexts?.trace).toMatchObject({ + op: 'pageload', + trace_id: META_TAG_TRACE_ID, + parent_span_id: META_TAG_PARENT_SPAN_ID, + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + + const headers = request.headers(); + + // sampling decision is propagated from meta tag's sentry-trace sampled flag + expect(headers['sentry-trace']).toMatch(new RegExp(`^${META_TAG_TRACE_ID}-[0-9a-f]{16}-1$`)); + expect(headers['baggage']).toBe(META_TAG_BAGGAGE); + }, +); From ee4091e8850dac6f6cd8970e814f2c7f74d1a74f Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 16 Apr 2024 14:21:09 +0200 Subject: [PATCH 04/21] fix(browser): Don't assume window.document is available (#11602) --- packages/browser-utils/src/metrics/types.ts | 5 ++++- .../src/metrics/web-vitals/getLCP.ts | 11 +++++++---- .../web-vitals/lib/getVisibilityWatcher.ts | 16 ++++++---------- .../src/metrics/web-vitals/lib/initMetric.ts | 4 ++-- .../src/metrics/web-vitals/lib/onHidden.ts | 13 ++++++++----- .../src/metrics/web-vitals/lib/whenActivated.ts | 2 +- .../src/metrics/web-vitals/onTTFB.ts | 4 ++-- .../src/tracing/browserTracingIntegration.ts | 4 +++- 8 files changed, 33 insertions(+), 26 deletions(-) diff --git a/packages/browser-utils/src/metrics/types.ts b/packages/browser-utils/src/metrics/types.ts index 068e4e058d32..c32d08c7f8e2 100644 --- a/packages/browser-utils/src/metrics/types.ts +++ b/packages/browser-utils/src/metrics/types.ts @@ -1,3 +1,6 @@ import { GLOBAL_OBJ } from '@sentry/utils'; -export const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window; +export const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & + // document is not available in all browser environments (webworkers). We make it optional so you have to explicitly check for it + Omit & + Partial>; diff --git a/packages/browser-utils/src/metrics/web-vitals/getLCP.ts b/packages/browser-utils/src/metrics/web-vitals/getLCP.ts index facda8ce7a9d..db1bd90fb71d 100644 --- a/packages/browser-utils/src/metrics/web-vitals/getLCP.ts +++ b/packages/browser-utils/src/metrics/web-vitals/getLCP.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { WINDOW } from '../types'; import { bindReporter } from './lib/bindReporter'; import { getActivationStart } from './lib/getActivationStart'; import { getVisibilityWatcher } from './lib/getVisibilityWatcher'; @@ -82,10 +83,12 @@ export const onLCP = (onReport: LCPReportCallback, opts: ReportOpts = {}) => { // stops LCP observation, it's unreliable since it can be programmatically // generated. See: https://github.com/GoogleChrome/web-vitals/issues/75 ['keydown', 'click'].forEach(type => { - // Wrap in a setTimeout so the callback is run in a separate task - // to avoid extending the keyboard/click handler to reduce INP impact - // https://github.com/GoogleChrome/web-vitals/issues/383 - addEventListener(type, () => setTimeout(stopListening, 0), true); + if (WINDOW.document) { + // Wrap in a setTimeout so the callback is run in a separate task + // to avoid extending the keyboard/click handler to reduce INP impact + // https://github.com/GoogleChrome/web-vitals/issues/383 + addEventListener(type, () => setTimeout(stopListening, 0), true); + } }); onHidden(stopListening); diff --git a/packages/browser-utils/src/metrics/web-vitals/lib/getVisibilityWatcher.ts b/packages/browser-utils/src/metrics/web-vitals/lib/getVisibilityWatcher.ts index 2cff287b2ae7..6fe3755f6f59 100644 --- a/packages/browser-utils/src/metrics/web-vitals/lib/getVisibilityWatcher.ts +++ b/packages/browser-utils/src/metrics/web-vitals/lib/getVisibilityWatcher.ts @@ -24,13 +24,13 @@ const initHiddenTime = () => { // that visibility state is always 'hidden' during prerendering, so we have // to ignore that case until prerendering finishes (see: `prerenderingchange` // event logic below). - return WINDOW.document.visibilityState === 'hidden' && !WINDOW.document.prerendering ? 0 : Infinity; + firstHiddenTime = WINDOW.document!.visibilityState === 'hidden' && !WINDOW.document!.prerendering ? 0 : Infinity; }; const onVisibilityUpdate = (event: Event) => { // If the document is 'hidden' and no previous hidden timestamp has been // set, update it based on the current event data. - if (WINDOW.document.visibilityState === 'hidden' && firstHiddenTime > -1) { + if (WINDOW.document!.visibilityState === 'hidden' && firstHiddenTime > -1) { // If the event is a 'visibilitychange' event, it means the page was // visible prior to this change, so the event timestamp is the first // hidden time. @@ -41,7 +41,8 @@ const onVisibilityUpdate = (event: Event) => { firstHiddenTime = event.type === 'visibilitychange' ? event.timeStamp : 0; // Remove all listeners now that a `firstHiddenTime` value has been set. - removeChangeListeners(); + removeEventListener('visibilitychange', onVisibilityUpdate, true); + removeEventListener('prerenderingchange', onVisibilityUpdate, true); } }; @@ -54,18 +55,13 @@ const addChangeListeners = () => { addEventListener('prerenderingchange', onVisibilityUpdate, true); }; -const removeChangeListeners = () => { - removeEventListener('visibilitychange', onVisibilityUpdate, true); - removeEventListener('prerenderingchange', onVisibilityUpdate, true); -}; - export const getVisibilityWatcher = () => { - if (firstHiddenTime < 0) { + if (WINDOW.document && firstHiddenTime < 0) { // If the document is hidden when this code runs, assume it was hidden // since navigation start. This isn't a perfect heuristic, but it's the // best we can do until an API is available to support querying past // visibilityState. - firstHiddenTime = initHiddenTime(); + initHiddenTime(); addChangeListeners(); } return { diff --git a/packages/browser-utils/src/metrics/web-vitals/lib/initMetric.ts b/packages/browser-utils/src/metrics/web-vitals/lib/initMetric.ts index 9098227ae1a4..386333b7eb2d 100644 --- a/packages/browser-utils/src/metrics/web-vitals/lib/initMetric.ts +++ b/packages/browser-utils/src/metrics/web-vitals/lib/initMetric.ts @@ -25,9 +25,9 @@ export const initMetric = (name: MetricNa let navigationType: MetricType['navigationType'] = 'navigate'; if (navEntry) { - if (WINDOW.document.prerendering || getActivationStart() > 0) { + if ((WINDOW.document && WINDOW.document.prerendering) || getActivationStart() > 0) { navigationType = 'prerender'; - } else if (WINDOW.document.wasDiscarded) { + } else if (WINDOW.document && WINDOW.document.wasDiscarded) { navigationType = 'restore'; } else if (navEntry.type) { navigationType = navEntry.type.replace(/_/g, '-') as MetricType['navigationType']; diff --git a/packages/browser-utils/src/metrics/web-vitals/lib/onHidden.ts b/packages/browser-utils/src/metrics/web-vitals/lib/onHidden.ts index f9ec1dc94b90..9f81c7369007 100644 --- a/packages/browser-utils/src/metrics/web-vitals/lib/onHidden.ts +++ b/packages/browser-utils/src/metrics/web-vitals/lib/onHidden.ts @@ -22,12 +22,15 @@ export interface OnHiddenCallback { export const onHidden = (cb: OnHiddenCallback) => { const onHiddenOrPageHide = (event: Event) => { - if (event.type === 'pagehide' || WINDOW.document.visibilityState === 'hidden') { + if (event.type === 'pagehide' || (WINDOW.document && WINDOW.document.visibilityState === 'hidden')) { cb(event); } }; - addEventListener('visibilitychange', onHiddenOrPageHide, true); - // Some browsers have buggy implementations of visibilitychange, - // so we use pagehide in addition, just to be safe. - addEventListener('pagehide', onHiddenOrPageHide, true); + + if (WINDOW.document) { + addEventListener('visibilitychange', onHiddenOrPageHide, true); + // Some browsers have buggy implementations of visibilitychange, + // so we use pagehide in addition, just to be safe. + addEventListener('pagehide', onHiddenOrPageHide, true); + } }; diff --git a/packages/browser-utils/src/metrics/web-vitals/lib/whenActivated.ts b/packages/browser-utils/src/metrics/web-vitals/lib/whenActivated.ts index a04af1dd0376..183a8566aeb4 100644 --- a/packages/browser-utils/src/metrics/web-vitals/lib/whenActivated.ts +++ b/packages/browser-utils/src/metrics/web-vitals/lib/whenActivated.ts @@ -17,7 +17,7 @@ import { WINDOW } from '../../types'; export const whenActivated = (callback: () => void) => { - if (WINDOW.document.prerendering) { + if (WINDOW.document && WINDOW.document.prerendering) { addEventListener('prerenderingchange', () => callback(), true); } else { callback(); diff --git a/packages/browser-utils/src/metrics/web-vitals/onTTFB.ts b/packages/browser-utils/src/metrics/web-vitals/onTTFB.ts index 13e6c6679309..993af7ca074e 100644 --- a/packages/browser-utils/src/metrics/web-vitals/onTTFB.ts +++ b/packages/browser-utils/src/metrics/web-vitals/onTTFB.ts @@ -30,9 +30,9 @@ export const TTFBThresholds: MetricRatingThresholds = [800, 1800]; * @param callback */ const whenReady = (callback: () => void) => { - if (WINDOW.document.prerendering) { + if (WINDOW.document && WINDOW.document.prerendering) { whenActivated(() => whenReady(callback)); - } else if (WINDOW.document.readyState !== 'complete') { + } else if (WINDOW.document && WINDOW.document.readyState !== 'complete') { addEventListener('load', () => whenReady(callback), true); } else { // Queue a task so the callback runs after `loadEventEnd`. diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index 16e715b15ad2..64510fc0b93f 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -448,6 +448,8 @@ function registerInteractionListener( }; ['click'].forEach(type => { - addEventListener(type, registerInteractionTransaction, { once: false, capture: true }); + if (WINDOW.document) { + addEventListener(type, registerInteractionTransaction, { once: false, capture: true }); + } }); } From 452bcae0ddf203f18866c73ed4ce0f415f100596 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 16 Apr 2024 15:48:49 +0200 Subject: [PATCH 05/21] fix(node): Allow use of `NodeClient` without calling `init` (#11585) This needs exporting so that the `NodeClient` can be used without calling the Node `init`. There is a lot called in the Node init that is a hard requirement for the `NodeClient` to function. Should these not be called in the constructor? --- .../node-exports-test-app/scripts/consistentExports.ts | 2 ++ packages/node/src/index.ts | 10 +++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts b/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts index 0aceb4418ddc..914b569f978e 100644 --- a/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts +++ b/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts @@ -14,6 +14,8 @@ const NODE_EXPORTS_IGNORE = [ 'default', // Probably generated by transpilation, no need to require it '__esModule', + // Only required from the Node package + 'setNodeAsyncContextStrategy', ]; const nodeExports = Object.keys(SentryNode).filter(e => !NODE_EXPORTS_IGNORE.includes(e)); diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index ee037c6fdc8b..1ece2f0d81b0 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -37,9 +37,13 @@ export type { NodeOptions } from './types'; export { addRequestDataToEvent, DEFAULT_USER_INCLUDES, extractRequestData } from '@sentry/utils'; -// These are custom variants that need to be used instead of the core one -// As they have slightly different implementations -export { continueTrace } from '@sentry/opentelemetry'; +export { + // These are custom variants that need to be used instead of the core one + // As they have slightly different implementations + continueTrace, + // This needs exporting so the NodeClient can be used without calling init + setOpenTelemetryContextAsyncContextStrategy as setNodeAsyncContextStrategy, +} from '@sentry/opentelemetry'; export { addBreadcrumb, From 1a22856abb1116b6d79a643b14a94b03497fe4f0 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 16 Apr 2024 15:49:26 +0200 Subject: [PATCH 06/21] feat(node): Collect Local Variables via a worker (#11586) Our Local Variables integration is limited by the fact that it executes debugger commands from the main app thread. This means that no async work can be completed while the debugger is paused and local variables are being collected. This in turn means that all the synchronous work needs to be queued which risks stack overflows. For this reason we've limited local variable collection to the 5 frames closest to the top of the stack. When debugging from a worker thread, there are no such limitations around performing async work during debugger pause. This PR adds a worker thread which is tasked with collecting local variables when the debugger pauses. It passes these back to the main thread via IPC which are then later added to event stack frames. ## Possible Improvements It's possible to combine both ANR and Local Variables into a single worker thread. --- dev-packages/rollup-utils/utils.mjs | 2 +- packages/node/rollup.anr-worker.config.mjs | 15 +- packages/node/rollup.npm.config.mjs | 22 ++- packages/node/src/integrations/anr/index.ts | 4 +- .../src/integrations/anr/worker-script.ts | 2 - .../integrations/local-variables/common.ts | 13 ++ .../src/integrations/local-variables/index.ts | 8 +- .../local-variables/inspector.d.ts | 31 +++ .../local-variables/local-variables-async.ts | 141 ++++++++++++++ .../integrations/local-variables/worker.ts | 182 ++++++++++++++++++ 10 files changed, 402 insertions(+), 18 deletions(-) delete mode 100644 packages/node/src/integrations/anr/worker-script.ts create mode 100644 packages/node/src/integrations/local-variables/inspector.d.ts create mode 100644 packages/node/src/integrations/local-variables/local-variables-async.ts create mode 100644 packages/node/src/integrations/local-variables/worker.ts diff --git a/dev-packages/rollup-utils/utils.mjs b/dev-packages/rollup-utils/utils.mjs index 81fed5949f97..94b8c4483c23 100644 --- a/dev-packages/rollup-utils/utils.mjs +++ b/dev-packages/rollup-utils/utils.mjs @@ -21,7 +21,7 @@ export function mergePlugins(pluginsA, pluginsB) { // here. // Additionally, the excludeReplay plugin must run before TS/Sucrase so that we can eliminate the replay code // before anything is type-checked (TS-only) and transpiled. - const order = ['excludeReplay', 'typescript', 'sucrase', '...', 'terser', 'license']; + const order = ['excludeReplay', 'typescript', 'sucrase', '...', 'terser', 'license', 'output-base64-worker-script']; const sortKeyA = order.includes(a.name) ? a.name : '...'; const sortKeyB = order.includes(b.name) ? b.name : '...'; diff --git a/packages/node/rollup.anr-worker.config.mjs b/packages/node/rollup.anr-worker.config.mjs index bd3c1d4b825c..260f889cacb6 100644 --- a/packages/node/rollup.anr-worker.config.mjs +++ b/packages/node/rollup.anr-worker.config.mjs @@ -1,17 +1,18 @@ import { makeBaseBundleConfig } from '@sentry-internal/rollup-utils'; -export function createAnrWorkerCode() { +export function createWorkerCodeBuilder(entry, outDir) { let base64Code; - return { - workerRollupConfig: makeBaseBundleConfig({ + return [ + makeBaseBundleConfig({ bundleType: 'node-worker', - entrypoints: ['src/integrations/anr/worker.ts'], + entrypoints: [entry], + sucrase: { disableESTransforms: true }, licenseTitle: '@sentry/node', outputFileBase: () => 'worker-script.js', packageSpecificConfig: { output: { - dir: 'build/esm/integrations/anr', + dir: outDir, sourcemap: false, }, plugins: [ @@ -24,8 +25,8 @@ export function createAnrWorkerCode() { ], }, }), - getBase64Code() { + () => { return base64Code; }, - }; + ]; } diff --git a/packages/node/rollup.npm.config.mjs b/packages/node/rollup.npm.config.mjs index 9622acb20112..c4a621cf7ee3 100644 --- a/packages/node/rollup.npm.config.mjs +++ b/packages/node/rollup.npm.config.mjs @@ -1,13 +1,22 @@ import replace from '@rollup/plugin-replace'; import { makeBaseNPMConfig, makeNPMConfigVariants, makeOtelLoaders } from '@sentry-internal/rollup-utils'; -import { createAnrWorkerCode } from './rollup.anr-worker.config.mjs'; +import { createWorkerCodeBuilder } from './rollup.anr-worker.config.mjs'; -const { workerRollupConfig, getBase64Code } = createAnrWorkerCode(); +const [anrWorkerConfig, getAnrBase64Code] = createWorkerCodeBuilder( + 'src/integrations/anr/worker.ts', + 'build/esm/integrations/anr', +); + +const [localVariablesWorkerConfig, getLocalVariablesBase64Code] = createWorkerCodeBuilder( + 'src/integrations/local-variables/worker.ts', + 'build/esm/integrations/local-variables', +); export default [ ...makeOtelLoaders('./build', 'otel'), - // The worker needs to be built first since it's output is used in the main bundle. - workerRollupConfig, + // The workers needs to be built first since it's their output is copied in the main bundle. + anrWorkerConfig, + localVariablesWorkerConfig, ...makeNPMConfigVariants( makeBaseNPMConfig({ packageSpecificConfig: { @@ -23,10 +32,11 @@ export default [ plugins: [ replace({ delimiters: ['###', '###'], - // removes some webpack warnings + // removes some rollup warnings preventAssignment: true, values: { - base64WorkerScript: () => getBase64Code(), + AnrWorkerScript: () => getAnrBase64Code(), + LocalVariablesWorkerScript: () => getLocalVariablesBase64Code(), }, }), ], diff --git a/packages/node/src/integrations/anr/index.ts b/packages/node/src/integrations/anr/index.ts index 7dbe9e905cb4..2cf32289f082 100644 --- a/packages/node/src/integrations/anr/index.ts +++ b/packages/node/src/integrations/anr/index.ts @@ -7,7 +7,9 @@ import { getCurrentScope, getGlobalScope, getIsolationScope } from '../..'; import { NODE_VERSION } from '../../nodeVersion'; import type { NodeClient } from '../../sdk/client'; import type { AnrIntegrationOptions, WorkerStartData } from './common'; -import { base64WorkerScript } from './worker-script'; + +// This string is a placeholder that gets overwritten with the worker code. +export const base64WorkerScript = '###AnrWorkerScript###'; const DEFAULT_INTERVAL = 50; const DEFAULT_HANG_THRESHOLD = 5000; diff --git a/packages/node/src/integrations/anr/worker-script.ts b/packages/node/src/integrations/anr/worker-script.ts deleted file mode 100644 index c70323e0fc50..000000000000 --- a/packages/node/src/integrations/anr/worker-script.ts +++ /dev/null @@ -1,2 +0,0 @@ -// This string is a placeholder that gets overwritten with the worker code. -export const base64WorkerScript = '###base64WorkerScript###'; diff --git a/packages/node/src/integrations/local-variables/common.ts b/packages/node/src/integrations/local-variables/common.ts index 3ffee8c0a824..990a3d71b061 100644 --- a/packages/node/src/integrations/local-variables/common.ts +++ b/packages/node/src/integrations/local-variables/common.ts @@ -117,3 +117,16 @@ export interface LocalVariablesIntegrationOptions { */ maxExceptionsPerSecond?: number; } + +export interface LocalVariablesWorkerArgs extends LocalVariablesIntegrationOptions { + /** + * Whether to enable debug logging. + */ + debug: boolean; + /** + * Base path used to calculate module name. + * + * Defaults to `dirname(process.argv[1])` and falls back to `process.cwd()` + */ + basePath?: string; +} diff --git a/packages/node/src/integrations/local-variables/index.ts b/packages/node/src/integrations/local-variables/index.ts index 60649b03118f..36db00bdb9aa 100644 --- a/packages/node/src/integrations/local-variables/index.ts +++ b/packages/node/src/integrations/local-variables/index.ts @@ -1,3 +1,9 @@ +import type { Integration } from '@sentry/types'; +import { NODE_VERSION } from '../../nodeVersion'; +import type { LocalVariablesIntegrationOptions } from './common'; +import { localVariablesAsyncIntegration } from './local-variables-async'; import { localVariablesSyncIntegration } from './local-variables-sync'; -export const localVariablesIntegration = localVariablesSyncIntegration; +export const localVariablesIntegration = (options: LocalVariablesIntegrationOptions = {}): Integration => { + return NODE_VERSION.major < 19 ? localVariablesSyncIntegration(options) : localVariablesAsyncIntegration(options); +}; diff --git a/packages/node/src/integrations/local-variables/inspector.d.ts b/packages/node/src/integrations/local-variables/inspector.d.ts new file mode 100644 index 000000000000..9ac6b857dcc0 --- /dev/null +++ b/packages/node/src/integrations/local-variables/inspector.d.ts @@ -0,0 +1,31 @@ +/** + * @types/node doesn't have a `node:inspector/promises` module, maybe because it's still experimental? + */ +declare module 'node:inspector/promises' { + /** + * Async Debugger session + */ + class Session { + public constructor(); + + public connect(): void; + public connectToMainThread(): void; + + public post(method: 'Debugger.pause' | 'Debugger.resume' | 'Debugger.enable' | 'Debugger.disable'): Promise; + public post( + method: 'Debugger.setPauseOnExceptions', + params: Debugger.SetPauseOnExceptionsParameterType, + ): Promise; + public post( + method: 'Runtime.getProperties', + params: Runtime.GetPropertiesParameterType, + ): Promise; + + public on( + event: 'Debugger.paused', + listener: (message: InspectorNotification) => void, + ): Session; + + public on(event: 'Debugger.resumed', listener: () => void): Session; + } +} diff --git a/packages/node/src/integrations/local-variables/local-variables-async.ts b/packages/node/src/integrations/local-variables/local-variables-async.ts new file mode 100644 index 000000000000..b8e827909ea8 --- /dev/null +++ b/packages/node/src/integrations/local-variables/local-variables-async.ts @@ -0,0 +1,141 @@ +import { defineIntegration } from '@sentry/core'; +import type { Event, Exception, IntegrationFn } from '@sentry/types'; +import { LRUMap, logger } from '@sentry/utils'; +import { Worker } from 'worker_threads'; + +import type { NodeClient } from '../../sdk/client'; +import type { FrameVariables, LocalVariablesIntegrationOptions, LocalVariablesWorkerArgs } from './common'; +import { functionNamesMatch, hashFrames } from './common'; + +// This string is a placeholder that gets overwritten with the worker code. +export const base64WorkerScript = '###LocalVariablesWorkerScript###'; + +function log(...args: unknown[]): void { + logger.log('[LocalVariables]', ...args); +} + +/** + * Adds local variables to exception frames + */ +export const localVariablesAsyncIntegration = defineIntegration((( + integrationOptions: LocalVariablesIntegrationOptions = {}, +) => { + const cachedFrames: LRUMap = new LRUMap(20); + + function addLocalVariablesToException(exception: Exception): void { + const hash = hashFrames(exception?.stacktrace?.frames); + + if (hash === undefined) { + return; + } + + // Check if we have local variables for an exception that matches the hash + // remove is identical to get but also removes the entry from the cache + const cachedFrame = cachedFrames.remove(hash); + + if (cachedFrame === undefined) { + return; + } + + // Filter out frames where the function name is `new Promise` since these are in the error.stack frames + // but do not appear in the debugger call frames + const frames = (exception.stacktrace?.frames || []).filter(frame => frame.function !== 'new Promise'); + + for (let i = 0; i < frames.length; i++) { + // Sentry frames are in reverse order + const frameIndex = frames.length - i - 1; + + // Drop out if we run out of frames to match up + if (!frames[frameIndex] || !cachedFrame[i]) { + break; + } + + if ( + // We need to have vars to add + cachedFrame[i].vars === undefined || + // We're not interested in frames that are not in_app because the vars are not relevant + frames[frameIndex].in_app === false || + // The function names need to match + !functionNamesMatch(frames[frameIndex].function, cachedFrame[i].function) + ) { + continue; + } + + frames[frameIndex].vars = cachedFrame[i].vars; + } + } + + function addLocalVariablesToEvent(event: Event): Event { + for (const exception of event.exception?.values || []) { + addLocalVariablesToException(exception); + } + + return event; + } + + async function startInspector(): Promise { + // We load inspector dynamically because on some platforms Node is built without inspector support + const inspector = await import('inspector'); + if (!inspector.url()) { + inspector.open(0); + } + } + + function startWorker(options: LocalVariablesWorkerArgs): void { + const worker = new Worker(new URL(`data:application/javascript;base64,${base64WorkerScript}`), { + workerData: options, + }); + + process.on('exit', () => { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + worker.terminate(); + }); + + worker.on('message', ({ exceptionHash, frames }) => { + cachedFrames.set(exceptionHash, frames); + }); + + worker.once('error', (err: Error) => { + log('Worker error', err); + }); + + worker.once('exit', (code: number) => { + log('Worker exit', code); + }); + + // Ensure this thread can't block app exit + worker.unref(); + } + + return { + name: 'LocalVariablesAsync', + setup(client: NodeClient) { + const clientOptions = client.getOptions(); + + if (!clientOptions.includeLocalVariables) { + return; + } + + const options: LocalVariablesWorkerArgs = { + ...integrationOptions, + debug: logger.isEnabled(), + }; + + startInspector().then( + () => { + try { + startWorker(options); + } catch (e) { + logger.error('Failed to start worker', e); + } + }, + e => { + logger.error('Failed to start inspector', e); + }, + ); + }, + processEvent(event: Event): Event { + return addLocalVariablesToEvent(event); + }, + }; +}) satisfies IntegrationFn); diff --git a/packages/node/src/integrations/local-variables/worker.ts b/packages/node/src/integrations/local-variables/worker.ts new file mode 100644 index 000000000000..5a104ce74f5b --- /dev/null +++ b/packages/node/src/integrations/local-variables/worker.ts @@ -0,0 +1,182 @@ +import { Session } from 'node:inspector/promises'; +import type { StackParser } from '@sentry/types'; +import { createStackParser, nodeStackLineParser } from '@sentry/utils'; +import type { Debugger, InspectorNotification, Runtime } from 'inspector'; +import { parentPort, workerData } from 'worker_threads'; +import { createGetModuleFromFilename } from '../../utils/module'; +import type { LocalVariablesWorkerArgs, PausedExceptionEvent, RateLimitIncrement, Variables } from './common'; +import { createRateLimiter, hashFromStack } from './common'; + +const options: LocalVariablesWorkerArgs = workerData; + +const stackParser = createStackParser(nodeStackLineParser(createGetModuleFromFilename(options.basePath))); + +function log(...args: unknown[]): void { + if (options.debug) { + // eslint-disable-next-line no-console + console.log('[LocalVariables Worker]', ...args); + } +} + +async function unrollArray(session: Session, objectId: string, name: string, vars: Variables): Promise { + const properties: Runtime.GetPropertiesReturnType = await session.post('Runtime.getProperties', { + objectId, + ownProperties: true, + }); + + vars[name] = properties.result + .filter(v => v.name !== 'length' && !isNaN(parseInt(v.name, 10))) + .sort((a, b) => parseInt(a.name, 10) - parseInt(b.name, 10)) + .map(v => v.value?.value); +} + +async function unrollObject(session: Session, objectId: string, name: string, vars: Variables): Promise { + const properties: Runtime.GetPropertiesReturnType = await session.post('Runtime.getProperties', { + objectId, + ownProperties: true, + }); + + vars[name] = properties.result + .map<[string, unknown]>(v => [v.name, v.value?.value]) + .reduce((obj, [key, val]) => { + obj[key] = val; + return obj; + }, {} as Variables); +} + +function unrollOther(prop: Runtime.PropertyDescriptor, vars: Variables): void { + if (!prop.value) { + return; + } + + if ('value' in prop.value) { + if (prop.value.value === undefined || prop.value.value === null) { + vars[prop.name] = `<${prop.value.value}>`; + } else { + vars[prop.name] = prop.value.value; + } + } else if ('description' in prop.value && prop.value.type !== 'function') { + vars[prop.name] = `<${prop.value.description}>`; + } else if (prop.value.type === 'undefined') { + vars[prop.name] = ''; + } +} + +async function getLocalVariables(session: Session, objectId: string): Promise { + const properties: Runtime.GetPropertiesReturnType = await session.post('Runtime.getProperties', { + objectId, + ownProperties: true, + }); + const variables = {}; + + for (const prop of properties.result) { + if (prop?.value?.objectId && prop?.value.className === 'Array') { + const id = prop.value.objectId; + await unrollArray(session, id, prop.name, variables); + } else if (prop?.value?.objectId && prop?.value?.className === 'Object') { + const id = prop.value.objectId; + await unrollObject(session, id, prop.name, variables); + } else if (prop?.value) { + unrollOther(prop, variables); + } + } + + return variables; +} + +let rateLimiter: RateLimitIncrement | undefined; + +async function handlePaused( + session: Session, + stackParser: StackParser, + { reason, data, callFrames }: PausedExceptionEvent, +): Promise { + if (reason !== 'exception' && reason !== 'promiseRejection') { + return; + } + + rateLimiter?.(); + + // data.description contains the original error.stack + const exceptionHash = hashFromStack(stackParser, data?.description); + + if (exceptionHash == undefined) { + return; + } + + const frames = []; + + for (let i = 0; i < callFrames.length; i++) { + const { scopeChain, functionName, this: obj } = callFrames[i]; + + const localScope = scopeChain.find(scope => scope.type === 'local'); + + // obj.className is undefined in ESM modules + const fn = obj.className === 'global' || !obj.className ? functionName : `${obj.className}.${functionName}`; + + if (localScope?.object.objectId === undefined) { + frames[i] = { function: fn }; + } else { + const vars = await getLocalVariables(session, localScope.object.objectId); + frames[i] = { function: fn, vars }; + } + } + + parentPort?.postMessage({ exceptionHash, frames }); +} + +async function startDebugger(): Promise { + const session = new Session(); + session.connectToMainThread(); + + log('Connected to main thread'); + + let isPaused = false; + + session.on('Debugger.resumed', () => { + isPaused = false; + }); + + session.on('Debugger.paused', (event: InspectorNotification) => { + isPaused = true; + + handlePaused(session, stackParser, event.params as PausedExceptionEvent).then( + () => { + // After the pause work is complete, resume execution! + return isPaused ? session.post('Debugger.resume') : Promise.resolve(); + }, + _ => { + // ignore + }, + ); + }); + + await session.post('Debugger.enable'); + + const captureAll = options.captureAllExceptions !== false; + await session.post('Debugger.setPauseOnExceptions', { state: captureAll ? 'all' : 'uncaught' }); + + if (captureAll) { + const max = options.maxExceptionsPerSecond || 50; + + rateLimiter = createRateLimiter( + max, + async () => { + log('Rate-limit lifted.'); + await session.post('Debugger.setPauseOnExceptions', { state: 'all' }); + }, + async seconds => { + log(`Rate-limit exceeded. Disabling capturing of caught exceptions for ${seconds} seconds.`); + await session.post('Debugger.setPauseOnExceptions', { state: 'uncaught' }); + }, + ); + } +} + +startDebugger().catch(e => { + log('Failed to start debugger', e); +}); + +setInterval(() => { + // Stop the worker from exiting +}, 10_000); From 44bc6cf6bcd905dab40467277150f16188a1129f Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 16 Apr 2024 16:25:49 +0200 Subject: [PATCH 07/21] fix(nextjs): Escape Next.js' OpenTelemetry instrumentation (#11625) --- .../src/common/utils/commonObjectTracing.ts | 27 -- .../src/common/utils/edgeWrapperUtils.ts | 101 +++---- .../nextjs/src/common/utils/tracingUtils.ts | 98 +++++++ .../withIsolationScopeOrReuseFromRootSpan.ts | 39 --- .../nextjs/src/common/utils/wrapperUtils.ts | 7 +- .../common/withServerActionInstrumentation.ts | 171 ++++++------ .../src/common/wrapApiHandlerWithSentry.ts | 260 +++++++++--------- .../wrapApiHandlerWithSentryVercelCrons.ts | 145 +++++----- .../wrapGenerationFunctionWithSentry.ts | 127 +++++---- .../src/common/wrapMiddlewareWithSentry.ts | 13 +- .../src/common/wrapPageComponentWithSentry.ts | 104 +++---- .../src/common/wrapRouteHandlerWithSentry.ts | 150 +++++----- .../common/wrapServerComponentWithSentry.ts | 93 ++++--- .../config/templates/apiWrapperTemplate.ts | 8 +- .../src/edge/wrapApiHandlerWithSentry.ts | 24 +- packages/nextjs/src/server/index.ts | 5 +- .../requestIsolationScopeIntegration.ts | 44 --- .../nextjs/test/config/withSentry.test.ts | 6 +- ...hIsolationScopeOrReuseFromRootSpan.test.ts | 96 ------- .../nextjs/test/edge/withSentryAPI.test.ts | 6 - ...hIsolationScopeOrReuseFromRootSpan.test.ts | 96 ------- 21 files changed, 704 insertions(+), 916 deletions(-) delete mode 100644 packages/nextjs/src/common/utils/commonObjectTracing.ts create mode 100644 packages/nextjs/src/common/utils/tracingUtils.ts delete mode 100644 packages/nextjs/src/common/utils/withIsolationScopeOrReuseFromRootSpan.ts delete mode 100644 packages/nextjs/src/server/requestIsolationScopeIntegration.ts delete mode 100644 packages/nextjs/test/edge/withIsolationScopeOrReuseFromRootSpan.test.ts delete mode 100644 packages/nextjs/test/server/withIsolationScopeOrReuseFromRootSpan.test.ts diff --git a/packages/nextjs/src/common/utils/commonObjectTracing.ts b/packages/nextjs/src/common/utils/commonObjectTracing.ts deleted file mode 100644 index 988dee391dc4..000000000000 --- a/packages/nextjs/src/common/utils/commonObjectTracing.ts +++ /dev/null @@ -1,27 +0,0 @@ -import type { PropagationContext } from '@sentry/types'; - -const commonMap = new WeakMap(); - -/** - * Takes a shared (garbage collectable) object between resources, e.g. a headers object shared between Next.js server components and returns a common propagation context. - * - * @param commonObject The shared object. - * @param propagationContext The propagation context that should be shared between all the resources if no propagation context was registered yet. - * @returns the shared propagation context. - */ -export function commonObjectToPropagationContext( - commonObject: unknown, - propagationContext: PropagationContext, -): PropagationContext { - if (typeof commonObject === 'object' && commonObject) { - const memoPropagationContext = commonMap.get(commonObject); - if (memoPropagationContext) { - return memoPropagationContext; - } else { - commonMap.set(commonObject, propagationContext); - return propagationContext; - } - } else { - return propagationContext; - } -} diff --git a/packages/nextjs/src/common/utils/edgeWrapperUtils.ts b/packages/nextjs/src/common/utils/edgeWrapperUtils.ts index 272889fa76db..df12a99259fa 100644 --- a/packages/nextjs/src/common/utils/edgeWrapperUtils.ts +++ b/packages/nextjs/src/common/utils/edgeWrapperUtils.ts @@ -4,15 +4,16 @@ import { SPAN_STATUS_OK, captureException, continueTrace, - getIsolationScope, handleCallbackErrors, setHttpStatus, startSpan, + withIsolationScope, } from '@sentry/core'; import { winterCGRequestToRequestData } from '@sentry/utils'; import type { EdgeRouteHandler } from '../../edge/types'; import { flushQueue } from './responseEnd'; +import { commonObjectToIsolationScope, escapeNextjsTracing } from './tracingUtils'; /** * Wraps a function on the edge runtime with error and performance monitoring. @@ -22,61 +23,65 @@ export function withEdgeWrapping( options: { spanDescription: string; spanOp: string; mechanismFunctionName: string }, ): (...params: Parameters) => Promise> { return async function (this: unknown, ...args) { - const req: unknown = args[0]; + return escapeNextjsTracing(() => { + const req: unknown = args[0]; + return withIsolationScope(commonObjectToIsolationScope(req), isolationScope => { + let sentryTrace; + let baggage; - let sentryTrace; - let baggage; + if (req instanceof Request) { + sentryTrace = req.headers.get('sentry-trace') || ''; + baggage = req.headers.get('baggage'); - if (req instanceof Request) { - sentryTrace = req.headers.get('sentry-trace') || ''; - baggage = req.headers.get('baggage'); - } + isolationScope.setSDKProcessingMetadata({ + request: winterCGRequestToRequestData(req), + }); + } - return continueTrace( - { - sentryTrace, - baggage, - }, - () => { - getIsolationScope().setSDKProcessingMetadata({ - request: req instanceof Request ? winterCGRequestToRequestData(req) : undefined, - }); - return startSpan( + return continueTrace( { - name: options.spanDescription, - op: options.spanOp, - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.withEdgeWrapping', - }, + sentryTrace, + baggage, }, - async span => { - const handlerResult = await handleCallbackErrors( - () => handler.apply(this, args), - error => { - captureException(error, { - mechanism: { - type: 'instrument', - handled: false, - data: { - function: options.mechanismFunctionName, - }, - }, - }); + () => { + return startSpan( + { + name: options.spanDescription, + op: options.spanOp, + forceTransaction: true, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.withEdgeWrapping', + }, }, - ); + async span => { + const handlerResult = await handleCallbackErrors( + () => handler.apply(this, args), + error => { + captureException(error, { + mechanism: { + type: 'instrument', + handled: false, + data: { + function: options.mechanismFunctionName, + }, + }, + }); + }, + ); - if (handlerResult instanceof Response) { - setHttpStatus(span, handlerResult.status); - } else { - span.setStatus({ code: SPAN_STATUS_OK }); - } + if (handlerResult instanceof Response) { + setHttpStatus(span, handlerResult.status); + } else { + span.setStatus({ code: SPAN_STATUS_OK }); + } - return handlerResult; + return handlerResult; + }, + ).finally(() => flushQueue()); }, - ).finally(() => flushQueue()); - }, - ); + ); + }); + }); }; } diff --git a/packages/nextjs/src/common/utils/tracingUtils.ts b/packages/nextjs/src/common/utils/tracingUtils.ts new file mode 100644 index 000000000000..0c03bc8f0ec9 --- /dev/null +++ b/packages/nextjs/src/common/utils/tracingUtils.ts @@ -0,0 +1,98 @@ +import { Scope, getCurrentScope, withActiveSpan } from '@sentry/core'; +import type { PropagationContext } from '@sentry/types'; +import { GLOBAL_OBJ, logger, uuid4 } from '@sentry/utils'; +import { DEBUG_BUILD } from '../debug-build'; + +const commonPropagationContextMap = new WeakMap(); + +/** + * Takes a shared (garbage collectable) object between resources, e.g. a headers object shared between Next.js server components and returns a common propagation context. + * + * @param commonObject The shared object. + * @param propagationContext The propagation context that should be shared between all the resources if no propagation context was registered yet. + * @returns the shared propagation context. + */ +export function commonObjectToPropagationContext( + commonObject: unknown, + propagationContext: PropagationContext, +): PropagationContext { + if (typeof commonObject === 'object' && commonObject) { + const memoPropagationContext = commonPropagationContextMap.get(commonObject); + if (memoPropagationContext) { + return memoPropagationContext; + } else { + commonPropagationContextMap.set(commonObject, propagationContext); + return propagationContext; + } + } else { + return propagationContext; + } +} + +const commonIsolationScopeMap = new WeakMap(); + +/** + * Takes a shared (garbage collectable) object between resources, e.g. a headers object shared between Next.js server components and returns a common propagation context. + * + * @param commonObject The shared object. + * @param isolationScope The isolationScope that should be shared between all the resources if no isolation scope was created yet. + * @returns the shared isolation scope. + */ +export function commonObjectToIsolationScope(commonObject: unknown): Scope { + if (typeof commonObject === 'object' && commonObject) { + const memoIsolationScope = commonIsolationScopeMap.get(commonObject); + if (memoIsolationScope) { + return memoIsolationScope; + } else { + const newIsolationScope = new Scope(); + commonIsolationScopeMap.set(commonObject, newIsolationScope); + return newIsolationScope; + } + } else { + return new Scope(); + } +} + +interface AsyncLocalStorage { + getStore(): T | undefined; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + run(store: T, callback: (...args: TArgs) => R, ...args: TArgs): R; +} + +let nextjsEscapedAsyncStorage: AsyncLocalStorage; + +/** + * Will mark the execution context of the callback as "escaped" from Next.js internal tracing by unsetting the active + * span and propagation context. When an execution passes through this function multiple times, it is a noop after the + * first time. + */ +export function escapeNextjsTracing(cb: () => T): T { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any + const MaybeGlobalAsyncLocalStorage = (GLOBAL_OBJ as any).AsyncLocalStorage; + + if (!MaybeGlobalAsyncLocalStorage) { + DEBUG_BUILD && + logger.warn( + "Tried to register AsyncLocalStorage async context strategy in a runtime that doesn't support AsyncLocalStorage.", + ); + return cb(); + } + + if (!nextjsEscapedAsyncStorage) { + nextjsEscapedAsyncStorage = new MaybeGlobalAsyncLocalStorage(); + } + + if (nextjsEscapedAsyncStorage.getStore()) { + return cb(); + } else { + return withActiveSpan(null, () => { + getCurrentScope().setPropagationContext({ + traceId: uuid4(), + spanId: uuid4().substring(16), + }); + return nextjsEscapedAsyncStorage.run(true, () => { + return cb(); + }); + }); + } +} diff --git a/packages/nextjs/src/common/utils/withIsolationScopeOrReuseFromRootSpan.ts b/packages/nextjs/src/common/utils/withIsolationScopeOrReuseFromRootSpan.ts deleted file mode 100644 index 82231022b980..000000000000 --- a/packages/nextjs/src/common/utils/withIsolationScopeOrReuseFromRootSpan.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { - getActiveSpan, - getCapturedScopesOnSpan, - getDefaultIsolationScope, - getRootSpan, - spanToJSON, - withIsolationScope, -} from '@sentry/core'; -import type { Scope } from '@sentry/types'; - -/** - * Wrap a callback with a new isolation scope. - * However, if we have an active root span that was generated by next, we want to reuse the isolation scope from that span. - */ -export function withIsolationScopeOrReuseFromRootSpan(cb: (isolationScope: Scope) => T): T { - const activeSpan = getActiveSpan(); - - if (!activeSpan) { - return withIsolationScope(cb); - } - - const rootSpan = getRootSpan(activeSpan); - - // Verify this is a next span - if (!spanToJSON(rootSpan).data?.['next.route']) { - return withIsolationScope(cb); - } - - const scopes = getCapturedScopesOnSpan(rootSpan); - - const isolationScope = scopes.isolationScope; - - // If this is the default isolation scope, we still want to fork one - if (isolationScope === getDefaultIsolationScope()) { - return withIsolationScope(cb); - } - - return withIsolationScope(isolationScope, cb); -} diff --git a/packages/nextjs/src/common/utils/wrapperUtils.ts b/packages/nextjs/src/common/utils/wrapperUtils.ts index ef14e19b02fa..20ea3a80bad9 100644 --- a/packages/nextjs/src/common/utils/wrapperUtils.ts +++ b/packages/nextjs/src/common/utils/wrapperUtils.ts @@ -10,13 +10,14 @@ import { startSpan, startSpanManual, withActiveSpan, + withIsolationScope, } from '@sentry/core'; import type { Span } from '@sentry/types'; import { isString } from '@sentry/utils'; import { platformSupportsStreaming } from './platformSupportsStreaming'; import { autoEndSpanOnResponseEnd, flushQueue } from './responseEnd'; -import { withIsolationScopeOrReuseFromRootSpan } from './withIsolationScopeOrReuseFromRootSpan'; +import { commonObjectToIsolationScope } from './tracingUtils'; declare module 'http' { interface IncomingMessage { @@ -89,7 +90,8 @@ export function withTracedServerSideDataFetcher Pr }, ): (...params: Parameters) => Promise> { return async function (this: unknown, ...args: Parameters): Promise> { - return withIsolationScopeOrReuseFromRootSpan(async isolationScope => { + const isolationScope = commonObjectToIsolationScope(req); + return withIsolationScope(isolationScope, () => { isolationScope.setSDKProcessingMetadata({ request: req, }); @@ -100,7 +102,6 @@ export function withTracedServerSideDataFetcher Pr return continueTrace({ sentryTrace, baggage }, () => { const requestSpan = getOrStartRequestSpan(req, res, options.requestedRouteName); - return withActiveSpan(requestSpan, () => { return startSpanManual( { diff --git a/packages/nextjs/src/common/withServerActionInstrumentation.ts b/packages/nextjs/src/common/withServerActionInstrumentation.ts index 83bdc9b7bbd3..a97d8fc17888 100644 --- a/packages/nextjs/src/common/withServerActionInstrumentation.ts +++ b/packages/nextjs/src/common/withServerActionInstrumentation.ts @@ -1,4 +1,9 @@ -import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, SPAN_STATUS_ERROR, getIsolationScope } from '@sentry/core'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + SPAN_STATUS_ERROR, + getIsolationScope, + withIsolationScope, +} from '@sentry/core'; import { captureException, continueTrace, getClient, handleCallbackErrors, startSpan } from '@sentry/core'; import { logger } from '@sentry/utils'; @@ -6,11 +11,11 @@ import { DEBUG_BUILD } from './debug-build'; import { isNotFoundNavigationError, isRedirectNavigationError } from './nextNavigationErrorUtils'; import { platformSupportsStreaming } from './utils/platformSupportsStreaming'; import { flushQueue } from './utils/responseEnd'; -import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; +import { escapeNextjsTracing } from './utils/tracingUtils'; interface Options { formData?: FormData; - // TODO: Whenever we decide to drop support for Next.js <= 12 we can automatically pick up the headers becauase "next/headers" will be resolvable. + // TODO(v8): Whenever we decide to drop support for Next.js <= 12 we can automatically pick up the headers becauase "next/headers" will be resolvable. headers?: Headers; recordResponse?: boolean; } @@ -50,93 +55,95 @@ async function withServerActionInstrumentationImplementation> { - return withIsolationScopeOrReuseFromRootSpan(isolationScope => { - const sendDefaultPii = getClient()?.getOptions().sendDefaultPii; + return escapeNextjsTracing(() => { + return withIsolationScope(isolationScope => { + const sendDefaultPii = getClient()?.getOptions().sendDefaultPii; - let sentryTraceHeader; - let baggageHeader; - const fullHeadersObject: Record = {}; - try { - sentryTraceHeader = options.headers?.get('sentry-trace') ?? undefined; - baggageHeader = options.headers?.get('baggage'); - options.headers?.forEach((value, key) => { - fullHeadersObject[key] = value; - }); - } catch (e) { - DEBUG_BUILD && - logger.warn( - "Sentry wasn't able to extract the tracing headers for a server action. Will not trace this request.", - ); - } + let sentryTraceHeader; + let baggageHeader; + const fullHeadersObject: Record = {}; + try { + sentryTraceHeader = options.headers?.get('sentry-trace') ?? undefined; + baggageHeader = options.headers?.get('baggage'); + options.headers?.forEach((value, key) => { + fullHeadersObject[key] = value; + }); + } catch (e) { + DEBUG_BUILD && + logger.warn( + "Sentry wasn't able to extract the tracing headers for a server action. Will not trace this request.", + ); + } - isolationScope.setSDKProcessingMetadata({ - request: { - headers: fullHeadersObject, - }, - }); + isolationScope.setSDKProcessingMetadata({ + request: { + headers: fullHeadersObject, + }, + }); - return continueTrace( - { - sentryTrace: sentryTraceHeader, - baggage: baggageHeader, - }, - async () => { - try { - return await startSpan( - { - op: 'function.server_action', - name: `serverAction/${serverActionName}`, - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + return continueTrace( + { + sentryTrace: sentryTraceHeader, + baggage: baggageHeader, + }, + async () => { + try { + return await startSpan( + { + op: 'function.server_action', + name: `serverAction/${serverActionName}`, + forceTransaction: true, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + }, }, - }, - async span => { - const result = await handleCallbackErrors(callback, error => { - if (isNotFoundNavigationError(error)) { - // We don't want to report "not-found"s - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); - } else if (isRedirectNavigationError(error)) { - // Don't do anything for redirects - } else { - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - captureException(error, { - mechanism: { - handled: false, - }, - }); - } - }); + async span => { + const result = await handleCallbackErrors(callback, error => { + if (isNotFoundNavigationError(error)) { + // We don't want to report "not-found"s + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); + } else if (isRedirectNavigationError(error)) { + // Don't do anything for redirects + } else { + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + captureException(error, { + mechanism: { + handled: false, + }, + }); + } + }); - if (options.recordResponse !== undefined ? options.recordResponse : sendDefaultPii) { - getIsolationScope().setExtra('server_action_result', result); - } + if (options.recordResponse !== undefined ? options.recordResponse : sendDefaultPii) { + getIsolationScope().setExtra('server_action_result', result); + } - if (options.formData) { - options.formData.forEach((value, key) => { - getIsolationScope().setExtra( - `server_action_form_data.${key}`, - typeof value === 'string' ? value : '[non-string value]', - ); - }); - } + if (options.formData) { + options.formData.forEach((value, key) => { + getIsolationScope().setExtra( + `server_action_form_data.${key}`, + typeof value === 'string' ? value : '[non-string value]', + ); + }); + } - return result; - }, - ); - } finally { - if (!platformSupportsStreaming()) { - // Lambdas require manual flushing to prevent execution freeze before the event is sent - await flushQueue(); - } + return result; + }, + ); + } finally { + if (!platformSupportsStreaming()) { + // Lambdas require manual flushing to prevent execution freeze before the event is sent + await flushQueue(); + } - if (process.env.NEXT_RUNTIME === 'edge') { - // flushQueue should not throw - // eslint-disable-next-line @typescript-eslint/no-floating-promises - flushQueue(); + if (process.env.NEXT_RUNTIME === 'edge') { + // flushQueue should not throw + // eslint-disable-next-line @typescript-eslint/no-floating-promises + flushQueue(); + } } - } - }, - ); + }, + ); + }); }); } diff --git a/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts index 37d0b83c7c5b..2a3ecd02bdb5 100644 --- a/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts @@ -4,6 +4,7 @@ import { continueTrace, setHttpStatus, startSpanManual, + withIsolationScope, } from '@sentry/core'; import { consoleSandbox, isString, logger, objectify, stripUrlQueryAndFragment } from '@sentry/utils'; @@ -11,7 +12,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import type { AugmentedNextApiRequest, AugmentedNextApiResponse, NextApiHandler } from './types'; import { platformSupportsStreaming } from './utils/platformSupportsStreaming'; import { flushQueue } from './utils/responseEnd'; -import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; +import { escapeNextjsTracing } from './utils/tracingUtils'; /** * Wrap the given API route handler for tracing and error capturing. Thin wrapper around `withSentry`, which only @@ -29,146 +30,149 @@ export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameteriz thisArg, args: [AugmentedNextApiRequest | undefined, AugmentedNextApiResponse | undefined], ) => { - const [req, res] = args; - - if (!req) { - logger.debug( - `Wrapped API handler on route "${parameterizedRoute}" was not passed a request object. Will not instrument.`, - ); - return wrappingTarget.apply(thisArg, args); - } else if (!res) { - logger.debug( - `Wrapped API handler on route "${parameterizedRoute}" was not passed a response object. Will not instrument.`, - ); - return wrappingTarget.apply(thisArg, args); - } - - // We're now auto-wrapping API route handlers using `wrapApiHandlerWithSentry` (which uses `withSentry` under the hood), but - // users still may have their routes manually wrapped with `withSentry`. This check makes `sentryWrappedHandler` - // idempotent so that those cases don't break anything. - if (req.__withSentry_applied__) { - return wrappingTarget.apply(thisArg, args); - } - req.__withSentry_applied__ = true; - - return withIsolationScopeOrReuseFromRootSpan(isolationScope => { - return continueTrace( - { - // TODO(v8): Make it so that continue trace will allow null as sentryTrace value and remove this fallback here - sentryTrace: req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined, - baggage: req.headers?.baggage, - }, - () => { - // prefer the parameterized route, if we have it (which we will if we've auto-wrapped the route handler) - let reqPath = parameterizedRoute; - - // If not, fake it by just replacing parameter values with their names, hoping that none of them match either - // each other or any hard-coded parts of the path - if (!reqPath) { - const url = `${req.url}`; - // pull off query string, if any - reqPath = stripUrlQueryAndFragment(url); - // Replace with placeholder - if (req.query) { - for (const [key, value] of Object.entries(req.query)) { - reqPath = reqPath.replace(`${value}`, `[${key}]`); + return escapeNextjsTracing(() => { + const [req, res] = args; + + if (!req) { + logger.debug( + `Wrapped API handler on route "${parameterizedRoute}" was not passed a request object. Will not instrument.`, + ); + return wrappingTarget.apply(thisArg, args); + } else if (!res) { + logger.debug( + `Wrapped API handler on route "${parameterizedRoute}" was not passed a response object. Will not instrument.`, + ); + return wrappingTarget.apply(thisArg, args); + } + + // We're now auto-wrapping API route handlers using `wrapApiHandlerWithSentry` (which uses `withSentry` under the hood), but + // users still may have their routes manually wrapped with `withSentry`. This check makes `sentryWrappedHandler` + // idempotent so that those cases don't break anything. + if (req.__withSentry_applied__) { + return wrappingTarget.apply(thisArg, args); + } + req.__withSentry_applied__ = true; + + return withIsolationScope(isolationScope => { + return continueTrace( + { + // TODO(v8): Make it so that continue trace will allow null as sentryTrace value and remove this fallback here + sentryTrace: + req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined, + baggage: req.headers?.baggage, + }, + () => { + // prefer the parameterized route, if we have it (which we will if we've auto-wrapped the route handler) + let reqPath = parameterizedRoute; + + // If not, fake it by just replacing parameter values with their names, hoping that none of them match either + // each other or any hard-coded parts of the path + if (!reqPath) { + const url = `${req.url}`; + // pull off query string, if any + reqPath = stripUrlQueryAndFragment(url); + // Replace with placeholder + if (req.query) { + for (const [key, value] of Object.entries(req.query)) { + reqPath = reqPath.replace(`${value}`, `[${key}]`); + } } } - } - const reqMethod = `${(req.method || 'GET').toUpperCase()} `; + const reqMethod = `${(req.method || 'GET').toUpperCase()} `; - isolationScope.setSDKProcessingMetadata({ request: req }); + isolationScope.setSDKProcessingMetadata({ request: req }); - return startSpanManual( - { - name: `${reqMethod}${reqPath}`, - op: 'http.server', - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.nextjs', + return startSpanManual( + { + name: `${reqMethod}${reqPath}`, + op: 'http.server', + forceTransaction: true, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.nextjs', + }, }, - }, - async span => { - // eslint-disable-next-line @typescript-eslint/unbound-method - res.end = new Proxy(res.end, { - apply(target, thisArg, argArray) { - setHttpStatus(span, res.statusCode); - span.end(); - if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) { - target.apply(thisArg, argArray); - } else { - // flushQueue will not reject - // eslint-disable-next-line @typescript-eslint/no-floating-promises - flushQueue().then(() => { + async span => { + // eslint-disable-next-line @typescript-eslint/unbound-method + res.end = new Proxy(res.end, { + apply(target, thisArg, argArray) { + setHttpStatus(span, res.statusCode); + span.end(); + if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) { target.apply(thisArg, argArray); + } else { + // flushQueue will not reject + // eslint-disable-next-line @typescript-eslint/no-floating-promises + flushQueue().then(() => { + target.apply(thisArg, argArray); + }); + } + }, + }); + + try { + const handlerResult = await wrappingTarget.apply(thisArg, args); + if ( + process.env.NODE_ENV === 'development' && + !process.env.SENTRY_IGNORE_API_RESOLUTION_ERROR && + !res.writableEnded + ) { + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.warn( + '[sentry] If Next.js logs a warning "API resolved without sending a response", it\'s a false positive, which may happen when you use `wrapApiHandlerWithSentry` manually to wrap your routes. To suppress this warning, set `SENTRY_IGNORE_API_RESOLUTION_ERROR` to 1 in your env. To suppress the nextjs warning, use the `externalResolver` API route option (see https://nextjs.org/docs/api-routes/api-middlewares#custom-config for details).', + ); }); } - }, - }); - - try { - const handlerResult = await wrappingTarget.apply(thisArg, args); - if ( - process.env.NODE_ENV === 'development' && - !process.env.SENTRY_IGNORE_API_RESOLUTION_ERROR && - !res.writableEnded - ) { - consoleSandbox(() => { - // eslint-disable-next-line no-console - console.warn( - '[sentry] If Next.js logs a warning "API resolved without sending a response", it\'s a false positive, which may happen when you use `wrapApiHandlerWithSentry` manually to wrap your routes. To suppress this warning, set `SENTRY_IGNORE_API_RESOLUTION_ERROR` to 1 in your env. To suppress the nextjs warning, use the `externalResolver` API route option (see https://nextjs.org/docs/api-routes/api-middlewares#custom-config for details).', - ); - }); - } - return handlerResult; - } catch (e) { - // In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can - // store a seen flag on it. (Because of the one-way-on-Vercel-one-way-off-of-Vercel approach we've been forced - // to take, it can happen that the same thrown object gets caught in two different ways, and flagging it is a - // way to prevent it from actually being reported twice.) - const objectifiedErr = objectify(e); - - captureException(objectifiedErr, { - mechanism: { - type: 'instrument', - handled: false, - data: { - wrapped_handler: wrappingTarget.name, - function: 'withSentry', + return handlerResult; + } catch (e) { + // In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can + // store a seen flag on it. (Because of the one-way-on-Vercel-one-way-off-of-Vercel approach we've been forced + // to take, it can happen that the same thrown object gets caught in two different ways, and flagging it is a + // way to prevent it from actually being reported twice.) + const objectifiedErr = objectify(e); + + captureException(objectifiedErr, { + mechanism: { + type: 'instrument', + handled: false, + data: { + wrapped_handler: wrappingTarget.name, + function: 'withSentry', + }, }, - }, - }); + }); - // Because we're going to finish and send the transaction before passing the error onto nextjs, it won't yet - // have had a chance to set the status to 500, so unless we do it ourselves now, we'll incorrectly report that - // the transaction was error-free - res.statusCode = 500; - res.statusMessage = 'Internal Server Error'; - - setHttpStatus(span, res.statusCode); - span.end(); - - // Make sure we have a chance to finish the transaction and flush events to Sentry before the handler errors - // out. (Apps which are deployed on Vercel run their API routes in lambdas, and those lambdas will shut down the - // moment they detect an error, so it's important to get this done before rethrowing the error. Apps not - // deployed serverlessly will run into this cleanup code again in `res.end(), but the transaction will already - // be finished and the queue will already be empty, so effectively it'll just no-op.) - if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) { - await flushQueue(); - } + // Because we're going to finish and send the transaction before passing the error onto nextjs, it won't yet + // have had a chance to set the status to 500, so unless we do it ourselves now, we'll incorrectly report that + // the transaction was error-free + res.statusCode = 500; + res.statusMessage = 'Internal Server Error'; - // We rethrow here so that nextjs can do with the error whatever it would normally do. (Sometimes "whatever it - // would normally do" is to allow the error to bubble up to the global handlers - another reason we need to mark - // the error as already having been captured.) - throw objectifiedErr; - } - }, - ); - }, - ); + setHttpStatus(span, res.statusCode); + span.end(); + + // Make sure we have a chance to finish the transaction and flush events to Sentry before the handler errors + // out. (Apps which are deployed on Vercel run their API routes in lambdas, and those lambdas will shut down the + // moment they detect an error, so it's important to get this done before rethrowing the error. Apps not + // deployed serverlessly will run into this cleanup code again in `res.end(), but the transaction will already + // be finished and the queue will already be empty, so effectively it'll just no-op.) + if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) { + await flushQueue(); + } + + // We rethrow here so that nextjs can do with the error whatever it would normally do. (Sometimes "whatever it + // would normally do" is to allow the error to bubble up to the global handlers - another reason we need to mark + // the error as already having been captured.) + throw objectifiedErr; + } + }, + ); + }, + ); + }); }); }, }); diff --git a/packages/nextjs/src/common/wrapApiHandlerWithSentryVercelCrons.ts b/packages/nextjs/src/common/wrapApiHandlerWithSentryVercelCrons.ts index 637f602b52ec..4974cd827e9a 100644 --- a/packages/nextjs/src/common/wrapApiHandlerWithSentryVercelCrons.ts +++ b/packages/nextjs/src/common/wrapApiHandlerWithSentryVercelCrons.ts @@ -2,7 +2,6 @@ import { captureCheckIn } from '@sentry/core'; import type { NextApiRequest } from 'next'; import type { VercelCronsConfig } from './types'; -import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; type EdgeRequest = { nextUrl: URL; @@ -20,92 +19,90 @@ export function wrapApiHandlerWithSentryVercelCrons { - return withIsolationScopeOrReuseFromRootSpan(() => { - if (!args || !args[0]) { - return originalFunction.apply(thisArg, args); - } + if (!args || !args[0]) { + return originalFunction.apply(thisArg, args); + } - const [req] = args as [NextApiRequest | EdgeRequest]; + const [req] = args as [NextApiRequest | EdgeRequest]; - let maybePromiseResult; - const cronsKey = 'nextUrl' in req ? req.nextUrl.pathname : req.url; - const userAgentHeader = 'nextUrl' in req ? req.headers.get('user-agent') : req.headers['user-agent']; + let maybePromiseResult; + const cronsKey = 'nextUrl' in req ? req.nextUrl.pathname : req.url; + const userAgentHeader = 'nextUrl' in req ? req.headers.get('user-agent') : req.headers['user-agent']; - if ( - !vercelCronsConfig || // do nothing if vercel crons config is missing - !userAgentHeader?.includes('vercel-cron') // do nothing if endpoint is not called from vercel crons - ) { - return originalFunction.apply(thisArg, args); - } + if ( + !vercelCronsConfig || // do nothing if vercel crons config is missing + !userAgentHeader?.includes('vercel-cron') // do nothing if endpoint is not called from vercel crons + ) { + return originalFunction.apply(thisArg, args); + } - const vercelCron = vercelCronsConfig.find(vercelCron => vercelCron.path === cronsKey); + const vercelCron = vercelCronsConfig.find(vercelCron => vercelCron.path === cronsKey); - if (!vercelCron || !vercelCron.path || !vercelCron.schedule) { - return originalFunction.apply(thisArg, args); - } + if (!vercelCron || !vercelCron.path || !vercelCron.schedule) { + return originalFunction.apply(thisArg, args); + } - const monitorSlug = vercelCron.path; + const monitorSlug = vercelCron.path; - const checkInId = captureCheckIn( - { - monitorSlug, - status: 'in_progress', + const checkInId = captureCheckIn( + { + monitorSlug, + status: 'in_progress', + }, + { + maxRuntime: 60 * 12, // (minutes) so 12 hours - just a very high arbitrary number since we don't know the actual duration of the users cron job + schedule: { + type: 'crontab', + value: vercelCron.schedule, }, - { - maxRuntime: 60 * 12, // (minutes) so 12 hours - just a very high arbitrary number since we don't know the actual duration of the users cron job - schedule: { - type: 'crontab', - value: vercelCron.schedule, - }, - }, - ); + }, + ); - const startTime = Date.now() / 1000; + const startTime = Date.now() / 1000; - const handleErrorCase = (): void => { - captureCheckIn({ - checkInId, - monitorSlug, - status: 'error', - duration: Date.now() / 1000 - startTime, - }); - }; + const handleErrorCase = (): void => { + captureCheckIn({ + checkInId, + monitorSlug, + status: 'error', + duration: Date.now() / 1000 - startTime, + }); + }; - try { - maybePromiseResult = originalFunction.apply(thisArg, args); - } catch (e) { - handleErrorCase(); - throw e; - } + try { + maybePromiseResult = originalFunction.apply(thisArg, args); + } catch (e) { + handleErrorCase(); + throw e; + } - if (typeof maybePromiseResult === 'object' && maybePromiseResult !== null && 'then' in maybePromiseResult) { - Promise.resolve(maybePromiseResult).then( - () => { - captureCheckIn({ - checkInId, - monitorSlug, - status: 'ok', - duration: Date.now() / 1000 - startTime, - }); - }, - () => { - handleErrorCase(); - }, - ); + if (typeof maybePromiseResult === 'object' && maybePromiseResult !== null && 'then' in maybePromiseResult) { + Promise.resolve(maybePromiseResult).then( + () => { + captureCheckIn({ + checkInId, + monitorSlug, + status: 'ok', + duration: Date.now() / 1000 - startTime, + }); + }, + () => { + handleErrorCase(); + }, + ); - // It is very important that we return the original promise here, because Next.js attaches various properties - // to that promise and will throw if they are not on the returned value. - return maybePromiseResult; - } else { - captureCheckIn({ - checkInId, - monitorSlug, - status: 'ok', - duration: Date.now() / 1000 - startTime, - }); - return maybePromiseResult; - } - }); + // It is very important that we return the original promise here, because Next.js attaches various properties + // to that promise and will throw if they are not on the returned value. + return maybePromiseResult; + } else { + captureCheckIn({ + checkInId, + monitorSlug, + status: 'ok', + duration: Date.now() / 1000 - startTime, + }); + return maybePromiseResult; + } }, }); } diff --git a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts index 9931e856e12f..d0e0573109a8 100644 --- a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts +++ b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts @@ -7,6 +7,7 @@ import { getCurrentScope, handleCallbackErrors, startSpanManual, + withIsolationScope, } from '@sentry/core'; import type { WebFetchHeaders } from '@sentry/types'; import { propagationContextFromHeaders, winterCGHeadersToDict } from '@sentry/utils'; @@ -14,8 +15,11 @@ import { propagationContextFromHeaders, winterCGHeadersToDict } from '@sentry/ut import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import type { GenerationFunctionContext } from '../common/types'; import { isNotFoundNavigationError, isRedirectNavigationError } from './nextNavigationErrorUtils'; -import { commonObjectToPropagationContext } from './utils/commonObjectTracing'; -import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; +import { + commonObjectToIsolationScope, + commonObjectToPropagationContext, + escapeNextjsTracing, +} from './utils/tracingUtils'; /** * Wraps a generation function (e.g. generateMetadata) with Sentry error and performance instrumentation. @@ -28,75 +32,78 @@ export function wrapGenerationFunctionWithSentry a const { requestAsyncStorage, componentRoute, componentType, generationFunctionIdentifier } = context; return new Proxy(generationFunction, { apply: (originalFunction, thisArg, args) => { - let headers: WebFetchHeaders | undefined = undefined; - // We try-catch here just in case anything goes wrong with the async storage here goes wrong since it is Next.js internal API - try { - headers = requestAsyncStorage?.getStore()?.headers; - } catch (e) { - /** empty */ - } - - let data: Record | undefined = undefined; - if (getClient()?.getOptions().sendDefaultPii) { - const props: unknown = args[0]; - const params = props && typeof props === 'object' && 'params' in props ? props.params : undefined; - const searchParams = - props && typeof props === 'object' && 'searchParams' in props ? props.searchParams : undefined; - data = { params, searchParams }; - } + return escapeNextjsTracing(() => { + let headers: WebFetchHeaders | undefined = undefined; + // We try-catch here just in case anything goes wrong with the async storage here goes wrong since it is Next.js internal API + try { + headers = requestAsyncStorage?.getStore()?.headers; + } catch (e) { + /** empty */ + } - return withIsolationScopeOrReuseFromRootSpan(isolationScope => { - isolationScope.setSDKProcessingMetadata({ - request: { - headers: headers ? winterCGHeadersToDict(headers) : undefined, - }, - }); - isolationScope.setExtra('route_data', data); + let data: Record | undefined = undefined; + if (getClient()?.getOptions().sendDefaultPii) { + const props: unknown = args[0]; + const params = props && typeof props === 'object' && 'params' in props ? props.params : undefined; + const searchParams = + props && typeof props === 'object' && 'searchParams' in props ? props.searchParams : undefined; + data = { params, searchParams }; + } const incomingPropagationContext = propagationContextFromHeaders( headers?.get('sentry-trace') ?? undefined, headers?.get('baggage'), ); + const isolationScope = commonObjectToIsolationScope(headers); const propagationContext = commonObjectToPropagationContext(headers, incomingPropagationContext); - isolationScope.setPropagationContext(propagationContext); - getCurrentScope().setPropagationContext(propagationContext); - return startSpanManual( - { - op: 'function.nextjs', - name: `${componentType}.${generationFunctionIdentifier} (${componentRoute})`, - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', + return withIsolationScope(isolationScope, () => { + isolationScope.setSDKProcessingMetadata({ + request: { + headers: headers ? winterCGHeadersToDict(headers) : undefined, }, - }, - span => { - return handleCallbackErrors( - () => originalFunction.apply(thisArg, args), - err => { - if (isNotFoundNavigationError(err)) { - // We don't want to report "not-found"s - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); - } else if (isRedirectNavigationError(err)) { - // We don't want to report redirects - span.setStatus({ code: SPAN_STATUS_OK }); - } else { - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - captureException(err, { - mechanism: { - handled: false, - }, - }); - } - }, - () => { - span.end(); + }); + + getCurrentScope().setExtra('route_data', data); + getCurrentScope().setPropagationContext(propagationContext); + + return startSpanManual( + { + op: 'function.nextjs', + name: `${componentType}.${generationFunctionIdentifier} (${componentRoute})`, + forceTransaction: true, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', }, - ); - }, - ); + }, + span => { + return handleCallbackErrors( + () => originalFunction.apply(thisArg, args), + err => { + if (isNotFoundNavigationError(err)) { + // We don't want to report "not-found"s + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); + } else if (isRedirectNavigationError(err)) { + // We don't want to report redirects + span.setStatus({ code: SPAN_STATUS_OK }); + } else { + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + captureException(err, { + mechanism: { + handled: false, + }, + }); + } + }, + () => { + span.end(); + }, + ); + }, + ); + }); }); }, }); diff --git a/packages/nextjs/src/common/wrapMiddlewareWithSentry.ts b/packages/nextjs/src/common/wrapMiddlewareWithSentry.ts index 25b0e2b6d5d4..66cbbb046300 100644 --- a/packages/nextjs/src/common/wrapMiddlewareWithSentry.ts +++ b/packages/nextjs/src/common/wrapMiddlewareWithSentry.ts @@ -1,6 +1,5 @@ import type { EdgeRouteHandler } from '../edge/types'; import { withEdgeWrapping } from './utils/edgeWrapperUtils'; -import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; /** * Wraps Next.js middleware with Sentry error and performance instrumentation. @@ -13,13 +12,11 @@ export function wrapMiddlewareWithSentry( ): (...params: Parameters) => Promise> { return new Proxy(middleware, { apply: (wrappingTarget, thisArg, args: Parameters) => { - return withIsolationScopeOrReuseFromRootSpan(() => { - return withEdgeWrapping(wrappingTarget, { - spanDescription: 'middleware', - spanOp: 'middleware.nextjs', - mechanismFunctionName: 'withSentryMiddleware', - }).apply(thisArg, args); - }); + return withEdgeWrapping(wrappingTarget, { + spanDescription: 'middleware', + spanOp: 'middleware.nextjs', + mechanismFunctionName: 'withSentryMiddleware', + }).apply(thisArg, args); }, }); } diff --git a/packages/nextjs/src/common/wrapPageComponentWithSentry.ts b/packages/nextjs/src/common/wrapPageComponentWithSentry.ts index b6ab022afb21..8cd4a250ac14 100644 --- a/packages/nextjs/src/common/wrapPageComponentWithSentry.ts +++ b/packages/nextjs/src/common/wrapPageComponentWithSentry.ts @@ -1,6 +1,6 @@ -import { captureException, getCurrentScope } from '@sentry/core'; +import { captureException, getCurrentScope, withIsolationScope } from '@sentry/core'; import { extractTraceparentData } from '@sentry/utils'; -import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; +import { escapeNextjsTracing } from './utils/tracingUtils'; interface FunctionComponent { (...args: unknown[]): unknown; @@ -25,64 +25,68 @@ export function wrapPageComponentWithSentry(pageComponent: FunctionComponent | C if (isReactClassComponent(pageComponent)) { return class SentryWrappedPageComponent extends pageComponent { public render(...args: unknown[]): unknown { - return withIsolationScopeOrReuseFromRootSpan(() => { - const scope = getCurrentScope(); - // We extract the sentry trace data that is put in the component props by datafetcher wrappers - const sentryTraceData = - typeof this.props === 'object' && - this.props !== null && - '_sentryTraceData' in this.props && - typeof this.props._sentryTraceData === 'string' - ? this.props._sentryTraceData - : undefined; + return escapeNextjsTracing(() => { + return withIsolationScope(() => { + const scope = getCurrentScope(); + // We extract the sentry trace data that is put in the component props by datafetcher wrappers + const sentryTraceData = + typeof this.props === 'object' && + this.props !== null && + '_sentryTraceData' in this.props && + typeof this.props._sentryTraceData === 'string' + ? this.props._sentryTraceData + : undefined; - if (sentryTraceData) { - const traceparentData = extractTraceparentData(sentryTraceData); - scope.setContext('trace', { - span_id: traceparentData?.parentSpanId, - trace_id: traceparentData?.traceId, - }); - } + if (sentryTraceData) { + const traceparentData = extractTraceparentData(sentryTraceData); + scope.setContext('trace', { + span_id: traceparentData?.parentSpanId, + trace_id: traceparentData?.traceId, + }); + } - try { - return super.render(...args); - } catch (e) { - captureException(e, { - mechanism: { - handled: false, - }, - }); - throw e; - } + try { + return super.render(...args); + } catch (e) { + captureException(e, { + mechanism: { + handled: false, + }, + }); + throw e; + } + }); }); } }; } else if (typeof pageComponent === 'function') { return new Proxy(pageComponent, { apply(target, thisArg, argArray: [{ _sentryTraceData?: string } | undefined]) { - return withIsolationScopeOrReuseFromRootSpan(() => { - const scope = getCurrentScope(); - // We extract the sentry trace data that is put in the component props by datafetcher wrappers - const sentryTraceData = argArray?.[0]?._sentryTraceData; + return escapeNextjsTracing(() => { + return withIsolationScope(() => { + const scope = getCurrentScope(); + // We extract the sentry trace data that is put in the component props by datafetcher wrappers + const sentryTraceData = argArray?.[0]?._sentryTraceData; - if (sentryTraceData) { - const traceparentData = extractTraceparentData(sentryTraceData); - scope.setContext('trace', { - span_id: traceparentData?.parentSpanId, - trace_id: traceparentData?.traceId, - }); - } + if (sentryTraceData) { + const traceparentData = extractTraceparentData(sentryTraceData); + scope.setContext('trace', { + span_id: traceparentData?.parentSpanId, + trace_id: traceparentData?.traceId, + }); + } - try { - return target.apply(thisArg, argArray); - } catch (e) { - captureException(e, { - mechanism: { - handled: false, - }, - }); - throw e; - } + try { + return target.apply(thisArg, argArray); + } catch (e) { + captureException(e, { + mechanism: { + handled: false, + }, + }); + throw e; + } + }); }); }, }); diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts index 1dadd72e3f43..ec074bb3075e 100644 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -4,63 +4,22 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, SPAN_STATUS_ERROR, captureException, - getActiveSpan, - getRootSpan, + getCurrentScope, handleCallbackErrors, setHttpStatus, - spanToJSON, startSpan, + withIsolationScope, } from '@sentry/core'; -import type { Span } from '@sentry/types'; -import { winterCGHeadersToDict } from '@sentry/utils'; +import { propagationContextFromHeaders, winterCGHeadersToDict } from '@sentry/utils'; import { isNotFoundNavigationError, isRedirectNavigationError } from './nextNavigationErrorUtils'; import type { RouteHandlerContext } from './types'; import { platformSupportsStreaming } from './utils/platformSupportsStreaming'; import { flushQueue } from './utils/responseEnd'; -import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; - -/** As our own HTTP integration is disabled (src/server/index.ts) the rootSpan comes from Next.js. - * In case there is no root span, we start a new span. */ -function startOrUpdateSpan(spanName: string, cb: (rootSpan: Span) => Promise): Promise { - const activeSpan = getActiveSpan(); - const rootSpan = activeSpan && getRootSpan(activeSpan); - - // We have different possible scenarios here: - // 1. If we have no root span, we just create a new span - // 2. We have a root span that that we want to update here - // 3. We have a root span that was already updated (e.g. if this is a nested call) - - const attributes = { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', - } as const; - - if (!rootSpan) { - return startSpan( - { - name: spanName, - forceTransaction: true, - attributes, - }, - cb, - ); - } - - // If `op` is set, we assume this was already processed before - // Probably this is a nested call, no need to update anything anymore - // OR, if we don't have next.span_type, we don't know where this comes from and don't want to mess with it - const existingAttributes = spanToJSON(rootSpan).data || {}; - if (existingAttributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] || !existingAttributes['next.span_type']) { - return cb(rootSpan); - } - - // Finally, we want to update the root span, as the ones generated by next are often not good enough for us - rootSpan.updateName(spanName); - rootSpan.setAttributes(attributes); - - return cb(rootSpan); -} +import { + commonObjectToIsolationScope, + commonObjectToPropagationContext, + escapeNextjsTracing, +} from './utils/tracingUtils'; /** * Wraps a Next.js route handler with performance and error instrumentation. @@ -74,50 +33,75 @@ export function wrapRouteHandlerWithSentry any>( return new Proxy(routeHandler, { apply: (originalFunction, thisArg, args) => { - return withIsolationScopeOrReuseFromRootSpan(async isolationScope => { + return escapeNextjsTracing(() => { + const isolationScope = commonObjectToIsolationScope(headers); + + const completeHeadersDict: Record = headers ? winterCGHeadersToDict(headers) : {}; + isolationScope.setSDKProcessingMetadata({ request: { - headers: headers ? winterCGHeadersToDict(headers) : undefined, + headers: completeHeadersDict, }, }); - try { - return await startOrUpdateSpan(`${method} ${parameterizedRoute}`, async (rootSpan: Span) => { - const response: Response = await handleCallbackErrors( - () => originalFunction.apply(thisArg, args), - error => { - // Next.js throws errors when calling `redirect()`. We don't wanna report these. - if (isRedirectNavigationError(error)) { - // Don't do anything - } else if (isNotFoundNavigationError(error) && rootSpan) { - rootSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); - } else { - captureException(error, { - mechanism: { - handled: false, - }, - }); + const incomingPropagationContext = propagationContextFromHeaders( + completeHeadersDict['sentry-trace'], + completeHeadersDict['baggage'], + ); + + const propagationContext = commonObjectToPropagationContext(headers, incomingPropagationContext); + + return withIsolationScope(isolationScope, async () => { + getCurrentScope().setPropagationContext(propagationContext); + try { + return startSpan( + { + name: `${method} ${parameterizedRoute}`, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', + }, + forceTransaction: true, + }, + async span => { + const response: Response = await handleCallbackErrors( + () => originalFunction.apply(thisArg, args), + error => { + // Next.js throws errors when calling `redirect()`. We don't wanna report these. + if (isRedirectNavigationError(error)) { + // Don't do anything + } else if (isNotFoundNavigationError(error) && span) { + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); + } else { + captureException(error, { + mechanism: { + handled: false, + }, + }); + } + }, + ); + + try { + if (span && response.status) { + setHttpStatus(span, response.status); + } + } catch { + // best effort - response may be undefined? } + + return response; }, ); - - try { - if (rootSpan && response.status) { - setHttpStatus(rootSpan, response.status); - } - } catch { - // best effort - response may be undefined? + } finally { + if (!platformSupportsStreaming() || process.env.NEXT_RUNTIME === 'edge') { + // 1. Edge transport requires manual flushing + // 2. Lambdas require manual flushing to prevent execution freeze before the event is sent + await flushQueue(); } - - return response; - }); - } finally { - if (!platformSupportsStreaming() || process.env.NEXT_RUNTIME === 'edge') { - // 1. Edge transport requires manual flushing - // 2. Lambdas require manual flushing to prevent execution freeze before the event is sent - await flushQueue(); } - } + }); }); }, }); diff --git a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts index a41cfd2fb052..165754614a22 100644 --- a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts +++ b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts @@ -6,15 +6,19 @@ import { getCurrentScope, handleCallbackErrors, startSpanManual, + withIsolationScope, } from '@sentry/core'; import { propagationContextFromHeaders, winterCGHeadersToDict } from '@sentry/utils'; import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import { isNotFoundNavigationError, isRedirectNavigationError } from '../common/nextNavigationErrorUtils'; import type { ServerComponentContext } from '../common/types'; -import { commonObjectToPropagationContext } from './utils/commonObjectTracing'; import { flushQueue } from './utils/responseEnd'; -import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; +import { + commonObjectToIsolationScope, + commonObjectToPropagationContext, + escapeNextjsTracing, +} from './utils/tracingUtils'; /** * Wraps an `app` directory server component with Sentry error instrumentation. @@ -25,14 +29,14 @@ export function wrapServerComponentWithSentry any> context: ServerComponentContext, ): F { const { componentRoute, componentType } = context; - // Even though users may define server components as async functions, for the client bundles // Next.js will turn them into synchronous functions and it will transform any `await`s into instances of the `use` // hook. 🤯 return new Proxy(appDirComponent, { apply: (originalFunction, thisArg, args) => { - // TODO: If we ever allow withIsolationScope to take a scope, we should pass a scope here that is shared between all of the server components, similar to what `commonObjectToPropagationContext` does. - return withIsolationScopeOrReuseFromRootSpan(isolationScope => { + return escapeNextjsTracing(() => { + const isolationScope = commonObjectToIsolationScope(context.headers); + const completeHeadersDict: Record = context.headers ? winterCGHeadersToDict(context.headers) : {}; @@ -50,47 +54,48 @@ export function wrapServerComponentWithSentry any> const propagationContext = commonObjectToPropagationContext(context.headers, incomingPropagationContext); - getCurrentScope().setPropagationContext(propagationContext); - - return startSpanManual( - { - op: 'function.nextjs', - name: `${componentType} Server Component (${componentRoute})`, - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', - }, - }, - span => { - return handleCallbackErrors( - () => originalFunction.apply(thisArg, args), - error => { - if (isNotFoundNavigationError(error)) { - // We don't want to report "not-found"s - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); - } else if (isRedirectNavigationError(error)) { - // We don't want to report redirects - span.setStatus({ code: SPAN_STATUS_OK }); - } else { - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - captureException(error, { - mechanism: { - handled: false, - }, - }); - } + return withIsolationScope(isolationScope, () => { + getCurrentScope().setPropagationContext(propagationContext); + return startSpanManual( + { + op: 'function.nextjs', + name: `${componentType} Server Component (${componentRoute})`, + forceTransaction: true, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', }, - () => { - span.end(); + }, + span => { + return handleCallbackErrors( + () => originalFunction.apply(thisArg, args), + error => { + if (isNotFoundNavigationError(error)) { + // We don't want to report "not-found"s + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); + } else if (isRedirectNavigationError(error)) { + // We don't want to report redirects + span.setStatus({ code: SPAN_STATUS_OK }); + } else { + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + captureException(error, { + mechanism: { + handled: false, + }, + }); + } + }, + () => { + span.end(); - // flushQueue should not throw - // eslint-disable-next-line @typescript-eslint/no-floating-promises - flushQueue(); - }, - ); - }, - ); + // flushQueue should not throw + // eslint-disable-next-line @typescript-eslint/no-floating-promises + flushQueue(); + }, + ); + }, + ); + }); }); }, }); diff --git a/packages/nextjs/src/config/templates/apiWrapperTemplate.ts b/packages/nextjs/src/config/templates/apiWrapperTemplate.ts index d5eae3687403..80b9a4f51d60 100644 --- a/packages/nextjs/src/config/templates/apiWrapperTemplate.ts +++ b/packages/nextjs/src/config/templates/apiWrapperTemplate.ts @@ -54,14 +54,14 @@ declare const __VERCEL_CRONS_CONFIGURATION__: VercelCronsConfig; let wrappedHandler = userProvidedHandler; -if (wrappedHandler) { - wrappedHandler = Sentry.wrapApiHandlerWithSentry(wrappedHandler, '__ROUTE__'); -} - if (wrappedHandler && __VERCEL_CRONS_CONFIGURATION__) { wrappedHandler = Sentry.wrapApiHandlerWithSentryVercelCrons(wrappedHandler, __VERCEL_CRONS_CONFIGURATION__); } +if (wrappedHandler) { + wrappedHandler = Sentry.wrapApiHandlerWithSentry(wrappedHandler, '__ROUTE__'); +} + export default wrappedHandler; // Re-export anything exported by the page module we're wrapping. When processing this code, Rollup is smart enough to diff --git a/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts index 717090905f6a..e5191ea27dbe 100644 --- a/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts +++ b/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts @@ -1,7 +1,4 @@ -import { getActiveSpan } from '@sentry/vercel-edge'; - import { withEdgeWrapping } from '../common/utils/edgeWrapperUtils'; -import { withIsolationScopeOrReuseFromRootSpan } from '../common/utils/withIsolationScopeOrReuseFromRootSpan'; import type { EdgeRouteHandler } from './types'; /** @@ -15,20 +12,15 @@ export function wrapApiHandlerWithSentry( apply: (wrappingTarget, thisArg, args: Parameters) => { const req = args[0]; - const activeSpan = getActiveSpan(); - - return withIsolationScopeOrReuseFromRootSpan(() => { - const wrappedHandler = withEdgeWrapping(wrappingTarget, { - spanDescription: - activeSpan || !(req instanceof Request) - ? `handler (${parameterizedRoute})` - : `${req.method} ${parameterizedRoute}`, - spanOp: activeSpan ? 'function' : 'http.server', - mechanismFunctionName: 'wrapApiHandlerWithSentry', - }); - - return wrappedHandler.apply(thisArg, args); + const wrappedHandler = withEdgeWrapping(wrappingTarget, { + spanDescription: !(req instanceof Request) + ? `handler (${parameterizedRoute})` + : `${req.method} ${parameterizedRoute}`, + spanOp: 'http.server', + mechanismFunctionName: 'wrapApiHandlerWithSentry', }); + + return wrappedHandler.apply(thisArg, args); }, }); } diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index 7d8f9a7a7d94..6895f1457727 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -11,7 +11,6 @@ import { distDirRewriteFramesIntegration } from './distDirRewriteFramesIntegrati export * from '@sentry/node'; import type { EventProcessor } from '@sentry/types'; -import { requestIsolationScopeIntegration } from './requestIsolationScopeIntegration'; export { captureUnderscoreErrorException } from '../common/_error'; @@ -73,10 +72,9 @@ export function init(options: NodeOptions): void { integration => // Next.js comes with its own Node-Fetch instrumentation, so we shouldn't add ours on-top integration.name !== 'NodeFetch' && - // Next.js comes with its own Http instrumentation for OTel which lead to double spans for route handler requests + // Next.js comes with its own Http instrumentation for OTel which would lead to double spans for route handler requests integration.name !== 'Http', ), - requestIsolationScopeIntegration(), ]; // This value is injected at build time, based on the output directory specified in the build config. Though a default @@ -147,6 +145,7 @@ export function init(options: NodeOptions): void { ), ); + // TODO(v8): Remove this because we have `suppressTracing` addEventProcessor( Object.assign( (event => { diff --git a/packages/nextjs/src/server/requestIsolationScopeIntegration.ts b/packages/nextjs/src/server/requestIsolationScopeIntegration.ts deleted file mode 100644 index b6020d891d8b..000000000000 --- a/packages/nextjs/src/server/requestIsolationScopeIntegration.ts +++ /dev/null @@ -1,44 +0,0 @@ -import { SpanKind } from '@opentelemetry/api'; -import { - defineIntegration, - getCapturedScopesOnSpan, - getCurrentScope, - getIsolationScope, - getRootSpan, - setCapturedScopesOnSpan, - spanToJSON, -} from '@sentry/core'; -import { getSpanKind } from '@sentry/opentelemetry'; - -/** - * This integration is responsible for creating isolation scopes for incoming Http requests. - * We do so by waiting for http spans to be created and then forking the isolation scope. - * - * Normally the isolation scopes would be created by our Http instrumentation, however Next.js brings it's own Http - * instrumentation so we had to disable ours. - */ -export const requestIsolationScopeIntegration = defineIntegration(() => { - return { - name: 'RequestIsolationScope', - setup(client) { - client.on('spanStart', span => { - const spanJson = spanToJSON(span); - const data = spanJson.data || {}; - - // The following check is a heuristic to determine whether the started span is a span that tracks an incoming HTTP request - if ( - (getSpanKind(span) === SpanKind.SERVER && data['http.method']) || - (span === getRootSpan(span) && data['next.route']) - ) { - const scopes = getCapturedScopesOnSpan(span); - - // Update the isolation scope, isolate this request - const isolationScope = (scopes.isolationScope || getIsolationScope()).clone(); - const scope = scopes.scope || getCurrentScope(); - - setCapturedScopesOnSpan(span, scope, isolationScope); - } - }); - }, - }; -}); diff --git a/packages/nextjs/test/config/withSentry.test.ts b/packages/nextjs/test/config/withSentry.test.ts index 7a29ab80afb7..30f34634d9fd 100644 --- a/packages/nextjs/test/config/withSentry.test.ts +++ b/packages/nextjs/test/config/withSentry.test.ts @@ -36,7 +36,7 @@ describe('withSentry', () => { }); describe('tracing', () => { - it('starts a transaction and sets metadata when tracing is enabled', async () => { + it('starts a transaction when tracing is enabled', async () => { await wrappedHandlerNoError(req, res); expect(startSpanManualSpy).toHaveBeenCalledWith( expect.objectContaining({ @@ -49,10 +49,6 @@ describe('withSentry', () => { }), expect.any(Function), ); - - expect(SentryCore.getIsolationScope().getScopeData().sdkProcessingMetadata).toEqual({ - request: expect.objectContaining({ url: 'http://dogs.are.great' }), - }); }); }); }); diff --git a/packages/nextjs/test/edge/withIsolationScopeOrReuseFromRootSpan.test.ts b/packages/nextjs/test/edge/withIsolationScopeOrReuseFromRootSpan.test.ts deleted file mode 100644 index 8f42b8a9264b..000000000000 --- a/packages/nextjs/test/edge/withIsolationScopeOrReuseFromRootSpan.test.ts +++ /dev/null @@ -1,96 +0,0 @@ -import { - Scope, - getCurrentScope, - getGlobalScope, - getIsolationScope, - setCapturedScopesOnSpan, - startSpan, -} from '@sentry/core'; -import { GLOBAL_OBJ } from '@sentry/utils'; -import { init } from '@sentry/vercel-edge'; -import { AsyncLocalStorage } from 'async_hooks'; - -import { withIsolationScopeOrReuseFromRootSpan } from '../../src/common/utils/withIsolationScopeOrReuseFromRootSpan'; - -describe('withIsolationScopeOrReuseFromRootSpan', () => { - beforeEach(() => { - getIsolationScope().clear(); - getCurrentScope().clear(); - getGlobalScope().clear(); - (GLOBAL_OBJ as any).AsyncLocalStorage = AsyncLocalStorage; - - init({ - enableTracing: true, - }); - }); - - it('works without any span', () => { - const initialIsolationScope = getIsolationScope(); - initialIsolationScope.setTag('aa', 'aa'); - - withIsolationScopeOrReuseFromRootSpan(isolationScope => { - isolationScope.setTag('bb', 'bb'); - expect(isolationScope).not.toBe(initialIsolationScope); - expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); - }); - }); - - it('works with a non-next.js span', () => { - const initialIsolationScope = getIsolationScope(); - initialIsolationScope.setTag('aa', 'aa'); - - const customScope = new Scope(); - - startSpan({ name: 'other' }, span => { - setCapturedScopesOnSpan(span, getCurrentScope(), customScope); - - withIsolationScopeOrReuseFromRootSpan(isolationScope => { - isolationScope.setTag('bb', 'bb'); - expect(isolationScope).not.toBe(initialIsolationScope); - expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); - }); - }); - }); - - it('works with a next.js span', () => { - const initialIsolationScope = getIsolationScope(); - initialIsolationScope.setTag('aa', 'aa'); - - const customScope = new Scope(); - - startSpan( - { - name: 'other', - attributes: { 'next.route': 'aha' }, - }, - span => { - setCapturedScopesOnSpan(span, getCurrentScope(), customScope); - - withIsolationScopeOrReuseFromRootSpan(isolationScope => { - isolationScope.setTag('bb', 'bb'); - expect(isolationScope).toBe(customScope); - expect(isolationScope.getScopeData().tags).toEqual({ bb: 'bb' }); - }); - }, - ); - }); - - it('works with a next.js span that has default isolation scope', () => { - const initialIsolationScope = getIsolationScope(); - initialIsolationScope.setTag('aa', 'aa'); - - startSpan( - { - name: 'other', - attributes: { 'next.route': 'aha' }, - }, - () => { - withIsolationScopeOrReuseFromRootSpan(isolationScope => { - isolationScope.setTag('bb', 'bb'); - expect(isolationScope).not.toBe(initialIsolationScope); - expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); - }); - }, - ); - }); -}); diff --git a/packages/nextjs/test/edge/withSentryAPI.test.ts b/packages/nextjs/test/edge/withSentryAPI.test.ts index 2b232c922968..6e24eca21bfe 100644 --- a/packages/nextjs/test/edge/withSentryAPI.test.ts +++ b/packages/nextjs/test/edge/withSentryAPI.test.ts @@ -58,10 +58,6 @@ describe('wrapApiHandlerWithSentry', () => { }), expect.any(Function), ); - - expect(coreSdk.getIsolationScope().getScopeData().sdkProcessingMetadata).toEqual({ - request: { headers: {}, method: 'POST', url: 'https://sentry.io/' }, - }); }); it('should return a function that calls trace without throwing when no request is passed', async () => { @@ -83,7 +79,5 @@ describe('wrapApiHandlerWithSentry', () => { }), expect.any(Function), ); - - expect(coreSdk.getIsolationScope().getScopeData().sdkProcessingMetadata).toEqual({}); }); }); diff --git a/packages/nextjs/test/server/withIsolationScopeOrReuseFromRootSpan.test.ts b/packages/nextjs/test/server/withIsolationScopeOrReuseFromRootSpan.test.ts deleted file mode 100644 index 0cfc53e0a5b2..000000000000 --- a/packages/nextjs/test/server/withIsolationScopeOrReuseFromRootSpan.test.ts +++ /dev/null @@ -1,96 +0,0 @@ -import { - Scope, - getCurrentScope, - getGlobalScope, - getIsolationScope, - setCapturedScopesOnSpan, - startSpan, -} from '@sentry/core'; -import { init } from '@sentry/node'; -import { GLOBAL_OBJ } from '@sentry/utils'; -import { AsyncLocalStorage } from 'async_hooks'; - -import { withIsolationScopeOrReuseFromRootSpan } from '../../src/common/utils/withIsolationScopeOrReuseFromRootSpan'; - -describe('withIsolationScopeOrReuseFromRootSpan', () => { - beforeEach(() => { - getIsolationScope().clear(); - getCurrentScope().clear(); - getGlobalScope().clear(); - (GLOBAL_OBJ as any).AsyncLocalStorage = AsyncLocalStorage; - - init({ - enableTracing: true, - }); - }); - - it('works without any span', () => { - const initialIsolationScope = getIsolationScope(); - initialIsolationScope.setTag('aa', 'aa'); - - withIsolationScopeOrReuseFromRootSpan(isolationScope => { - isolationScope.setTag('bb', 'bb'); - expect(isolationScope).not.toBe(initialIsolationScope); - expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); - }); - }); - - it('works with a non-next.js span', () => { - const initialIsolationScope = getIsolationScope(); - initialIsolationScope.setTag('aa', 'aa'); - - const customScope = new Scope(); - - startSpan({ name: 'other' }, span => { - setCapturedScopesOnSpan(span, getCurrentScope(), customScope); - - withIsolationScopeOrReuseFromRootSpan(isolationScope => { - isolationScope.setTag('bb', 'bb'); - expect(isolationScope).not.toBe(initialIsolationScope); - expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); - }); - }); - }); - - it('works with a next.js span', () => { - const initialIsolationScope = getIsolationScope(); - initialIsolationScope.setTag('aa', 'aa'); - - const customScope = new Scope(); - - startSpan( - { - name: 'other', - attributes: { 'next.route': 'aha' }, - }, - span => { - setCapturedScopesOnSpan(span, getCurrentScope(), customScope); - - withIsolationScopeOrReuseFromRootSpan(isolationScope => { - isolationScope.setTag('bb', 'bb'); - expect(isolationScope).toBe(customScope); - expect(isolationScope.getScopeData().tags).toEqual({ bb: 'bb' }); - }); - }, - ); - }); - - it('works with a next.js span that has default isolation scope', () => { - const initialIsolationScope = getIsolationScope(); - initialIsolationScope.setTag('aa', 'aa'); - - startSpan( - { - name: 'other', - attributes: { 'next.route': 'aha' }, - }, - () => { - withIsolationScopeOrReuseFromRootSpan(isolationScope => { - isolationScope.setTag('bb', 'bb'); - expect(isolationScope).not.toBe(initialIsolationScope); - expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); - }); - }, - ); - }); -}); From d5ac938e5c7b82e0ef3fb9ef7182f21a4bab000d Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 16 Apr 2024 16:31:11 +0200 Subject: [PATCH 08/21] ref(core): Rename `Hub` to `AsyncContextStack` & remove unneeded methods (#11630) Should save some bundle size...! I also moved stuff around a bit so we don't have so much somewhat unrelated stuff packed into the former `hub.ts` file - needed some work to avoid circular dependencies, but I think it's fine now. I left the stuff we actually need in, and renamed it for clarity so this is not confusing anymore. closes #11482 --- packages/core/src/asyncContext.ts | 119 ---- packages/core/src/asyncContext/index.ts | 31 + .../core/src/asyncContext/stackStrategy.ts | 178 ++++++ packages/core/src/asyncContext/types.ts | 67 ++ packages/core/src/carrier.ts | 59 ++ packages/core/src/currentScopes.ts | 14 +- packages/core/src/hub.ts | 581 ------------------ packages/core/src/index.ts | 16 +- packages/core/src/sdk.ts | 9 +- packages/core/src/tracing/trace.ts | 6 +- packages/core/src/utils/spanUtils.ts | 4 +- packages/core/test/lib/tracing/trace.test.ts | 2 +- 12 files changed, 362 insertions(+), 724 deletions(-) delete mode 100644 packages/core/src/asyncContext.ts create mode 100644 packages/core/src/asyncContext/index.ts create mode 100644 packages/core/src/asyncContext/stackStrategy.ts create mode 100644 packages/core/src/asyncContext/types.ts create mode 100644 packages/core/src/carrier.ts delete mode 100644 packages/core/src/hub.ts diff --git a/packages/core/src/asyncContext.ts b/packages/core/src/asyncContext.ts deleted file mode 100644 index 82d336ded509..000000000000 --- a/packages/core/src/asyncContext.ts +++ /dev/null @@ -1,119 +0,0 @@ -import type { Integration } from '@sentry/types'; -import type { Scope } from '@sentry/types'; -import { GLOBAL_OBJ } from '@sentry/utils'; -import type { startInactiveSpan, startSpan, startSpanManual, suppressTracing, withActiveSpan } from './tracing/trace'; -import type { getActiveSpan } from './utils/spanUtils'; - -/** - * @private Private API with no semver guarantees! - * - * Strategy used to track async context. - */ -export interface AsyncContextStrategy { - /** - * Fork the isolation scope inside of the provided callback. - */ - withIsolationScope: (callback: (isolationScope: Scope) => T) => T; - - /** - * Fork the current scope inside of the provided callback. - */ - withScope: (callback: (isolationScope: Scope) => T) => T; - - /** - * Set the provided scope as the current scope inside of the provided callback. - */ - withSetScope: (scope: Scope, callback: (scope: Scope) => T) => T; - - /** - * Set the provided isolation as the current isolation scope inside of the provided callback. - */ - withSetIsolationScope: (isolationScope: Scope, callback: (isolationScope: Scope) => T) => T; - - /** - * Get the currently active scope. - */ - getCurrentScope: () => Scope; - - /** - * Get the currently active isolation scope. - */ - getIsolationScope: () => Scope; - - // OPTIONAL: Custom tracing methods - // These are used so that we can provide OTEL-based implementations - - /** Start an active span. */ - startSpan?: typeof startSpan; - - /** Start an inactive span. */ - startInactiveSpan?: typeof startInactiveSpan; - - /** Start an active manual span. */ - startSpanManual?: typeof startSpanManual; - - /** Get the currently active span. */ - getActiveSpan?: typeof getActiveSpan; - - /** Make a span the active span in the context of the callback. */ - withActiveSpan?: typeof withActiveSpan; - - /** Suppress tracing in the given callback, ensuring no spans are generated inside of it. */ - suppressTracing?: typeof suppressTracing; -} - -/** - * An object that contains a hub and maintains a scope stack. - * @hidden - */ -export interface Carrier { - __SENTRY__?: SentryCarrier; -} - -interface SentryCarrier { - acs?: AsyncContextStrategy; - /** - * Extra Hub properties injected by various SDKs - */ - integrations?: Integration[]; - extensions?: { - /** Extension methods for the hub, which are bound to the current Hub instance */ - // eslint-disable-next-line @typescript-eslint/ban-types - [key: string]: Function; - }; -} - -/** - * Returns the global shim registry. - * - * FIXME: This function is problematic, because despite always returning a valid Carrier, - * it has an optional `__SENTRY__` property, which then in turn requires us to always perform an unnecessary check - * at the call-site. We always access the carrier through this function, so we can guarantee that `__SENTRY__` is there. - **/ -export function getMainCarrier(): Carrier { - // This ensures a Sentry carrier exists - getSentryCarrier(GLOBAL_OBJ); - return GLOBAL_OBJ; -} - -/** - * @private Private API with no semver guarantees! - * - * Sets the global async context strategy - */ -export function setAsyncContextStrategy(strategy: AsyncContextStrategy | undefined): void { - // Get main carrier (global for every environment) - const registry = getMainCarrier(); - const sentry = getSentryCarrier(registry); - sentry.acs = strategy; -} - -/** Will either get the existing sentry carrier, or create a new one. */ -export function getSentryCarrier(carrier: Carrier): SentryCarrier { - if (!carrier.__SENTRY__) { - carrier.__SENTRY__ = { - extensions: {}, - }; - } - return carrier.__SENTRY__; -} diff --git a/packages/core/src/asyncContext/index.ts b/packages/core/src/asyncContext/index.ts new file mode 100644 index 000000000000..d13874bd854b --- /dev/null +++ b/packages/core/src/asyncContext/index.ts @@ -0,0 +1,31 @@ +import type { Carrier } from './../carrier'; +import { getMainCarrier, getSentryCarrier } from './../carrier'; +import { getStackAsyncContextStrategy } from './stackStrategy'; +import type { AsyncContextStrategy } from './types'; + +/** + * @private Private API with no semver guarantees! + * + * Sets the global async context strategy + */ +export function setAsyncContextStrategy(strategy: AsyncContextStrategy | undefined): void { + // Get main carrier (global for every environment) + const registry = getMainCarrier(); + const sentry = getSentryCarrier(registry); + sentry.acs = strategy; +} + +/** + * Get the current async context strategy. + * If none has been setup, the default will be used. + */ +export function getAsyncContextStrategy(carrier: Carrier): AsyncContextStrategy { + const sentry = getSentryCarrier(carrier); + + if (sentry.acs) { + return sentry.acs; + } + + // Otherwise, use the default one (stack) + return getStackAsyncContextStrategy(); +} diff --git a/packages/core/src/asyncContext/stackStrategy.ts b/packages/core/src/asyncContext/stackStrategy.ts new file mode 100644 index 000000000000..4ec7445d1b3e --- /dev/null +++ b/packages/core/src/asyncContext/stackStrategy.ts @@ -0,0 +1,178 @@ +import type { Client, Scope as ScopeInterface } from '@sentry/types'; +import { isThenable } from '@sentry/utils'; +import { getDefaultCurrentScope, getDefaultIsolationScope } from '../currentScopes'; +import { Scope } from '../scope'; + +import { getMainCarrier, getSentryCarrier } from './../carrier'; +import type { AsyncContextStrategy } from './types'; + +interface Layer { + client?: Client; + scope: ScopeInterface; +} + +/** + * This is an object that holds a stack of scopes. + */ +export class AsyncContextStack { + private readonly _stack: Layer[]; + private _isolationScope: ScopeInterface; + + public constructor(scope?: ScopeInterface, isolationScope?: ScopeInterface) { + let assignedScope; + if (!scope) { + assignedScope = new Scope(); + } else { + assignedScope = scope; + } + + let assignedIsolationScope; + if (!isolationScope) { + assignedIsolationScope = new Scope(); + } else { + assignedIsolationScope = isolationScope; + } + + this._stack = [{ scope: assignedScope }]; + this._isolationScope = assignedIsolationScope; + } + + /** + * Fork a scope for the stack. + */ + public withScope(callback: (scope: ScopeInterface) => T): T { + const scope = this._pushScope(); + + let maybePromiseResult: T; + try { + maybePromiseResult = callback(scope); + } catch (e) { + this._popScope(); + throw e; + } + + if (isThenable(maybePromiseResult)) { + // @ts-expect-error - isThenable returns the wrong type + return maybePromiseResult.then( + res => { + this._popScope(); + return res; + }, + e => { + this._popScope(); + throw e; + }, + ); + } + + this._popScope(); + return maybePromiseResult; + } + + /** + * Get the client of the stack. + */ + public getClient(): C | undefined { + return this.getStackTop().client as C; + } + + /** + * Returns the scope of the top stack. + */ + public getScope(): ScopeInterface { + return this.getStackTop().scope; + } + + /** + * Get the isolation scope for the stack. + */ + public getIsolationScope(): ScopeInterface { + return this._isolationScope; + } + + /** + * Returns the scope stack for domains or the process. + */ + public getStack(): Layer[] { + return this._stack; + } + + /** + * Returns the topmost scope layer in the order domain > local > process. + */ + public getStackTop(): Layer { + return this._stack[this._stack.length - 1]; + } + + /** + * Push a scope to the stack. + */ + private _pushScope(): ScopeInterface { + // We want to clone the content of prev scope + const scope = this.getScope().clone(); + this.getStack().push({ + client: this.getClient(), + scope, + }); + return scope; + } + + /** + * Pop a scope from the stack. + */ + private _popScope(): boolean { + if (this.getStack().length <= 1) return false; + return !!this.getStack().pop(); + } +} + +/** + * Get the global async context stack. + * This will be removed during the v8 cycle and is only here to make migration easier. + */ +function getAsyncContextStack(): AsyncContextStack { + const registry = getMainCarrier(); + const sentry = getSentryCarrier(registry) as { hub?: AsyncContextStack }; + + // If there's no hub, or its an old API, assign a new one + if (sentry.hub) { + return sentry.hub; + } + + sentry.hub = new AsyncContextStack(getDefaultCurrentScope(), getDefaultIsolationScope()); + return sentry.hub; +} + +function withScope(callback: (scope: ScopeInterface) => T): T { + return getAsyncContextStack().withScope(callback); +} + +function withSetScope(scope: ScopeInterface, callback: (scope: ScopeInterface) => T): T { + const hub = getAsyncContextStack() as AsyncContextStack; + return hub.withScope(() => { + hub.getStackTop().scope = scope; + return callback(scope); + }); +} + +function withIsolationScope(callback: (isolationScope: ScopeInterface) => T): T { + return getAsyncContextStack().withScope(() => { + return callback(getAsyncContextStack().getIsolationScope()); + }); +} + +/** + * Get the stack-based async context strategy. + */ +export function getStackAsyncContextStrategy(): AsyncContextStrategy { + return { + withIsolationScope, + withScope, + withSetScope, + withSetIsolationScope: (_isolationScope: ScopeInterface, callback: (isolationScope: ScopeInterface) => T) => { + return withIsolationScope(callback); + }, + getCurrentScope: () => getAsyncContextStack().getScope(), + getIsolationScope: () => getAsyncContextStack().getIsolationScope(), + }; +} diff --git a/packages/core/src/asyncContext/types.ts b/packages/core/src/asyncContext/types.ts new file mode 100644 index 000000000000..bd69c8e63e78 --- /dev/null +++ b/packages/core/src/asyncContext/types.ts @@ -0,0 +1,67 @@ +import type { Scope } from '@sentry/types'; +import type { + startInactiveSpan, + startSpan, + startSpanManual, + suppressTracing, + withActiveSpan, +} from './../tracing/trace'; +import type { getActiveSpan } from './../utils/spanUtils'; + +/** + * @private Private API with no semver guarantees! + * + * Strategy used to track async context. + */ +export interface AsyncContextStrategy { + /** + * Fork the isolation scope inside of the provided callback. + */ + withIsolationScope: (callback: (isolationScope: Scope) => T) => T; + + /** + * Fork the current scope inside of the provided callback. + */ + withScope: (callback: (isolationScope: Scope) => T) => T; + + /** + * Set the provided scope as the current scope inside of the provided callback. + */ + withSetScope: (scope: Scope, callback: (scope: Scope) => T) => T; + + /** + * Set the provided isolation as the current isolation scope inside of the provided callback. + */ + withSetIsolationScope: (isolationScope: Scope, callback: (isolationScope: Scope) => T) => T; + + /** + * Get the currently active scope. + */ + getCurrentScope: () => Scope; + + /** + * Get the currently active isolation scope. + */ + getIsolationScope: () => Scope; + + // OPTIONAL: Custom tracing methods + // These are used so that we can provide OTEL-based implementations + + /** Start an active span. */ + startSpan?: typeof startSpan; + + /** Start an inactive span. */ + startInactiveSpan?: typeof startInactiveSpan; + + /** Start an active manual span. */ + startSpanManual?: typeof startSpanManual; + + /** Get the currently active span. */ + getActiveSpan?: typeof getActiveSpan; + + /** Make a span the active span in the context of the callback. */ + withActiveSpan?: typeof withActiveSpan; + + /** Suppress tracing in the given callback, ensuring no spans are generated inside of it. */ + suppressTracing?: typeof suppressTracing; +} diff --git a/packages/core/src/carrier.ts b/packages/core/src/carrier.ts new file mode 100644 index 000000000000..215ed9673148 --- /dev/null +++ b/packages/core/src/carrier.ts @@ -0,0 +1,59 @@ +import type { Integration } from '@sentry/types'; +import { GLOBAL_OBJ } from '@sentry/utils'; +import type { AsyncContextStrategy } from './asyncContext/types'; + +/** + * An object that contains a hub and maintains a scope stack. + * @hidden + */ +export interface Carrier { + __SENTRY__?: SentryCarrier; +} + +interface SentryCarrier { + acs?: AsyncContextStrategy; +} + +/** + * An object that contains a hub and maintains a scope stack. + * @hidden + */ +export interface Carrier { + __SENTRY__?: SentryCarrier; +} + +interface SentryCarrier { + acs?: AsyncContextStrategy; + /** + * Extra Hub properties injected by various SDKs + */ + integrations?: Integration[]; + extensions?: { + /** Extension methods for the hub, which are bound to the current Hub instance */ + // eslint-disable-next-line @typescript-eslint/ban-types + [key: string]: Function; + }; +} + +/** + * Returns the global shim registry. + * + * FIXME: This function is problematic, because despite always returning a valid Carrier, + * it has an optional `__SENTRY__` property, which then in turn requires us to always perform an unnecessary check + * at the call-site. We always access the carrier through this function, so we can guarantee that `__SENTRY__` is there. + **/ +export function getMainCarrier(): Carrier { + // This ensures a Sentry carrier exists + getSentryCarrier(GLOBAL_OBJ); + return GLOBAL_OBJ; +} + +/** Will either get the existing sentry carrier, or create a new one. */ +export function getSentryCarrier(carrier: Carrier): SentryCarrier { + if (!carrier.__SENTRY__) { + carrier.__SENTRY__ = { + extensions: {}, + }; + } + return carrier.__SENTRY__; +} diff --git a/packages/core/src/currentScopes.ts b/packages/core/src/currentScopes.ts index e0b598c1ad39..86978759252b 100644 --- a/packages/core/src/currentScopes.ts +++ b/packages/core/src/currentScopes.ts @@ -1,10 +1,20 @@ import type { Scope } from '@sentry/types'; import type { Client } from '@sentry/types'; import { getGlobalSingleton } from '@sentry/utils'; -import { getMainCarrier } from './asyncContext'; -import { getAsyncContextStrategy } from './hub'; +import { getAsyncContextStrategy } from './asyncContext'; +import { getMainCarrier } from './carrier'; import { Scope as ScopeClass } from './scope'; +/** Get the default current scope. */ +export function getDefaultCurrentScope(): Scope { + return getGlobalSingleton('defaultCurrentScope', () => new ScopeClass()); +} + +/** Get the default isolation scope. */ +export function getDefaultIsolationScope(): Scope { + return getGlobalSingleton('defaultIsolationScope', () => new ScopeClass()); +} + /** * Get the currently active scope. */ diff --git a/packages/core/src/hub.ts b/packages/core/src/hub.ts deleted file mode 100644 index 1bd3ec7c64b6..000000000000 --- a/packages/core/src/hub.ts +++ /dev/null @@ -1,581 +0,0 @@ -/* eslint-disable max-lines */ -import type { - Breadcrumb, - BreadcrumbHint, - Client, - Event, - EventHint, - Extra, - Extras, - Hub as HubInterface, - Integration, - IntegrationClass, - Primitive, - Scope as ScopeInterface, - Session, - SessionContext, - SeverityLevel, - User, -} from '@sentry/types'; -import { getGlobalSingleton } from '@sentry/utils'; -import { GLOBAL_OBJ, consoleSandbox, dateTimestampInSeconds, isThenable, logger, uuid4 } from '@sentry/utils'; - -import type { AsyncContextStrategy, Carrier } from './asyncContext'; -import { getMainCarrier, getSentryCarrier } from './asyncContext'; -import { DEFAULT_ENVIRONMENT } from './constants'; -import { DEBUG_BUILD } from './debug-build'; -import { Scope } from './scope'; -import { closeSession, makeSession, updateSession } from './session'; -import { SDK_VERSION } from './version'; - -/** - * API compatibility version of this hub. - * - * WARNING: This number should only be increased when the global interface - * changes and new methods are introduced. - * - * @hidden - */ -export const API_VERSION = parseFloat(SDK_VERSION); - -/** - * Default maximum number of breadcrumbs added to an event. Can be overwritten - * with {@link Options.maxBreadcrumbs}. - */ -const DEFAULT_BREADCRUMBS = 100; - -/** - * A layer in the process stack. - * @hidden - */ -export interface Layer { - client?: Client; - scope: ScopeInterface; -} - -/** - * @inheritDoc - * @deprecated This class will be removed in v8 (tmp-deprecating so we're aware of where this is a problem) - */ -// eslint-disable-next-line deprecation/deprecation -export class Hub implements HubInterface { - /** Is a {@link Layer}[] containing the client and scope */ - private readonly _stack: Layer[]; - - private _isolationScope: ScopeInterface; - - /** - * Creates a new instance of the hub, will push one {@link Layer} into the - * internal stack on creation. - * - * @param client bound to the hub. - * @param scope bound to the hub. - * @param version number, higher number means higher priority. - * - * @deprecated Instantiation of Hub objects is deprecated and the constructor will be removed in version 8 of the SDK. - * - * If you are currently using the Hub for multi-client use like so: - * - * ``` - * // OLD - * const hub = new Hub(); - * hub.bindClient(client); - * makeMain(hub) - * ``` - * - * instead initialize the client as follows: - * - * ``` - * // NEW - * Sentry.withIsolationScope(() => { - * Sentry.setCurrentClient(client); - * client.init(); - * }); - * ``` - * - * If you are using the Hub to capture events like so: - * - * ``` - * // OLD - * const client = new Client(); - * const hub = new Hub(client); - * hub.captureException() - * ``` - * - * instead capture isolated events as follows: - * - * ``` - * // NEW - * const client = new Client(); - * const scope = new Scope(); - * scope.setClient(client); - * scope.captureException(); - * ``` - */ - public constructor( - client?: Client, - scope?: ScopeInterface, - isolationScope?: ScopeInterface, - private readonly _version: number = API_VERSION, - ) { - let assignedScope; - if (!scope) { - assignedScope = new Scope(); - assignedScope.setClient(client); - } else { - assignedScope = scope; - } - - let assignedIsolationScope; - if (!isolationScope) { - assignedIsolationScope = new Scope(); - assignedIsolationScope.setClient(client); - } else { - assignedIsolationScope = isolationScope; - } - - this._stack = [{ scope: assignedScope }]; - - if (client) { - // eslint-disable-next-line deprecation/deprecation - this.bindClient(client); - } - - this._isolationScope = assignedIsolationScope; - } - - /** - * This binds the given client to the current scope. - * @param client An SDK client (client) instance. - * - * @deprecated Use `initAndBind()` directly, or `setCurrentClient()` and/or `client.init()` instead. - */ - public bindClient(client?: Client): void { - // eslint-disable-next-line deprecation/deprecation - const top = this.getStackTop(); - top.client = client; - top.scope.setClient(client); - if (client) { - client.init(); - } - } - - /** - * @inheritDoc - * - * @deprecated Use `Sentry.withScope()` instead. - */ - public withScope(callback: (scope: ScopeInterface) => T): T { - const scope = this._pushScope(); - - let maybePromiseResult: T; - try { - maybePromiseResult = callback(scope); - } catch (e) { - this._popScope(); - throw e; - } - - if (isThenable(maybePromiseResult)) { - // @ts-expect-error - isThenable returns the wrong type - return maybePromiseResult.then( - res => { - this._popScope(); - return res; - }, - e => { - this._popScope(); - throw e; - }, - ); - } - - this._popScope(); - return maybePromiseResult; - } - - /** - * @inheritDoc - * - * @deprecated Use `Sentry.getClient()` instead. - */ - public getClient(): C | undefined { - // eslint-disable-next-line deprecation/deprecation - return this.getStackTop().client as C; - } - - /** - * Returns the scope of the top stack. - * - * @deprecated Use `Sentry.getCurrentScope()` instead. - */ - public getScope(): ScopeInterface { - // eslint-disable-next-line deprecation/deprecation - return this.getStackTop().scope; - } - - /** - * @deprecated Use `Sentry.getIsolationScope()` instead. - */ - public getIsolationScope(): ScopeInterface { - return this._isolationScope; - } - - /** - * Returns the scope stack for domains or the process. - * @deprecated This will be removed in v8. - */ - public getStack(): Layer[] { - return this._stack; - } - - /** - * Returns the topmost scope layer in the order domain > local > process. - * @deprecated This will be removed in v8. - */ - public getStackTop(): Layer { - return this._stack[this._stack.length - 1]; - } - - /** - * @inheritDoc - * - * @deprecated Use `Sentry.captureException()` instead. - */ - public captureException(exception: unknown, hint?: EventHint): string { - const eventId = hint && hint.event_id ? hint.event_id : uuid4(); - const syntheticException = new Error('Sentry syntheticException'); - // eslint-disable-next-line deprecation/deprecation - this.getScope().captureException(exception, { - originalException: exception, - syntheticException, - ...hint, - event_id: eventId, - }); - - return eventId; - } - - /** - * @inheritDoc - * - * @deprecated Use `Sentry.captureMessage()` instead. - */ - public captureMessage(message: string, level?: SeverityLevel, hint?: EventHint): string { - const eventId = hint && hint.event_id ? hint.event_id : uuid4(); - const syntheticException = new Error(message); - // eslint-disable-next-line deprecation/deprecation - this.getScope().captureMessage(message, level, { - originalException: message, - syntheticException, - ...hint, - event_id: eventId, - }); - - return eventId; - } - - /** - * @inheritDoc - * - * @deprecated Use `Sentry.captureEvent()` instead. - */ - public captureEvent(event: Event, hint?: EventHint): string { - const eventId = hint && hint.event_id ? hint.event_id : uuid4(); - // eslint-disable-next-line deprecation/deprecation - this.getScope().captureEvent(event, { ...hint, event_id: eventId }); - return eventId; - } - - /** - * @inheritDoc - * - * @deprecated Use `Sentry.addBreadcrumb()` instead. - */ - public addBreadcrumb(breadcrumb: Breadcrumb, hint?: BreadcrumbHint): void { - // eslint-disable-next-line deprecation/deprecation - const { client } = this.getStackTop(); - - if (!client) return; - - const { beforeBreadcrumb = null, maxBreadcrumbs = DEFAULT_BREADCRUMBS } = - (client.getOptions && client.getOptions()) || {}; - - if (maxBreadcrumbs <= 0) return; - - const timestamp = dateTimestampInSeconds(); - const mergedBreadcrumb = { timestamp, ...breadcrumb }; - const finalBreadcrumb = beforeBreadcrumb - ? (consoleSandbox(() => beforeBreadcrumb(mergedBreadcrumb, hint)) as Breadcrumb | null) - : mergedBreadcrumb; - - if (finalBreadcrumb === null) return; - - client.emit('beforeAddBreadcrumb', finalBreadcrumb, hint); - - // eslint-disable-next-line deprecation/deprecation - this.getIsolationScope().addBreadcrumb(finalBreadcrumb, maxBreadcrumbs); - } - - /** - * @inheritDoc - * @deprecated Use `Sentry.setUser()` instead. - */ - public setUser(user: User | null): void { - // eslint-disable-next-line deprecation/deprecation - this.getIsolationScope().setUser(user); - } - - /** - * @inheritDoc - * @deprecated Use `Sentry.setTags()` instead. - */ - public setTags(tags: { [key: string]: Primitive }): void { - // eslint-disable-next-line deprecation/deprecation - this.getIsolationScope().setTags(tags); - } - - /** - * @inheritDoc - * @deprecated Use `Sentry.setExtras()` instead. - */ - public setExtras(extras: Extras): void { - // eslint-disable-next-line deprecation/deprecation - this.getIsolationScope().setExtras(extras); - } - - /** - * @inheritDoc - * @deprecated Use `Sentry.setTag()` instead. - */ - public setTag(key: string, value: Primitive): void { - // eslint-disable-next-line deprecation/deprecation - this.getIsolationScope().setTag(key, value); - } - - /** - * @inheritDoc - * @deprecated Use `Sentry.setExtra()` instead. - */ - public setExtra(key: string, extra: Extra): void { - // eslint-disable-next-line deprecation/deprecation - this.getIsolationScope().setExtra(key, extra); - } - - /** - * @inheritDoc - * @deprecated Use `Sentry.setContext()` instead. - */ - // eslint-disable-next-line @typescript-eslint/no-explicit-any - public setContext(name: string, context: { [key: string]: any } | null): void { - // eslint-disable-next-line deprecation/deprecation - this.getIsolationScope().setContext(name, context); - } - - /** - * @inheritDoc - * @deprecated Use `Sentry.getClient().getIntegrationByName()` instead. - */ - public getIntegration(integration: IntegrationClass): T | null { - // eslint-disable-next-line deprecation/deprecation - const client = this.getClient(); - if (!client) return null; - try { - return client.getIntegrationByName(integration.id) || null; - } catch (_oO) { - DEBUG_BUILD && logger.warn(`Cannot retrieve integration ${integration.id} from the current Hub`); - return null; - } - } - - /** - * @inheritDoc - * - * @deprecated Use top level `captureSession` instead. - */ - public captureSession(endSession: boolean = false): void { - // both send the update and pull the session from the scope - if (endSession) { - // eslint-disable-next-line deprecation/deprecation - return this.endSession(); - } - - // only send the update - this._sendSessionUpdate(); - } - - /** - * @inheritDoc - * @deprecated Use top level `endSession` instead. - */ - public endSession(): void { - // eslint-disable-next-line deprecation/deprecation - const layer = this.getStackTop(); - const scope = layer.scope; - const session = scope.getSession(); - if (session) { - closeSession(session); - } - this._sendSessionUpdate(); - - // the session is over; take it off of the scope - scope.setSession(); - } - - /** - * @inheritDoc - * @deprecated Use top level `startSession` instead. - */ - public startSession(context?: SessionContext): Session { - // eslint-disable-next-line deprecation/deprecation - const { scope, client } = this.getStackTop(); - const { release, environment = DEFAULT_ENVIRONMENT } = (client && client.getOptions()) || {}; - - // Will fetch userAgent if called from browser sdk - const { userAgent } = GLOBAL_OBJ.navigator || {}; - - const session = makeSession({ - release, - environment, - user: scope.getUser(), - ...(userAgent && { userAgent }), - ...context, - }); - - // End existing session if there's one - const currentSession = scope.getSession && scope.getSession(); - if (currentSession && currentSession.status === 'ok') { - updateSession(currentSession, { status: 'exited' }); - } - // eslint-disable-next-line deprecation/deprecation - this.endSession(); - - // Afterwards we set the new session on the scope - scope.setSession(session); - - return session; - } - - /** - * Sends the current Session on the scope - */ - private _sendSessionUpdate(): void { - // eslint-disable-next-line deprecation/deprecation - const { scope, client } = this.getStackTop(); - - const session = scope.getSession(); - if (session && client && client.captureSession) { - client.captureSession(session); - } - } - - /** - * Push a scope to the stack. - */ - private _pushScope(): ScopeInterface { - // We want to clone the content of prev scope - // eslint-disable-next-line deprecation/deprecation - const scope = this.getScope().clone(); - // eslint-disable-next-line deprecation/deprecation - this.getStack().push({ - // eslint-disable-next-line deprecation/deprecation - client: this.getClient(), - scope, - }); - return scope; - } - - /** - * Pop a scope from the stack. - */ - private _popScope(): boolean { - // eslint-disable-next-line deprecation/deprecation - if (this.getStack().length <= 1) return false; - // eslint-disable-next-line deprecation/deprecation - return !!this.getStack().pop(); - } -} - -/** Get the default current scope. */ -export function getDefaultCurrentScope(): Scope { - return getGlobalSingleton('defaultCurrentScope', () => new Scope()); -} - -/** Get the default isolation scope. */ -export function getDefaultIsolationScope(): Scope { - return getGlobalSingleton('defaultIsolationScope', () => new Scope()); -} - -/** - * Get the global hub. - * This will be removed during the v8 cycle and is only here to make migration easier. - */ -// eslint-disable-next-line deprecation/deprecation -export function getGlobalHub(): HubInterface { - const registry = getMainCarrier(); - // eslint-disable-next-line deprecation/deprecation - const sentry = getSentryCarrier(registry) as { hub?: HubInterface }; - - // If there's no hub, or its an old API, assign a new one - if (sentry.hub) { - return sentry.hub; - } - - // eslint-disable-next-line deprecation/deprecation - sentry.hub = new Hub(undefined, getDefaultCurrentScope(), getDefaultIsolationScope()); - return sentry.hub; -} - -/** - * Get the current async context strategy. - * If none has been setup, the default will be used. - */ -export function getAsyncContextStrategy(carrier: Carrier): AsyncContextStrategy { - const sentry = getSentryCarrier(carrier); - - if (sentry.acs) { - return sentry.acs; - } - - // Otherwise, use the default one - return getHubStackAsyncContextStrategy(); -} - -function withScope(callback: (scope: ScopeInterface) => T): T { - // eslint-disable-next-line deprecation/deprecation - return getGlobalHub().withScope(callback); -} - -function withSetScope(scope: ScopeInterface, callback: (scope: ScopeInterface) => T): T { - // eslint-disable-next-line deprecation/deprecation - const hub = getGlobalHub() as Hub; - // eslint-disable-next-line deprecation/deprecation - return hub.withScope(() => { - // eslint-disable-next-line deprecation/deprecation - hub.getStackTop().scope = scope as Scope; - return callback(scope); - }); -} - -function withIsolationScope(callback: (isolationScope: ScopeInterface) => T): T { - // eslint-disable-next-line deprecation/deprecation - return getGlobalHub().withScope(() => { - // eslint-disable-next-line deprecation/deprecation - return callback(getGlobalHub().getIsolationScope()); - }); -} - -/* eslint-disable deprecation/deprecation */ -function getHubStackAsyncContextStrategy(): AsyncContextStrategy { - return { - withIsolationScope, - withScope, - withSetScope, - withSetIsolationScope: (_isolationScope: ScopeInterface, callback: (isolationScope: ScopeInterface) => T) => { - return withIsolationScope(callback); - }, - getCurrentScope: () => getGlobalHub().getScope(), - getIsolationScope: () => getGlobalHub().getIsolationScope(), - }; -} -/* eslint-enable deprecation/deprecation */ diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index cf3415302314..a863c6ee271d 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1,6 +1,6 @@ export type { ClientClass } from './sdk'; -export type { Layer } from './hub'; -export type { AsyncContextStrategy, Carrier } from './asyncContext'; +export type { AsyncContextStrategy } from './asyncContext/types'; +export type { Carrier } from './carrier'; export type { OfflineStore, OfflineTransportOptions } from './transports/offline'; export type { ServerRuntimeClientOptions } from './server-runtime-client'; export type { RequestDataIntegrationOptions } from './integrations/requestdata'; @@ -29,10 +29,6 @@ export { captureSession, addEventProcessor, } from './exports'; -export { - getDefaultCurrentScope, - getDefaultIsolationScope, -} from './hub'; export { getCurrentScope, getIsolationScope, @@ -40,11 +36,11 @@ export { withScope, withIsolationScope, getClient, + getDefaultCurrentScope, + getDefaultIsolationScope, } from './currentScopes'; -export { - getMainCarrier, - setAsyncContextStrategy, -} from './asyncContext'; +export { setAsyncContextStrategy } from './asyncContext'; +export { getMainCarrier } from './carrier'; export { makeSession, closeSession, updateSession } from './session'; export { SessionFlusher } from './sessionflusher'; export { Scope } from './scope'; diff --git a/packages/core/src/sdk.ts b/packages/core/src/sdk.ts index ebe8f9a6ca22..85fdeae6e4d6 100644 --- a/packages/core/src/sdk.ts +++ b/packages/core/src/sdk.ts @@ -2,9 +2,9 @@ import type { Client, ClientOptions } from '@sentry/types'; import { consoleSandbox, logger } from '@sentry/utils'; import { getCurrentScope } from './currentScopes'; -import { getMainCarrier, getSentryCarrier } from './asyncContext'; +import type { AsyncContextStack } from './asyncContext/stackStrategy'; +import { getMainCarrier, getSentryCarrier } from './carrier'; import { DEBUG_BUILD } from './debug-build'; -import type { Hub } from './hub'; /** A class object that can instantiate Client objects. */ export type ClientClass = new (options: O) => F; @@ -55,11 +55,8 @@ export function setCurrentClient(client: Client): void { * @see {@link hub.ts getGlobalHub} */ function registerClientOnGlobalHub(client: Client): void { - // eslint-disable-next-line deprecation/deprecation - const sentryGlobal = getSentryCarrier(getMainCarrier()) as { hub?: Hub }; - // eslint-disable-next-line deprecation/deprecation + const sentryGlobal = getSentryCarrier(getMainCarrier()) as { hub?: AsyncContextStack }; if (sentryGlobal.hub && typeof sentryGlobal.hub.getStackTop === 'function') { - // eslint-disable-next-line deprecation/deprecation sentryGlobal.hub.getStackTop().client = client; } } diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 791cae88a573..065403c78aed 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -1,11 +1,11 @@ import type { ClientOptions, Scope, SentrySpanArguments, Span, SpanTimeInput, StartSpanOptions } from '@sentry/types'; import { propagationContextFromHeaders } from '@sentry/utils'; -import type { AsyncContextStrategy } from '../asyncContext'; -import { getMainCarrier } from '../asyncContext'; +import type { AsyncContextStrategy } from '../asyncContext/types'; +import { getMainCarrier } from '../carrier'; import { getClient, getCurrentScope, getIsolationScope, withScope } from '../currentScopes'; -import { getAsyncContextStrategy } from '../hub'; +import { getAsyncContextStrategy } from '../asyncContext'; import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes'; import { handleCallbackErrors } from '../utils/handleCallbackErrors'; import { hasTracingEnabled } from '../utils/hasTracingEnabled'; diff --git a/packages/core/src/utils/spanUtils.ts b/packages/core/src/utils/spanUtils.ts index 3d06aa0d3204..b4fb587dd5d3 100644 --- a/packages/core/src/utils/spanUtils.ts +++ b/packages/core/src/utils/spanUtils.ts @@ -15,9 +15,9 @@ import { generateSentryTraceHeader, timestampInSeconds, } from '@sentry/utils'; -import { getMainCarrier } from '../asyncContext'; +import { getAsyncContextStrategy } from '../asyncContext'; +import { getMainCarrier } from '../carrier'; import { getCurrentScope } from '../currentScopes'; -import { getAsyncContextStrategy } from '../hub'; import { getMetricSummaryJsonForSpan, updateMetricSummaryOnSpan } from '../metrics/metric-summary'; import type { MetricType } from '../metrics/types'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../semanticAttributes'; diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index b4b73307e174..110a90d01f6f 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -12,7 +12,7 @@ import { spanToJSON, withScope, } from '../../../src'; -import { getAsyncContextStrategy } from '../../../src/hub'; +import { getAsyncContextStrategy } from '../../../src/asyncContext'; import { SentrySpan, continueTrace, From 379a9e51d7293f3747c1b14cb7d8bfadfbc11960 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 16 Apr 2024 16:34:47 +0200 Subject: [PATCH 09/21] fix(node): Ensure DSC is correctly set in envelope headers (#11628) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was not correctly set before when we had an active span - the `error-active-span` & `error-active-span-unsampled` scenarios failed before and had no `trace` set on the envelope headers. The fix was to actually fix our `setupEventContextTrace` utility for OTEL, where we now just set the dsc on the event sdkProcessingMetadata, which is picked up when event processing. I also moved this to `preprocessEvent` to ensure this runs before other stuff. This should fix https://github.com/getsentry/sentry-javascript-examples/pull/7/files#r1561220216 which @s1gr1d found in her example app. 🚀 --- .../error-active-span-unsampled/scenario.ts | 15 +++++ .../error-active-span-unsampled/test.ts | 18 ++++++ .../error-active-span/scenario.ts | 15 +++++ .../envelope-header/error-active-span/test.ts | 20 +++++++ .../tracing/envelope-header/error/scenario.ts | 13 ++++ .../tracing/envelope-header/error/test.ts | 17 ++++++ .../transaction-route/scenario.ts | 26 ++++++++ .../envelope-header/transaction-route/test.ts | 20 +++++++ .../transaction-url/scenario.ts | 26 ++++++++ .../envelope-header/transaction-url/test.ts | 19 ++++++ .../envelope-header/transaction/scenario.ts | 15 +++++ .../envelope-header/transaction/test.ts | 20 +++++++ .../node-integration-tests/utils/runner.ts | 59 ++++++++++++++++++- .../src/setupEventContextTrace.ts | 12 ++-- 14 files changed, 289 insertions(+), 6 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span-unsampled/scenario.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span-unsampled/test.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span/scenario.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span/test.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/envelope-header/error/scenario.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/envelope-header/error/test.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-route/scenario.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-route/test.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-url/scenario.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-url/test.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction/scenario.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction/test.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span-unsampled/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span-unsampled/scenario.ts new file mode 100644 index 000000000000..d88751f77ff5 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span-unsampled/scenario.ts @@ -0,0 +1,15 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracePropagationTargets: [/\/v0/, 'v1'], + tracesSampleRate: 0, + integrations: [], + transport: loggingTransport, +}); + +Sentry.startSpan({ name: 'test span' }, () => { + Sentry.captureException(new Error('foo')); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span-unsampled/test.ts b/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span-unsampled/test.ts new file mode 100644 index 000000000000..b4d971654ea0 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span-unsampled/test.ts @@ -0,0 +1,18 @@ +import { createRunner } from '../../../../utils/runner'; + +test('envelope header for error event during active unsampled span is correct', done => { + createRunner(__dirname, 'scenario.ts') + .ignore('session', 'sessions', 'transaction') + .expectHeader({ + event: { + trace: { + trace_id: expect.any(String), + public_key: 'public', + environment: 'production', + release: '1.0', + sampled: 'false', + }, + }, + }) + .start(done); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span/scenario.ts new file mode 100644 index 000000000000..082db9d94b82 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span/scenario.ts @@ -0,0 +1,15 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracePropagationTargets: [/\/v0/, 'v1'], + tracesSampleRate: 1, + integrations: [], + transport: loggingTransport, +}); + +Sentry.startSpan({ name: 'test span' }, () => { + Sentry.captureException(new Error('foo')); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span/test.ts b/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span/test.ts new file mode 100644 index 000000000000..0e425ac58d55 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span/test.ts @@ -0,0 +1,20 @@ +import { createRunner } from '../../../../utils/runner'; + +test('envelope header for error event during active span is correct', done => { + createRunner(__dirname, 'scenario.ts') + .ignore('session', 'sessions', 'transaction') + .expectHeader({ + event: { + trace: { + trace_id: expect.any(String), + public_key: 'public', + environment: 'production', + release: '1.0', + sample_rate: '1', + sampled: 'true', + transaction: 'test span', + }, + }, + }) + .start(done); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/envelope-header/error/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/envelope-header/error/scenario.ts new file mode 100644 index 000000000000..16eba4ecfd4c --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/envelope-header/error/scenario.ts @@ -0,0 +1,13 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracePropagationTargets: [/\/v0/, 'v1'], + tracesSampleRate: 0, + integrations: [], + transport: loggingTransport, +}); + +Sentry.captureException(new Error('foo')); diff --git a/dev-packages/node-integration-tests/suites/tracing/envelope-header/error/test.ts b/dev-packages/node-integration-tests/suites/tracing/envelope-header/error/test.ts new file mode 100644 index 000000000000..e45e18baa29a --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/envelope-header/error/test.ts @@ -0,0 +1,17 @@ +import { createRunner } from '../../../../utils/runner'; + +test('envelope header for error events is correct', done => { + createRunner(__dirname, 'scenario.ts') + .ignore('session', 'sessions') + .expectHeader({ + event: { + trace: { + trace_id: expect.any(String), + environment: 'production', + public_key: 'public', + release: '1.0', + }, + }, + }) + .start(done); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-route/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-route/scenario.ts new file mode 100644 index 000000000000..21bc821787fe --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-route/scenario.ts @@ -0,0 +1,26 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracePropagationTargets: [/\/v0/, 'v1'], + tracesSampleRate: 1, + integrations: [], + transport: loggingTransport, +}); + +Sentry.startSpan( + { + name: 'GET /route', + attributes: { + 'http.method': 'GET', + 'http.route': '/route', + [Sentry.SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', + [Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + }, + }, + () => { + // noop + }, +); diff --git a/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-route/test.ts b/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-route/test.ts new file mode 100644 index 000000000000..b64ef4ff55ed --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-route/test.ts @@ -0,0 +1,20 @@ +import { createRunner } from '../../../../utils/runner'; + +test('envelope header for transaction event of route correct', done => { + createRunner(__dirname, 'scenario.ts') + .ignore('session', 'sessions') + .expectHeader({ + transaction: { + trace: { + trace_id: expect.any(String), + public_key: 'public', + transaction: 'GET /route', + environment: 'production', + release: '1.0', + sample_rate: '1', + sampled: 'true', + }, + }, + }) + .start(done); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-url/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-url/scenario.ts new file mode 100644 index 000000000000..a4a5f9290216 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-url/scenario.ts @@ -0,0 +1,26 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracePropagationTargets: [/\/v0/, 'v1'], + tracesSampleRate: 1, + integrations: [], + transport: loggingTransport, +}); + +Sentry.startSpan( + { + name: 'GET /route/1', + attributes: { + 'http.method': 'GET', + 'http.route': '/route', + [Sentry.SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', + [Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + }, + }, + () => { + // noop + }, +); diff --git a/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-url/test.ts b/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-url/test.ts new file mode 100644 index 000000000000..80f187165614 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction-url/test.ts @@ -0,0 +1,19 @@ +import { createRunner } from '../../../../utils/runner'; + +test('envelope header for transaction event with source=url correct', done => { + createRunner(__dirname, 'scenario.ts') + .ignore('session', 'sessions') + .expectHeader({ + transaction: { + trace: { + trace_id: expect.any(String), + public_key: 'public', + environment: 'production', + release: '1.0', + sample_rate: '1', + sampled: 'true', + }, + }, + }) + .start(done); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction/scenario.ts new file mode 100644 index 000000000000..7fe727534bc9 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction/scenario.ts @@ -0,0 +1,15 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracePropagationTargets: [/\/v0/, 'v1'], + tracesSampleRate: 1, + integrations: [], + transport: loggingTransport, +}); + +Sentry.startSpan({ name: 'test span' }, () => { + // noop +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction/test.ts b/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction/test.ts new file mode 100644 index 000000000000..10ef348dfaf0 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/envelope-header/transaction/test.ts @@ -0,0 +1,20 @@ +import { createRunner } from '../../../../utils/runner'; + +test('envelope header for transaction event is correct', done => { + createRunner(__dirname, 'scenario.ts') + .ignore('session', 'sessions') + .expectHeader({ + transaction: { + trace: { + trace_id: expect.any(String), + public_key: 'public', + environment: 'production', + release: '1.0', + sample_rate: '1', + sampled: 'true', + transaction: 'test span', + }, + }, + }) + .start(done); +}); diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index eb96059d41c5..98d2e6283687 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -1,6 +1,15 @@ +/* eslint-disable max-lines */ import { spawn, spawnSync } from 'child_process'; import { join } from 'path'; -import type { Envelope, EnvelopeItemType, Event, SerializedSession, SessionAggregates } from '@sentry/types'; +import { SDK_VERSION } from '@sentry/node'; +import type { + Envelope, + EnvelopeItemType, + Event, + EventEnvelope, + SerializedSession, + SessionAggregates, +} from '@sentry/types'; import axios from 'axios'; import { createBasicSentryServer } from './server'; @@ -29,6 +38,18 @@ export function assertSentryTransaction(actual: Event, expected: Partial) }); } +export function assertEnvelopeHeader(actual: Envelope[0], expected: Partial): void { + expect(actual).toEqual({ + event_id: expect.any(String), + sent_at: expect.any(String), + sdk: { + name: 'sentry.javascript.node', + version: SDK_VERSION, + }, + ...expected, + }); +} + const CLEANUP_STEPS = new Set(); export function cleanupChildProcesses(): void { @@ -118,12 +139,19 @@ type Expected = sessions: Partial | ((event: SessionAggregates) => void); }; +type ExpectedEnvelopeHeader = + | { event: Partial } + | { transaction: Partial } + | { session: Partial } + | { sessions: Partial }; + /** Creates a test runner */ // eslint-disable-next-line @typescript-eslint/explicit-function-return-type export function createRunner(...paths: string[]) { const testPath = join(...paths); const expectedEnvelopes: Expected[] = []; + let expectedEnvelopeHeaders: ExpectedEnvelopeHeader[] | undefined = undefined; const flags: string[] = []; const ignored: EnvelopeItemType[] = []; let withEnv: Record = {}; @@ -141,6 +169,14 @@ export function createRunner(...paths: string[]) { expectedEnvelopes.push(expected); return this; }, + expectHeader: function (expected: ExpectedEnvelopeHeader) { + if (!expectedEnvelopeHeaders) { + expectedEnvelopeHeaders = []; + } + + expectedEnvelopeHeaders.push(expected); + return this; + }, expectError: function () { expectError = true; return this; @@ -170,7 +206,7 @@ export function createRunner(...paths: string[]) { return this; }, start: function (done?: (e?: unknown) => void) { - const expectedEnvelopeCount = expectedEnvelopes.length; + const expectedEnvelopeCount = Math.max(expectedEnvelopes.length, (expectedEnvelopeHeaders || []).length); let envelopeCount = 0; let scenarioServerPort: number | undefined; @@ -198,6 +234,25 @@ export function createRunner(...paths: string[]) { continue; } + if (expectedEnvelopeHeaders) { + const header = envelope[0]; + const expected = expectedEnvelopeHeaders.shift()?.[envelopeItemType as keyof ExpectedEnvelopeHeader]; + + try { + if (!expected) { + throw new Error(`No more expected envelope items but we received ${JSON.stringify(header)}`); + } + + assertEnvelopeHeader(header, expected); + + expectCallbackCalled(); + } catch (e) { + complete(e as Error); + } + + return; + } + const expected = expectedEnvelopes.shift(); // Catch any error or failed assertions and pass them to done to end the test quickly diff --git a/packages/opentelemetry/src/setupEventContextTrace.ts b/packages/opentelemetry/src/setupEventContextTrace.ts index 1aa1edbbe12c..c22aa46c57a4 100644 --- a/packages/opentelemetry/src/setupEventContextTrace.ts +++ b/packages/opentelemetry/src/setupEventContextTrace.ts @@ -1,18 +1,19 @@ import { getRootSpan } from '@sentry/core'; import type { Client } from '@sentry/types'; import { dropUndefinedKeys } from '@sentry/utils'; +import { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext'; import { getActiveSpan } from './utils/getActiveSpan'; import { spanHasName, spanHasParentId } from './utils/spanTypes'; /** Ensure the `trace` context is set on all events. */ export function setupEventContextTrace(client: Client): void { - client.addEventProcessor(event => { + client.on('preprocessEvent', event => { const span = getActiveSpan(); // For transaction events, this is handled separately // Because the active span may not be the span that is actually the transaction event if (!span || event.type === 'transaction') { - return event; + return; } const spanContext = span.spanContext(); @@ -27,12 +28,15 @@ export function setupEventContextTrace(client: Client): void { ...event.contexts, }; + event.sdkProcessingMetadata = { + dynamicSamplingContext: getDynamicSamplingContextFromSpan(span), + ...event.sdkProcessingMetadata, + }; + const rootSpan = getRootSpan(span); const transactionName = spanHasName(rootSpan) ? rootSpan.name : undefined; if (transactionName && !event.transaction) { event.transaction = transactionName; } - - return event; }); } From f1c4611c551816ca2fad537a6b5a17ead1b75b16 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 16 Apr 2024 16:37:43 +0200 Subject: [PATCH 10/21] test(browser-integration-test): Add trace lifetime tests for XHR requests (#11624) Add tests that check that XHR request headers propagate the correct traceId, as spec'd out in #11599. --- .../tracing/trace-lifetime/navigation/test.ts | 158 +++++++++++++--- .../pageload-meta/template.html | 1 + .../trace-lifetime/pageload-meta/test.ts | 58 ++++++ .../tracing/trace-lifetime/pageload/test.ts | 169 ++++++++++++++---- .../suites/tracing/trace-lifetime/subject.js | 7 + .../tracing/trace-lifetime/template.html | 1 + 6 files changed, 332 insertions(+), 62 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts index 0ed99e3b7ec9..4bf1d1f7d49a 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts @@ -18,15 +18,24 @@ sentryTest('creates a new trace on each navigation', async ({ getLocalTestPath, const navigationEvent1 = await getFirstSentryEnvelopeRequest(page, `${url}#foo`); const navigationEvent2 = await getFirstSentryEnvelopeRequest(page, `${url}#bar`); - expect(navigationEvent1.contexts?.trace?.op).toBe('navigation'); - expect(navigationEvent2.contexts?.trace?.op).toBe('navigation'); - - const navigation1TraceId = navigationEvent1.contexts?.trace?.trace_id; - const navigation2TraceId = navigationEvent2.contexts?.trace?.trace_id; - - expect(navigation1TraceId).toMatch(/^[0-9a-f]{32}$/); - expect(navigation2TraceId).toMatch(/^[0-9a-f]{32}$/); - expect(navigation1TraceId).not.toEqual(navigation2TraceId); + const navigation1TraceContext = navigationEvent1.contexts?.trace; + const navigation2TraceContext = navigationEvent2.contexts?.trace; + + expect(navigation1TraceContext).toMatchObject({ + op: 'navigation', + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + expect(navigation1TraceContext).not.toHaveProperty('parent_span_id'); + + expect(navigation2TraceContext).toMatchObject({ + op: 'navigation', + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + expect(navigation2TraceContext).not.toHaveProperty('parent_span_id'); + + expect(navigation1TraceContext?.trace_id).not.toEqual(navigation2TraceContext?.trace_id); }); sentryTest('error after navigation has navigation traceId', async ({ getLocalTestPath, page }) => { @@ -40,17 +49,24 @@ sentryTest('error after navigation has navigation traceId', async ({ getLocalTes await getFirstSentryEnvelopeRequest(page, url); const navigationEvent = await getFirstSentryEnvelopeRequest(page, `${url}#foo`); - expect(navigationEvent.contexts?.trace?.op).toBe('navigation'); + const navigationTraceContext = navigationEvent.contexts?.trace; - const navigationTraceId = navigationEvent.contexts?.trace?.trace_id; - expect(navigationTraceId).toMatch(/^[0-9a-f]{32}$/); + expect(navigationTraceContext).toMatchObject({ + op: 'navigation', + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + expect(navigationTraceContext).not.toHaveProperty('parent_span_id'); const errorEventPromise = getFirstSentryEnvelopeRequest(page); await page.locator('#errorBtn').click(); const errorEvent = await errorEventPromise; - const errorTraceId = errorEvent.contexts?.trace?.trace_id; - expect(errorTraceId).toBe(navigationTraceId); + const errorTraceContext = errorEvent.contexts?.trace; + expect(errorTraceContext).toEqual({ + trace_id: navigationTraceContext?.trace_id, + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); }); sentryTest('error during navigation has new navigation traceId', async ({ getLocalTestPath, page }) => { @@ -71,13 +87,20 @@ sentryTest('error during navigation has new navigation traceId', async ({ getLoc const navigationEvent = events.find(event => event.type === 'transaction'); const errorEvent = events.find(event => !event.type); - expect(navigationEvent?.contexts?.trace?.op).toBe('navigation'); - - const navigationTraceId = navigationEvent?.contexts?.trace?.trace_id; - expect(navigationTraceId).toMatch(/^[0-9a-f]{32}$/); - - const errorTraceId = errorEvent?.contexts?.trace?.trace_id; - expect(errorTraceId).toBe(navigationTraceId); + const navigationTraceContext = navigationEvent?.contexts?.trace; + expect(navigationTraceContext).toMatchObject({ + op: 'navigation', + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + expect(navigationTraceContext).not.toHaveProperty('parent_span_id'); + + const errorTraceContext = errorEvent?.contexts?.trace; + expect(errorTraceContext).toMatchObject({ + op: 'navigation', + trace_id: errorTraceContext?.trace_id, + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); }); sentryTest( @@ -93,10 +116,14 @@ sentryTest( await getFirstSentryEnvelopeRequest(page, url); const navigationEvent = await getFirstSentryEnvelopeRequest(page, `${url}#foo`); - expect(navigationEvent.contexts?.trace?.op).toBe('navigation'); - const navigationTraceId = navigationEvent.contexts?.trace?.trace_id; - expect(navigationTraceId).toMatch(/^[0-9a-f]{32}$/); + const navigationTraceContext = navigationEvent.contexts?.trace; + expect(navigationTraceContext).toMatchObject({ + op: 'navigation', + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + expect(navigationTraceContext).not.toHaveProperty('parent_span_id'); const requestPromise = page.waitForRequest('http://example.com/*'); await page.locator('#fetchBtn').click(); @@ -104,6 +131,7 @@ sentryTest( const headers = request.headers(); // sampling decision is deferred b/c of no active span at the time of request + const navigationTraceId = navigationTraceContext?.trace_id; expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}$`)); expect(headers['baggage']).toEqual( `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId}`, @@ -129,14 +157,90 @@ sentryTest( await page.locator('#fetchBtn').click(); const [navigationEvent, request] = await Promise.all([navigationEventPromise, requestPromise]); - expect(navigationEvent.contexts?.trace?.op).toBe('navigation'); + const navigationTraceContext = navigationEvent.contexts?.trace; + expect(navigationTraceContext).toMatchObject({ + op: 'navigation', + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + expect(navigationTraceContext).not.toHaveProperty('parent_span_id'); + + const headers = request.headers(); + + // sampling decision is propagated from active span sampling decision + const navigationTraceId = navigationTraceContext?.trace_id; + expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}-1$`)); + expect(headers['baggage']).toEqual( + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sample_rate=1,sentry-sampled=true`, + ); + }, +); + +sentryTest( + 'outgoing XHR request after navigation has navigation traceId in headers', + async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + // ensure navigation transaction is finished + await getFirstSentryEnvelopeRequest(page, url); + + const navigationEvent = await getFirstSentryEnvelopeRequest(page, `${url}#foo`); + + const navigationTraceContext = navigationEvent.contexts?.trace; + expect(navigationTraceContext).toMatchObject({ + op: 'navigation', + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + expect(navigationTraceContext).not.toHaveProperty('parent_span_id'); + + const xhrPromise = page.waitForRequest('http://example.com/*'); + await page.locator('#xhrBtn').click(); + const request = await xhrPromise; + const headers = request.headers(); + + // sampling decision is deferred b/c of no active span at the time of request + const navigationTraceId = navigationTraceContext?.trace_id; + expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}$`)); + expect(headers['baggage']).toEqual( + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId}`, + ); + }, +); + +sentryTest( + 'outgoing XHR request during navigation has navigation traceId in headers', + async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } - const navigationTraceId = navigationEvent.contexts?.trace?.trace_id; - expect(navigationTraceId).toMatch(/^[0-9a-f]{32}$/); + const url = await getLocalTestPath({ testDir: __dirname }); + + // ensure navigation transaction is finished + await getFirstSentryEnvelopeRequest(page, url); + + const navigationEventPromise = getFirstSentryEnvelopeRequest(page); + const requestPromise = page.waitForRequest('http://example.com/*'); + await page.goto(`${url}#foo`); + await page.locator('#xhrBtn').click(); + const [navigationEvent, request] = await Promise.all([navigationEventPromise, requestPromise]); + const navigationTraceContext = navigationEvent.contexts?.trace; + expect(navigationTraceContext).toMatchObject({ + op: 'navigation', + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + expect(navigationTraceContext).not.toHaveProperty('parent_span_id'); const headers = request.headers(); // sampling decision is propagated from active span sampling decision + const navigationTraceId = navigationTraceContext?.trace_id; expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}-1$`)); expect(headers['baggage']).toEqual( `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sample_rate=1,sentry-sampled=true`, diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/template.html b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/template.html index 61372c8605e5..dd44e58fbb4a 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/template.html +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/template.html @@ -9,5 +9,6 @@ + diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts index 2f872b398d87..d037bac433aa 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts @@ -157,3 +157,61 @@ sentryTest( expect(headers['baggage']).toBe(META_TAG_BAGGAGE); }, ); + +sentryTest( + 'outgoing XHR request after tag pageload has pageload traceId in headers', + async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + const pageloadEvent = await getFirstSentryEnvelopeRequest(page, url); + expect(pageloadEvent?.contexts?.trace).toMatchObject({ + op: 'pageload', + trace_id: META_TAG_TRACE_ID, + parent_span_id: META_TAG_PARENT_SPAN_ID, + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + + const requestPromise = page.waitForRequest('http://example.com/*'); + await page.locator('#xhrBtn').click(); + const request = await requestPromise; + const headers = request.headers(); + + // sampling decision is propagated from meta tag's sentry-trace sampled flag + expect(headers['sentry-trace']).toMatch(new RegExp(`^${META_TAG_TRACE_ID}-[0-9a-f]{16}-1$`)); + expect(headers['baggage']).toBe(META_TAG_BAGGAGE); + }, +); + +sentryTest( + 'outgoing XHR request during tag pageload has pageload traceId in headers', + async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + const pageloadEventPromise = getFirstSentryEnvelopeRequest(page); + const requestPromise = page.waitForRequest('http://example.com/*'); + await page.goto(url); + await page.locator('#xhrBtn').click(); + const [pageloadEvent, request] = await Promise.all([pageloadEventPromise, requestPromise]); + + expect(pageloadEvent?.contexts?.trace).toMatchObject({ + op: 'pageload', + trace_id: META_TAG_TRACE_ID, + parent_span_id: META_TAG_PARENT_SPAN_ID, + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + + const headers = request.headers(); + + // sampling decision is propagated from meta tag's sentry-trace sampled flag + expect(headers['sentry-trace']).toMatch(new RegExp(`^${META_TAG_TRACE_ID}-[0-9a-f]{16}-1$`)); + expect(headers['baggage']).toBe(META_TAG_BAGGAGE); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts index 69141a107bbc..333381d28b9a 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts @@ -17,17 +17,26 @@ sentryTest( const url = await getLocalTestPath({ testDir: __dirname }); const pageloadEvent = await getFirstSentryEnvelopeRequest(page, url); - const navigationEvent1 = await getFirstSentryEnvelopeRequest(page, `${url}#foo`); - - expect(pageloadEvent.contexts?.trace?.op).toBe('pageload'); - expect(navigationEvent1.contexts?.trace?.op).toBe('navigation'); - - const pageloadTraceId = pageloadEvent.contexts?.trace?.trace_id; - const navigation1TraceId = navigationEvent1.contexts?.trace?.trace_id; - - expect(pageloadTraceId).toMatch(/^[0-9a-f]{32}$/); - expect(navigation1TraceId).toMatch(/^[0-9a-f]{32}$/); - expect(pageloadTraceId).not.toEqual(navigation1TraceId); + const navigationEvent = await getFirstSentryEnvelopeRequest(page, `${url}#foo`); + + const pageloadTraceContext = pageloadEvent.contexts?.trace; + const navigationTraceContext = navigationEvent.contexts?.trace; + + expect(pageloadTraceContext).toMatchObject({ + op: 'pageload', + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + expect(pageloadTraceContext).not.toHaveProperty('parent_span_id'); + + expect(navigationTraceContext).toMatchObject({ + op: 'navigation', + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + expect(navigationTraceContext).not.toHaveProperty('parent_span_id'); + + expect(pageloadTraceContext?.span_id).not.toEqual(navigationTraceContext?.span_id); }, ); @@ -39,18 +48,25 @@ sentryTest('error after pageload has pageload traceId', async ({ getLocalTestPat const url = await getLocalTestPath({ testDir: __dirname }); const pageloadEvent = await getFirstSentryEnvelopeRequest(page, url); - expect(pageloadEvent.contexts?.trace?.op).toBe('pageload'); + const pageloadTraceContext = pageloadEvent.contexts?.trace; - const pageloadTraceId = pageloadEvent.contexts?.trace?.trace_id; - expect(pageloadTraceId).toMatch(/^[0-9a-f]{32}$/); + expect(pageloadTraceContext).toMatchObject({ + op: 'pageload', + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + expect(pageloadTraceContext).not.toHaveProperty('parent_span_id'); - const [, errorEvent] = await Promise.all([ - page.locator('#errorBtn').click(), - getFirstSentryEnvelopeRequest(page), - ]); + const errorEventPromise = getFirstSentryEnvelopeRequest(page); + await page.locator('#errorBtn').click(); + const errorEvent = await errorEventPromise; - const errorTraceId = errorEvent.contexts?.trace?.trace_id; - expect(errorTraceId).toBe(pageloadTraceId); + const errorTraceContext = errorEvent.contexts?.trace; + + expect(errorTraceContext).toEqual({ + trace_id: pageloadTraceContext?.trace_id, + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); }); sentryTest('error during pageload has pageload traceId', async ({ getLocalTestPath, page }) => { @@ -68,13 +84,20 @@ sentryTest('error during pageload has pageload traceId', async ({ getLocalTestPa const pageloadEvent = events.find(event => event.type === 'transaction'); const errorEvent = events.find(event => !event.type); - expect(pageloadEvent?.contexts?.trace?.op).toBe('pageload'); - - const pageloadTraceId = pageloadEvent?.contexts?.trace?.trace_id; - expect(pageloadTraceId).toMatch(/^[0-9a-f]{32}$/); - - const errorTraceId = errorEvent?.contexts?.trace?.trace_id; - expect(errorTraceId).toBe(pageloadTraceId); + const pageloadTraceContext = pageloadEvent?.contexts?.trace; + expect(pageloadTraceContext).toMatchObject({ + op: 'pageload', + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + expect(pageloadTraceContext).not.toHaveProperty('parent_span_id'); + + const errorTraceContext = errorEvent?.contexts?.trace; + expect(errorTraceContext).toMatchObject({ + op: 'pageload', + trace_id: pageloadTraceContext?.trace_id, + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); }); sentryTest( @@ -87,10 +110,14 @@ sentryTest( const url = await getLocalTestPath({ testDir: __dirname }); const pageloadEvent = await getFirstSentryEnvelopeRequest(page, url); - expect(pageloadEvent.contexts?.trace?.op).toBe('pageload'); + const pageloadTraceContext = pageloadEvent.contexts?.trace; - const pageloadTraceId = pageloadEvent.contexts?.trace?.trace_id; - expect(pageloadTraceId).toMatch(/^[0-9a-f]{32}$/); + expect(pageloadTraceContext).toMatchObject({ + op: 'pageload', + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + expect(pageloadTraceContext).not.toHaveProperty('parent_span_id'); const requestPromise = page.waitForRequest('http://example.com/*'); await page.locator('#fetchBtn').click(); @@ -98,6 +125,7 @@ sentryTest( const headers = request.headers(); // sampling decision is deferred b/c of no active span at the time of request + const pageloadTraceId = pageloadTraceContext?.trace_id; expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}$`)); expect(headers['baggage']).toEqual( `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId}`, @@ -120,17 +148,88 @@ sentryTest( await page.locator('#fetchBtn').click(); const [pageloadEvent, request] = await Promise.all([pageloadEventPromise, requestPromise]); - expect(pageloadEvent.contexts?.trace?.op).toBe('pageload'); + const pageloadTraceContext = pageloadEvent.contexts?.trace; + expect(pageloadTraceContext).toMatchObject({ + op: 'pageload', + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + expect(pageloadTraceContext).not.toHaveProperty('parent_span_id'); + + const headers = request.headers(); + + // sampling decision is propagated from active span sampling decision + const pageloadTraceId = pageloadTraceContext?.trace_id; + expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}-1$`)); + expect(headers['baggage']).toEqual( + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId},sentry-sample_rate=1,sentry-sampled=true`, + ); + }, +); + +sentryTest( + 'outgoing XHR request after pageload has pageload traceId in headers', + async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + const pageloadEvent = await getFirstSentryEnvelopeRequest(page, url); + const pageloadTraceContext = pageloadEvent.contexts?.trace; + + expect(pageloadTraceContext).toMatchObject({ + op: 'pageload', + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + expect(pageloadTraceContext).not.toHaveProperty('parent_span_id'); + + const requestPromise = page.waitForRequest('http://example.com/*'); + await page.locator('#xhrBtn').click(); + const request = await requestPromise; + const headers = request.headers(); + + // sampling decision is deferred b/c of no active span at the time of request + const pageloadTraceId = pageloadTraceContext?.trace_id; + expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}$`)); + expect(headers['baggage']).toEqual( + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId}`, + ); + }, +); + +sentryTest( + 'outgoing XHR request during pageload has pageload traceId in headers', + async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + const pageloadEventPromise = getFirstSentryEnvelopeRequest(page); + const requestPromise = page.waitForRequest('http://example.com/*'); + await page.goto(url); + await page.locator('#xhrBtn').click(); + const [pageloadEvent, request] = await Promise.all([pageloadEventPromise, requestPromise]); - const navigationTraceId = pageloadEvent.contexts?.trace?.trace_id; - expect(navigationTraceId).toMatch(/^[0-9a-f]{32}$/); + const pageloadTraceContext = pageloadEvent.contexts?.trace; + expect(pageloadTraceContext).toMatchObject({ + op: 'pageload', + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + expect(pageloadTraceContext).not.toHaveProperty('parent_span_id'); const headers = request.headers(); // sampling decision is propagated from active span sampling decision - expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}-1$`)); + const pageloadTraceId = pageloadTraceContext?.trace_id; + expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}-1$`)); expect(headers['baggage']).toEqual( - `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sample_rate=1,sentry-sampled=true`, + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId},sentry-sample_rate=1,sentry-sampled=true`, ); }, ); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/subject.js b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/subject.js index 38e475a5f216..71e03b3c8a7c 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/subject.js +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/subject.js @@ -7,3 +7,10 @@ const fetchBtn = document.getElementById('fetchBtn'); fetchBtn.addEventListener('click', async () => { await fetch('http://example.com'); }); + +const xhrBtn = document.getElementById('xhrBtn'); +xhrBtn.addEventListener('click', () => { + const xhr = new XMLHttpRequest(); + xhr.open('GET', 'http://example.com'); + xhr.send(); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/template.html b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/template.html index 218c42031ccf..a3c17f442605 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/template.html +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/template.html @@ -6,5 +6,6 @@ + From 0d2c96003ddfd74e06a6c3b6cdd66a20f94dc55a Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 16 Apr 2024 16:53:25 +0200 Subject: [PATCH 11/21] feat(nextjs): Skip OTEL root spans emitted by Next.js (#11623) We now skip any OTEL spans that Next.js emits, unless we already have a parent span that was created elsewhere (e.g. by our auto instrumentation). This way, we should be able to ensure consistent root spans for all cases. --- packages/opentelemetry/src/sampler.ts | 42 ++++++++++++++++++--------- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/packages/opentelemetry/src/sampler.ts b/packages/opentelemetry/src/sampler.ts index 024f802c5ae6..17490686b5bd 100644 --- a/packages/opentelemetry/src/sampler.ts +++ b/packages/opentelemetry/src/sampler.ts @@ -62,19 +62,14 @@ export class SentrySampler implements Sampler { return { decision: SamplingDecision.NOT_RECORD, traceState }; } - let parentSampled: boolean | undefined = undefined; - - // Only inherit sample rate if `traceId` is the same - // Note for testing: `isSpanContextValid()` checks the format of the traceId/spanId, so we need to pass valid ones - if (parentSpan && parentContext && isSpanContextValid(parentContext) && parentContext.traceId === traceId) { - if (parentContext.isRemote) { - parentSampled = getParentRemoteSampled(parentSpan); - DEBUG_BUILD && - logger.log(`[Tracing] Inheriting remote parent's sampled decision for ${spanName}: ${parentSampled}`); - } else { - parentSampled = getSamplingDecision(parentContext); - DEBUG_BUILD && logger.log(`[Tracing] Inheriting parent's sampled decision for ${spanName}: ${parentSampled}`); - } + const parentSampled = parentSpan ? getParentSampled(parentSpan, traceId, spanName) : undefined; + + // If we encounter a span emitted by Next.js, we do not want to sample it + // The reason for this is that the data quality of the spans varies, it is different per version of Next, + // and we need to keep our manual instrumentation around for the edge runtime anyhow. + // BUT we only do this if we don't have a parent span with a sampling decision yet + if (spanAttributes['next.span_type'] && typeof parentSampled !== 'boolean') { + return { decision: SamplingDecision.NOT_RECORD, traceState: traceState }; } const [sampled, sampleRate] = sampleSpan(options, { @@ -129,3 +124,24 @@ function getParentRemoteSampled(parentSpan: Span): boolean | undefined { // Only inherit sampled if `traceId` is the same return traceparentData && traceId === traceparentData.traceId ? traceparentData.sampled : undefined; } + +function getParentSampled(parentSpan: Span, traceId: string, spanName: string): boolean | undefined { + const parentContext = parentSpan.spanContext(); + + // Only inherit sample rate if `traceId` is the same + // Note for testing: `isSpanContextValid()` checks the format of the traceId/spanId, so we need to pass valid ones + if (isSpanContextValid(parentContext) && parentContext.traceId === traceId) { + if (parentContext.isRemote) { + const parentSampled = getParentRemoteSampled(parentSpan); + DEBUG_BUILD && + logger.log(`[Tracing] Inheriting remote parent's sampled decision for ${spanName}: ${parentSampled}`); + return parentSampled; + } + + const parentSampled = getSamplingDecision(parentContext); + DEBUG_BUILD && logger.log(`[Tracing] Inheriting parent's sampled decision for ${spanName}: ${parentSampled}`); + return parentSampled; + } + + return undefined; +} From 919f60b43249c11991e1f4625b3bd89566515b28 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 16 Apr 2024 17:19:16 +0200 Subject: [PATCH 12/21] doc: Add clarifying comment for hub on ACS (#11633) Added a comment for https://github.com/getsentry/sentry-javascript/pull/11630#discussion_r1567484029! --- packages/core/src/asyncContext/stackStrategy.ts | 6 +++++- packages/core/src/sdk.ts | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/core/src/asyncContext/stackStrategy.ts b/packages/core/src/asyncContext/stackStrategy.ts index 4ec7445d1b3e..05160d685e89 100644 --- a/packages/core/src/asyncContext/stackStrategy.ts +++ b/packages/core/src/asyncContext/stackStrategy.ts @@ -132,9 +132,13 @@ export class AsyncContextStack { */ function getAsyncContextStack(): AsyncContextStack { const registry = getMainCarrier(); + + // For now we continue to keep this as `hub` on the ACS, + // as e.g. the Loader Script relies on this. + // Eventually we may change this if/when we update the loader to not require this field anymore + // Related, we also write to `hub` in {@link ./../sdk.ts registerClientOnGlobalHub} const sentry = getSentryCarrier(registry) as { hub?: AsyncContextStack }; - // If there's no hub, or its an old API, assign a new one if (sentry.hub) { return sentry.hub; } diff --git a/packages/core/src/sdk.ts b/packages/core/src/sdk.ts index 85fdeae6e4d6..70bf19779c9b 100644 --- a/packages/core/src/sdk.ts +++ b/packages/core/src/sdk.ts @@ -48,11 +48,11 @@ export function setCurrentClient(client: Client): void { } /** - * Unfortunately, we still have to manually bind the client to the "hub" set on the global + * Unfortunately, we still have to manually bind the client to the "hub" property set on the global * Sentry carrier object. This is because certain scripts (e.g. our loader script) obtain * the client via `window.__SENTRY__.hub.getClient()`. * - * @see {@link hub.ts getGlobalHub} + * @see {@link ./asyncContext/stackStrategy.ts getAsyncContextStack} */ function registerClientOnGlobalHub(client: Client): void { const sentryGlobal = getSentryCarrier(getMainCarrier()) as { hub?: AsyncContextStack }; From 8b6f83861f5d3f3b494f15772422f58bc6735294 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 16 Apr 2024 18:08:29 +0200 Subject: [PATCH 13/21] feat(opentelemetry): Update OTEL packages & relax some version ranges (#11580) This bumps all our OTEL dependencies to require the most up to date versions as of today for all the core packages. This allows us to use the new semantic attributes, which esp. also some instrumentation uses in newer versions. By requiring this as a minimum version, we can ensure that we can update all our instrumentation. This also relaxes all of the core packages to `^` range, so users can easier use `@sentry/node` together with their own otel instrumentation. The instrumentation we add remains hard-pinned. To avoid deprecation warnings I updated all the semantic attributes usage to the new syntax. --- packages/node/package.json | 12 +- packages/node/src/integrations/tracing/koa.ts | 4 +- packages/node/src/sdk/initOtel.ts | 12 +- packages/opentelemetry/package.json | 19 ++- packages/opentelemetry/src/propagator.ts | 4 +- packages/opentelemetry/src/sampler.ts | 8 +- packages/opentelemetry/src/spanExporter.ts | 6 +- .../src/utils/getRequestSpanData.ts | 8 +- .../src/utils/isSentryRequest.ts | 4 +- packages/opentelemetry/src/utils/mapStatus.ts | 6 +- .../src/utils/parseSpanDescription.ts | 30 +++-- .../opentelemetry/test/helpers/initOtel.ts | 12 +- packages/opentelemetry/test/trace.test.ts | 49 +++---- .../test/utils/getRequestSpanData.test.ts | 12 +- .../test/utils/mapStatus.test.ts | 6 +- .../test/utils/parseSpanDescription.test.ts | 120 ++++++++++-------- yarn.lock | 77 ++++++----- 17 files changed, 201 insertions(+), 188 deletions(-) diff --git a/packages/node/package.json b/packages/node/package.json index ab37ba76cec9..be62b7f59db7 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -53,9 +53,9 @@ "access": "public" }, "dependencies": { - "@opentelemetry/api": "1.7.0", - "@opentelemetry/context-async-hooks": "1.21.0", - "@opentelemetry/core": "1.21.0", + "@opentelemetry/api": "^1.8.0", + "@opentelemetry/context-async-hooks": "^1.23.0", + "@opentelemetry/core": "^1.23.0", "@opentelemetry/instrumentation": "0.48.0", "@opentelemetry/instrumentation-express": "0.35.0", "@opentelemetry/instrumentation-fastify": "0.33.0", @@ -69,9 +69,9 @@ "@opentelemetry/instrumentation-mysql2": "0.35.0", "@opentelemetry/instrumentation-nestjs-core": "0.34.0", "@opentelemetry/instrumentation-pg": "0.38.0", - "@opentelemetry/resources": "1.21.0", - "@opentelemetry/sdk-trace-base": "1.21.0", - "@opentelemetry/semantic-conventions": "1.21.0", + "@opentelemetry/resources": "^1.23.0", + "@opentelemetry/sdk-trace-base": "^1.23.0", + "@opentelemetry/semantic-conventions": "^1.23.0", "@prisma/instrumentation": "5.9.0", "@sentry/core": "8.0.0-beta.1", "@sentry/opentelemetry": "8.0.0-beta.1", diff --git a/packages/node/src/integrations/tracing/koa.ts b/packages/node/src/integrations/tracing/koa.ts index 938e815e2b2f..7ebb9de48d15 100644 --- a/packages/node/src/integrations/tracing/koa.ts +++ b/packages/node/src/integrations/tracing/koa.ts @@ -1,6 +1,6 @@ import { registerInstrumentations } from '@opentelemetry/instrumentation'; import { KoaInstrumentation } from '@opentelemetry/instrumentation-koa'; -import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; +import { SEMATTRS_HTTP_ROUTE } from '@opentelemetry/semantic-conventions'; import { captureException, defineIntegration, @@ -26,7 +26,7 @@ const _koaIntegration = (() => { return; } const attributes = spanToJSON(span).data; - const route = attributes && attributes[SemanticAttributes.HTTP_ROUTE]; + const route = attributes && attributes[SEMATTRS_HTTP_ROUTE]; const method = info.context.request.method.toUpperCase() || 'GET'; if (route) { getIsolationScope().setTransactionName(`${method} ${route}`); diff --git a/packages/node/src/sdk/initOtel.ts b/packages/node/src/sdk/initOtel.ts index 62473969f7d4..f27635610c9c 100644 --- a/packages/node/src/sdk/initOtel.ts +++ b/packages/node/src/sdk/initOtel.ts @@ -1,7 +1,11 @@ import { DiagLogLevel, diag } from '@opentelemetry/api'; import { Resource } from '@opentelemetry/resources'; import { BasicTracerProvider } from '@opentelemetry/sdk-trace-base'; -import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions'; +import { + SEMRESATTRS_SERVICE_NAME, + SEMRESATTRS_SERVICE_NAMESPACE, + SEMRESATTRS_SERVICE_VERSION, +} from '@opentelemetry/semantic-conventions'; import { SDK_VERSION } from '@sentry/core'; import { SentryPropagator, SentrySampler, SentrySpanProcessor, setupEventContextTrace } from '@sentry/opentelemetry'; import { logger } from '@sentry/utils'; @@ -36,9 +40,9 @@ export function setupOtel(client: NodeClient): BasicTracerProvider { const provider = new BasicTracerProvider({ sampler: new SentrySampler(client), resource: new Resource({ - [SemanticResourceAttributes.SERVICE_NAME]: 'node', - [SemanticResourceAttributes.SERVICE_NAMESPACE]: 'sentry', - [SemanticResourceAttributes.SERVICE_VERSION]: SDK_VERSION, + [SEMRESATTRS_SERVICE_NAME]: 'node', + [SEMRESATTRS_SERVICE_NAMESPACE]: 'sentry', + [SEMRESATTRS_SERVICE_VERSION]: SDK_VERSION, }), forceFlushTimeoutMillis: 500, }); diff --git a/packages/opentelemetry/package.json b/packages/opentelemetry/package.json index 3c8e7a4ced31..1d0f01d2875f 100644 --- a/packages/opentelemetry/package.json +++ b/packages/opentelemetry/package.json @@ -47,18 +47,17 @@ "@sentry/utils": "8.0.0-beta.1" }, "peerDependencies": { - "@opentelemetry/api": "^1.0.0", - "@opentelemetry/core": "^1.0.0", - "@opentelemetry/sdk-trace-base": "^1.0.0", - "@opentelemetry/semantic-conventions": "^1.0.0" + "@opentelemetry/api": "^1.8.0", + "@opentelemetry/core": "^1.23.0", + "@opentelemetry/sdk-trace-base": "^1.23.0", + "@opentelemetry/semantic-conventions": "^1.23.0" }, "devDependencies": { - "@opentelemetry/api": "^1.6.0", - "@opentelemetry/context-async-hooks": "^1.17.1", - "@opentelemetry/core": "^1.17.1", - "@opentelemetry/sdk-trace-base": "^1.17.1", - "@opentelemetry/sdk-trace-node": "^1.17.1", - "@opentelemetry/semantic-conventions": "^1.17.1" + "@opentelemetry/api": "^1.8.0", + "@opentelemetry/context-async-hooks": "^1.23.0", + "@opentelemetry/core": "^1.23.0", + "@opentelemetry/sdk-trace-base": "^1.23.0", + "@opentelemetry/semantic-conventions": "^1.23.0" }, "scripts": { "build": "run-p build:transpile build:types", diff --git a/packages/opentelemetry/src/propagator.ts b/packages/opentelemetry/src/propagator.ts index 6640a0f4e0f1..45dcf811eaa8 100644 --- a/packages/opentelemetry/src/propagator.ts +++ b/packages/opentelemetry/src/propagator.ts @@ -2,7 +2,7 @@ import type { Baggage, Context, Span, SpanContext, TextMapGetter, TextMapSetter import { context } from '@opentelemetry/api'; import { TraceFlags, propagation, trace } from '@opentelemetry/api'; import { TraceState, W3CBaggagePropagator, isTracingSuppressed } from '@opentelemetry/core'; -import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; +import { SEMATTRS_HTTP_URL } from '@opentelemetry/semantic-conventions'; import type { continueTrace } from '@sentry/core'; import { hasTracingEnabled } from '@sentry/core'; import { getRootSpan } from '@sentry/core'; @@ -329,7 +329,7 @@ function getExistingBaggage(carrier: unknown): string | undefined { * 2. Else, if the active span has no URL attribute (e.g. it is unsampled), we check a special trace state (which we set in our sampler). */ function getCurrentURL(span: Span): string | undefined { - const urlAttribute = spanToJSON(span).data?.[SemanticAttributes.HTTP_URL]; + const urlAttribute = spanToJSON(span).data?.[SEMATTRS_HTTP_URL]; if (urlAttribute) { return urlAttribute; } diff --git a/packages/opentelemetry/src/sampler.ts b/packages/opentelemetry/src/sampler.ts index 17490686b5bd..10be97b0a4bd 100644 --- a/packages/opentelemetry/src/sampler.ts +++ b/packages/opentelemetry/src/sampler.ts @@ -9,7 +9,7 @@ import type { Client, SpanAttributes } from '@sentry/types'; import { logger } from '@sentry/utils'; import { SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING, SENTRY_TRACE_STATE_URL } from './constants'; -import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; +import { SEMATTRS_HTTP_METHOD, SEMATTRS_HTTP_URL } from '@opentelemetry/semantic-conventions'; import { DEBUG_BUILD } from './debug-build'; import { getPropagationContextFromSpan } from './propagator'; import { getSamplingDecision } from './utils/getSamplingDecision'; @@ -43,7 +43,7 @@ export class SentrySampler implements Sampler { let traceState = parentContext?.traceState || new TraceState(); // We always keep the URL on the trace state, so we can access it in the propagator - const url = spanAttributes[SemanticAttributes.HTTP_URL]; + const url = spanAttributes[SEMATTRS_HTTP_URL]; if (url && typeof url === 'string') { traceState = traceState.set(SENTRY_TRACE_STATE_URL, url); } @@ -56,7 +56,7 @@ export class SentrySampler implements Sampler { // but we want to leave downstream sampling decisions up to the server if ( spanKind === SpanKind.CLIENT && - spanAttributes[SemanticAttributes.HTTP_METHOD] && + spanAttributes[SEMATTRS_HTTP_METHOD] && (!parentSpan || parentContext?.isRemote) ) { return { decision: SamplingDecision.NOT_RECORD, traceState }; @@ -86,7 +86,7 @@ export class SentrySampler implements Sampler { [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate, }; - const method = `${spanAttributes[SemanticAttributes.HTTP_METHOD]}`.toUpperCase(); + const method = `${spanAttributes[SEMATTRS_HTTP_METHOD]}`.toUpperCase(); if (method === 'OPTIONS' || method === 'HEAD') { DEBUG_BUILD && logger.log(`[Tracing] Not sampling span because HTTP method is '${method}' for ${spanName}`); return { diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index 3ff88742db64..abf82ba8a3f0 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -1,7 +1,7 @@ import type { Span } from '@opentelemetry/api'; import { SpanKind } from '@opentelemetry/api'; import type { ReadableSpan } from '@opentelemetry/sdk-trace-base'; -import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; +import { SEMATTRS_HTTP_STATUS_CODE } from '@opentelemetry/semantic-conventions'; import { captureEvent, getCapturedScopesOnSpan, @@ -335,8 +335,8 @@ function getData(span: ReadableSpan): Record { 'otel.kind': SpanKind[span.kind], }; - if (attributes[SemanticAttributes.HTTP_STATUS_CODE]) { - const statusCode = attributes[SemanticAttributes.HTTP_STATUS_CODE] as string; + if (attributes[SEMATTRS_HTTP_STATUS_CODE]) { + const statusCode = attributes[SEMATTRS_HTTP_STATUS_CODE] as string; data['http.response.status_code'] = statusCode; } diff --git a/packages/opentelemetry/src/utils/getRequestSpanData.ts b/packages/opentelemetry/src/utils/getRequestSpanData.ts index 0154f8e4cd3e..8ce4419c925d 100644 --- a/packages/opentelemetry/src/utils/getRequestSpanData.ts +++ b/packages/opentelemetry/src/utils/getRequestSpanData.ts @@ -1,6 +1,6 @@ import type { Span } from '@opentelemetry/api'; import type { ReadableSpan } from '@opentelemetry/sdk-trace-base'; -import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; +import { SEMATTRS_HTTP_METHOD, SEMATTRS_HTTP_URL } from '@opentelemetry/semantic-conventions'; import type { SanitizedRequestData } from '@sentry/types'; import { getSanitizedUrlString, parseUrl } from '@sentry/utils'; @@ -16,8 +16,8 @@ export function getRequestSpanData(span: Span | ReadableSpan): Partial = { - url: span.attributes[SemanticAttributes.HTTP_URL] as string | undefined, - 'http.method': span.attributes[SemanticAttributes.HTTP_METHOD] as string | undefined, + url: span.attributes[SEMATTRS_HTTP_URL] as string | undefined, + 'http.method': span.attributes[SEMATTRS_HTTP_METHOD] as string | undefined, }; // Default to GET if URL is set but method is not @@ -26,7 +26,7 @@ export function getRequestSpanData(span: Span | ReadableSpan): Partial'; // if http.method exists, this is an http request span - const httpMethod = attributes[SemanticAttributes.HTTP_METHOD]; + const httpMethod = attributes[SEMATTRS_HTTP_METHOD]; if (httpMethod) { return descriptionForHttpMethod({ attributes, name, kind: getSpanKind(span) }, httpMethod); } // If db.type exists then this is a database call span. - const dbSystem = attributes[SemanticAttributes.DB_SYSTEM]; + const dbSystem = attributes[SEMATTRS_DB_SYSTEM]; if (dbSystem) { return descriptionForDbSystem({ attributes, name }); } // If rpc.service exists then this is a rpc call span. - const rpcService = attributes[SemanticAttributes.RPC_SERVICE]; + const rpcService = attributes[SEMATTRS_RPC_SERVICE]; if (rpcService) { return { op: 'rpc', @@ -47,7 +57,7 @@ export function parseSpanDescription(span: AbstractSpan): SpanDescription { } // If messaging.system exists then this is a messaging system span. - const messagingSystem = attributes[SemanticAttributes.MESSAGING_SYSTEM]; + const messagingSystem = attributes[SEMATTRS_MESSAGING_SYSTEM]; if (messagingSystem) { return { op: 'message', @@ -57,7 +67,7 @@ export function parseSpanDescription(span: AbstractSpan): SpanDescription { } // If faas.trigger exists then this is a function as a service span. - const faasTrigger = attributes[SemanticAttributes.FAAS_TRIGGER]; + const faasTrigger = attributes[SEMATTRS_FAAS_TRIGGER]; if (faasTrigger) { return { op: faasTrigger.toString(), description: name, source: 'route' }; } @@ -67,7 +77,7 @@ export function parseSpanDescription(span: AbstractSpan): SpanDescription { function descriptionForDbSystem({ attributes, name }: { attributes: Attributes; name: string }): SpanDescription { // Use DB statement (Ex "SELECT * FROM table") if possible as description. - const statement = attributes[SemanticAttributes.DB_STATEMENT]; + const statement = attributes[SEMATTRS_DB_STATEMENT]; const description = statement ? statement.toString() : name; @@ -134,11 +144,11 @@ export function getSanitizedUrl( hasRoute: boolean; } { // This is the relative path of the URL, e.g. /sub - const httpTarget = attributes[SemanticAttributes.HTTP_TARGET]; + const httpTarget = attributes[SEMATTRS_HTTP_TARGET]; // This is the full URL, including host & query params etc., e.g. https://example.com/sub?foo=bar - const httpUrl = attributes[SemanticAttributes.HTTP_URL]; + const httpUrl = attributes[SEMATTRS_HTTP_URL]; // This is the normalized route name - may not always be available! - const httpRoute = attributes[SemanticAttributes.HTTP_ROUTE]; + const httpRoute = attributes[SEMATTRS_HTTP_ROUTE]; const parsedUrl = typeof httpUrl === 'string' ? parseUrl(httpUrl) : undefined; const url = parsedUrl ? getSanitizedUrlString(parsedUrl) : undefined; diff --git a/packages/opentelemetry/test/helpers/initOtel.ts b/packages/opentelemetry/test/helpers/initOtel.ts index fb0ac135e015..3a680024811c 100644 --- a/packages/opentelemetry/test/helpers/initOtel.ts +++ b/packages/opentelemetry/test/helpers/initOtel.ts @@ -2,7 +2,11 @@ import { DiagLogLevel, diag } from '@opentelemetry/api'; import { AsyncLocalStorageContextManager } from '@opentelemetry/context-async-hooks'; import { Resource } from '@opentelemetry/resources'; import { BasicTracerProvider } from '@opentelemetry/sdk-trace-base'; -import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions'; +import { + SEMRESATTRS_SERVICE_NAME, + SEMRESATTRS_SERVICE_NAMESPACE, + SEMRESATTRS_SERVICE_VERSION, +} from '@opentelemetry/semantic-conventions'; import { SDK_VERSION, getClient } from '@sentry/core'; import { logger } from '@sentry/utils'; @@ -51,9 +55,9 @@ export function setupOtel(client: TestClientInterface): BasicTracerProvider { const provider = new BasicTracerProvider({ sampler: new SentrySampler(client), resource: new Resource({ - [SemanticResourceAttributes.SERVICE_NAME]: 'opentelemetry-test', - [SemanticResourceAttributes.SERVICE_NAMESPACE]: 'sentry', - [SemanticResourceAttributes.SERVICE_VERSION]: SDK_VERSION, + [SEMRESATTRS_SERVICE_NAME]: 'opentelemetry-test', + [SEMRESATTRS_SERVICE_NAMESPACE]: 'sentry', + [SEMRESATTRS_SERVICE_VERSION]: SDK_VERSION, }), forceFlushTimeoutMillis: 500, }); diff --git a/packages/opentelemetry/test/trace.test.ts b/packages/opentelemetry/test/trace.test.ts index 4ef8a8d90f6d..ab662b19db5b 100644 --- a/packages/opentelemetry/test/trace.test.ts +++ b/packages/opentelemetry/test/trace.test.ts @@ -21,7 +21,7 @@ import { import type { Event, Scope } from '@sentry/types'; import { makeTraceState } from '../src/propagator'; -import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; +import { SEMATTRS_HTTP_METHOD } from '@opentelemetry/semantic-conventions'; import { continueTrace, startInactiveSpan, startSpan, startSpanManual } from '../src/trace'; import type { AbstractSpan } from '../src/types'; import { getDynamicSamplingContextFromSpan } from '../src/utils/dynamicSamplingContext'; @@ -1370,35 +1370,26 @@ describe('HTTP methods (sampling)', () => { }); it('does sample when HTTP method is other than OPTIONS or HEAD', () => { - const spanGET = startSpanManual( - { name: 'test span', attributes: { [SemanticAttributes.HTTP_METHOD]: 'GET' } }, - span => { - return span; - }, - ); + const spanGET = startSpanManual({ name: 'test span', attributes: { [SEMATTRS_HTTP_METHOD]: 'GET' } }, span => { + return span; + }); expect(spanIsSampled(spanGET)).toBe(true); expect(getSamplingDecision(spanGET.spanContext())).toBe(true); - const spanPOST = startSpanManual( - { name: 'test span', attributes: { [SemanticAttributes.HTTP_METHOD]: 'POST' } }, - span => { - return span; - }, - ); + const spanPOST = startSpanManual({ name: 'test span', attributes: { [SEMATTRS_HTTP_METHOD]: 'POST' } }, span => { + return span; + }); expect(spanIsSampled(spanPOST)).toBe(true); expect(getSamplingDecision(spanPOST.spanContext())).toBe(true); - const spanPUT = startSpanManual( - { name: 'test span', attributes: { [SemanticAttributes.HTTP_METHOD]: 'PUT' } }, - span => { - return span; - }, - ); + const spanPUT = startSpanManual({ name: 'test span', attributes: { [SEMATTRS_HTTP_METHOD]: 'PUT' } }, span => { + return span; + }); expect(spanIsSampled(spanPUT)).toBe(true); expect(getSamplingDecision(spanPUT.spanContext())).toBe(true); const spanDELETE = startSpanManual( - { name: 'test span', attributes: { [SemanticAttributes.HTTP_METHOD]: 'DELETE' } }, + { name: 'test span', attributes: { [SEMATTRS_HTTP_METHOD]: 'DELETE' } }, span => { return span; }, @@ -1408,23 +1399,17 @@ describe('HTTP methods (sampling)', () => { }); it('does not sample when HTTP method is OPTIONS', () => { - const span = startSpanManual( - { name: 'test span', attributes: { [SemanticAttributes.HTTP_METHOD]: 'OPTIONS' } }, - span => { - return span; - }, - ); + const span = startSpanManual({ name: 'test span', attributes: { [SEMATTRS_HTTP_METHOD]: 'OPTIONS' } }, span => { + return span; + }); expect(spanIsSampled(span)).toBe(false); expect(getSamplingDecision(span.spanContext())).toBe(false); }); it('does not sample when HTTP method is HEAD', () => { - const span = startSpanManual( - { name: 'test span', attributes: { [SemanticAttributes.HTTP_METHOD]: 'HEAD' } }, - span => { - return span; - }, - ); + const span = startSpanManual({ name: 'test span', attributes: { [SEMATTRS_HTTP_METHOD]: 'HEAD' } }, span => { + return span; + }); expect(spanIsSampled(span)).toBe(false); expect(getSamplingDecision(span.spanContext())).toBe(false); }); diff --git a/packages/opentelemetry/test/utils/getRequestSpanData.test.ts b/packages/opentelemetry/test/utils/getRequestSpanData.test.ts index 0edd2befea6c..5b067189740e 100644 --- a/packages/opentelemetry/test/utils/getRequestSpanData.test.ts +++ b/packages/opentelemetry/test/utils/getRequestSpanData.test.ts @@ -1,4 +1,4 @@ -import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; +import { SEMATTRS_HTTP_METHOD, SEMATTRS_HTTP_URL } from '@opentelemetry/semantic-conventions'; import { getRequestSpanData } from '../../src/utils/getRequestSpanData'; import { createSpan } from '../helpers/createSpan'; @@ -14,8 +14,8 @@ describe('getRequestSpanData', () => { it('works with http span', () => { const span = createSpan(); span.setAttributes({ - [SemanticAttributes.HTTP_URL]: 'http://example.com?foo=bar#baz', - [SemanticAttributes.HTTP_METHOD]: 'GET', + [SEMATTRS_HTTP_URL]: 'http://example.com?foo=bar#baz', + [SEMATTRS_HTTP_METHOD]: 'GET', }); const data = getRequestSpanData(span); @@ -31,7 +31,7 @@ describe('getRequestSpanData', () => { it('works without method', () => { const span = createSpan(); span.setAttributes({ - [SemanticAttributes.HTTP_URL]: 'http://example.com', + [SEMATTRS_HTTP_URL]: 'http://example.com', }); const data = getRequestSpanData(span); @@ -45,8 +45,8 @@ describe('getRequestSpanData', () => { it('works with incorrect URL', () => { const span = createSpan(); span.setAttributes({ - [SemanticAttributes.HTTP_URL]: 'malformed-url-here', - [SemanticAttributes.HTTP_METHOD]: 'GET', + [SEMATTRS_HTTP_URL]: 'malformed-url-here', + [SEMATTRS_HTTP_METHOD]: 'GET', }); const data = getRequestSpanData(span); diff --git a/packages/opentelemetry/test/utils/mapStatus.test.ts b/packages/opentelemetry/test/utils/mapStatus.test.ts index 5b9e65e36d76..8dcf55a9267b 100644 --- a/packages/opentelemetry/test/utils/mapStatus.test.ts +++ b/packages/opentelemetry/test/utils/mapStatus.test.ts @@ -1,4 +1,4 @@ -import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; +import { SEMATTRS_HTTP_STATUS_CODE, SEMATTRS_RPC_GRPC_STATUS_CODE } from '@opentelemetry/semantic-conventions'; import { SPAN_STATUS_ERROR, SPAN_STATUS_OK } from '@sentry/core'; import type { SpanStatus } from '@sentry/types'; @@ -62,11 +62,11 @@ describe('mapStatus', () => { span.setStatus({ code: 0 }); // UNSET if (httpCode) { - span.setAttribute(SemanticAttributes.HTTP_STATUS_CODE, httpCode); + span.setAttribute(SEMATTRS_HTTP_STATUS_CODE, httpCode); } if (grpcCode) { - span.setAttribute(SemanticAttributes.RPC_GRPC_STATUS_CODE, grpcCode); + span.setAttribute(SEMATTRS_RPC_GRPC_STATUS_CODE, grpcCode); } const actual = mapStatus(span); diff --git a/packages/opentelemetry/test/utils/parseSpanDescription.test.ts b/packages/opentelemetry/test/utils/parseSpanDescription.test.ts index aa78526f8ffe..3e25356d1025 100644 --- a/packages/opentelemetry/test/utils/parseSpanDescription.test.ts +++ b/packages/opentelemetry/test/utils/parseSpanDescription.test.ts @@ -1,6 +1,18 @@ import type { Span } from '@opentelemetry/api'; import { SpanKind } from '@opentelemetry/api'; -import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; +import { + SEMATTRS_DB_STATEMENT, + SEMATTRS_DB_SYSTEM, + SEMATTRS_FAAS_TRIGGER, + SEMATTRS_HTTP_HOST, + SEMATTRS_HTTP_METHOD, + SEMATTRS_HTTP_ROUTE, + SEMATTRS_HTTP_STATUS_CODE, + SEMATTRS_HTTP_TARGET, + SEMATTRS_HTTP_URL, + SEMATTRS_MESSAGING_SYSTEM, + SEMATTRS_RPC_SERVICE, +} from '@opentelemetry/semantic-conventions'; import { descriptionForHttpMethod, getSanitizedUrl, parseSpanDescription } from '../../src/utils/parseSpanDescription'; @@ -31,7 +43,7 @@ describe('parseSpanDescription', () => { [ 'works with http method', { - [SemanticAttributes.HTTP_METHOD]: 'GET', + [SEMATTRS_HTTP_METHOD]: 'GET', }, 'test name', SpanKind.CLIENT, @@ -44,8 +56,8 @@ describe('parseSpanDescription', () => { [ 'works with db system', { - [SemanticAttributes.DB_SYSTEM]: 'mysql', - [SemanticAttributes.DB_STATEMENT]: 'SELECT * from users', + [SEMATTRS_DB_SYSTEM]: 'mysql', + [SEMATTRS_DB_STATEMENT]: 'SELECT * from users', }, 'test name', SpanKind.CLIENT, @@ -58,7 +70,7 @@ describe('parseSpanDescription', () => { [ 'works with db system without statement', { - [SemanticAttributes.DB_SYSTEM]: 'mysql', + [SEMATTRS_DB_SYSTEM]: 'mysql', }, 'test name', SpanKind.CLIENT, @@ -71,7 +83,7 @@ describe('parseSpanDescription', () => { [ 'works with rpc service', { - [SemanticAttributes.RPC_SERVICE]: 'rpc-test-service', + [SEMATTRS_RPC_SERVICE]: 'rpc-test-service', }, 'test name', undefined, @@ -84,7 +96,7 @@ describe('parseSpanDescription', () => { [ 'works with messaging system', { - [SemanticAttributes.MESSAGING_SYSTEM]: 'test-messaging-system', + [SEMATTRS_MESSAGING_SYSTEM]: 'test-messaging-system', }, 'test name', undefined, @@ -97,7 +109,7 @@ describe('parseSpanDescription', () => { [ 'works with faas trigger', { - [SemanticAttributes.FAAS_TRIGGER]: 'test-faas-trigger', + [SEMATTRS_FAAS_TRIGGER]: 'test-faas-trigger', }, 'test name', undefined, @@ -131,9 +143,9 @@ describe('descriptionForHttpMethod', () => { 'works with basic client GET', 'GET', { - [SemanticAttributes.HTTP_METHOD]: 'GET', - [SemanticAttributes.HTTP_URL]: 'https://www.example.com/my-path', - [SemanticAttributes.HTTP_TARGET]: '/my-path', + [SEMATTRS_HTTP_METHOD]: 'GET', + [SEMATTRS_HTTP_URL]: 'https://www.example.com/my-path', + [SEMATTRS_HTTP_TARGET]: '/my-path', }, 'test name', SpanKind.CLIENT, @@ -150,9 +162,9 @@ describe('descriptionForHttpMethod', () => { 'works with basic server POST', 'POST', { - [SemanticAttributes.HTTP_METHOD]: 'POST', - [SemanticAttributes.HTTP_URL]: 'https://www.example.com/my-path', - [SemanticAttributes.HTTP_TARGET]: '/my-path', + [SEMATTRS_HTTP_METHOD]: 'POST', + [SEMATTRS_HTTP_URL]: 'https://www.example.com/my-path', + [SEMATTRS_HTTP_TARGET]: '/my-path', }, 'test name', SpanKind.SERVER, @@ -169,10 +181,10 @@ describe('descriptionForHttpMethod', () => { 'works with client GET with route', 'GET', { - [SemanticAttributes.HTTP_METHOD]: 'GET', - [SemanticAttributes.HTTP_URL]: 'https://www.example.com/my-path/123', - [SemanticAttributes.HTTP_TARGET]: '/my-path/123', - [SemanticAttributes.HTTP_ROUTE]: '/my-path/:id', + [SEMATTRS_HTTP_METHOD]: 'GET', + [SEMATTRS_HTTP_URL]: 'https://www.example.com/my-path/123', + [SEMATTRS_HTTP_TARGET]: '/my-path/123', + [SEMATTRS_HTTP_ROUTE]: '/my-path/:id', }, 'test name', SpanKind.CLIENT, @@ -208,11 +220,11 @@ describe('getSanitizedUrl', () => { [ 'uses url without query for client request', { - [SemanticAttributes.HTTP_URL]: 'http://example.com/?what=true', - [SemanticAttributes.HTTP_METHOD]: 'GET', - [SemanticAttributes.HTTP_TARGET]: '/?what=true', - [SemanticAttributes.HTTP_HOST]: 'example.com:80', - [SemanticAttributes.HTTP_STATUS_CODE]: 200, + [SEMATTRS_HTTP_URL]: 'http://example.com/?what=true', + [SEMATTRS_HTTP_METHOD]: 'GET', + [SEMATTRS_HTTP_TARGET]: '/?what=true', + [SEMATTRS_HTTP_HOST]: 'example.com:80', + [SEMATTRS_HTTP_STATUS_CODE]: 200, }, SpanKind.CLIENT, { @@ -226,11 +238,11 @@ describe('getSanitizedUrl', () => { [ 'uses url without hash for client request', { - [SemanticAttributes.HTTP_URL]: 'http://example.com/sub#hash', - [SemanticAttributes.HTTP_METHOD]: 'GET', - [SemanticAttributes.HTTP_TARGET]: '/sub#hash', - [SemanticAttributes.HTTP_HOST]: 'example.com:80', - [SemanticAttributes.HTTP_STATUS_CODE]: 200, + [SEMATTRS_HTTP_URL]: 'http://example.com/sub#hash', + [SEMATTRS_HTTP_METHOD]: 'GET', + [SEMATTRS_HTTP_TARGET]: '/sub#hash', + [SEMATTRS_HTTP_HOST]: 'example.com:80', + [SEMATTRS_HTTP_STATUS_CODE]: 200, }, SpanKind.CLIENT, { @@ -244,12 +256,12 @@ describe('getSanitizedUrl', () => { [ 'uses route if available for client request', { - [SemanticAttributes.HTTP_URL]: 'http://example.com/?what=true', - [SemanticAttributes.HTTP_METHOD]: 'GET', - [SemanticAttributes.HTTP_TARGET]: '/?what=true', - [SemanticAttributes.HTTP_ROUTE]: '/my-route', - [SemanticAttributes.HTTP_HOST]: 'example.com:80', - [SemanticAttributes.HTTP_STATUS_CODE]: 200, + [SEMATTRS_HTTP_URL]: 'http://example.com/?what=true', + [SEMATTRS_HTTP_METHOD]: 'GET', + [SEMATTRS_HTTP_TARGET]: '/?what=true', + [SEMATTRS_HTTP_ROUTE]: '/my-route', + [SEMATTRS_HTTP_HOST]: 'example.com:80', + [SEMATTRS_HTTP_STATUS_CODE]: 200, }, SpanKind.CLIENT, { @@ -263,10 +275,10 @@ describe('getSanitizedUrl', () => { [ 'falls back to target for client request if url not available', { - [SemanticAttributes.HTTP_METHOD]: 'GET', - [SemanticAttributes.HTTP_TARGET]: '/?what=true', - [SemanticAttributes.HTTP_HOST]: 'example.com:80', - [SemanticAttributes.HTTP_STATUS_CODE]: 200, + [SEMATTRS_HTTP_METHOD]: 'GET', + [SEMATTRS_HTTP_TARGET]: '/?what=true', + [SEMATTRS_HTTP_HOST]: 'example.com:80', + [SEMATTRS_HTTP_STATUS_CODE]: 200, }, SpanKind.CLIENT, { @@ -280,11 +292,11 @@ describe('getSanitizedUrl', () => { [ 'uses target without query for server request', { - [SemanticAttributes.HTTP_URL]: 'http://example.com/?what=true', - [SemanticAttributes.HTTP_METHOD]: 'GET', - [SemanticAttributes.HTTP_TARGET]: '/?what=true', - [SemanticAttributes.HTTP_HOST]: 'example.com:80', - [SemanticAttributes.HTTP_STATUS_CODE]: 200, + [SEMATTRS_HTTP_URL]: 'http://example.com/?what=true', + [SEMATTRS_HTTP_METHOD]: 'GET', + [SEMATTRS_HTTP_TARGET]: '/?what=true', + [SEMATTRS_HTTP_HOST]: 'example.com:80', + [SEMATTRS_HTTP_STATUS_CODE]: 200, }, SpanKind.SERVER, { @@ -298,11 +310,11 @@ describe('getSanitizedUrl', () => { [ 'uses target without hash for server request', { - [SemanticAttributes.HTTP_URL]: 'http://example.com/?what=true', - [SemanticAttributes.HTTP_METHOD]: 'GET', - [SemanticAttributes.HTTP_TARGET]: '/sub#hash', - [SemanticAttributes.HTTP_HOST]: 'example.com:80', - [SemanticAttributes.HTTP_STATUS_CODE]: 200, + [SEMATTRS_HTTP_URL]: 'http://example.com/?what=true', + [SEMATTRS_HTTP_METHOD]: 'GET', + [SEMATTRS_HTTP_TARGET]: '/sub#hash', + [SEMATTRS_HTTP_HOST]: 'example.com:80', + [SEMATTRS_HTTP_STATUS_CODE]: 200, }, SpanKind.SERVER, { @@ -316,12 +328,12 @@ describe('getSanitizedUrl', () => { [ 'uses route for server request if available', { - [SemanticAttributes.HTTP_URL]: 'http://example.com/?what=true', - [SemanticAttributes.HTTP_METHOD]: 'GET', - [SemanticAttributes.HTTP_TARGET]: '/?what=true', - [SemanticAttributes.HTTP_ROUTE]: '/my-route', - [SemanticAttributes.HTTP_HOST]: 'example.com:80', - [SemanticAttributes.HTTP_STATUS_CODE]: 200, + [SEMATTRS_HTTP_URL]: 'http://example.com/?what=true', + [SEMATTRS_HTTP_METHOD]: 'GET', + [SEMATTRS_HTTP_TARGET]: '/?what=true', + [SEMATTRS_HTTP_ROUTE]: '/my-route', + [SEMATTRS_HTTP_HOST]: 'example.com:80', + [SEMATTRS_HTTP_STATUS_CODE]: 200, }, SpanKind.SERVER, { diff --git a/yarn.lock b/yarn.lock index b6c340ac1d3f..07f83fa7b980 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4888,10 +4888,15 @@ dependencies: "@opentelemetry/context-base" "^0.12.0" -"@opentelemetry/context-async-hooks@1.21.0", "@opentelemetry/context-async-hooks@^1.17.1": - version "1.21.0" - resolved "https://registry.yarnpkg.com/@opentelemetry/context-async-hooks/-/context-async-hooks-1.21.0.tgz#a56fa461e7786605bcbde2ff66f21b2392afacda" - integrity sha512-t0iulGPiMjG/NrSjinPQoIf8ST/o9V0dGOJthfrFporJlNdlKIQPfC7lkrV+5s2dyBThfmSbJlp/4hO1eOcDXA== +"@opentelemetry/api@^1.8.0": + version "1.8.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/api/-/api-1.8.0.tgz#5aa7abb48f23f693068ed2999ae627d2f7d902ec" + integrity sha512-I/s6F7yKUDdtMsoBWXJe8Qz40Tui5vsuKCWJEWVL+5q9sSWRzzx6v2KeNsOBEwd94j0eWkpWCH4yB6rZg9Mf0w== + +"@opentelemetry/context-async-hooks@^1.23.0": + version "1.23.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/context-async-hooks/-/context-async-hooks-1.23.0.tgz#4c4627fe2857324459b0a78b5a83cbc64a415d14" + integrity sha512-wazGJZDRevibOJ+VgyrT+9+8sybZAxpZx2G7vy30OAtk92OpZCg7HgNxT11NUx0VBDWcRx1dOatMYGOVplQ7QA== "@opentelemetry/context-base@^0.12.0": version "0.12.0" @@ -4905,13 +4910,20 @@ dependencies: "@opentelemetry/semantic-conventions" "1.20.0" -"@opentelemetry/core@1.21.0", "@opentelemetry/core@^1.1.0", "@opentelemetry/core@^1.17.1", "@opentelemetry/core@^1.8.0": +"@opentelemetry/core@1.21.0", "@opentelemetry/core@^1.1.0", "@opentelemetry/core@^1.8.0": version "1.21.0" resolved "https://registry.yarnpkg.com/@opentelemetry/core/-/core-1.21.0.tgz#8c16faf16edf861b073c03c9d45977b3f4003ee1" integrity sha512-KP+OIweb3wYoP7qTYL/j5IpOlu52uxBv5M4+QhSmmUfLyTgu1OIS71msK3chFo1D6Y61BIH3wMiMYRCxJCQctA== dependencies: "@opentelemetry/semantic-conventions" "1.21.0" +"@opentelemetry/core@1.23.0", "@opentelemetry/core@^1.23.0": + version "1.23.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/core/-/core-1.23.0.tgz#f2e7ada7f35750f3c1674aef1e52c879005c0731" + integrity sha512-hdQ/a9TMzMQF/BO8Cz1juA43/L5YGtCSiKoOHmrTEf7VMDAZgy8ucpWx3eQTnQ3gBloRcWtzvcrMZABC3PTSKQ== + dependencies: + "@opentelemetry/semantic-conventions" "1.23.0" + "@opentelemetry/core@^0.12.0": version "0.12.0" resolved "https://registry.yarnpkg.com/@opentelemetry/core/-/core-0.12.0.tgz#a888badc9a408fa1f13976a574e69d14be32488e" @@ -5065,20 +5077,6 @@ semver "^7.5.2" shimmer "^1.2.1" -"@opentelemetry/propagator-b3@1.21.0": - version "1.21.0" - resolved "https://registry.yarnpkg.com/@opentelemetry/propagator-b3/-/propagator-b3-1.21.0.tgz#72fadc4a07afb2c83f0830b8a06071e0361eacb2" - integrity sha512-3ZTobj2VDIOzLsIvvYCdpw6tunxUVElPxDvog9lS49YX4hohHeD84A8u9Ns/6UYUcaN5GSoEf891lzhcBFiOLA== - dependencies: - "@opentelemetry/core" "1.21.0" - -"@opentelemetry/propagator-jaeger@1.21.0": - version "1.21.0" - resolved "https://registry.yarnpkg.com/@opentelemetry/propagator-jaeger/-/propagator-jaeger-1.21.0.tgz#bfc1fa3a050496ec67a253040dfdec4d16339225" - integrity sha512-8TQSwXjBmaDx7JkxRD7hdmBmRK2RGRgzHX1ArJfJhIc5trzlVweyorzqQrXOvqVEdEg+zxUMHkL5qbGH/HDTPA== - dependencies: - "@opentelemetry/core" "1.21.0" - "@opentelemetry/resources@1.20.0": version "1.20.0" resolved "https://registry.yarnpkg.com/@opentelemetry/resources/-/resources-1.20.0.tgz#7165c39837e6e41b695f0088e40d15a5793f1469" @@ -5095,6 +5093,14 @@ "@opentelemetry/core" "1.21.0" "@opentelemetry/semantic-conventions" "1.21.0" +"@opentelemetry/resources@1.23.0", "@opentelemetry/resources@^1.23.0": + version "1.23.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/resources/-/resources-1.23.0.tgz#4c71430f3e20c4d88b67ef5629759fae108485e5" + integrity sha512-iPRLfVfcEQynYGo7e4Di+ti+YQTAY0h5mQEUJcHlU9JOqpb4x965O6PZ+wMcwYVY63G96KtdS86YCM1BF1vQZg== + dependencies: + "@opentelemetry/core" "1.23.0" + "@opentelemetry/semantic-conventions" "1.23.0" + "@opentelemetry/resources@^0.12.0": version "0.12.0" resolved "https://registry.yarnpkg.com/@opentelemetry/resources/-/resources-0.12.0.tgz#5eb287c3032a2bebb2bb9f69b44bd160d2a7d591" @@ -5121,37 +5127,30 @@ "@opentelemetry/resources" "1.20.0" "@opentelemetry/semantic-conventions" "1.20.0" -"@opentelemetry/sdk-trace-base@1.21.0", "@opentelemetry/sdk-trace-base@^1.17.1": - version "1.21.0" - resolved "https://registry.yarnpkg.com/@opentelemetry/sdk-trace-base/-/sdk-trace-base-1.21.0.tgz#ffad912e453a92044fb220bd5d2f6743bf37bb8a" - integrity sha512-yrElGX5Fv0umzp8Nxpta/XqU71+jCAyaLk34GmBzNcrW43nqbrqvdPs4gj4MVy/HcTjr6hifCDCYA3rMkajxxA== - dependencies: - "@opentelemetry/core" "1.21.0" - "@opentelemetry/resources" "1.21.0" - "@opentelemetry/semantic-conventions" "1.21.0" - -"@opentelemetry/sdk-trace-node@^1.17.1": - version "1.21.0" - resolved "https://registry.yarnpkg.com/@opentelemetry/sdk-trace-node/-/sdk-trace-node-1.21.0.tgz#20599f42a6b59bf71c64ef8630d28464e6e18f2a" - integrity sha512-1pdm8jnqs+LuJ0Bvx6sNL28EhC8Rv7NYV8rnoXq3GIQo7uOHBDAFSj7makAfbakrla7ecO1FRfI8emnR4WvhYA== +"@opentelemetry/sdk-trace-base@^1.23.0": + version "1.23.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/sdk-trace-base/-/sdk-trace-base-1.23.0.tgz#ff0a0f8ec47205e0b14b3b765ea2a34de1ad01dd" + integrity sha512-PzBmZM8hBomUqvCddF/5Olyyviayka44O5nDWq673np3ctnvwMOvNrsUORZjKja1zJbwEuD9niAGbnVrz3jwRQ== dependencies: - "@opentelemetry/context-async-hooks" "1.21.0" - "@opentelemetry/core" "1.21.0" - "@opentelemetry/propagator-b3" "1.21.0" - "@opentelemetry/propagator-jaeger" "1.21.0" - "@opentelemetry/sdk-trace-base" "1.21.0" - semver "^7.5.2" + "@opentelemetry/core" "1.23.0" + "@opentelemetry/resources" "1.23.0" + "@opentelemetry/semantic-conventions" "1.23.0" "@opentelemetry/semantic-conventions@1.20.0": version "1.20.0" resolved "https://registry.yarnpkg.com/@opentelemetry/semantic-conventions/-/semantic-conventions-1.20.0.tgz#4d9b88188e18056a218644ea30fae130a7857766" integrity sha512-3zLJJCgTKYpbqFX8drl8hOCHtdchELC+kGqlVcV4mHW1DiElTtv1Nt9EKBptTd1IfL56QkuYnWJ3DeHd2Gtu/A== -"@opentelemetry/semantic-conventions@1.21.0", "@opentelemetry/semantic-conventions@^1.0.0", "@opentelemetry/semantic-conventions@^1.17.0", "@opentelemetry/semantic-conventions@^1.17.1": +"@opentelemetry/semantic-conventions@1.21.0", "@opentelemetry/semantic-conventions@^1.0.0", "@opentelemetry/semantic-conventions@^1.17.0": version "1.21.0" resolved "https://registry.yarnpkg.com/@opentelemetry/semantic-conventions/-/semantic-conventions-1.21.0.tgz#83f7479c524ab523ac2df702ade30b9724476c72" integrity sha512-lkC8kZYntxVKr7b8xmjCVUgE0a8xgDakPyDo9uSWavXPyYqLgYYGdEd2j8NxihRyb6UwpX3G/hFUF4/9q2V+/g== +"@opentelemetry/semantic-conventions@1.23.0", "@opentelemetry/semantic-conventions@^1.23.0": + version "1.23.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/semantic-conventions/-/semantic-conventions-1.23.0.tgz#627f2721b960fe586b7f72a07912cb7699f06eef" + integrity sha512-MiqFvfOzfR31t8cc74CTP1OZfz7MbqpAnLCra8NqQoaHJX6ncIRTdYOQYBDQ2uFISDq0WY8Y9dDTWvsgzzBYRg== + "@opentelemetry/semantic-conventions@^0.12.0": version "0.12.0" resolved "https://registry.yarnpkg.com/@opentelemetry/semantic-conventions/-/semantic-conventions-0.12.0.tgz#7e392aecdbdbd5d737d3995998b120dc17589ab0" From 38758b96c9ff2c889dcf0f7ffc0769da66d5002d Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Tue, 16 Apr 2024 12:09:28 -0400 Subject: [PATCH 14/21] fix(feedback): Fix timeout on feedback submission (#11619) The timeout id wasn't getting reset to null after submission which was causing additional clicks on the feedback button to pop up the success message instead. Now, a click on the success message brings back the feedback button, which brings up the dialog when clicked. Fixes [#418](https://github.com/getsentry/team-replay/issues/418) --- packages/feedback/src/modal/components/DialogContainer.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/feedback/src/modal/components/DialogContainer.tsx b/packages/feedback/src/modal/components/DialogContainer.tsx index 97d1b7f84e96..3f955dc524ab 100644 --- a/packages/feedback/src/modal/components/DialogContainer.tsx +++ b/packages/feedback/src/modal/components/DialogContainer.tsx @@ -33,7 +33,12 @@ export function DialogComponent({ open, onFormSubmitted, successMessageText, ... const onSubmitSuccess = useCallback( (data: FeedbackFormData) => { props.onSubmitSuccess(data); - setTimeoutId(() => setTimeout(onFormSubmitted, SUCCESS_MESSAGE_TIMEOUT)); + setTimeoutId( + setTimeout(() => { + onFormSubmitted(); + setTimeoutId(null); + }, SUCCESS_MESSAGE_TIMEOUT), + ); }, [onFormSubmitted], ); From 08e26ab31b25bce016838c6250e64ac663baab26 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 16 Apr 2024 16:28:24 +0000 Subject: [PATCH 15/21] feat(deps): bump @opentelemetry/instrumentation-koa from 0.37.0 to 0.39.0 (#11495) Bumps [@opentelemetry/instrumentation-koa](https://github.com/open-telemetry/opentelemetry-js-contrib) from 0.37.0 to 0.39.0.
Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=@opentelemetry/instrumentation-koa&package-manager=npm_and_yarn&previous-version=0.37.0&new-version=0.39.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- packages/node/package.json | 2 +- yarn.lock | 41 ++++++++++++++++++++++++++++---------- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/packages/node/package.json b/packages/node/package.json index be62b7f59db7..0013de64e3d1 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -62,7 +62,7 @@ "@opentelemetry/instrumentation-graphql": "0.37.0", "@opentelemetry/instrumentation-hapi": "0.34.0", "@opentelemetry/instrumentation-http": "0.48.0", - "@opentelemetry/instrumentation-koa": "0.37.0", + "@opentelemetry/instrumentation-koa": "0.39.0", "@opentelemetry/instrumentation-mongodb": "0.39.0", "@opentelemetry/instrumentation-mongoose": "0.35.0", "@opentelemetry/instrumentation-mysql": "0.35.0", diff --git a/yarn.lock b/yarn.lock index 07f83fa7b980..26af712238f6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4876,6 +4876,13 @@ dependencies: "@octokit/openapi-types" "^18.0.0" +"@opentelemetry/api-logs@0.50.0": + version "0.50.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/api-logs/-/api-logs-0.50.0.tgz#d46b76daab0bc18fa92dcdabacfc106c380d19a1" + integrity sha512-JdZuKrhOYggqOpUljAq4WWNi5nB10PmgoF0y2CvedLGXd0kSawb/UBnWT8gg1ND3bHCNHStAIVT0ELlxJJRqrA== + dependencies: + "@opentelemetry/api" "^1.0.0" + "@opentelemetry/api@1.7.0", "@opentelemetry/api@^1.6.0": version "1.7.0" resolved "https://registry.yarnpkg.com/@opentelemetry/api/-/api-1.7.0.tgz#b139c81999c23e3c8d3c0a7234480e945920fc40" @@ -4888,7 +4895,7 @@ dependencies: "@opentelemetry/context-base" "^0.12.0" -"@opentelemetry/api@^1.8.0": +"@opentelemetry/api@^1.0.0", "@opentelemetry/api@^1.8.0": version "1.8.0" resolved "https://registry.yarnpkg.com/@opentelemetry/api/-/api-1.8.0.tgz#5aa7abb48f23f693068ed2999ae627d2f7d902ec" integrity sha512-I/s6F7yKUDdtMsoBWXJe8Qz40Tui5vsuKCWJEWVL+5q9sSWRzzx6v2KeNsOBEwd94j0eWkpWCH4yB6rZg9Mf0w== @@ -4910,14 +4917,14 @@ dependencies: "@opentelemetry/semantic-conventions" "1.20.0" -"@opentelemetry/core@1.21.0", "@opentelemetry/core@^1.1.0", "@opentelemetry/core@^1.8.0": +"@opentelemetry/core@1.21.0": version "1.21.0" resolved "https://registry.yarnpkg.com/@opentelemetry/core/-/core-1.21.0.tgz#8c16faf16edf861b073c03c9d45977b3f4003ee1" integrity sha512-KP+OIweb3wYoP7qTYL/j5IpOlu52uxBv5M4+QhSmmUfLyTgu1OIS71msK3chFo1D6Y61BIH3wMiMYRCxJCQctA== dependencies: "@opentelemetry/semantic-conventions" "1.21.0" -"@opentelemetry/core@1.23.0", "@opentelemetry/core@^1.23.0": +"@opentelemetry/core@1.23.0", "@opentelemetry/core@^1.1.0", "@opentelemetry/core@^1.23.0", "@opentelemetry/core@^1.8.0": version "1.23.0" resolved "https://registry.yarnpkg.com/@opentelemetry/core/-/core-1.23.0.tgz#f2e7ada7f35750f3c1674aef1e52c879005c0731" integrity sha512-hdQ/a9TMzMQF/BO8Cz1juA43/L5YGtCSiKoOHmrTEf7VMDAZgy8ucpWx3eQTnQ3gBloRcWtzvcrMZABC3PTSKQ== @@ -4978,14 +4985,14 @@ "@opentelemetry/semantic-conventions" "1.21.0" semver "^7.5.2" -"@opentelemetry/instrumentation-koa@0.37.0": - version "0.37.0" - resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-koa/-/instrumentation-koa-0.37.0.tgz#f12e608afb7b58cee0f27abb3c2a166ea8596c68" - integrity sha512-EfuGv1RJCSZh77dDc3PtvZXGwcsTufn9tU6T9VOTFcxovpyJ6w0og73eD0D02syR8R+kzv6rg1TeS8+lj7pyrQ== +"@opentelemetry/instrumentation-koa@0.39.0": + version "0.39.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-koa/-/instrumentation-koa-0.39.0.tgz#9c01d40a444e592a95b6e39ba0bbe94e096bfc31" + integrity sha512-eSqPzDykJVF9AcOuQvYqYCA/TN8tnU9/RYgrdPclaQcH6nfp0ZbQqLsAMGOwatfwJ8p06FAj+koPBy5NQNFMow== dependencies: "@opentelemetry/core" "^1.8.0" - "@opentelemetry/instrumentation" "^0.48.0" - "@opentelemetry/semantic-conventions" "^1.0.0" + "@opentelemetry/instrumentation" "^0.50.0" + "@opentelemetry/semantic-conventions" "^1.22.0" "@types/koa" "2.14.0" "@types/koa__router" "12.0.3" @@ -5077,6 +5084,18 @@ semver "^7.5.2" shimmer "^1.2.1" +"@opentelemetry/instrumentation@^0.50.0": + version "0.50.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation/-/instrumentation-0.50.0.tgz#c558cfc64b84c11d304f31ccdf0de312ec60a2c9" + integrity sha512-bhGhbJiZKpuu7wTaSak4hyZcFPlnDeuSF/2vglze8B4w2LubcSbbOnkVTzTs5SXtzh4Xz8eRjaNnAm+u2GYufQ== + dependencies: + "@opentelemetry/api-logs" "0.50.0" + "@types/shimmer" "^1.0.2" + import-in-the-middle "1.7.1" + require-in-the-middle "^7.1.1" + semver "^7.5.2" + shimmer "^1.2.1" + "@opentelemetry/resources@1.20.0": version "1.20.0" resolved "https://registry.yarnpkg.com/@opentelemetry/resources/-/resources-1.20.0.tgz#7165c39837e6e41b695f0088e40d15a5793f1469" @@ -5141,12 +5160,12 @@ resolved "https://registry.yarnpkg.com/@opentelemetry/semantic-conventions/-/semantic-conventions-1.20.0.tgz#4d9b88188e18056a218644ea30fae130a7857766" integrity sha512-3zLJJCgTKYpbqFX8drl8hOCHtdchELC+kGqlVcV4mHW1DiElTtv1Nt9EKBptTd1IfL56QkuYnWJ3DeHd2Gtu/A== -"@opentelemetry/semantic-conventions@1.21.0", "@opentelemetry/semantic-conventions@^1.0.0", "@opentelemetry/semantic-conventions@^1.17.0": +"@opentelemetry/semantic-conventions@1.21.0": version "1.21.0" resolved "https://registry.yarnpkg.com/@opentelemetry/semantic-conventions/-/semantic-conventions-1.21.0.tgz#83f7479c524ab523ac2df702ade30b9724476c72" integrity sha512-lkC8kZYntxVKr7b8xmjCVUgE0a8xgDakPyDo9uSWavXPyYqLgYYGdEd2j8NxihRyb6UwpX3G/hFUF4/9q2V+/g== -"@opentelemetry/semantic-conventions@1.23.0", "@opentelemetry/semantic-conventions@^1.23.0": +"@opentelemetry/semantic-conventions@1.23.0", "@opentelemetry/semantic-conventions@^1.0.0", "@opentelemetry/semantic-conventions@^1.17.0", "@opentelemetry/semantic-conventions@^1.22.0", "@opentelemetry/semantic-conventions@^1.23.0": version "1.23.0" resolved "https://registry.yarnpkg.com/@opentelemetry/semantic-conventions/-/semantic-conventions-1.23.0.tgz#627f2721b960fe586b7f72a07912cb7699f06eef" integrity sha512-MiqFvfOzfR31t8cc74CTP1OZfz7MbqpAnLCra8NqQoaHJX6ncIRTdYOQYBDQ2uFISDq0WY8Y9dDTWvsgzzBYRg== From 6f5dd5008ffef3f0898b77b876be13d8520a8c33 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 16 Apr 2024 16:28:48 +0000 Subject: [PATCH 16/21] feat(deps): bump @opentelemetry/instrumentation-pg from 0.38.0 to 0.40.0 (#11494) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps [@opentelemetry/instrumentation-pg](https://github.com/open-telemetry/opentelemetry-js-contrib) from 0.38.0 to 0.40.0.
Commits
  • f81f8a7 chore: release main (#1539)
  • 8d9687d feat(fastify): Skip update HTTP's span name and update RpcMetadata's route in...
  • bf25eb1 chore(renovate): change strategy for @​opentelemetry/api, run experimental upd...
  • 3139dbf chore: update renovate.json (#1575)
  • 273993b chore: re-enable instrumentation-fastify unit test on node@18 (#1568)
  • 84a2377 fix(deps): update otel core experimental to ^0.41.0 (#1566)
  • ffb45fe chore(renovate): split patch and minor rules (#1572)
  • 8e2f518 feat(express): Skip update HTTP's span name and update RpcMetadata's route in...
  • 774d254 fix(document-load): compatibility issue with @​opentelemetry/sdk-trace-web@​1.1...
  • a18b074 docs: document merge reqiurements (#1553)
  • Additional commits viewable in compare view

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=@opentelemetry/instrumentation-pg&package-manager=npm_and_yarn&previous-version=0.38.0&new-version=0.40.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- packages/node/package.json | 2 +- yarn.lock | 23 +++++++---------------- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/packages/node/package.json b/packages/node/package.json index 0013de64e3d1..79f980ce32f4 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -68,7 +68,7 @@ "@opentelemetry/instrumentation-mysql": "0.35.0", "@opentelemetry/instrumentation-mysql2": "0.35.0", "@opentelemetry/instrumentation-nestjs-core": "0.34.0", - "@opentelemetry/instrumentation-pg": "0.38.0", + "@opentelemetry/instrumentation-pg": "0.40.0", "@opentelemetry/resources": "^1.23.0", "@opentelemetry/sdk-trace-base": "^1.23.0", "@opentelemetry/semantic-conventions": "^1.23.0", diff --git a/yarn.lock b/yarn.lock index 26af712238f6..3b7f8d0965e1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5040,13 +5040,13 @@ "@opentelemetry/instrumentation" "^0.48.0" "@opentelemetry/semantic-conventions" "^1.0.0" -"@opentelemetry/instrumentation-pg@0.38.0": - version "0.38.0" - resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-pg/-/instrumentation-pg-0.38.0.tgz#19d49cc301ab63124a0482f21f64be3fbb81321c" - integrity sha512-Q7V/OJ1OZwaWYNOP/E9S6sfS03Z+PNU1SAjdAoXTj5j4u4iJSMSieLRWXFaHwsbefIOMkYvA00EBKF9IgbgbLA== +"@opentelemetry/instrumentation-pg@0.40.0": + version "0.40.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-pg/-/instrumentation-pg-0.40.0.tgz#f83bd217ffb41cd471f9acfb5c1d5c1d420a70bb" + integrity sha512-z3v8OzfImnycykWgqIdS44aUZlRwq51yYIo8GfmiRBd8yyMl2ESQyv6z/IAWBWyT015IWGy3ZTijySe65P9J1w== dependencies: - "@opentelemetry/instrumentation" "^0.48.0" - "@opentelemetry/semantic-conventions" "^1.0.0" + "@opentelemetry/instrumentation" "^0.50.0" + "@opentelemetry/semantic-conventions" "^1.22.0" "@opentelemetry/sql-common" "^0.40.0" "@types/pg" "8.6.1" "@types/pg-pool" "2.0.4" @@ -6973,7 +6973,7 @@ dependencies: "@types/pg" "*" -"@types/pg@*": +"@types/pg@*", "@types/pg@^8.6.5": version "8.10.2" resolved "https://registry.yarnpkg.com/@types/pg/-/pg-8.10.2.tgz#7814d1ca02c8071f4d0864c1b17c589b061dba43" integrity sha512-MKFs9P6nJ+LAeHLU3V0cODEOgyThJ3OAnmOlsZsxux6sfQs3HRXR5bBn7xG5DjckEFhTAxsXi7k7cd0pCMxpJw== @@ -6991,15 +6991,6 @@ pg-protocol "*" pg-types "^2.2.0" -"@types/pg@^8.6.5": - version "8.6.5" - resolved "https://registry.yarnpkg.com/@types/pg/-/pg-8.6.5.tgz#2dce9cb468a6a5e0f1296a59aea3ac75dd27b702" - integrity sha512-tOkGtAqRVkHa/PVZicq67zuujI4Oorfglsr2IbKofDwBSysnaqSx7W1mDqFqdkGE6Fbgh+PZAl0r/BWON/mozw== - dependencies: - "@types/node" "*" - pg-protocol "*" - pg-types "^2.2.0" - "@types/prettier@^2.1.5": version "2.4.4" resolved "https://registry.yarnpkg.com/@types/prettier/-/prettier-2.4.4.tgz#5d9b63132df54d8909fce1c3f8ca260fdd693e17" From 2c840c3cb87fec505ebc566e8097562fb46132c4 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 16 Apr 2024 20:31:52 +0000 Subject: [PATCH 17/21] feat(deps): bump @opentelemetry/instrumentation-hapi from 0.34.0 to 0.36.0 (#11496) --- packages/node/package.json | 2 +- yarn.lock | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/node/package.json b/packages/node/package.json index 79f980ce32f4..8474a22122e4 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -60,7 +60,7 @@ "@opentelemetry/instrumentation-express": "0.35.0", "@opentelemetry/instrumentation-fastify": "0.33.0", "@opentelemetry/instrumentation-graphql": "0.37.0", - "@opentelemetry/instrumentation-hapi": "0.34.0", + "@opentelemetry/instrumentation-hapi": "0.36.0", "@opentelemetry/instrumentation-http": "0.48.0", "@opentelemetry/instrumentation-koa": "0.39.0", "@opentelemetry/instrumentation-mongodb": "0.39.0", diff --git a/yarn.lock b/yarn.lock index 3b7f8d0965e1..1a81f9473805 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4965,13 +4965,13 @@ dependencies: "@opentelemetry/instrumentation" "^0.48.0" -"@opentelemetry/instrumentation-hapi@0.34.0": - version "0.34.0" - resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-hapi/-/instrumentation-hapi-0.34.0.tgz#edab0a9175ca141cd289d28c7d1677a2da80d993" - integrity sha512-qUENVxwCYbRbJ8HBY54ZL1Z9q1guCEurW6tCFFJJKQFu/MKEw7GSFImy5DR2Mp8b5ggZO36lYFcx0QUxfy4GJg== +"@opentelemetry/instrumentation-hapi@0.36.0": + version "0.36.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-hapi/-/instrumentation-hapi-0.36.0.tgz#ce3ed250ee0006f4335bda4cc06bc2d531307d4c" + integrity sha512-0NnXuRF89Windosn+iUpq5Fn/Foy8PMJxtLfe6CakDJIUGPj/g1+erz5irqSOc0P5mM3rEqKC/cYCoSIMKo/eA== dependencies: "@opentelemetry/core" "^1.8.0" - "@opentelemetry/instrumentation" "^0.48.0" + "@opentelemetry/instrumentation" "^0.50.0" "@opentelemetry/semantic-conventions" "^1.0.0" "@types/hapi__hapi" "20.0.13" From daf2edf9bab32e028780332eb6a45cb58af75762 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 17 Apr 2024 09:57:28 +0200 Subject: [PATCH 18/21] feat(core): Add `server.address` to browser `http.client` spans (#11634) Closes https://github.com/getsentry/sentry-javascript/issues/11632 --- .size-limit.js | 2 +- .../request/fetch-relative-url/init.js | 9 +++ .../request/fetch-relative-url/subject.js | 3 + .../request/fetch-relative-url/test.ts | 79 +++++++++++++++++++ .../suites/tracing/request/fetch/test.ts | 7 ++ .../tracing/request/xhr-relative-url/init.js | 9 +++ .../request/xhr-relative-url/subject.js | 12 +++ .../tracing/request/xhr-relative-url/test.ts | 79 +++++++++++++++++++ .../suites/tracing/request/xhr/test.ts | 7 ++ .../nextjs-app-dir/tests/middleware.test.ts | 2 + packages/browser/src/tracing/request.ts | 32 +++++++- packages/core/src/fetch.ts | 55 ++++++++----- .../test/client/tracingFetch.test.ts | 2 + 13 files changed, 278 insertions(+), 20 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/fetch-relative-url/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/fetch-relative-url/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/fetch-relative-url/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/xhr-relative-url/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/xhr-relative-url/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/xhr-relative-url/test.ts diff --git a/.size-limit.js b/.size-limit.js index 6e005fd7c3e7..3b91cb51a3c5 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -208,7 +208,7 @@ module.exports = [ 'tls', ], gzip: true, - limit: '150 KB', + limit: '160 KB', }, ]; diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-relative-url/init.js b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-relative-url/init.js new file mode 100644 index 000000000000..83076460599f --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-relative-url/init.js @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +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/request/fetch-relative-url/subject.js b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-relative-url/subject.js new file mode 100644 index 000000000000..b0d4abc78c65 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-relative-url/subject.js @@ -0,0 +1,3 @@ +fetch('/test-req/0').then( + fetch('/test-req/1', { headers: { 'X-Test-Header': 'existing-header' } }).then(fetch('/test-req/2')), +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-relative-url/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-relative-url/test.ts new file mode 100644 index 000000000000..d4a6065a177b --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-relative-url/test.ts @@ -0,0 +1,79 @@ +import { expect } from '@playwright/test'; + +import { TEST_HOST, sentryTest } from '../../../../utils/fixtures'; +import { + envelopeRequestParser, + shouldSkipTracingTest, + waitForTransactionRequestOnUrl, +} from '../../../../utils/helpers'; + +sentryTest('should create spans for fetch requests', async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + const req = await waitForTransactionRequestOnUrl(page, url); + const tracingEvent = envelopeRequestParser(req); + + const requestSpans = tracingEvent.spans?.filter(({ op }) => op === 'http.client'); + + expect(requestSpans).toHaveLength(3); + + requestSpans?.forEach((span, index) => + expect(span).toMatchObject({ + description: `GET /test-req/${index}`, + parent_span_id: tracingEvent.contexts?.trace?.span_id, + span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + trace_id: tracingEvent.contexts?.trace?.trace_id, + data: { + 'http.method': 'GET', + 'http.url': `${TEST_HOST}/test-req/${index}`, + url: `/test-req/${index}`, + 'server.address': 'sentry-test.io', + type: 'fetch', + }, + }), + ); +}); + +sentryTest('should attach `sentry-trace` header to fetch requests', async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const requests = ( + await Promise.all([ + page.goto(url), + Promise.all([0, 1, 2].map(idx => page.waitForRequest(`${TEST_HOST}/test-req/${idx}`))), + ]) + )[1]; + + expect(requests).toHaveLength(3); + + const request1 = requests[0]; + const requestHeaders1 = request1.headers(); + expect(requestHeaders1).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/), + baggage: expect.any(String), + }); + + const request2 = requests[1]; + const requestHeaders2 = request2.headers(); + expect(requestHeaders2).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/), + baggage: expect.any(String), + 'x-test-header': 'existing-header', + }); + + const request3 = requests[2]; + const requestHeaders3 = request3.headers(); + expect(requestHeaders3).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/), + baggage: expect.any(String), + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/fetch/test.ts index 00cf0baafc6a..de6b1521d686 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/fetch/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch/test.ts @@ -36,6 +36,13 @@ sentryTest('should create spans for fetch requests', async ({ getLocalTestPath, start_timestamp: expect.any(Number), timestamp: expect.any(Number), trace_id: tracingEvent.contexts?.trace?.trace_id, + data: { + 'http.method': 'GET', + 'http.url': `http://example.com/${index}`, + url: `http://example.com/${index}`, + 'server.address': 'example.com', + type: 'fetch', + }, }), ); }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-relative-url/init.js b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-relative-url/init.js new file mode 100644 index 000000000000..83076460599f --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-relative-url/init.js @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +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/request/xhr-relative-url/subject.js b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-relative-url/subject.js new file mode 100644 index 000000000000..5fc9f91ab568 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-relative-url/subject.js @@ -0,0 +1,12 @@ +const xhr_1 = new XMLHttpRequest(); +xhr_1.open('GET', '/test-req/0'); +xhr_1.send(); + +const xhr_2 = new XMLHttpRequest(); +xhr_2.open('GET', '/test-req/1'); +xhr_2.setRequestHeader('X-Test-Header', 'existing-header'); +xhr_2.send(); + +const xhr_3 = new XMLHttpRequest(); +xhr_3.open('GET', '/test-req/2'); +xhr_3.send(); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-relative-url/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-relative-url/test.ts new file mode 100644 index 000000000000..f3dca9359b8b --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-relative-url/test.ts @@ -0,0 +1,79 @@ +import { expect } from '@playwright/test'; + +import { TEST_HOST, sentryTest } from '../../../../utils/fixtures'; +import { + envelopeRequestParser, + shouldSkipTracingTest, + waitForTransactionRequestOnUrl, +} from '../../../../utils/helpers'; + +sentryTest('should create spans for xhr requests', async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + const req = await waitForTransactionRequestOnUrl(page, url); + const tracingEvent = envelopeRequestParser(req); + + const requestSpans = tracingEvent.spans?.filter(({ op }) => op === 'http.client'); + + expect(requestSpans).toHaveLength(3); + + requestSpans?.forEach((span, index) => + expect(span).toMatchObject({ + description: `GET /test-req/${index}`, + parent_span_id: tracingEvent.contexts?.trace?.span_id, + span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + trace_id: tracingEvent.contexts?.trace?.trace_id, + data: { + 'http.method': 'GET', + 'http.url': `${TEST_HOST}/test-req/${index}`, + url: `/test-req/${index}`, + 'server.address': 'sentry-test.io', + type: 'xhr', + }, + }), + ); +}); + +sentryTest('should attach `sentry-trace` header to xhr requests', async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const requests = ( + await Promise.all([ + page.goto(url), + Promise.all([0, 1, 2].map(idx => page.waitForRequest(`${TEST_HOST}/test-req/${idx}`))), + ]) + )[1]; + + expect(requests).toHaveLength(3); + + const request1 = requests[0]; + const requestHeaders1 = request1.headers(); + expect(requestHeaders1).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/), + baggage: expect.any(String), + }); + + const request2 = requests[1]; + const requestHeaders2 = request2.headers(); + expect(requestHeaders2).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/), + baggage: expect.any(String), + 'x-test-header': 'existing-header', + }); + + const request3 = requests[2]; + const requestHeaders3 = request3.headers(); + expect(requestHeaders3).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/), + baggage: expect.any(String), + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/xhr/test.ts index 13646a34826e..5dbfd3edf4cb 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/xhr/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr/test.ts @@ -24,6 +24,13 @@ sentryTest('should create spans for XHR requests', async ({ getLocalTestPath, pa start_timestamp: expect.any(Number), timestamp: expect.any(Number), trace_id: eventData.contexts?.trace?.trace_id, + data: { + 'http.method': 'GET', + 'http.url': `http://example.com/${index}`, + url: `http://example.com/${index}`, + 'server.address': 'example.com', + type: 'xhr', + }, }), ); }); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts index 79b07bd37a15..da0c4f303604 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts @@ -76,6 +76,8 @@ test('Should trace outgoing fetch requests inside middleware and create breadcru 'http.response.status_code': 200, type: 'fetch', url: 'http://localhost:3030/', + 'http.url': 'http://localhost:3030/', + 'server.address': 'localhost:3030', 'sentry.op': 'http.client', 'sentry.origin': 'auto.http.wintercg_fetch', }, diff --git a/packages/browser/src/tracing/request.ts b/packages/browser/src/tracing/request.ts index 2c013fe27232..45b373c2468e 100644 --- a/packages/browser/src/tracing/request.ts +++ b/packages/browser/src/tracing/request.ts @@ -4,6 +4,7 @@ import { addXhrInstrumentationHandler, } from '@sentry-internal/browser-utils'; import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SentryNonRecordingSpan, getActiveSpan, @@ -26,6 +27,7 @@ import { browserPerformanceTimeOrigin, dynamicSamplingContextToSentryBaggageHeader, generateSentryTraceHeader, + parseUrl, stringMatchesSomePattern, } from '@sentry/utils'; import { WINDOW } from '../helpers'; @@ -115,6 +117,18 @@ export function instrumentOutgoingRequests(_options?: Partial { const createdSpan = instrumentFetchRequest(handlerData, shouldCreateSpan, shouldAttachHeadersWithTargets, spans); + // We cannot use `window.location` in the generic fetch instrumentation, + // but we need it for reliable `server.address` attribute. + // so we extend this in here + if (createdSpan) { + const fullUrl = getFullURL(handlerData.fetchData.url); + const host = fullUrl ? parseUrl(fullUrl).host : undefined; + createdSpan.setAttributes({ + 'http.url': fullUrl, + 'server.address': host, + }); + } + if (enableHTTPTimings && createdSpan) { addHTTPTimings(createdSpan); } @@ -310,6 +324,9 @@ export function xhrCallback( const hasParent = !!getActiveSpan(); + const fullUrl = getFullURL(sentryXhrData.url); + const host = fullUrl ? parseUrl(fullUrl).host : undefined; + const span = shouldCreateSpanResult && hasParent ? startInactiveSpan({ @@ -317,10 +334,12 @@ export function xhrCallback( attributes: { type: 'xhr', 'http.method': sentryXhrData.method, + 'http.url': fullUrl, url: sentryXhrData.url, + 'server.address': host, [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.client', }, - op: 'http.client', }) : new SentryNonRecordingSpan(); @@ -381,3 +400,14 @@ function setHeaderOnXhr( // Error: InvalidStateError: Failed to execute 'setRequestHeader' on 'XMLHttpRequest': The object's state must be OPENED. } } + +function getFullURL(url: string): string | undefined { + try { + // By adding a base URL to new URL(), this will also work for relative urls + // If `url` is a full URL, the base URL is ignored anyhow + const parsed = new URL(url, WINDOW.location.origin); + return parsed.href; + } catch { + return undefined; + } +} diff --git a/packages/core/src/fetch.ts b/packages/core/src/fetch.ts index f9069b6b6efa..394f70d7fa47 100644 --- a/packages/core/src/fetch.ts +++ b/packages/core/src/fetch.ts @@ -4,9 +4,10 @@ import { dynamicSamplingContextToSentryBaggageHeader, generateSentryTraceHeader, isInstanceOf, + parseUrl, } from '@sentry/utils'; import { getClient, getCurrentScope, getIsolationScope } from './currentScopes'; -import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from './semanticAttributes'; +import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from './semanticAttributes'; import { SPAN_STATUS_ERROR, getDynamicSamplingContextFromClient, @@ -53,22 +54,7 @@ export function instrumentFetchRequest( const span = spans[spanId]; if (span) { - if (handlerData.response) { - setHttpStatus(span, handlerData.response.status); - - const contentLength = - handlerData.response && handlerData.response.headers && handlerData.response.headers.get('content-length'); - - if (contentLength) { - const contentLengthNum = parseInt(contentLength); - if (contentLengthNum > 0) { - span.setAttribute('http.response_content_length', contentLengthNum); - } - } - } else if (handlerData.error) { - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - } - span.end(); + endSpan(span, handlerData); // eslint-disable-next-line @typescript-eslint/no-dynamic-delete delete spans[spanId]; @@ -83,6 +69,9 @@ export function instrumentFetchRequest( const hasParent = !!getActiveSpan(); + const fullUrl = getFullURL(url); + const host = fullUrl ? parseUrl(fullUrl).host : undefined; + const span = shouldCreateSpanResult && hasParent ? startInactiveSpan({ @@ -91,9 +80,11 @@ export function instrumentFetchRequest( url, type: 'fetch', 'http.method': method, + 'http.url': fullUrl, + 'server.address': host, [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: spanOrigin, + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.client', }, - op: 'http.client', }) : new SentryNonRecordingSpan(); @@ -203,3 +194,31 @@ export function addTracingHeadersToFetchRequest( }; } } + +function getFullURL(url: string): string | undefined { + try { + const parsed = new URL(url); + return parsed.href; + } catch { + return undefined; + } +} + +function endSpan(span: Span, handlerData: HandlerDataFetch): void { + if (handlerData.response) { + setHttpStatus(span, handlerData.response.status); + + const contentLength = + handlerData.response && handlerData.response.headers && handlerData.response.headers.get('content-length'); + + if (contentLength) { + const contentLengthNum = parseInt(contentLength); + if (contentLengthNum > 0) { + span.setAttribute('http.response_content_length', contentLengthNum); + } + } + } else if (handlerData.error) { + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + } + span.end(); +} diff --git a/packages/nextjs/test/integration/test/client/tracingFetch.test.ts b/packages/nextjs/test/integration/test/client/tracingFetch.test.ts index 8517b4ab0fce..debff6001fce 100644 --- a/packages/nextjs/test/integration/test/client/tracingFetch.test.ts +++ b/packages/nextjs/test/integration/test/client/tracingFetch.test.ts @@ -33,6 +33,8 @@ test('should correctly instrument `fetch` for performance tracing', async ({ pag data: { 'http.method': 'GET', url: 'http://example.com', + 'http.url': 'http://example.com/', + 'server.address': 'example.com', type: 'fetch', 'http.response_content_length': expect.any(Number), 'http.response.status_code': 200, From cfcd226e9ca75ef739fd30e7471baac0b1545519 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 17 Apr 2024 13:39:19 +0200 Subject: [PATCH 19/21] feat(browser): Update `propagationContext` on `spanEnd` to keep trace consistent (#11631) As spec'd out in #11599 and agreed upon in internal discussions, a trace should to stay consistent over the entire time span of one route. Therefore, when the initial pageload or navigation span ends, we update the scope's propagation context to keep span-specific attributes like the `sampled` decision and the dynamic sampling context on the scope's propagation context, even after the transaction has ended. This ensures that the trace data is consistent for the entire duration of the route. Subsequent navigations will reset the propagation context (see #11377). --- .../tracing/trace-lifetime/navigation/test.ts | 12 +++--- .../pageload-meta/template.html | 2 +- .../trace-lifetime/pageload-meta/test.ts | 2 +- .../tracing/trace-lifetime/pageload/test.ts | 12 +++--- .../src/tracing/browserTracingIntegration.ts | 25 +++++++++++ .../tracing/browserTracingIntegration.test.ts | 41 ++++++++++++++++++- 6 files changed, 79 insertions(+), 15 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts index 4bf1d1f7d49a..f24ca0507c66 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts @@ -130,11 +130,11 @@ sentryTest( const request = await requestPromise; const headers = request.headers(); - // sampling decision is deferred b/c of no active span at the time of request + // sampling decision and DSC are continued from navigation span, even after it ended const navigationTraceId = navigationTraceContext?.trace_id; - expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}$`)); + expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}-1$`)); expect(headers['baggage']).toEqual( - `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId}`, + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sample_rate=1,sentry-sampled=true`, ); }, ); @@ -203,11 +203,11 @@ sentryTest( const request = await xhrPromise; const headers = request.headers(); - // sampling decision is deferred b/c of no active span at the time of request + // sampling decision and DSC are continued from navigation span, even after it ended const navigationTraceId = navigationTraceContext?.trace_id; - expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}$`)); + expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}-1$`)); expect(headers['baggage']).toEqual( - `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId}`, + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sample_rate=1,sentry-sampled=true`, ); }, ); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/template.html b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/template.html index dd44e58fbb4a..0dee204aef16 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/template.html +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/template.html @@ -4,7 +4,7 @@ + content="sentry-trace_id=12345678901234567890123456789012,sentry-sample_rate=0.2,sentry-sampled=true,sentry-transaction=my-transaction,sentry-public_key=public,sentry-release=1.0.0,sentry-environment=prod"/> diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts index d037bac433aa..75e1d4f1c3b6 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts @@ -10,7 +10,7 @@ import { const META_TAG_TRACE_ID = '12345678901234567890123456789012'; const META_TAG_PARENT_SPAN_ID = '1234567890123456'; const META_TAG_BAGGAGE = - 'sentry-trace_id=12345678901234567890123456789012,sentry-sample_rate=0.2,sentry-transaction=my-transaction,sentry-public_key=public,sentry-release=1.0.0,sentry-environment=prod'; + 'sentry-trace_id=12345678901234567890123456789012,sentry-sample_rate=0.2,sentry-sampled=true,sentry-transaction=my-transaction,sentry-public_key=public,sentry-release=1.0.0,sentry-environment=prod'; sentryTest( 'create a new trace for a navigation after the tag pageload trace', diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts index 333381d28b9a..dc87dea9760b 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts @@ -124,11 +124,11 @@ sentryTest( const request = await requestPromise; const headers = request.headers(); - // sampling decision is deferred b/c of no active span at the time of request + // sampling decision and DSC are continued from the pageload span even after it ended const pageloadTraceId = pageloadTraceContext?.trace_id; - expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}$`)); + expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}-1$`)); expect(headers['baggage']).toEqual( - `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId}`, + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId},sentry-sample_rate=1,sentry-sampled=true`, ); }, ); @@ -191,11 +191,11 @@ sentryTest( const request = await requestPromise; const headers = request.headers(); - // sampling decision is deferred b/c of no active span at the time of request + // sampling decision and DSC are continued from the pageload span even after it ended const pageloadTraceId = pageloadTraceContext?.trace_id; - expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}$`)); + expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}-1$`)); expect(headers['baggage']).toEqual( - `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId}`, + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId},sentry-sample_rate=1,sentry-sampled=true`, ); }, ); diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index 64510fc0b93f..8e61c95106e1 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -14,9 +14,11 @@ import { getActiveSpan, getClient, getCurrentScope, + getDynamicSamplingContextFromSpan, getIsolationScope, getRootSpan, registerSpanErrorInstrumentation, + spanIsSampled, spanToJSON, startIdleSpan, withScope, @@ -282,6 +284,29 @@ export const browserTracingIntegration = ((_options: Partial { + const op = spanToJSON(span).op; + if (span !== getRootSpan(span) || (op !== 'navigation' && op !== 'pageload')) { + return; + } + + const scope = getCurrentScope(); + const oldPropagationContext = scope.getPropagationContext(); + + const newPropagationContext = { + ...oldPropagationContext, + sampled: oldPropagationContext.sampled !== undefined ? oldPropagationContext.sampled : spanIsSampled(span), + dsc: oldPropagationContext.dsc || getDynamicSamplingContextFromSpan(span), + }; + + scope.setPropagationContext(newPropagationContext); + }); + if (options.instrumentPageLoad && WINDOW.location) { const startSpanOptions: StartSpanOptions = { name: WINDOW.location.pathname, diff --git a/packages/browser/test/unit/tracing/browserTracingIntegration.test.ts b/packages/browser/test/unit/tracing/browserTracingIntegration.test.ts index 7fde92ab764e..f4201e68a29a 100644 --- a/packages/browser/test/unit/tracing/browserTracingIntegration.test.ts +++ b/packages/browser/test/unit/tracing/browserTracingIntegration.test.ts @@ -638,7 +638,7 @@ describe('browserTracingIntegration', () => { expect(getCurrentScope().getScopeData().transactionName).toBe('test navigation span'); }); - it("resets the scopes' propagationContexts", () => { + it("updates the scopes' propagationContexts on a navigation", () => { const client = new BrowserClient( getDefaultBrowserClientOptions({ integrations: [browserTracingIntegration()], @@ -675,6 +675,45 @@ describe('browserTracingIntegration', () => { expect(newIsolationScopePropCtx?.traceId).not.toEqual(oldIsolationScopePropCtx?.traceId); expect(newCurrentScopePropCtx?.traceId).not.toEqual(oldCurrentScopePropCtx?.traceId); }); + + it("saves the span's sampling decision and its DSC on the propagationContext when the span finishes", () => { + const client = new BrowserClient( + getDefaultBrowserClientOptions({ + tracesSampleRate: 1, + integrations: [browserTracingIntegration({ instrumentPageLoad: false })], + }), + ); + setCurrentClient(client); + client.init(); + + const navigationSpan = startBrowserTracingNavigationSpan(client, { + name: 'mySpan', + attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route' }, + }); + + const propCtxBeforeEnd = getCurrentScope().getPropagationContext(); + expect(propCtxBeforeEnd).toStrictEqual({ + spanId: expect.stringMatching(/[a-f0-9]{16}/), + traceId: expect.stringMatching(/[a-f0-9]{32}/), + }); + + navigationSpan!.end(); + + const propCtxAfterEnd = getCurrentScope().getPropagationContext(); + expect(propCtxAfterEnd).toStrictEqual({ + spanId: propCtxBeforeEnd?.spanId, + traceId: propCtxBeforeEnd?.traceId, + sampled: true, + dsc: { + environment: 'production', + public_key: 'examplePublicKey', + sample_rate: '1', + sampled: 'true', + transaction: 'mySpan', + trace_id: propCtxBeforeEnd?.traceId, + }, + }); + }); }); describe('using the tag data', () => { From 64dffbf9438cf9964338da6b4069b3976f661bb2 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Wed, 17 Apr 2024 14:14:05 +0200 Subject: [PATCH 20/21] update changelog --- CHANGELOG.md | 30 ++++++++++++++++++++++++++++++ docs/publishing-a-release.md | 7 ++++--- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c7657346696..0059cb50436d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,36 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 8.0.0-beta.2 + +### Important Changes + +- **feat(browser): Update `propagationContext` on `spanEnd` to keep trace consistent** + +To ensure consistency throughout a route's duration, we update the scope's propagation context when the initial page +load or navigation span ends. This keeps span-specific attributes like the sampled decision and dynamic sampling context +on the scope, even after the transaction has ended. + +- **fix(browser): Don't assume window.document is available (#11602)** + +We won't assume `window.dodument` is available in the browser SDKs anymore. This should prevent errors in environments +where `window.document` is not available (such as web workers). + +### Other changes + +- feat(core): Add `server.address` to browser `http.client` spans (#11634) +- feat(opentelemetry): Update OTEL packages & relax some version ranges (#11580) +- feat(deps): bump @opentelemetry/instrumentation-hapi from 0.34.0 to 0.36.0 (#11496) +- feat(deps): bump @opentelemetry/instrumentation-koa from 0.37.0 to 0.39.0 (#11495) +- feat(deps): bump @opentelemetry/instrumentation-pg from 0.38.0 to 0.40.0 (#11494) +- feat(nextjs): Skip OTEL root spans emitted by Next.js (#11623) +- feat(node): Collect Local Variables via a worker (#11586) +- fix(nextjs): Escape Next.js' OpenTelemetry instrumentation (#11625) +- fix(feedback): Fix timeout on feedback submission (#11619) +- fix(node): Allow use of `NodeClient` without calling `init` (#11585) +- fix(node): Ensure DSC is correctly set in envelope headers (#11628) +- ref(core): Rename `Hub` to `AsyncContextStack` & remove unneeded methods (#11630) + ## 8.0.0-beta.1 This is the first beta release of Sentry JavaScript SDK v8. With this release, there are no more planned breaking diff --git a/docs/publishing-a-release.md b/docs/publishing-a-release.md index ab3044529ddc..3ab3f77563b1 100644 --- a/docs/publishing-a-release.md +++ b/docs/publishing-a-release.md @@ -23,9 +23,10 @@ _These steps are only relevant to Sentry employees when preparing and publishing 3. Create a new section in the changelog, deciding based on the changes whether it should be a minor bump or a patch release. 4. Paste in the logs you copied earlier. -5. Delete any which aren't user-facing changes. -6. If any of the PRs are from external contributors, include underneath the commits +5. Delete any which aren't user-facing changes (such as docs or tests). +6. Highlight any important changes with subheadings. +7. If any of the PRs are from external contributors, include underneath the commits `Work in this release contributed by . Thank you for your contributions!`. If there's only one external PR, don't forget to remove the final `s`. If there are three or more, use an Oxford comma. (It's in the Sentry styleguide!) -7. Commit, push, and open a PR with the title `meta(changelog): Update changelog for VERSION` against `master` branch. +8. Commit, push, and open a PR with the title `meta(changelog): Update changelog for VERSION` against `master` branch. From 3be73a64c629857884274096fb4e81f94183fc69 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Wed, 17 Apr 2024 14:22:17 +0200 Subject: [PATCH 21/21] delete non-user-facing change --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0059cb50436d..1501c66636f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,7 +32,6 @@ where `window.document` is not available (such as web workers). - fix(feedback): Fix timeout on feedback submission (#11619) - fix(node): Allow use of `NodeClient` without calling `init` (#11585) - fix(node): Ensure DSC is correctly set in envelope headers (#11628) -- ref(core): Rename `Hub` to `AsyncContextStack` & remove unneeded methods (#11630) ## 8.0.0-beta.1