diff --git a/core/base-service/base-non-memory-caching.js b/core/base-service/base-non-memory-caching.js index 8aaacf3a2bbcc23a294d57087b148bc1181df07d..d2671f3f56e77ffd4525624f3d67d6975e47f692 100644 --- a/core/base-service/base-non-memory-caching.js +++ b/core/base-service/base-non-memory-caching.js @@ -2,6 +2,7 @@ const makeBadge = require('../../gh-badges/lib/make-badge') const BaseService = require('./base') +const { MetricHelper } = require('./metric-helper') const { setCacheHeaders } = require('./cache-headers') const { makeSend } = require('./legacy-result-sender') const coalesceBadge = require('./coalesce-badge') @@ -24,16 +25,19 @@ const { prepareRoute, namedParamsForMatch } = require('./route') // configured by the service, the user's request, and the server's default // cache length. module.exports = class NonMemoryCachingBaseService extends BaseService { - static register({ camp, requestCounter }, serviceConfig) { + static register({ camp, metricInstance }, serviceConfig) { const { cacheHeaders: cacheHeaderConfig } = serviceConfig const { _cacheLength: serviceDefaultCacheLengthSeconds } = this const { regex, captureNames } = prepareRoute(this.route) - const serviceRequestCounter = this._createServiceRequestCounter({ - requestCounter, + const metricHelper = MetricHelper.create({ + metricInstance, + ServiceClass: this, }) camp.route(regex, async (queryParams, match, end, ask) => { + const metricHandle = metricHelper.startRequest() + const namedParams = namedParamsForMatch(captureNames, match, this) const serviceData = await this.invoke( {}, @@ -64,7 +68,7 @@ module.exports = class NonMemoryCachingBaseService extends BaseService { makeSend(format, ask.res, end)(svg) - serviceRequestCounter.inc() + metricHandle.noteResponseSent() }) } } diff --git a/core/base-service/base-static.js b/core/base-service/base-static.js index 7ecd598f323e876ff302a67bf0e5955639d035eb..ae1ab70229c771b1a209ec368f8b4b52c91536fb 100644 --- a/core/base-service/base-static.js +++ b/core/base-service/base-static.js @@ -7,18 +7,20 @@ const { setCacheHeadersForStaticResource, } = require('./cache-headers') const { makeSend } = require('./legacy-result-sender') +const { MetricHelper } = require('./metric-helper') const coalesceBadge = require('./coalesce-badge') const { prepareRoute, namedParamsForMatch } = require('./route') module.exports = class BaseStaticService extends BaseService { - static register({ camp, requestCounter }, serviceConfig) { + static register({ camp, metricInstance }, serviceConfig) { const { profiling: { makeBadge: shouldProfileMakeBadge }, } = serviceConfig const { regex, captureNames } = prepareRoute(this.route) - const serviceRequestCounter = this._createServiceRequestCounter({ - requestCounter, + const metricHelper = MetricHelper.create({ + metricInstance, + ServiceClass: this, }) camp.route(regex, async (queryParams, match, end, ask) => { @@ -29,6 +31,8 @@ module.exports = class BaseStaticService extends BaseService { return } + const metricHandle = metricHelper.startRequest() + const namedParams = namedParamsForMatch(captureNames, match, this) const serviceData = await this.invoke( {}, @@ -60,7 +64,7 @@ module.exports = class BaseStaticService extends BaseService { makeSend(format, ask.res, end)(svg) - serviceRequestCounter.inc() + metricHandle.noteResponseSent() }) } } diff --git a/core/base-service/base.js b/core/base-service/base.js index 7ca0923cce5b557e65f08e36651fd2ae4de2343c..2db71947a6a7fab3bfbac673019eeda13286c9af 100644 --- a/core/base-service/base.js +++ b/core/base-service/base.js @@ -3,12 +3,12 @@ * @module */ -const decamelize = require('decamelize') // See available emoji at http://emoji.muan.co/ const emojic = require('emojic') const Joi = require('@hapi/joi') const log = require('../server/log') const { AuthHelper } = require('./auth-helper') +const { MetricHelper } = require('./metric-helper') const { assertValidCategory } = require('./categories') const checkErrorResponse = require('./check-error-response') const coalesceBadge = require('./coalesce-badge') @@ -391,27 +391,17 @@ class BaseService { return serviceData } - static _createServiceRequestCounter({ requestCounter }) { - if (requestCounter) { - const { category, serviceFamily, name } = this - const service = decamelize(name) - return requestCounter.labels(category, serviceFamily, service) - } else { - // When metrics are disabled, return a mock counter. - return { inc: () => {} } - } - } - static register( - { camp, handleRequest, githubApiProvider, requestCounter }, + { camp, handleRequest, githubApiProvider, metricInstance }, serviceConfig ) { const { cacheHeaders: cacheHeaderConfig, fetchLimitBytes } = serviceConfig const { regex, captureNames } = prepareRoute(this.route) const queryParams = getQueryParamNames(this.route) - const serviceRequestCounter = this._createServiceRequestCounter({ - requestCounter, + const metricHelper = MetricHelper.create({ + metricInstance, + ServiceClass: this, }) camp.route( @@ -419,6 +409,8 @@ class BaseService { handleRequest(cacheHeaderConfig, { queryParams, handler: async (queryParams, match, sendBadge, request) => { + const metricHandle = metricHelper.startRequest() + const namedParams = namedParamsForMatch(captureNames, match, this) const serviceData = await this.invoke( { @@ -441,7 +433,7 @@ class BaseService { const format = (match.slice(-1)[0] || '.svg').replace(/^\./, '') sendBadge(format, badgeData) - serviceRequestCounter.inc() + metricHandle.noteResponseSent() }, cacheLength: this._cacheLength, fetchLimitBytes, diff --git a/core/base-service/metric-helper.js b/core/base-service/metric-helper.js new file mode 100644 index 0000000000000000000000000000000000000000..3e92e7f13668e8580b1e611506d8ab1a8e35a87c --- /dev/null +++ b/core/base-service/metric-helper.js @@ -0,0 +1,45 @@ +'use strict' + +const { performance } = require('perf_hooks') + +class MetricHelper { + constructor({ metricInstance }, { category, serviceFamily, name }) { + if (metricInstance) { + this.metricInstance = metricInstance + this.serviceRequestCounter = metricInstance.createNumRequestCounter({ + category, + serviceFamily, + name, + }) + } else { + this.metricInstance = undefined + this.serviceRequestCounter = undefined + } + } + + static create({ metricInstance, ServiceClass }) { + const { category, serviceFamily, name } = ServiceClass + return new this({ metricInstance }, { category, serviceFamily, name }) + } + + startRequest() { + const { metricInstance, serviceRequestCounter } = this + + const requestStartTime = performance.now() + + return { + noteResponseSent() { + if (metricInstance) { + const elapsedTime = performance.now() - requestStartTime + metricInstance.noteResponseTime(elapsedTime) + } + + if (serviceRequestCounter) { + serviceRequestCounter.inc() + } + }, + } + } +} + +module.exports = { MetricHelper } diff --git a/core/base-service/redirector.js b/core/base-service/redirector.js index 9b9acbf8d4da18276b80c614eb0fe9590315fac4..29ede9a73e55c6964ff07ecd520d676ad478d747 100644 --- a/core/base-service/redirector.js +++ b/core/base-service/redirector.js @@ -10,6 +10,7 @@ const { setCacheHeadersForStaticResource, } = require('./cache-headers') const { isValidCategory } = require('./categories') +const { MetricHelper } = require('./metric-helper') const { isValidRoute, prepareRoute, namedParamsForMatch } = require('./route') const trace = require('./trace') @@ -62,14 +63,15 @@ module.exports = function redirector(attrs) { return route } - static register({ camp, requestCounter }, { rasterUrl }) { + static register({ camp, metricInstance }, { rasterUrl }) { const { regex, captureNames } = prepareRoute({ ...this.route, withPng: Boolean(rasterUrl), }) - const serviceRequestCounter = this._createServiceRequestCounter({ - requestCounter, + const metricHelper = MetricHelper.create({ + metricInstance, + ServiceClass: this, }) camp.route(regex, async (queryParams, match, end, ask) => { @@ -80,6 +82,8 @@ module.exports = function redirector(attrs) { return } + const metricHandle = metricHelper.startRequest() + const namedParams = namedParamsForMatch(captureNames, match, this) trace.logTrace( 'inbound', @@ -121,7 +125,7 @@ module.exports = function redirector(attrs) { ask.res.end() - serviceRequestCounter.inc() + metricHandle.noteResponseSent() }) } } diff --git a/core/server/prometheus-metrics.js b/core/server/prometheus-metrics.js index 9acd4ccd47dd5d203cc7bf27b535623f083afcfe..97a4f7765fe26930e48718bdc093f925c3d0998c 100644 --- a/core/server/prometheus-metrics.js +++ b/core/server/prometheus-metrics.js @@ -1,16 +1,59 @@ 'use strict' +const decamelize = require('decamelize') const prometheus = require('prom-client') module.exports = class PrometheusMetrics { constructor() { this.register = new prometheus.Registry() - this.requestCounter = new prometheus.Counter({ - name: 'service_requests_total', - help: 'Total service requests', - labelNames: ['category', 'family', 'service'], - registers: [this.register], - }) + this.counters = { + numRequests: new prometheus.Counter({ + name: 'service_requests_total', + help: 'Total service requests', + labelNames: ['category', 'family', 'service'], + registers: [this.register], + }), + responseTime: new prometheus.Histogram({ + name: 'service_response_millis', + help: 'Service response time in milliseconds', + // 250 ms increments up to 2 seconds, then 500 ms increments up to 8 + // seconds, then 1 second increments up to 15 seconds. + buckets: [ + 250, + 500, + 750, + 1000, + 1250, + 1500, + 1750, + 2000, + 2250, + 2500, + 2750, + 3000, + 3250, + 3500, + 3750, + 4000, + 4500, + 5000, + 5500, + 6000, + 6500, + 7000, + 7500, + 8000, + 9000, + 10000, + 11000, + 12000, + 13000, + 14000, + 15000, + ], + registers: [this.register], + }), + } } async initialize(server) { @@ -30,4 +73,16 @@ module.exports = class PrometheusMetrics { this.interval = undefined } } + + /** + * @returns {object} `{ inc() {} }`. + */ + createNumRequestCounter({ category, serviceFamily, name }) { + const service = decamelize(name) + return this.counters.numRequests.labels(category, serviceFamily, service) + } + + noteResponseTime(responseTime) { + return this.counters.responseTime.observe(responseTime) + } } diff --git a/core/server/server.js b/core/server/server.js index d15c4fb73fa2b317b4903e376afcd1589736034b..fca237bebc779ac898deb307f38407cc7e3c0da8 100644 --- a/core/server/server.js +++ b/core/server/server.js @@ -148,7 +148,7 @@ class Server { private: privateConfig, }) if (publicConfig.metrics.prometheus.enabled) { - this.metrics = new PrometheusMetrics() + this.metricInstance = new PrometheusMetrics() } } @@ -263,13 +263,12 @@ class Server { * load each service and register a Scoutcamp route for each service. */ registerServices() { - const { config, camp } = this + const { config, camp, metricInstance } = this const { apiProvider: githubApiProvider } = this.githubConstellation - const { requestCounter } = this.metrics || {} loadServiceClasses().forEach(serviceClass => serviceClass.register( - { camp, handleRequest, githubApiProvider, requestCounter }, + { camp, handleRequest, githubApiProvider, metricInstance }, { handleInternalErrors: config.public.handleInternalErrors, cacheHeaders: config.public.cacheHeaders, @@ -309,10 +308,10 @@ class Server { this.cleanupMonitor = sysMonitor.setRoutes({ rateLimit }, camp) - const { githubConstellation, metrics } = this + const { githubConstellation, metricInstance } = this githubConstellation.initialize(camp) - if (metrics) { - metrics.initialize(camp) + if (metricInstance) { + metricInstance.initialize(camp) } const { apiProvider: githubApiProvider } = this.githubConstellation @@ -355,8 +354,8 @@ class Server { this.githubConstellation = undefined } - if (this.metrics) { - this.metrics.stop() + if (this.metricInstance) { + this.metricInstance.stop() } } }