From 6b3db7b5a57949a96c2b264cf4ad7644290c8953 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 9 Aug 2023 12:05:04 +0200 Subject: [PATCH 01/15] feat(node): Add basic cloud resource context (#8764) --- packages/node/src/integrations/context.ts | 88 ++++++++++++++++++++++- packages/types/src/context.ts | 11 +++ packages/types/src/index.ts | 11 ++- 3 files changed, 106 insertions(+), 4 deletions(-) diff --git a/packages/node/src/integrations/context.ts b/packages/node/src/integrations/context.ts index 3039b5fae58b..5b683d5c688d 100644 --- a/packages/node/src/integrations/context.ts +++ b/packages/node/src/integrations/context.ts @@ -1,6 +1,7 @@ /* eslint-disable max-lines */ import type { AppContext, + CloudResourceContext, Contexts, CultureContext, DeviceContext, @@ -29,6 +30,7 @@ interface ContextOptions { os?: boolean; device?: DeviceContextOptions | boolean; culture?: boolean; + cloudResource?: boolean; } /** Add node modules / packages to the event */ @@ -48,9 +50,15 @@ export class Context implements Integration { */ private _cachedContext: Promise | undefined; - public constructor(private readonly _options: ContextOptions = { app: true, os: true, device: true, culture: true }) { - // - } + public constructor( + private readonly _options: ContextOptions = { + app: true, + os: true, + device: true, + culture: true, + cloudResource: true, + }, + ) {} /** * @inheritDoc @@ -73,6 +81,7 @@ export class Context implements Integration { os: { ...updatedContext.os, ...event.contexts?.os }, device: { ...updatedContext.device, ...event.contexts?.device }, culture: { ...updatedContext.culture, ...event.contexts?.culture }, + cloud_resource: { ...updatedContext.cloud_resource, ...event.contexts?.cloud_resource }, }; return event; @@ -120,6 +129,10 @@ export class Context implements Integration { } } + if (this._options.cloudResource) { + contexts.cloud_resource = getCloudResourceContext(); + } + return contexts; } } @@ -380,3 +393,72 @@ async function getLinuxInfo(): Promise { return linuxInfo; } + +/** + * Grabs some information about hosting provider based on best effort. + */ +function getCloudResourceContext(): CloudResourceContext | undefined { + if (process.env.VERCEL) { + // https://vercel.com/docs/concepts/projects/environment-variables/system-environment-variables#system-environment-variables + return { + 'cloud.provider': 'vercel', + 'cloud.region': process.env.VERCEL_REGION, + }; + } else if (process.env.AWS_REGION) { + // https://docs.aws.amazon.com/lambda/latest/dg/configuration-envvars.html + return { + 'cloud.provider': 'aws', + 'cloud.region': process.env.AWS_REGION, + 'cloud.platform': process.env.AWS_EXECUTION_ENV, + }; + } else if (process.env.GCP_PROJECT) { + // https://cloud.google.com/composer/docs/how-to/managing/environment-variables#reserved_variables + return { + 'cloud.provider': 'gcp', + }; + } else if (process.env.ALIYUN_REGION_ID) { + // TODO: find where I found these environment variables - at least gc.github.com returns something + return { + 'cloud.provider': 'alibaba_cloud', + 'cloud.region': process.env.ALIYUN_REGION_ID, + }; + } else if (process.env.WEBSITE_SITE_NAME && process.env.REGION_NAME) { + // https://learn.microsoft.com/en-us/azure/app-service/reference-app-settings?tabs=kudu%2Cdotnet#app-environment + return { + 'cloud.provider': 'azure', + 'cloud.region': process.env.REGION_NAME, + }; + } else if (process.env.IBM_CLOUD_REGION) { + // TODO: find where I found these environment variables - at least gc.github.com returns something + return { + 'cloud.provider': 'ibm_cloud', + 'cloud.region': process.env.IBM_CLOUD_REGION, + }; + } else if (process.env.TENCENTCLOUD_REGION) { + // https://www.tencentcloud.com/document/product/583/32748 + return { + 'cloud.provider': 'tencent_cloud', + 'cloud.region': process.env.TENCENTCLOUD_REGION, + 'cloud.account.id': process.env.TENCENTCLOUD_APPID, + 'cloud.availability_zone': process.env.TENCENTCLOUD_ZONE, + }; + } else if (process.env.NETLIFY) { + // https://docs.netlify.com/configure-builds/environment-variables/#read-only-variables + return { + 'cloud.provider': 'netlify', + }; + } else if (process.env.FLY_REGION) { + // https://fly.io/docs/reference/runtime-environment/ + return { + 'cloud.provider': 'fly.io', + 'cloud.region': process.env.FLY_REGION, + }; + } else if (process.env.DYNO) { + // https://devcenter.heroku.com/articles/dynos#local-environment-variables + return { + 'cloud.provider': 'heroku', + }; + } else { + return undefined; + } +} diff --git a/packages/types/src/context.ts b/packages/types/src/context.ts index 052d6f4a6523..110267284fc0 100644 --- a/packages/types/src/context.ts +++ b/packages/types/src/context.ts @@ -9,6 +9,7 @@ export interface Contexts extends Record { culture?: CultureContext; response?: ResponseContext; trace?: TraceContext; + cloud_resource?: CloudResourceContext; } export interface AppContext extends Record { @@ -93,3 +94,13 @@ export interface TraceContext extends Record { tags?: { [key: string]: Primitive }; trace_id: string; } + +export interface CloudResourceContext extends Record { + ['cloud.provider']?: string; + ['cloud.account.id']?: string; + ['cloud.region']?: string; + ['cloud.availability_zone']?: string; + ['cloud.platform']?: string; + ['host.id']?: string; + ['host.type']?: string; +} diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 5ce2e1fe6ce5..5b9490045a02 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -9,7 +9,16 @@ export type { } from './breadcrumb'; export type { Client } from './client'; export type { ClientReport, Outcome, EventDropReason } from './clientreport'; -export type { Context, Contexts, DeviceContext, OsContext, AppContext, CultureContext, TraceContext } from './context'; +export type { + Context, + Contexts, + DeviceContext, + OsContext, + AppContext, + CultureContext, + TraceContext, + CloudResourceContext, +} from './context'; export type { DataCategory } from './datacategory'; export type { DsnComponents, DsnLike, DsnProtocol } from './dsn'; export type { DebugImage, DebugMeta } from './debugMeta'; From bffbaf63444929704842cd206d45b9f1527587d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kry=C5=A1tof=20Wold=C5=99ich?= <31292499+krystofwoldrich@users.noreply.github.com> Date: Wed, 9 Aug 2023 14:10:56 +0200 Subject: [PATCH 02/15] chore(eventbuilder): Export `exceptionFromError` for use in hybrid SDKs (#8766) --- packages/browser/src/exports.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index 1f7117620c93..03533bdbc90d 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -61,7 +61,7 @@ export { opera11StackLineParser, winjsStackLineParser, } from './stack-parsers'; -export { eventFromException, eventFromMessage } from './eventbuilder'; +export { eventFromException, eventFromMessage, exceptionFromError } from './eventbuilder'; export { createUserFeedbackEnvelope } from './userfeedback'; export { defaultIntegrations, forceLoad, init, onLoad, showReportDialog, wrap, captureUserFeedback } from './sdk'; export { GlobalHandlers, TryCatch, Breadcrumbs, LinkedErrors, HttpContext, Dedupe } from './integrations'; From bf3eb7f3df832987d9ed9be9798932ff27339c09 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 9 Aug 2023 15:10:35 +0200 Subject: [PATCH 03/15] ref(replay): Skip events being added too long after initial segment (#8768) This ensures we ignore any events being added which are after the session max age. Maybe this helps with cases where we send weird replays with a single event much after initial timestamp. I added a test to ensure this does not regress in buffer mode, where recording is continuous and _can_ be "forever". --- packages/replay/src/util/addEvent.ts | 10 ++++ .../test/integration/errorSampleRate.test.ts | 44 ++++++++++++++ .../replay/test/integration/flush.test.ts | 60 +++++++++++++++++++ 3 files changed, 114 insertions(+) diff --git a/packages/replay/src/util/addEvent.ts b/packages/replay/src/util/addEvent.ts index d1e1d366a9e9..6ffd5d03bacd 100644 --- a/packages/replay/src/util/addEvent.ts +++ b/packages/replay/src/util/addEvent.ts @@ -4,6 +4,7 @@ import { logger } from '@sentry/utils'; import { EventBufferSizeExceededError } from '../eventBuffer/error'; import type { AddEventResult, RecordingEvent, ReplayContainer, ReplayFrameEvent, ReplayPluginOptions } from '../types'; +import { logInfo } from './log'; import { timestampToMs } from './timestamp'; function isCustomEvent(event: RecordingEvent): event is ReplayFrameEvent { @@ -39,6 +40,15 @@ export async function addEvent( return null; } + // Throw out events that are +60min from the initial timestamp + if (timestampInMs > replay.getContext().initialTimestamp + replay.timeouts.maxSessionLife) { + logInfo( + `[Replay] Skipping event with timestamp ${timestampInMs} because it is after maxSessionLife`, + replay.getOptions()._experiments.traceInternals, + ); + return null; + } + try { if (isCheckout && replay.recordingMode === 'buffer') { replay.eventBuffer.clear(); diff --git a/packages/replay/test/integration/errorSampleRate.test.ts b/packages/replay/test/integration/errorSampleRate.test.ts index 7ed9c4774c71..777cb437f7e3 100644 --- a/packages/replay/test/integration/errorSampleRate.test.ts +++ b/packages/replay/test/integration/errorSampleRate.test.ts @@ -883,6 +883,50 @@ describe('Integration | errorSampleRate', () => { expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0); expect(replay.isEnabled()).toBe(false); }); + + it('handles very long active buffer session', async () => { + const stepDuration = 10_000; + const steps = 5_000; + + jest.setSystemTime(BASE_TIMESTAMP); + + expect(replay).not.toHaveLastSentReplay(); + + let optionsEvent = createOptionsEvent(replay); + + for (let i = 1; i <= steps; i++) { + jest.advanceTimersByTime(stepDuration); + optionsEvent = createOptionsEvent(replay); + mockRecord._emitter({ data: { step: i }, timestamp: BASE_TIMESTAMP + stepDuration * i, type: 2 }, true); + mockRecord._emitter({ data: { step: i }, timestamp: BASE_TIMESTAMP + stepDuration * i + 5, type: 3 }); + } + + expect(replay).not.toHaveLastSentReplay(); + + expect(replay.isEnabled()).toBe(true); + expect(replay.isPaused()).toBe(false); + expect(replay.recordingMode).toBe('buffer'); + + // Now capture an error + captureException(new Error('testing')); + await waitForBufferFlush(); + + expect(replay).toHaveLastSentReplay({ + recordingData: JSON.stringify([ + { data: { step: steps }, timestamp: BASE_TIMESTAMP + stepDuration * steps, type: 2 }, + optionsEvent, + { data: { step: steps }, timestamp: BASE_TIMESTAMP + stepDuration * steps + 5, type: 3 }, + ]), + replayEventPayload: expect.objectContaining({ + replay_start_timestamp: (BASE_TIMESTAMP + stepDuration * steps) / 1000, + error_ids: [expect.any(String)], + trace_ids: [], + urls: ['http://localhost/'], + replay_id: expect.any(String), + }), + recordingPayloadHeader: { segment_id: 0 }, + }); + }); }); /** diff --git a/packages/replay/test/integration/flush.test.ts b/packages/replay/test/integration/flush.test.ts index 611a1043df1a..6e6e02d86620 100644 --- a/packages/replay/test/integration/flush.test.ts +++ b/packages/replay/test/integration/flush.test.ts @@ -26,6 +26,7 @@ type MockFlush = jest.MockedFunction; type MockRunFlush = jest.MockedFunction; const prevLocation = WINDOW.location; +const prevBrowserPerformanceTimeOrigin = SentryUtils.browserPerformanceTimeOrigin; describe('Integration | flush', () => { let domHandler: (args: any) => any; @@ -91,6 +92,11 @@ describe('Integration | flush', () => { } mockEventBufferFinish = replay.eventBuffer?.finish as MockEventBufferFinish; mockEventBufferFinish.mockClear(); + + Object.defineProperty(SentryUtils, 'browserPerformanceTimeOrigin', { + value: BASE_TIMESTAMP, + writable: true, + }); }); afterEach(async () => { @@ -102,6 +108,10 @@ describe('Integration | flush', () => { value: prevLocation, writable: true, }); + Object.defineProperty(SentryUtils, 'browserPerformanceTimeOrigin', { + value: prevBrowserPerformanceTimeOrigin, + writable: true, + }); }); afterAll(() => { @@ -224,6 +234,7 @@ describe('Integration | flush', () => { // flush #5 @ t=25s - debounced flush calls `flush` // 20s + `flushMinDelay` which is 5 seconds await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); + expect(mockFlush).toHaveBeenCalledTimes(5); expect(mockRunFlush).toHaveBeenCalledTimes(2); expect(mockSendReplay).toHaveBeenLastCalledWith({ @@ -382,4 +393,53 @@ describe('Integration | flush', () => { replay.getOptions()._experiments.traceInternals = false; }); + + it('logs warning if adding event that is after maxSessionLife', async () => { + replay.getOptions()._experiments.traceInternals = true; + + sessionStorage.clear(); + clearSession(replay); + replay['_loadAndCheckSession'](); + await new Promise(process.nextTick); + jest.setSystemTime(BASE_TIMESTAMP); + + replay.eventBuffer!.clear(); + + // We do not care about this warning here + replay.eventBuffer!.hasCheckout = true; + + // Add event that is too long after session start + const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP + MAX_SESSION_LIFE + 100, type: 2 }; + mockRecord._emitter(TEST_EVENT); + + // no checkout! + await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); + + expect(mockFlush).toHaveBeenCalledTimes(1); + expect(mockSendReplay).toHaveBeenCalledTimes(1); + + const replayData = mockSendReplay.mock.calls[0][0]; + + expect(JSON.parse(replayData.recordingData)).toEqual([ + { + type: 5, + timestamp: BASE_TIMESTAMP, + data: { + tag: 'breadcrumb', + payload: { + timestamp: BASE_TIMESTAMP / 1000, + type: 'default', + category: 'console', + data: { logger: 'replay' }, + level: 'info', + message: `[Replay] Skipping event with timestamp ${ + BASE_TIMESTAMP + MAX_SESSION_LIFE + 100 + } because it is after maxSessionLife`, + }, + }, + }, + ]); + + replay.getOptions()._experiments.traceInternals = false; + }); }); From 52d38825ceb5d8826e000cb25d1fd45969c17831 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kry=C5=A1tof=20Wold=C5=99ich?= <31292499+krystofwoldrich@users.noreply.github.com> Date: Wed, 9 Aug 2023 15:44:17 +0200 Subject: [PATCH 04/15] fix(jsdoc): `tracePropagationTargets` defaults are localhost and same origin in Browse (#8749) --- packages/node/src/types.ts | 20 +++++++++++++++++++- packages/types/src/options.ts | 6 ++++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/packages/node/src/types.ts b/packages/node/src/types.ts index fb128653870f..b63c49e440e7 100644 --- a/packages/node/src/types.ts +++ b/packages/node/src/types.ts @@ -1,9 +1,27 @@ -import type { ClientOptions, Options, SamplingContext } from '@sentry/types'; +import type { ClientOptions, Options, SamplingContext, TracePropagationTargets } from '@sentry/types'; import type { NodeClient } from './client'; import type { NodeTransportOptions } from './transports'; export interface BaseNodeOptions { + /** + * List of strings/regex controlling to which outgoing requests + * the SDK will attach tracing headers. + * + * By default the SDK will attach those headers to all outgoing + * requests. If this option is provided, the SDK will match the + * request URL of outgoing requests against the items in this + * array, and only attach tracing headers if a match was found. + * + * @example + * ```js + * Sentry.init({ + * tracePropagationTargets: ['api.site.com'], + * }); + * ``` + */ + tracePropagationTargets?: TracePropagationTargets; + /** * Sets profiling sample rate when @sentry/profiling-node is installed */ diff --git a/packages/types/src/options.ts b/packages/types/src/options.ts index 60d747136d90..74bcfad771f4 100644 --- a/packages/types/src/options.ts +++ b/packages/types/src/options.ts @@ -226,8 +226,8 @@ export interface ClientOptions Date: Wed, 9 Aug 2023 21:02:45 +0000 Subject: [PATCH 05/15] build(deps): bump @opentelemetry/instrumentation from 0.41.0 to 0.41.2 Bumps [@opentelemetry/instrumentation](https://github.com/open-telemetry/opentelemetry-js) from 0.41.0 to 0.41.2. - [Release notes](https://github.com/open-telemetry/opentelemetry-js/releases) - [Changelog](https://github.com/open-telemetry/opentelemetry-js/blob/main/CHANGELOG.md) - [Commits](https://github.com/open-telemetry/opentelemetry-js/compare/experimental/v0.41.0...experimental/v0.41.2) --- updated-dependencies: - dependency-name: "@opentelemetry/instrumentation" dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- yarn.lock | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/yarn.lock b/yarn.lock index adbbc6797a04..af50dde468cf 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3886,7 +3886,7 @@ semver "^7.3.2" shimmer "^1.2.1" -"@opentelemetry/instrumentation@0.41.0", "@opentelemetry/instrumentation@^0.41.0", "@opentelemetry/instrumentation@~0.41.0": +"@opentelemetry/instrumentation@0.41.0": version "0.41.0" resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation/-/instrumentation-0.41.0.tgz#a67a7f6d215d8802c2e956907a913b793037e04d" integrity sha512-Ut9SnZfi7MexOk+GHCMjEtYHogIb6v1dfbnq+oTbQj0lOQUSNLtlO6bXwUdtmPhbvrx6bC0AGr1L6g3rNimv9w== @@ -3898,6 +3898,17 @@ shimmer "^1.2.1" tslib "^2.3.1" +"@opentelemetry/instrumentation@^0.41.0", "@opentelemetry/instrumentation@~0.41.0": + version "0.41.2" + resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation/-/instrumentation-0.41.2.tgz#cae11fa64485dcf03dae331f35b315b64bc6189f" + integrity sha512-rxU72E0pKNH6ae2w5+xgVYZLzc5mlxAbGzF4shxMVK8YC2QQsfN38B2GPbj0jvrKWWNUElfclQ+YTykkNg/grw== + dependencies: + "@types/shimmer" "^1.0.2" + import-in-the-middle "1.4.2" + require-in-the-middle "^7.1.1" + semver "^7.5.1" + shimmer "^1.2.1" + "@opentelemetry/propagator-b3@1.15.0": version "1.15.0" resolved "https://registry.yarnpkg.com/@opentelemetry/propagator-b3/-/propagator-b3-1.15.0.tgz#502c3b7030646e7654ce243ae24b67aeac405a53" @@ -15772,6 +15783,16 @@ import-in-the-middle@1.4.1: cjs-module-lexer "^1.2.2" module-details-from-path "^1.0.3" +import-in-the-middle@1.4.2: + version "1.4.2" + resolved "https://registry.yarnpkg.com/import-in-the-middle/-/import-in-the-middle-1.4.2.tgz#2a266676e3495e72c04bbaa5ec14756ba168391b" + integrity sha512-9WOz1Yh/cvO/p69sxRmhyQwrIGGSp7EIdcb+fFNVi7CzQGQB8U1/1XrKVSbEd/GNOAeM0peJtmi7+qphe7NvAw== + dependencies: + acorn "^8.8.2" + acorn-import-assertions "^1.9.0" + cjs-module-lexer "^1.2.2" + module-details-from-path "^1.0.3" + import-lazy@^2.1.0: version "2.1.0" resolved "https://registry.yarnpkg.com/import-lazy/-/import-lazy-2.1.0.tgz#05698e3d45c88e8d7e9d92cb0584e77f096f3e43" From f83b90658a3d8828ba2b6a4da4acedeaa9424db6 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 10 Aug 2023 11:36:21 +0200 Subject: [PATCH 06/15] fix(replay): Handle multiple clicks in a short time (#8773) We assumed that the click debounce should only let one click per second through. however, that does not work if clicking different elements quickly (as it only debounces a single click/element). This adds a guard to ensure we only handle a single click breadcrumb per second. Fixes https://github.com/getsentry/sentry/issues/54293 --- .../replay/src/coreHandlers/handleClick.ts | 8 ++ .../unit/coreHandlers/handleClick.test.ts | 91 +++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/packages/replay/src/coreHandlers/handleClick.ts b/packages/replay/src/coreHandlers/handleClick.ts index cc1d816c5435..c4243c1e7e1b 100644 --- a/packages/replay/src/coreHandlers/handleClick.ts +++ b/packages/replay/src/coreHandlers/handleClick.ts @@ -136,6 +136,14 @@ export class ClickDetector implements ReplayClickDetector { clickCount: 0, node, }; + + // If there was a click in the last 1s on the same element, ignore it - only keep a single reference per second + if ( + this._clicks.some(click => click.node === newClick.node && Math.abs(click.timestamp - newClick.timestamp) < 1) + ) { + return; + } + this._clicks.push(newClick); // If this is the first new click, set a timeout to check for multi clicks diff --git a/packages/replay/test/unit/coreHandlers/handleClick.test.ts b/packages/replay/test/unit/coreHandlers/handleClick.test.ts index f2980e60df69..ae52d2076293 100644 --- a/packages/replay/test/unit/coreHandlers/handleClick.test.ts +++ b/packages/replay/test/unit/coreHandlers/handleClick.test.ts @@ -71,6 +71,97 @@ describe('Unit | coreHandlers | handleClick', () => { expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(1); }); + test('it captures multiple clicks', async () => { + const replay = { + getCurrentRoute: () => 'test-route', + } as ReplayContainer; + + const mockAddBreadcrumbEvent = jest.fn(); + + const detector = new ClickDetector( + replay, + { + threshold: 1_000, + timeout: 3_000, + scrollTimeout: 200, + ignoreSelector: '', + }, + mockAddBreadcrumbEvent, + ); + + const breadcrumb1: Breadcrumb = { + timestamp: BASE_TIMESTAMP / 1000, + data: { + nodeId: 1, + }, + }; + const breadcrumb2: Breadcrumb = { + timestamp: (BASE_TIMESTAMP + 200) / 1000, + data: { + nodeId: 1, + }, + }; + const breadcrumb3: Breadcrumb = { + timestamp: (BASE_TIMESTAMP + 1200) / 1000, + data: { + nodeId: 1, + }, + }; + const node = document.createElement('button'); + detector.handleClick(breadcrumb1, node); + detector.handleClick(breadcrumb2, node); + detector.handleClick(breadcrumb3, node); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + + jest.advanceTimersByTime(1_000); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + + jest.advanceTimersByTime(1_000); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + + jest.advanceTimersByTime(1_000); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(1); + expect(mockAddBreadcrumbEvent).toHaveBeenCalledWith(replay, { + category: 'ui.slowClickDetected', + type: 'default', + data: { + clickCount: 1, + endReason: 'timeout', + nodeId: 1, + route: 'test-route', + timeAfterClickMs: 3000, + url: 'http://localhost/', + }, + message: undefined, + timestamp: BASE_TIMESTAMP / 1000, + }); + + jest.advanceTimersByTime(2_000); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(2); + expect(mockAddBreadcrumbEvent).toHaveBeenLastCalledWith(replay, { + category: 'ui.slowClickDetected', + type: 'default', + data: { + clickCount: 1, + endReason: 'timeout', + nodeId: 1, + route: 'test-route', + timeAfterClickMs: 3000, + url: 'http://localhost/', + }, + message: undefined, + timestamp: (BASE_TIMESTAMP + 1200) / 1000, + }); + + jest.advanceTimersByTime(5_000); + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(2); + }); + test('it captures clicks on different elements', async () => { const replay = { getCurrentRoute: () => 'test-route', From dd6273e91491ce0ad19cb9bce3777e4d7e453fd2 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 10 Aug 2023 12:43:01 +0200 Subject: [PATCH 07/15] fix(nextjs): Execute sentry config independently of `autoInstrumentServerFunctions` and `autoInstrumentAppDirectory` (#8781) --- packages/nextjs/rollup.npm.config.js | 6 +- .../src/config/loaders/wrappingLoader.ts | 32 ++++--- .../templates/sentryInitWrapperTemplate.ts | 11 +++ packages/nextjs/src/config/webpack.ts | 85 +++++++++++++------ 4 files changed, 93 insertions(+), 41 deletions(-) create mode 100644 packages/nextjs/src/config/templates/sentryInitWrapperTemplate.ts diff --git a/packages/nextjs/rollup.npm.config.js b/packages/nextjs/rollup.npm.config.js index f9c498c8f39e..ce15b951235e 100644 --- a/packages/nextjs/rollup.npm.config.js +++ b/packages/nextjs/rollup.npm.config.js @@ -24,11 +24,12 @@ export default [ ...makeNPMConfigVariants( makeBaseNPMConfig({ entrypoints: [ - 'src/config/templates/pageWrapperTemplate.ts', 'src/config/templates/apiWrapperTemplate.ts', 'src/config/templates/middlewareWrapperTemplate.ts', - 'src/config/templates/serverComponentWrapperTemplate.ts', + 'src/config/templates/pageWrapperTemplate.ts', 'src/config/templates/requestAsyncStorageShim.ts', + 'src/config/templates/sentryInitWrapperTemplate.ts', + 'src/config/templates/serverComponentWrapperTemplate.ts', ], packageSpecificConfig: { @@ -47,6 +48,7 @@ export default [ external: [ '@sentry/nextjs', 'next/dist/client/components/request-async-storage', + '__SENTRY_CONFIG_IMPORT_PATH__', '__SENTRY_WRAPPING_TARGET_FILE__', '__SENTRY_NEXTJS_REQUEST_ASYNC_STORAGE_SHIM__', ], diff --git a/packages/nextjs/src/config/loaders/wrappingLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts index e4d58c579420..f2e07ab6537d 100644 --- a/packages/nextjs/src/config/loaders/wrappingLoader.ts +++ b/packages/nextjs/src/config/loaders/wrappingLoader.ts @@ -30,6 +30,9 @@ const requestAsyncStorageShimPath = path.resolve(__dirname, '..', 'templates', ' const requestAsyncStorageModuleExists = moduleExists(NEXTJS_REQUEST_ASYNC_STORAGE_MODULE_PATH); let showedMissingAsyncStorageModuleWarning = false; +const sentryInitWrapperTemplatePath = path.resolve(__dirname, '..', 'templates', 'sentryInitWrapperTemplate.js'); +const sentryInitWrapperTemplateCode = fs.readFileSync(sentryInitWrapperTemplatePath, { encoding: 'utf8' }); + const serverComponentWrapperTemplatePath = path.resolve( __dirname, '..', @@ -43,7 +46,7 @@ type LoaderOptions = { appDir: string; pageExtensionRegex: string; excludeServerRoutes: Array; - wrappingTargetKind: 'page' | 'api-route' | 'middleware' | 'server-component'; + wrappingTargetKind: 'page' | 'api-route' | 'middleware' | 'server-component' | 'sentry-init'; sentryConfigFilePath?: string; vercelCronsConfig?: VercelCronsConfig; }; @@ -83,7 +86,23 @@ export default function wrappingLoader( let templateCode: string; - if (wrappingTargetKind === 'page' || wrappingTargetKind === 'api-route') { + if (wrappingTargetKind === 'sentry-init') { + templateCode = sentryInitWrapperTemplateCode; + + // Absolute paths to the sentry config do not work with Windows: https://github.com/getsentry/sentry-javascript/issues/8133 + // Se we need check whether `this.resourcePath` is absolute because there is no contract by webpack that says it is absolute. + // Examples where `this.resourcePath` could possibly be non-absolute are virtual modules. + if (sentryConfigFilePath && path.isAbsolute(this.resourcePath)) { + const sentryConfigImportPath = path + .relative(path.dirname(this.resourcePath), sentryConfigFilePath) + .replace(/\\/g, '/'); + templateCode = templateCode.replace(/__SENTRY_CONFIG_IMPORT_PATH__/g, sentryConfigImportPath); + } else { + // Bail without doing any wrapping + this.callback(null, userCode, userModuleSourceMap); + return; + } + } else if (wrappingTargetKind === 'page' || wrappingTargetKind === 'api-route') { // Get the parameterized route name from this page's filepath const parameterizedPagesRoute = path.posix .normalize( @@ -207,15 +226,6 @@ export default function wrappingLoader( throw new Error(`Invariant: Could not get template code of unknown kind "${wrappingTargetKind}"`); } - // We check whether `this.resourcePath` is absolute because there is no contract by webpack that says it is absolute, - // however we can only create relative paths to the sentry config from absolute paths.Examples where this could possibly be non - absolute are virtual modules. - if (sentryConfigFilePath && path.isAbsolute(this.resourcePath)) { - const sentryConfigImportPath = path - .relative(path.dirname(this.resourcePath), sentryConfigFilePath) // Absolute paths do not work with Windows: https://github.com/getsentry/sentry-javascript/issues/8133 - .replace(/\\/g, '/'); - templateCode = `import "${sentryConfigImportPath}";\n`.concat(templateCode); - } - // Replace the import path of the wrapping target in the template with a path that the `wrapUserCode` function will understand. templateCode = templateCode.replace(/__SENTRY_WRAPPING_TARGET_FILE__/g, WRAPPING_TARGET_MODULE_NAME); diff --git a/packages/nextjs/src/config/templates/sentryInitWrapperTemplate.ts b/packages/nextjs/src/config/templates/sentryInitWrapperTemplate.ts new file mode 100644 index 000000000000..1720c3b62672 --- /dev/null +++ b/packages/nextjs/src/config/templates/sentryInitWrapperTemplate.ts @@ -0,0 +1,11 @@ +// @ts-ignore This will be replaced with the user's sentry config gile +// eslint-disable-next-line import/no-unresolved +import '__SENTRY_CONFIG_IMPORT_PATH__'; + +// @ts-ignore This is the file we're wrapping +// eslint-disable-next-line import/no-unresolved +export * from '__SENTRY_WRAPPING_TARGET_FILE__'; + +// @ts-ignore This is the file we're wrapping +// eslint-disable-next-line import/no-unresolved +export { default } from '__SENTRY_WRAPPING_TARGET_FILE__'; diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 49f450bb27a4..d711fef5c14f 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -140,19 +140,45 @@ export function constructWebpackConfigFunction( return path.normalize(absoluteResourcePath); }; + const isPageResource = (resourcePath: string): boolean => { + const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); + return ( + normalizedAbsoluteResourcePath.startsWith(pagesDirPath + path.sep) && + !normalizedAbsoluteResourcePath.startsWith(apiRoutesPath + path.sep) && + dotPrefixedPageExtensions.some(ext => normalizedAbsoluteResourcePath.endsWith(ext)) + ); + }; + + const isApiRouteResource = (resourcePath: string): boolean => { + const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); + return ( + normalizedAbsoluteResourcePath.startsWith(apiRoutesPath + path.sep) && + dotPrefixedPageExtensions.some(ext => normalizedAbsoluteResourcePath.endsWith(ext)) + ); + }; + + const isMiddlewareResource = (resourcePath: string): boolean => { + const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); + return normalizedAbsoluteResourcePath === middlewareJsPath || normalizedAbsoluteResourcePath === middlewareTsPath; + }; + + const isServerComponentResource = (resourcePath: string): boolean => { + const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); + + // ".js, .jsx, or .tsx file extensions can be used for Pages" + // https://beta.nextjs.org/docs/routing/pages-and-layouts#pages:~:text=.js%2C%20.jsx%2C%20or%20.tsx%20file%20extensions%20can%20be%20used%20for%20Pages. + return ( + normalizedAbsoluteResourcePath.startsWith(appDirPath + path.sep) && + !!normalizedAbsoluteResourcePath.match(/[\\/](page|layout|loading|head|not-found)\.(js|jsx|tsx)$/) + ); + }; + if (isServer && userSentryOptions.autoInstrumentServerFunctions !== false) { // It is very important that we insert our loaders at the beginning of the array because we expect any sort of transformations/transpilations (e.g. TS -> JS) to already have happened. // Wrap pages newConfig.module.rules.unshift({ - test: resourcePath => { - const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); - return ( - normalizedAbsoluteResourcePath.startsWith(pagesDirPath + path.sep) && - !normalizedAbsoluteResourcePath.startsWith(apiRoutesPath + path.sep) && - dotPrefixedPageExtensions.some(ext => normalizedAbsoluteResourcePath.endsWith(ext)) - ); - }, + test: isPageResource, use: [ { loader: path.resolve(__dirname, 'loaders', 'wrappingLoader.js'), @@ -190,13 +216,7 @@ export function constructWebpackConfigFunction( // Wrap api routes newConfig.module.rules.unshift({ - test: resourcePath => { - const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); - return ( - normalizedAbsoluteResourcePath.startsWith(apiRoutesPath + path.sep) && - dotPrefixedPageExtensions.some(ext => normalizedAbsoluteResourcePath.endsWith(ext)) - ); - }, + test: isApiRouteResource, use: [ { loader: path.resolve(__dirname, 'loaders', 'wrappingLoader.js'), @@ -211,12 +231,7 @@ export function constructWebpackConfigFunction( // Wrap middleware newConfig.module.rules.unshift({ - test: resourcePath => { - const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); - return ( - normalizedAbsoluteResourcePath === middlewareJsPath || normalizedAbsoluteResourcePath === middlewareTsPath - ); - }, + test: isMiddlewareResource, use: [ { loader: path.resolve(__dirname, 'loaders', 'wrappingLoader.js'), @@ -232,14 +247,28 @@ export function constructWebpackConfigFunction( if (isServer && userSentryOptions.autoInstrumentAppDirectory !== false) { // Wrap page server components newConfig.module.rules.unshift({ - test: resourcePath => { - const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); + test: isServerComponentResource, + use: [ + { + loader: path.resolve(__dirname, 'loaders', 'wrappingLoader.js'), + options: { + ...staticWrappingLoaderOptions, + wrappingTargetKind: 'server-component', + }, + }, + ], + }); + } - // ".js, .jsx, or .tsx file extensions can be used for Pages" - // https://beta.nextjs.org/docs/routing/pages-and-layouts#pages:~:text=.js%2C%20.jsx%2C%20or%20.tsx%20file%20extensions%20can%20be%20used%20for%20Pages. + if (isServer) { + // Import the Sentry config in every user file + newConfig.module.rules.unshift({ + test: resourcePath => { return ( - normalizedAbsoluteResourcePath.startsWith(appDirPath + path.sep) && - !!normalizedAbsoluteResourcePath.match(/[\\/](page|layout|loading|head|not-found)\.(js|jsx|tsx)$/) + isPageResource(resourcePath) || + isApiRouteResource(resourcePath) || + isMiddlewareResource(resourcePath) || + isServerComponentResource(resourcePath) ); }, use: [ @@ -247,7 +276,7 @@ export function constructWebpackConfigFunction( loader: path.resolve(__dirname, 'loaders', 'wrappingLoader.js'), options: { ...staticWrappingLoaderOptions, - wrappingTargetKind: 'server-component', + wrappingTargetKind: 'sentry-init', }, }, ], From da1b592a04f644694450c76aa4ca7a9c55ce0d21 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 10 Aug 2023 12:45:39 +0200 Subject: [PATCH 08/15] fix(replay): Fix `hasCheckout` handling (#8782) 1. Make sure it is correctly kept when doing the compression worker switchover 2. Make sure it is correctly set for `session` recordings --- packages/replay/src/eventBuffer/EventBufferProxy.ts | 4 +++- packages/replay/src/util/addEvent.ts | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/replay/src/eventBuffer/EventBufferProxy.ts b/packages/replay/src/eventBuffer/EventBufferProxy.ts index 0b5a6bdfed11..eb49b7229c58 100644 --- a/packages/replay/src/eventBuffer/EventBufferProxy.ts +++ b/packages/replay/src/eventBuffer/EventBufferProxy.ts @@ -99,13 +99,15 @@ export class EventBufferProxy implements EventBuffer { /** Switch the used buffer to the compression worker. */ private async _switchToCompressionWorker(): Promise { - const { events } = this._fallback; + const { events, hasCheckout } = this._fallback; const addEventPromises: Promise[] = []; for (const event of events) { addEventPromises.push(this._compression.addEvent(event)); } + this._compression.hasCheckout = hasCheckout; + // We switch over to the new buffer immediately - any further events will be added // after the previously buffered ones this._used = this._compression; diff --git a/packages/replay/src/util/addEvent.ts b/packages/replay/src/util/addEvent.ts index 6ffd5d03bacd..e9898ee9461c 100644 --- a/packages/replay/src/util/addEvent.ts +++ b/packages/replay/src/util/addEvent.ts @@ -52,6 +52,9 @@ export async function addEvent( try { if (isCheckout && replay.recordingMode === 'buffer') { replay.eventBuffer.clear(); + } + + if (isCheckout) { replay.eventBuffer.hasCheckout = true; } From 0bade5df1b7368fd8303dccf2458bb44cec27086 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 10 Aug 2023 12:49:49 +0200 Subject: [PATCH 09/15] fix(replay): Ensure we do not flush if flush took too long (#8784) Add another safeguard to avoid too-long replays, in the case where flushing takes too long. --- packages/replay/src/replay.ts | 11 ++++- .../replay/test/integration/flush.test.ts | 42 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index dd82e26f3d57..4a0a7e91d97d 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -1067,6 +1067,15 @@ export class ReplayContainer implements ReplayContainerInterface { // Note this empties the event buffer regardless of outcome of sending replay const recordingData = await this.eventBuffer.finish(); + const timestamp = Date.now(); + + // Check total duration again, to avoid sending outdated stuff + // We leave 30s wiggle room to accomodate late flushing etc. + // This _could_ happen when the browser is suspended during flushing, in which case we just want to stop + if (timestamp - this._context.initialTimestamp > this.timeouts.maxSessionLife + 30_000) { + throw new Error('Session is too long, not sending replay'); + } + // NOTE: Copy values from instance members, as it's possible they could // change before the flush finishes. const replayId = this.session.id; @@ -1082,7 +1091,7 @@ export class ReplayContainer implements ReplayContainerInterface { eventContext, session: this.session, options: this.getOptions(), - timestamp: Date.now(), + timestamp, }); } catch (err) { this._handleException(err); diff --git a/packages/replay/test/integration/flush.test.ts b/packages/replay/test/integration/flush.test.ts index 6e6e02d86620..1e0b3b1366d6 100644 --- a/packages/replay/test/integration/flush.test.ts +++ b/packages/replay/test/integration/flush.test.ts @@ -442,4 +442,46 @@ describe('Integration | flush', () => { replay.getOptions()._experiments.traceInternals = false; }); + + /** + * This tests the case where a flush happens in time, + * but something takes too long (e.g. because we are idle, ...) + * so by the time we actually send the replay it's too late. + * In this case, we want to stop the replay. + */ + it('stops if flushing after maxSessionLife', async () => { + replay.timeouts.maxSessionLife = 100_000; + + sessionStorage.clear(); + clearSession(replay); + replay['_loadAndCheckSession'](); + await new Promise(process.nextTick); + jest.setSystemTime(BASE_TIMESTAMP); + + replay.eventBuffer!.clear(); + + // We do not care about this warning here + replay.eventBuffer!.hasCheckout = true; + + // We want to simulate that flushing happens _way_ late + replay['_addPerformanceEntries'] = () => { + return new Promise(resolve => setTimeout(resolve, 140_000)); + }; + + // Add event inside of session life timespan + const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP + 100, type: 2 }; + mockRecord._emitter(TEST_EVENT); + + await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); + await advanceTimers(160_000); + + expect(mockFlush).toHaveBeenCalledTimes(2); + expect(mockSendReplay).toHaveBeenCalledTimes(0); + expect(replay.isEnabled()).toBe(false); + + replay.timeouts.maxSessionLife = MAX_SESSION_LIFE; + + // Start again for following tests + await replay.start(); + }); }); From fdd17e75948b86af1ea7b4d65b5a1adfa07ac363 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 10 Aug 2023 09:23:56 -0400 Subject: [PATCH 10/15] feat(tracing): Improve data collection for mongodb spans (#8774) Align mongodb spans with https://github.com/open-telemetry/semantic-conventions/blob/main/docs/database/mongodb.md needed for starfish views. --- .../auto-instrument/mongodb/test.ts | 42 +++++++++---------- .../tracing/auto-instrument/mongodb/test.ts | 42 +++++++++---------- .../src/node/integrations/mongo.ts | 9 ++-- .../test/integrations/node/mongo.test.ts | 22 +++++----- 4 files changed, 58 insertions(+), 57 deletions(-) diff --git a/packages/node-integration-tests/suites/tracing-new/auto-instrument/mongodb/test.ts b/packages/node-integration-tests/suites/tracing-new/auto-instrument/mongodb/test.ts index 8511841c6da8..2d2ec05553e3 100644 --- a/packages/node-integration-tests/suites/tracing-new/auto-instrument/mongodb/test.ts +++ b/packages/node-integration-tests/suites/tracing-new/auto-instrument/mongodb/test.ts @@ -30,56 +30,56 @@ conditionalTest({ min: 12 })('MongoDB Test', () => { spans: [ { data: { - collectionName: 'movies', - dbName: 'admin', - namespace: 'admin.movies', - doc: '{"title":"Rick and Morty"}', 'db.system': 'mongodb', + 'db.name': 'admin', + 'db.operation': 'insertOne', + 'db.mongodb.collection': 'movies', + 'db.mongodb.doc': '{"title":"Rick and Morty"}', }, description: 'insertOne', op: 'db', }, { data: { - collectionName: 'movies', - dbName: 'admin', - namespace: 'admin.movies', - query: '{"title":"Back to the Future"}', 'db.system': 'mongodb', + 'db.name': 'admin', + 'db.operation': 'findOne', + 'db.mongodb.collection': 'movies', + 'db.mongodb.query': '{"title":"Back to the Future"}', }, description: 'findOne', op: 'db', }, { data: { - collectionName: 'movies', - dbName: 'admin', - namespace: 'admin.movies', - filter: '{"title":"Back to the Future"}', - update: '{"$set":{"title":"South Park"}}', 'db.system': 'mongodb', + 'db.name': 'admin', + 'db.operation': 'updateOne', + 'db.mongodb.collection': 'movies', + 'db.mongodb.filter': '{"title":"Back to the Future"}', + 'db.mongodb.update': '{"$set":{"title":"South Park"}}', }, description: 'updateOne', op: 'db', }, { data: { - collectionName: 'movies', - dbName: 'admin', - namespace: 'admin.movies', - query: '{"title":"South Park"}', 'db.system': 'mongodb', + 'db.name': 'admin', + 'db.operation': 'findOne', + 'db.mongodb.collection': 'movies', + 'db.mongodb.query': '{"title":"South Park"}', }, description: 'findOne', op: 'db', }, { data: { - collectionName: 'movies', - dbName: 'admin', - namespace: 'admin.movies', - query: '{"title":"South Park"}', 'db.system': 'mongodb', + 'db.name': 'admin', + 'db.operation': 'find', + 'db.mongodb.collection': 'movies', + 'db.mongodb.query': '{"title":"South Park"}', }, description: 'find', op: 'db', diff --git a/packages/node-integration-tests/suites/tracing/auto-instrument/mongodb/test.ts b/packages/node-integration-tests/suites/tracing/auto-instrument/mongodb/test.ts index 8511841c6da8..2d2ec05553e3 100644 --- a/packages/node-integration-tests/suites/tracing/auto-instrument/mongodb/test.ts +++ b/packages/node-integration-tests/suites/tracing/auto-instrument/mongodb/test.ts @@ -30,56 +30,56 @@ conditionalTest({ min: 12 })('MongoDB Test', () => { spans: [ { data: { - collectionName: 'movies', - dbName: 'admin', - namespace: 'admin.movies', - doc: '{"title":"Rick and Morty"}', 'db.system': 'mongodb', + 'db.name': 'admin', + 'db.operation': 'insertOne', + 'db.mongodb.collection': 'movies', + 'db.mongodb.doc': '{"title":"Rick and Morty"}', }, description: 'insertOne', op: 'db', }, { data: { - collectionName: 'movies', - dbName: 'admin', - namespace: 'admin.movies', - query: '{"title":"Back to the Future"}', 'db.system': 'mongodb', + 'db.name': 'admin', + 'db.operation': 'findOne', + 'db.mongodb.collection': 'movies', + 'db.mongodb.query': '{"title":"Back to the Future"}', }, description: 'findOne', op: 'db', }, { data: { - collectionName: 'movies', - dbName: 'admin', - namespace: 'admin.movies', - filter: '{"title":"Back to the Future"}', - update: '{"$set":{"title":"South Park"}}', 'db.system': 'mongodb', + 'db.name': 'admin', + 'db.operation': 'updateOne', + 'db.mongodb.collection': 'movies', + 'db.mongodb.filter': '{"title":"Back to the Future"}', + 'db.mongodb.update': '{"$set":{"title":"South Park"}}', }, description: 'updateOne', op: 'db', }, { data: { - collectionName: 'movies', - dbName: 'admin', - namespace: 'admin.movies', - query: '{"title":"South Park"}', 'db.system': 'mongodb', + 'db.name': 'admin', + 'db.operation': 'findOne', + 'db.mongodb.collection': 'movies', + 'db.mongodb.query': '{"title":"South Park"}', }, description: 'findOne', op: 'db', }, { data: { - collectionName: 'movies', - dbName: 'admin', - namespace: 'admin.movies', - query: '{"title":"South Park"}', 'db.system': 'mongodb', + 'db.name': 'admin', + 'db.operation': 'find', + 'db.mongodb.collection': 'movies', + 'db.mongodb.query': '{"title":"South Park"}', }, description: 'find', op: 'db', diff --git a/packages/tracing-internal/src/node/integrations/mongo.ts b/packages/tracing-internal/src/node/integrations/mongo.ts index 637b7d124b33..c462ea4dcdf2 100644 --- a/packages/tracing-internal/src/node/integrations/mongo.ts +++ b/packages/tracing-internal/src/node/integrations/mongo.ts @@ -229,13 +229,14 @@ export class Mongo implements LazyLoadedIntegration { args: unknown[], ): SpanContext { const data: { [key: string]: string } = { - collectionName: collection.collectionName, - dbName: collection.dbName, - namespace: collection.namespace, 'db.system': 'mongodb', + 'db.name': collection.dbName, + 'db.operation': operation, + 'db.mongodb.collection': collection.collectionName, }; const spanContext: SpanContext = { op: 'db', + // TODO v8: Use `${collection.collectionName}.${operation}` description: operation, data, }; @@ -259,7 +260,7 @@ export class Mongo implements LazyLoadedIntegration { data[signature[1]] = typeof reduce === 'string' ? reduce : reduce.name || ''; } else { for (let i = 0; i < signature.length; i++) { - data[signature[i]] = JSON.stringify(args[i]); + data[`db.mongodb.${signature[i]}`] = JSON.stringify(args[i]); } } } catch (_oO) { diff --git a/packages/tracing/test/integrations/node/mongo.test.ts b/packages/tracing/test/integrations/node/mongo.test.ts index 547b7708b55e..4e866f608cb8 100644 --- a/packages/tracing/test/integrations/node/mongo.test.ts +++ b/packages/tracing/test/integrations/node/mongo.test.ts @@ -74,10 +74,10 @@ describe('patchOperation()', () => { expect(scope.getSpan).toBeCalled(); expect(parentSpan.startChild).toBeCalledWith({ data: { - collectionName: 'mockedCollectionName', - dbName: 'mockedDbName', - doc: JSON.stringify(doc), - namespace: 'mockedNamespace', + 'db.mongodb.collection': 'mockedCollectionName', + 'db.name': 'mockedDbName', + 'db.operation': 'insertOne', + 'db.mongodb.doc': JSON.stringify(doc), 'db.system': 'mongodb', }, op: 'db', @@ -93,10 +93,10 @@ describe('patchOperation()', () => { expect(scope.getSpan).toBeCalled(); expect(parentSpan.startChild).toBeCalledWith({ data: { - collectionName: 'mockedCollectionName', - dbName: 'mockedDbName', - doc: JSON.stringify(doc), - namespace: 'mockedNamespace', + 'db.mongodb.collection': 'mockedCollectionName', + 'db.name': 'mockedDbName', + 'db.operation': 'insertOne', + 'db.mongodb.doc': JSON.stringify(doc), 'db.system': 'mongodb', }, op: 'db', @@ -110,9 +110,9 @@ describe('patchOperation()', () => { expect(scope.getSpan).toBeCalled(); expect(parentSpan.startChild).toBeCalledWith({ data: { - collectionName: 'mockedCollectionName', - dbName: 'mockedDbName', - namespace: 'mockedNamespace', + 'db.mongodb.collection': 'mockedCollectionName', + 'db.name': 'mockedDbName', + 'db.operation': 'initializeOrderedBulkOp', 'db.system': 'mongodb', }, op: 'db', From 8f5922daefae282d7ce74c8979982603f10d1f1c Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 10 Aug 2023 09:24:41 -0400 Subject: [PATCH 11/15] feat(tracing): Add db connection attributes for mysql spans (#8775) Align mysql spans with https://github.com/open-telemetry/semantic-conventions/blob/main/docs/database/mssql.md needed for starfish views. --- .../tracing/auto-instrument/mysql/test.ts | 6 ++++ .../src/node/integrations/mysql.ts | 36 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/packages/node-integration-tests/suites/tracing/auto-instrument/mysql/test.ts b/packages/node-integration-tests/suites/tracing/auto-instrument/mysql/test.ts index dbdd658f6ef4..b28c6b613e48 100644 --- a/packages/node-integration-tests/suites/tracing/auto-instrument/mysql/test.ts +++ b/packages/node-integration-tests/suites/tracing/auto-instrument/mysql/test.ts @@ -14,6 +14,9 @@ test('should auto-instrument `mysql` package.', async () => { op: 'db', data: { 'db.system': 'mysql', + 'db.user': 'root', + 'server.address': expect.any(String), + 'server.port': expect.any(Number), }, }, @@ -22,6 +25,9 @@ test('should auto-instrument `mysql` package.', async () => { op: 'db', data: { 'db.system': 'mysql', + 'db.user': 'root', + 'server.address': expect.any(String), + 'server.port': expect.any(Number), }, }, ], diff --git a/packages/tracing-internal/src/node/integrations/mysql.ts b/packages/tracing-internal/src/node/integrations/mysql.ts index 8f85de1c9a8f..77ce6a66cd32 100644 --- a/packages/tracing-internal/src/node/integrations/mysql.ts +++ b/packages/tracing-internal/src/node/integrations/mysql.ts @@ -6,9 +6,18 @@ import type { LazyLoadedIntegration } from './lazy'; import { shouldDisableAutoInstrumentation } from './utils/node-utils'; interface MysqlConnection { + prototype: { + connect: () => void; + }; createQuery: () => void; } +interface MysqlConnectionConfig { + host: string; + port: number; + user: string; +} + /** Tracing integration for node-mysql package */ export class Mysql implements LazyLoadedIntegration { /** @@ -48,6 +57,32 @@ export class Mysql implements LazyLoadedIntegration { return; } + let mySqlConfig: MysqlConnectionConfig | undefined = undefined; + + try { + pkg.prototype.connect = new Proxy(pkg.prototype.connect, { + apply(wrappingTarget, thisArg: { config: MysqlConnectionConfig }, args) { + if (!mySqlConfig) { + mySqlConfig = thisArg.config; + } + return wrappingTarget.apply(thisArg, args); + }, + }); + } catch (e) { + __DEBUG_BUILD__ && logger.error('Mysql Integration was unable to instrument `mysql` config.'); + } + + function spanDataFromConfig(): Record { + if (!mySqlConfig) { + return {}; + } + return { + 'server.address': mySqlConfig.host, + 'server.port': mySqlConfig.port, + 'db.user': mySqlConfig.user, + }; + } + // The original function will have one of these signatures: // function (callback) => void // function (options, callback) => void @@ -60,6 +95,7 @@ export class Mysql implements LazyLoadedIntegration { description: typeof options === 'string' ? options : (options as { sql: string }).sql, op: 'db', data: { + ...spanDataFromConfig(), 'db.system': 'mysql', }, }); From 12932e06db2f2e7c0b9fdcc94c1763f7336f037f Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 10 Aug 2023 09:27:11 -0400 Subject: [PATCH 12/15] feat(tracing): Add db connection attributes for postgres spans (#8778) Align postgres spans with https://github.com/open-telemetry/semantic-conventions/blob/main/docs/database/sql.md needed for starfish views. --- .../suites/tracing/auto-instrument/pg/test.ts | 17 +++++++++ .../src/node/integrations/postgres.ts | 35 ++++++++++++++++--- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/packages/node-integration-tests/suites/tracing/auto-instrument/pg/test.ts b/packages/node-integration-tests/suites/tracing/auto-instrument/pg/test.ts index 359e04d7d5f0..a9d4b56b0b3c 100644 --- a/packages/node-integration-tests/suites/tracing/auto-instrument/pg/test.ts +++ b/packages/node-integration-tests/suites/tracing/auto-instrument/pg/test.ts @@ -1,6 +1,11 @@ import { assertSentryTransaction, TestEnv } from '../../../../utils'; class PgClient { + database?: string = 'test'; + user?: string = 'user'; + host?: string = 'localhost'; + port?: number = 5432; + // https://node-postgres.com/api/client#clientquery public query(_text: unknown, values: unknown, callback?: () => void) { if (typeof callback === 'function') { @@ -42,6 +47,10 @@ test('should auto-instrument `pg` package.', async () => { op: 'db', data: { 'db.system': 'postgresql', + 'db.user': 'user', + 'db.name': 'test', + 'server.address': 'localhost', + 'server.port': 5432, }, }, { @@ -49,6 +58,10 @@ test('should auto-instrument `pg` package.', async () => { op: 'db', data: { 'db.system': 'postgresql', + 'db.user': 'user', + 'db.name': 'test', + 'server.address': 'localhost', + 'server.port': 5432, }, }, { @@ -56,6 +69,10 @@ test('should auto-instrument `pg` package.', async () => { op: 'db', data: { 'db.system': 'postgresql', + 'db.user': 'user', + 'db.name': 'test', + 'server.address': 'localhost', + 'server.port': 5432, }, }, ], diff --git a/packages/tracing-internal/src/node/integrations/postgres.ts b/packages/tracing-internal/src/node/integrations/postgres.ts index 85c021c4f24b..f226c9264938 100644 --- a/packages/tracing-internal/src/node/integrations/postgres.ts +++ b/packages/tracing-internal/src/node/integrations/postgres.ts @@ -11,6 +11,13 @@ interface PgClient { }; } +interface PgClientThis { + database?: string; + host?: string; + port?: number; + user?: string; +} + interface PgOptions { usePgNative?: boolean; } @@ -74,15 +81,35 @@ export class Postgres implements LazyLoadedIntegration { * function (pg.Cursor) => pg.Cursor */ fill(Client.prototype, 'query', function (orig: () => void | Promise) { - return function (this: unknown, config: unknown, values: unknown, callback: unknown) { + return function (this: PgClientThis, config: unknown, values: unknown, callback: unknown) { const scope = getCurrentHub().getScope(); const parentSpan = scope?.getSpan(); + + const data: Record = { + 'db.system': 'postgresql', + }; + + try { + if (this.database) { + data['db.name'] = this.database; + } + if (this.host) { + data['server.address'] = this.host; + } + if (this.port) { + data['server.port'] = this.port; + } + if (this.user) { + data['db.user'] = this.user; + } + } catch (e) { + // ignore + } + const span = parentSpan?.startChild({ description: typeof config === 'string' ? config : (config as { text: string }).text, op: 'db', - data: { - 'db.system': 'postgresql', - }, + data, }); if (typeof callback === 'function') { From ad0cb9bd55c8f94ab498e7099e9769c942587a70 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 10 Aug 2023 15:38:33 +0200 Subject: [PATCH 13/15] feat(node-experimental): Re-export from node (#8786) Just re-export everything manually from node so it can be used in node-experimental. --- packages/node-experimental/src/index.ts | 63 +++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/packages/node-experimental/src/index.ts b/packages/node-experimental/src/index.ts index 74108e528513..3c246bdf2b97 100644 --- a/packages/node-experimental/src/index.ts +++ b/packages/node-experimental/src/index.ts @@ -11,3 +11,66 @@ export { init } from './sdk/init'; export { INTEGRATIONS as Integrations }; export { getAutoPerformanceIntegrations } from './integrations/getAutoPerformanceIntegrations'; export * as Handlers from './sdk/handlers'; + +export { + makeNodeTransport, + defaultStackParser, + getSentryRelease, + addRequestDataToEvent, + DEFAULT_USER_INCLUDES, + extractRequestData, + deepReadDirSync, + getModuleFromFilename, + addGlobalEventProcessor, + addBreadcrumb, + captureException, + captureEvent, + captureMessage, + close, + configureScope, + createTransport, + extractTraceparentData, + flush, + getActiveTransaction, + getHubFromCarrier, + getCurrentHub, + Hub, + lastEventId, + makeMain, + runWithAsyncContext, + Scope, + startTransaction, + SDK_VERSION, + setContext, + setExtra, + setExtras, + setTag, + setTags, + setUser, + spanStatusfromHttpCode, + trace, + withScope, + captureCheckIn, +} from '@sentry/node'; + +export type { + SpanStatusType, + TransactionNamingScheme, + AddRequestDataToEventOptions, + Breadcrumb, + BreadcrumbHint, + PolymorphicRequest, + Request, + SdkInfo, + Event, + EventHint, + Exception, + Session, + SeverityLevel, + Span, + StackFrame, + Stacktrace, + Thread, + Transaction, + User, +} from '@sentry/node'; From fc7344fb0e6e0cd85eb16c905797a3735a9712ad Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 10 Aug 2023 16:54:05 +0200 Subject: [PATCH 14/15] fix(replay): Ensure we do not try to flush when we force stop replay (#8783) In our `stop()` method, we always tried to force a final flush when in `session` mode. However, that may lead to weird behaviour when we internally `stop()` due to a failure - e.g. think a send replay request fails, we do not want to force a flush again in that case. Note that in tests this seems to be generally passing because `flush()` usually has a running `_flushLock` at this time and thus does not attempt to flush again immediately but only schedules a flush later. I added a test that properly failed for this before and is now fixed. --- .../replay/eventBufferError/template.html | 10 +++ .../suites/replay/eventBufferError/test.ts | 89 +++++++++++++++++++ .../utils/replayHelpers.ts | 4 +- packages/replay/src/integration.ts | 2 +- packages/replay/src/replay.ts | 10 +-- packages/replay/src/types/replay.ts | 2 +- packages/replay/src/util/addEvent.ts | 2 +- .../replay/test/integration/flush.test.ts | 3 +- 8 files changed, 110 insertions(+), 12 deletions(-) create mode 100644 packages/browser-integration-tests/suites/replay/eventBufferError/template.html create mode 100644 packages/browser-integration-tests/suites/replay/eventBufferError/test.ts diff --git a/packages/browser-integration-tests/suites/replay/eventBufferError/template.html b/packages/browser-integration-tests/suites/replay/eventBufferError/template.html new file mode 100644 index 000000000000..24fc4828baf1 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/eventBufferError/template.html @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/packages/browser-integration-tests/suites/replay/eventBufferError/test.ts b/packages/browser-integration-tests/suites/replay/eventBufferError/test.ts new file mode 100644 index 000000000000..10e9ad6f7196 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/eventBufferError/test.ts @@ -0,0 +1,89 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../utils/fixtures'; +import { envelopeRequestParser } from '../../../utils/helpers'; +import { + getDecompressedRecordingEvents, + getReplaySnapshot, + isReplayEvent, + REPLAY_DEFAULT_FLUSH_MAX_DELAY, + shouldSkipReplayTest, + waitForReplayRequest, +} from '../../../utils/replayHelpers'; + +sentryTest( + 'should stop recording when running into eventBuffer error', + async ({ getLocalTestPath, page, forceFlushReplay }) => { + if (shouldSkipReplayTest()) { + sentryTest.skip(); + } + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + return route.fulfill({ + status: 200, + }); + }); + + const url = await getLocalTestPath({ testDir: __dirname }); + await page.goto(url); + + await waitForReplayRequest(page); + const replay = await getReplaySnapshot(page); + expect(replay._isEnabled).toBe(true); + + await forceFlushReplay(); + + let called = 0; + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + const event = envelopeRequestParser(route.request()); + + // We only want to count replays here + if (event && isReplayEvent(event)) { + const events = getDecompressedRecordingEvents(route.request()); + // this makes sure we ignore e.g. mouse move events which can otherwise lead to flakes + if (events.length > 0) { + called++; + } + } + + return route.fulfill({ + status: 200, + }); + }); + + called = 0; + + /** + * We test the following here: + * 1. First click should add an event (so the eventbuffer is not empty) + * 2. Second click should throw an error in eventBuffer (which should lead to stopping the replay) + * 3. Nothing should be sent to API, as we stop the replay due to the eventBuffer error. + */ + await page.evaluate(` +window._count = 0; +window._addEvent = window.Replay._replay.eventBuffer.addEvent.bind(window.Replay._replay.eventBuffer); +window.Replay._replay.eventBuffer.addEvent = (...args) => { + window._count++; + if (window._count === 2) { + throw new Error('provoked error'); + } + window._addEvent(...args); +}; +`); + + void page.click('#button1'); + void page.click('#button2'); + + // Should immediately skip retrying and just cancel, no backoff + // This waitForTimeout call should be okay, as we're not checking for any + // further network requests afterwards. + await page.waitForTimeout(REPLAY_DEFAULT_FLUSH_MAX_DELAY + 100); + + expect(called).toBe(0); + + const replay2 = await getReplaySnapshot(page); + + expect(replay2._isEnabled).toBe(false); + }, +); diff --git a/packages/browser-integration-tests/utils/replayHelpers.ts b/packages/browser-integration-tests/utils/replayHelpers.ts index cb05baffb62b..5ddcf0c46e05 100644 --- a/packages/browser-integration-tests/utils/replayHelpers.ts +++ b/packages/browser-integration-tests/utils/replayHelpers.ts @@ -267,7 +267,7 @@ function getOptionsEvents(replayRequest: Request): CustomRecordingEvent[] { return getAllCustomRrwebRecordingEvents(events).filter(data => data.tag === 'options'); } -function getDecompressedRecordingEvents(resOrReq: Request | Response): RecordingSnapshot[] { +export function getDecompressedRecordingEvents(resOrReq: Request | Response): RecordingSnapshot[] { const replayRequest = getRequest(resOrReq); return ( (replayEnvelopeRequestParser(replayRequest, 5) as eventWithTime[]) @@ -302,7 +302,7 @@ const replayEnvelopeRequestParser = (request: Request | null, envelopeIndex = 2) return envelope[envelopeIndex] as Event; }; -const replayEnvelopeParser = (request: Request | null): unknown[] => { +export const replayEnvelopeParser = (request: Request | null): unknown[] => { // https://develop.sentry.dev/sdk/envelopes/ const envelopeBytes = request?.postDataBuffer() || ''; diff --git a/packages/replay/src/integration.ts b/packages/replay/src/integration.ts index 15a39391636c..e4c17ea04ba1 100644 --- a/packages/replay/src/integration.ts +++ b/packages/replay/src/integration.ts @@ -263,7 +263,7 @@ Sentry.init({ replaysOnErrorSampleRate: ${errorSampleRate} })`, return Promise.resolve(); } - return this._replay.stop(); + return this._replay.stop({ forceFlush: this._replay.recordingMode === 'session' }); } /** diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 4a0a7e91d97d..6bba4eb66a0a 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -368,7 +368,7 @@ export class ReplayContainer implements ReplayContainerInterface { * Currently, this needs to be manually called (e.g. for tests). Sentry SDK * does not support a teardown */ - public async stop(reason?: string): Promise { + public async stop({ forceFlush = false, reason }: { forceFlush?: boolean; reason?: string } = {}): Promise { if (!this._isEnabled) { return; } @@ -388,7 +388,7 @@ export class ReplayContainer implements ReplayContainerInterface { this._debouncedFlush.cancel(); // See comment above re: `_isEnabled`, we "force" a flush, ignoring the // `_isEnabled` state of the plugin since it was disabled above. - if (this.recordingMode === 'session') { + if (forceFlush) { await this._flush({ force: true }); } @@ -777,7 +777,7 @@ export class ReplayContainer implements ReplayContainerInterface { this.session = session; if (!this.session.sampled) { - void this.stop('session not refreshed'); + void this.stop({ reason: 'session not refreshed' }); return false; } @@ -1099,7 +1099,7 @@ export class ReplayContainer implements ReplayContainerInterface { // This means we retried 3 times and all of them failed, // or we ran into a problem we don't want to retry, like rate limiting. // In this case, we want to completely stop the replay - otherwise, we may get inconsistent segments - void this.stop('sendReplay'); + void this.stop({ reason: 'sendReplay' }); const client = getCurrentHub().getClient(); @@ -1223,7 +1223,7 @@ export class ReplayContainer implements ReplayContainerInterface { // Stop replay if over the mutation limit if (overMutationLimit) { - void this.stop('mutationLimit'); + void this.stop({ reason: 'mutationLimit', forceFlush: this.recordingMode === 'session' }); return false; } diff --git a/packages/replay/src/types/replay.ts b/packages/replay/src/types/replay.ts index 1fbf44aa1b95..00f363003979 100644 --- a/packages/replay/src/types/replay.ts +++ b/packages/replay/src/types/replay.ts @@ -451,7 +451,7 @@ export interface ReplayContainer { getContext(): InternalEventContext; initializeSampling(): void; start(): void; - stop(reason?: string): Promise; + stop(options?: { reason?: string; forceflush?: boolean }): Promise; pause(): void; resume(): void; startRecording(): void; diff --git a/packages/replay/src/util/addEvent.ts b/packages/replay/src/util/addEvent.ts index e9898ee9461c..9024c3cbb6bd 100644 --- a/packages/replay/src/util/addEvent.ts +++ b/packages/replay/src/util/addEvent.ts @@ -71,7 +71,7 @@ export async function addEvent( const reason = error && error instanceof EventBufferSizeExceededError ? 'addEventSizeExceeded' : 'addEvent'; __DEBUG_BUILD__ && logger.error(error); - await replay.stop(reason); + await replay.stop({ reason }); const client = getCurrentHub().getClient(); diff --git a/packages/replay/test/integration/flush.test.ts b/packages/replay/test/integration/flush.test.ts index 1e0b3b1366d6..29ce2ba527fd 100644 --- a/packages/replay/test/integration/flush.test.ts +++ b/packages/replay/test/integration/flush.test.ts @@ -472,10 +472,9 @@ describe('Integration | flush', () => { const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP + 100, type: 2 }; mockRecord._emitter(TEST_EVENT); - await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); await advanceTimers(160_000); - expect(mockFlush).toHaveBeenCalledTimes(2); + expect(mockFlush).toHaveBeenCalledTimes(1); expect(mockSendReplay).toHaveBeenCalledTimes(0); expect(replay.isEnabled()).toBe(false); From 3439d3991f14dcfb0f0337f49cb854fdfad79606 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 10 Aug 2023 17:05:59 +0200 Subject: [PATCH 15/15] meta(changelog): Update changelog for 7.63.0 --- CHANGELOG.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb8cc846b983..83c74ac68f4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,21 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 7.63.0 + +- build(deps): bump @opentelemetry/instrumentation from 0.41.0 to 0.41.2 +- feat(eventbuilder): Export `exceptionFromError` for use in hybrid SDKs (#8766) +- feat(node-experimental): Re-export from node (#8786) +- feat(tracing): Add db connection attributes for mysql spans (#8775) +- feat(tracing): Add db connection attributes for postgres spans (#8778) +- feat(tracing): Improve data collection for mongodb spans (#8774) +- fix(nextjs): Execute sentry config independently of `autoInstrumentServerFunctions` and `autoInstrumentAppDirectory` (#8781) +- fix(replay): Ensure we do not flush if flush took too long (#8784) +- fix(replay): Ensure we do not try to flush when we force stop replay (#8783) +- fix(replay): Fix `hasCheckout` handling (#8782) +- fix(replay): Handle multiple clicks in a short time (#8773) +- ref(replay): Skip events being added too long after initial segment (#8768) + ## 7.62.0 ### Important Changes