From c92bdfdf4bf039d7d79ef0c9fdba392d4be8b99f Mon Sep 17 00:00:00 2001 From: burgerni10 Date: Fri, 16 Sep 2022 10:23:19 +0200 Subject: [PATCH] fix(health-signal): log signal when forwarding with disabled http --- src/client/Engine/HealthSignal.jsx | 4 +- .../Engine/__snapshots__/Engine.spec.jsx.snap | 12 +- .../__snapshots__/HealthSignal.spec.jsx.snap | 6 +- src/engine/HealthSignal.class.js | 53 +++-- src/engine/HealthSignal.class.spec.js | 207 +++++++++++------- src/engine/OIBusEngine.class.js | 4 +- src/server/controllers/engineController.js | 3 +- 7 files changed, 167 insertions(+), 122 deletions(-) diff --git a/src/client/Engine/HealthSignal.jsx b/src/client/Engine/HealthSignal.jsx index a50fade5c7..af967478cd 100644 --- a/src/client/Engine/HealthSignal.jsx +++ b/src/client/Engine/HealthSignal.jsx @@ -82,9 +82,9 @@ const HealthSignal = ({ onChange, healthSignal }) => ( label="Endpoint" name="engine.healthSignal.http.endpoint" value={healthSignal.http.endpoint} - defaultValue="" + defaultValue="/engine/aliveSignal" valid={validation.engine.healthSignal.http.endpoint} - help={
The endpoint send the health signal
} + help={
The endpoint send the health signal.
} onChange={onChange} /> diff --git a/src/client/Engine/__snapshots__/Engine.spec.jsx.snap b/src/client/Engine/__snapshots__/Engine.spec.jsx.snap index 30ed298883..821d621564 100644 --- a/src/client/Engine/__snapshots__/Engine.spec.jsx.snap +++ b/src/client/Engine/__snapshots__/Engine.spec.jsx.snap @@ -3054,7 +3054,7 @@ exports[`Engine check Engine 1`] = ` class="form-text text-muted" >
- The endpoint send the health signal + The endpoint send the health signal.
@@ -6181,7 +6181,7 @@ exports[`Engine check Engine when config has no proxies 1`] = ` class="form-text text-muted" >
- The endpoint send the health signal + The endpoint send the health signal.
@@ -9732,7 +9732,7 @@ exports[`Engine check change engineName 1`] = ` class="form-text text-muted" >
- The endpoint send the health signal + The endpoint send the health signal.
@@ -13278,7 +13278,7 @@ exports[`Engine check change password 1`] = ` class="form-text text-muted" >
- The endpoint send the health signal + The endpoint send the health signal.
@@ -16824,7 +16824,7 @@ exports[`Engine check change port 1`] = ` class="form-text text-muted" >
- The endpoint send the health signal + The endpoint send the health signal.
@@ -20370,7 +20370,7 @@ exports[`Engine check change user 1`] = ` class="form-text text-muted" >
- The endpoint send the health signal + The endpoint send the health signal.
diff --git a/src/client/Engine/__snapshots__/HealthSignal.spec.jsx.snap b/src/client/Engine/__snapshots__/HealthSignal.spec.jsx.snap index 6ac9b6ae31..6edd04f7ed 100644 --- a/src/client/Engine/__snapshots__/HealthSignal.spec.jsx.snap +++ b/src/client/Engine/__snapshots__/HealthSignal.spec.jsx.snap @@ -215,7 +215,7 @@ exports[`HealthSignal check HealthSignal 1`] = ` class="form-text text-muted" >
- The endpoint send the health signal + The endpoint send the health signal.
@@ -617,7 +617,7 @@ exports[`HealthSignal check change healthSignal http to false 1`] = ` class="form-text text-muted" >
- The endpoint send the health signal + The endpoint send the health signal.
@@ -1019,7 +1019,7 @@ exports[`HealthSignal check change healthSignal log to false 1`] = ` class="form-text text-muted" >
- The endpoint send the health signal + The endpoint send the health signal.
diff --git a/src/engine/HealthSignal.class.js b/src/engine/HealthSignal.class.js index 4fb13c1290..6e90e4ea35 100644 --- a/src/engine/HealthSignal.class.js +++ b/src/engine/HealthSignal.class.js @@ -1,5 +1,5 @@ /** - * Class HealthSignal - sends health signal to a remote host + * Class HealthSignal - sends health signal to a remote host or into the logs */ class HealthSignal { /** @@ -18,7 +18,7 @@ class HealthSignal { this.logging = logging this.http = http - this.http.proxy = Array.isArray(engineConfig.proxies) && engineConfig.proxies.find(({ name }) => name === this.http.proxy) + this.http.proxy = Array.isArray(engineConfig.proxies) ? engineConfig.proxies.find(({ name }) => name === this.http.proxy) : null this.httpTimer = null this.loggingTimer = null this.engineName = engineConfig.engineName @@ -30,11 +30,11 @@ class HealthSignal { */ start() { if (this.http.enabled) { - this.logger.info('Initializing http health signal') + this.logger.debug('Initializing HTTP health signal.') this.httpTimer = setInterval(this.sendHttpSignal.bind(this), this.http.frequency * 1000) } if (this.logging.enabled) { - this.logger.info('Initializing logging health signal') + this.logger.debug('Initializing logging health signal.') this.loggingTimer = setInterval(this.sendLoggingSignal.bind(this), this.logging.frequency * 1000) } } @@ -57,9 +57,7 @@ class HealthSignal { * @return {Promise} - The response */ async sendHttpSignal() { - this.logger.trace('sendHttpSignal') - - const healthStatus = await this.prepareStatus(this.http.verbose) + const healthStatus = this.prepareStatus(this.http.verbose) healthStatus.id = this.engineName try { const data = JSON.stringify(healthStatus) @@ -72,9 +70,9 @@ class HealthSignal { data, headers, ) - this.logger.debug('Health signal successful') + this.logger.debug('HTTP health signal sent successfully.') } catch (error) { - this.logger.error(`sendRequest error status: ${error}`) + this.logger.error(error) } } @@ -82,20 +80,18 @@ class HealthSignal { * Callback to send the health signal with logger. * @returns {void} */ - async sendLoggingSignal() { - this.logger.trace('sendHttpSignal') - - const healthStatus = await this.prepareStatus(true) + sendLoggingSignal() { + const healthStatus = this.prepareStatus(true) this.logger.info(JSON.stringify(healthStatus)) } /** * Retrieve status information from the engine - * @param {boolean} verbose - return only the id when false, return full status when true - * @returns {object} - the status of OIBus + * @param {Boolean} verbose - Return only the static OIBus info when false, return full status when true + * @returns {Object} - The status of OIBus */ - async prepareStatus(verbose) { - let status = await this.engine.getOIBusInfo() + prepareStatus(verbose) { + let status = this.engine.getOIBusInfo() if (verbose) { status = { ...status, ...this.engine.statusData } } @@ -103,17 +99,28 @@ class HealthSignal { } /** - * Forward an healthSignal request. - * @param {object} data - The content to forward + * Log and forward an healthSignal request. + * @param {Object} data - The content to forward * @return {Promise} - The response */ async forwardRequest(data) { + const stringData = JSON.stringify(data) if (this.http.enabled) { - this.logger.debug('Forwarding healthSignal request') - const stringData = JSON.stringify(data) + this.logger.trace(`Forwarding health signal to "${this.http.host}".`) const headers = { 'Content-Type': 'application/json' } - await this.engine.requestService.httpSend(this.http.host, 'POST', this.http.authentication, this.http.proxy, stringData, headers) - this.logger.debug('Forwarding healthSignal was successful') + this.logger.info(stringData) + await this.engine.requestService.httpSend( + `${this.http.host}${this.http.endpoint}`, + 'POST', + this.http.authentication, + this.http.proxy, + stringData, + headers, + ) + this.logger.trace(`Health signal successfully forwarded to "${this.http.host}".`) + } else { + this.logger.warn('HTTP health signal is disabled. Cannot forward payload.') + this.logger.info(stringData) } } } diff --git a/src/engine/HealthSignal.class.spec.js b/src/engine/HealthSignal.class.spec.js index 1329d44183..c3ad47983b 100644 --- a/src/engine/HealthSignal.class.spec.js +++ b/src/engine/HealthSignal.class.spec.js @@ -1,102 +1,108 @@ const HealthSignal = require('./HealthSignal.class') // Mock engine -const engine = jest.mock('./OIBusEngine.class') -engine.logger = { error: jest.fn(), debug: jest.fn(), info: jest.fn(), trace: jest.fn() } - -beforeEach(() => { - jest.resetAllMocks() - jest.useFakeTimers() -}) +const engine = { + logger: { error: jest.fn(), warn: jest.fn(), debug: jest.fn(), info: jest.fn(), trace: jest.fn() }, + configService: { getConfig: jest.fn() }, + getOIBusInfo: jest.fn(), + requestService: { httpSend: jest.fn() }, +} + +const proxy = { + name: 'proxy_name', + protocol: 'http', + host: 'proxy.host', + port: 666, + username: 'proxy_user', + password: 'proxy_pass', +} +let healthSignalSettings = null +let healthSignal = null describe('HealthSignal', () => { - const proxy = { - name: 'proxy_name', - protocol: 'http', - host: 'proxy.host', - port: 666, - username: 'proxy_user', - password: 'proxy_pass', - } - - const healthSignalConfig = { - logging: { - enabled: true, - frequency: 3600, - id: 'OIBus', - }, - http: { - enabled: true, - host: 'https://example.com', - endpoint: '/endpoint/info', - authentication: { - type: 'Basic', - username: 'username', - password: 'password', + beforeEach(() => { + jest.resetAllMocks() + jest.useFakeTimers() + + healthSignalSettings = { + logging: { + enabled: true, + frequency: 3600, + id: 'OIBus', }, - frequency: 300, - proxy: 'proxy_name', - }, - } - const engineConfig = { - healthSignal: healthSignalConfig, - proxies: [proxy], - } - engine.configService = { getConfig: () => ({ engineConfig }) } - engine.getOIBusInfo = jest.fn() - engine.requestService = { httpSend: jest.fn() } + http: { + enabled: true, + host: 'https://example.com', + endpoint: '/endpoint/info', + authentication: { + type: 'Basic', + username: 'username', + password: 'password', + }, + frequency: 300, + proxy: 'proxy_name', + }, + } + const engineConfig = { + engineName: 'OIBus', + healthSignal: healthSignalSettings, + proxies: [proxy], + } + engine.configService.getConfig.mockReturnValue({ engineConfig }) + healthSignal = new HealthSignal(engine) + }) it('should be properly initialized', () => { - const healthSignal = new HealthSignal(engine) - - expect(healthSignal.http.enabled).toBe(healthSignalConfig.http.enabled) - expect(healthSignal.http.host).toBe(healthSignalConfig.http.host) - expect(healthSignal.http.endpoint).toBe(healthSignalConfig.http.endpoint) - expect(healthSignal.http.authentication).toBe(healthSignalConfig.http.authentication) - expect(healthSignal.http.id).toBe(healthSignalConfig.http.id) - expect(healthSignal.http.frequency).toBe(healthSignalConfig.http.frequency) + expect(healthSignal.http.enabled).toBe(healthSignalSettings.http.enabled) + expect(healthSignal.http.host).toBe(healthSignalSettings.http.host) + expect(healthSignal.http.endpoint).toBe(healthSignalSettings.http.endpoint) + expect(healthSignal.http.authentication).toBe(healthSignalSettings.http.authentication) + expect(healthSignal.http.id).toBe(healthSignalSettings.http.id) + expect(healthSignal.http.frequency).toBe(healthSignalSettings.http.frequency) expect(healthSignal.http.proxy).toBe(proxy) expect(healthSignal.httpTimer).toBeNull() }) it('should also initialize if proxy is not configured', () => { - delete engineConfig.proxies - const healthSignal = new HealthSignal(engine) - expect(healthSignal.http.enabled).toBe(healthSignalConfig.http.enabled) - expect(healthSignal.http.host).toBe(healthSignalConfig.http.host) - expect(healthSignal.http.endpoint).toBe(healthSignalConfig.http.endpoint) - expect(healthSignal.http.authentication).toBe(healthSignalConfig.http.authentication) - expect(healthSignal.http.id).toBe(healthSignalConfig.http.id) - expect(healthSignal.http.frequency).toBe(healthSignalConfig.http.frequency) - expect(healthSignal.http.proxy).toBe(false) - expect(healthSignal.httpTimer).toBeNull() - // restore - engineConfig.proxies = [proxy] + engine.configService.getConfig.mockReturnValue({ + engineConfig: + { + engineName: 'OIBus', + healthSignal: healthSignalSettings, + }, + }) + + const healthSignalWithoutProxy = new HealthSignal(engine) + expect(healthSignalWithoutProxy.http.enabled).toBe(healthSignalSettings.http.enabled) + expect(healthSignalWithoutProxy.http.host).toBe(healthSignalSettings.http.host) + expect(healthSignalWithoutProxy.http.endpoint).toBe(healthSignalSettings.http.endpoint) + expect(healthSignalWithoutProxy.http.authentication).toBe(healthSignalSettings.http.authentication) + expect(healthSignalWithoutProxy.http.id).toBe(healthSignalSettings.http.id) + expect(healthSignalWithoutProxy.http.frequency).toBe(healthSignalSettings.http.frequency) + expect(healthSignalWithoutProxy.http.proxy).toBe(null) + expect(healthSignalWithoutProxy.httpTimer).toBeNull() }) it('should properly start when http enabled and call callback function', () => { - const healthSignal = new HealthSignal(engine) const callback = jest.spyOn(healthSignal, 'sendHttpSignal').mockImplementation(() => ({ })) healthSignal.http.enabled = true healthSignal.logging.enabled = true healthSignal.start() - jest.advanceTimersByTime(1000 * healthSignalConfig.http.frequency) + jest.advanceTimersByTime(1000 * healthSignalSettings.http.frequency) expect(callback).toHaveBeenCalledTimes(1) }) it('should properly start when disabled', () => { const setTimeoutSpy = jest.spyOn(global, 'setTimeout') - const healthSignal = new HealthSignal(engine) healthSignal.http.enabled = false healthSignal.logging.enabled = false healthSignal.start() - expect(setTimeoutSpy).not.toBeCalled() + expect(setTimeoutSpy).not.toHaveBeenCalled() }) it('should prepare simple status info when verbose is not enabled', async () => { - const healthSignal = new HealthSignal(engine) engine.getOIBusInfo.mockReturnValue({ version: 'version' }) engine.statusData = { randomStatusData: 'test' } @@ -107,7 +113,6 @@ describe('HealthSignal', () => { }) it('should prepare full status info when verbose is enabled', async () => { - const healthSignal = new HealthSignal(engine) engine.getOIBusInfo.mockReturnValue({ status: 'status', version: 'version' }) engine.statusData = { randomStatusData: 'test' } @@ -117,17 +122,14 @@ describe('HealthSignal', () => { expect(status).toEqual({ status: 'status', version: 'version', randomStatusData: 'test' }) }) - it('should call RequestService httpSend()', async () => { + it('should call send http signal', async () => { const status = { status: 'status' } - const healthSignal = new HealthSignal(engine) - healthSignal.prepareStatus = jest.fn() healthSignal.prepareStatus.mockReturnValue(status) await healthSignal.sendHttpSignal() - expect(healthSignal.logger.trace).toBeCalledWith('sendHttpSignal') expect(healthSignal.prepareStatus).toBeCalled() const calledStatus = JSON.stringify({ ...status }) const headers = { 'Content-Type': 'application/json' } @@ -135,31 +137,68 @@ describe('HealthSignal', () => { `${healthSignal.http.host}${healthSignal.http.endpoint}`, 'POST', healthSignal.http.authentication, - healthSignal.http.proxy, + proxy, calledStatus, headers, ) - expect(healthSignal.logger.debug).toBeCalledWith('Health signal successful') + expect(healthSignal.logger.debug).toBeCalledWith('HTTP health signal sent successfully.') + }) + + it('should log error if http signal fails', async () => { + const status = { status: 'status' } + + healthSignal.prepareStatus = jest.fn() + healthSignal.prepareStatus.mockReturnValue(status) + + engine.requestService.httpSend.mockImplementation(() => { + throw new Error('http error') + }) + await healthSignal.sendHttpSignal() + + expect(healthSignal.logger.error).toBeCalledWith(new Error('http error')) }) - it('should properly stop when enabled', () => { + it('should properly stop health signal', () => { const clearIntervalSpy = jest.spyOn(global, 'clearInterval') - const healthSignal = new HealthSignal(engine) - healthSignal.http.enabled = true - healthSignal.start() + + healthSignal.stop() + expect(clearIntervalSpy).not.toHaveBeenCalled() + healthSignal.httpTimer = () => {} + healthSignal.loggingTimer = () => {} healthSignal.stop() - expect(clearIntervalSpy).toHaveBeenCalledTimes(1) + expect(clearIntervalSpy).toHaveBeenCalledTimes(2) }) - it('should properly stop when disabled', () => { - const clearIntervalSpy = jest.spyOn(global, 'clearInterval') - const healthSignal = new HealthSignal(engine) + it('should log health signal', () => { + healthSignal.prepareStatus = jest.fn(() => ({ status: 'status' })) + healthSignal.sendLoggingSignal() + expect(healthSignal.logger.info).toHaveBeenCalledWith(JSON.stringify({ status: 'status' })) + expect(healthSignal.prepareStatus).toHaveBeenCalledTimes(1) + }) + + it('should not forward health signal', async () => { + const data = { status: 'status' } healthSignal.http.enabled = false - healthSignal.start() - healthSignal.stop() + await healthSignal.forwardRequest(data) + expect(healthSignal.logger.warn).toHaveBeenCalledWith('HTTP health signal is disabled. Cannot forward payload.') + expect(healthSignal.logger.info).toHaveBeenCalledWith(JSON.stringify(data)) + }) - expect(clearIntervalSpy).not.toBeCalled() + it('should forward health signal', async () => { + const data = { status: 'status' } + healthSignal.http.enabled = true + await healthSignal.forwardRequest(data) + expect(healthSignal.logger.trace).toHaveBeenCalledWith(`Forwarding health signal to "${healthSignalSettings.http.host}".`) + expect(healthSignal.logger.trace).toHaveBeenCalledWith(`Health signal successfully forwarded to "${healthSignalSettings.http.host}".`) + expect(engine.requestService.httpSend).toHaveBeenCalledWith( + `${healthSignalSettings.http.host}${healthSignalSettings.http.endpoint}`, + 'POST', + healthSignalSettings.http.authentication, + proxy, + JSON.stringify(data), + { 'Content-Type': 'application/json' }, + ) }) }) diff --git a/src/engine/OIBusEngine.class.js b/src/engine/OIBusEngine.class.js index bb4315d374..46efb01e38 100644 --- a/src/engine/OIBusEngine.class.js +++ b/src/engine/OIBusEngine.class.js @@ -423,9 +423,9 @@ class OIBusEngine extends BaseEngine { /** * Get status information. - * @returns {object} - The status information + * @returns {Object} - The status information */ - async getOIBusInfo() { + getOIBusInfo() { return { version: this.getVersion(), architecture: process.arch, diff --git a/src/server/controllers/engineController.js b/src/server/controllers/engineController.js index 9c9670a2c0..019df02ef8 100644 --- a/src/server/controllers/engineController.js +++ b/src/server/controllers/engineController.js @@ -77,8 +77,7 @@ const addFile = async (ctx) => { */ const aliveSignal = async (ctx) => { try { - ctx.app.engine.forwardedAliveSignalMessages += 1 - await ctx.app.engine.aliveSignal.forwardRequest(ctx.request.body) + await ctx.app.engine.healthSignal.forwardRequest(ctx.request.body) ctx.ok() } catch (error) { ctx.throw(500, 'Unable to forward the aliveSignal request')