Skip to content
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

create domain per request, so hub per request is created #21

Merged
merged 7 commits into from
Jan 20, 2020
Merged
Show file tree
Hide file tree
Changes from 5 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
24 changes: 22 additions & 2 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const schema = require('./schema');

const Hoek = require('@hapi/hoek');
const joi = require('@hapi/joi');
const domain = require('domain');

exports.register = (server, options) => {
const opts = joi.attempt(options, schema, 'Invalid hapi-sentry options:');
Expand All @@ -29,11 +30,20 @@ exports.register = (server, options) => {
// expose sentry client at server.plugins['hapi-sentry'].client
server.expose('client', Sentry);

// attach a new scope to each request for breadcrumbs/tags/extras/etc capturing
server.ext({
type: 'onRequest',
method(request, h) {
request.sentryScope = new Sentry.Scope();
if (opts.useDomainPerRequest) {
// Sentry looks for current hub in active domain
// Therefore simply by creating&entering domain Sentry will create
// request scoped hub for breadcrumps and other scope metadata
request.__sentryDomain = domain.create();
request.__sentryDomain.enter();
}

// attach a new scope to each request for breadcrumbs/tags/extras/etc capturing
request.sentryScope = Sentry.getCurrentHub().getScope();

return h.continue;
},
});
Expand Down Expand Up @@ -86,6 +96,16 @@ exports.register = (server, options) => {
});
});

if (opts.useDomainPerRequest) {
server.events.on('response', request => {
if (request.__sentryDomain) {
// exiting domain, not sure if thats necessary, hard to find definitive answer,
// but its safer to prevent potentional memory leaks
request.__sentryDomain.exit();
}
});
}

if (opts.catchLogErrors) {
server.events.on({ name: 'log', channels: ['app'] }, event => {
if (!event.error) return; // no error, just a log message
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@
"dependencies": {
"@hapi/hoek": "^9.0.2",
"@hapi/joi": "^15.1.0",
"@sentry/node": "^5.4.3"
"@sentry/node": "^5.4.3",
"delay": "^4.3.0"
guischdi marked this conversation as resolved.
Show resolved Hide resolved
},
"devDependencies": {
"@hapi/hapi": "^19.0.2",
Expand Down
1 change: 1 addition & 0 deletions schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,5 @@ module.exports = joi.object().keys({
joi.boolean(),
joi.array().items(joi.string()),
).default(false),
useDomainPerRequest: joi.boolean().default(false),
});
86 changes: 86 additions & 0 deletions test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
const test = require('ava');
const hapi = require('@hapi/hapi');
const defer = require('p-defer');
const delay = require('delay');

const plugin = require('./');

const dsn = 'https://[email protected]/project';
Expand Down Expand Up @@ -81,6 +83,13 @@ test('uses a custom sentry client', async t => {
const deferred = defer();
const customSentry = {
Scope: class Scope {},
getCurrentHub() {
return {
getScope() {
return {};
},
};
},
// arity needed to pass joi validation
Handlers: { parseRequest: (x, y) => { } }, // eslint-disable-line no-unused-vars
withScope: cb => cb({ addEventProcessor: () => { } }),
Expand Down Expand Up @@ -210,6 +219,83 @@ test('parses request metadata', async t => {
t.is(request.url, `http://${request.headers.host}/route`);
});

// sorry for console.log being the easiest way to set breadcrumbs
test('collects breadcrumbs per domain', async t => {
const { server } = t.context;


server.route({
method: 'GET',
path: '/route1',
async handler() {
await delay(400);
clearInterval(interval);
throw new Error('Error 1');
},
});

server.route({
method: 'GET',
path: '/route2',
async handler() {
await delay(200);
console.log('domain breadcrumb');
throw new Error('Error 2');
},
});

const deferred1 = defer();
const deferred2 = defer();
await server.register({
plugin,
options: {
useDomainPerRequest: true,
client: {
dsn,
beforeSend(event) {
const { request } = event;
if (request.url === `http://${request.headers.host}/route1`) {
return deferred1.resolve(event);
}
return deferred2.resolve(event);
},
},
},
});

let n = 1;
const interval = setInterval(() => console.log(`global breadcrumb ${n++}`), 60);

await Promise.all([
server.inject({ method: 'GET', url: '/route1' }),
server.inject({ method: 'GET', url: '/route2' }),
]);

// /route1 should not see local breadcrumb of /route2
const event1 = await deferred1.promise;
t.is(event1.exception.values[0].value, 'Error 1');
const event1Breadcrumbs = event1.breadcrumbs.map(b => b.message);
const breadcrumbs1 = [
// /route2 consumes number 1 to 3, leaves 4 to 6 for /route1
'global breadcrumb 4',
'global breadcrumb 5',
'global breadcrumb 6',
];
t.true(breadcrumbs1.every(b => event1Breadcrumbs.includes(b)));

const event2 = await deferred2.promise;
t.is(event2.exception.values[0].value, 'Error 2');
const event2Breadcrumbs = event2.breadcrumbs.map(b => b.message);
const breadcrumbs2 = [
'global breadcrumb 1',
'global breadcrumb 2',
'global breadcrumb 3',
'domain breadcrumb',
];
t.true(event2Breadcrumbs.some(b => b.includes('[DEP0097]'))); // yeah, domains are deprecated
t.true(breadcrumbs2.every(b => event2Breadcrumbs.includes(b)));
});

test('sanitizes user info from auth', async t => {
const { server } = t.context;

Expand Down