fix(logs): use one global logger for settings and children logger for…

… connectors
OptimistikSAS · Nov 28, 2022 · cc720c5 · cc720c5
1 parent f865dec
commit cc720c5
Show file tree

Hide file tree

Showing 80 changed files with 839 additions and 3,298 deletions.
diff --git a/src/engine/base-engine.js b/src/engine/base-engine.js
@@ -26,7 +26,6 @@ southList.Modbus = require('../south/south-modbus/south-modbus')
 southList.OPCHDA = require('../south/south-opchda/south-opchda')
 southList.RestApi = require('../south/south-rest/south-rest')
 
-const LoggerService = require('../service/logger/logger.service')
 const StatusService = require('../service/status.service')
 
 /**
@@ -39,10 +38,16 @@ class BaseEngine {
    * @constructor
    * @param {ConfigurationService} configService - The config service
    * @param {EncryptionService} encryptionService - The encryption service
+   * @param {LoggerService} loggerService - The logger service
    * @param {String} cacheFolder - The base cache folder used by the engine and its connectors
    * @return {void}
    */
-  constructor(configService, encryptionService, cacheFolder) {
+  constructor(
+    configService,
+    encryptionService,
+    loggerService,
+    cacheFolder,
+  ) {
     this.version = VERSION
     this.cacheFolder = path.resolve(cacheFolder)
 
@@ -51,6 +56,7 @@ class BaseEngine {
 
     this.configService = configService
     this.encryptionService = encryptionService
+    this.loggerService = loggerService
 
     // Variable initialized in initEngineServices
     this.statusService = null
@@ -60,18 +66,13 @@ class BaseEngine {
   /**
    * Method used to init async services (like logger when loki is used with Bearer token auth)
    * @param {Object} engineConfig - the config retrieved from the file
-   * @param {String} loggerScope - the scope used in the logger (for example 'OIBusEngine')
    * @returns {Promise<void>} - The result promise
    */
-  async initEngineServices(engineConfig, loggerScope) {
+  async initEngineServices(engineConfig) {
     this.oibusName = engineConfig.name
     this.defaultLogParameters = engineConfig.logParameters
     this.proxies = engineConfig.proxies
     this.statusService = new StatusService()
-    // Configure the logger
-    this.logger = new LoggerService(loggerScope)
-    this.logger.setEncryptionService(this.encryptionService)
-    await this.logger.changeParameters(this.oibusName, this.defaultLogParameters)
   }
 
   /**
@@ -118,13 +119,14 @@ class BaseEngine {
   /**
    * Return the South connector
    * @param {Object} configuration - The South connector configuration
+   * @param {Object} logger - The logger to use
    * @returns {SouthConnector|null} - The South connector
    */
-  createSouth(configuration) {
+  createSouth(configuration, logger) {
     try {
       const SouthConnector = this.installedSouthConnectors[configuration.type]
       if (SouthConnector) {
-        return new SouthConnector(configuration, this.addValues.bind(this), this.addFile.bind(this))
+        return new SouthConnector(configuration, this.addValues.bind(this), this.addFile.bind(this), logger)
       }
       this.logger.error(`South connector for "${configuration.name}" is not found: ${configuration.type}`)
     } catch (error) {
@@ -148,13 +150,14 @@ class BaseEngine {
   /**
    * Return the North connector
    * @param {Object} configuration - The North connector configuration
+   * @param {Object} logger - The logger to use
    * @returns {NorthConnector|null} - The North connector
    */
-  createNorth(configuration) {
+  createNorth(configuration, logger) {
     try {
       const NorthConnector = this.installedNorthConnectors[configuration.type]
       if (NorthConnector) {
-        return new NorthConnector(configuration, this.proxies)
+        return new NorthConnector(configuration, this.proxies, logger)
       }
       this.logger.error(`North connector for "${configuration.name}" is not found: ${configuration.type}`)
     } catch (error) {

diff --git a/src/engine/base-engine.spec.js b/src/engine/base-engine.spec.js
@@ -4,6 +4,15 @@ const ConfigurationService = require('../service/configuration.service')
 
 const { testConfig: config } = require('../../tests/test-config')
 
+// Mock logger
+const logger = {
+  error: jest.fn(),
+  warn: jest.fn(),
+  info: jest.fn(),
+  debug: jest.fn(),
+  trace: jest.fn(),
+}
+
 // Mock services
 jest.mock('../service/configuration.service')
 jest.mock('../service/logger/logger.service')
@@ -21,11 +30,12 @@ describe('BaseEngine', () => {
       engineConfig: config.engine,
       southConfig: config.south,
     })
-
     ConfigurationService.mockImplementation(() => mockConfigService)
 
-    engine = new BaseEngine(mockConfigService, EncryptionService.getInstance(), 'myCacheFolder')
-    await engine.initEngineServices(config.engine, 'base')
+    engine = new BaseEngine(mockConfigService, EncryptionService.getInstance(), {}, 'myCacheFolder')
+    engine.logger = logger
+
+    await engine.initEngineServices(config.engine)
   })
 
   it('should warn when calling add values', async () => {

diff --git a/src/engine/history-query-engine.js b/src/engine/history-query-engine.js
@@ -25,9 +25,15 @@ class HistoryQueryEngine extends BaseEngine {
    * @constructor
    * @param {ConfigurationService} configService - The config service
    * @param {EncryptionService} encryptionService - The encryption service
+   * @param {LoggerService} loggerService - The logger service
    */
-  constructor(configService, encryptionService) {
-    super(configService, encryptionService, CACHE_FOLDER)
+  constructor(configService, encryptionService, loggerService) {
+    super(
+      configService,
+      encryptionService,
+      loggerService,
+      CACHE_FOLDER,
+    )
     this.cacheFolder = path.resolve(CACHE_FOLDER)
 
     this.historyQueryRepository = null
@@ -38,11 +44,12 @@ class HistoryQueryEngine extends BaseEngine {
   /**
    * Method used to init async services (like logger when loki is used with Bearer token auth)
    * @param {Object} engineConfig - the config retrieved from the file
-   * @param {String} loggerScope - the scope used for the logger
    * @returns {Promise<void>} - The result promise
    */
-  async initEngineServices(engineConfig, loggerScope = 'HistoryQueryEngine') {
-    await super.initEngineServices(engineConfig, loggerScope)
+  async initEngineServices(engineConfig) {
+    await super.initEngineServices(engineConfig)
+    this.logger = this.loggerService.createChildLogger('HistoryQueryEngine')
+
     this.statusService.updateStatusDataStream({ ongoingHistoryQueryId: null })
 
     try {
@@ -71,7 +78,7 @@ class HistoryQueryEngine extends BaseEngine {
 
     this.logger.trace(`Add ${values.length} historian values to cache from South "${this.historyQuery.south.name}".`)
     if (values.length) {
-      this.historyQuery.north.cacheValues(southId, values)
+      await this.historyQuery.north.cacheValues(southId, values)
     }
   }
 
@@ -188,7 +195,9 @@ class HistoryQueryEngine extends BaseEngine {
       return
     }
 
-    this.historyQuery = new HistoryQuery(this, historyConfiguration, southToUse, northToUse)
+    const historyLogger = this.loggerService.createChildLogger(`History: ${historyConfiguration.id}`)
+
+    this.historyQuery = new HistoryQuery(this, historyConfiguration, southToUse, northToUse, historyLogger)
     this.statusService.updateStatusDataStream({ ongoingHistoryQueryId: historyConfiguration.id })
     this.logger.info(`Starting history query "${historyConfiguration.id}".`)
     this.historyOnGoing = true

diff --git a/src/engine/history-query-engine.spec.js b/src/engine/history-query-engine.spec.js
@@ -5,12 +5,20 @@ const ConfigurationService = require('../service/configuration.service')
 // Mock fs
 jest.mock('node:fs/promises')
 
+// Mock logger
+const logger = {
+  error: jest.fn(),
+  warn: jest.fn(),
+  info: jest.fn(),
+  debug: jest.fn(),
+  trace: jest.fn(),
+}
+
 // Mock services
 jest.mock('./history-query/history-query-repository')
 jest.mock('../service/database.service')
 jest.mock('../service/configuration.service')
 jest.mock('../service/logger/logger.service')
-jest.mock('../service/encryption.service')
 jest.mock('../service/encryption.service', () => ({ getInstance: () => ({ decryptText: (password) => password }) }))
 
 let engine = null
@@ -27,8 +35,9 @@ describe('HistoryQueryEngine', () => {
     })
 
     ConfigurationService.mockImplementation(() => mockConfigService)
+    const mockLoggerService = { createChildLogger: jest.fn(() => logger) }
 
-    engine = new HistoryQueryEngine(mockConfigService)
+    engine = new HistoryQueryEngine(mockConfigService, {}, mockLoggerService)
   })
 
   it('should be properly initialized', async () => {

diff --git a/src/engine/history-query/history-query.js b/src/engine/history-query/history-query.js
@@ -15,9 +15,9 @@ class HistoryQuery {
   // History query finished
   static STATUS_FINISHED = 'finished'
 
-  constructor(engine, historyConfiguration, southConfiguration, northConfiguration) {
+  constructor(engine, historyConfiguration, southConfiguration, northConfiguration, logger) {
     this.engine = engine
-    this.logger = engine.logger
+    this.logger = logger
     this.historyConfiguration = historyConfiguration
     this.southConfiguration = southConfiguration
     this.south = null
@@ -41,7 +41,7 @@ class HistoryQuery {
     this.statusService = new StatusService()
     this.statusService.updateStatusDataStream({ status: this.historyConfiguration.status })
     await createFolder(this.cacheFolder)
-    this.south = this.engine.createSouth(this.southConfiguration)
+    this.south = this.engine.createSouth(this.southConfiguration, this.logger)
     if (!this.south) {
       this.logger.error(`South connector "${this.southConfiguration.name}" is not found. `
           + `Disabling history query "${this.historyConfiguration.id}".`)
@@ -54,7 +54,7 @@ class HistoryQuery {
       await this.disable()
       return
     }
-    this.north = this.engine.createNorth(this.northConfiguration)
+    this.north = this.engine.createNorth(this.northConfiguration, this.logger)
     if (!this.north) {
       this.logger.error(`North connector "${this.northConfiguration.name}" is not found. `
           + `Disabling history query "${this.historyConfiguration.id}".`)

diff --git a/src/engine/oibus-engine.js b/src/engine/oibus-engine.js
@@ -21,10 +21,16 @@ class OIBusEngine extends BaseEngine {
    * @constructor
    * @param {ConfigurationService} configService - The config service
    * @param {EncryptionService} encryptionService - The encryption service
+   * @param {LoggerService} loggerService - The logger service
    * @return {void}
    */
-  constructor(configService, encryptionService) {
-    super(configService, encryptionService, CACHE_FOLDER)
+  constructor(configService, encryptionService, loggerService) {
+    super(
+      configService,
+      encryptionService,
+      loggerService,
+      CACHE_FOLDER,
+    )
 
     // Will only contain South/North connectors enabled based on the config file
     this.activeSouths = []
@@ -50,11 +56,11 @@ class OIBusEngine extends BaseEngine {
   /**
    * Method used to init async services (like logger when loki is used with Bearer token auth)
    * @param {Object} engineConfig - the config retrieved from the file
-   * @param {String} loggerScope - the scope used for the logger
    * @returns {Promise<void>} - The result promise
    */
-  async initEngineServices(engineConfig, loggerScope = 'OIBusEngine') {
-    await super.initEngineServices(engineConfig, loggerScope)
+  async initEngineServices(engineConfig) {
+    await super.initEngineServices(engineConfig)
+    this.logger = this.loggerService.createChildLogger('OIBusEngine')
     this.logger.info(`Starting OIBusEngine: ${JSON.stringify(this.getOIBusInfo(), null, 4)}`)
 
     engineConfig.scanModes.forEach(({ scanMode }) => {
@@ -80,15 +86,10 @@ class OIBusEngine extends BaseEngine {
    * @returns {Promise<void>} - The result promise
    */
   async addValues(southId, values) {
-    // When coming from an external source, the south won't be found.
-    const southOrigin = this.activeSouths.find((south) => south.id === southId)
-    this.logger.trace(`Add ${values.length} values to cache from South "${southOrigin?.name || southId}".`)
-    if (values.length) {
-      // Do not resolve promise if one of the connector fails. Otherwise, if a file is removed after a North fails,
-      // the file can be lost.
-      await Promise.all(this.activeNorths.filter((north) => north.canHandleValues && north.isSubscribed(southId))
-        .map((north) => north.cacheValues(values)))
-    }
+    // Do not resolve promise if one of the connector fails. Otherwise, if a file is removed after a North fails,
+    // the file can be lost.
+    await Promise.all(this.activeNorths.filter((north) => north.canHandleValues && north.isSubscribed(southId))
+      .map((north) => north.cacheValues(values)))
   }
 
   /**
@@ -100,10 +101,6 @@ class OIBusEngine extends BaseEngine {
    * @returns {Promise<void>} - The result promise
    */
   async addFile(southId, filePath, preserveFiles) {
-    // When coming from an external source, the south won't be found.
-    const southOrigin = this.activeSouths.find((south) => south.id === southId)
-    this.logger.trace(`Add file "${filePath}" to cache from South "${southOrigin?.name || southId}".`)
-
     try {
       // Do not resolve promise if one of the connector fails. Otherwise, if a file is removed after a North fails,
       // the file can be lost.
@@ -145,8 +142,10 @@ class OIBusEngine extends BaseEngine {
     }
 
     // 2. North connectors
-    this.activeNorths = northConfig.filter(({ enabled }) => enabled)
-      .map((northConfiguration) => this.createNorth(northConfiguration))
+    this.activeNorths = northConfig.filter(({ enabled }) => enabled).map((northConfiguration) => {
+      const northLogger = this.loggerService.createChildLogger(`North:${northConfiguration.name}`)
+      return this.createNorth(northConfiguration, northLogger)
+    })
     // Allows init/connect failure of a connector to not block other connectors
     await Promise.allSettled(this.activeNorths.map((north) => {
       const initAndConnect = async () => {
@@ -161,41 +160,41 @@ class OIBusEngine extends BaseEngine {
     }))
 
     // 3. South connectors
-    this.activeSouths = southConfig.filter(({ enabled }) => enabled)
-      .map((southConfiguration) => {
-        const south = this.createSouth(southConfiguration)
-        if (south) {
-          // Associate the scanMode to all corresponding South connectors so the engine will know which South to
-          // activate when a scanMode has a tick.
-          if (southConfiguration.scanMode) {
-            if (!this.scanLists[southConfiguration.scanMode]) {
-              this.logger.error(`South connector ${southConfiguration.name} has an unknown scan mode: ${southConfiguration.scanMode}`)
-            } else if (!this.scanLists[southConfiguration.scanMode].includes(southConfiguration.id)) {
-              // Add the South for this scan only if not already there
-              this.scanLists[southConfiguration.scanMode].push(southConfiguration.id)
-            }
-          } else if (Array.isArray(southConfiguration.points)) {
-            if (southConfiguration.points.length > 0) {
-              southConfiguration.points.forEach((point) => {
-                if (point.scanMode !== 'listen') {
-                  if (!this.scanLists[point.scanMode]) {
-                    this.logger.error(`Point: ${point.pointId} in South connector `
+    this.activeSouths = southConfig.filter(({ enabled }) => enabled).map((southConfiguration) => {
+      const southLogger = this.loggerService.createChildLogger(`South:${southConfiguration.name}`)
+      const south = this.createSouth(southConfiguration, southLogger)
+      if (south) {
+        // Associate the scanMode to all corresponding South connectors so the engine will know which South to
+        // activate when a scanMode has a tick.
+        if (southConfiguration.scanMode) {
+          if (!this.scanLists[southConfiguration.scanMode]) {
+            this.logger.error(`South connector ${southConfiguration.name} has an unknown scan mode: ${southConfiguration.scanMode}`)
+          } else if (!this.scanLists[southConfiguration.scanMode].includes(southConfiguration.id)) {
+            // Add the South for this scan only if not already there
+            this.scanLists[southConfiguration.scanMode].push(southConfiguration.id)
+          }
+        } else if (Array.isArray(southConfiguration.points)) {
+          if (southConfiguration.points.length > 0) {
+            southConfiguration.points.forEach((point) => {
+              if (point.scanMode !== 'listen') {
+                if (!this.scanLists[point.scanMode]) {
+                  this.logger.error(`Point: ${point.pointId} in South connector `
                         + `${southConfiguration.name} has an unknown scan mode: ${point.scanMode}`)
-                  } else if (!this.scanLists[point.scanMode].includes(southConfiguration.id)) {
-                    // Add the South for this scan only if not already there
-                    this.scanLists[point.scanMode].push(southConfiguration.id)
-                  }
+                } else if (!this.scanLists[point.scanMode].includes(southConfiguration.id)) {
+                  // Add the South for this scan only if not already there
+                  this.scanLists[point.scanMode].push(southConfiguration.id)
                 }
-              })
-            } else {
-              this.logger.warn(`South "${southConfiguration.name}" has no point.`)
-            }
+              }
+            })
           } else {
-            this.logger.error(`South "${southConfiguration.name}" has no scan mode defined.`)
+            this.logger.warn(`South "${southConfiguration.name}" has no point.`)
           }
+        } else {
+          this.logger.error(`South "${southConfiguration.name}" has no scan mode defined.`)
         }
-        return south
-      })
+      }
+      return south
+    })
     this.logger.debug(JSON.stringify(this.scanLists, null, ' '))
     // Allows init/connect failure of a connector to not block other connectors
     await Promise.allSettled(this.activeSouths.map((south) => {