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.

Sign up for GitHub

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

Jump to bottom

Remove process listener #1522

Merged
merged 4 commits into from
Sep 21, 2020
Merged

Remove process listener #1522

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a0b3de0
chore: remove SIGTERM listener
dyladan Sep 4, 2020
2edf8f1
Merge remote-tracking branch 'origin/master' into no-process-listener
dyladan Sep 21, 2020
f0cddd0
chore: add documentation for shutdown method
dyladan Sep 21, 2020
98810d2
Merge branch 'master' into no-process-listener
dyladan Sep 21, 2020
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
32 changes: 0 additions & 32 deletions packages/opentelemetry-core/src/platform/browser/ShutdownNotifier.ts
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion packages/opentelemetry-core/src/platform/browser/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,3 @@ export * from './RandomIdGenerator';
export * from './performance';
export * from './sdk-info';
export * from './timer-util';
export * from './ShutdownNotifier';
32 changes: 0 additions & 32 deletions packages/opentelemetry-core/src/platform/node/ShutdownNotifier.ts
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion packages/opentelemetry-core/src/platform/node/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,3 @@ export * from './RandomIdGenerator';
export * from './performance';
export * from './sdk-info';
export * from './timer-util';
export * from './ShutdownNotifier';
76 changes: 0 additions & 76 deletions packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@
*/

import { ObserverResult } from '@opentelemetry/api';
import {
notifyOnGlobalShutdown,
_invokeGlobalShutdown,
} from '@opentelemetry/core';
import {
CounterMetric,
SumAggregator,
Expand All @@ -32,7 +28,6 @@ import { PrometheusExporter } from '../src';
import { mockAggregator, mockedHrTimeMs } from './util';

describe('PrometheusExporter', () => {
let removeEvent: Function | undefined;
mockAggregator(SumAggregator);
mockAggregator(MinMaxLastSumCountAggregator);

Expand Down Expand Up @@ -185,7 +180,6 @@ describe('PrometheusExporter', () => {
exporter = new PrometheusExporter();
meterProvider = new MeterProvider({
interval: Math.pow(2, 31) - 1,
gracefulShutdown: true,
});
meter = meterProvider.getMeter('test-prometheus', '1', {
exporter: exporter,
Expand All @@ -195,10 +189,6 @@ describe('PrometheusExporter', () => {

afterEach(done => {
exporter.shutdown().then(done);
if (removeEvent) {
removeEvent();
removeEvent = undefined;
}
});

it('should export a count aggregation', done => {
Expand Down Expand Up @@ -320,39 +310,6 @@ describe('PrometheusExporter', () => {
});
});

it('should export multiple labels on graceful shutdown', done => {
const counter = meter.createCounter('counter', {
description: 'a test description',
}) as CounterMetric;

counter.bind({ counterKey1: 'labelValue1' }).add(10);
counter.bind({ counterKey1: 'labelValue2' }).add(20);
counter.bind({ counterKey1: 'labelValue3' }).add(30);

removeEvent = notifyOnGlobalShutdown(() => {
http
.get('http://localhost:9464/metrics', res => {
res.on('data', chunk => {
const body = chunk.toString();
const lines = body.split('\n');

assert.deepStrictEqual(lines, [
'# HELP counter a test description',
'# TYPE counter counter',
`counter{counterKey1="labelValue1"} 10 ${mockedHrTimeMs}`,
`counter{counterKey1="labelValue2"} 20 ${mockedHrTimeMs}`,
`counter{counterKey1="labelValue3"} 30 ${mockedHrTimeMs}`,
'',
]);

done();
});
})
.on('error', errorHandler(done));
});
_invokeGlobalShutdown();
});

it('should export multiple labels on manual shutdown', done => {
const counter = meter.createCounter('counter', {
description: 'a test description',
Expand Down Expand Up @@ -384,39 +341,6 @@ describe('PrometheusExporter', () => {
});
});

it('should export multiple labels on graceful shutdown', done => {
const counter = meter.createCounter('counter', {
description: 'a test description',
}) as CounterMetric;

counter.bind({ counterKey1: 'labelValue1' }).add(10);
counter.bind({ counterKey1: 'labelValue2' }).add(20);
counter.bind({ counterKey1: 'labelValue3' }).add(30);

removeEvent = notifyOnGlobalShutdown(() => {
http
.get('http://localhost:9464/metrics', res => {
res.on('data', chunk => {
const body = chunk.toString();
const lines = body.split('\n');

assert.deepStrictEqual(lines, [
'# HELP counter a test description',
'# TYPE counter counter',
`counter{counterKey1="labelValue1"} 10 ${mockedHrTimeMs}`,
`counter{counterKey1="labelValue2"} 20 ${mockedHrTimeMs}`,
`counter{counterKey1="labelValue3"} 30 ${mockedHrTimeMs}`,
'',
]);

done();
});
})
.on('error', errorHandler(done));
});
_invokeGlobalShutdown();
});

it('should export multiple labels on manual shutdown', done => {
const counter = meter.createCounter('counter', {
description: 'a test description',
Expand Down
16 changes: 1 addition & 15 deletions packages/opentelemetry-metrics/src/MeterProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import { ConsoleLogger, notifyOnGlobalShutdown } from '@opentelemetry/core';
import { ConsoleLogger } from '@opentelemetry/core';
import * as api from '@opentelemetry/api';
import { Resource } from '@opentelemetry/resources';
import { Meter } from '.';
Expand All @@ -26,7 +26,6 @@ import { DEFAULT_CONFIG, MeterConfig } from './types';
export class MeterProvider implements api.MeterProvider {
private readonly _config: MeterConfig;
private readonly _meters: Map<string, Meter> = new Map();
private _cleanNotifyOnGlobalShutdown: Function | undefined;
private _shuttingDownPromise: Promise<void> = Promise.resolve();
private _isShutdown = false;
readonly resource: Resource;
Expand All @@ -39,11 +38,6 @@ export class MeterProvider implements api.MeterProvider {
logger: this.logger,
resource: this.resource,
});
if (this._config.gracefulShutdown) {
this._cleanNotifyOnGlobalShutdown = notifyOnGlobalShutdown(() => {
this._shutdownAllMeters().catch();
});
}
}

/**
Expand All @@ -64,14 +58,6 @@ export class MeterProvider implements api.MeterProvider {
}

shutdown(): Promise<void> {
if (this._cleanNotifyOnGlobalShutdown) {
this._cleanNotifyOnGlobalShutdown();
this._cleanNotifyOnGlobalShutdown = undefined;
}
return this._shutdownAllMeters();
}

private _shutdownAllMeters(): Promise<void> {
if (this._isShutdown) {
return this._shuttingDownPromise;
}
Expand Down
4 changes: 0 additions & 4 deletions packages/opentelemetry-metrics/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,11 @@ export interface MeterConfig {

/** Metric batcher. */
batcher?: Batcher;

/** Bool for whether or not graceful shutdown is enabled. If disabled metrics will not be exported when SIGTERM is recieved */
gracefulShutdown?: boolean;
}

/** Default Meter configuration. */
export const DEFAULT_CONFIG = {
logLevel: getEnv().OTEL_LOG_LEVEL,
gracefulShutdown: true,
};

/** The default metric creation options value. */
Expand Down
58 changes: 1 addition & 57 deletions packages/opentelemetry-metrics/test/MeterProvider.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,9 @@
import * as assert from 'assert';
import * as sinon from 'sinon';
import { MeterProvider, Meter, CounterMetric } from '../src';
import {
NoopLogger,
notifyOnGlobalShutdown,
_invokeGlobalShutdown,
} from '@opentelemetry/core';
import { NoopLogger } from '@opentelemetry/core';

describe('MeterProvider', () => {
let removeEvent: Function | undefined;
let sandbox: sinon.SinonSandbox;

beforeEach(() => {
sandbox = sinon.createSandbox();
});

afterEach(() => {
if (removeEvent) {
removeEvent();
removeEvent = undefined;
}
});

describe('constructor', () => {
it('should construct an instance without any options', () => {
const provider = new MeterProvider();
Expand Down Expand Up @@ -94,32 +76,9 @@ describe('MeterProvider', () => {
});

describe('shutdown()', () => {
it('should call shutdown when SIGTERM is received', () => {
const meterProvider = new MeterProvider({
interval: Math.pow(2, 31) - 1,
gracefulShutdown: true,
});
const shutdownStub1 = sandbox.stub(
meterProvider.getMeter('meter1'),
'shutdown'
);
const shutdownStub2 = sandbox.stub(
meterProvider.getMeter('meter2'),
'shutdown'
);
removeEvent = notifyOnGlobalShutdown(() => {
setImmediate(() => {
sinon.assert.calledOnce(shutdownStub1);
sinon.assert.calledOnce(shutdownStub2);
});
});
_invokeGlobalShutdown();
});

it('should call shutdown when manually invoked', () => {
const meterProvider = new MeterProvider({
interval: Math.pow(2, 31) - 1,
gracefulShutdown: true,
});
const sandbox = sinon.createSandbox();
const shutdownStub1 = sandbox.stub(
Expand All @@ -135,20 +94,5 @@ describe('MeterProvider', () => {
sinon.assert.calledOnce(shutdownStub2);
});
});

it('should not trigger shutdown if graceful shutdown is turned off', () => {
const meterProvider = new MeterProvider({
interval: Math.pow(2, 31) - 1,
gracefulShutdown: false,
});
const shutdownStub = sandbox.stub(
meterProvider.getMeter('meter1'),
'shutdown'
);
removeEvent = notifyOnGlobalShutdown(() => {
sinon.assert.notCalled(shutdownStub);
});
_invokeGlobalShutdown();
});
});
});
23 changes: 23 additions & 0 deletions packages/opentelemetry-sdk-node/src/sdk.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ export class NodeSDK {

private _autoDetectResources: boolean;

private _tracerProvider?: NodeTracerProvider;
private _meterProvider?: MeterProvider;

/**
* Create a new NodeJS SDK instance
*/
Expand Down Expand Up @@ -154,6 +157,8 @@ export class NodeSDK {
resource: this._resource,
});

this._tracerProvider = tracerProvider;

tracerProvider.addSpanProcessor(this._tracerProviderConfig.spanProcessor);
tracerProvider.register({
contextManager: this._tracerProviderConfig.contextManager,
Expand All @@ -167,7 +172,25 @@ export class NodeSDK {
resource: this._resource,
});

this._meterProvider = meterProvider;

metrics.setGlobalMeterProvider(meterProvider);
}
}

public shutdown(): Promise<void> {
const promises: Promise<unknown>[] = [];
if (this._tracerProvider) {
promises.push(this._tracerProvider.shutdown());
}
if (this._meterProvider) {
promises.push(this._meterProvider.shutdown());
}

return (
Promise.all(promises)
// return void instead of the array from Promise.all
.then(() => {})
);
}
}
Loading