Skip to content

Commit

Permalink
fix: Update engines to prevent agent from being used with versions of…
Browse files Browse the repository at this point in the history
… Node.js where v8 profilers have memory leaks (#699)
  • Loading branch information
nolanmar511 authored Oct 20, 2020
1 parent d098850 commit 160d1f6
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 18 deletions.
3 changes: 1 addition & 2 deletions .github/sync-repo-settings.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ branchProtectionRules:
- "docs"
- "lint"
- "test (10)"
- "test (12)"
- "test (13)"
- "test (12.15)"
- "windows"
- "system-test"
10 changes: 5 additions & 5 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
node: [10, 12, 13]
node: [10, 12.15]
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
Expand All @@ -31,7 +31,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
with:
node-version: 12
node-version: 12.15
- run: npm install
- run: npm test
- name: coverage
Expand All @@ -45,7 +45,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
with:
node-version: 12
node-version: 12.15
- run: npm install
- run: npm run lint
docs:
Expand All @@ -54,7 +54,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
with:
node-version: 12
node-version: 12.15
- run: npm install
- run: npm run docs-test
system-test:
Expand All @@ -63,7 +63,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
with:
node-version: 12
node-version: 12.15
- run: npm install
- run: npm run system-test
- name: coverage
Expand Down
3 changes: 2 additions & 1 deletion .kokoro/system-test.sh

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 11 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,17 @@ npm install @google-cloud/profiler

### Prerequisites

1. Your application will need to be using Node.js 10.4.1 or greater or Node.js
12. The profiler will not be enabled when using earlier versions of 10 because
versions of Node.js 10 prior to 10.4.1 are impacted by
[this](https://bugs.chromium.org/p/chromium/issues/detail?id=847863) issue,
which can cause garbage collection to take several minutes when heap profiling
is enabled.
1. Your application will need to be using Node.js 10.4.1 or greater on the 10.x
version branch or Node.js 12.15.0 or lower on the 12.x version branch. The
profiler will not be enabled when using version prior to 10.4.1 or after
12.15.0. This is because:
* Versions of Node.js 10 prior to 10.4.1 are impacted by
[this](https://bugs.chromium.org/p/chromium/issues/detail?id=847863) issue,
which can cause garbage collection to take several minutes when heap
profiling is enabled.
* Versions of Node.js 12 after 12.15.0 are impacted by
[this memory leak](https://bugs.chromium.org/p/v8/issues/detail?id=10883)
when profiling is enabled.

1. `@google-cloud/profiler` depends on the
[`pprof`](https://www.npmjs.com/package/pprof) module, a module with a native
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,6 @@
]
},
"engines": {
"node": ">=10.4.1"
"node": ">=10.4.1 <=12.15.0"
}
}
6 changes: 3 additions & 3 deletions system-test/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ func TestAgentIntegration(t *testing.T) {
},
name: fmt.Sprintf("profiler-test-node12-%s-gce", runID),
wantProfiles: wantProfiles,
nodeVersion: "12",
nodeVersion: "12.15",
timeout: gceTestTimeout,
benchDuration: gceBenchDuration,
},
Expand All @@ -289,7 +289,7 @@ func TestAgentIntegration(t *testing.T) {
},
name: fmt.Sprintf("profiler-test-node13-%s-gce", runID),
wantProfiles: wantProfiles,
nodeVersion: "12",
nodeVersion: "12.15",
timeout: gceTestTimeout,
benchDuration: gceBenchDuration,
},
Expand All @@ -310,7 +310,7 @@ func TestAgentIntegration(t *testing.T) {
},
name: fmt.Sprintf("profiler-backoff-test-node12-%s", runID),
backoffTest: true,
nodeVersion: "12",
nodeVersion: "12.15",
timeout: backoffTestTimeout,
benchDuration: backoffBenchDuration,
})
Expand Down
12 changes: 12 additions & 0 deletions test/test-init-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,18 @@ describe('nodeVersionOkay', () => {
it('should not accept v10.4.0', () => {
assert.strictEqual(false, nodeVersionOkay('v10.4.0'));
});
it('should accept v12.15.0', () => {
assert.strictEqual(true, nodeVersionOkay('v12.15.0'));
});
it('should not accept v12.16.0', () => {
assert.strictEqual(false, nodeVersionOkay('v12.16.0'));
});
it('should not accept v14.0.0', () => {
assert.strictEqual(false, nodeVersionOkay('v14.0.0'));
});
it('should not accept v14.13.0', () => {
assert.strictEqual(false, nodeVersionOkay('v14.13.0'));
});
});

describe('createProfiler', () => {
Expand Down

0 comments on commit 160d1f6

Please sign in to comment.