fix: remove enabled flag from startMetrics (#429)

* fix: remove enabled flag from startMetrics * Change files * fix change json message
signalfx · Mar 15, 2022 · 5b42126 · 5b42126
1 parent a21db72
commit 5b42126
Show file tree

Hide file tree

Showing 5 changed files with 15 additions and 44 deletions.
diff --git a/change/@splunk-otel-c79d7401-d939-4997-b613-c4d48d65fdbe.json b/change/@splunk-otel-c79d7401-d939-4997-b613-c4d48d65fdbe.json
@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "remove enabled flag from startMetrics options",
+  "packageName": "@splunk/otel",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/docs/advanced-config.md b/docs/advanced-config.md
@@ -61,7 +61,7 @@ The following config options can be set by passing them as arguments to `startTr
 
 | Environment variable<br>``startMetrics()`` argument             | Default value           | Support | Notes
 | --------------------------------------------------------------- | ----------------------- | ------- | ---
-| `SPLUNK_METRICS_ENABLED`<br>`enabled`                           | `false`                 | Experimental | Enabled metrics export. See [metrics documentation](metrics.md) for more information.
+| `SPLUNK_METRICS_ENABLED`                                        | `false`                 | Experimental | Enabled metrics export. See [metrics documentation](metrics.md) for more information.
 | `SPLUNK_METRICS_ENDPOINT`<br>`endpoint`                         | `http://localhost:9943` | Experimental | The metrics endpoint to send to.
 | `SPLUNK_METRICS_EXPORT_INTERVAL`<br>`exportInterval`            | `5000`                  | Experimental | The interval, in milliseconds, of metrics collection and exporting.
 

diff --git a/src/instrument.ts b/src/instrument.ts
@@ -22,5 +22,9 @@ import { getEnvBoolean } from './options';
 if (getEnvBoolean('SPLUNK_PROFILER_ENABLED', false)) {
   startProfiling();
 }
+
 startTracing();
-startMetrics();
+
+if (getEnvBoolean('SPLUNK_METRICS_ENABLED', false)) {
+  startMetrics();
+}
diff --git a/src/metrics/index.ts b/src/metrics/index.ts
@@ -17,13 +17,12 @@
 import { context, diag } from '@opentelemetry/api';
 import { suppressTracing } from '@opentelemetry/core';
 import { collectMemoryInfo, MemoryInfo } from './memory';
-import { defaultServiceName, getEnvBoolean, getEnvNumber } from '../options';
+import { defaultServiceName, getEnvNumber } from '../options';
 import { EnvResourceDetector } from '../resource';
 import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions';
 import * as signalfx from 'signalfx';
 
 interface MetricsOptions {
-  enabled: boolean;
   serviceName: string;
   accessToken: string;
   endpoint: string;
@@ -80,13 +79,6 @@ export type StartMetricsOptions = Partial<MetricsOptions> & {
 export function startMetrics(opts: StartMetricsOptions = {}) {
   const options = _setDefaultOptions(opts);
 
-  if (!options.enabled) {
-    return {
-      stopMetrics: () => {},
-      getSignalFxClient: () => undefined,
-    };
-  }
-
   const signalFxClient = options.sfxClient;
   const registry = _createSignalFxMetricsRegistry(options.sfxClient);
 
@@ -275,8 +267,6 @@ function _loadExtension(): CountersExtension | undefined {
 export function _setDefaultOptions(
   options: StartMetricsOptions = {}
 ): MetricsOptions & { sfxClient: signalfx.SignalClient } {
-  const enabled =
-    options.enabled ?? getEnvBoolean('SPLUNK_METRICS_ENABLED', false);
   const accessToken =
     options.accessToken || process.env.SPLUNK_ACCESS_TOKEN || '';
   const endpoint =
@@ -310,7 +300,6 @@ export function _setDefaultOptions(
     });
 
   return {
-    enabled,
     serviceName: serviceName,
     accessToken,
     endpoint,

diff --git a/test/metrics.test.ts b/test/metrics.test.ts
@@ -94,7 +94,6 @@ describe('metrics', () => {
 
     it('has expected defaults', () => {
       const options = _setDefaultOptions();
-      assert.deepEqual(options.enabled, false);
       assert.deepEqual(options.serviceName, 'unnamed-node-service');
       assert.deepEqual(options.accessToken, '');
       assert.deepEqual(options.endpoint, 'http://localhost:9943');
@@ -111,12 +110,10 @@ describe('metrics', () => {
     it('is possible to set options via env vars', () => {
       process.env.SPLUNK_ACCESS_TOKEN = 'foo';
       process.env.OTEL_SERVICE_NAME = 'bigmetric';
-      process.env.SPLUNK_METRICS_ENABLED = 'true';
       process.env.SPLUNK_METRICS_ENDPOINT = 'http://localhost:9999';
       process.env.SPLUNK_METRICS_EXPORT_INTERVAL = '1000';
 
       const options = _setDefaultOptions();
-      assert.deepEqual(options.enabled, true);
       assert.deepEqual(options.serviceName, 'bigmetric');
       assert.deepEqual(options.accessToken, 'foo');
       assert.deepEqual(options.endpoint, 'http://localhost:9999');
@@ -140,7 +137,6 @@ describe('metrics', () => {
       const gcMetric = (name: string) => m =>
         metric(name)(m) && m.dimensions['gctype'] === 'all';
       const { stopMetrics } = startMetrics({
-        enabled: true,
         exportInterval: 100,
         signalfx: {
           client: {
@@ -171,33 +167,8 @@ describe('metrics', () => {
       });
     });
 
-    it('does not export metrics when disabled', done => {
-      startMetrics({
-        exportInterval: 1,
-        signalfx: {
-          client: {
-            send: () => {
-              assert(false, 'did not expect metric send to be called');
-            },
-          },
-        },
-      });
-
-      setTimeout(() => {
-        done();
-      }, 30);
-    });
-
-    it('returns an undefined SignalFx client when disabled', () => {
-      const { getSignalFxClient } = startMetrics();
-      const client = getSignalFxClient();
-      assert.equal(client, undefined);
-    });
-
     it('is possible to get the current signalfx client', () => {
-      const { stopMetrics, getSignalFxClient } = startMetrics({
-        enabled: true,
-      });
+      const { stopMetrics, getSignalFxClient } = startMetrics();
       const client = getSignalFxClient();
       stopMetrics();
       assert(client);