feat: standardize logger format and output (#211)

* feat: update application logger to enforce a logging format * feat: update all logger calls to the new enforced signature * feat: add stack trace into prod logs * fix: remove wrongly negated check for logging * feat(logger): add greater clarity to what errorHunter does * refactor(request): remove redundant req.get check * test(AdminFormController): add dummy req.get function not used in the test, but getRequestIp for logging uses it * Merge branch 'develop' into feat/standardize-logger # Conflicts: # src/app/controllers/authentication.server.controller.js # src/app/services/webhooks.service.ts
opengovsg · Aug 27, 2020 · 525c1bb · 525c1bb
1 parent 4647f12
commit 525c1bb
Show file tree

Hide file tree

Showing 33 changed files with 1,214 additions and 445 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -150,6 +150,7 @@
     "sortablejs": "^1.8.4",
     "text-encoding": "^0.7.0",
     "toastr": "^2.1.4",
+    "triple-beam": "^1.3.0",
     "tweetnacl": "^1.0.1",
     "twilio": "^3.33.1",
     "ui-select": "^0.19.8",
@@ -182,6 +183,7 @@
     "@types/nodemailer": "^6.4.0",
     "@types/nodemailer-direct-transport": "^1.0.31",
     "@types/puppeteer-core": "^2.0.0",
+    "@types/triple-beam": "^1.3.2",
     "@types/uid-generator": "^2.0.2",
     "@types/uuid": "^8.0.0",
     "@types/validator": "^13.0.0",

diff --git a/src/app/controllers/admin-console.server.controller.js b/src/app/controllers/admin-console.server.controller.js
@@ -19,9 +19,7 @@ const FormStatisticsTotal = getFormStatisticsTotalModel(mongoose)
 const Form = getFormModel(mongoose)
 const _ = require('lodash')
 
-const logger = require('../../config/logger').createLoggerWithLabel(
-  'admin-console',
-)
+const logger = require('../../config/logger').createLoggerWithLabel(module)
 const { getRequestIp } = require('../utils/request')
 
 // Examples search-specific constants
@@ -572,8 +570,18 @@ exports.getExampleFormsUsingAggregateCollection = function (req, res) {
     lookupFormStatisticsInfo,
     projectSubmissionInfo,
     req.query,
-    (err, status, result) => {
-      if (err) logger.error(getRequestIp(req), req.url, req.headers, err)
+    (error, status, result) => {
+      if (error)
+        logger.error({
+          message: 'Failed to retrieve example forms',
+          meta: {
+            action: 'getExampleFormsUsingAggregateCollection',
+            ip: getRequestIp(req),
+            url: req.url,
+            headers: req.headers,
+          },
+          error,
+        })
       return res.status(status).send(result)
     },
   )
@@ -594,8 +602,19 @@ exports.getExampleFormsUsingSubmissionsCollection = function (req, res) {
     lookupSubmissionInfo,
     groupSubmissionsByFormId,
     req.query,
-    (err, status, result) => {
-      if (err) logger.error(getRequestIp(req), req.url, req.headers, err)
+    (error, status, result) => {
+      if (error) {
+        logger.error({
+          message: 'Failed to retrieve example forms',
+          meta: {
+            action: 'getExampleFormsUsingSubmissionsCollection',
+            ip: getRequestIp(req),
+            url: req.url,
+            headers: req.headers,
+          },
+          error,
+        })
+      }
       return res.status(status).send(result)
     },
   )
@@ -673,8 +692,19 @@ exports.getSingleExampleFormUsingSubmissionCollection = function (req, res) {
   getSingleExampleFormUsing(
     req.params.formId,
     createFormIdSubmissionQuery,
-    (err, status, result) => {
-      if (err) logger.error(getRequestIp(req), req.url, req.headers, err)
+    (error, status, result) => {
+      if (error) {
+        logger.error({
+          message: 'Failed to retrieve a single example form',
+          meta: {
+            action: 'getSingleExampleFormUsingSubmissionCollection',
+            ip: getRequestIp(req),
+            url: req.url,
+            headers: req.headers,
+          },
+          error,
+        })
+      }
       return res.status(status).send(result)
     },
   )
@@ -691,7 +721,18 @@ exports.getSingleExampleFormUsingAggregateCollection = function (req, res) {
     req.params.formId,
     createFormIdStatsQuery,
     (err, status, result) => {
-      if (err) logger.error(getRequestIp(req), req.url, req.headers, err)
+      if (err) {
+        logger.error({
+          message: 'Failed to retrieve single example form',
+          meta: {
+            action: 'getSingleExampleFormUsingAggregateCollection',
+            ip: getRequestIp(req),
+            url: req.url,
+            headers: req.headers,
+          },
+          error: err,
+        })
+      }
       return res.status(status).send(result)
     },
   )
@@ -760,21 +801,33 @@ exports.getLoginStats = function (req, res) {
         },
       },
     ],
-    function (err, loginStats) {
-      if (err) {
-        logger.error(getRequestIp(req), req.url, req.headers, err)
+    function (error, loginStats) {
+      if (error) {
+        logger.error({
+          message: 'Failed to retrieve billing records',
+          meta: {
+            action: 'getLoginStats',
+            ip: getRequestIp(req),
+            url: req.url,
+            headers: req.headers,
+          },
+          error,
+        })
         return res
           .status(HttpStatus.INTERNAL_SERVER_ERROR)
           .send('Error in retrieving billing records')
       } else if (!loginStats) {
         return res.status(HttpStatus.NOT_FOUND).send('No billing records found')
       } else {
-        logger.info(
-          'Billing search for',
-          esrvcId,
-          'by',
-          req.session.user && req.session.user.email,
-        )
+        logger.info({
+          message: `Billing search for ${esrvcId} by ${
+            req.session.user && req.session.user.email
+          }`,
+          meta: {
+            action: 'getLoginStats',
+          },
+        })
+
         return res.send({
           loginStats,
         })