diff --git a/core/base-service/base.js b/core/base-service/base.js index 352314d8363f0188ab5edd73248b5473ece2f7a6..2d4dfdb644571acd29bee0f9af52a8060c876717 100644 --- a/core/base-service/base.js +++ b/core/base-service/base.js @@ -8,7 +8,7 @@ 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 { MetricHelper, MetricNames } = require('./metric-helper') const { assertValidCategory } = require('./categories') const checkErrorResponse = require('./check-error-response') const coalesceBadge = require('./coalesce-badge') @@ -214,20 +214,43 @@ class BaseService { return result } - constructor({ sendAndCacheRequest, authHelper }, { handleInternalErrors }) { + constructor( + { sendAndCacheRequest, authHelper, metricHelper }, + { handleInternalErrors } + ) { this._requestFetcher = sendAndCacheRequest this.authHelper = authHelper this._handleInternalErrors = handleInternalErrors + this._metricHelper = metricHelper } async _request({ url, options = {}, errorMessages = {} }) { const logTrace = (...args) => trace.logTrace('fetch', ...args) logTrace(emojic.bowAndArrow, 'Request', url, '\n', options) const { res, buffer } = await this._requestFetcher(url, options) + await this._meterResponse(res, buffer) logTrace(emojic.dart, 'Response status code', res.statusCode) return checkErrorResponse(errorMessages)({ buffer, res }) } + static get enabledMetrics() { + return [] + } + + static isMetricEnabled(metricName) { + return this.enabledMetrics.includes(metricName) + } + + async _meterResponse(res, buffer) { + if ( + this._metricHelper && + this.constructor.isMetricEnabled(MetricNames.SERVICE_RESPONSE_SIZE) && + res.statusCode === 200 + ) { + this._metricHelper.noteServiceResponseSize(buffer.length) + } + } + static _validate( data, schema, @@ -426,6 +449,7 @@ class BaseService { sendAndCacheRequest: request.asPromise, sendAndCacheRequestWithCallbacks: request, githubApiProvider, + metricHelper, }, serviceConfig, namedParams, diff --git a/core/base-service/base.spec.js b/core/base-service/base.spec.js index 11025c6856b2e07640c140d5c385395ec0f42702..06d9cfe0d3f66e4a53ef3d6974fe7777b9faa2f7 100644 --- a/core/base-service/base.spec.js +++ b/core/base-service/base.spec.js @@ -4,6 +4,8 @@ const Joi = require('@hapi/joi') const chai = require('chai') const { expect } = chai const sinon = require('sinon') +const prometheus = require('prom-client') +const PrometheusMetrics = require('../server/prometheus-metrics') const trace = require('./trace') const { NotFound, @@ -13,7 +15,7 @@ const { Deprecated, } = require('./errors') const BaseService = require('./base') - +const { MetricHelper, MetricNames } = require('./metric-helper') require('../register-chai-plugins.spec') chai.use(require('chai-as-promised')) @@ -65,6 +67,12 @@ class DummyService extends BaseService { } } +class DummyServiceWithServiceResponseSizeMetricEnabled extends DummyService { + static get enabledMetrics() { + return [MetricNames.SERVICE_RESPONSE_SIZE] + } +} + describe('BaseService', function() { const defaultConfig = { handleInternalErrors: false, private: {} } @@ -498,6 +506,59 @@ describe('BaseService', function() { }) }) + describe('Metrics', function() { + let register + beforeEach(function() { + register = new prometheus.Registry() + }) + const url = 'some-url' + + it('service response size metric is optional', async function() { + const metricHelper = MetricHelper.create({ + metricInstance: new PrometheusMetrics({ register }), + ServiceClass: DummyServiceWithServiceResponseSizeMetricEnabled, + }) + const sendAndCacheRequest = async () => ({ + buffer: 'x'.repeat(65536 + 1), + res: { statusCode: 200 }, + }) + const serviceInstance = new DummyServiceWithServiceResponseSizeMetricEnabled( + { sendAndCacheRequest, metricHelper }, + defaultConfig + ) + + await serviceInstance._request({ url }) + + expect(register.getSingleMetricAsString('service_response_bytes')) + .to.contain( + 'service_response_bytes_bucket{le="65536",category="other",family="undefined",service="dummy_service_with_service_response_size_metric_enabled"} 0\n' + ) + .and.to.contain( + 'service_response_bytes_bucket{le="131072",category="other",family="undefined",service="dummy_service_with_service_response_size_metric_enabled"} 1\n' + ) + }) + + it('service response size metric is disabled by default', async function() { + const metricHelper = MetricHelper.create({ + metricInstance: new PrometheusMetrics({ register }), + ServiceClass: DummyService, + }) + const sendAndCacheRequest = async () => ({ + buffer: 'x', + res: { statusCode: 200 }, + }) + const serviceInstance = new DummyService( + { sendAndCacheRequest, metricHelper }, + defaultConfig + ) + + await serviceInstance._request({ url }) + + expect( + register.getSingleMetricAsString('service_response_bytes') + ).to.not.contain('service_response_bytes_bucket') + }) + }) describe('auth', function() { class AuthService extends DummyService { static get auth() { diff --git a/core/base-service/metric-helper.js b/core/base-service/metric-helper.js index 3e92e7f13668e8580b1e611506d8ab1a8e35a87c..aab44226318fb0498b7cc63d8c576ad6c77bc97f 100644 --- a/core/base-service/metric-helper.js +++ b/core/base-service/metric-helper.js @@ -11,9 +11,17 @@ class MetricHelper { serviceFamily, name, }) + this.serviceResponseSizeHistogram = metricInstance.createServiceResponseSizeHistogram( + { + category, + serviceFamily, + name, + } + ) } else { this.metricInstance = undefined this.serviceRequestCounter = undefined + this.serviceResponseSizeHistogram = undefined } } @@ -40,6 +48,16 @@ class MetricHelper { }, } } + + noteServiceResponseSize(size) { + if (this.serviceResponseSizeHistogram) { + return this.serviceResponseSizeHistogram.observe(size) + } + } } -module.exports = { MetricHelper } +const MetricNames = Object.freeze({ + SERVICE_RESPONSE_SIZE: Symbol('service-response-size'), +}) + +module.exports = { MetricHelper, MetricNames } diff --git a/core/server/prometheus-metrics.js b/core/server/prometheus-metrics.js index 1c7e276b487bb5e4b25a46e39b97b860c151fb56..bda312a40109ec6a352e828058cb24ddc14fc629 100644 --- a/core/server/prometheus-metrics.js +++ b/core/server/prometheus-metrics.js @@ -4,8 +4,8 @@ const decamelize = require('decamelize') const prometheus = require('prom-client') module.exports = class PrometheusMetrics { - constructor() { - this.register = new prometheus.Registry() + constructor({ register } = {}) { + this.register = register || new prometheus.Registry() this.counters = { numRequests: new prometheus.Counter({ name: 'service_requests_total', @@ -59,6 +59,14 @@ module.exports = class PrometheusMetrics { labelNames: ['rate_limit_type'], registers: [this.register], }), + serviceResponseSize: new prometheus.Histogram({ + name: 'service_response_bytes', + help: 'Service response size in bytes', + labelNames: ['category', 'family', 'service'], + // buckets: 64KiB, 128KiB, 256KiB, 512KiB, 1MiB, 2MiB, 4MiB, 8MiB + buckets: prometheus.exponentialBuckets(64 * 1024, 2, 8), + registers: [this.register], + }), } } @@ -95,4 +103,13 @@ module.exports = class PrometheusMetrics { noteRateLimitExceeded(rateLimitType) { return this.counters.rateLimitExceeded.labels(rateLimitType).inc() } + + createServiceResponseSizeHistogram({ category, serviceFamily, name }) { + const service = decamelize(name) + return this.counters.serviceResponseSize.labels( + category, + serviceFamily, + service + ) + } } diff --git a/services/dynamic/dynamic-json.service.js b/services/dynamic/dynamic-json.service.js index 398bd53842376ad0bf43f40c88684f48581d2d46..b9d129cf4a0ba21212e66855ed09e8d80ed779f7 100644 --- a/services/dynamic/dynamic-json.service.js +++ b/services/dynamic/dynamic-json.service.js @@ -1,10 +1,15 @@ 'use strict' +const { MetricNames } = require('../../core/base-service/metric-helper') const { createRoute } = require('./dynamic-helpers') const jsonPath = require('./json-path') const { BaseJsonService } = require('..') module.exports = class DynamicJson extends jsonPath(BaseJsonService) { + static get enabledMetrics() { + return [MetricNames.SERVICE_RESPONSE_SIZE] + } + static get route() { return createRoute('json') } diff --git a/services/dynamic/dynamic-xml.service.js b/services/dynamic/dynamic-xml.service.js index 0182024f693c2b3005d12afb7fe5c3ee3b7780cc..773889be0ec03b286a9ce94265206e0fe19d1181 100644 --- a/services/dynamic/dynamic-xml.service.js +++ b/services/dynamic/dynamic-xml.service.js @@ -2,6 +2,7 @@ const { DOMParser } = require('xmldom') const xpath = require('xpath') +const { MetricNames } = require('../../core/base-service/metric-helper') const { renderDynamicBadge, errorMessages } = require('../dynamic-common') const { createRoute } = require('./dynamic-helpers') const { BaseService, InvalidResponse, InvalidParameter } = require('..') @@ -17,6 +18,10 @@ module.exports = class DynamicXml extends BaseService { return 'dynamic' } + static get enabledMetrics() { + return [MetricNames.SERVICE_RESPONSE_SIZE] + } + static get route() { return createRoute('xml') } diff --git a/services/dynamic/dynamic-yaml.service.js b/services/dynamic/dynamic-yaml.service.js index 6e0e40eeb9ebbfc597552e72d0c6d8e83a27a51c..66dab8ece6c08f3bb4399e027c0ace72f9e3cfb5 100644 --- a/services/dynamic/dynamic-yaml.service.js +++ b/services/dynamic/dynamic-yaml.service.js @@ -1,10 +1,15 @@ 'use strict' +const { MetricNames } = require('../../core/base-service/metric-helper') const { createRoute } = require('./dynamic-helpers') const jsonPath = require('./json-path') const { BaseYamlService } = require('..') module.exports = class DynamicYaml extends jsonPath(BaseYamlService) { + static get enabledMetrics() { + return [MetricNames.SERVICE_RESPONSE_SIZE] + } + static get route() { return createRoute('yaml') }