From 533e2a37b83b5915a47e87bbbc955c8677774394 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Fri, 4 Sep 2020 14:32:33 -0400 Subject: [PATCH] chore: remove SIGTERM listener --- .../src/platform/browser/ShutdownNotifier.ts | 32 --------- .../src/platform/node/ShutdownNotifier.ts | 32 --------- .../test/PrometheusExporter.test.ts | 67 ------------------- .../src/MeterProvider.ts | 13 ---- packages/opentelemetry-metrics/src/types.ts | 4 -- .../test/MeterProvider.test.ts | 40 ----------- packages/opentelemetry-sdk-node/src/sdk.ts | 21 ++++++ .../src/BasicTracerProvider.ts | 14 ---- .../test/BasicTracerProvider.test.ts | 29 -------- .../test/MultiSpanProcessor.test.ts | 50 -------------- 10 files changed, 21 insertions(+), 281 deletions(-) delete mode 100644 packages/opentelemetry-core/src/platform/browser/ShutdownNotifier.ts delete mode 100644 packages/opentelemetry-core/src/platform/node/ShutdownNotifier.ts diff --git a/packages/opentelemetry-core/src/platform/browser/ShutdownNotifier.ts b/packages/opentelemetry-core/src/platform/browser/ShutdownNotifier.ts deleted file mode 100644 index 05ccc38e011..00000000000 --- a/packages/opentelemetry-core/src/platform/browser/ShutdownNotifier.ts +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * Adds an event listener to trigger a callback when an unload event in the window is detected - */ -export function notifyOnGlobalShutdown(cb: () => void): () => void { - window.addEventListener('unload', cb, { once: true }); - return function removeCallbackFromGlobalShutdown() { - window.removeEventListener('unload', cb, false); - }; -} - -/** - * Warning: meant for internal use only! Closes the current window, triggering the unload event - */ -export function _invokeGlobalShutdown() { - window.close(); -} diff --git a/packages/opentelemetry-core/src/platform/node/ShutdownNotifier.ts b/packages/opentelemetry-core/src/platform/node/ShutdownNotifier.ts deleted file mode 100644 index f9868105aff..00000000000 --- a/packages/opentelemetry-core/src/platform/node/ShutdownNotifier.ts +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * Adds an event listener to trigger a callback when a SIGTERM is detected in the process - */ -export function notifyOnGlobalShutdown(cb: () => void): () => void { - process.once('SIGTERM', cb); - return function removeCallbackFromGlobalShutdown() { - process.removeListener('SIGTERM', cb); - }; -} - -/** - * Warning: meant for internal use only! Sends a SIGTERM to the current process - */ -export function _invokeGlobalShutdown() { - process.kill(process.pid, 'SIGTERM'); -} diff --git a/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts b/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts index 4153ebf8c38..19baaf5c4fb 100644 --- a/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts +++ b/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts @@ -16,7 +16,6 @@ import { ObserverResult } from '@opentelemetry/api'; import { - notifyOnGlobalShutdown, _invokeGlobalShutdown, } from '@opentelemetry/core'; import { @@ -320,39 +319,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', @@ -384,39 +350,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', diff --git a/packages/opentelemetry-metrics/src/MeterProvider.ts b/packages/opentelemetry-metrics/src/MeterProvider.ts index 5fb6b87e4a1..24e7543ea75 100644 --- a/packages/opentelemetry-metrics/src/MeterProvider.ts +++ b/packages/opentelemetry-metrics/src/MeterProvider.ts @@ -39,11 +39,6 @@ export class MeterProvider implements api.MeterProvider { logger: this.logger, resource: this.resource, }); - if (this._config.gracefulShutdown) { - this._cleanNotifyOnGlobalShutdown = notifyOnGlobalShutdown(() => { - this._shutdownAllMeters().catch(); - }); - } } /** @@ -64,14 +59,6 @@ export class MeterProvider implements api.MeterProvider { } shutdown(): Promise { - if (this._cleanNotifyOnGlobalShutdown) { - this._cleanNotifyOnGlobalShutdown(); - this._cleanNotifyOnGlobalShutdown = undefined; - } - return this._shutdownAllMeters(); - } - - private _shutdownAllMeters(): Promise { if (this._isShutdown) { return this._shuttingDownPromise; } diff --git a/packages/opentelemetry-metrics/src/types.ts b/packages/opentelemetry-metrics/src/types.ts index d8e5768a482..1eed57a46e5 100644 --- a/packages/opentelemetry-metrics/src/types.ts +++ b/packages/opentelemetry-metrics/src/types.ts @@ -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. */ diff --git a/packages/opentelemetry-metrics/test/MeterProvider.test.ts b/packages/opentelemetry-metrics/test/MeterProvider.test.ts index 1aee28f4307..f819336a6f3 100644 --- a/packages/opentelemetry-metrics/test/MeterProvider.test.ts +++ b/packages/opentelemetry-metrics/test/MeterProvider.test.ts @@ -19,8 +19,6 @@ import * as sinon from 'sinon'; import { MeterProvider, Meter, CounterMetric } from '../src'; import { NoopLogger, - notifyOnGlobalShutdown, - _invokeGlobalShutdown, } from '@opentelemetry/core'; describe('MeterProvider', () => { @@ -94,32 +92,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( @@ -135,20 +110,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(); - }); }); }); diff --git a/packages/opentelemetry-sdk-node/src/sdk.ts b/packages/opentelemetry-sdk-node/src/sdk.ts index a411ab85dfe..49a1f127c5b 100644 --- a/packages/opentelemetry-sdk-node/src/sdk.ts +++ b/packages/opentelemetry-sdk-node/src/sdk.ts @@ -43,6 +43,9 @@ export class NodeSDK { private _autoDetectResources: boolean; + private _tracerProvider?: NodeTracerProvider; + private _meterProvider?: MeterProvider; + /** * Create a new NodeJS SDK instance */ @@ -154,6 +157,8 @@ export class NodeSDK { resource: this._resource, }); + this._tracerProvider = tracerProvider; + tracerProvider.addSpanProcessor(this._tracerProviderConfig.spanProcessor); tracerProvider.register({ contextManager: this._tracerProviderConfig.contextManager, @@ -167,7 +172,23 @@ export class NodeSDK { resource: this._resource, }); + this._meterProvider = meterProvider; + metrics.setGlobalMeterProvider(meterProvider); } } + + public shutdown(): Promise { + const promises: Promise[] = []; + 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(() => {}); + } } diff --git a/packages/opentelemetry-tracing/src/BasicTracerProvider.ts b/packages/opentelemetry-tracing/src/BasicTracerProvider.ts index 065e74b2b89..a0669129b95 100644 --- a/packages/opentelemetry-tracing/src/BasicTracerProvider.ts +++ b/packages/opentelemetry-tracing/src/BasicTracerProvider.ts @@ -20,7 +20,6 @@ import { HttpTraceContext, HttpCorrelationContext, CompositePropagator, - notifyOnGlobalShutdown, } from '@opentelemetry/core'; import { SpanProcessor, Tracer } from '.'; import { DEFAULT_CONFIG } from './config'; @@ -49,11 +48,6 @@ export class BasicTracerProvider implements api.TracerProvider { logger: this.logger, resource: this.resource, }); - if (this._config.gracefulShutdown) { - this._cleanNotifyOnGlobalShutdown = notifyOnGlobalShutdown( - this._shutdownActiveProcessor.bind(this) - ); - } } getTracer(name: string, version = '*', config?: TracerConfig): Tracer { @@ -108,14 +102,6 @@ export class BasicTracerProvider implements api.TracerProvider { } shutdown() { - if (this._cleanNotifyOnGlobalShutdown) { - this._cleanNotifyOnGlobalShutdown(); - this._cleanNotifyOnGlobalShutdown = undefined; - } - return this.activeSpanProcessor.shutdown(); - } - - private _shutdownActiveProcessor() { return this.activeSpanProcessor.shutdown(); } } diff --git a/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts b/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts index 2e95fce0203..e8e7aea32aa 100644 --- a/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts +++ b/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts @@ -24,8 +24,6 @@ import { setActiveSpan, setExtractedSpanContext, TraceState, - notifyOnGlobalShutdown, - _invokeGlobalShutdown, } from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; import * as assert from 'assert'; @@ -369,18 +367,6 @@ describe('BasicTracerProvider', () => { }); describe('.shutdown()', () => { - it('should trigger shutdown when SIGTERM is recieved', () => { - const tracerProvider = new BasicTracerProvider(); - const shutdownStub = sandbox.stub( - tracerProvider.getActiveSpanProcessor(), - 'shutdown' - ); - removeEvent = notifyOnGlobalShutdown(() => { - sinon.assert.calledOnce(shutdownStub); - }); - _invokeGlobalShutdown(); - }); - it('should trigger shutdown when manually invoked', () => { const tracerProvider = new BasicTracerProvider(); const shutdownStub = sandbox.stub( @@ -390,20 +376,5 @@ describe('BasicTracerProvider', () => { tracerProvider.shutdown(); sinon.assert.calledOnce(shutdownStub); }); - - it('should not trigger shutdown if graceful shutdown is turned off', () => { - const tracerProvider = new BasicTracerProvider({ - gracefulShutdown: false, - }); - const sandbox = sinon.createSandbox(); - const shutdownStub = sandbox.stub( - tracerProvider.getActiveSpanProcessor(), - 'shutdown' - ); - removeEvent = notifyOnGlobalShutdown(() => { - sinon.assert.notCalled(shutdownStub); - }); - _invokeGlobalShutdown(); - }); }); }); diff --git a/packages/opentelemetry-tracing/test/MultiSpanProcessor.test.ts b/packages/opentelemetry-tracing/test/MultiSpanProcessor.test.ts index c0e4c58d7da..d732917d5b8 100644 --- a/packages/opentelemetry-tracing/test/MultiSpanProcessor.test.ts +++ b/packages/opentelemetry-tracing/test/MultiSpanProcessor.test.ts @@ -23,10 +23,6 @@ import { Span, SpanProcessor, } from '../src'; -import { - notifyOnGlobalShutdown, - _invokeGlobalShutdown, -} from '@opentelemetry/core'; import { MultiSpanProcessor } from '../src/MultiSpanProcessor'; class TestProcessor implements SpanProcessor { @@ -99,29 +95,6 @@ describe('MultiSpanProcessor', () => { assert.strictEqual(processor1.spans.length, processor2.spans.length); }); - it('should export spans on graceful shutdown from two span processor', () => { - const processor1 = new TestProcessor(); - const processor2 = new TestProcessor(); - const multiSpanProcessor = new MultiSpanProcessor([processor1, processor2]); - - const tracerProvider = new BasicTracerProvider(); - tracerProvider.addSpanProcessor(multiSpanProcessor); - const tracer = tracerProvider.getTracer('default'); - const span = tracer.startSpan('one'); - assert.strictEqual(processor1.spans.length, 0); - assert.strictEqual(processor1.spans.length, processor2.spans.length); - - span.end(); - assert.strictEqual(processor1.spans.length, 1); - assert.strictEqual(processor1.spans.length, processor2.spans.length); - - removeEvent = notifyOnGlobalShutdown(() => { - assert.strictEqual(processor1.spans.length, 0); - assert.strictEqual(processor1.spans.length, processor2.spans.length); - }); - _invokeGlobalShutdown(); - }); - it('should export spans on manual shutdown from two span processor', () => { const processor1 = new TestProcessor(); const processor2 = new TestProcessor(); @@ -144,29 +117,6 @@ describe('MultiSpanProcessor', () => { }); }); - it('should export spans on graceful shutdown from two span processor', () => { - const processor1 = new TestProcessor(); - const processor2 = new TestProcessor(); - const multiSpanProcessor = new MultiSpanProcessor([processor1, processor2]); - - const tracerProvider = new BasicTracerProvider(); - tracerProvider.addSpanProcessor(multiSpanProcessor); - const tracer = tracerProvider.getTracer('default'); - const span = tracer.startSpan('one'); - assert.strictEqual(processor1.spans.length, 0); - assert.strictEqual(processor1.spans.length, processor2.spans.length); - - span.end(); - assert.strictEqual(processor1.spans.length, 1); - assert.strictEqual(processor1.spans.length, processor2.spans.length); - - removeEvent = notifyOnGlobalShutdown(() => { - assert.strictEqual(processor1.spans.length, 0); - assert.strictEqual(processor1.spans.length, processor2.spans.length); - }); - _invokeGlobalShutdown(); - }); - it('should export spans on manual shutdown from two span processor', () => { const processor1 = new TestProcessor(); const processor2 = new TestProcessor();