Skip to content

Commit 16642e0

Browse files
authored
node: Port memory-leak tests to @sentry/node (#1784)
1 parent 2a43e94 commit 16642e0

File tree

10 files changed

+426
-9
lines changed

10 files changed

+426
-9
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
## Unreleased
44

5+
- [node] Port memory-leak tests from raven-node
6+
- [core] feat: ExtraErrorData integration
7+
58
## 4.4.1
69

710
- [core] Bump dependencies to remove flatmap-stream

packages/node/package.json

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,22 +52,17 @@
5252
"test": "run-s test:jest test:express",
5353
"test:jest": "jest",
5454
"test:watch": "jest --watch --notify",
55-
"test:express": "node test/manual/express.js",
55+
"test:express": "node test/manual/express-scope-separation/start.js",
5656
"version": "node ../../scripts/versionbump.js src/version.ts"
5757
},
5858
"jest": {
5959
"collectCoverage": true,
6060
"transform": {
6161
"^.+\\.ts$": "ts-jest"
6262
},
63-
"moduleFileExtensions": [
64-
"js",
65-
"ts"
66-
],
63+
"moduleFileExtensions": ["js", "ts"],
6764
"testEnvironment": "node",
68-
"testMatch": [
69-
"**/*.test.ts"
70-
],
65+
"testMatch": ["**/*.test.ts"],
7166
"globals": {
7267
"ts-jest": {
7368
"tsConfig": "./tsconfig.json",

packages/node/test/manual/express.js renamed to packages/node/test/manual/express-scope-separation/start.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
const http = require('http');
22
const express = require('express');
33
const app = express();
4-
const Sentry = require('../../dist');
4+
const Sentry = require('../../../dist');
55

66
function assertTags(actual, expected) {
77
if (JSON.stringify(actual) !== JSON.stringify(expected)) {
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
large-module-dist.js
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
# Manual Tests
2+
3+
## How this works
4+
`express-patient.js` is an express app with a collection of endpoints that exercise various functionalities of @sentry/node, including exception capturing, contexts, autobreadcrumbs, and the express middleware.
5+
6+
It uses [memwatch-next](https://www.npmjs.com/package/memwatch-next) to record memory usage after each GC. `manager.js` does some child process stuff to have a fresh patient process for each test scenario, while poke-patient.sh uses apache bench to send a bunch of traffic so we can see what happens.
7+
8+
## Routes and what we test
9+
The @sentry/node express middleware is used on all endpoints, so each request constitutes its own context.
10+
- `/hello`: just send a basic response without doing anything
11+
- `/context/basic`: `setContext` call
12+
- `/breadcrumbs/capture`: manual `captureBreadcrumb` call
13+
- `/breadcrumbs/auto/console`: console log with console autoBreadcrumbs enabled
14+
- `/breadcrumbs/auto/http`: send an http request with http autoBreadcrumbs enabled
15+
- uses nock to mock the response, not actual request
16+
- If the request has querystring param `doError=true`, we pass an error via Express's error handling mechanism with `next(new Error(responseText))` which will then be captured by the @sentry/node express middleware error handler.
17+
- We test all 5 above cases with and without `doError=true`
18+
19+
We also have a `/gc` endpoint for forcing a garbage collection; this is used at the end of each test scenario to see final memory usage.
20+
21+
Note: there's a `/capture` endpoint which does a basic `captureException` call 1000 times. That's our current problem child requiring some more investigation on its memory usage.
22+
23+
## How to run it
24+
```bash
25+
npm install memwatch-next nock
26+
node manager.js
27+
# in another tab send some traffic at it:
28+
curl localhost:3000/capture
29+
```
30+
31+
## Why this can't be more automated
32+
Some objects can have long lifecycles or not be cleaned up by GC when you think they would be, and so it isn't straightforward to make the assertion "memory usage should have returned to baseline by now". Also, when the numbers look bad, it's pretty obvious to a trained eye that they're bad, but it can be hard to quantify an exact threshold of pass or fail.
33+
34+
## Interpreting results
35+
Starting the manager and then running `ab -c 5 -n 5000 /context/basic && sleep 1 && curl localhost:3000/gc` will get us this output:
36+
<details>
37+
```
38+
:[/Users/lewis/dev/raven-node/test/manual]#memleak-tests?$ node manager.js
39+
starting child
40+
patient is waiting to be poked on port 3000
41+
gc #1: min 0, max 0, est base 11639328, curr base 11639328
42+
gc #2: min 0, max 0, est base 11582672, curr base 11582672
43+
hit /context/basic for first time
44+
gc #3: min 16864536, max 16864536, est base 16864536, curr base 16864536
45+
gc #4: min 14830680, max 16864536, est base 14830680, curr base 14830680
46+
gc #5: min 14830680, max 16864536, est base 16013904, curr base 16013904
47+
hit /gc for first time
48+
gc #6: min 12115288, max 16864536, est base 12115288, curr base 12115288
49+
gc #7: min 11673824, max 16864536, est base 11673824, curr base 11673824
50+
```
51+
</details>
52+
This test stores some basic data in the request's Raven context, with the hope being for that context data to go out of scope and be garbage collected after the request is over. We can see that we start at a base of ~11.6MB, go up to ~16.8MB during the test, and then return to ~11.6MB. Everything checks out, no memory leak issue here.
53+
54+
Back when we had a memory leak in `captureException`, if we started the manager and ran:
55+
```shell
56+
ab -c 5 -n 5000 localhost:3000/context/basic?doError=true && sleep 5 && curl localhost:3000/gc
57+
sleep 5
58+
curl localhost:3000/gc
59+
sleep 10
60+
curl localhost:3000/gc
61+
sleep 15
62+
curl localhost:3000/gc
63+
```
64+
we'd get this output:
65+
<details>
66+
```
67+
[/Users/lewis/dev/raven-node/test/manual]#memleak-tests?$ node manager.js
68+
starting child
69+
patient is waiting to be poked on port 3000
70+
gc #1: min 0, max 0, est base 11657056, curr base 11657056
71+
gc #2: min 0, max 0, est base 11599392, curr base 11599392
72+
hit /context/basic?doError=true for first time
73+
gc #3: min 20607752, max 20607752, est base 20607752, curr base 20607752
74+
gc #4: min 20607752, max 20969872, est base 20969872, curr base 20969872
75+
gc #5: min 19217632, max 20969872, est base 19217632, curr base 19217632
76+
gc #6: min 19217632, max 21025056, est base 21025056, curr base 21025056
77+
gc #7: min 19217632, max 21096656, est base 21096656, curr base 21096656
78+
gc #8: min 19085432, max 21096656, est base 19085432, curr base 19085432
79+
gc #9: min 19085432, max 22666768, est base 22666768, curr base 22666768
80+
gc #10: min 19085432, max 22666768, est base 22487320, curr base 20872288
81+
gc #11: min 19085432, max 22708656, est base 22509453, curr base 22708656
82+
gc #12: min 19085432, max 22708656, est base 22470302, curr base 22117952
83+
gc #13: min 19085432, max 22708656, est base 22440838, curr base 22175664
84+
gc #14: min 19085432, max 22829952, est base 22479749, curr base 22829952
85+
gc #15: min 19085432, max 25273504, est base 22759124, curr base 25273504
86+
gc #16: min 19085432, max 25273504, est base 22707814, curr base 22246024
87+
gc #17: min 19085432, max 33286216, est base 23765654, curr base 33286216
88+
gc #18: min 19085432, max 33286216, est base 23863713, curr base 24746248
89+
gc #19: min 19085432, max 33286216, est base 23685980, curr base 22086392
90+
gc #20: min 19085432, max 33286216, est base 23705022, curr base 23876400
91+
gc #21: min 19085432, max 33286216, est base 23769947, curr base 24354272
92+
gc #22: min 19085432, max 33286216, est base 23987724, curr base 25947720
93+
gc #23: min 19085432, max 33286216, est base 24636946, curr base 30479952
94+
gc #24: min 19085432, max 33286216, est base 24668561, curr base 24953096
95+
gc #25: min 19085432, max 33286216, est base 24750980, curr base 25492760
96+
gc #26: min 19085432, max 33286216, est base 24956242, curr base 26803600
97+
gc #27: min 19085432, max 33286216, est base 25127122, curr base 26665048
98+
gc #28: min 19085432, max 33286216, est base 25357309, curr base 27428992
99+
gc #29: min 19085432, max 33286216, est base 25519102, curr base 26975240
100+
gc #30: min 19085432, max 33286216, est base 25830428, curr base 28632368
101+
gc #31: min 19085432, max 33286216, est base 26113116, curr base 28657312
102+
gc #32: min 19085432, max 33286216, est base 26474999, curr base 29731952
103+
gc #33: min 19085432, max 41429616, est base 27970460, curr base 41429616
104+
gc #34: min 19085432, max 41429616, est base 29262386, curr base 40889728
105+
gc #35: min 19085432, max 41429616, est base 29402336, curr base 30661888
106+
gc #36: min 19085432, max 41429616, est base 29602979, curr base 31408768
107+
gc #37: min 19085432, max 42724544, est base 30915135, curr base 42724544
108+
gc #38: min 19085432, max 42724544, est base 31095390, curr base 32717688
109+
gc #39: min 19085432, max 42724544, est base 31907458, curr base 39216072
110+
gc #40: min 19085432, max 42724544, est base 32093021, curr base 33763088
111+
gc #41: min 19085432, max 42724544, est base 32281586, curr base 33978672
112+
gc #42: min 19085432, max 42724544, est base 32543090, curr base 34896632
113+
gc #43: min 19085432, max 42724544, est base 32743548, curr base 34547672
114+
gc #44: min 19085432, max 42724544, est base 33191109, curr base 37219160
115+
gc #45: min 19085432, max 42724544, est base 33659862, curr base 37878640
116+
gc #46: min 19085432, max 42724544, est base 34162262, curr base 38683864
117+
gc #47: min 19085432, max 42724544, est base 34624103, curr base 38780680
118+
gc #48: min 19085432, max 42724544, est base 35125267, curr base 39635752
119+
gc #49: min 19085432, max 42724544, est base 35547207, curr base 39344672
120+
gc #50: min 19085432, max 42724544, est base 35827942, curr base 38354560
121+
gc #51: min 19085432, max 42724544, est base 36185625, curr base 39404776
122+
gc #52: min 19085432, max 52995432, est base 37866605, curr base 52995432
123+
gc #53: min 19085432, max 52995432, est base 39230884, curr base 51509400
124+
gc #54: min 19085432, max 52995432, est base 39651220, curr base 43434248
125+
gc #55: min 19085432, max 52995432, est base 40010377, curr base 43242792
126+
gc #56: min 19085432, max 52995432, est base 40443827, curr base 44344880
127+
gc #57: min 19085432, max 52995432, est base 40979365, curr base 45799208
128+
gc #58: min 19085432, max 52995432, est base 41337723, curr base 44562952
129+
gc #59: min 19085432, max 57831608, est base 42987111, curr base 57831608
130+
hit /gc for first time
131+
gc #60: min 19085432, max 57831608, est base 42763791, curr base 40753920
132+
gc #61: min 19085432, max 57831608, est base 42427528, curr base 39401168
133+
gc #62: min 19085432, max 57831608, est base 42125779, curr base 39410040
134+
gc #63: min 19085432, max 57831608, est base 41850385, curr base 39371848
135+
gc #64: min 19085432, max 57831608, est base 41606578, curr base 39412320
136+
gc #65: min 19085432, max 57831608, est base 41386124, curr base 39402040
137+
```
138+
</details>
139+
This test, after storing some basic data in the request's SDK context, generates an error which SDK's express error handling middleware will capture. We can see that we started at a base of ~11.6MB, climbed steadily throughout the test to ~40-50MB toward the end, returned to ~39.4MB after the test ends, and were then still at ~39.4MB after 30 seconds and more GCing. This was worrysome, being 30MB over our baseline after 1000 captures. Something was up with capturing exceptions and we uncovered and fixed a memory leak as a result. Now the test returns to a baseline of ~13MB; the slight increase over 11.6MB is due to some warmup costs, but the marginal cost of additional capturing is zero (i.e. we return to that ~13MB baseline whether we do 1000 captures or 5000).
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
const Sentry = require('../../../dist');
2+
3+
Sentry.init({ dsn: 'https://[email protected]/12345' });
4+
5+
// We create a bunch of contexts, capture some breadcrumb data in all of them,
6+
// then watch memory usage. It'll go up to ~40 megs then after 10 or 20 seconds
7+
// gc will drop it back to ~5.
8+
9+
console.log(process.memoryUsage());
10+
11+
for (let i = 0; i < 10000; i++) {
12+
Sentry.withScope(() => {
13+
Sentry.addBreadcrumb({ message: Array(1000).join('.') });
14+
15+
setTimeout(() => {
16+
Sentry.addBreadcrumb({ message: Array(1000).join('a') });
17+
}, 2000);
18+
});
19+
}
20+
21+
console.log(process.memoryUsage());
22+
23+
setInterval(function() {
24+
console.log(process.memoryUsage());
25+
}, 1000);
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
const Sentry = require('../../../dist');
2+
3+
Sentry.init({ dsn: 'https://[email protected]/12345' });
4+
5+
const util = require('util');
6+
const http = require('http');
7+
const nock = require('nock');
8+
9+
// have to call this for each request :/ ref https://github.com/node-nock/nock#read-this---about-interceptors
10+
function nockRequest() {
11+
nock('https://app.getsentry.com')
12+
.filteringRequestBody(/.*/, '*')
13+
.post(/.*/, '*')
14+
.reply(200, 'OK');
15+
}
16+
17+
const memwatch = require('memwatch-next');
18+
memwatch.on('stats', function(stats) {
19+
process._rawDebug(
20+
util.format(
21+
'gc #%d: min %d, max %d, est base %d, curr base %d',
22+
stats.num_full_gc,
23+
stats.min,
24+
stats.max,
25+
stats.estimated_base,
26+
stats.current_base,
27+
),
28+
);
29+
});
30+
31+
const express = require('express');
32+
const app = express();
33+
34+
const hitBefore = {};
35+
36+
app.use(Sentry.Handlers.requestHandler());
37+
38+
app.use((req, res, next) => {
39+
if (!hitBefore[req.url]) {
40+
hitBefore[req.url] = true;
41+
process._rawDebug('hit ' + req.url + ' for first time');
42+
}
43+
next();
44+
});
45+
46+
app.get('/context/basic', (req, res, next) => {
47+
Sentry.configureScope(scope => {
48+
scope.setExtra('example', 'hey look we set some example context data yay');
49+
});
50+
51+
res.textToSend = 'hello there! we set some stuff to the context';
52+
next();
53+
});
54+
55+
app.get('/breadcrumbs/capture', (req, res, next) => {
56+
Sentry.captureBreadcrumb({
57+
message: 'Captured example breadcrumb',
58+
category: 'log',
59+
data: {
60+
example: 'hey look we captured this example breadcrumb yay',
61+
},
62+
});
63+
res.textToSend = 'hello there! we captured an example breadcrumb';
64+
next();
65+
});
66+
67+
app.get('/breadcrumbs/auto/console', (req, res, next) => {
68+
console.log('hello there! i am printing to the console!');
69+
res.textToSend = 'hello there! we printed to the console';
70+
next();
71+
});
72+
73+
app.get('/breadcrumbs/auto/http', (req, res, next) => {
74+
const scope = nock('http://www.example.com')
75+
.get('/hello')
76+
.reply(200, 'hello world');
77+
78+
http
79+
.get('http://www.example.com/hello', function(nockRes) {
80+
scope.done();
81+
res.textToSend = 'hello there! we got hello world from example.com';
82+
next();
83+
})
84+
.on('error', next);
85+
});
86+
87+
app.get('/hello', (req, res, next) => {
88+
res.textToSend = 'hello!';
89+
next();
90+
});
91+
92+
app.get('/gc', (req, res, next) => {
93+
memwatch.gc();
94+
res.textToSend = 'collected garbage';
95+
next();
96+
});
97+
98+
app.get('/shutdown', (req, res, next) => {
99+
setTimeout(function() {
100+
server.close(function() {
101+
process.exit();
102+
});
103+
}, 100);
104+
return res.send('shutting down');
105+
});
106+
107+
app.get('/capture', (req, res, next) => {
108+
for (let i = 0; i < 1000; ++i) {
109+
nockRequest();
110+
Sentry.captureException(new Error('oh no an exception to capture'));
111+
}
112+
memwatch.gc();
113+
res.textToSend = 'capturing an exception!';
114+
next();
115+
});
116+
117+
app.get('/capture_large_source', (req, res, next) => {
118+
nockRequest();
119+
120+
// largeModule.run recurses 1000 times, largeModule is a 5MB file
121+
// if we read the largeModule source once for each frame, we'll use a ton of memory
122+
const largeModule = require('./large-module-dist');
123+
124+
try {
125+
largeModule.run();
126+
} catch (e) {
127+
Sentry.captureException(e);
128+
}
129+
130+
memwatch.gc();
131+
res.textToSend = 'capturing an exception!';
132+
next();
133+
});
134+
135+
app.use((req, res, next) => {
136+
if (req.query.doError) {
137+
nockRequest();
138+
return next(new Error(res.textToSend));
139+
}
140+
return res.send(res.textToSend);
141+
});
142+
143+
app.use(Sentry.Handlers.errorHandler());
144+
145+
app.use((err, req, res, next) => {
146+
return res.status(500).send('oh no there was an error: ' + err.message);
147+
});
148+
149+
const server = app.listen(3000, () => {
150+
process._rawDebug('patient is waiting to be poked on port 3000');
151+
memwatch.gc();
152+
});
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
'use strict';
2+
3+
function run(n) {
4+
if (n == null) return run(1000);
5+
if (n === 0) throw new Error('we did it!');
6+
console.log('run ' + n);
7+
return run(n - 1);
8+
}
9+
10+
module.exports.run = run;
11+
12+
// below is 5MB worth of 'A', so reading this file multiple times concurrently will use lots of memory
13+
var a = '{{template}}';

0 commit comments

Comments
 (0)