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.

Sign up for GitHub

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

fix: align compliancy to Prometheus naming convention #284

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

fix: align compliancy to Prometheus naming convention #284

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c73c243
fix: align compliancy to Prometheus naming convention
Aug 27, 2019
4ca7a27
Merge branch 'master' of github.com:siimon/prom-client
Nov 22, 2019
5e4b768
fix: avoid omitting timestamps during aggregation
Nov 22, 2019
441cb38
Merge branch 'master' of https://github.com/siimon/prom-client into s…
Apr 14, 2021
19c092d
add yarn.lock to git index
Apr 14, 2021
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
4 changes: 3 additions & 1 deletion lib/metricAggregators.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ function AggregatorFactory(aggregatorFn) {
if (values[0].metricName) {
valObj.metricName = values[0].metricName;
}
// NB: Timestamps are omitted.
if (values[0].timestamp) {
valObj.timestamp = values[0].timestamp;
}
result.values.push(valObj);
});
return result;
Expand Down
8 changes: 4 additions & 4 deletions lib/metrics/processHandles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
'use strict';

const Gauge = require('../gauge');
const { aggregateByObjectName } = require('./helpers/processMetricsHelpers');
const { updateMetrics } = require('./helpers/processMetricsHelpers');
const Gauge = require('../gauge');

const NODEJS_ACTIVE_HANDLES = 'nodejs_active_handles';
const NODEJS_ACTIVE_HANDLES_TOTAL = 'nodejs_active_handles_total';
const NODEJS_ACTIVE_HANDLES_TOTAL_NUMBER = 'nodejs_active_handles_total_number';
Copy link
Contributor

@sam-github sam-github Feb 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The core problem here is that promtool thinks that anything called *_total should be COUNT, right, not a GAUGE? Would renaming to nodejs_active_handles also address this?

Copy link
Author

@izonder izonder Feb 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every renaming is very easy to check with built-in prometheus "linter". Probably, it even could be a good idea just to add the linting as a job to CI pipeline

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need any help with this? I've love to pay it forward


module.exports = (registry, config = {}) => {
// Don't do anything if the function is removed in later nodes (exists in node@6)
Expand All @@ -23,7 +23,7 @@ module.exports = (registry, config = {}) => {
registers: registry ? [registry] : undefined
});
const totalGauge = new Gauge({
name: namePrefix + NODEJS_ACTIVE_HANDLES_TOTAL,
name: namePrefix + NODEJS_ACTIVE_HANDLES_TOTAL_NUMBER,
help: 'Total number of active handles.',
registers: registry ? [registry] : undefined
});
Expand All @@ -45,5 +45,5 @@ module.exports = (registry, config = {}) => {

module.exports.metricNames = [
NODEJS_ACTIVE_HANDLES,
NODEJS_ACTIVE_HANDLES_TOTAL
NODEJS_ACTIVE_HANDLES_TOTAL_NUMBER
];
7 changes: 4 additions & 3 deletions lib/metrics/processRequests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
'use strict';

const Gauge = require('../gauge');
const { aggregateByObjectName } = require('./helpers/processMetricsHelpers');
const { updateMetrics } = require('./helpers/processMetricsHelpers');

const NODEJS_ACTIVE_REQUESTS = 'nodejs_active_requests';
const NODEJS_ACTIVE_REQUESTS_TOTAL = 'nodejs_active_requests_total';
const NODEJS_ACTIVE_REQUESTS_TOTAL_NUMBER = 'nodejs_active_requests_total_number'; //eslint-disable-line prettier/prettier

module.exports = (registry, config = {}) => {
// Don't do anything if the function is removed in later nodes (exists in node@6)
Expand All @@ -23,7 +24,7 @@ module.exports = (registry, config = {}) => {
});

const totalGauge = new Gauge({
name: namePrefix + NODEJS_ACTIVE_REQUESTS_TOTAL,
name: namePrefix + NODEJS_ACTIVE_REQUESTS_TOTAL_NUMBER,
help: 'Total number of active requests.',
registers: registry ? [registry] : undefined
});
Expand All @@ -37,5 +38,5 @@ module.exports = (registry, config = {}) => {

module.exports.metricNames = [
NODEJS_ACTIVE_REQUESTS,
NODEJS_ACTIVE_REQUESTS_TOTAL
NODEJS_ACTIVE_REQUESTS_TOTAL_NUMBER
];
2 changes: 1 addition & 1 deletion test/metrics/processHandlesTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@ describe('processHandles', () => {

expect(metrics[1].help).toEqual('Total number of active handles.');
expect(metrics[1].type).toEqual('gauge');
expect(metrics[1].name).toEqual('nodejs_active_handles_total');
expect(metrics[1].name).toEqual('nodejs_active_handles_total_number');
});
});
2 changes: 1 addition & 1 deletion test/metrics/processRequestsTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,6 @@ describe('processRequests', () => {

expect(metrics[1].help).toEqual('Total number of active requests.');
expect(metrics[1].type).toEqual('gauge');
expect(metrics[1].name).toEqual('nodejs_active_requests_total');
expect(metrics[1].name).toEqual('nodejs_active_requests_total_number');
});
});