Skip to content

Commit

Permalink
fix: process_max_fds is process limit, not OS (#314)
Browse files Browse the repository at this point in the history
* fix: missing test for process_max_fds

* fix: process_max_fds is process limit, not OS

Fix: #311

* fixup! fix: process_max_fds is process limit, not OS
  • Loading branch information
sam-github authored and zbjornson committed Jan 28, 2020
1 parent f71eb75 commit 9b204e4
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ project adheres to [Semantic Versioning](http://semver.org/).

### Changed

- fix: `process_max_fds` is process limit, not OS (#314)
- Changed `Metric` labelNames & labelValues in TypeScript declaration to a generic type `T extends string`, instead of `string`
- Lazy-load Node.js Cluster module to fix Passenger support (#293)
- fix: avoid mutation bug in `registry.getMetricsAsJSON()`
Expand Down
21 changes: 20 additions & 1 deletion lib/metrics/processMaxFileDescriptors.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,33 @@ module.exports = (registry, config = {}) => {
return;
}

fs.readFile('/proc/sys/fs/file-max', 'utf8', (err, maxFds) => {
fs.readFile('/proc/self/limits', 'utf8', (err, limits) => {
if (err) {
return;
}

const lines = limits.split('\n');

let maxFds;
lines.find(line => {
if (line.startsWith('Max open files')) {
const parts = line.split(/ +/);
maxFds = parts[1];
return true;
}
});

if (maxFds === undefined) return;

isSet = true;

fileDescriptorsGauge.set(Number(maxFds));

// Only for interal use by tests, so they know when the
// value has been read.
if (config.ready) {
config.ready();
}
});
};
};
Expand Down
61 changes: 61 additions & 0 deletions test/metrics/maxFileDescriptorsTest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
'use strict';

const exec = require('child_process').execSync;

describe('processMaxFileDescriptors', () => {
const register = require('../../index').register;
const processMaxFileDescriptors = require('../../lib/metrics/processMaxFileDescriptors');

beforeAll(() => {
register.clear();
});

afterEach(() => {
register.clear();
});

if (process.platform !== 'linux') {
it('should not add metric to the registry', () => {
expect(register.getMetricsAsJSON()).toHaveLength(0);

processMaxFileDescriptors()();

expect(register.getMetricsAsJSON()).toHaveLength(0);
});
} else {
it('should add metric to the registry', () => {
expect(register.getMetricsAsJSON()).toHaveLength(0);

processMaxFileDescriptors()();

const metrics = register.getMetricsAsJSON();

expect(metrics).toHaveLength(1);
expect(metrics[0].help).toEqual(
'Maximum number of open file descriptors.'
);
expect(metrics[0].type).toEqual('gauge');
expect(metrics[0].name).toEqual('process_max_fds');
expect(metrics[0].values).toHaveLength(1);
});

it('should have a reasonable metric value', done => {
const maxFiles = Number(exec('ulimit -Hn', { encoding: 'utf8' }));

expect(register.getMetricsAsJSON()).toHaveLength(0);
processMaxFileDescriptors(register, { ready })();

function ready() {
const metrics = register.getMetricsAsJSON();

expect(metrics).toHaveLength(1);
expect(metrics[0].values).toHaveLength(1);

expect(metrics[0].values[0].value).toBeLessThanOrEqual(maxFiles);
expect(metrics[0].values[0].value).toBeGreaterThan(0);

return done();
}
});
}
});

0 comments on commit 9b204e4

Please sign in to comment.