From 6b10225c16617567c99488e5d4fd0f142f785ad5 Mon Sep 17 00:00:00 2001 From: Maha Benzekri Date: Wed, 15 May 2024 14:27:45 +0200 Subject: [PATCH] fixups post reviews --- CountItems/CountManager.js | 16 -------- CountItems/CountMaster.js | 11 +----- CountItems/masterProcess.js | 15 ++++--- utils/S3UtilsMongoClient.js | 6 +-- utils/monitoring.js | 79 +++++++++++++------------------------ 5 files changed, 40 insertions(+), 87 deletions(-) diff --git a/CountItems/CountManager.js b/CountItems/CountManager.js index 4fcaa6c0..e82accc3 100644 --- a/CountItems/CountManager.js +++ b/CountItems/CountManager.js @@ -27,9 +27,6 @@ class CountManager { account: {}, }; this.workerList = []; - this.uniqueAccounts = new Set(); - this.uniqueLocations = new Set(); - this.uniqueBuckets = new Set(); this._setupQueue(); } @@ -53,7 +50,6 @@ class CountManager { if (err) { return done(err); } - monitoring.bucketCount.inc(); this._consolidateData(res); this.workerList.push(id); return done(); @@ -95,18 +91,6 @@ class CountManager { // metricLevel can only be 'bucket', 'location' or 'account' if (validStorageMetricLevels.has(metricLevel)) { Object.keys(results.dataMetrics[metricLevel]).forEach(resourceName => { - if (metricLevel === 'account' && !this.uniqueAccounts.has(resourceName)) { - this.uniqueAccounts.add(resourceName); - monitoring.metricsCount.inc({ metricLevel: 'account' }); - } - if (metricLevel === 'location' && !this.uniqueLocations.has(resourceName)) { - this.uniqueLocations.add(resourceName); - monitoring.metricsCount.inc({ metricLevel: 'location' }); - } - if (metricLevel === 'bucket' && !this.uniqueBuckets.has(resourceName)) { - this.uniqueBuckets.add(resourceName); - monitoring.metricsCount.inc({ metricLevel: 'bucket' }); - } // resourceName can be the name of bucket, location or account this.dataMetrics[metricLevel][resourceName] = consolidateDataMetrics( this.dataMetrics[metricLevel][resourceName], diff --git a/CountItems/CountMaster.js b/CountItems/CountMaster.js index edd9ec8c..e7ec5582 100644 --- a/CountItems/CountMaster.js +++ b/CountItems/CountMaster.js @@ -1,13 +1,11 @@ const async = require('async'); const { reshapeExceptionError } = require('arsenal').errorUtils; -const { threshold, frequency } = require('../utils/constants'); class CountMaster { constructor(params) { this.log = params.log; this.manager = params.manager; this.client = params.client; - this.metrics = params.metrics; CountMaster.waitingForPromScraping = false; } @@ -37,13 +35,6 @@ class CountMaster { } return next(); }), - next => { - this.metrics.start(); - this.log.info('metrics server started', { - port: 8003, - }); - return next(); - }, next => this.manager.setup(next), next => this.client.getBucketInfos(this.log, (err, bucketList) => { if (err) { @@ -69,7 +60,7 @@ class CountMaster { return this.stop(null, () => callback(err)); } CountMaster.waitingForPromScraping = true; - return setTimeout(() => this.stop(null, () => callback()), threshold * frequency * 1000 * 4); + return callback(); }); } } diff --git a/CountItems/masterProcess.js b/CountItems/masterProcess.js index 5eb4800d..2605b4f1 100644 --- a/CountItems/masterProcess.js +++ b/CountItems/masterProcess.js @@ -1,6 +1,7 @@ const werelogs = require('werelogs'); const { network } = require('arsenal'); const { reshapeExceptionError } = require('arsenal').errorUtils; +const { threshold, frequency } = require('../utils/constants'); const S3UtilsMongoClient = require('../utils/S3UtilsMongoClient'); const CountMaster = require('./CountMaster'); @@ -31,7 +32,6 @@ const concurrentCursors = (process.env.CONCURRENT_CURSORS ? Number.parseInt(process.env.CONCURRENT_CURSORS, 10) : 5; -const metricServer = new WebServer(8003, log); const countMaster = new CountMaster({ log, manager: new CountManager({ @@ -40,17 +40,18 @@ const countMaster = new CountMaster({ maxConcurrent: concurrentCursors, }), client: new S3UtilsMongoClient(createMongoParams(log)), - metrics: metricServer, }); -metricServer.onRequest((req, res) => monitoring.metricsHandler( - countMaster, +const metricServer = new WebServer(8003, log).onRequest((req, res) => monitoring.metricsHandler( () => { - process.exit(1); + if (CountMaster.waitingForPromScraping === true) { + countMaster.stop(null, () => process.exit(1)); + } }, req, res, )); +metricServer.start(); const handleSignal = sig => countMaster.stop(sig, () => process.exit(0)); process.on('SIGINT', handleSignal); @@ -68,5 +69,7 @@ countMaster.start(err => { if (err) { process.exit(1); } - process.exit(0); + setTimeout(() => { + countMaster.stop(null, () => process.exit(0)); + }, threshold * frequency * 1000 * 4); }); diff --git a/utils/S3UtilsMongoClient.js b/utils/S3UtilsMongoClient.js index 4e1f48b9..ab7b3689 100644 --- a/utils/S3UtilsMongoClient.js +++ b/utils/S3UtilsMongoClient.js @@ -151,7 +151,7 @@ class S3UtilsMongoClient extends MongoClientInterface { entry: res, error, }); - monitoring.objectsCount.inc({ state: 'error' }); + monitoring.objectsCount.inc({ status: 'error' }); return; } @@ -161,7 +161,7 @@ class S3UtilsMongoClient extends MongoClientInterface { method: 'getObjectMDStats', entry: res, }); - monitoring.objectsCount.inc({ state: 'skipped' }); + monitoring.objectsCount.inc({ status: 'skipped' }); return; } @@ -234,7 +234,7 @@ class S3UtilsMongoClient extends MongoClientInterface { collRes.account[account].locations[location].deleteMarkerCount += res.value.isDeleteMarker ? 1 : 0; }); }); - monitoring.objectsCount.inc({ state: 'success' }); + monitoring.objectsCount.inc({ status: 'success' }); processed++; }, err => { diff --git a/utils/monitoring.js b/utils/monitoring.js index ce934e27..1e682755 100644 --- a/utils/monitoring.js +++ b/utils/monitoring.js @@ -1,82 +1,59 @@ const { errors } = require('arsenal'); const promClient = require('prom-client'); const { Registry } = require('prom-client'); +const { http } = require('httpagent'); const CountMaster = require('../CountItems/CountMaster'); const aggregatorRegistry = new promClient.AggregatorRegistry(); const { collectDefaultMetrics } = promClient; -const bucketCount = (promClient.register.getSingleMetric('bucketCount_metrics')) - || new promClient.Gauge({ - name: 'bucketCount_metrics', - help: 'Total number of buckets processed', - labelNames: ['bucketName'], - }); // Histogram of the bucket processing duration, by the utilization service. +const bucketProcessingDuration = new promClient.Histogram({ + name: 's3_countitems_bucket_listing_duration_seconds', + help: 'Bucket processing duration', + buckets: [1, 10, 60, 600, 3600, 18000, 36000], +}); -const bucketProcessingDuration = (promClient.register.getSingleMetric('count_items_bucketProcessingDuration')) - || new promClient.Histogram({ - name: 'count_items_bucketProcessingDuration', - help: 'Bucket processing duration', - labelNames: ['service'], - buckets: [1, 10, 60, 600, 3600, 18000, 36000], - }); +const consolidationDuration = new promClient.Histogram({ + name: 's3_countitems_bucket_merge_duration_seconds', + help: 'Duration of metrics consolidation in seconds', + buckets: [0.01, 0.05, 0.1, 0.2, 0.5, 1, 2, 5, 10], +}); -const consolidationDuration = (promClient.register.getSingleMetric('count_items_consolidationDuration')) - || new promClient.Histogram({ - name: 'count_items_consolidationDuration', - help: 'Duration of metrics consolidation in seconds', - labelNames: ['service'], - buckets: [0.01, 0.05, 0.1, 0.2, 0.5, 1, 1.5, 2], - }); +const workersCount = new promClient.Counter({ + name: 's3_countitems_worker_errors_count', + help: 'Number of errors in counting workers', + labelNames: ['state'], +}); -const workersCount = (promClient.register.getSingleMetric('count_items_workersCountErrors')) - || new promClient.Counter({ - name: 'count_items_workersCountErrors', - help: 'Number of errors in counting workers', - labelNames: ['state'], - }); +const objectsCount = new promClient.Counter({ + name: 's3_countitems_total_objects_count', + help: 'Number of objects processed', + labelNames: ['status'], +}); -const objectsCount = (promClient.register.getSingleMetric('count_items_objectsCount')) - || new promClient.Counter({ - name: 'count_items_objectsCount', - help: 'Number of objects processed', - labelNames: ['state'], - }); - -const metricsCount = (promClient.register.getSingleMetric('count_items_metricsCount')) - || new promClient.Counter({ - name: 'count_items_metricsCount', - help: 'Number of entities with data usage', - labelNames: ['metricLevel'], - }); /** * @param {http.ServerResponse} res - http response object * @param {Error | errors.ArsenalError} error - Error * @return {void} */ function _writeResponse(res, error) { - let statusCode = 200; - if (error) { - if (error instanceof errors.ArsenalError && Number.isInteger(error.code)) { - statusCode = error.code; - } else { - statusCode = 500; - } + let statusCode = 500; + if (error instanceof errors.ArsenalError && Number.isInteger(error.code)) { + statusCode = error.code; } res.writeHead(statusCode, { 'Content-Type': 'application/json' }); res.end(); } /** - * @param {number} countMasterInstance - countMaster instance * @param {function} onScraped - callback to call when metrics are scraped * @param {http.IncomingMessage} req - http request object * @param {http.ServerResponse} res - http response object * @return {void} */ -async function metricsHandler(countMasterInstance, onScraped, req, res) { +async function metricsHandler(onScraped, req, res) { if (req.method !== 'GET' || req.url !== '/metrics') { return _writeResponse(res, errors.MethodNotAllowed); } @@ -94,8 +71,8 @@ async function metricsHandler(countMasterInstance, onScraped, req, res) { } catch (ex) { return _writeResponse(res, ex); } finally { - if (CountMaster.waitingForPromScraping === true) { - countMasterInstance.stop(null, onScraped); + if (onScraped) { + onScraped(); } } } @@ -104,10 +81,8 @@ module.exports = { client: promClient, collectDefaultMetrics, metricsHandler, - bucketCount, bucketProcessingDuration, consolidationDuration, workersCount, objectsCount, - metricsCount, };