Skip to content

Commit

Permalink
Try and improve options
Browse files Browse the repository at this point in the history
Fix test
  • Loading branch information
timfish committed Dec 7, 2021
1 parent 79d3841 commit 1864d1d
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 65 deletions.
23 changes: 10 additions & 13 deletions src/main/integrations/electron-breadcrumbs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,43 +10,43 @@ import { ElectronMainOptions } from '../sdk';
type EventFunction = (name: string) => boolean;
type EventTypes = boolean | string[] | EventFunction | undefined;

interface ElectronBreadcrumbsOptions<EventTypes> {
interface ElectronBreadcrumbsOptions<T> {
/**
* app events
*
* default: (name) => !name.startsWith('remote-')
*/
app: EventTypes;
app: T;
/**
* autoUpdater events
*
* default: all
*/
autoUpdater: EventTypes;
autoUpdater: T;
/**
* webContents events
* default: ['dom-ready', 'context-menu', 'load-url', 'destroyed']
*/
webContents: EventTypes;
webContents: T;
/**
* BrowserWindow events
*
* default: ['closed', 'close', 'unresponsive', 'responsive', 'show', 'blur', 'focus', 'hide',
* 'maximize', 'minimize', 'restore', 'enter-full-screen', 'leave-full-screen' ]
*/
browserWindow: EventTypes;
browserWindow: T;
/**
* screen events
*
* default: all
*/
screen: EventTypes;
screen: T;
/**
* powerMonitor events
*
* default: all
*/
powerMonitor: EventTypes;
powerMonitor: T;
}

const DEFAULT_OPTIONS: ElectronBreadcrumbsOptions<EventFunction> = {
Expand Down Expand Up @@ -97,16 +97,13 @@ export class ElectronBreadcrumbs implements Integration {
/** @inheritDoc */
public name: string = ElectronBreadcrumbs.id;

private readonly _options: ElectronBreadcrumbsOptions<EventFunction | undefined>;
private readonly _options: ElectronBreadcrumbsOptions<EventFunction | false>;

/**
* @param _options Integration options
*/
public constructor(options: Partial<ElectronBreadcrumbsOptions<EventTypes>> = {}) {
this._options = {
...normalizeOptions(options),
...DEFAULT_OPTIONS,
};
this._options = { ...DEFAULT_OPTIONS, ...normalizeOptions(options) };
}

/** @inheritDoc */
Expand Down Expand Up @@ -169,7 +166,7 @@ export class ElectronBreadcrumbs implements Integration {
private _patchEventEmitter(
emitter: NodeJS.EventEmitter | WebContents | BrowserWindow,
category: string,
shouldCapture: EventFunction | undefined,
shouldCapture: EventFunction | undefined | false,
): void {
const emit = emitter.emit.bind(emitter) as (event: string, ...args: unknown[]) => boolean;

Expand Down
101 changes: 58 additions & 43 deletions src/main/integrations/net-breadcrumbs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,46 +5,62 @@ import { fill } from '@sentry/utils';
import { ClientRequest, ClientRequestConstructorOptions, IncomingMessage, net } from 'electron';
import * as urlModule from 'url';

type OrBool<T> = {
[P in keyof T]: T[P] | boolean;
};

type OrFalse<T> = {
[P in keyof T]: T[P] | false;
};

type ShouldTraceFn = (method: string, url: string) => boolean;

interface NetOptions {
breadcrumbs: boolean;
tracing: ShouldTraceFn;
tracingOrigins: ShouldTraceFn;
}

const DEFAULT_OPTIONS: NetOptions = {
breadcrumbs: true,
tracing: (_method, _url) => true,
tracingOrigins: (_method, _url) => true,
};

/** Converts all user supplied options to T | false */
export function normalizeOptions(options: Partial<OrBool<NetOptions>>): Partial<OrFalse<NetOptions>> {
return (Object.keys(options) as (keyof NetOptions)[]).reduce((obj, k) => {
if (typeof options[k] === 'function' || options[k] === false) {
obj[k] = options[k] as boolean & (false | ShouldTraceFn);
}
return obj;
}, {} as Partial<OrFalse<NetOptions>>);
}

/** http module integration */
export class Net implements Integration {
/**
* @inheritDoc
*/
/** @inheritDoc */
public static id: string = 'Net';

/**
* @inheritDoc
*/
/** @inheritDoc */
public name: string = Net.id;

/**
* @inheritDoc
*/
private readonly _breadcrumbs: boolean;

/**
* @inheritDoc
*/
private readonly _tracing: boolean;

/**
* @inheritDoc
*/
public constructor(options: { breadcrumbs?: boolean; tracing?: boolean } = {}) {
this._breadcrumbs = typeof options.breadcrumbs === 'undefined' ? true : options.breadcrumbs;
this._tracing = typeof options.tracing === 'undefined' ? true : options.tracing;
private readonly _options: OrFalse<NetOptions>;

/** @inheritDoc */
public constructor(options: Partial<OrBool<NetOptions>> = {}) {
this._options = {
...DEFAULT_OPTIONS,
...normalizeOptions(options),
};
}

/**
* @inheritDoc
*/
/** @inheritDoc */
public setupOnce(): void {
// No need to instrument if we don't want to track anything
if (!this._breadcrumbs && !this._tracing) {
return;
if (this._options.breadcrumbs || this._options.tracing) {
fill(net, 'request', createWrappedRequestFactory(this._options));
}

fill(net, 'request', createWrappedRequestFactory(this._breadcrumbs, this._tracing));
}
}

Expand Down Expand Up @@ -98,17 +114,14 @@ type RequestMethod = (opt: RequestOptions) => ClientRequest;
type WrappedRequestMethodFactory = (original: RequestMethod) => RequestMethod;

/** */
function createWrappedRequestFactory(
breadcrumbsEnabled: boolean,
tracingEnabled: boolean,
): WrappedRequestMethodFactory {
function createWrappedRequestFactory(options: OrFalse<NetOptions>): WrappedRequestMethodFactory {
return function wrappedRequestMethodFactory(originalRequestMethod: RequestMethod): RequestMethod {
return function requestMethod(this: typeof net, options: RequestOptions): ClientRequest {
return function requestMethod(this: typeof net, reqOptions: RequestOptions): ClientRequest {
// eslint-disable-next-line @typescript-eslint/no-this-alias
const netModule = this;

const { url, method } = parseOptions(options);
const request = originalRequestMethod.apply(netModule, [options]) as ClientRequest;
const { url, method } = parseOptions(reqOptions);
const request = originalRequestMethod.apply(netModule, [reqOptions]) as ClientRequest;

if (url.match(/sentry_key/) || request.getHeader('x-sentry-auth')) {
return request;
Expand All @@ -117,27 +130,29 @@ function createWrappedRequestFactory(
let span: Span | undefined;

const scope = getCurrentHub().getScope();
if (scope && tracingEnabled) {
if (scope && options.tracing && options.tracing(method, url)) {
const parentSpan = scope.getSpan();

if (parentSpan) {
span = parentSpan.startChild({
description: `${method} ${url}`,
op: 'request',
op: 'http.client',
});

request.setHeader('sentry-trace', span.toTraceparent());
if (options.tracingOrigins && options.tracingOrigins(method, url)) {
request.setHeader('sentry-trace', span.toTraceparent());
}
}
}

return request
.once('response', function (this: ClientRequest, res: IncomingMessage): void {
// eslint-disable-next-line @typescript-eslint/no-this-alias
const req = this;
if (breadcrumbsEnabled) {
if (options.breadcrumbs) {
addRequestBreadcrumb('response', method, url, req, res);
}
if (tracingEnabled && span) {
if (span) {
if (res.statusCode) {
span.setHttpStatus(res.statusCode);
}
Expand All @@ -148,10 +163,10 @@ function createWrappedRequestFactory(
// eslint-disable-next-line @typescript-eslint/no-this-alias
const req = this;

if (breadcrumbsEnabled) {
if (options.breadcrumbs) {
addRequestBreadcrumb('error', method, url, req, undefined);
}
if (tracingEnabled && span) {
if (span) {
span.setHttpStatus(500);
span.finish();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
},
"spans": [
{
"op": "request",
"op": "http.client",
"trace_id": "{{id}}",
"parent_span_id": "{{id}}",
"span_id": "{{id}}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
"main": "src/main.js",
"dependencies": {
"@sentry/electron": "3.0.0",
"@sentry/tracing": "6.13.3",
"electron-fetch": "1.7.4"
}
}
14 changes: 7 additions & 7 deletions test/e2e/test-apps/other/net-breadcrumbs-tracing/src/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ init({
onFatalError: () => {},
});

app.on('ready', async () => {
app.on('ready', () => {
const transaction = startTransaction({ name: 'some-transaction' });
getCurrentHub().configureScope((scope) => scope.setSpan(transaction));

await fetch.default('http://localhost:8123/something');
fetch.default('http://localhost:8123/something').then(() => {
transaction.finish();

transaction.finish();

setTimeout(() => {
app.quit();
}, 1000);
setTimeout(() => {
app.quit();
}, 1000);
});
});

0 comments on commit 1864d1d

Please sign in to comment.