refactor(metrics-util): improved readability (#1581)

Problems I have discovered while refactoring in #1556

- inconsistency coding styles (see comments)
- very hard to metrics util and the usage of it
- we need much more pull requests to improve the readability such as "when to init" and "when to activate". but not right now

packages/aws-fargate/src/metrics/index.js

@@ -6,7 +6,6 @@
 'use strict';
 
 const { consoleLogger } = require('@instana/serverless');
-
 const transmissionCycle = require('./transmissionCycle');
 
 let logger = consoleLogger;

packages/aws-fargate/src/metrics/processorRegistry.js

@@ -5,11 +5,7 @@
 
 'use strict';
 
-const {
-  HttpDataSource,
-  nodejs: { coreAndShared, NodeJsProcessor },
-  process: { ProcessProcessor }
-} = require('@instana/metrics-util');
+const metricsUtil = require('@instana/metrics-util');
 
 const InstrumentedEcsContainerProcessor = require('./container/InstrumentedEcsContainerProcessor');
 const SecondaryEcsContainerFactory = require('./container/SecondaryEcsContainerFactory');
@@ -21,23 +17,30 @@ const SecondaryDockerProcessor = require('./docker/SecondaryDockerProcessor');
 const allProcessors = [];
 
 exports.init = function init(config, metadataUri, onReady) {
-  coreAndShared.init(config);
-
   const oneMinute = 60 * 1000;
-  const metadataRootDataSource = new HttpDataSource(metadataUri, oneMinute);
-  const metadataTaskDataSource = new HttpDataSource(`${metadataUri}/task`, oneMinute);
-  const metadataRootStatsDataSource = new HttpDataSource(`${metadataUri}/stats`);
-  const metadataTaskStatsDataSource = new HttpDataSource(`${metadataUri}/task/stats`);
 
+  // Common processors from metrics-util
+  const metadataRootDataSource = new metricsUtil.HttpDataSource(metadataUri, oneMinute);
+  const metadataTaskDataSource = new metricsUtil.HttpDataSource(`${metadataUri}/task`, oneMinute);
+  const metadataRootStatsDataSource = new metricsUtil.HttpDataSource(`${metadataUri}/stats`);
+  const metadataTaskStatsDataSource = new metricsUtil.HttpDataSource(`${metadataUri}/task/stats`);
+  const coreAndSharedMetricsDataSource = new metricsUtil.nodejs.CoreDataSource(config);
+  const nodeJsProcessor = new metricsUtil.nodejs.NodeJsProcessor(coreAndSharedMetricsDataSource, process.pid);
+  const processProcessor = new metricsUtil.process.ProcessProcessor('docker');
+
+  // Local fargate specific processors
   const ecsTaskProcessor = new EcsTaskProcessor(metadataTaskDataSource);
-  allProcessors.push(ecsTaskProcessor);
   const instrumentedEcsContainerProcessor = new InstrumentedEcsContainerProcessor(metadataRootDataSource);
-  allProcessors.push(instrumentedEcsContainerProcessor);
-  allProcessors.push(new InstrumentedDockerProcessor(metadataRootDataSource, metadataRootStatsDataSource));
+  const instrumentedDockerProcessor = new InstrumentedDockerProcessor(
+    metadataRootDataSource,
+    metadataRootStatsDataSource
+  );
 
-  const processProcessor = new ProcessProcessor('docker');
+  allProcessors.push(ecsTaskProcessor);
+  allProcessors.push(instrumentedEcsContainerProcessor);
+  allProcessors.push(instrumentedDockerProcessor);
   allProcessors.push(processProcessor);
-  allProcessors.push(new NodeJsProcessor(coreAndShared, process.pid));
+  allProcessors.push(nodeJsProcessor);
 
   instrumentedEcsContainerProcessor.once('ready', ecsContainerPayload => {
     onReady(null, ecsContainerPayload);

packages/aws-fargate/src/metrics/transmissionCycle.js

@@ -6,7 +6,6 @@
 'use strict';
 
 const { backendConnector, consoleLogger } = require('@instana/serverless');
-
 const processorRegistry = require('./processorRegistry');
 
 let logger = consoleLogger;

packages/collector/src/metrics/transmissionCycle.js

@@ -5,7 +5,7 @@
 
 'use strict';
 
-const { clone, compression } = require('@instana/core').util;
+const core = require('@instana/core');
 
 /** @type {import('@instana/core/src/core').GenericLogger} */
 let logger;
@@ -92,15 +92,15 @@ function sendMetrics() {
   }
 
   // clone retrieved objects to allow mutations in metric retrievers
-  const newValueToTransmit = clone(metrics.gatherData());
+  const newValueToTransmit = core.util.clone(metrics.gatherData());
 
   /** @type {Object<string, *>} */
   let payload;
   const isFullTransmission = transmissionsSinceLastFullDataEmit > resendFullDataEveryXTransmissions;
   if (isFullTransmission) {
     payload = newValueToTransmit;
   } else {
-    payload = compression(previousTransmittedValue, newValueToTransmit);
+    payload = core.util.compression(previousTransmittedValue, newValueToTransmit);
   }
 
   downstreamConnection.sendMetrics(payload, onMetricsHaveBeenSent.bind(null, isFullTransmission, newValueToTransmit));

packages/google-cloud-run/src/metrics/processorRegistry.js

@@ -5,40 +5,46 @@
 
 'use strict';
 
-const {
-  HttpDataSource,
-  process: { ProcessProcessor },
-  nodejs: { coreAndShared, NodeJsProcessor }
-} = require('@instana/metrics-util');
+const metricsUtil = require('@instana/metrics-util');
 
 const CloudRunServiceRevisionInstanceProcessor = require('./instance/CloudRunServiceRevisionInstanceProcessor');
 const identityProvider = require('../identity_provider');
 
 const allProcessors = [];
 
 exports.init = function init(config, metadataBaseUrl, onReady) {
-  coreAndShared.init(config);
-
   const neverRefresh = 2147483647; // max 32 bit int = max value for setTimeout
   const fetchOptions = {
     headers: { 'Metadata-Flavor': 'Google' }
   };
-  const projectDataSource = new HttpDataSource(`${metadataBaseUrl}project/?recursive=true`, neverRefresh, fetchOptions);
-  const instanceDataSource = new HttpDataSource(
+
+  // Common processors from metrics-util
+  const projectDataSource = new metricsUtil.HttpDataSource(
+    `${metadataBaseUrl}project/?recursive=true`,
+    neverRefresh,
+    fetchOptions
+  );
+  const instanceDataSource = new metricsUtil.HttpDataSource(
     `${metadataBaseUrl}instance/?recursive=true`,
     neverRefresh,
     fetchOptions
   );
+  const processProcessor = new metricsUtil.process.ProcessProcessor(
+    'gcpCloudRunInstance',
+    identityProvider.getHostHeader()
+  );
+  const coreAndSharedMetricsDataSource = new metricsUtil.nodejs.CoreDataSource(config);
+  const nodeJsProcessor = new metricsUtil.nodejs.NodeJsProcessor(coreAndSharedMetricsDataSource, process.pid);
 
+  // Local GC processors
   const cloudRunServiceRevisionProcessor = new CloudRunServiceRevisionInstanceProcessor(
     projectDataSource,
     instanceDataSource
   );
-  allProcessors.push(cloudRunServiceRevisionProcessor);
 
-  const processProcessor = new ProcessProcessor('gcpCloudRunInstance', identityProvider.getHostHeader());
+  allProcessors.push(cloudRunServiceRevisionProcessor);
   allProcessors.push(processProcessor);
-  allProcessors.push(new NodeJsProcessor(coreAndShared, process.pid));
+  allProcessors.push(nodeJsProcessor);
 
   cloudRunServiceRevisionProcessor.once('ready', payload => {
     if (onReady) {

packages/metrics-util/src/DataProcessor.js

@@ -6,10 +6,7 @@
 'use strict';
 
 const EventEmitter = require('events');
-
-const {
-  util: { compression: applyCompression }
-} = require('@instana/core');
+const core = require('@instana/core');
 
 const SKIPPED = {};
 
@@ -156,7 +153,7 @@ class DataProcessor extends EventEmitter {
     if (shouldSendUncompressedUpdate) {
       dataToBeSent = uncompressedData;
     } else {
-      dataToBeSent = applyCompression(this.lastTransmittedPayload, uncompressedData, this.compressionExcludeList);
+      dataToBeSent = core.util.compression(this.lastTransmittedPayload, uncompressedData, this.compressionExcludeList);
     }
 
     if (this.compressedTransmissionsSinceLastUncompressed >= this.sendUncompressedEveryXTransmissions) {

packages/metrics-util/src/index.js

@@ -5,10 +5,14 @@
 
 'use strict';
 
-exports = module.exports = {
-  DataProcessor: require('./DataProcessor'),
-  DataSource: require('./DataSource'),
-  HttpDataSource: require('./HttpDataSource'),
-  nodejs: require('./nodejs'),
-  process: require('./process')
-};
+const DataProcessor = require('./DataProcessor');
+const DataSource = require('./DataSource');
+const HttpDataSource = require('./HttpDataSource');
+const nodejs = require('./nodejs');
+const process = require('./process');
+
+exports.nodejs = nodejs;
+exports.process = process;
+exports.DataProcessor = DataProcessor;
+exports.DataSource = DataSource;
+exports.HttpDataSource = HttpDataSource;

packages/metrics-util/src/nodejs/CoreDataSource.js

@@ -5,33 +5,48 @@
 
 'use strict';
 
+const core = require('@instana/core');
+const sharedMetrics = require('@instana/shared-metrics');
+const { consoleLogger } = require('@instana/serverless');
 const DataSource = require('../DataSource');
 
 /**
- * A source for snapshot data and metrics that adapts the metrics from @instana/core and @instana/shared-metrics.
+ * This data source holds metrics from @instana/core and @instana/shared-metrics.
+ * This source depends on external libraries.
+ * The NodejsProcessor owns this source.
+ *
+ * A data source class defines how to collect or define data from a specific source.
+ * The outside does not need to know how the data is collected.
  */
 class CoreDataSource extends DataSource {
-  constructor(coreMetrics, refreshDelay) {
+  constructor(config, refreshDelay) {
     super(refreshDelay);
-    this.coreMetrics = coreMetrics;
+
+    core.metrics.init(config);
+    core.metrics.registerAdditionalMetrics(sharedMetrics.allMetrics);
+
+    // This will be removed by the new logger refactoring PR.
+    core.metrics.setLogger(consoleLogger);
   }
 
   activate() {
     if (!this.active) {
-      this.coreMetrics.activate();
+      core.metrics.activate();
     }
+
     super.activate();
   }
 
   deactivate() {
     if (this.active) {
-      this.coreMetrics.deactivate();
+      core.metrics.deactivate();
     }
+
     super.deactivate();
   }
 
   doRefresh(callback) {
-    this.rawData = this.coreMetrics.gatherData();
+    this.rawData = core.metrics.gatherData();
     process.nextTick(() => callback(null, this.rawData));
   }
 }

packages/metrics-util/src/nodejs/NodeJsProcessor.js

@@ -7,13 +7,11 @@
 
 const DataProcessor = require('../DataProcessor');
 
-const CoreDataSource = require('./CoreDataSource');
-
 class NodeJsProcessor extends DataProcessor {
-  constructor(coreAndShared, pid) {
+  constructor(coreAndSharedMetricsDataSource, pid) {
     super('com.instana.plugin.nodejs');
     this.pid = pid;
-    this.addSource('core', new CoreDataSource(coreAndShared));
+    this.addSource('core', coreAndSharedMetricsDataSource);
   }
 
   getEntityId() {

packages/metrics-util/src/nodejs/index.js

@@ -6,6 +6,6 @@
 'use strict';
 
 exports = module.exports = {
-  coreAndShared: require('./coreAndShared'),
-  NodeJsProcessor: require('./NodeJsProcessor')
+  NodeJsProcessor: require('./NodeJsProcessor'),
+  CoreDataSource: require('./CoreDataSource')
 };

packages/metrics-util/src/process/ProcessProcessor.js

@@ -5,7 +5,7 @@
 
 'use strict';
 
-const { secrets } = require('@instana/core');
+const core = require('@instana/core');
 
 const DataProcessor = require('../DataProcessor');
 const ProcessSnapshotDataSource = require('./ProcessSnapshotDataSource');
@@ -56,7 +56,7 @@ class ProcessProcessor extends DataProcessor {
       // the application sees.
       snapshot.env = Object.assign({}, snapshot.env);
       Object.keys(snapshot.env).forEach(envVar => {
-        if (secrets.isSecret(envVar) || envVar === 'INSTANA_AGENT_KEY') {
+        if (core.secrets.isSecret(envVar) || envVar === 'INSTANA_AGENT_KEY') {
           snapshot.env[envVar] = '<redacted>';
         }
       });

packages/metrics-util/test/nodejs/CoreDataSource_test.js

@@ -8,27 +8,18 @@
 const { expect } = require('chai');
 const semver = require('semver');
 
-const { metrics: coreMetrics } = require('@instana/core');
-
+const core = require('@instana/core');
 const { delay, retry } = require('../../../core/test/test_util');
-const config = require('@instana/core/test/config');
-
-const sharedMetrics = require('@instana/shared-metrics');
-
+const testConfig = require('@instana/core/test/config');
 const CoreDataSource = require('../../src/nodejs/CoreDataSource');
 
 describe('core data source', function () {
-  this.timeout(config.getTestTimeout());
+  this.timeout(testConfig.getTestTimeout());
 
   let dataSource;
   before(() => {
-    coreMetrics.registerAdditionalMetrics(sharedMetrics.allMetrics);
-    coreMetrics.init({
-      metrics: {
-        transmissionDelay: 1000
-      }
-    });
-    dataSource = new CoreDataSource(coreMetrics);
+    const config = core.util.normalizeConfig({});
+    dataSource = new CoreDataSource(config);
   });
 
   afterEach(() => {