From 1bf073f99b1dda7872dd1e9924470893f7c2ba07 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 8 Jan 2024 17:45:11 +0100 Subject: [PATCH] feat: Add support for performance metrics (#811) --- src/common/ipc.ts | 14 +++++++ src/main/ipc.ts | 39 +++++++++++++++-- src/preload/index.ts | 3 +- src/preload/legacy.ts | 3 +- src/renderer/index.ts | 13 +++++- .../integrations/metrics-aggregator.ts | 24 +++++++++++ src/renderer/ipc.ts | 14 ++++++- src/renderer/metrics.ts | 42 +++++++++++++++++++ src/renderer/sdk.ts | 3 +- test/e2e/recipe/normalize.ts | 4 ++ test/e2e/server/index.ts | 12 +++++- test/e2e/test-apps/other/metrics/event.json | 6 +++ test/e2e/test-apps/other/metrics/package.json | 8 ++++ test/e2e/test-apps/other/metrics/recipe.yml | 3 ++ .../test-apps/other/metrics/src/index.html | 14 +++++++ test/e2e/test-apps/other/metrics/src/main.js | 32 ++++++++++++++ 16 files changed, 223 insertions(+), 11 deletions(-) create mode 100644 src/renderer/integrations/metrics-aggregator.ts create mode 100644 src/renderer/metrics.ts create mode 100644 test/e2e/test-apps/other/metrics/event.json create mode 100644 test/e2e/test-apps/other/metrics/package.json create mode 100644 test/e2e/test-apps/other/metrics/recipe.yml create mode 100644 test/e2e/test-apps/other/metrics/src/index.html create mode 100644 test/e2e/test-apps/other/metrics/src/main.js diff --git a/src/common/ipc.ts b/src/common/ipc.ts index ac26f837..76f86547 100644 --- a/src/common/ipc.ts +++ b/src/common/ipc.ts @@ -1,3 +1,5 @@ +import { MeasurementUnit, Primitive } from '@sentry/types'; + export const PROTOCOL_SCHEME = 'sentry-ipc'; export enum IPCChannel { @@ -11,6 +13,8 @@ export enum IPCChannel { ENVELOPE = 'sentry-electron.envelope', /** IPC to pass renderer status updates */ STATUS = 'sentry-electron.status', + /** IPC to pass renderer metric additions to the main process */ + ADD_METRIC = 'sentry-electron.add-metric', } export interface RendererProcessAnrOptions { @@ -39,12 +43,22 @@ export interface RendererStatus { config: RendererProcessAnrOptions; } +export interface MetricIPCMessage { + metricType: 'c' | 'g' | 's' | 'd'; + name: string; + value: number | string; + unit?: MeasurementUnit; + tags?: Record; + timestamp?: number; +} + export interface IPCInterface { sendRendererStart: () => void; sendScope: (scope: string) => void; sendEvent: (event: string) => void; sendEnvelope: (evn: Uint8Array | string) => void; sendStatus: (state: RendererStatus) => void; + sendAddMetric: (metric: MetricIPCMessage) => void; } export const RENDERER_ID_HEADER = 'sentry-electron-renderer-id'; diff --git a/src/main/ipc.ts b/src/main/ipc.ts index 49748cf7..6b17547b 100644 --- a/src/main/ipc.ts +++ b/src/main/ipc.ts @@ -1,5 +1,14 @@ -import { captureEvent, getCurrentHub, getCurrentScope } from '@sentry/core'; -import { Attachment, AttachmentItem, Envelope, Event, EventItem, Profile, ScopeData } from '@sentry/types'; +import { BaseClient, captureEvent, getClient, getCurrentScope } from '@sentry/core'; +import { + Attachment, + AttachmentItem, + ClientOptions, + Envelope, + Event, + EventItem, + Profile, + ScopeData, +} from '@sentry/types'; import { forEachEnvelopeItem, logger, parseEnvelope, SentryError } from '@sentry/utils'; import { app, ipcMain, protocol, WebContents, webContents } from 'electron'; import { TextDecoder, TextEncoder } from 'util'; @@ -8,6 +17,7 @@ import { IPCChannel, IPCMode, mergeEvents, + MetricIPCMessage, normalizeUrlsInReplayEnvelope, PROTOCOL_SCHEME, RendererStatus, @@ -127,7 +137,26 @@ function handleEnvelope(options: ElectronMainOptionsInternal, env: Uint8Array | } else { const normalizedEnvelope = normalizeUrlsInReplayEnvelope(envelope, app.getAppPath()); // Pass other types of envelope straight to the transport - void getCurrentHub().getClient()?.getTransport()?.send(normalizedEnvelope); + void getClient()?.getTransport()?.send(normalizedEnvelope); + } +} + +function handleMetric(metric: MetricIPCMessage): void { + const client = getClient>(); + + if (client?.metricsAggregator) { + client.metricsAggregator.add( + metric.metricType, + metric.name, + metric.value, + metric.unit, + metric.tags, + metric.timestamp, + ); + } else { + logger.warn( + `Metric was dropped because the aggregator is not configured in the main process. Enable via '_experiments.metricsAggregator: true' in your init call.`, + ); } } @@ -208,6 +237,8 @@ function configureProtocol(options: ElectronMainOptionsInternal): void { handleScope(options, data.toString()); } else if (request.url.startsWith(`${PROTOCOL_SCHEME}://${IPCChannel.ENVELOPE}`) && data) { handleEnvelope(options, data, getWebContents()); + } else if (request.url.startsWith(`${PROTOCOL_SCHEME}://${IPCChannel.ADD_METRIC}`) && data) { + handleMetric(JSON.parse(data.toString()) as MetricIPCMessage); } else if (request.url.startsWith(`${PROTOCOL_SCHEME}://${IPCChannel.STATUS}`) && data) { const contents = getWebContents(); if (contents) { @@ -245,6 +276,8 @@ function configureClassic(options: ElectronMainOptionsInternal): void { const rendererStatusChanged = createRendererAnrStatusHandler(); ipcMain.on(IPCChannel.STATUS, ({ sender }, status: RendererStatus) => rendererStatusChanged(status, sender)); + + ipcMain.on(IPCChannel.ADD_METRIC, (_, metric: MetricIPCMessage) => handleMetric(metric)); } /** Sets up communication channels with the renderer */ diff --git a/src/preload/index.ts b/src/preload/index.ts index 78f91f99..1fecd68d 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -4,7 +4,7 @@ import { contextBridge, ipcRenderer } from 'electron'; -import { IPCChannel, RendererStatus } from '../common/ipc'; +import { IPCChannel, MetricIPCMessage, RendererStatus } from '../common/ipc'; // eslint-disable-next-line no-restricted-globals if (window.__SENTRY_IPC__) { @@ -17,6 +17,7 @@ if (window.__SENTRY_IPC__) { sendEvent: (eventJson: string) => ipcRenderer.send(IPCChannel.EVENT, eventJson), sendEnvelope: (envelope: Uint8Array | string) => ipcRenderer.send(IPCChannel.ENVELOPE, envelope), sendStatus: (status: RendererStatus) => ipcRenderer.send(IPCChannel.STATUS, status), + sendAddMetric: (metric: MetricIPCMessage) => ipcRenderer.send(IPCChannel.ADD_METRIC, metric), }; // eslint-disable-next-line no-restricted-globals diff --git a/src/preload/legacy.ts b/src/preload/legacy.ts index 9a9d4d15..cc9e8589 100644 --- a/src/preload/legacy.ts +++ b/src/preload/legacy.ts @@ -5,7 +5,7 @@ import { contextBridge, crashReporter, ipcRenderer } from 'electron'; import * as electron from 'electron'; -import { IPCChannel, RendererStatus } from '../common/ipc'; +import { IPCChannel, MetricIPCMessage, RendererStatus } from '../common/ipc'; // eslint-disable-next-line no-restricted-globals if (window.__SENTRY_IPC__) { @@ -28,6 +28,7 @@ if (window.__SENTRY_IPC__) { sendEvent: (eventJson: string) => ipcRenderer.send(IPCChannel.EVENT, eventJson), sendEnvelope: (envelope: Uint8Array | string) => ipcRenderer.send(IPCChannel.ENVELOPE, envelope), sendStatus: (status: RendererStatus) => ipcRenderer.send(IPCChannel.STATUS, status), + sendAddMetric: (metric: MetricIPCMessage) => ipcRenderer.send(IPCChannel.ADD_METRIC, metric), }; // eslint-disable-next-line no-restricted-globals diff --git a/src/renderer/index.ts b/src/renderer/index.ts index 34211124..1c9a7335 100644 --- a/src/renderer/index.ts +++ b/src/renderer/index.ts @@ -69,10 +69,19 @@ export { startInactiveSpan, startSpanManual, continueTrace, - metrics, } from '@sentry/core'; export type { SpanStatusType } from '@sentry/core'; +import { metrics as coreMetrics } from '@sentry/core'; + +import { MetricsAggregator } from './integrations/metrics-aggregator'; + +export const metrics = { + ...coreMetrics, + // Override the default browser metrics aggregator with the Electron renderer one + MetricsAggregator, +}; + export { addTracingExtensions, BrowserClient, @@ -86,5 +95,5 @@ export { // eslint-disable-next-line deprecation/deprecation export type { BrowserOptions, ReportDialogOptions } from '@sentry/browser'; -export const Integrations = { ...ElectronRendererIntegrations, ...BrowserIntegrations }; +export const Integrations = { ...BrowserIntegrations, ...ElectronRendererIntegrations }; export { init, defaultIntegrations } from './sdk'; diff --git a/src/renderer/integrations/metrics-aggregator.ts b/src/renderer/integrations/metrics-aggregator.ts new file mode 100644 index 00000000..a52faf81 --- /dev/null +++ b/src/renderer/integrations/metrics-aggregator.ts @@ -0,0 +1,24 @@ +import { BrowserClient } from '@sentry/browser'; +import { convertIntegrationFnToClass } from '@sentry/core'; +import type { IntegrationFn } from '@sentry/types'; + +import { ElectronRendererMetricsAggregator } from '../metrics'; + +const INTEGRATION_NAME = 'MetricsAggregator'; + +const metricsAggregatorIntegration: IntegrationFn = () => { + return { + name: INTEGRATION_NAME, + setup(client: BrowserClient) { + client.metricsAggregator = new ElectronRendererMetricsAggregator(); + }, + }; +}; + +/** + * Enables Sentry metrics monitoring. + * + * @experimental This API is experimental and might having breaking changes in the future. + */ +// eslint-disable-next-line deprecation/deprecation +export const MetricsAggregator = convertIntegrationFnToClass(INTEGRATION_NAME, metricsAggregatorIntegration); diff --git a/src/renderer/ipc.ts b/src/renderer/ipc.ts index 549874aa..2b72eea4 100644 --- a/src/renderer/ipc.ts +++ b/src/renderer/ipc.ts @@ -2,7 +2,14 @@ /* eslint-disable no-console */ import { logger, uuid4 } from '@sentry/utils'; -import { IPCChannel, IPCInterface, PROTOCOL_SCHEME, RENDERER_ID_HEADER, RendererStatus } from '../common/ipc'; +import { + IPCChannel, + IPCInterface, + MetricIPCMessage, + PROTOCOL_SCHEME, + RENDERER_ID_HEADER, + RendererStatus, +} from '../common/ipc'; function buildUrl(channel: IPCChannel): string { // We include sentry_key in the URL so these don't end up in fetch breadcrumbs @@ -52,6 +59,11 @@ function getImplementation(): IPCInterface { // ignore }); }, + sendAddMetric: (metric: MetricIPCMessage) => { + fetch(buildUrl(IPCChannel.ADD_METRIC), { method: 'POST', body: JSON.stringify(metric), headers }).catch(() => { + // ignore + }); + }, }; } } diff --git a/src/renderer/metrics.ts b/src/renderer/metrics.ts new file mode 100644 index 00000000..83648689 --- /dev/null +++ b/src/renderer/metrics.ts @@ -0,0 +1,42 @@ +import { MeasurementUnit, MetricsAggregator, Primitive } from '@sentry/types'; + +import { IPCInterface } from '../common/ipc'; +import { getIPC } from './ipc'; + +/** + * Sends metrics to the Electron main process where they can be aggregated in a single process + */ +export class ElectronRendererMetricsAggregator implements MetricsAggregator { + private readonly _ipc: IPCInterface; + + public constructor() { + this._ipc = getIPC(); + } + + /** @inheritdoc */ + public add( + metricType: 'c' | 'g' | 's' | 'd', + name: string, + value: string | number, + unit?: MeasurementUnit | undefined, + tags?: Record | undefined, + timestamp?: number | undefined, + ): void { + this._ipc.sendAddMetric({ metricType, name, value, unit, tags, timestamp }); + } + + /** @inheritdoc */ + public flush(): void { + // do nothing + } + + /** @inheritdoc */ + public close(): void { + // do nothing + } + + /** @inheritdoc */ + public toString(): string { + return ''; + } +} diff --git a/src/renderer/sdk.ts b/src/renderer/sdk.ts index 38b05809..e29a45ef 100644 --- a/src/renderer/sdk.ts +++ b/src/renderer/sdk.ts @@ -9,10 +9,11 @@ import { logger } from '@sentry/utils'; import { ensureProcess, RendererProcessAnrOptions } from '../common'; import { enableAnrRendererMessages } from './anr'; import { ScopeToMain } from './integrations'; +import { MetricsAggregator } from './integrations/metrics-aggregator'; import { electronRendererStackParser } from './stack-parse'; import { makeRendererTransport } from './transport'; -export const defaultIntegrations = [...defaultBrowserIntegrations, new ScopeToMain()]; +export const defaultIntegrations = [...defaultBrowserIntegrations, new ScopeToMain(), new MetricsAggregator()]; interface ElectronRendererOptions extends BrowserOptions { /** diff --git a/test/e2e/recipe/normalize.ts b/test/e2e/recipe/normalize.ts index 535ec10c..e4d63413 100644 --- a/test/e2e/recipe/normalize.ts +++ b/test/e2e/recipe/normalize.ts @@ -13,6 +13,10 @@ export function normalize(event: TestServerEvent) } normalizeProfile(event.profile); + + if (event.metrics) { + event.metrics = event.metrics.replace(/T\d{1,10}\n/g, 'T0000000000\n'); + } } export function eventIsSession(data: EventOrSession): boolean { diff --git a/test/e2e/server/index.ts b/test/e2e/server/index.ts index 00e501f3..3e1b7be4 100644 --- a/test/e2e/server/index.ts +++ b/test/e2e/server/index.ts @@ -40,6 +40,8 @@ export interface TestServerEvent { attachments?: Attachment[]; /** Profiling data */ profile?: Profile; + /** Metrics data */ + metrics?: string; /** API method used for submission */ method: 'envelope' | 'minidump' | 'store'; } @@ -124,6 +126,7 @@ export class TestServer { let data: Event | Transaction | Session | ReplayEvent | undefined; const attachments: Attachment[] = []; let profile: Profile | undefined; + let metrics: string | undefined; forEachEnvelopeItem(envelope, ([headers, item]) => { if (headers.type === 'event' || headers.type === 'transaction' || headers.type === 'session') { @@ -142,16 +145,21 @@ export class TestServer { attachments.push(headers); } + if (headers.type === 'statsd') { + metrics = item.toString(); + } + if (headers.type === 'profile') { profile = item as unknown as Profile; } }); - if (data) { + if (data || metrics) { this._addEvent({ - data, + data: data || {}, attachments, profile, + metrics, appId: ctx.params.id, sentryKey: keyMatch[1], method: 'envelope', diff --git a/test/e2e/test-apps/other/metrics/event.json b/test/e2e/test-apps/other/metrics/event.json new file mode 100644 index 00000000..9570aaf3 --- /dev/null +++ b/test/e2e/test-apps/other/metrics/event.json @@ -0,0 +1,6 @@ +{ + "method": "envelope", + "sentryKey": "37f8a2ee37c0409d8970bc7559c7c7e4", + "appId": "277345", + "metrics": "parallel_requests@none:2:2:2:2:1|g|#release:metrics@1.0.0,environment:development,type:a|T0000000000\nhits@none:4|c|T0000000000\n" +} diff --git a/test/e2e/test-apps/other/metrics/package.json b/test/e2e/test-apps/other/metrics/package.json new file mode 100644 index 00000000..c0ca402b --- /dev/null +++ b/test/e2e/test-apps/other/metrics/package.json @@ -0,0 +1,8 @@ +{ + "name": "metrics", + "version": "1.0.0", + "main": "src/main.js", + "dependencies": { + "@sentry/electron": "3.0.0" + } +} diff --git a/test/e2e/test-apps/other/metrics/recipe.yml b/test/e2e/test-apps/other/metrics/recipe.yml new file mode 100644 index 00000000..3ce62df7 --- /dev/null +++ b/test/e2e/test-apps/other/metrics/recipe.yml @@ -0,0 +1,3 @@ +description: Metrics +command: yarn + diff --git a/test/e2e/test-apps/other/metrics/src/index.html b/test/e2e/test-apps/other/metrics/src/index.html new file mode 100644 index 00000000..78319a0a --- /dev/null +++ b/test/e2e/test-apps/other/metrics/src/index.html @@ -0,0 +1,14 @@ + + + + + + + + + diff --git a/test/e2e/test-apps/other/metrics/src/main.js b/test/e2e/test-apps/other/metrics/src/main.js new file mode 100644 index 00000000..ad6eb1b1 --- /dev/null +++ b/test/e2e/test-apps/other/metrics/src/main.js @@ -0,0 +1,32 @@ +const path = require('path'); + +const { app, BrowserWindow } = require('electron'); +const Sentry = require('@sentry/electron'); + +Sentry.init({ + dsn: '__DSN__', + debug: true, + autoSessionTracking: false, + onFatalError: () => {}, + _experiments: { + metricsAggregator: true, + }, +}); + +Sentry.metrics.gauge('parallel_requests', 2, { tags: { type: 'a' } }); + +app.on('ready', () => { + const mainWindow = new BrowserWindow({ + show: false, + webPreferences: { + nodeIntegration: true, + contextIsolation: false, + }, + }); + + mainWindow.loadFile(path.join(__dirname, 'index.html')); + + setTimeout(() => { + Sentry.flush(2000); + }, 2000); +});