Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove enabled flag from startMetrics #429

Merged
merged 3 commits into from
Mar 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "remove enabled flag from startMetrics options",
"packageName": "@splunk/otel",
"email": "[email protected]",
"dependentChangeType": "patch"
}
2 changes: 1 addition & 1 deletion docs/advanced-config.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
6 changes: 5 additions & 1 deletion src/instrument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,9 @@ import { getEnvBoolean } from './options';
if (getEnvBoolean('SPLUNK_PROFILER_ENABLED', false)) {
startProfiling();
}

startTracing();
startMetrics();

if (getEnvBoolean('SPLUNK_METRICS_ENABLED', false)) {
startMetrics();
}
13 changes: 1 addition & 12 deletions src/metrics/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -310,7 +300,6 @@ export function _setDefaultOptions(
});

return {
enabled,
serviceName: serviceName,
accessToken,
endpoint,
Expand Down
31 changes: 1 addition & 30 deletions test/metrics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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');
Expand All @@ -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: {
Expand Down Expand Up @@ -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);
Expand Down