From 471518b41ddb51a98652652a95e461d3fc654a0c Mon Sep 17 00:00:00 2001 From: ivaosthu Date: Thu, 1 Dec 2016 18:10:25 +0100 Subject: [PATCH] Simplify metrics: We only need timings as it includes counts for free --- lib/app.js | 6 +++--- lib/events.js | 5 ----- lib/metrics.js | 27 +++------------------------ lib/metrics.test.js | 16 ++++++++++++++++ lib/routes/feature.js | 5 ----- lib/routes/metrics.js | 7 ------- 6 files changed, 22 insertions(+), 44 deletions(-) create mode 100644 lib/metrics.test.js diff --git a/lib/app.js b/lib/app.js index 00af2aa011..4b98379f15 100644 --- a/lib/app.js +++ b/lib/app.js @@ -12,7 +12,7 @@ const routes = require('./routes'); const path = require('path'); const errorHandler = require('errorhandler'); -const {REQUEST_TIME, REQUEST_STATUS} = require('./events'); +const {REQUEST_TIME} = require('./events'); module.exports = function (config) { const app = express(); @@ -30,8 +30,8 @@ module.exports = function (config) { } app.use(responseTime((req, res, time) => { - config.eventBus.emit(REQUEST_TIME, req.path, req.method, time); - config.eventBus.emit(REQUEST_STATUS, req.path, req.method, res.statusCode); + const timingInfo = { path: req.path, method: req.method, statusCode: res.statusCode, time }; + config.eventBus.emit(REQUEST_TIME, timingInfo); })); app.use(validator([])); diff --git a/lib/events.js b/lib/events.js index e5b97aad4c..f133328219 100644 --- a/lib/events.js +++ b/lib/events.js @@ -1,8 +1,3 @@ module.exports = { - TOGGLES_FETCH: 'toggles:fetch', - TOGGLES_CREATE: 'toggles:create', - CLIENT_REGISTER: 'client:register', - CLIENT_METRICS: 'toggles:metrics', REQUEST_TIME: 'request_time', - REQUEST_STATUS: 'request_status' } \ No newline at end of file diff --git a/lib/metrics.js b/lib/metrics.js index afec639d3b..d72a4c7526 100644 --- a/lib/metrics.js +++ b/lib/metrics.js @@ -6,33 +6,12 @@ exports.startMonitoring = (enable, eventBus) => { } const client = require('prom-client'); - const toggleFetch = new client.Counter('toggles_fetch_counter', 'Number of fetch toggles request'); - const requestDuration = new client.Summary('http_request_duration_milliseconds', 'App response time', ['uri', 'method'], { + const requestDuration = new client.Summary('http_request_duration_milliseconds', 'App response time', ['path', 'method', 'status'], { percentiles: [0.1, 0.5, 0.9, 0.99], }); - const requestCount = new client.Counter('http_requests_total', 'HTTP request duration', ['uri', 'method', 'status']); - const clientRegister = new client.Counter('client_register_counter', 'Number client register requests'); - const clientMetrics = new client.Counter('client_metrics_counter', 'Number client metrics requests'); - - eventBus.on(events.TOGGLES_FETCH, () => { - toggleFetch.inc(); - }); - - eventBus.on(events.CLIENT_REGISTER, () => { - clientRegister.inc(); - }); - - eventBus.on(events.CLIENT_METRICS, () => { - clientMetrics.inc(); - }); - - eventBus.on(events.REQUEST_TIME, (uri, method, time) => { - requestDuration.labels(uri, method).observe(time); - }); - - eventBus.on(events.REQUEST_STATUS, (uri, method, status) => { - requestCount.labels(uri, method, status).inc(); + eventBus.on(events.REQUEST_TIME, ({path, method, time, statusCode}) => { + requestDuration.labels(path, method, statusCode).observe(time); }); }; \ No newline at end of file diff --git a/lib/metrics.test.js b/lib/metrics.test.js new file mode 100644 index 0000000000..dea82ba8bc --- /dev/null +++ b/lib/metrics.test.js @@ -0,0 +1,16 @@ + +const test = require('ava'); +const { EventEmitter } = require('events'); +const eventBus = new EventEmitter(); +const { REQUEST_TIME } = require('./events'); +const { startMonitoring } = require('./metrics'); +const prometheusRegister = require('prom-client/lib/register'); + +test('should collect metrics for requests', t => { + startMonitoring(true, eventBus); + eventBus.emit(REQUEST_TIME, { path: 'somePath', method: 'GET', statusCode: 200, time: 1337 }); + + const metrics = prometheusRegister.metrics(); + t.regex(metrics, /http_request_duration_milliseconds{quantile="0.99",status="200",method="GET",path="somePath"} 1337/) + +}); \ No newline at end of file diff --git a/lib/routes/feature.js b/lib/routes/feature.js index 527efd6be0..ae59d8a507 100644 --- a/lib/routes/feature.js +++ b/lib/routes/feature.js @@ -8,8 +8,6 @@ const ValidationError = require('../error/validation-error.js'); const validateRequest = require('../error/validate-request'); const extractUser = require('../extract-user'); -const { TOGGLES_FETCH } = require('../events'); - const legacyFeatureMapper = require('../data-helper/legacy-feature-mapper'); const version = 1; @@ -39,11 +37,8 @@ const handleErrors = (req, res, error) => { module.exports = function (app, config) { const { featureToggleStore, eventStore } = config.stores; - const { eventBus } = config; app.get('/features', (req, res) => { - eventBus.emit(TOGGLES_FETCH); - featureToggleStore.getFeatures() .then((features) => features.map(legacyFeatureMapper.addOldFields)) .then(features => res.json({ version, features })); diff --git a/lib/routes/metrics.js b/lib/routes/metrics.js index 1529f50458..906d4b8538 100644 --- a/lib/routes/metrics.js +++ b/lib/routes/metrics.js @@ -4,7 +4,6 @@ const logger = require('../logger'); const ClientMetrics = require('../client-metrics'); const joi = require('joi'); const { clientMetricsSchema, clientRegisterSchema } = require('./metrics-schema'); -const { CLIENT_REGISTER, CLIENT_METRICS } = require('../events'); const { catchLogAndSendErrorResponse } = require('./route-utils'); module.exports = function (app, config) { @@ -14,8 +13,6 @@ module.exports = function (app, config) { clientInstanceStore, } = config.stores; - const { eventBus } = config; - const metrics = new ClientMetrics(clientMetricsStore); app.get('/client/seen-toggles', (req, res) => { @@ -36,8 +33,6 @@ module.exports = function (app, config) { return res.status(400).json(err); } - eventBus.emit(CLIENT_METRICS); - clientMetricsStore .insert(cleaned) .then(() => clientInstanceStore.insert({ @@ -60,8 +55,6 @@ module.exports = function (app, config) { return res.status(400).json(err); } - eventBus.emit(CLIENT_REGISTER); - clientStrategyStore .insert(cleaned.appName, cleaned.strategies) .then(() => clientInstanceStore.insert({