Skip to content

ref(node): Refactor node integrations to use processEvent #9018

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 28 additions & 24 deletions packages/node/src/integrations/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import type {
CultureContext,
DeviceContext,
Event,
EventProcessor,
Integration,
OsContext,
} from '@sentry/types';
Expand Down Expand Up @@ -60,20 +59,25 @@ export class Context implements Integration {
},
) {}

/**
* @inheritDoc
*/
public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void): void {
addGlobalEventProcessor(event => this.addContext(event));
/** @inheritDoc */
public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void {
// noop
}

/** Processes an event and adds context */
/** @inheritDoc */
public processEvent(event: Event): Promise<Event> {
return this.addContext(event);
}

/**
* Processes an event and adds context.
*/
public async addContext(event: Event): Promise<Event> {
if (this._cachedContext === undefined) {
this._cachedContext = this._getContexts();
}

const updatedContext = this._updateContext(await this._cachedContext);
const updatedContext = _updateContext(await this._cachedContext);

event.contexts = {
...event.contexts,
Expand All @@ -87,22 +91,6 @@ export class Context implements Integration {
return event;
}

/**
* Updates the context with dynamic values that can change
*/
private _updateContext(contexts: Contexts): Contexts {
// Only update properties if they exist
if (contexts?.app?.app_memory) {
contexts.app.app_memory = process.memoryUsage().rss;
}

if (contexts?.device?.free_memory) {
contexts.device.free_memory = os.freemem();
}

return contexts;
}

/**
* Gets the contexts for the current environment
*/
Expand Down Expand Up @@ -137,6 +125,22 @@ export class Context implements Integration {
}
}

/**
* Updates the context with dynamic values that can change
*/
function _updateContext(contexts: Contexts): Contexts {
// Only update properties if they exist
if (contexts?.app?.app_memory) {
contexts.app.app_memory = process.memoryUsage().rss;
}

if (contexts?.device?.free_memory) {
contexts.device.free_memory = os.freemem();
}

return contexts;
}

/**
* Returns the operating system context.
*
Expand Down
15 changes: 7 additions & 8 deletions packages/node/src/integrations/contextlines.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,13 @@ export class ContextLines implements Integration {
/**
* @inheritDoc
*/
public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
addGlobalEventProcessor(event => {
const self = getCurrentHub().getIntegration(ContextLines);
if (!self) {
return event;
}
return this.addSourceContext(event);
});
public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
// noop
}

/** @inheritDoc */
public processEvent(event: Event): Promise<Event> {
return this.addSourceContext(event);
}

/** Processes an event and adds context lines */
Expand Down
40 changes: 20 additions & 20 deletions packages/node/src/integrations/modules.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { existsSync, readFileSync } from 'fs';
import { dirname, join } from 'path';
import type { EventProcessor, Hub, Integration } from '@sentry/types';
import type { Event, EventProcessor, Hub, Integration } from '@sentry/types';

let moduleCache: { [key: string]: string };

Expand Down Expand Up @@ -65,6 +65,14 @@ function collectModules(): {
return infos;
}

/** Fetches the list of modules and the versions loaded by the entry file for your node.js app. */
function _getModules(): { [key: string]: string } {
if (!moduleCache) {
moduleCache = collectModules();
}
return moduleCache;
}

/** Add node modules / packages to the event */
export class Modules implements Integration {
/**
Expand All @@ -80,26 +88,18 @@ export class Modules implements Integration {
/**
* @inheritDoc
*/
public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
addGlobalEventProcessor(event => {
if (!getCurrentHub().getIntegration(Modules)) {
return event;
}
return {
...event,
modules: {
...event.modules,
...this._getModules(),
},
};
});
public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
// noop
}

/** Fetches the list of modules and the versions loaded by the entry file for your node.js app. */
private _getModules(): { [key: string]: string } {
if (!moduleCache) {
moduleCache = collectModules();
}
return moduleCache;
/** @inheritdoc */
public processEvent(event: Event): Event {
return {
...event,
modules: {
...event.modules,
..._getModules(),
},
};
}
}
39 changes: 22 additions & 17 deletions packages/node/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,34 +165,39 @@ describe('SentryNode', () => {
}
});

test('capture an exception with pre/post context', done => {
expect.assertions(10);
test('capture an exception with pre/post context', async () => {
const beforeSend = jest.fn((event: Event) => {
expect(event.tags).toEqual({ test: '1' });
expect(event.exception).not.toBeUndefined();
expect(event.exception!.values![0]).not.toBeUndefined();
expect(event.exception!.values![0].stacktrace!).not.toBeUndefined();
expect(event.exception!.values![0].stacktrace!.frames![1]).not.toBeUndefined();
expect(event.exception!.values![0].stacktrace!.frames![1].pre_context).not.toBeUndefined();
expect(event.exception!.values![0].stacktrace!.frames![1].post_context).not.toBeUndefined();
expect(event.exception!.values![0].type).toBe('Error');
expect(event.exception!.values![0].value).toBe('test');
expect(event.exception!.values![0].stacktrace).toBeTruthy();
return null;
});

const options = getDefaultNodeClientOptions({
stackParser: defaultStackParser,
beforeSend: (event: Event) => {
expect(event.tags).toEqual({ test: '1' });
expect(event.exception).not.toBeUndefined();
expect(event.exception!.values![0]).not.toBeUndefined();
expect(event.exception!.values![0].stacktrace!).not.toBeUndefined();
expect(event.exception!.values![0].stacktrace!.frames![1]).not.toBeUndefined();
expect(event.exception!.values![0].stacktrace!.frames![1].pre_context).not.toBeUndefined();
expect(event.exception!.values![0].stacktrace!.frames![1].post_context).not.toBeUndefined();
expect(event.exception!.values![0].type).toBe('Error');
expect(event.exception!.values![0].value).toBe('test');
expect(event.exception!.values![0].stacktrace).toBeTruthy();
done();
return null;
},
beforeSend,
dsn,
integrations: [new ContextLines()],
});
getCurrentHub().bindClient(new NodeClient(options));
const client = new NodeClient(options);
getCurrentHub().bindClient(client);
getCurrentScope().setTag('test', '1');
try {
throw new Error('test');
} catch (e) {
captureException(e);
}

await client.flush();

expect(beforeSend).toHaveBeenCalledTimes(1);
});

test('capture a linked exception with pre/post context', done => {
Expand Down