fixups post reviews

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],

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();
         });
     }
 }

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);
 });

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 => {

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,
 };