From 02290d1974f0cf2c1df961c9594bd0d67bdb57a4 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 10 Sep 2024 17:19:59 +0200 Subject: [PATCH 01/39] feat(nextjs): Improve Next.js serverside span data quality (#13652) Sometimes we were sending faulty `default` ops for Next.js transactions. This seemed to happen when the HTTP integration couldn't get a handle of the incoming requests (yet to figure out why). This PR: - Backfills the op for faulty transactions - Sets the origin of spans generated by Next.js to `auto` (I couldn't come up with a more specific origin because the second part of it is an op and none are satisfactory) - Remove the `sentry.skip_span_data_inference` which is only used internally. This change is hard to test because it seems to happen flakily that the http integration isn't working. Fixes https://github.com/getsentry/sentry-javascript/issues/13598 --- .../tests/generation-functions.test.ts | 4 ++-- packages/nextjs/src/server/index.ts | 24 +++++++++++++++++++ packages/opentelemetry/src/spanExporter.ts | 1 + 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/tests/generation-functions.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/tests/generation-functions.test.ts index 303582ec1b24..bf3eca58a307 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-14/tests/generation-functions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/tests/generation-functions.test.ts @@ -17,7 +17,7 @@ test('Should emit a span for a generateMetadata() function invokation', async ({ expect(transaction.spans).toContainEqual( expect.objectContaining({ description: 'generateMetadata /generation-functions/page', - origin: 'manual', + origin: 'auto', parent_span_id: expect.any(String), span_id: expect.any(String), status: 'ok', @@ -74,7 +74,7 @@ test('Should send a transaction event for a generateViewport() function invokati expect((await transactionPromise).spans).toContainEqual( expect.objectContaining({ description: 'generateViewport /generation-functions/page', - origin: 'manual', + origin: 'auto', parent_span_id: expect.any(String), span_id: expect.any(String), status: 'ok', diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index 96c97371df24..e787f978cf22 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -1,4 +1,5 @@ import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, applySdkMetadata, getClient, @@ -189,6 +190,7 @@ export function init(options: NodeOptions): NodeClient | undefined { // with patterns (e.g. http.server spans) that will produce confusing data. if (spanAttributes?.['next.span_type'] !== undefined) { span.setAttribute('sentry.skip_span_data_inference', true); + span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto'); } // We want to rename these spans because they look like "GET /path/to/route" and we already emit spans that look @@ -286,6 +288,28 @@ export function init(options: NodeOptions): NodeClient | undefined { ), ); + getGlobalScope().addEventProcessor( + Object.assign( + (event => { + // Sometimes, the HTTP integration will not work, causing us not to properly set an op for spans generated by + // Next.js that are actually more or less correct server HTTP spans, so we are backfilling the op here. + if ( + event.type === 'transaction' && + event.transaction?.match(/^(RSC )?GET /) && + event.contexts?.trace?.data?.['sentry.rsc'] === true && + !event.contexts.trace.op + ) { + event.contexts.trace.data = event.contexts.trace.data || {}; + event.contexts.trace.data[SEMANTIC_ATTRIBUTE_SENTRY_OP] = 'http.server'; + event.contexts.trace.op = 'http.server'; + } + + return event; + }) satisfies EventProcessor, + { id: 'NextjsTransactionEnhancer' }, + ), + ); + if (process.env.NODE_ENV === 'development') { getGlobalScope().addEventProcessor(devErrorSymbolicationEventProcessor); } diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index d00319ec2c98..18c935863b75 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -345,6 +345,7 @@ function removeSentryAttributes(data: Record): Record Date: Tue, 10 Sep 2024 17:20:18 +0200 Subject: [PATCH 02/39] feat(nextjs): Give app router prefetch requests a `http.server.prefetch` op (#13600) Ref (not complete fix) https://github.com/getsentry/sentry-javascript/issues/13596 This gives Next.js prefetch requests a `http.server.prefetch` op, when a `Next-Router-Prefetch: 1` header is present. In some situations Next.js doesn't seem to attach the header for prefetch requests. Seems like it is only attached when the current route is a dynamic route. --- dev-packages/e2e-tests/package.json | 2 +- packages/node/src/integrations/http.ts | 12 ++++++++++++ .../opentelemetry/src/utils/parseSpanDescription.ts | 5 +++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/package.json b/dev-packages/e2e-tests/package.json index 966dab87a6bc..0a375d118749 100644 --- a/dev-packages/e2e-tests/package.json +++ b/dev-packages/e2e-tests/package.json @@ -14,7 +14,7 @@ "test:prepare": "ts-node prepare.ts", "test:validate": "run-s test:validate-configuration test:validate-test-app-setups", "clean": "rimraf tmp node_modules pnpm-lock.yaml && yarn clean:test-applications", - "clean:test-applications": "rimraf test-applications/**/{node_modules,dist,build,.next,.sveltekit,pnpm-lock.yaml} .last-run.json && pnpm store prune" + "clean:test-applications": "rimraf --glob test-applications/**/{node_modules,dist,build,.next,.sveltekit,pnpm-lock.yaml} .last-run.json && pnpm store prune" }, "devDependencies": { "@types/glob": "8.0.0", diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index 0d5b2d4814d1..d9e5e671b702 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -165,6 +165,10 @@ export const instrumentHttp = Object.assign( isolationScope.setTransactionName(bestEffortTransactionName); + if (isKnownPrefetchRequest(req)) { + span.setAttribute('sentry.http.prefetch', true); + } + _httpOptions.instrumentation?.requestHook?.(span, req); }, responseHook: (span, res) => { @@ -275,3 +279,11 @@ function getBreadcrumbData(request: ClientRequest): Partial Date: Tue, 10 Sep 2024 16:08:29 -0230 Subject: [PATCH 03/39] fix(replay): Fixes potential out-of-order segments (#13609) This fixes a potential issue where segments can come in out of order due to our flush "lock" was not being respected in two cases: 1) No current flush in progress, but flush throws an error (this should be rare as the common errors that get thrown should stop the replay completely) 2) Flush is in progress, which skips the code block that releases lock and then calls debouncedFlush. This leaves the lock always set to a resolved (or rejected) promise. This ultimately should not change too much as the flush calls are debounced anyway, but this cleans up the code a bit and also logs any exceptions that may occur. However this can fix issues where segments can come in out of order depending on how long the send request takes. e.g. ![image](https://github.com/user-attachments/assets/ea304892-1c72-4e96-acc6-c714d263980c) where ideally it looks like ![image](https://github.com/user-attachments/assets/8c3e706c-d3b2-43bd-a970-561b32b05458) --- packages/replay-internal/src/replay.ts | 28 ++++----- .../test/integration/flush.test.ts | 58 +++++++++++++++++++ 2 files changed, 73 insertions(+), 13 deletions(-) diff --git a/packages/replay-internal/src/replay.ts b/packages/replay-internal/src/replay.ts index b48ac787543b..cfeb26841911 100644 --- a/packages/replay-internal/src/replay.ts +++ b/packages/replay-internal/src/replay.ts @@ -1226,27 +1226,29 @@ export class ReplayContainer implements ReplayContainerInterface { // TODO FN: Evaluate if we want to stop here, or remove this again? } - // this._flushLock acts as a lock so that future calls to `_flush()` - // will be blocked until this promise resolves + const _flushInProgress = !!this._flushLock; + + // this._flushLock acts as a lock so that future calls to `_flush()` will + // be blocked until current flush is finished (i.e. this promise resolves) if (!this._flushLock) { this._flushLock = this._runFlush(); - await this._flushLock; - this._flushLock = undefined; - return; } - // Wait for previous flush to finish, then call the debounced `_flush()`. - // It's possible there are other flush requests queued and waiting for it - // to resolve. We want to reduce all outstanding requests (as well as any - // new flush requests that occur within a second of the locked flush - // completing) into a single flush. - try { await this._flushLock; } catch (err) { - DEBUG_BUILD && logger.error(err); + this.handleException(err); } finally { - this._debouncedFlush(); + this._flushLock = undefined; + + if (_flushInProgress) { + // Wait for previous flush to finish, then call the debounced + // `_flush()`. It's possible there are other flush requests queued and + // waiting for it to resolve. We want to reduce all outstanding + // requests (as well as any new flush requests that occur within a + // second of the locked flush completing) into a single flush. + this._debouncedFlush(); + } } }; diff --git a/packages/replay-internal/test/integration/flush.test.ts b/packages/replay-internal/test/integration/flush.test.ts index ffc0a83bb141..52654fa909d3 100644 --- a/packages/replay-internal/test/integration/flush.test.ts +++ b/packages/replay-internal/test/integration/flush.test.ts @@ -493,6 +493,64 @@ describe('Integration | flush', () => { await replay.start(); }); + it('resets flush lock if runFlush rejects/throws', async () => { + mockRunFlush.mockImplementation( + () => + new Promise((resolve, reject) => { + reject(new Error('runFlush')); + }), + ); + try { + await replay['_flush'](); + } catch { + // do nothing + } + expect(replay['_flushLock']).toBeUndefined(); + }); + + it('resets flush lock when flush is called multiple times before it resolves', async () => { + let _resolve; + mockRunFlush.mockImplementation( + () => + new Promise(resolve => { + _resolve = resolve; + }), + ); + const mockDebouncedFlush: MockedFunction = vi.spyOn(replay, '_debouncedFlush'); + mockDebouncedFlush.mockImplementation(vi.fn); + mockDebouncedFlush.cancel = vi.fn(); + + const results = [replay['_flush'](), replay['_flush']()]; + expect(replay['_flushLock']).not.toBeUndefined(); + + _resolve && _resolve(); + await Promise.all(results); + expect(replay['_flushLock']).toBeUndefined(); + mockDebouncedFlush.mockRestore(); + }); + + it('resets flush lock when flush is called multiple times before it rejects', async () => { + let _reject; + mockRunFlush.mockImplementation( + () => + new Promise((_, reject) => { + _reject = reject; + }), + ); + const mockDebouncedFlush: MockedFunction = vi.spyOn(replay, '_debouncedFlush'); + mockDebouncedFlush.mockImplementation(vi.fn); + mockDebouncedFlush.cancel = vi.fn(); + expect(replay['_flushLock']).toBeUndefined(); + replay['_flush'](); + const result = replay['_flush'](); + expect(replay['_flushLock']).not.toBeUndefined(); + + _reject && _reject(new Error('Throw runFlush')); + await result; + expect(replay['_flushLock']).toBeUndefined(); + mockDebouncedFlush.mockRestore(); + }); + /** * Assuming the user wants to record a session * when calling flush() without replay being enabled From e944daa3d71272f87fc324ae3f0b540ed0645b8e Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 10 Sep 2024 18:06:44 -0230 Subject: [PATCH 04/39] feat(replay): Add experimental option to allow for a checkout every 6 minutes (#13069) Including more checkouts will improve replayer scrubbing since it will reduce the number of mutations that need to be processed (especially for longer replays). The downside is that it will increase the size of replays since we will have up to 9 more snapshots per replay (max replay duration is 60 minutes / 6 minute checkouts). --- packages/replay-internal/src/replay.ts | 14 ++++++- packages/replay-internal/src/types/replay.ts | 1 + .../src/util/handleRecordingEmit.ts | 15 ++++--- .../test/integration/rrweb.test.ts | 40 +++++++++++++++++++ 4 files changed, 64 insertions(+), 6 deletions(-) diff --git a/packages/replay-internal/src/replay.ts b/packages/replay-internal/src/replay.ts index cfeb26841911..06f81b6982c6 100644 --- a/packages/replay-internal/src/replay.ts +++ b/packages/replay-internal/src/replay.ts @@ -377,7 +377,19 @@ export class ReplayContainer implements ReplayContainerInterface { // When running in error sampling mode, we need to overwrite `checkoutEveryNms` // Without this, it would record forever, until an error happens, which we don't want // instead, we'll always keep the last 60 seconds of replay before an error happened - ...(this.recordingMode === 'buffer' && { checkoutEveryNms: BUFFER_CHECKOUT_TIME }), + ...(this.recordingMode === 'buffer' + ? { checkoutEveryNms: BUFFER_CHECKOUT_TIME } + : // Otherwise, use experimental option w/ min checkout time of 6 minutes + // This is to improve playback seeking as there could potentially be + // less mutations to process in the worse cases. + // + // checkout by "N" events is probably ideal, but means we have less + // control about the number of checkouts we make (which generally + // increases replay size) + this._options._experiments.continuousCheckout && { + // Minimum checkout time is 6 minutes + checkoutEveryNms: Math.max(360_000, this._options._experiments.continuousCheckout), + }), emit: getHandleRecordingEmit(this), onMutation: this._onMutationHandler, ...(canvasOptions diff --git a/packages/replay-internal/src/types/replay.ts b/packages/replay-internal/src/types/replay.ts index 1e510e2bc519..0605ba97449a 100644 --- a/packages/replay-internal/src/types/replay.ts +++ b/packages/replay-internal/src/types/replay.ts @@ -232,6 +232,7 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions { _experiments: Partial<{ captureExceptions: boolean; traceInternals: boolean; + continuousCheckout: number; }>; } diff --git a/packages/replay-internal/src/util/handleRecordingEmit.ts b/packages/replay-internal/src/util/handleRecordingEmit.ts index 6b87845d793f..0467edefa9a2 100644 --- a/packages/replay-internal/src/util/handleRecordingEmit.ts +++ b/packages/replay-internal/src/util/handleRecordingEmit.ts @@ -58,9 +58,14 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa return false; } + const session = replay.session; + // Additionally, create a meta event that will capture certain SDK settings. // In order to handle buffer mode, this needs to either be done when we - // receive checkout events or at flush time. + // receive checkout events or at flush time. We have an experimental mode + // to perform multiple checkouts a session (the idea is to improve + // seeking during playback), so also only include if segmentId is 0 + // (handled in `addSettingsEvent`). // // `isCheckout` is always true, but want to be explicit that it should // only be added for checkouts @@ -72,22 +77,22 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa // of the previous session. Do not immediately flush in this case // to avoid capturing only the checkout and instead the replay will // be captured if they perform any follow-up actions. - if (replay.session && replay.session.previousSessionId) { + if (session && session.previousSessionId) { return true; } // When in buffer mode, make sure we adjust the session started date to the current earliest event of the buffer // this should usually be the timestamp of the checkout event, but to be safe... - if (replay.recordingMode === 'buffer' && replay.session && replay.eventBuffer) { + if (replay.recordingMode === 'buffer' && session && replay.eventBuffer) { const earliestEvent = replay.eventBuffer.getEarliestTimestamp(); if (earliestEvent) { DEBUG_BUILD && logger.info(`Updating session start time to earliest event in buffer to ${new Date(earliestEvent)}`); - replay.session.started = earliestEvent; + session.started = earliestEvent; if (replay.getOptions().stickySession) { - saveSession(replay.session); + saveSession(session); } } } diff --git a/packages/replay-internal/test/integration/rrweb.test.ts b/packages/replay-internal/test/integration/rrweb.test.ts index 863baab45bce..4327ddb21de1 100644 --- a/packages/replay-internal/test/integration/rrweb.test.ts +++ b/packages/replay-internal/test/integration/rrweb.test.ts @@ -46,4 +46,44 @@ describe('Integration | rrweb', () => { } `); }); + + it('calls rrweb.record with checkoutEveryNms', async () => { + const { mockRecord } = await resetSdkMock({ + replayOptions: { + _experiments: { + continuousCheckout: 1, + }, + }, + sentryOptions: { + replaysOnErrorSampleRate: 0.0, + replaysSessionSampleRate: 1.0, + }, + }); + + expect(mockRecord.mock.calls[0]?.[0]).toMatchInlineSnapshot(` + { + "blockSelector": ".sentry-block,[data-sentry-block],base[href="/"],img,image,svg,video,object,picture,embed,map,audio,link[rel="icon"],link[rel="apple-touch-icon"]", + "checkoutEveryNms": 360000, + "collectFonts": true, + "emit": [Function], + "errorHandler": [Function], + "ignoreSelector": ".sentry-ignore,[data-sentry-ignore],input[type="file"]", + "inlineImages": false, + "inlineStylesheet": true, + "maskAllInputs": true, + "maskAllText": true, + "maskAttributeFn": [Function], + "maskInputFn": undefined, + "maskInputOptions": { + "password": true, + }, + "maskTextFn": undefined, + "maskTextSelector": ".sentry-mask,[data-sentry-mask]", + "onMutation": [Function], + "slimDOMOptions": "all", + "unblockSelector": "", + "unmaskTextSelector": "", + } + `); + }); }); From fa9fa5d81619187122c9778b627b73ea52cba312 Mon Sep 17 00:00:00 2001 From: Sigrid Huemer <32902192+s1gr1d@users.noreply.github.com> Date: Thu, 12 Sep 2024 16:46:47 +0200 Subject: [PATCH 05/39] test(nuxt): Add tests for Vue component tracking (#13633) --- .../nuxt-3/sentry.client.config.ts | 1 + .../nuxt-3/tests/performance.client.test.ts | 31 ---------- .../nuxt-3/tests/tracing.client.test.ts | 57 +++++++++++++++++++ ....server.test.ts => tracing.server.test.ts} | 0 packages/nuxt/src/index.types.ts | 3 +- 5 files changed, 60 insertions(+), 32 deletions(-) delete mode 100644 dev-packages/e2e-tests/test-applications/nuxt-3/tests/performance.client.test.ts create mode 100644 dev-packages/e2e-tests/test-applications/nuxt-3/tests/tracing.client.test.ts rename dev-packages/e2e-tests/test-applications/nuxt-3/tests/{performance.server.test.ts => tracing.server.test.ts} (100%) diff --git a/dev-packages/e2e-tests/test-applications/nuxt-3/sentry.client.config.ts b/dev-packages/e2e-tests/test-applications/nuxt-3/sentry.client.config.ts index 5c4e0f892ca8..7547bafa6618 100644 --- a/dev-packages/e2e-tests/test-applications/nuxt-3/sentry.client.config.ts +++ b/dev-packages/e2e-tests/test-applications/nuxt-3/sentry.client.config.ts @@ -6,4 +6,5 @@ Sentry.init({ dsn: useRuntimeConfig().public.sentry.dsn, tunnel: `http://localhost:3031/`, // proxy server tracesSampleRate: 1.0, + trackComponents: true, }); diff --git a/dev-packages/e2e-tests/test-applications/nuxt-3/tests/performance.client.test.ts b/dev-packages/e2e-tests/test-applications/nuxt-3/tests/performance.client.test.ts deleted file mode 100644 index 66c8c9dfce2d..000000000000 --- a/dev-packages/e2e-tests/test-applications/nuxt-3/tests/performance.client.test.ts +++ /dev/null @@ -1,31 +0,0 @@ -import { expect, test } from '@nuxt/test-utils/playwright'; -import { waitForTransaction } from '@sentry-internal/test-utils'; - -test('sends a pageload root span with a parameterized URL', async ({ page }) => { - const transactionPromise = waitForTransaction('nuxt-3', async transactionEvent => { - return transactionEvent.transaction === '/test-param/:param()'; - }); - - await page.goto(`/test-param/1234`); - - const rootSpan = await transactionPromise; - - expect(rootSpan).toMatchObject({ - contexts: { - trace: { - data: { - 'sentry.source': 'route', - 'sentry.origin': 'auto.pageload.vue', - 'sentry.op': 'pageload', - 'params.param': '1234', - }, - op: 'pageload', - origin: 'auto.pageload.vue', - }, - }, - transaction: '/test-param/:param()', - transaction_info: { - source: 'route', - }, - }); -}); diff --git a/dev-packages/e2e-tests/test-applications/nuxt-3/tests/tracing.client.test.ts b/dev-packages/e2e-tests/test-applications/nuxt-3/tests/tracing.client.test.ts new file mode 100644 index 000000000000..76b2a9094531 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nuxt-3/tests/tracing.client.test.ts @@ -0,0 +1,57 @@ +import { expect, test } from '@nuxt/test-utils/playwright'; +import { waitForTransaction } from '@sentry-internal/test-utils'; +import type { Span } from '@sentry/nuxt'; + +test('sends a pageload root span with a parameterized URL', async ({ page }) => { + const transactionPromise = waitForTransaction('nuxt-3', async transactionEvent => { + return transactionEvent.transaction === '/test-param/:param()'; + }); + + await page.goto(`/test-param/1234`); + + const rootSpan = await transactionPromise; + + expect(rootSpan).toMatchObject({ + contexts: { + trace: { + data: { + 'sentry.source': 'route', + 'sentry.origin': 'auto.pageload.vue', + 'sentry.op': 'pageload', + 'params.param': '1234', + }, + op: 'pageload', + origin: 'auto.pageload.vue', + }, + }, + transaction: '/test-param/:param()', + transaction_info: { + source: 'route', + }, + }); +}); + +test('sends component tracking spans when `trackComponents` is enabled', async ({ page }) => { + const transactionPromise = waitForTransaction('nuxt-3', async transactionEvent => { + return transactionEvent.transaction === '/client-error'; + }); + + await page.goto(`/client-error`); + + const rootSpan = await transactionPromise; + const errorButtonSpan = rootSpan.spans.find((span: Span) => span.description === 'Vue '); + + const expected = { + data: { 'sentry.origin': 'auto.ui.vue', 'sentry.op': 'ui.vue.mount' }, + description: 'Vue ', + op: 'ui.vue.mount', + parent_span_id: expect.any(String), + span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + trace_id: expect.any(String), + origin: 'auto.ui.vue', + }; + + expect(errorButtonSpan).toMatchObject(expected); +}); diff --git a/dev-packages/e2e-tests/test-applications/nuxt-3/tests/performance.server.test.ts b/dev-packages/e2e-tests/test-applications/nuxt-3/tests/tracing.server.test.ts similarity index 100% rename from dev-packages/e2e-tests/test-applications/nuxt-3/tests/performance.server.test.ts rename to dev-packages/e2e-tests/test-applications/nuxt-3/tests/tracing.server.test.ts diff --git a/packages/nuxt/src/index.types.ts b/packages/nuxt/src/index.types.ts index d1850eec18ec..614b27bdefe3 100644 --- a/packages/nuxt/src/index.types.ts +++ b/packages/nuxt/src/index.types.ts @@ -1,4 +1,5 @@ import type { Integration, Options, StackParser } from '@sentry/types'; +import type { SentryNuxtClientOptions } from './common/types'; import type * as clientSdk from './index.client'; import type * as serverSdk from './index.server'; @@ -8,7 +9,7 @@ export * from './index.client'; export * from './index.server'; // re-export colliding types -export declare function init(options: Options | clientSdk.BrowserOptions | serverSdk.NodeOptions): void; +export declare function init(options: Options | SentryNuxtClientOptions | serverSdk.NodeOptions): void; export declare const linkedErrorsIntegration: typeof clientSdk.linkedErrorsIntegration; export declare const contextLinesIntegration: typeof clientSdk.contextLinesIntegration; export declare const getDefaultIntegrations: (options: Options) => Integration[]; From 1664dc729e8a4bf4a4cc280a2adbb8407101caa9 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com> Date: Thu, 12 Sep 2024 17:37:01 +0200 Subject: [PATCH 06/39] fix(normalize): Treat Infinity as NaN both are non-serializable numbers (#13406) RN SDK uses the normalize function before passing data over the RN Bridge, which only accepts serializable data. Infinity causes -> https://github.com/getsentry/sentry-react-native/issues/4024 --- .../suites/public-api/setExtras/consecutive_calls/test.ts | 7 ++++++- packages/utils/src/normalize.ts | 7 ++++--- packages/utils/test/normalize.test.ts | 2 ++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/public-api/setExtras/consecutive_calls/test.ts b/dev-packages/browser-integration-tests/suites/public-api/setExtras/consecutive_calls/test.ts index 9caae5b0bc7c..555686058366 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/setExtras/consecutive_calls/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/setExtras/consecutive_calls/test.ts @@ -10,5 +10,10 @@ sentryTest('should set extras from multiple consecutive calls', async ({ getLoca const eventData = await getFirstSentryEnvelopeRequest(page, url); expect(eventData.message).toBe('consecutive_calls'); - expect(eventData.extra).toMatchObject({ extra: [], Infinity: 2, null: null, obj: { foo: ['bar', 'baz', 1] } }); + expect(eventData.extra).toMatchObject({ + extra: [], + Infinity: 2, + null: '[Infinity]', + obj: { foo: ['bar', 'baz', 1] }, + }); }); diff --git a/packages/utils/src/normalize.ts b/packages/utils/src/normalize.ts index d86af9561c89..18b41f1c9357 100644 --- a/packages/utils/src/normalize.ts +++ b/packages/utils/src/normalize.ts @@ -81,7 +81,8 @@ function visit( // Get the simple cases out of the way first if ( value == null || // this matches null and undefined -> eqeq not eqeqeq - (['number', 'boolean', 'string'].includes(typeof value) && !Number.isNaN(value)) + ['boolean', 'string'].includes(typeof value) || + (typeof value === 'number' && Number.isFinite(value)) ) { return value as Primitive; } @@ -220,8 +221,8 @@ function stringifyValue( return '[SyntheticEvent]'; } - if (typeof value === 'number' && value !== value) { - return '[NaN]'; + if (typeof value === 'number' && !Number.isFinite(value)) { + return `[${value}]`; } if (typeof value === 'function') { diff --git a/packages/utils/test/normalize.test.ts b/packages/utils/test/normalize.test.ts index 5a2414d52e43..d8a8a1329352 100644 --- a/packages/utils/test/normalize.test.ts +++ b/packages/utils/test/normalize.test.ts @@ -403,6 +403,8 @@ describe('normalize()', () => { describe('changes unserializeable/global values/classes to their respective string representations', () => { test('primitive values', () => { expect(normalize(NaN)).toEqual('[NaN]'); + expect(normalize(Infinity)).toEqual('[Infinity]'); + expect(normalize(-Infinity)).toEqual('[-Infinity]'); expect(normalize(Symbol('dogs'))).toEqual('[Symbol(dogs)]'); expect(normalize(BigInt(1121201212312012))).toEqual('[BigInt: 1121201212312012]'); }); From df79871ab34707932511bd562ef2627e552c7457 Mon Sep 17 00:00:00 2001 From: Sigrid Huemer <32902192+s1gr1d@users.noreply.github.com> Date: Fri, 13 Sep 2024 09:48:45 +0200 Subject: [PATCH 07/39] feat(nuxt): Adding `experimental_basicServerTracing` option to Nuxt module (#13643) Enabling this option will import the Sentry server config at the top of the server entry file. This can be used when adding the node option `--import` does not work. This however only comes with limited tracing functionality. Example Usage: ```js export default defineNuxtConfig({ sentry: { // ... other options simplifiedDeployment: true }, }) --- packages/nuxt/README.md | 54 ++++++++++++++++------- packages/nuxt/src/common/types.ts | 12 +++++ packages/nuxt/src/module.ts | 21 +++++---- packages/nuxt/src/vite/addServerConfig.ts | 41 +++++++++++++++-- 4 files changed, 102 insertions(+), 26 deletions(-) diff --git a/packages/nuxt/README.md b/packages/nuxt/README.md index df41599e45b9..b16d555a3648 100644 --- a/packages/nuxt/README.md +++ b/packages/nuxt/README.md @@ -37,21 +37,23 @@ functionality related to Nuxt. **What is partly working:** +- Source Maps +- Connected Tracing (Frontend & Backend) - Tracing by setting `tracesSampleRate` - UI (Vue) traces - HTTP (Node) traces -**What is not yet(!) included:** - -- Source Maps -- Nuxt-specific traces and connecting frontend & backend traces - **Known Issues:** -- When adding `sentry.server.config.(ts/js)`, you get this error: "Failed to register ESM hook", but the application - will still work -- When initializing Sentry on the server with `instrument.server.(js|ts)`, you get an `'import-in-the-middle'` error, - and the application won't work +- When adding `sentry.server.config.(ts/js)`, you get an error like this: + "`Failed to register ESM hook (import-in-the-middle/hook.mjs)`". You can add a resolution for `@vercel/nft` to fix + this. This will add the `hook.mjs` file to your build output + ([issue here](https://github.com/unjs/nitro/issues/2703)). + ```json + "resolutions": { + "@vercel/nft": "^0.27.4" + } + ``` ## Automatic Setup @@ -93,16 +95,18 @@ export default defineNuxtConfig({ Add a `sentry.client.config.(js|ts)` file to the root of your project: ```javascript +import { useRuntimeConfig } from '#imports'; import * as Sentry from '@sentry/nuxt'; Sentry.init({ - dsn: process.env.SENTRY_DSN, + // If set up, you can use your runtime config here + dsn: useRuntimeConfig().public.sentry.dsn, }); ``` ### 4. Server-side setup -Add an `instrument.server.mjs` file to your `public` folder: +Add an `sentry.client.config.(js|ts)` file to the root of your project: ```javascript import * as Sentry from '@sentry/nuxt'; @@ -110,18 +114,38 @@ import * as Sentry from '@sentry/nuxt'; // Only run `init` when process.env.SENTRY_DSN is available. if (process.env.SENTRY_DSN) { Sentry.init({ - dsn: process.env.SENTRY_DSN, + dsn: 'your-dsn', }); } ``` -Add an import flag to the `NODE_OPTIONS` of your preview script in the `package.json` file, so the file loads before any -other imports: +The Nuxt runtime config does not work in the Sentry server to technical reasons (it has to be loaded before Nuxt is +loaded). To be able to use `process.env` you either have to add `--env-file=.env` to your node command + +```bash +node --env-file=.env --import ./.output/server/sentry.server.config.mjs .output/server/index.mjs +``` + +or use the `dotenv` package: + +```javascript +import dotenv from 'dotenv'; +import * as Sentry from '@sentry/nuxt'; + +dotenv.config(); + +Sentry.init({ + dsn: process.env.SENTRY_DSN, +}); +``` + +Add an import flag to the Node options of your `node` command (not `nuxt preview`), so the file loads before any other +imports (keep in mind the `.mjs` file ending): ```json { "scripts": { - "preview": "NODE_OPTIONS='--import ./public/instrument.server.mjs' nuxt preview" + "start": "node --import ./.output/server/sentry.server.config.mjs .output/server/index.mjs" } } ``` diff --git a/packages/nuxt/src/common/types.ts b/packages/nuxt/src/common/types.ts index 08dc0d2b805e..5fbe68bd89cb 100644 --- a/packages/nuxt/src/common/types.ts +++ b/packages/nuxt/src/common/types.ts @@ -99,4 +99,16 @@ export type SentryNuxtModuleOptions = { * Enabling this will give you, for example, logs about source maps. */ debug?: boolean; + + /** + * Enabling basic server tracing can be used for environments where modifying the node option `--import` is not possible. + * However, enabling this option only supports limited tracing instrumentation. Only http traces will be collected (but no database-specific traces etc.). + * + * If this option is `true`, the Sentry SDK will import the Sentry server config at the top of the server entry file to load the SDK on the server. + * + * **DO NOT** enable this option if you've already added the node option `--import` in your node start script. This would initialize Sentry twice on the server-side and leads to unexpected issues. + * + * @default false + */ + experimental_basicServerTracing?: boolean; }; diff --git a/packages/nuxt/src/module.ts b/packages/nuxt/src/module.ts index faa48e5c3c26..0afdeaa03de7 100644 --- a/packages/nuxt/src/module.ts +++ b/packages/nuxt/src/module.ts @@ -1,6 +1,6 @@ import { addPlugin, addPluginTemplate, addServerPlugin, createResolver, defineNuxtModule } from '@nuxt/kit'; import type { SentryNuxtModuleOptions } from './common/types'; -import { addServerConfigToBuild } from './vite/addServerConfig'; +import { addSentryTopImport, addServerConfigToBuild } from './vite/addServerConfig'; import { setupSourceMaps } from './vite/sourceMaps'; import { findDefaultSdkInitFile } from './vite/utils'; @@ -62,15 +62,20 @@ export default defineNuxtModule({ if (clientConfigFile || serverConfigFile) { setupSourceMaps(moduleOptions, nuxt); } - if (serverConfigFile && serverConfigFile.includes('.server.config')) { - if (moduleOptions.debug) { - // eslint-disable-next-line no-console - console.log( - `[Sentry] Using your \`${serverConfigFile}\` file for the server-side Sentry configuration. In case you have a \`public/instrument.server\` file, the \`public/instrument.server\` file will be ignored. Make sure the file path in your node \`--import\` option matches the Sentry server config file in your \`.output\` folder and has a \`.mjs\` extension.`, - ); - } + if (serverConfigFile && serverConfigFile.includes('.server.config')) { addServerConfigToBuild(moduleOptions, nuxt, serverConfigFile); + + if (moduleOptions.experimental_basicServerTracing) { + addSentryTopImport(moduleOptions, nuxt); + } else { + if (moduleOptions.debug) { + // eslint-disable-next-line no-console + console.log( + `[Sentry] Using your \`${serverConfigFile}\` file for the server-side Sentry configuration. In case you have a \`public/instrument.server\` file, the \`public/instrument.server\` file will be ignored. Make sure the file path in your node \`--import\` option matches the Sentry server config file in your \`.output\` folder and has a \`.mjs\` extension.`, + ); + } + } } }, }); diff --git a/packages/nuxt/src/vite/addServerConfig.ts b/packages/nuxt/src/vite/addServerConfig.ts index dc1fc21dd6bd..dee15ee34dce 100644 --- a/packages/nuxt/src/vite/addServerConfig.ts +++ b/packages/nuxt/src/vite/addServerConfig.ts @@ -1,5 +1,4 @@ import * as fs from 'fs'; -import * as path from 'path'; import { createResolver } from '@nuxt/kit'; import type { Nuxt } from '@nuxt/schema'; import type { SentryNuxtModuleOptions } from '../common/types'; @@ -30,8 +29,9 @@ export function addServerConfigToBuild( * This is necessary because we need to reference this file path in the node --import option. */ nuxt.hook('close', async () => { - const source = path.resolve('.nuxt/dist/server/sentry.server.config.mjs'); - const destination = path.resolve('.output/server/sentry.server.config.mjs'); + const rootDirResolver = createResolver(nuxt.options.rootDir); + const source = rootDirResolver.resolve('.nuxt/dist/server/sentry.server.config.mjs'); + const destination = rootDirResolver.resolve('.output/server/sentry.server.config.mjs'); try { await fs.promises.access(source, fs.constants.F_OK); @@ -55,3 +55,38 @@ export function addServerConfigToBuild( }); }); } + +/** + * Adds the Sentry server config import at the top of the server entry file to load the SDK on the server. + * This is necessary for environments where modifying the node option `--import` is not possible. + * However, only limited tracing instrumentation is supported when doing this. + */ +export function addSentryTopImport(moduleOptions: SentryNuxtModuleOptions, nuxt: Nuxt): void { + nuxt.hook('close', async () => { + const rootDirResolver = createResolver(nuxt.options.rootDir); + const entryFilePath = rootDirResolver.resolve('.output/server/index.mjs'); + + try { + fs.readFile(entryFilePath, 'utf8', (err, data) => { + const updatedContent = `import './sentry.server.config.mjs';\n${data}`; + + fs.writeFile(entryFilePath, updatedContent, 'utf8', () => { + if (moduleOptions.debug) { + // eslint-disable-next-line no-console + console.log( + `[Sentry] Successfully added the Sentry import to the server entry file "\`${entryFilePath}\`"`, + ); + } + }); + }); + } catch (err) { + if (moduleOptions.debug) { + // eslint-disable-next-line no-console + console.warn( + `[Sentry] An error occurred when trying to add the Sentry import to the server entry file "\`${entryFilePath}\`":`, + err, + ); + } + } + }); +} From bcd2a175e669d1435ef38c449805921b5d7c55d0 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Fri, 13 Sep 2024 10:42:18 +0200 Subject: [PATCH 08/39] fix(node): Don't overwrite local variables for re-thrown errors (#13644) Another way to fix this issue would be to check via the debugger if the property exists already on the error object and bail early before we have local variables. However, this would add an extra round trip call via the debugger for every error. Since re-thrown errors are far less likely, I decided instead to just not overwrite existing local variables. This PR also adds a `Debugger.resume` call in the catch case to ensure that they debugger will always resume even if we get errors while debugging. It's worth noting that this only fixes the issue in Node v20+ where we use the async debugging interface. --- .../LocalVariables/local-variables-rethrow.js | 45 +++++++++++++++++++ .../suites/public-api/LocalVariables/test.ts | 8 ++++ .../integrations/local-variables/worker.ts | 10 +++-- 3 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-rethrow.js diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-rethrow.js b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-rethrow.js new file mode 100644 index 000000000000..744cb747c70c --- /dev/null +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-rethrow.js @@ -0,0 +1,45 @@ +/* eslint-disable no-unused-vars */ +const Sentry = require('@sentry/node'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + includeLocalVariables: true, + transport: loggingTransport, +}); + +class Some { + two(name) { + throw new Error('Enough!'); + } +} + +function one(name) { + const arr = [1, '2', null]; + const obj = { + name, + num: 5, + }; + const bool = false; + const num = 0; + const str = ''; + const something = undefined; + const somethingElse = null; + + const ty = new Some(); + + ty.two(name); +} + +setTimeout(() => { + try { + try { + one('some name'); + } catch (e) { + const more = 'here'; + throw e; + } + } catch (e) { + Sentry.captureException(e); + } +}, 1000); diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts index 2640ecf94461..bf01ed999708 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts @@ -78,6 +78,14 @@ conditionalTest({ min: 18 })('LocalVariables integration', () => { }); }); + conditionalTest({ min: 20 })('Node v20+', () => { + test('Should retain original local variables when error is re-thrown', done => { + createRunner(__dirname, 'local-variables-rethrow.js') + .expect({ event: EXPECTED_LOCAL_VARIABLES_EVENT }) + .start(done); + }); + }); + test('Includes local variables for caught exceptions when enabled', done => { createRunner(__dirname, 'local-variables-caught.js').expect({ event: EXPECTED_LOCAL_VARIABLES_EVENT }).start(done); }); diff --git a/packages/node/src/integrations/local-variables/worker.ts b/packages/node/src/integrations/local-variables/worker.ts index eb4fee87947c..f0b3e20ce9b2 100644 --- a/packages/node/src/integrations/local-variables/worker.ts +++ b/packages/node/src/integrations/local-variables/worker.ts @@ -118,7 +118,9 @@ async function handlePaused( // We write the local variables to a property on the error object. These can be read by the integration as the error // event pass through the SDK event pipeline await session.post('Runtime.callFunctionOn', { - functionDeclaration: `function() { this.${LOCAL_VARIABLES_KEY} = ${JSON.stringify(frames)}; }`, + functionDeclaration: `function() { this.${LOCAL_VARIABLES_KEY} = this.${LOCAL_VARIABLES_KEY} || ${JSON.stringify( + frames, + )}; }`, silent: true, objectId, }); @@ -156,8 +158,10 @@ async function startDebugger(): Promise { }, 1_000); } }, - _ => { - // ignore any errors + async _ => { + if (isPaused) { + await session.post('Debugger.resume'); + } }, ); }); From 8711bdda72a3dbe401f8ec714625653ef8e1ca4e Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 13 Sep 2024 10:57:45 +0200 Subject: [PATCH 09/39] deps(sveltekit): Bump `sorcery` to 1.0.0 (#13660) Bump `sorcery` to version 1.0.0. the main changes made were to move the package to output ESM code. The SvelteKit build complained a bit about ESM initially but moving from a static to a dynamic import worked, so I'll take that. --- packages/sveltekit/package.json | 2 +- packages/sveltekit/src/vite/sourceMaps.ts | 5 ++- .../sveltekit/test/vite/sourceMaps.test.ts | 15 ++++++-- yarn.lock | 38 ++++++------------- 4 files changed, 27 insertions(+), 33 deletions(-) diff --git a/packages/sveltekit/package.json b/packages/sveltekit/package.json index 32446d0b2246..2876aa4aa1ab 100644 --- a/packages/sveltekit/package.json +++ b/packages/sveltekit/package.json @@ -49,7 +49,7 @@ "@sentry/vite-plugin": "2.22.3", "magic-string": "0.30.7", "magicast": "0.2.8", - "sorcery": "0.11.0" + "sorcery": "1.0.0" }, "devDependencies": { "@babel/types": "7.20.7", diff --git a/packages/sveltekit/src/vite/sourceMaps.ts b/packages/sveltekit/src/vite/sourceMaps.ts index b2ceace40529..d228eb6da30e 100644 --- a/packages/sveltekit/src/vite/sourceMaps.ts +++ b/packages/sveltekit/src/vite/sourceMaps.ts @@ -5,8 +5,6 @@ import { getSentryRelease } from '@sentry/node'; import { escapeStringForRegex, uuid4 } from '@sentry/utils'; import type { SentryVitePluginOptions } from '@sentry/vite-plugin'; import { sentryVitePlugin } from '@sentry/vite-plugin'; -// @ts-expect-error -sorcery has no types :( -import * as sorcery from 'sorcery'; import type { Plugin } from 'vite'; import MagicString from 'magic-string'; @@ -187,6 +185,9 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug // eslint-disable-next-line no-console debug && console.log('[Source Maps Plugin] Flattening source maps'); + // @ts-expect-error - we're using dynamic import here and TS complains about that. It works though. + const sorcery = await import('sorcery'); + for (const file of jsFiles) { try { await (sorcery as Sorcery).load(file).then(async chain => { diff --git a/packages/sveltekit/test/vite/sourceMaps.test.ts b/packages/sveltekit/test/vite/sourceMaps.test.ts index c25ec48a53cb..2d12c9835b58 100644 --- a/packages/sveltekit/test/vite/sourceMaps.test.ts +++ b/packages/sveltekit/test/vite/sourceMaps.test.ts @@ -17,6 +17,15 @@ vi.mock('@sentry/vite-plugin', async () => { }; }); +vi.mock('sorcery', async () => { + return { + load: vi.fn().mockResolvedValue({ + apply: vi.fn().mockResolvedValue(undefined), + write: vi.fn().mockResolvedValue(undefined), + }), + }; +}); + beforeEach(() => { vi.clearAllMocks(); }); @@ -79,7 +88,7 @@ describe('makeCustomSentryVitePlugin()', () => { // @ts-expect-error this function exists! plugin.configResolved({ build: { ssr: true } }); // @ts-expect-error this function exists! - plugin.closeBundle(); + await plugin.closeBundle(); expect(mockedSentryVitePlugin.writeBundle).toHaveBeenCalledTimes(1); }); @@ -89,7 +98,7 @@ describe('makeCustomSentryVitePlugin()', () => { // @ts-expect-error this function exists! plugin.configResolved({ build: { ssr: false } }); // @ts-expect-error this function exists! - plugin.closeBundle(); + await plugin.closeBundle(); expect(mockedSentryVitePlugin.writeBundle).not.toHaveBeenCalled(); }); }); @@ -110,7 +119,7 @@ describe('makeCustomSentryVitePlugin()', () => { // @ts-expect-error this function exists! plugin.configResolved({ build: { ssr: true } }); // @ts-expect-error this function exists! - plugin.closeBundle(); + await plugin.closeBundle(); expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('Failed to upload source maps')); expect(consoleLogSpy).toHaveBeenCalled(); diff --git a/yarn.lock b/yarn.lock index a9e1df0710b6..9f6846f29f95 100644 --- a/yarn.lock +++ b/yarn.lock @@ -13474,16 +13474,16 @@ bson@^1.1.4: resolved "https://registry.yarnpkg.com/bson/-/bson-1.1.6.tgz#fb819be9a60cd677e0853aee4ca712a785d6618a" integrity sha512-EvVNVeGo4tHxwi8L6bPj3y3itEvStdwvvlojVxxbyYfoaxJ6keLgrTuKdyfEAszFK+H3olzBuafE0yoh0D1gdg== -buffer-crc32@^0.2.5, buffer-crc32@~0.2.3: - version "0.2.13" - resolved "https://registry.yarnpkg.com/buffer-crc32/-/buffer-crc32-0.2.13.tgz#0d333e3f00eac50aa1454abd30ef8c2a5d9a7242" - integrity sha1-DTM+PwDqxQqhRUq9MO+MKl2ackI= - buffer-crc32@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/buffer-crc32/-/buffer-crc32-1.0.0.tgz#a10993b9055081d55304bd9feb4a072de179f405" integrity sha512-Db1SbgBS/fg/392AblrMJk97KggmvYhr4pB5ZIMTWtaivCPMWLkmb7m21cJvpvgK+J3nsU2CmmixNBZx4vFj/w== +buffer-crc32@~0.2.3: + version "0.2.13" + resolved "https://registry.yarnpkg.com/buffer-crc32/-/buffer-crc32-0.2.13.tgz#0d333e3f00eac50aa1454abd30ef8c2a5d9a7242" + integrity sha1-DTM+PwDqxQqhRUq9MO+MKl2ackI= + buffer-equal-constant-time@1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/buffer-equal-constant-time/-/buffer-equal-constant-time-1.0.1.tgz#f8e71132f7ffe6e01a5c9697a4c6f3e48d5cc819" @@ -17028,11 +17028,6 @@ es6-object-assign@^1.1.0: resolved "https://registry.yarnpkg.com/es6-object-assign/-/es6-object-assign-1.1.0.tgz#c2c3582656247c39ea107cb1e6652b6f9f24523c" integrity sha1-wsNYJlYkfDnqEHyx5mUrb58kUjw= -es6-promise@^3.1.2: - version "3.3.1" - resolved "https://registry.yarnpkg.com/es6-promise/-/es6-promise-3.3.1.tgz#a08cdde84ccdbf34d027a1451bc91d4bcd28a613" - integrity sha512-SOp9Phqvqn7jtEUxPWdWfWoLmyt2VaJ6MpvP9Comy1MceMXqE6bxvaTu4iaxpYYPzhny28Lc+M87/c2cPK6lDg== - esbuild-android-64@0.15.18: version "0.15.18" resolved "https://registry.yarnpkg.com/esbuild-android-64/-/esbuild-android-64-0.15.18.tgz#20a7ae1416c8eaade917fb2453c1259302c637a5" @@ -29334,7 +29329,7 @@ rfdc@^1.4.1: resolved "https://registry.yarnpkg.com/rfdc/-/rfdc-1.4.1.tgz#778f76c4fb731d93414e8f925fbecf64cce7f6ca" integrity sha512-q1b3N5QkRUWUl7iyylaaj3kOpIT0N2i9MqIEQXP73GVsN9cw3fdx8X63cEmWhJGi2PPCF23Ijp7ktmd39rawIA== -rimraf@^2.2.8, rimraf@^2.3.4, rimraf@^2.4.3, rimraf@^2.5.2, rimraf@^2.5.3, rimraf@^2.5.4, rimraf@^2.6.1, rimraf@^2.6.2, rimraf@^2.6.3: +rimraf@^2.2.8, rimraf@^2.3.4, rimraf@^2.4.3, rimraf@^2.5.3, rimraf@^2.5.4, rimraf@^2.6.1, rimraf@^2.6.2, rimraf@^2.6.3: version "2.7.1" resolved "https://registry.yarnpkg.com/rimraf/-/rimraf-2.7.1.tgz#35797f13a7fdadc566142c29d4f07ccad483e3ec" integrity sha512-uWjbaKIK3T1OSVptzX7Nl6PvQ3qAGtKEtVRjRuazjfL3Bx5eI409VZSqgND+4UNnmzLVdPj9FqFJNPqBZFve4w== @@ -29662,16 +29657,6 @@ safe-stable-stringify@^2.4.1: resolved "https://registry.yarnpkg.com/safer-buffer/-/safer-buffer-2.1.2.tgz#44fa161b0187b9549dd84bb91802f9bd8385cd6a" integrity sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg== -sander@^0.5.0: - version "0.5.1" - resolved "https://registry.yarnpkg.com/sander/-/sander-0.5.1.tgz#741e245e231f07cafb6fdf0f133adfa216a502ad" - integrity sha512-3lVqBir7WuKDHGrKRDn/1Ye3kwpXaDOMsiRP1wd6wpZW56gJhsbp5RqQpA6JG/P+pkXizygnr1dKR8vzWaVsfA== - dependencies: - es6-promise "^3.1.2" - graceful-fs "^4.1.3" - mkdirp "^0.5.1" - rimraf "^2.5.2" - sane@^4.0.0: version "4.1.0" resolved "https://registry.yarnpkg.com/sane/-/sane-4.1.0.tgz#ed881fd922733a6c461bc189dc2b6c006f3ffded" @@ -30446,15 +30431,14 @@ solid-use@^0.8.0: resolved "https://registry.yarnpkg.com/solid-use/-/solid-use-0.8.0.tgz#d46258c45edb0f4c621285e0ad1aa6b6a674d79b" integrity sha512-YX+XmcKLvSx3bwMimMhFy40ZkDnShnUcEw6cW6fSscwKEgl1TG3GlgAvkBmQ3AeWjvQSd8+HGTr82ImsrjkkqA== -sorcery@0.11.0: - version "0.11.0" - resolved "https://registry.yarnpkg.com/sorcery/-/sorcery-0.11.0.tgz#310c80ee993433854bb55bb9aa4003acd147fca8" - integrity sha512-J69LQ22xrQB1cIFJhPfgtLuI6BpWRiWu1Y3vSsIwK/eAScqJxd/+CJlUuHQRdX2C9NGFamq+KqNywGgaThwfHw== +sorcery@1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/sorcery/-/sorcery-1.0.0.tgz#b5bb81fb9706c0c240f5f2d3214b4d2be649e07f" + integrity sha512-5ay9oJE+7sNmhzl3YNG18jEEEf4AOQCM/FAqR5wMmzqd1FtRorFbJXn3w3SKOhbiQaVgHM+Q1lszZspjri7bpA== dependencies: "@jridgewell/sourcemap-codec" "^1.4.14" - buffer-crc32 "^0.2.5" minimist "^1.2.0" - sander "^0.5.0" + tiny-glob "^0.2.9" sort-keys@^2.0.0: version "2.0.0" From e8a016e041cd8859ea0c58d8ccecd348203a1452 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 13 Sep 2024 09:17:28 +0000 Subject: [PATCH 10/39] chore(deps): bump express from 4.19.2 to 4.20.0 in /dev-packages/e2e-tests/test-applications/node-express (#13685) --- .../e2e-tests/test-applications/node-express/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/node-express/package.json b/dev-packages/e2e-tests/test-applications/node-express/package.json index 34643b63553e..97947e70d06e 100644 --- a/dev-packages/e2e-tests/test-applications/node-express/package.json +++ b/dev-packages/e2e-tests/test-applications/node-express/package.json @@ -18,7 +18,7 @@ "@trpc/client": "10.45.2", "@types/express": "4.17.17", "@types/node": "18.15.1", - "express": "4.19.2", + "express": "4.20.0", "typescript": "4.9.5", "zod": "~3.22.4" }, From f44785cac6b393e5aba1ecb51f5c2a0ef5a91328 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 13 Sep 2024 11:21:12 +0200 Subject: [PATCH 11/39] build: Add codecov test analytics for playwright tests (#13654) https://docs.codecov.com/docs/test-result-ingestion-beta https://playwright.dev/docs/test-reporters Adds codecov test analytics to the repo, specifically for our playwright tests. This works by using the junit reporter with playwright, and then uploading that via the `codecov/test-results-action@v1` GitHub Action. --- .github/workflows/build.yml | 22 +++++++++++++++++++ .../browser-integration-tests/package.json | 2 +- .../playwright.config.ts | 2 ++ .../ember-classic/playwright.config.ts | 2 +- .../ember-embroider/playwright.config.ts | 2 +- .../playwright.config.mjs | 2 +- .../playwright.config.mjs | 2 +- .../test-utils/src/playwright-config.ts | 2 +- packages/remix/package.json | 2 +- packages/remix/playwright.config.ts | 1 + 10 files changed, 32 insertions(+), 7 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 38102ff204c7..c1ff3d4b8e9c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -610,6 +610,13 @@ jobs: overwrite: true retention-days: 7 + - name: Upload test results to Codecov + if: cancelled() == false + uses: codecov/test-results-action@v1 + with: + directory: dev-packages/browser-integration-tests + token: ${{ secrets.CODECOV_TOKEN }} + job_browser_loader_tests: name: PW ${{ matrix.bundle }} Tests needs: [job_get_metadata, job_build] @@ -653,6 +660,7 @@ jobs: run: | cd dev-packages/browser-integration-tests yarn test:loader + - name: Upload Playwright Traces uses: actions/upload-artifact@v4 if: failure() @@ -662,6 +670,13 @@ jobs: overwrite: true retention-days: 7 + - name: Upload test results to Codecov + if: cancelled() == false + uses: codecov/test-results-action@v1 + with: + directory: dev-packages/browser-integration-tests + token: ${{ secrets.CODECOV_TOKEN }} + job_check_for_faulty_dts: name: Check for faulty .d.ts files needs: [job_get_metadata, job_build] @@ -1013,6 +1028,13 @@ jobs: overwrite: true retention-days: 7 + - name: Upload test results to Codecov + if: cancelled() == false + uses: codecov/test-results-action@v1 + with: + directory: dev-packages/e2e-tests + token: ${{ secrets.CODECOV_TOKEN }} + job_optional_e2e_tests: name: E2E ${{ matrix.label || matrix.test-application }} Test # We only run E2E tests for non-fork PRs because the E2E tests require secrets to work and they can't be accessed from forks diff --git a/dev-packages/browser-integration-tests/package.json b/dev-packages/browser-integration-tests/package.json index 719dba1eece3..63edc3c9318f 100644 --- a/dev-packages/browser-integration-tests/package.json +++ b/dev-packages/browser-integration-tests/package.json @@ -35,7 +35,7 @@ "test:loader:replay_buffer": "PW_BUNDLE=loader_replay_buffer yarn test:loader", "test:loader:full": "PW_BUNDLE=loader_tracing_replay yarn test:loader", "test:loader:debug": "PW_BUNDLE=loader_debug yarn test:loader", - "test:ci": "yarn test:all --reporter='line'", + "test:ci": "yarn test:all", "test:update-snapshots": "yarn test:all --update-snapshots", "test:detect-flaky": "ts-node scripts/detectFlakyTests.ts" }, diff --git a/dev-packages/browser-integration-tests/playwright.config.ts b/dev-packages/browser-integration-tests/playwright.config.ts index 45a548311eef..498e7529f37a 100644 --- a/dev-packages/browser-integration-tests/playwright.config.ts +++ b/dev-packages/browser-integration-tests/playwright.config.ts @@ -30,6 +30,8 @@ const config: PlaywrightTestConfig = { }, ], + reporter: process.env.CI ? [['line'], ['junit', { outputFile: 'results.junit.xml' }]] : 'list', + globalSetup: require.resolve('./playwright.setup.ts'), globalTeardown: require.resolve('./playwright.teardown.ts'), }; diff --git a/dev-packages/e2e-tests/test-applications/ember-classic/playwright.config.ts b/dev-packages/e2e-tests/test-applications/ember-classic/playwright.config.ts index 6c2442587de4..a092503a9fc2 100644 --- a/dev-packages/e2e-tests/test-applications/ember-classic/playwright.config.ts +++ b/dev-packages/e2e-tests/test-applications/ember-classic/playwright.config.ts @@ -35,7 +35,7 @@ const config: PlaywrightTestConfig = { forbidOnly: !!process.env.CI, retries: 0, /* Reporter to use. See https://playwright.dev/docs/test-reporters */ - reporter: 'list', + reporter: process.env.CI ? [['line'], ['junit', { outputFile: 'results.junit.xml' }]] : 'list', /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ use: { /* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */ diff --git a/dev-packages/e2e-tests/test-applications/ember-embroider/playwright.config.ts b/dev-packages/e2e-tests/test-applications/ember-embroider/playwright.config.ts index 6c2442587de4..a092503a9fc2 100644 --- a/dev-packages/e2e-tests/test-applications/ember-embroider/playwright.config.ts +++ b/dev-packages/e2e-tests/test-applications/ember-embroider/playwright.config.ts @@ -35,7 +35,7 @@ const config: PlaywrightTestConfig = { forbidOnly: !!process.env.CI, retries: 0, /* Reporter to use. See https://playwright.dev/docs/test-reporters */ - reporter: 'list', + reporter: process.env.CI ? [['line'], ['junit', { outputFile: 'results.junit.xml' }]] : 'list', /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ use: { /* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */ diff --git a/dev-packages/e2e-tests/test-applications/node-express-send-to-sentry/playwright.config.mjs b/dev-packages/e2e-tests/test-applications/node-express-send-to-sentry/playwright.config.mjs index 9340e7e9436a..f29509db795c 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-send-to-sentry/playwright.config.mjs +++ b/dev-packages/e2e-tests/test-applications/node-express-send-to-sentry/playwright.config.mjs @@ -23,7 +23,7 @@ const config = { /* Retry on CI only */ retries: 0, /* Reporter to use. See https://playwright.dev/docs/test-reporters */ - reporter: 'list', + reporter: process.env.CI ? [['line'], ['junit', { outputFile: 'results.junit.xml' }]] : 'list', /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ use: { /* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */ diff --git a/dev-packages/e2e-tests/test-applications/react-send-to-sentry/playwright.config.mjs b/dev-packages/e2e-tests/test-applications/react-send-to-sentry/playwright.config.mjs index 7d04e3b6dd4b..aa8fc9bfd4b7 100644 --- a/dev-packages/e2e-tests/test-applications/react-send-to-sentry/playwright.config.mjs +++ b/dev-packages/e2e-tests/test-applications/react-send-to-sentry/playwright.config.mjs @@ -23,7 +23,7 @@ const config = { /* Opt out of parallel tests on CI. */ workers: 1, /* Reporter to use. See https://playwright.dev/docs/test-reporters */ - reporter: 'list', + reporter: process.env.CI ? [['line'], ['junit', { outputFile: 'results.junit.xml' }]] : 'list', /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ use: { /* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */ diff --git a/dev-packages/test-utils/src/playwright-config.ts b/dev-packages/test-utils/src/playwright-config.ts index d30c8cad4475..c380c9547ec0 100644 --- a/dev-packages/test-utils/src/playwright-config.ts +++ b/dev-packages/test-utils/src/playwright-config.ts @@ -37,7 +37,7 @@ export function getPlaywrightConfig( /* In dev mode some apps are flaky, so we allow retry there... */ retries: testEnv === 'development' ? 3 : 0, /* Reporter to use. See https://playwright.dev/docs/test-reporters */ - reporter: process.env.CI ? 'line' : 'list', + reporter: process.env.CI ? [['line'], ['junit', { outputFile: 'results.junit.xml' }]] : 'list', /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ use: { /* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */ diff --git a/packages/remix/package.json b/packages/remix/package.json index ada7991ead3c..c338d0df30a8 100644 --- a/packages/remix/package.json +++ b/packages/remix/package.json @@ -106,7 +106,7 @@ "test:integration:prepare": "(cd test/integration && yarn install)", "test:integration:clean": "(cd test/integration && rimraf .cache node_modules build)", "test:integration:client": "yarn playwright install-deps && yarn playwright test test/integration/test/client/ --project='chromium'", - "test:integration:client:ci": "yarn test:integration:client --reporter='line'", + "test:integration:client:ci": "yarn test:integration:client", "test:integration:server": "export NODE_OPTIONS='--stack-trace-limit=25' && vitest run", "test:unit": "jest", "test:watch": "jest --watch", diff --git a/packages/remix/playwright.config.ts b/packages/remix/playwright.config.ts index 72e9bd749a52..a1570f27f50d 100644 --- a/packages/remix/playwright.config.ts +++ b/packages/remix/playwright.config.ts @@ -8,6 +8,7 @@ const config: PlaywrightTestConfig = { }, // Run tests inside of a single file in parallel fullyParallel: true, + reporter: process.env.CI ? [['line'], ['junit', { outputFile: 'results.junit.xml' }]] : 'list', // Use 3 workers on CI, else use defaults (based on available CPU cores) // Note that 3 is a random number selected to work well with our CI setup workers: process.env.CI ? 3 : undefined, From 8dd3de137b5ffc71b75af894647a8fcd15036c3d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 13 Sep 2024 09:36:45 +0000 Subject: [PATCH 12/39] chore(deps): bump express from 4.19.2 to 4.20.0 in /dev-packages/e2e-tests/test-applications/node-express-incorrect-instrumentation (#13687) --- .../node-express-incorrect-instrumentation/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/node-express-incorrect-instrumentation/package.json b/dev-packages/e2e-tests/test-applications/node-express-incorrect-instrumentation/package.json index 3df947bb58c5..a8b325bc075d 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-incorrect-instrumentation/package.json +++ b/dev-packages/e2e-tests/test-applications/node-express-incorrect-instrumentation/package.json @@ -18,7 +18,7 @@ "@trpc/client": "10.45.2", "@types/express": "4.17.17", "@types/node": "18.15.1", - "express": "4.19.2", + "express": "4.20.0", "typescript": "4.9.5", "zod": "~3.22.4" }, From 0f122a153ee2ece925f41c850c233222748b1704 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Fri, 13 Sep 2024 11:56:55 +0200 Subject: [PATCH 13/39] fix(nestjs): Preserve original function name on `SentryTraced` functions (#13684) --- .../test-applications/nestjs-basic/src/app.controller.ts | 5 +++++ .../test-applications/nestjs-basic/src/app.service.ts | 5 +++++ .../nestjs-basic/tests/span-decorator.test.ts | 7 +++++++ packages/nestjs/src/decorators/sentry-traced.ts | 9 +++++++++ 4 files changed, 26 insertions(+) diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts index 75308e8f0ea9..77e25a72dad5 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts @@ -116,4 +116,9 @@ export class AppController { testServiceWithCanActivate() { return this.appService.canActivate(); } + + @Get('test-function-name') + testFunctionName() { + return this.appService.getFunctionName(); + } } diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.service.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.service.ts index 3e4639040a7e..72aef6947a6c 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.service.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.service.ts @@ -58,6 +58,11 @@ export class AppService { return { result: 'test' }; } + @SentryTraced('return the function name') + getFunctionName(): { result: string } { + return { result: this.getFunctionName.name }; + } + async testSpanDecoratorSync() { const returned = this.getString(); // Will fail if getString() is async, because returned will be a Promise<> diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/span-decorator.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/span-decorator.test.ts index 4b3ea2c0ba40..ee7666a50f18 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/span-decorator.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/span-decorator.test.ts @@ -70,3 +70,10 @@ test('Transaction includes span and correct value for decorated sync function', ]), ); }); + +test('preserves original function name on decorated functions', async ({ baseURL }) => { + const response = await fetch(`${baseURL}/test-function-name`); + const body = await response.json(); + + expect(body.result).toEqual('getFunctionName'); +}); diff --git a/packages/nestjs/src/decorators/sentry-traced.ts b/packages/nestjs/src/decorators/sentry-traced.ts index b9ef861bc3b2..2f90e4dab5d9 100644 --- a/packages/nestjs/src/decorators/sentry-traced.ts +++ b/packages/nestjs/src/decorators/sentry-traced.ts @@ -20,6 +20,15 @@ export function SentryTraced(op: string = 'function') { }, ); }; + + // preserve the original name on the decorated function + Object.defineProperty(descriptor.value, 'name', { + value: originalMethod.name, + configurable: true, + enumerable: true, + writable: true, + }); + return descriptor; }; } From cbb94758796b875c68a1a309fe2f1faf15c1a1a4 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 13 Sep 2024 12:52:43 +0200 Subject: [PATCH 14/39] feat(wasm): Unconditionally parse instruction addresses (#13655) Currently we are only setting the `platform` and `instruction_addr` for a wasm frame if we find a matching debug image. This is not always the case and unconditionally parsing the instruction address is almost always desirable. --- packages/wasm/src/index.ts | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/wasm/src/index.ts b/packages/wasm/src/index.ts index 88eb1915ce06..832a7e89b687 100644 --- a/packages/wasm/src/index.ts +++ b/packages/wasm/src/index.ts @@ -13,17 +13,18 @@ const _wasmIntegration = (() => { patchWebAssembly(); }, processEvent(event: Event): Event { - let haveWasm = false; + let hasAtLeastOneWasmFrameWithImage = false; if (event.exception && event.exception.values) { event.exception.values.forEach(exception => { if (exception.stacktrace && exception.stacktrace.frames) { - haveWasm = haveWasm || patchFrames(exception.stacktrace.frames); + hasAtLeastOneWasmFrameWithImage = + hasAtLeastOneWasmFrameWithImage || patchFrames(exception.stacktrace.frames); } }); } - if (haveWasm) { + if (hasAtLeastOneWasmFrameWithImage) { event.debug_meta = event.debug_meta || {}; event.debug_meta.images = [...(event.debug_meta.images || []), ...getImages()]; } @@ -37,10 +38,11 @@ export const wasmIntegration = defineIntegration(_wasmIntegration); /** * Patches a list of stackframes with wasm data needed for server-side symbolication - * if applicable. Returns true if any frames were patched. + * if applicable. Returns true if the provided list of stack frames had at least one + * matching registered image. */ function patchFrames(frames: Array): boolean { - let haveWasm = false; + let hasAtLeastOneWasmFrameWithImage = false; frames.forEach(frame => { if (!frame.filename) { return; @@ -50,14 +52,15 @@ function patchFrames(frames: Array): boolean { | [string, string, string]; if (match) { const index = getImage(match[1]); + frame.instruction_addr = match[2]; + frame.filename = match[1]; + frame.platform = 'native'; + if (index >= 0) { - frame.instruction_addr = match[2]; frame.addr_mode = `rel:${index}`; - frame.filename = match[1]; - frame.platform = 'native'; - haveWasm = true; + hasAtLeastOneWasmFrameWithImage = true; } } }); - return haveWasm; + return hasAtLeastOneWasmFrameWithImage; } From 7878d197e340e1fe4621a0b0a2f9b77d2867726f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 13 Sep 2024 11:11:52 +0000 Subject: [PATCH 15/39] chore(deps): bump express from 4.19.2 to 4.20.0 in /dev-packages/e2e-tests/test-applications/node-express-esm-without-loader (#13689) --- .../node-express-esm-without-loader/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/node-express-esm-without-loader/package.json b/dev-packages/e2e-tests/test-applications/node-express-esm-without-loader/package.json index 7939cf85a7ca..844ca51fd038 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-esm-without-loader/package.json +++ b/dev-packages/e2e-tests/test-applications/node-express-esm-without-loader/package.json @@ -11,7 +11,7 @@ "dependencies": { "@sentry/node": "latest || *", "@sentry/opentelemetry": "latest || *", - "express": "4.19.2" + "express": "4.20.0" }, "devDependencies": { "@playwright/test": "^1.44.1", From 17951da4166c7d75573f066407ed08b83077df37 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 13 Sep 2024 11:22:05 +0000 Subject: [PATCH 16/39] chore(deps): bump express from 4.19.2 to 4.20.0 in /dev-packages/e2e-tests/test-applications/node-express-esm-preload (#13691) --- .../test-applications/node-express-esm-preload/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/node-express-esm-preload/package.json b/dev-packages/e2e-tests/test-applications/node-express-esm-preload/package.json index df6fcaf29adc..03f483307290 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-esm-preload/package.json +++ b/dev-packages/e2e-tests/test-applications/node-express-esm-preload/package.json @@ -11,7 +11,7 @@ "dependencies": { "@sentry/node": "latest || *", "@sentry/opentelemetry": "latest || *", - "express": "4.19.2" + "express": "4.20.0" }, "devDependencies": { "@playwright/test": "^1.44.1", From 79efa636f053b82cce6748654e51996fa7e7784f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 13 Sep 2024 11:32:14 +0000 Subject: [PATCH 17/39] chore(deps): bump express from 4.19.2 to 4.20.0 in /dev-packages/e2e-tests/test-applications/node-express-esm-loader (#13692) --- .../test-applications/node-express-esm-loader/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/node-express-esm-loader/package.json b/dev-packages/e2e-tests/test-applications/node-express-esm-loader/package.json index 61fc40619560..6156211e27f8 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-esm-loader/package.json +++ b/dev-packages/e2e-tests/test-applications/node-express-esm-loader/package.json @@ -11,7 +11,7 @@ "dependencies": { "@sentry/node": "latest || *", "@sentry/opentelemetry": "latest || *", - "express": "4.19.2" + "express": "4.20.0" }, "devDependencies": { "@playwright/test": "^1.44.1", From 69e74271e3bce4d76460deef9352756ebe57b930 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 13 Sep 2024 11:42:21 +0000 Subject: [PATCH 18/39] chore(deps): bump express from 4.19.2 to 4.20.0 in /dev-packages/e2e-tests/test-applications/node-express-cjs-preload (#13694) --- .../test-applications/node-express-cjs-preload/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/node-express-cjs-preload/package.json b/dev-packages/e2e-tests/test-applications/node-express-cjs-preload/package.json index 5a3074df94eb..363c1e06636c 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-cjs-preload/package.json +++ b/dev-packages/e2e-tests/test-applications/node-express-cjs-preload/package.json @@ -11,7 +11,7 @@ "dependencies": { "@sentry/node": "latest || *", "@sentry/opentelemetry": "latest || *", - "express": "4.19.2" + "express": "4.20.0" }, "devDependencies": { "@playwright/test": "^1.44.1", From 5d0094a1d015566b11c408331418ae84c9b3a361 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 13 Sep 2024 14:06:34 +0200 Subject: [PATCH 19/39] fix: Ensure all logs are wrapped with `consoleSandbox` (#13690) To avoid infinite loops etc. --- packages/angular/src/errorhandler.ts | 4 +- .../integrations/local-variables/worker.ts | 3 +- packages/nuxt/src/module.ts | 11 +++-- packages/nuxt/src/vite/addServerConfig.ts | 45 +++++++++++-------- packages/sveltekit/src/server/handleError.ts | 3 +- 5 files changed, 40 insertions(+), 26 deletions(-) diff --git a/packages/angular/src/errorhandler.ts b/packages/angular/src/errorhandler.ts index 14ca380ea3ea..f1771ba81b7e 100644 --- a/packages/angular/src/errorhandler.ts +++ b/packages/angular/src/errorhandler.ts @@ -4,7 +4,7 @@ import { Inject, Injectable } from '@angular/core'; import * as Sentry from '@sentry/browser'; import type { ReportDialogOptions } from '@sentry/browser'; import type { Event } from '@sentry/types'; -import { isString } from '@sentry/utils'; +import { consoleSandbox, isString } from '@sentry/utils'; import { runOutsideAngular } from './zone'; @@ -119,7 +119,7 @@ class SentryErrorHandler implements AngularErrorHandler, OnDestroy { // When in development mode, log the error to console for immediate feedback. if (this._options.logErrors) { // eslint-disable-next-line no-console - console.error(extractedError); + consoleSandbox(() => console.error(extractedError)); } // Optionally show user dialog to provide details on what happened. diff --git a/packages/node/src/integrations/local-variables/worker.ts b/packages/node/src/integrations/local-variables/worker.ts index f0b3e20ce9b2..77299c0aff29 100644 --- a/packages/node/src/integrations/local-variables/worker.ts +++ b/packages/node/src/integrations/local-variables/worker.ts @@ -1,6 +1,7 @@ import type { Debugger, InspectorNotification, Runtime } from 'node:inspector'; import { Session } from 'node:inspector/promises'; import { workerData } from 'node:worker_threads'; +import { consoleSandbox } from '@sentry/utils'; import type { LocalVariablesWorkerArgs, PausedExceptionEvent, RateLimitIncrement, Variables } from './common'; import { LOCAL_VARIABLES_KEY } from './common'; import { createRateLimiter } from './common'; @@ -10,7 +11,7 @@ const options: LocalVariablesWorkerArgs = workerData; function log(...args: unknown[]): void { if (options.debug) { // eslint-disable-next-line no-console - console.log('[LocalVariables Worker]', ...args); + consoleSandbox(() => console.log('[LocalVariables Worker]', ...args)); } } diff --git a/packages/nuxt/src/module.ts b/packages/nuxt/src/module.ts index 0afdeaa03de7..f43f30d7e5ee 100644 --- a/packages/nuxt/src/module.ts +++ b/packages/nuxt/src/module.ts @@ -1,4 +1,5 @@ import { addPlugin, addPluginTemplate, addServerPlugin, createResolver, defineNuxtModule } from '@nuxt/kit'; +import { consoleSandbox } from '@sentry/utils'; import type { SentryNuxtModuleOptions } from './common/types'; import { addSentryTopImport, addServerConfigToBuild } from './vite/addServerConfig'; import { setupSourceMaps } from './vite/sourceMaps'; @@ -70,10 +71,12 @@ export default defineNuxtModule({ addSentryTopImport(moduleOptions, nuxt); } else { if (moduleOptions.debug) { - // eslint-disable-next-line no-console - console.log( - `[Sentry] Using your \`${serverConfigFile}\` file for the server-side Sentry configuration. In case you have a \`public/instrument.server\` file, the \`public/instrument.server\` file will be ignored. Make sure the file path in your node \`--import\` option matches the Sentry server config file in your \`.output\` folder and has a \`.mjs\` extension.`, - ); + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.log( + `[Sentry] Using your \`${serverConfigFile}\` file for the server-side Sentry configuration. In case you have a \`public/instrument.server\` file, the \`public/instrument.server\` file will be ignored. Make sure the file path in your node \`--import\` option matches the Sentry server config file in your \`.output\` folder and has a \`.mjs\` extension.`, + ); + }); } } } diff --git a/packages/nuxt/src/vite/addServerConfig.ts b/packages/nuxt/src/vite/addServerConfig.ts index dee15ee34dce..60f2452cde5b 100644 --- a/packages/nuxt/src/vite/addServerConfig.ts +++ b/packages/nuxt/src/vite/addServerConfig.ts @@ -1,6 +1,7 @@ import * as fs from 'fs'; import { createResolver } from '@nuxt/kit'; import type { Nuxt } from '@nuxt/schema'; +import { consoleSandbox } from '@sentry/utils'; import type { SentryNuxtModuleOptions } from '../common/types'; /** @@ -38,18 +39,22 @@ export function addServerConfigToBuild( await fs.promises.copyFile(source, destination); if (moduleOptions.debug) { - // eslint-disable-next-line no-console - console.log( - `[Sentry] Successfully added the content of the \`${serverConfigFile}\` file to \`${destination}\``, - ); + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.log( + `[Sentry] Successfully added the content of the \`${serverConfigFile}\` file to \`${destination}\``, + ); + }); } } catch (error) { if (moduleOptions.debug) { - // eslint-disable-next-line no-console - console.warn( - `[Sentry] An error occurred when trying to add the \`${serverConfigFile}\` file to the \`.output\` directory`, - error, - ); + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.warn( + `[Sentry] An error occurred when trying to add the \`${serverConfigFile}\` file to the \`.output\` directory`, + error, + ); + }); } } }); @@ -72,20 +77,24 @@ export function addSentryTopImport(moduleOptions: SentryNuxtModuleOptions, nuxt: fs.writeFile(entryFilePath, updatedContent, 'utf8', () => { if (moduleOptions.debug) { - // eslint-disable-next-line no-console - console.log( - `[Sentry] Successfully added the Sentry import to the server entry file "\`${entryFilePath}\`"`, - ); + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.log( + `[Sentry] Successfully added the Sentry import to the server entry file "\`${entryFilePath}\`"`, + ); + }); } }); }); } catch (err) { if (moduleOptions.debug) { - // eslint-disable-next-line no-console - console.warn( - `[Sentry] An error occurred when trying to add the Sentry import to the server entry file "\`${entryFilePath}\`":`, - err, - ); + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.warn( + `[Sentry] An error occurred when trying to add the Sentry import to the server entry file "\`${entryFilePath}\`":`, + err, + ); + }); } } }); diff --git a/packages/sveltekit/src/server/handleError.ts b/packages/sveltekit/src/server/handleError.ts index 1289e76a5ee2..f61251245c4d 100644 --- a/packages/sveltekit/src/server/handleError.ts +++ b/packages/sveltekit/src/server/handleError.ts @@ -1,4 +1,5 @@ import { captureException } from '@sentry/node'; +import { consoleSandbox } from '@sentry/utils'; import type { HandleServerError } from '@sveltejs/kit'; import { flushIfServerless } from './utils'; @@ -8,7 +9,7 @@ import { flushIfServerless } from './utils'; function defaultErrorHandler({ error }: Parameters[0]): ReturnType { // @ts-expect-error this conforms to the default implementation (including this ts-expect-error) // eslint-disable-next-line no-console - console.error(error && error.stack); + consoleSandbox(() => console.error(error && error.stack)); } type HandleServerErrorInput = Parameters[0]; From 214008317da9a088321fdf5ba7b4a82f7a653f75 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 13 Sep 2024 14:11:47 +0200 Subject: [PATCH 20/39] feat(browser): Add navigation `activationStart` timestamp to pageload span (#13658) Add the navigation performance entry `activationStart` property as a span attribute to the pageload span. The attribute is called `performance.activationStart` and it is measured in ms relative to performance.timeOrigin. --- .../suites/tracing/metrics/web-vitals/test.ts | 37 +++++++++++-------- .../tests/transactions.test.ts | 1 + .../src/metrics/browserMetrics.ts | 9 +++++ 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals/test.ts index 3ff09a2862c5..51a76797a23b 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals/test.ts @@ -59,23 +59,30 @@ sentryTest('paint web vitals values are greater than TTFB', async ({ browserName expect(fpValue).toBeGreaterThanOrEqual(ttfbValue!); }); -sentryTest('captures time origin as span attribute', async ({ getLocalTestPath, page }) => { - // Only run in chromium to ensure all vitals are present - if (shouldSkipTracingTest()) { - sentryTest.skip(); - } +sentryTest( + 'captures time origin and navigation activationStart as span attributes', + async ({ getLocalTestPath, page }) => { + // Only run in chromium to ensure all vitals are present + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } - const url = await getLocalTestPath({ testDir: __dirname }); - const [eventData] = await Promise.all([getFirstSentryEnvelopeRequest(page), page.goto(url)]); + const url = await getLocalTestPath({ testDir: __dirname }); + const [eventData] = await Promise.all([getFirstSentryEnvelopeRequest(page), page.goto(url)]); - const timeOriginAttribute = eventData.contexts?.trace?.data?.['performance.timeOrigin']; - const transactionStartTimestamp = eventData.start_timestamp; + const timeOriginAttribute = eventData.contexts?.trace?.data?.['performance.timeOrigin']; + const activationStart = eventData.contexts?.trace?.data?.['performance.activationStart']; - expect(timeOriginAttribute).toBeDefined(); - expect(transactionStartTimestamp).toBeDefined(); + const transactionStartTimestamp = eventData.start_timestamp; - const delta = Math.abs(transactionStartTimestamp! - timeOriginAttribute); + expect(timeOriginAttribute).toBeDefined(); + expect(transactionStartTimestamp).toBeDefined(); - // The delta should be less than 1ms if this flakes, we should increase the threshold - expect(delta).toBeLessThanOrEqual(1); -}); + const delta = Math.abs(transactionStartTimestamp! - timeOriginAttribute); + + // The delta should be less than 1ms if this flakes, we should increase the threshold + expect(delta).toBeLessThanOrEqual(1); + + expect(activationStart).toBeGreaterThanOrEqual(0); + }, +); diff --git a/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/transactions.test.ts index 861b6c420fbb..fa6b40c6a49d 100644 --- a/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/transactions.test.ts @@ -23,6 +23,7 @@ test('Captures a pageload transaction', async ({ page }) => { 'sentry.sample_rate': 1, 'sentry.source': 'route', 'performance.timeOrigin': expect.any(Number), + 'performance.activationStart': expect.any(Number), }, op: 'pageload', span_id: expect.any(String), diff --git a/packages/browser-utils/src/metrics/browserMetrics.ts b/packages/browser-utils/src/metrics/browserMetrics.ts index 066eba1e6839..92fe66a832ee 100644 --- a/packages/browser-utils/src/metrics/browserMetrics.ts +++ b/packages/browser-utils/src/metrics/browserMetrics.ts @@ -17,6 +17,7 @@ import { addTtfbInstrumentationHandler, } from './instrument'; import { getBrowserPerformanceAPI, isMeasurementValue, msToSec, startAndEndSpan } from './utils'; +import { getActivationStart } from './web-vitals/lib/getActivationStart'; import { getNavigationEntry } from './web-vitals/lib/getNavigationEntry'; import { getVisibilityWatcher } from './web-vitals/lib/getVisibilityWatcher'; @@ -383,6 +384,14 @@ export function addPerformanceEntries(span: Span, options: AddPerformanceEntries // Set timeOrigin which denotes the timestamp which to base the LCP/FCP/FP/TTFB measurements on span.setAttribute('performance.timeOrigin', timeOrigin); + // In prerendering scenarios, where a page might be prefetched and pre-rendered before the user clicks the link, + // the navigation starts earlier than when the user clicks it. Web Vitals should always be based on the + // user-perceived time, so they are not reported from the actual start of the navigation, but rather from the + // time where the user actively started the navigation, for example by clicking a link. + // This is user action is called "activation" and the time between navigation and activation is stored in + // the `activationStart` attribute of the "navigation" PerformanceEntry. + span.setAttribute('performance.activationStart', getActivationStart()); + _setWebVitalAttributes(span); } From 8d2e1890b488544195bde899abc3d3ceaa722514 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 13 Sep 2024 14:12:41 +0200 Subject: [PATCH 21/39] chore: Add cloudflare sdk to main README (#13688) Co-authored-by: IEatBeans <75859301+KyGuy2002@users.noreply.github.com> --- CHANGELOG.md | 2 ++ README.md | 2 ++ 2 files changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bf504ce80b8..45e39dcaec80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +Work in this release was contributed by @KyGuy2002. Thank you for your contribution! + ## 8.30.0 ### Important Changes diff --git a/README.md b/README.md index 3309f0521986..be275766a156 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,8 @@ package. Please refer to the README and instructions of those SDKs for more deta for native crashes - [`@sentry/bun`](https://github.com/getsentry/sentry-javascript/tree/master/packages/bun): SDK for Bun - [`@sentry/deno`](https://github.com/getsentry/sentry-javascript/tree/master/packages/deno): SDK for Deno +- [`@sentry/cloudflare`](https://github.com/getsentry/sentry-javascript/tree/master/packages/cloudflare): SDK for + Cloudflare ## Version Support Policy From ee0b5b5e266d1cdd9c4ed144ffc4f4f965db6fdf Mon Sep 17 00:00:00 2001 From: Jonas Date: Mon, 16 Sep 2024 10:50:21 -0400 Subject: [PATCH 22/39] ref(profiling) use cleanup instead of destructor (#13661) I think what is happening is that by the time our close handler is called, the handler had already been deleted, which causes a use after free error. I also updated the code to check for is_closing which is what that underlying libuv method actually asserts on. Fixes https://github.com/getsentry/sentry-javascript/pull/13661 --- .../profiling-node/bindings/cpu_profiler.cc | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/packages/profiling-node/bindings/cpu_profiler.cc b/packages/profiling-node/bindings/cpu_profiler.cc index 00996db9e8c9..ad3ca8079e00 100644 --- a/packages/profiling-node/bindings/cpu_profiler.cc +++ b/packages/profiling-node/bindings/cpu_profiler.cc @@ -73,7 +73,7 @@ enum class ProfileStatus { class MeasurementsTicker { private: - uv_timer_t timer; + uv_timer_t *timer; uint64_t period_ms; std::unordered_map> @@ -87,9 +87,10 @@ class MeasurementsTicker { public: MeasurementsTicker(uv_loop_t *loop) : period_ms(100), isolate(v8::Isolate::GetCurrent()) { - uv_timer_init(loop, &timer); - uv_handle_set_data(reinterpret_cast(&timer), this); - uv_unref(reinterpret_cast(&timer)); + timer = new uv_timer_t; + uv_timer_init(loop, timer); + uv_handle_set_data((uv_handle_t *)timer, this); + uv_ref((uv_handle_t *)timer); } static void ticker(uv_timer_t *); @@ -112,13 +113,13 @@ class MeasurementsTicker { size_t listener_count(); ~MeasurementsTicker() { - uv_timer_stop(&timer); + uv_handle_t *handle = (uv_handle_t *)timer; - auto handle = reinterpret_cast(&timer); + uv_timer_stop(timer); + uv_unref(handle); - // Calling uv_close on an inactive handle will cause a segfault. - if (uv_is_active(handle)) { - uv_close(handle, nullptr); + if (!uv_is_closing(handle)) { + uv_close(handle, [](uv_handle_t *handle) { delete handle; }); } } }; @@ -143,8 +144,8 @@ void MeasurementsTicker::add_heap_listener( heap_listeners.emplace(profile_id, cb); if (listener_count() == 1) { - uv_timer_set_repeat(&timer, period_ms); - uv_timer_start(&timer, ticker, 0, period_ms); + uv_timer_set_repeat(timer, period_ms); + uv_timer_start(timer, ticker, 0, period_ms); } } @@ -154,7 +155,7 @@ void MeasurementsTicker::remove_heap_listener( heap_listeners.erase(profile_id); if (listener_count() == 0) { - uv_timer_stop(&timer); + uv_timer_stop(timer); } }; @@ -223,8 +224,8 @@ void MeasurementsTicker::add_cpu_listener( cpu_listeners.emplace(profile_id, cb); if (listener_count() == 1) { - uv_timer_set_repeat(&timer, period_ms); - uv_timer_start(&timer, ticker, 0, period_ms); + uv_timer_set_repeat(timer, period_ms); + uv_timer_start(timer, ticker, 0, period_ms); } } @@ -233,7 +234,7 @@ void MeasurementsTicker::remove_cpu_listener( cpu_listeners.erase(profile_id); if (listener_count() == 0) { - uv_timer_stop(&timer); + uv_timer_stop(timer); } }; From afa79b68640caf7ea3f3bc91c584e92225a49bc8 Mon Sep 17 00:00:00 2001 From: Jonas Date: Mon, 16 Sep 2024 11:00:02 -0400 Subject: [PATCH 23/39] ref(profiling): reinitialize profilerId on explicit stop calls (#13681) Modifies the functionality of profiler start and stop calls to regenerate the profilerId --- packages/profiling-node/src/integration.ts | 107 +++++++++++----- .../test/spanProfileUtils.test.ts | 119 +++++++++--------- 2 files changed, 135 insertions(+), 91 deletions(-) diff --git a/packages/profiling-node/src/integration.ts b/packages/profiling-node/src/integration.ts index c1a96015f0c4..2eb0a59142ae 100644 --- a/packages/profiling-node/src/integration.ts +++ b/packages/profiling-node/src/integration.ts @@ -17,7 +17,7 @@ import { CpuProfilerBindings } from './cpu_profiler'; import { DEBUG_BUILD } from './debug-build'; import { NODE_MAJOR, NODE_VERSION } from './nodeVersion'; import { MAX_PROFILE_DURATION_MS, maybeProfileSpan, stopSpanProfile } from './spanProfileUtils'; -import type { RawChunkCpuProfile, RawThreadCpuProfile } from './types'; +import type { RawThreadCpuProfile } from './types'; import { ProfileFormat } from './types'; import { PROFILER_THREAD_NAME } from './utils'; @@ -161,7 +161,7 @@ interface ChunkData { } class ContinuousProfiler { - private _profilerId = uuid4(); + private _profilerId: string | undefined; private _client: NodeClient | undefined = undefined; private _chunkData: ChunkData | undefined = undefined; @@ -172,15 +172,48 @@ class ContinuousProfiler { */ public initialize(client: NodeClient): void { this._client = client; + + // Attaches a listener to beforeSend which will add the threadId data to the event being sent. + // This adds a constant overhead to all events being sent which could be improved to only attach + // and detach the listener during a profiler session + this._client.on('beforeSendEvent', this._onBeforeSendThreadContextAssignment.bind(this)); } /** - * Recursively schedules chunk profiling to start and stop at a set interval. - * Once the user calls stop(), the current chunk will be stopped and flushed to Sentry and no new chunks will - * will be started. To restart continuous mode after calling stop(), the user must call start() again. + * Initializes a new profilerId session and schedules chunk profiling. * @returns void */ public start(): void { + if (!this._client) { + DEBUG_BUILD && logger.log('[Profiling] Failed to start, sentry client was never attached to the profiler.'); + return; + } + + // Flush any existing chunks before starting a new one. + this._chunkStop(); + + // Restart the profiler session + this._setupSpanChunkInstrumentation(); + this._chunkStart(); + } + + /** + * Stops the current chunk and flushes the profile to Sentry. + * @returns void + */ + public stop(): void { + if (!this._client) { + DEBUG_BUILD && logger.log('[Profiling] Failed to stop, sentry client was never attached to the profiler.'); + return; + } + this._chunkStop(); + this._teardownSpanChunkInstrumentation(); + } + + /** + * Stop profiler and initializes profiling of the next chunk + */ + private _chunkStart(): void { if (!this._client) { // The client is not attached to the profiler if the user has not enabled continuous profiling. // In this case, calling start() and stop() is a noop action.The reason this exists is because @@ -193,20 +226,16 @@ class ContinuousProfiler { logger.log( `[Profiling] Chunk with chunk_id ${this._chunkData.id} is still running, current chunk will be stopped a new chunk will be started.`, ); - this.stop(); + this._chunkStop(); } - const traceId = - getCurrentScope().getPropagationContext().traceId || getIsolationScope().getPropagationContext().traceId; - this._initializeChunk(traceId); - this._startChunkProfiling(this._chunkData!); + this._startChunkProfiling(); } /** - * Stops the current chunk and flushes the profile to Sentry. - * @returns void + * Stops profiling of the current chunks and flushes the profile to Sentry */ - public stop(): void { + private _chunkStop(): void { if (this._chunkData?.timer) { global.clearTimeout(this._chunkData.timer); this._chunkData.timer = undefined; @@ -223,12 +252,17 @@ class ContinuousProfiler { return; } - const profile = this._stopChunkProfiling(this._chunkData); + const profile = CpuProfilerBindings.stopProfiling(this._chunkData.id, ProfileFormat.CHUNK); if (!profile) { DEBUG_BUILD && logger.log(`[Profiling] _chunkiledStartTraceID to collect profile for: ${this._chunkData.id}`); return; } + if (!this._profilerId) { + DEBUG_BUILD && + logger.log('[Profiling] Profile chunk does not contain a valid profiler_id, this is a bug in the SDK'); + return; + } if (profile) { DEBUG_BUILD && logger.log(`[Profiling] Sending profile chunk ${this._chunkData.id}.`); } @@ -248,7 +282,7 @@ class ContinuousProfiler { if (!chunk) { DEBUG_BUILD && logger.log(`[Profiling] Failed to create profile chunk for: ${this._chunkData.id}`); - this._reset(); + this._resetChunkData(); return; } @@ -257,7 +291,7 @@ class ContinuousProfiler { // the format may negatively impact the performance of the application. To avoid // blocking for too long, enqueue the next chunk start inside the next macrotask. // clear current chunk - this._reset(); + this._resetChunkData(); } /** @@ -287,29 +321,23 @@ class ContinuousProfiler { }); } - /** - * Stops the profile and clears chunk instrumentation from global scope - * @returns void - */ - private _stopChunkProfiling(chunk: ChunkData): RawChunkCpuProfile | null { - this._teardownSpanChunkInstrumentation(); - return CpuProfilerBindings.stopProfiling(chunk.id, ProfileFormat.CHUNK); - } - /** * Starts the profiler and registers the flush timer for a given chunk. * @param chunk */ - private _startChunkProfiling(chunk: ChunkData): void { - this._setupSpanChunkInstrumentation(); + private _startChunkProfiling(): void { + const traceId = + getCurrentScope().getPropagationContext().traceId || getIsolationScope().getPropagationContext().traceId; + const chunk = this._initializeChunk(traceId); + CpuProfilerBindings.startProfiling(chunk.id); DEBUG_BUILD && logger.log(`[Profiling] starting profiling chunk: ${chunk.id}`); chunk.timer = global.setTimeout(() => { DEBUG_BUILD && logger.log(`[Profiling] Stopping profiling chunk: ${chunk.id}`); - this.stop(); + this._chunkStop(); DEBUG_BUILD && logger.log('[Profiling] Starting new profiling chunk.'); - setImmediate(this.start.bind(this)); + setImmediate(this._chunkStart.bind(this)); }, CHUNK_INTERVAL_MS); // Unref timeout so it doesn't keep the process alive. @@ -323,21 +351,31 @@ class ContinuousProfiler { private _setupSpanChunkInstrumentation(): void { if (!this._client) { DEBUG_BUILD && - logger.log('[Profiling] Failed to collect profile, sentry client was never attached to the profiler.'); + logger.log( + '[Profiling] Failed to initialize span profiling, sentry client was never attached to the profiler.', + ); return; } + this._profilerId = uuid4(); getGlobalScope().setContext('profile', { profiler_id: this._profilerId, }); + } - this._client.on('beforeSendEvent', e => this._assignThreadIdContext(e)); + /** + * Assigns thread_id and thread name context to a profiled event if there is an active profiler session + */ + private _onBeforeSendThreadContextAssignment(event: Event): void { + if (!this._client || !this._profilerId) return; + this._assignThreadIdContext(event); } /** * Clear profiling information from global context when a profile is not running. */ private _teardownSpanChunkInstrumentation(): void { + this._profilerId = undefined; const globalScope = getGlobalScope(); globalScope.setContext('profile', {}); } @@ -345,18 +383,19 @@ class ContinuousProfiler { /** * Initializes new profile chunk metadata */ - private _initializeChunk(traceId: string): void { + private _initializeChunk(traceId: string): ChunkData { this._chunkData = { id: uuid4(), startTraceID: traceId, timer: undefined, }; + return this._chunkData; } /** * Assigns thread_id and thread name context to a profiled event. */ - private _assignThreadIdContext(event: Event): any { + private _assignThreadIdContext(event: Event): void { if (!event?.['contexts']?.['profile']) { return; } @@ -380,7 +419,7 @@ class ContinuousProfiler { /** * Resets the current chunk state. */ - private _reset(): void { + private _resetChunkData(): void { this._chunkData = undefined; } } diff --git a/packages/profiling-node/test/spanProfileUtils.test.ts b/packages/profiling-node/test/spanProfileUtils.test.ts index f65556f57ab4..fd2c95ec79e4 100644 --- a/packages/profiling-node/test/spanProfileUtils.test.ts +++ b/packages/profiling-node/test/spanProfileUtils.test.ts @@ -52,6 +52,12 @@ function makeContinuousProfilingClient(): [Sentry.NodeClient, Transport] { return [client, client.getTransport() as Transport]; } +function getProfilerId(): string { + return ( + Sentry.getClient()?.getIntegrationByName>('ProfilingIntegration') as any + )?._profiler?._profilerId; +} + function makeClientOptions( options: Omit, ): NodeClientOptions { @@ -394,7 +400,7 @@ describe('continuous profiling', () => { const integration = client?.getIntegrationByName>('ProfilingIntegration'); if (integration) { - integration._profiler.stop(); + Sentry.profiler.stopProfiler(); } jest.clearAllMocks(); @@ -432,14 +438,9 @@ describe('continuous profiling', () => { client.init(); const transportSpy = jest.spyOn(transport, 'send').mockReturnValue(Promise.resolve({})); - - const integration = client.getIntegrationByName>('ProfilingIntegration'); - if (!integration) { - throw new Error('Profiling integration not found'); - } - integration._profiler.start(); + Sentry.profiler.startProfiler(); jest.advanceTimersByTime(1000); - integration._profiler.stop(); + Sentry.profiler.stopProfiler(); jest.advanceTimersByTime(1000); const profile = transportSpy.mock.calls?.[0]?.[0]?.[1]?.[0]?.[1] as ProfileChunk; @@ -455,14 +456,10 @@ describe('continuous profiling', () => { client.init(); expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); + Sentry.profiler.startProfiler(); const integration = client.getIntegrationByName>('ProfilingIntegration'); - if (!integration) { - throw new Error('Profiling integration not found'); - } - integration._profiler.start(); - - expect(integration._profiler).toBeDefined(); + expect(integration?._profiler).toBeDefined(); }); it('starts a continuous profile', () => { @@ -473,11 +470,7 @@ describe('continuous profiling', () => { client.init(); expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); - const integration = client.getIntegrationByName>('ProfilingIntegration'); - if (!integration) { - throw new Error('Profiling integration not found'); - } - integration._profiler.start(); + Sentry.profiler.startProfiler(); expect(startProfilingSpy).toHaveBeenCalledTimes(1); }); @@ -490,12 +483,9 @@ describe('continuous profiling', () => { client.init(); expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); - const integration = client.getIntegrationByName>('ProfilingIntegration'); - if (!integration) { - throw new Error('Profiling integration not found'); - } - integration._profiler.start(); - integration._profiler.start(); + Sentry.profiler.startProfiler(); + Sentry.profiler.startProfiler(); + expect(startProfilingSpy).toHaveBeenCalledTimes(2); expect(stopProfilingSpy).toHaveBeenCalledTimes(1); }); @@ -509,15 +499,46 @@ describe('continuous profiling', () => { client.init(); expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); - const integration = client.getIntegrationByName>('ProfilingIntegration'); - if (!integration) { - throw new Error('Profiling integration not found'); - } - integration._profiler.start(); + Sentry.profiler.startProfiler(); + + jest.advanceTimersByTime(5001); + expect(stopProfilingSpy).toHaveBeenCalledTimes(1); + expect(startProfilingSpy).toHaveBeenCalledTimes(2); + }); + + it('chunks share the same profilerId', async () => { + const startProfilingSpy = jest.spyOn(CpuProfilerBindings, 'startProfiling'); + const stopProfilingSpy = jest.spyOn(CpuProfilerBindings, 'stopProfiling'); + + const [client] = makeContinuousProfilingClient(); + Sentry.setCurrentClient(client); + client.init(); + + expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); + Sentry.profiler.startProfiler(); + const profilerId = getProfilerId(); jest.advanceTimersByTime(5001); expect(stopProfilingSpy).toHaveBeenCalledTimes(1); expect(startProfilingSpy).toHaveBeenCalledTimes(2); + expect(getProfilerId()).toBe(profilerId); + }); + + it('explicit calls to stop clear profilerId', async () => { + const startProfilingSpy = jest.spyOn(CpuProfilerBindings, 'startProfiling'); + + const [client] = makeContinuousProfilingClient(); + Sentry.setCurrentClient(client); + client.init(); + + expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); + Sentry.profiler.startProfiler(); + const profilerId = getProfilerId(); + Sentry.profiler.stopProfiler(); + Sentry.profiler.startProfiler(); + + expect(getProfilerId()).toEqual(expect.any(String)); + expect(getProfilerId()).not.toBe(profilerId); }); it('stops a continuous profile after interval', async () => { @@ -529,11 +550,7 @@ describe('continuous profiling', () => { client.init(); expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); - const integration = client.getIntegrationByName>('ProfilingIntegration'); - if (!integration) { - throw new Error('Profiling integration not found'); - } - integration._profiler.start(); + Sentry.profiler.startProfiler(); jest.advanceTimersByTime(5001); expect(stopProfilingSpy).toHaveBeenCalledTimes(1); @@ -548,15 +565,11 @@ describe('continuous profiling', () => { client.init(); expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); - const integration = client.getIntegrationByName>('ProfilingIntegration'); - if (!integration) { - throw new Error('Profiling integration not found'); - } - integration._profiler.start(); + Sentry.profiler.startProfiler(); jest.advanceTimersByTime(1000); - integration._profiler.stop(); + Sentry.profiler.stopProfiler(); expect(stopProfilingSpy).toHaveBeenCalledTimes(1); jest.advanceTimersByTime(1000); @@ -603,14 +616,9 @@ describe('continuous profiling', () => { client.init(); const transportSpy = jest.spyOn(transport, 'send').mockReturnValue(Promise.resolve({})); - - const integration = client.getIntegrationByName>('ProfilingIntegration'); - if (!integration) { - throw new Error('Profiling integration not found'); - } - integration._profiler.start(); + Sentry.profiler.startProfiler(); jest.advanceTimersByTime(1000); - integration._profiler.stop(); + Sentry.profiler.stopProfiler(); jest.advanceTimersByTime(1000); expect(transportSpy.mock.calls?.[0]?.[0]?.[1]?.[0]?.[0]?.type).toBe('profile_chunk'); @@ -640,7 +648,7 @@ describe('continuous profiling', () => { integration._profiler.start(); const profiledTransaction = Sentry.startInactiveSpan({ forceTransaction: true, name: 'profile_hub' }); profiledTransaction.end(); - integration._profiler.stop(); + Sentry.profiler.stopProfiler(); expect(transportSpy.mock.calls?.[1]?.[0]?.[1]?.[0]?.[1]).toMatchObject({ contexts: { @@ -658,7 +666,7 @@ describe('continuous profiling', () => { }); }); -describe('span profiling mode', () => { +describe('continuous profiling does not start in span profiling mode', () => { it.each([ ['profilesSampleRate=1', makeClientOptions({ profilesSampleRate: 1 })], ['profilesSampler is defined', makeClientOptions({ profilesSampler: () => 1 })], @@ -699,7 +707,9 @@ describe('span profiling mode', () => { } integration._profiler.start(); - expect(logSpy).toHaveBeenLastCalledWith('[Profiling] Profiler was never attached to the client.'); + expect(logSpy).toHaveBeenLastCalledWith( + '[Profiling] Failed to start, sentry client was never attached to the profiler.', + ); }); }); describe('continuous profiling mode', () => { @@ -742,12 +752,7 @@ describe('continuous profiling mode', () => { } jest.spyOn(transport, 'send').mockReturnValue(Promise.resolve({})); - - const integration = client.getIntegrationByName>('ProfilingIntegration'); - if (!integration) { - throw new Error('Profiling integration not found'); - } - integration._profiler.start(); + Sentry.profiler.startProfiler(); const callCount = startProfilingSpy.mock.calls.length; expect(startProfilingSpy).toHaveBeenCalled(); From e31fd637c70b49eb1e08ac29e15b9b9d6e458730 Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Tue, 17 Sep 2024 11:27:10 -0400 Subject: [PATCH 24/39] fix(feedback): Actor color applies to feedback icon (#13702) Fixes https://github.com/getsentry/sentry-javascript/issues/13513 --- packages/feedback/src/core/components/FeedbackIcon.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/feedback/src/core/components/FeedbackIcon.ts b/packages/feedback/src/core/components/FeedbackIcon.ts index d4ca23d9349a..cb025bdb5546 100644 --- a/packages/feedback/src/core/components/FeedbackIcon.ts +++ b/packages/feedback/src/core/components/FeedbackIcon.ts @@ -14,7 +14,7 @@ export function FeedbackIcon(): SVGElement { width: `${SIZE}`, height: `${SIZE}`, viewBox: `0 0 ${SIZE} ${SIZE}`, - fill: 'var(--foreground)', + fill: 'var(--actor-color, var(--foreground))', }); const g = setAttributesNS(createElementNS('g'), { From e0015c51438afa797806de2d7136a5a2bbd4938b Mon Sep 17 00:00:00 2001 From: Artem Zhukov Date: Tue, 17 Sep 2024 22:26:28 +0300 Subject: [PATCH 25/39] fix(feedback): Fix form width on mobile devices (#13068) On mobile devices, the form looked narrow due to the unspecified value of the css variable --form-width. I set it to 100% for mobile devices and this stretched the form to the full width (see before/after screenshots). before: before after: after --------- Co-authored-by: Catherine Lee <55311782+c298lee@users.noreply.github.com> --- packages/feedback/src/modal/components/Dialog.css.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/feedback/src/modal/components/Dialog.css.ts b/packages/feedback/src/modal/components/Dialog.css.ts index 576044578247..fc0e00580a5d 100644 --- a/packages/feedback/src/modal/components/Dialog.css.ts +++ b/packages/feedback/src/modal/components/Dialog.css.ts @@ -100,7 +100,7 @@ const FORM = ` } .form__right { - flex: 0 0 var(--form-width, 272px); + flex: 0 0 auto; width: var(--form-width, 272px); display: flex; overflow: auto; @@ -111,7 +111,7 @@ const FORM = ` @media (max-width: 600px) { .form__right { - width: auto; + width: var(--form-width, 100%); } } From b09a679aa9c72b4c2fbf43cb57c1841e59a2359d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 18 Sep 2024 09:08:33 +0200 Subject: [PATCH 26/39] chore(deps-dev): bump vite from 4.5.3 to 4.5.5 in /packages/astro (#13712) Bumps [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite) from 4.5.3 to 4.5.5.
Changelog

Sourced from vite's changelog.

4.5.5 (2024-09-16)

4.5.4 (2024-09-16)

Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=vite&package-manager=npm_and_yarn&previous-version=4.5.3&new-version=4.5.5)](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) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/getsentry/sentry-javascript/network/alerts).
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- packages/astro/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/astro/package.json b/packages/astro/package.json index 42792e7554e7..069bffbf8f97 100644 --- a/packages/astro/package.json +++ b/packages/astro/package.json @@ -65,7 +65,7 @@ }, "devDependencies": { "astro": "^3.5.0", - "vite": "4.5.3" + "vite": "4.5.5" }, "scripts": { "build": "run-p build:transpile build:types", From 18c9ab6913711a96634d33d1edc1ac25e2a56a2e Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 18 Sep 2024 09:31:12 +0200 Subject: [PATCH 27/39] feat(node): Add `disableInstrumentationWarnings` option (#13693) Closes https://github.com/getsentry/sentry-javascript/issues/13471 This can be used when you know what you're doing to avoid the warning log. --- packages/node/src/types.ts | 6 ++ packages/node/src/utils/ensureIsWrapped.ts | 13 +++- .../node/test/utils/ensureIsWrapped.test.ts | 71 +++++++++++++++++++ 3 files changed, 87 insertions(+), 3 deletions(-) create mode 100644 packages/node/test/utils/ensureIsWrapped.test.ts diff --git a/packages/node/src/types.ts b/packages/node/src/types.ts index aa9873e2da91..f6b08a394f8c 100644 --- a/packages/node/src/types.ts +++ b/packages/node/src/types.ts @@ -125,6 +125,12 @@ export interface BaseNodeOptions { */ clientReportFlushInterval?: number; + /** + * By default, the SDK will try to identify problems with your instrumentation setup and warn you about it. + * If you want to disable these warnings, set this to `true`. + */ + disableInstrumentationWarnings?: boolean; + /** Callback that is executed when a fatal global error occurs. */ onFatalError?(this: void, error: Error): void; } diff --git a/packages/node/src/utils/ensureIsWrapped.ts b/packages/node/src/utils/ensureIsWrapped.ts index 05185a293bed..a11c60f949d5 100644 --- a/packages/node/src/utils/ensureIsWrapped.ts +++ b/packages/node/src/utils/ensureIsWrapped.ts @@ -1,6 +1,7 @@ import { isWrapped } from '@opentelemetry/core'; -import { getGlobalScope, hasTracingEnabled, isEnabled } from '@sentry/core'; +import { getClient, getGlobalScope, hasTracingEnabled, isEnabled } from '@sentry/core'; import { consoleSandbox } from '@sentry/utils'; +import type { NodeClient } from '../sdk/client'; import { isCjs } from './commonjs'; import { createMissingInstrumentationContext } from './createMissingInstrumentationContext'; @@ -8,10 +9,16 @@ import { createMissingInstrumentationContext } from './createMissingInstrumentat * Checks and warns if a framework isn't wrapped by opentelemetry. */ export function ensureIsWrapped( - maybeWrappedModule: unknown, + maybeWrappedFunction: unknown, name: 'express' | 'connect' | 'fastify' | 'hapi' | 'koa', ): void { - if (!isWrapped(maybeWrappedModule) && isEnabled() && hasTracingEnabled()) { + const client = getClient(); + if ( + !client?.getOptions().disableInstrumentationWarnings && + !isWrapped(maybeWrappedFunction) && + isEnabled() && + hasTracingEnabled() + ) { consoleSandbox(() => { if (isCjs()) { // eslint-disable-next-line no-console diff --git a/packages/node/test/utils/ensureIsWrapped.test.ts b/packages/node/test/utils/ensureIsWrapped.test.ts new file mode 100644 index 000000000000..c328545315ce --- /dev/null +++ b/packages/node/test/utils/ensureIsWrapped.test.ts @@ -0,0 +1,71 @@ +import { ensureIsWrapped } from '../../src/utils/ensureIsWrapped'; +import { cleanupOtel, mockSdkInit, resetGlobals } from '../helpers/mockSdkInit'; + +const unwrappedFunction = () => {}; + +// We simulate a wrapped function +const wrappedfunction = Object.assign(() => {}, { + __wrapped: true, + __original: () => {}, + __unwrap: () => {}, +}); + +describe('ensureIsWrapped', () => { + afterEach(() => { + jest.restoreAllMocks(); + cleanupOtel(); + resetGlobals(); + }); + + it('warns when the method is unwrapped', () => { + const spyWarn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + + mockSdkInit({ tracesSampleRate: 1 }); + + ensureIsWrapped(unwrappedFunction, 'express'); + + expect(spyWarn).toHaveBeenCalledTimes(1); + expect(spyWarn).toHaveBeenCalledWith( + '[Sentry] express is not instrumented. This is likely because you required/imported express before calling `Sentry.init()`.', + ); + }); + + it('does not warn when the method is wrapped', () => { + const spyWarn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + + mockSdkInit({ tracesSampleRate: 1 }); + + ensureIsWrapped(wrappedfunction, 'express'); + + expect(spyWarn).toHaveBeenCalledTimes(0); + }); + + it('does not warn without a client', () => { + const spyWarn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + resetGlobals(); + + ensureIsWrapped(wrappedfunction, 'express'); + + expect(spyWarn).toHaveBeenCalledTimes(0); + }); + + it('does not warn without tracing', () => { + const spyWarn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + + mockSdkInit({}); + + ensureIsWrapped(unwrappedFunction, 'express'); + + expect(spyWarn).toHaveBeenCalledTimes(0); + }); + + it('does not warn if disableInstrumentationWarnings=true', () => { + const spyWarn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + + mockSdkInit({ tracesSampleRate: 1, disableInstrumentationWarnings: true }); + + ensureIsWrapped(unwrappedFunction, 'express'); + + expect(spyWarn).toHaveBeenCalledTimes(0); + }); +}); From e1783a653fafda6df9eb55e1eaf61e113f5df3db Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 18 Sep 2024 07:39:54 +0000 Subject: [PATCH 28/39] ref: Add external contributor to CHANGELOG.md (#13706) This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #13068 Co-authored-by: c298lee <55311782+c298lee@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45e39dcaec80..005f0204866b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott -Work in this release was contributed by @KyGuy2002. Thank you for your contribution! +Work in this release was contributed by @KyGuy2002 and @artzhookov. Thank you for your contributions! ## 8.30.0 From 03eb680a83e304cfb3818058a90527ab294174bd Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 18 Sep 2024 14:55:09 +0200 Subject: [PATCH 29/39] fix(browser): Try multiple options for `lazyLoadIntegration` script parent element lookup (#13717) Change the order for looking up parent elements to inject the integration script: 1. `document.body` 2. `document.head` 3. `document.currentScript.parentElement` --- packages/browser/src/utils/lazyLoadIntegration.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/browser/src/utils/lazyLoadIntegration.ts b/packages/browser/src/utils/lazyLoadIntegration.ts index 168d1fd1013b..82260ae9724f 100644 --- a/packages/browser/src/utils/lazyLoadIntegration.ts +++ b/packages/browser/src/utils/lazyLoadIntegration.ts @@ -68,7 +68,14 @@ export async function lazyLoadIntegration( script.addEventListener('error', reject); }); - WINDOW.document.body.appendChild(script); + const currentScript = WINDOW.document.currentScript; + const parent = WINDOW.document.body || WINDOW.document.head || (currentScript && currentScript.parentElement); + + if (parent) { + parent.appendChild(script); + } else { + throw new Error(`Could not find parent element to insert lazy-loaded ${name} script`); + } try { await waitForLoad; From 479aa1120dc9f2254296f71119e706b2a181563e Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 18 Sep 2024 16:35:01 +0200 Subject: [PATCH 30/39] ref: Update http instrumentation name for logging (#13716) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With this change, it is easier to figure out from logs if the correct or incorrect http instrumentation is added. Now, if you see e.g. this in the logs, if users have enabled logs (`debug: true` if not using `skipOpenTelemetrySetup: true`, else using native OTEL debug logs with e.g. `diag.setLogger(new DiagConsoleLogger(), DiagLogLevel.DEBUG)`): ```js @opentelemetry/instrumentation-http-sentry Applying instrumentation patch for nodejs core module on require hook { module: 'http' } @opentelemetry/instrumentation-http Applying instrumentation patch for nodejs core module on require hook { module: 'http' } ``` you can tell that that it has been double instrumenting this incorrectly. You should never see the `@opentelemetry/instrumentation-http` entry anymore, otherwise something is wrong there. This came out of https://github.com/getsentry/sentry-docs/pull/11378, I looked into various ways to debug this but there is not really an API provided by OTEL that allows us to figure this out 😬 --- packages/node/src/integrations/http.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index d9e5e671b702..126f22a06063 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -1,5 +1,6 @@ import type { ClientRequest, IncomingMessage, RequestOptions, ServerResponse } from 'node:http'; import type { Span } from '@opentelemetry/api'; +import { diag } from '@opentelemetry/api'; import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'; import { addOpenTelemetryInstrumentation } from '@sentry/opentelemetry'; @@ -23,6 +24,8 @@ import { getRequestUrl } from '../utils/getRequestUrl'; const INTEGRATION_NAME = 'Http'; +const INSTRUMENTATION_NAME = '@opentelemetry_sentry-patched/instrumentation-http'; + interface HttpOptions { /** * Whether breadcrumbs should be recorded for requests. @@ -195,6 +198,17 @@ export const instrumentHttp = Object.assign( }, }); + // We want to update the logger namespace so we can better identify what is happening here + try { + _httpInstrumentation['_diag'] = diag.createComponentLogger({ + namespace: INSTRUMENTATION_NAME, + }); + + // @ts-expect-error This is marked as read-only, but we overwrite it anyhow + _httpInstrumentation.instrumentationName = INSTRUMENTATION_NAME; + } catch { + // ignore errors here... + } addOpenTelemetryInstrumentation(_httpInstrumentation); }, { From 14f0b6efeee5add22257a76e8d309891adbef839 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 18 Sep 2024 16:37:49 +0200 Subject: [PATCH 31/39] fix(opentelemetry): Always use active span in `Propagator.inject` (#13381) This PR removes the check for `hasTracingEnabled` in `getInjectionData` and simply always takes the non-recording span if there is one. Which is always the case in server applications when handling a request. For non-request-handling situations, we fall back anyway to the propagation context. --- .../tracing/meta-tags-twp-errors/no-server.js | 20 +++++++ .../tracing/meta-tags-twp-errors/server.js | 30 +++++++++++ .../tracing/meta-tags-twp-errors/test.ts | 52 +++++++++++++++++++ packages/opentelemetry/src/propagator.ts | 3 +- 4 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/meta-tags-twp-errors/no-server.js create mode 100644 dev-packages/node-integration-tests/suites/tracing/meta-tags-twp-errors/server.js create mode 100644 dev-packages/node-integration-tests/suites/tracing/meta-tags-twp-errors/test.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp-errors/no-server.js b/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp-errors/no-server.js new file mode 100644 index 000000000000..ac0122b48380 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp-errors/no-server.js @@ -0,0 +1,20 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + transport: loggingTransport, + beforeSend(event) { + event.contexts = { + ...event.contexts, + traceData: { + ...Sentry.getTraceData(), + metaTags: Sentry.getTraceMetaTags(), + }, + }; + return event; + }, +}); + +Sentry.captureException(new Error('test error')); diff --git a/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp-errors/server.js b/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp-errors/server.js new file mode 100644 index 000000000000..19877ffe3613 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp-errors/server.js @@ -0,0 +1,30 @@ +const { loggingTransport, startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + transport: loggingTransport, + beforeSend(event) { + event.contexts = { + ...event.contexts, + traceData: { + ...Sentry.getTraceData(), + metaTags: Sentry.getTraceMetaTags(), + }, + }; + return event; + }, +}); + +// express must be required after Sentry is initialized +const express = require('express'); + +const app = express(); + +app.get('/test', () => { + throw new Error('test error'); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp-errors/test.ts b/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp-errors/test.ts new file mode 100644 index 000000000000..e6c0bfff822d --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp-errors/test.ts @@ -0,0 +1,52 @@ +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +describe('errors in TwP mode have same trace in trace context and getTraceData()', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + test('in incoming request', async () => { + createRunner(__dirname, 'server.js') + .expect({ + event: event => { + const { contexts } = event; + const { trace_id, span_id } = contexts?.trace || {}; + expect(trace_id).toMatch(/^[a-f0-9]{32}$/); + expect(span_id).toMatch(/^[a-f0-9]{16}$/); + + const traceData = contexts?.traceData || {}; + + expect(traceData['sentry-trace']).toEqual(`${trace_id}-${span_id}`); + expect(traceData.baggage).toContain(`sentry-trace_id=${trace_id}`); + + expect(traceData.metaTags).toContain(``); + expect(traceData.metaTags).toContain(`sentr y-trace_id=${trace_id}`); + expect(traceData.metaTags).not.toContain('sentry-sampled='); + }, + }) + .start() + .makeRequest('get', '/test'); + }); + + test('outside of a request handler', done => { + createRunner(__dirname, 'no-server.js') + .expect({ + event: event => { + const { contexts } = event; + const { trace_id, span_id } = contexts?.trace || {}; + expect(trace_id).toMatch(/^[a-f0-9]{32}$/); + expect(span_id).toMatch(/^[a-f0-9]{16}$/); + + const traceData = contexts?.traceData || {}; + + expect(traceData['sentry-trace']).toEqual(`${trace_id}-${span_id}`); + expect(traceData.baggage).toContain(`sentry-trace_id=${trace_id}`); + + expect(traceData.metaTags).toContain(``); + expect(traceData.metaTags).toContain(`sentry-trace_id=${trace_id}`); + expect(traceData.metaTags).not.toContain('sentry-sampled='); + }, + }) + .start(done); + }); +}); diff --git a/packages/opentelemetry/src/propagator.ts b/packages/opentelemetry/src/propagator.ts index 387943cf9cf0..f4fcb1fa91e9 100644 --- a/packages/opentelemetry/src/propagator.ts +++ b/packages/opentelemetry/src/propagator.ts @@ -5,7 +5,6 @@ import { propagation, trace } from '@opentelemetry/api'; import { W3CBaggagePropagator, isTracingSuppressed } from '@opentelemetry/core'; import { ATTR_URL_FULL, SEMATTRS_HTTP_URL } from '@opentelemetry/semantic-conventions'; import type { continueTrace } from '@sentry/core'; -import { hasTracingEnabled } from '@sentry/core'; import { getRootSpan } from '@sentry/core'; import { spanToJSON } from '@sentry/core'; import { @@ -198,7 +197,7 @@ function getInjectionData(context: Context): { spanId: string | undefined; sampled: boolean | undefined; } { - const span = hasTracingEnabled() ? trace.getSpan(context) : undefined; + const span = trace.getSpan(context); const spanIsRemote = span?.spanContext().isRemote; // If we have a local span, we can just pick everything from it From 1e9a1a3c80b10d2aca19b640666af9134d3bf40a Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 18 Sep 2024 16:42:15 +0200 Subject: [PATCH 32/39] test(ci): Use latest node 22 version again for Node unit tests (#13720) Looks like the bug that made us pin to Node 22.6.0 in our Node unit tests was fixed and released in 22.8.0. This PR reverts the pin, meaning we test against the latest Node 22 version again. --- .github/workflows/build.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c1ff3d4b8e9c..9b36572032b1 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -458,8 +458,7 @@ jobs: strategy: fail-fast: false matrix: - # TODO(lforst): Unpin Node.js version 22 when https://github.com/protobufjs/protobuf.js/issues/2025 is resolved which broke the nodejs tests - node: [14, 16, 18, 20, '22.6.0'] + node: [14, 16, 18, 20, 22] steps: - name: Check out base commit (${{ github.event.pull_request.base.sha }}) uses: actions/checkout@v4 From fc7634ebafeeaa53e79086ad5944cab503fdecdb Mon Sep 17 00:00:00 2001 From: Julian Garcia Castillo Date: Thu, 19 Sep 2024 13:03:34 +0200 Subject: [PATCH 33/39] feat(gatsby): Add optional `deleteSourcemapsAfterUpload` (#13610) Related #13582 This work adds the `deleteSourcemapsAfterUpload` option to the Gatsby plugin, allowing it to be passed to the Webpack plugin to set the `sourceMapFilesToDeleteAfterUpload` without exposing the API. This simplifies our workflow by eliminating the need to apply a Yarn patch to modify the package whenever we want to use `filesToDeleteAfterUpload`. Before submitting a pull request, please take a look at our [Contributing](https://github.com/getsentry/sentry-javascript/blob/master/CONTRIBUTING.md) guidelines and verify: - [x] If you've added code that should be tested, please add tests. - [x] Ensure your code lints and the test suite passes (`yarn lint`) & (`yarn test`). --------- Co-authored-by: Luca Forstner --- packages/gatsby/README.md | 18 ++++++++++++++ packages/gatsby/gatsby-node.js | 3 +++ packages/gatsby/test/gatsby-node.test.ts | 31 ++++++++++++++++++++++++ 3 files changed, 52 insertions(+) diff --git a/packages/gatsby/README.md b/packages/gatsby/README.md index 5de12ed78410..cf5eadf7045b 100644 --- a/packages/gatsby/README.md +++ b/packages/gatsby/README.md @@ -65,6 +65,24 @@ module.exports = { }; ``` +Additionally, you can delete source map files after they have been uploaded by setting the `deleteSourcemapsAfterUpload` +option to be `true`. + +```javascript +module.exports = { + // ... + plugins: [ + { + resolve: '@sentry/gatsby', + options: { + deleteSourcemapsAfterUpload: true, + }, + }, + // ... + ], +}; +``` + ## Links - [Official SDK Docs](https://docs.sentry.io/quickstart/) diff --git a/packages/gatsby/gatsby-node.js b/packages/gatsby/gatsby-node.js index de88ee73adc0..911fcda7b437 100644 --- a/packages/gatsby/gatsby-node.js +++ b/packages/gatsby/gatsby-node.js @@ -7,12 +7,15 @@ const SENTRY_USER_CONFIG = ['./sentry.config.js', './sentry.config.ts']; exports.onCreateWebpackConfig = ({ getConfig, actions }, options) => { const enableClientWebpackPlugin = options.enableClientWebpackPlugin !== false; if (process.env.NODE_ENV === 'production' && enableClientWebpackPlugin) { + const deleteSourcemapsAfterUpload = options.deleteSourcemapsAfterUpload === true; actions.setWebpackConfig({ plugins: [ sentryWebpackPlugin({ sourcemaps: { // Only include files from the build output directory assets: ['./public/**'], + // Delete source files after uploading + filesToDeleteAfterUpload: deleteSourcemapsAfterUpload ? ['./public/**/*.map'] : undefined, // Ignore files that aren't users' source code related ignore: [ 'polyfill-*', // related to polyfills diff --git a/packages/gatsby/test/gatsby-node.test.ts b/packages/gatsby/test/gatsby-node.test.ts index 2e80ac03dcaa..006cb6f9e2c0 100644 --- a/packages/gatsby/test/gatsby-node.test.ts +++ b/packages/gatsby/test/gatsby-node.test.ts @@ -1,5 +1,12 @@ +import { sentryWebpackPlugin } from '@sentry/webpack-plugin'; import { onCreateWebpackConfig } from '../gatsby-node'; +jest.mock('@sentry/webpack-plugin', () => ({ + sentryWebpackPlugin: jest.fn().mockReturnValue({ + apply: jest.fn(), + }), +})); + describe('onCreateWebpackConfig', () => { let originalNodeEnv: string | undefined; @@ -12,6 +19,10 @@ describe('onCreateWebpackConfig', () => { process.env.NODE_ENV = originalNodeEnv; }); + afterEach(() => { + jest.clearAllMocks(); + }); + it('sets a webpack config', () => { const actions = { setWebpackConfig: jest.fn(), @@ -36,4 +47,24 @@ describe('onCreateWebpackConfig', () => { expect(actions.setWebpackConfig).toHaveBeenCalledTimes(0); }); + + it('sets sourceMapFilesToDeleteAfterUpload when provided in options', () => { + const actions = { + setWebpackConfig: jest.fn(), + }; + + const getConfig = jest.fn(); + + onCreateWebpackConfig({ actions, getConfig }, { deleteSourcemapsAfterUpload: true }); + + expect(actions.setWebpackConfig).toHaveBeenCalledTimes(1); + + expect(sentryWebpackPlugin).toHaveBeenCalledWith( + expect.objectContaining({ + sourcemaps: expect.objectContaining({ + filesToDeleteAfterUpload: ['./public/**/*.map'], + }), + }), + ); + }); }); From 32f5f00d9d3d4d8a85c3ee141b0803f9a687dbbb Mon Sep 17 00:00:00 2001 From: Sigrid Huemer <32902192+s1gr1d@users.noreply.github.com> Date: Thu, 19 Sep 2024 14:26:30 +0200 Subject: [PATCH 34/39] fix(nuxt): Use correct server output file path (#13725) Depending on the [nitro preset](https://nitro.unjs.io/deploy), the build output changes. By using the `serverDir` option, the directory can be retrieved dynamically. --- packages/nuxt/src/module.ts | 28 ++++++++++++----------- packages/nuxt/src/vite/addServerConfig.ts | 28 ++++++++++++++++------- 2 files changed, 35 insertions(+), 21 deletions(-) diff --git a/packages/nuxt/src/module.ts b/packages/nuxt/src/module.ts index f43f30d7e5ee..b379a7d7a206 100644 --- a/packages/nuxt/src/module.ts +++ b/packages/nuxt/src/module.ts @@ -64,21 +64,23 @@ export default defineNuxtModule({ setupSourceMaps(moduleOptions, nuxt); } - if (serverConfigFile && serverConfigFile.includes('.server.config')) { - addServerConfigToBuild(moduleOptions, nuxt, serverConfigFile); + nuxt.hooks.hook('nitro:init', nitro => { + if (serverConfigFile && serverConfigFile.includes('.server.config')) { + addServerConfigToBuild(moduleOptions, nuxt, nitro, serverConfigFile); - if (moduleOptions.experimental_basicServerTracing) { - addSentryTopImport(moduleOptions, nuxt); - } else { - if (moduleOptions.debug) { - consoleSandbox(() => { - // eslint-disable-next-line no-console - console.log( - `[Sentry] Using your \`${serverConfigFile}\` file for the server-side Sentry configuration. In case you have a \`public/instrument.server\` file, the \`public/instrument.server\` file will be ignored. Make sure the file path in your node \`--import\` option matches the Sentry server config file in your \`.output\` folder and has a \`.mjs\` extension.`, - ); - }); + if (moduleOptions.experimental_basicServerTracing) { + addSentryTopImport(moduleOptions, nitro); + } else { + if (moduleOptions.debug) { + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.log( + `[Sentry] Using your \`${serverConfigFile}\` file for the server-side Sentry configuration. In case you have a \`public/instrument.server\` file, the \`public/instrument.server\` file will be ignored. Make sure the file path in your node \`--import\` option matches the Sentry server config file in your \`.output\` folder and has a \`.mjs\` extension.`, + ); + }); + } } } - } + }); }, }); diff --git a/packages/nuxt/src/vite/addServerConfig.ts b/packages/nuxt/src/vite/addServerConfig.ts index 60f2452cde5b..845228c58b0c 100644 --- a/packages/nuxt/src/vite/addServerConfig.ts +++ b/packages/nuxt/src/vite/addServerConfig.ts @@ -2,6 +2,7 @@ import * as fs from 'fs'; import { createResolver } from '@nuxt/kit'; import type { Nuxt } from '@nuxt/schema'; import { consoleSandbox } from '@sentry/utils'; +import type { Nitro } from 'nitropack'; import type { SentryNuxtModuleOptions } from '../common/types'; /** @@ -13,6 +14,7 @@ import type { SentryNuxtModuleOptions } from '../common/types'; export function addServerConfigToBuild( moduleOptions: SentryNuxtModuleOptions, nuxt: Nuxt, + nitro: Nitro, serverConfigFile: string, ): void { nuxt.hook('vite:extendConfig', async (viteInlineConfig, _env) => { @@ -29,10 +31,11 @@ export function addServerConfigToBuild( * When the build process is finished, copy the `sentry.server.config` file to the `.output` directory. * This is necessary because we need to reference this file path in the node --import option. */ - nuxt.hook('close', async () => { - const rootDirResolver = createResolver(nuxt.options.rootDir); - const source = rootDirResolver.resolve('.nuxt/dist/server/sentry.server.config.mjs'); - const destination = rootDirResolver.resolve('.output/server/sentry.server.config.mjs'); + nitro.hooks.hook('close', async () => { + const buildDirResolver = createResolver(nitro.options.buildDir); + const serverDirResolver = createResolver(nitro.options.output.serverDir); + const source = buildDirResolver.resolve('dist/server/sentry.server.config.mjs'); + const destination = serverDirResolver.resolve('sentry.server.config.mjs'); try { await fs.promises.access(source, fs.constants.F_OK); @@ -66,10 +69,19 @@ export function addServerConfigToBuild( * This is necessary for environments where modifying the node option `--import` is not possible. * However, only limited tracing instrumentation is supported when doing this. */ -export function addSentryTopImport(moduleOptions: SentryNuxtModuleOptions, nuxt: Nuxt): void { - nuxt.hook('close', async () => { - const rootDirResolver = createResolver(nuxt.options.rootDir); - const entryFilePath = rootDirResolver.resolve('.output/server/index.mjs'); +export function addSentryTopImport(moduleOptions: SentryNuxtModuleOptions, nitro: Nitro): void { + nitro.hooks.hook('close', () => { + // other presets ('node-server' or 'vercel') have an index.mjs + const presetsWithServerFile = ['netlify']; + const entryFileName = + typeof nitro.options.rollupConfig?.output.entryFileNames === 'string' + ? nitro.options.rollupConfig?.output.entryFileNames + : presetsWithServerFile.includes(nitro.options.preset) + ? 'server.mjs' + : 'index.mjs'; + + const serverDirResolver = createResolver(nitro.options.output.serverDir); + const entryFilePath = serverDirResolver.resolve(entryFileName); try { fs.readFile(entryFilePath, 'utf8', (err, data) => { From 37c4c42a83fa549bdd213e50cb8dd459ba05a522 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 19 Sep 2024 14:33:46 +0200 Subject: [PATCH 35/39] ref: Add external contributor to CHANGELOG.md (#13727) This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #13610 --------- Co-authored-by: lforst <8118419+lforst@users.noreply.github.com> Co-authored-by: Abhijeet Prasad --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 005f0204866b..936edff4c346 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott -Work in this release was contributed by @KyGuy2002 and @artzhookov. Thank you for your contributions! +Work in this release was contributed by @KyGuy2002, @artzhookov, and @julianCast. Thank you for your contributions! ## 8.30.0 From 2ab7518b7f559bf2f21eb6277e8cb1c1097473d8 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 19 Sep 2024 16:57:35 +0300 Subject: [PATCH 36/39] feat(node): Add `dataloader` integration (#13664) Adds integration for `dataloader` using [`@opentelemetry/instrumentation-dataloader`](https://www.npmjs.com/package/@opentelemetry/instrumentation-dataloader) on the background. A few notes: - We currently don't have access to the lookup / request as there is no hook from `@opentelemetry/instrumentation-dataloader`. So, we don't have `cache.hit`, `cache.key`, `cache.item_size` and so on, in this integration. I can try to implement those upstream, but if you have another way in mind to access those please let me know. - `@opentelemetry/instrumentation-dataloader` only records spans for `load`, `loadMany` and `batch`, which all are `cache.get` operations. There are also `prime`, `clear`, `clearAll`. We also can implement those upstream and update the integration in future. --- .../node-integration-tests/package.json | 1 + .../suites/tracing/dataloader/scenario.js | 33 +++++++++++ .../suites/tracing/dataloader/test.ts | 40 +++++++++++++ packages/astro/src/index.server.ts | 1 + packages/aws-serverless/src/index.ts | 1 + packages/bun/src/index.ts | 1 + packages/google-cloud-serverless/src/index.ts | 1 + packages/node/package.json | 1 + packages/node/src/index.ts | 1 + .../src/integrations/tracing/dataloader.ts | 57 +++++++++++++++++++ .../node/src/integrations/tracing/index.ts | 3 + yarn.lock | 12 ++++ 12 files changed, 152 insertions(+) create mode 100644 dev-packages/node-integration-tests/suites/tracing/dataloader/scenario.js create mode 100644 dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts create mode 100644 packages/node/src/integrations/tracing/dataloader.ts diff --git a/dev-packages/node-integration-tests/package.json b/dev-packages/node-integration-tests/package.json index 8c5a7dfe1bc3..e954fb631c97 100644 --- a/dev-packages/node-integration-tests/package.json +++ b/dev-packages/node-integration-tests/package.json @@ -43,6 +43,7 @@ "connect": "^3.7.0", "cors": "^2.8.5", "cron": "^3.1.6", + "dataloader": "2.2.2", "express": "^4.17.3", "generic-pool": "^3.9.0", "graphql": "^16.3.0", diff --git a/dev-packages/node-integration-tests/suites/tracing/dataloader/scenario.js b/dev-packages/node-integration-tests/suites/tracing/dataloader/scenario.js new file mode 100644 index 000000000000..569d23276f0b --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/dataloader/scenario.js @@ -0,0 +1,33 @@ +const { loggingTransport, startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +const PORT = 8008; + +// Stop the process from exiting before the transaction is sent +setInterval(() => {}, 1000); + +const run = async () => { + const express = require('express'); + const Dataloader = require('dataloader'); + + const app = express(); + const dataloader = new Dataloader(async keys => keys.map((_, idx) => idx), { + cache: false, + }); + + app.get('/', (req, res) => { + const user = dataloader.load('user-1'); + res.send(user); + }); + + startExpressServerAndSendPortToRunner(app, PORT); +}; + +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts b/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts new file mode 100644 index 000000000000..27a2511f1a6e --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts @@ -0,0 +1,40 @@ +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +describe('dataloader auto-instrumentation', () => { + afterAll(async () => { + cleanupChildProcesses(); + }); + + const EXPECTED_TRANSACTION = { + transaction: 'GET /', + spans: expect.arrayContaining([ + expect.objectContaining({ + data: expect.objectContaining({ + 'sentry.origin': 'auto.db.otel.dataloader', + 'sentry.op': 'cache.get', + }), + description: 'dataloader.load', + origin: 'auto.db.otel.dataloader', + op: 'cache.get', + status: 'ok', + }), + expect.objectContaining({ + data: expect.objectContaining({ + 'sentry.origin': 'auto.db.otel.dataloader', + 'sentry.op': 'cache.get', + }), + description: 'dataloader.batch', + origin: 'auto.db.otel.dataloader', + op: 'cache.get', + status: 'ok', + }), + ]), + }; + + test('should auto-instrument `dataloader` package.', done => { + createRunner(__dirname, 'scenario.js') + .expect({ transaction: EXPECTED_TRANSACTION }) + .start(done) + .makeRequest('get', '/'); + }); +}); diff --git a/packages/astro/src/index.server.ts b/packages/astro/src/index.server.ts index 2645151a9ede..0239e2a55798 100644 --- a/packages/astro/src/index.server.ts +++ b/packages/astro/src/index.server.ts @@ -29,6 +29,7 @@ export { createGetModuleFromFilename, createTransport, cron, + dataloaderIntegration, debugIntegration, dedupeIntegration, DEFAULT_USER_INCLUDES, diff --git a/packages/aws-serverless/src/index.ts b/packages/aws-serverless/src/index.ts index 19c90e3aef3f..44414824cdc1 100644 --- a/packages/aws-serverless/src/index.ts +++ b/packages/aws-serverless/src/index.ts @@ -78,6 +78,7 @@ export { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + dataloaderIntegration, expressIntegration, expressErrorHandler, setupExpressErrorHandler, diff --git a/packages/bun/src/index.ts b/packages/bun/src/index.ts index fcb3d1331f46..267adda6fac4 100644 --- a/packages/bun/src/index.ts +++ b/packages/bun/src/index.ts @@ -99,6 +99,7 @@ export { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + dataloaderIntegration, expressIntegration, expressErrorHandler, setupExpressErrorHandler, diff --git a/packages/google-cloud-serverless/src/index.ts b/packages/google-cloud-serverless/src/index.ts index 14aa0996cb7c..33fdc6ea314f 100644 --- a/packages/google-cloud-serverless/src/index.ts +++ b/packages/google-cloud-serverless/src/index.ts @@ -79,6 +79,7 @@ export { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + dataloaderIntegration, expressIntegration, expressErrorHandler, setupExpressErrorHandler, diff --git a/packages/node/package.json b/packages/node/package.json index dace1125f2e4..d80869cd9251 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -70,6 +70,7 @@ "@opentelemetry/core": "^1.25.1", "@opentelemetry/instrumentation": "^0.53.0", "@opentelemetry/instrumentation-connect": "0.39.0", + "@opentelemetry/instrumentation-dataloader": "0.12.0", "@opentelemetry/instrumentation-express": "0.42.0", "@opentelemetry/instrumentation-fastify": "0.39.0", "@opentelemetry/instrumentation-fs": "0.15.0", diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index d4cbcb9544a9..f3c945f5316d 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -28,6 +28,7 @@ export { koaIntegration, setupKoaErrorHandler } from './integrations/tracing/koa export { connectIntegration, setupConnectErrorHandler } from './integrations/tracing/connect'; export { spotlightIntegration } from './integrations/spotlight'; export { genericPoolIntegration } from './integrations/tracing/genericPool'; +export { dataloaderIntegration } from './integrations/tracing/dataloader'; export { SentryContextManager } from './otel/contextManager'; export { generateInstrumentOnce } from './otel/instrument'; diff --git a/packages/node/src/integrations/tracing/dataloader.ts b/packages/node/src/integrations/tracing/dataloader.ts new file mode 100644 index 000000000000..d4567ea0dfbe --- /dev/null +++ b/packages/node/src/integrations/tracing/dataloader.ts @@ -0,0 +1,57 @@ +import { DataloaderInstrumentation } from '@opentelemetry/instrumentation-dataloader'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + defineIntegration, + spanToJSON, +} from '@sentry/core'; +import type { IntegrationFn } from '@sentry/types'; +import { generateInstrumentOnce } from '../../otel/instrument'; + +const INTEGRATION_NAME = 'Dataloader'; + +export const instrumentDataloader = generateInstrumentOnce( + INTEGRATION_NAME, + () => + new DataloaderInstrumentation({ + requireParentSpan: true, + }), +); + +const _dataloaderIntegration = (() => { + return { + name: INTEGRATION_NAME, + setupOnce() { + instrumentDataloader(); + }, + + setup(client) { + client.on('spanStart', span => { + const spanJSON = spanToJSON(span); + if (spanJSON.description?.startsWith('dataloader')) { + span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.dataloader'); + } + + // These are all possible dataloader span descriptions + // Still checking for the future versions + // in case they add support for `clear` and `prime` + if ( + spanJSON.description === 'dataloader.load' || + spanJSON.description === 'dataloader.loadMany' || + spanJSON.description === 'dataloader.batch' + ) { + span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'cache.get'); + // TODO: We can try adding `key` to the `data` attribute upstream. + // Or alternatively, we can add `requestHook` to the dataloader instrumentation. + } + }); + }, + }; +}) satisfies IntegrationFn; + +/** + * Dataloader integration + * + * Capture tracing data for Dataloader. + */ +export const dataloaderIntegration = defineIntegration(_dataloaderIntegration); diff --git a/packages/node/src/integrations/tracing/index.ts b/packages/node/src/integrations/tracing/index.ts index 69ffc24a8be2..0248e3fbae21 100644 --- a/packages/node/src/integrations/tracing/index.ts +++ b/packages/node/src/integrations/tracing/index.ts @@ -2,6 +2,7 @@ import type { Integration } from '@sentry/types'; import { instrumentHttp } from '../http'; import { connectIntegration, instrumentConnect } from './connect'; +import { dataloaderIntegration, instrumentDataloader } from './dataloader'; import { expressIntegration, instrumentExpress } from './express'; import { fastifyIntegration, instrumentFastify } from './fastify'; import { genericPoolIntegration, instrumentGenericPool } from './genericPool'; @@ -41,6 +42,7 @@ export function getAutoPerformanceIntegrations(): Integration[] { connectIntegration(), genericPoolIntegration(), kafkaIntegration(), + dataloaderIntegration(), ]; } @@ -67,5 +69,6 @@ export function getOpenTelemetryInstrumentationToPreload(): (((options?: any) => instrumentGraphql, instrumentRedis, instrumentGenericPool, + instrumentDataloader, ]; } diff --git a/yarn.lock b/yarn.lock index 9f6846f29f95..c94576d2e979 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7101,6 +7101,13 @@ "@opentelemetry/semantic-conventions" "^1.27.0" "@types/connect" "3.4.36" +"@opentelemetry/instrumentation-dataloader@0.12.0": + version "0.12.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-dataloader/-/instrumentation-dataloader-0.12.0.tgz#de03a3948dec4f15fed80aa424d6bd5d6a8d10c7" + integrity sha512-pnPxatoFE0OXIZDQhL2okF//dmbiWFzcSc8pUg9TqofCLYZySSxDCgQc69CJBo5JnI3Gz1KP+mOjS4WAeRIH4g== + dependencies: + "@opentelemetry/instrumentation" "^0.53.0" + "@opentelemetry/instrumentation-express@0.42.0": version "0.42.0" resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-express/-/instrumentation-express-0.42.0.tgz#279f195aa66baee2b98623a16666c6229c8e7564" @@ -15235,6 +15242,11 @@ data-urls@^4.0.0: whatwg-mimetype "^3.0.0" whatwg-url "^12.0.0" +dataloader@2.2.2: + version "2.2.2" + resolved "https://registry.yarnpkg.com/dataloader/-/dataloader-2.2.2.tgz#216dc509b5abe39d43a9b9d97e6e5e473dfbe3e0" + integrity sha512-8YnDaaf7N3k/q5HnTJVuzSyLETjoZjVmHc4AeKAzOvKHEFQKcn64OKBfzHYtE9zGjctNM7V9I0MfnUVLpi7M5g== + date-fns@^2.29.2: version "2.29.3" resolved "https://registry.yarnpkg.com/date-fns/-/date-fns-2.29.3.tgz#27402d2fc67eb442b511b70bbdf98e6411cd68a8" From 336a23664f1bfbc8f83d176b30d4e41649847356 Mon Sep 17 00:00:00 2001 From: Sigrid Huemer <32902192+s1gr1d@users.noreply.github.com> Date: Thu, 19 Sep 2024 17:11:14 +0200 Subject: [PATCH 37/39] feat(nuxt): Improve logs about adding Node option 'import' (#13726) Adding the node option can be a confusing step. This adds a log output which already includes the correct file path to add. It looks like this: ``` [Sentry] Using your sentry.server.config.ts file for the server-side Sentry configuration. Make sure to add the Node option import to the Node command where you deploy and/or run your application. This preloads the Sentry configuration at server startup. You can do this via a command-line flag (node --import ./.output/server/sentry.server.config.mjs [...]) or via an environment variable (NODE_OPTIONS='--import ./.output/server/sentry.server.config.mjs' node [...]). ``` --- packages/nuxt/src/module.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/nuxt/src/module.ts b/packages/nuxt/src/module.ts index b379a7d7a206..c74fe32b93fe 100644 --- a/packages/nuxt/src/module.ts +++ b/packages/nuxt/src/module.ts @@ -1,3 +1,4 @@ +import * as path from 'path'; import { addPlugin, addPluginTemplate, addServerPlugin, createResolver, defineNuxtModule } from '@nuxt/kit'; import { consoleSandbox } from '@sentry/utils'; import type { SentryNuxtModuleOptions } from './common/types'; @@ -72,10 +73,16 @@ export default defineNuxtModule({ addSentryTopImport(moduleOptions, nitro); } else { if (moduleOptions.debug) { + const serverDirResolver = createResolver(nitro.options.output.serverDir); + const serverConfigPath = serverDirResolver.resolve('sentry.server.config.mjs'); + + // For the default nitro node-preset build output this relative path would be: ./.output/server/sentry.server.config.mjs + const serverConfigRelativePath = `.${path.sep}${path.relative(nitro.options.rootDir, serverConfigPath)}`; + consoleSandbox(() => { // eslint-disable-next-line no-console console.log( - `[Sentry] Using your \`${serverConfigFile}\` file for the server-side Sentry configuration. In case you have a \`public/instrument.server\` file, the \`public/instrument.server\` file will be ignored. Make sure the file path in your node \`--import\` option matches the Sentry server config file in your \`.output\` folder and has a \`.mjs\` extension.`, + `[Sentry] Using your \`${serverConfigFile}\` file for the server-side Sentry configuration. Make sure to add the Node option \`import\` to the Node command where you deploy and/or run your application. This preloads the Sentry configuration at server startup. You can do this via a command-line flag (\`node --import ${serverConfigRelativePath} [...]\`) or via an environment variable (\`NODE_OPTIONS='--import ${serverConfigRelativePath}' node [...]\`).`, ); }); } From cf0152a9e25e1c2911e30c154c54d56fe0b79117 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Thu, 19 Sep 2024 12:30:30 -0400 Subject: [PATCH 38/39] feat(replay): Add `onError` callback + other small improvements to debugging (#13721) * Adds an `onError` callback for replay SDK exceptions * Do not log empty messages when calling `logger.exception` * Send `ratelimit_backoff` client report when necessary (instead of generic `send_error`) --- packages/replay-internal/src/replay.ts | 9 +- packages/replay-internal/src/types/replay.ts | 8 +- packages/replay-internal/src/util/logger.ts | 10 +-- .../replay-internal/src/util/sendReplay.ts | 9 +- .../test/integration/flush.test.ts | 4 +- .../test/unit/util/logger.test.ts | 88 +++++++++++++++++++ 6 files changed, 113 insertions(+), 15 deletions(-) create mode 100644 packages/replay-internal/test/unit/util/logger.test.ts diff --git a/packages/replay-internal/src/replay.ts b/packages/replay-internal/src/replay.ts index 06f81b6982c6..ea7df8b7afa7 100644 --- a/packages/replay-internal/src/replay.ts +++ b/packages/replay-internal/src/replay.ts @@ -54,6 +54,7 @@ import { getHandleRecordingEmit } from './util/handleRecordingEmit'; import { isExpired } from './util/isExpired'; import { isSessionExpired } from './util/isSessionExpired'; import { sendReplay } from './util/sendReplay'; +import { RateLimitError } from './util/sendReplayRequest'; import type { SKIPPED } from './util/throttle'; import { THROTTLED, throttle } from './util/throttle'; @@ -245,6 +246,9 @@ export class ReplayContainer implements ReplayContainerInterface { /** A wrapper to conditionally capture exceptions. */ public handleException(error: unknown): void { DEBUG_BUILD && logger.exception(error); + if (this._options.onError) { + this._options.onError(error); + } } /** @@ -1157,8 +1161,8 @@ export class ReplayContainer implements ReplayContainerInterface { segmentId, eventContext, session: this.session, - options: this.getOptions(), timestamp, + onError: err => this.handleException(err), }); } catch (err) { this.handleException(err); @@ -1173,7 +1177,8 @@ export class ReplayContainer implements ReplayContainerInterface { const client = getClient(); if (client) { - client.recordDroppedEvent('send_error', 'replay'); + const dropReason = err instanceof RateLimitError ? 'ratelimit_backoff' : 'send_error'; + client.recordDroppedEvent(dropReason, 'replay'); } } } diff --git a/packages/replay-internal/src/types/replay.ts b/packages/replay-internal/src/types/replay.ts index 0605ba97449a..6892c05ee179 100644 --- a/packages/replay-internal/src/types/replay.ts +++ b/packages/replay-internal/src/types/replay.ts @@ -26,7 +26,7 @@ export interface SendReplayData { eventContext: PopEventContext; timestamp: number; session: Session; - options: ReplayPluginOptions; + onError?: (err: unknown) => void; } export interface Timeouts { @@ -222,6 +222,12 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions { */ beforeErrorSampling?: (event: ErrorEvent) => boolean; + /** + * Callback when an internal SDK error occurs. This can be used to debug SDK + * issues. + */ + onError?: (err: unknown) => void; + /** * _experiments allows users to enable experimental or internal features. * We don't consider such features as part of the public API and hence we don't guarantee semver for them. diff --git a/packages/replay-internal/src/util/logger.ts b/packages/replay-internal/src/util/logger.ts index 80445409164b..1b505f41703a 100644 --- a/packages/replay-internal/src/util/logger.ts +++ b/packages/replay-internal/src/util/logger.ts @@ -1,6 +1,6 @@ import { addBreadcrumb, captureException } from '@sentry/core'; import type { ConsoleLevel, SeverityLevel } from '@sentry/types'; -import { logger as coreLogger } from '@sentry/utils'; +import { logger as coreLogger, severityLevelFromString } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; @@ -64,13 +64,13 @@ function makeReplayLogger(): ReplayLogger { _logger[name] = (...args: unknown[]) => { coreLogger[name](PREFIX, ...args); if (_trace) { - _addBreadcrumb(args[0]); + _addBreadcrumb(args.join(''), severityLevelFromString(name)); } }; }); _logger.exception = (error: unknown, ...message: unknown[]) => { - if (_logger.error) { + if (message.length && _logger.error) { _logger.error(...message); } @@ -79,9 +79,9 @@ function makeReplayLogger(): ReplayLogger { if (_capture) { captureException(error); } else if (_trace) { - // No need for a breadcrumb is `_capture` is enabled since it should be + // No need for a breadcrumb if `_capture` is enabled since it should be // captured as an exception - _addBreadcrumb(error); + _addBreadcrumb(error, 'error'); } }; diff --git a/packages/replay-internal/src/util/sendReplay.ts b/packages/replay-internal/src/util/sendReplay.ts index 973c3fb9a556..c0c6483502b9 100644 --- a/packages/replay-internal/src/util/sendReplay.ts +++ b/packages/replay-internal/src/util/sendReplay.ts @@ -1,8 +1,7 @@ import { setTimeout } from '@sentry-internal/browser-utils'; -import { captureException, setContext } from '@sentry/core'; +import { setContext } from '@sentry/core'; import { RETRY_BASE_INTERVAL, RETRY_MAX_COUNT, UNABLE_TO_SEND_REPLAY } from '../constants'; -import { DEBUG_BUILD } from '../debug-build'; import type { SendReplayData } from '../types'; import { RateLimitError, TransportStatusCodeError, sendReplayRequest } from './sendReplayRequest'; @@ -16,7 +15,7 @@ export async function sendReplay( interval: RETRY_BASE_INTERVAL, }, ): Promise { - const { recordingData, options } = replayData; + const { recordingData, onError } = replayData; // short circuit if there's no events to upload (this shouldn't happen as _runFlush makes this check) if (!recordingData.length) { @@ -36,8 +35,8 @@ export async function sendReplay( _retryCount: retryConfig.count, }); - if (DEBUG_BUILD && options._experiments && options._experiments.captureExceptions) { - captureException(err); + if (onError) { + onError(err); } // If an error happened here, it's likely that uploading the attachment diff --git a/packages/replay-internal/test/integration/flush.test.ts b/packages/replay-internal/test/integration/flush.test.ts index 52654fa909d3..72ef104d0633 100644 --- a/packages/replay-internal/test/integration/flush.test.ts +++ b/packages/replay-internal/test/integration/flush.test.ts @@ -188,8 +188,8 @@ describe('Integration | flush', () => { segmentId: 0, eventContext: expect.anything(), session: expect.any(Object), - options: expect.any(Object), timestamp: expect.any(Number), + onError: expect.any(Function), }); // Add this to test that segment ID increases @@ -238,7 +238,7 @@ describe('Integration | flush', () => { segmentId: 1, eventContext: expect.anything(), session: expect.any(Object), - options: expect.any(Object), + onError: expect.any(Function), timestamp: expect.any(Number), }); diff --git a/packages/replay-internal/test/unit/util/logger.test.ts b/packages/replay-internal/test/unit/util/logger.test.ts new file mode 100644 index 000000000000..075a6f27a841 --- /dev/null +++ b/packages/replay-internal/test/unit/util/logger.test.ts @@ -0,0 +1,88 @@ +import { beforeEach, describe, expect, it } from 'vitest'; + +import * as SentryCore from '@sentry/core'; +import { logger as coreLogger } from '@sentry/utils'; +import { logger } from '../../../src/util/logger'; + +const mockCaptureException = vi.spyOn(SentryCore, 'captureException'); +const mockAddBreadcrumb = vi.spyOn(SentryCore, 'addBreadcrumb'); +const mockLogError = vi.spyOn(coreLogger, 'error'); +vi.spyOn(coreLogger, 'info'); +vi.spyOn(coreLogger, 'log'); +vi.spyOn(coreLogger, 'warn'); + +describe('logger', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe.each([ + [false, false], + [false, true], + [true, false], + [true, true], + ])('with options: captureExceptions:%s, traceInternals:%s', (captureExceptions, traceInternals) => { + beforeEach(() => { + logger.setConfig({ + captureExceptions, + traceInternals, + }); + }); + + it.each([ + ['info', 'info', 'info message'], + ['log', 'log', 'log message'], + ['warn', 'warning', 'warn message'], + ['error', 'error', 'error message'], + ])('%s', (fn, level, message) => { + logger[fn](message); + expect(coreLogger[fn]).toHaveBeenCalledWith('[Replay] ', message); + + if (traceInternals) { + expect(mockAddBreadcrumb).toHaveBeenLastCalledWith( + { + category: 'console', + data: { logger: 'replay' }, + level, + message: `[Replay] ${message}`, + }, + { level }, + ); + } + }); + + it('logs exceptions with a message', () => { + const err = new Error('An error'); + logger.exception(err, 'a message'); + if (captureExceptions) { + expect(mockCaptureException).toHaveBeenCalledWith(err); + } + expect(mockLogError).toHaveBeenCalledWith('[Replay] ', 'a message'); + expect(mockLogError).toHaveBeenLastCalledWith('[Replay] ', err); + expect(mockLogError).toHaveBeenCalledTimes(2); + + if (traceInternals) { + expect(mockAddBreadcrumb).toHaveBeenCalledWith( + { + category: 'console', + data: { logger: 'replay' }, + level: 'error', + message: '[Replay] a message', + }, + { level: 'error' }, + ); + } + }); + + it('logs exceptions without a message', () => { + const err = new Error('An error'); + logger.exception(err); + if (captureExceptions) { + expect(mockCaptureException).toHaveBeenCalledWith(err); + expect(mockAddBreadcrumb).not.toHaveBeenCalled(); + } + expect(mockLogError).toHaveBeenCalledTimes(1); + expect(mockLogError).toHaveBeenLastCalledWith('[Replay] ', err); + }); + }); +}); From 19143766da5256781a945a01b0e8c757c32c2a97 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Fri, 20 Sep 2024 09:34:54 +0200 Subject: [PATCH 39/39] meta(changelog): Update changelog for 8.31.0 --- CHANGELOG.md | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 936edff4c346..52187092ea55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,13 +10,52 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 8.31.0 + +### Important Changes + +- **feat(node): Add `dataloader` integration (#13664)** + +This release adds a new integration for the [`dataloader` package](https://www.npmjs.com/package/dataloader). The Node +SDK (and all SDKs that depend on it) will now automatically instrument `dataloader` instances. You can also add it +manually: + +```js +Sentry.init({ + integrations: [Sentry.dataloaderIntegration()], +}); +``` + +### Other Changes + +- feat(browser): Add navigation `activationStart` timestamp to pageload span (#13658) +- feat(gatsby): Add optional `deleteSourcemapsAfterUpload` (#13610) +- feat(nextjs): Give app router prefetch requests a `http.server.prefetch` op (#13600) +- feat(nextjs): Improve Next.js serverside span data quality (#13652) +- feat(node): Add `disableInstrumentationWarnings` option (#13693) +- feat(nuxt): Adding `experimental_basicServerTracing` option to Nuxt module (#13643) +- feat(nuxt): Improve logs about adding Node option 'import' (#13726) +- feat(replay): Add `onError` callback + other small improvements to debugging (#13721) +- feat(replay): Add experimental option to allow for a checkout every 6 minutes (#13069) +- feat(wasm): Unconditionally parse instruction addresses (#13655) +- fix: Ensure all logs are wrapped with `consoleSandbox` (#13690) +- fix(browser): Try multiple options for `lazyLoadIntegration` script parent element lookup (#13717) +- fix(feedback): Actor color applies to feedback icon (#13702) +- fix(feedback): Fix form width on mobile devices (#13068) +- fix(nestjs): Preserve original function name on `SentryTraced` functions (#13684) +- fix(node): Don't overwrite local variables for re-thrown errors (#13644) +- fix(normalize): Treat Infinity as NaN both are non-serializable numbers (#13406) +- fix(nuxt): Use correct server output file path (#13725) +- fix(opentelemetry): Always use active span in `Propagator.inject` (#13381) +- fix(replay): Fixes potential out-of-order segments (#13609) + Work in this release was contributed by @KyGuy2002, @artzhookov, and @julianCast. Thank you for your contributions! ## 8.30.0 ### Important Changes -- _feat(node): Add `kafkajs` integration (#13528)_ +- **feat(node): Add `kafkajs` integration (#13528)** This release adds a new integration that instruments `kafkajs` library with spans and traces. This integration is automatically enabled by default, but can be included with the `Sentry.kafkaIntegration()` import.