From 6961f4148843693963c0a90907a7f6844cdf9419 Mon Sep 17 00:00:00 2001
From: Christopher Hoskin <christopher.hoskin@overleaf.com>
Date: Mon, 20 Jul 2020 16:57:43 +0100
Subject: [PATCH] Promisfy log level checks

---
 libraries/logger/logging-manager.js           | 61 ++++++++++---------
 .../logger/test/unit/loggingManagerTests.js   | 20 +++---
 2 files changed, 44 insertions(+), 37 deletions(-)

diff --git a/libraries/logger/logging-manager.js b/libraries/logger/logging-manager.js
index 7f61360d5f..16c0154955 100644
--- a/libraries/logger/logging-manager.js
+++ b/libraries/logger/logging-manager.js
@@ -22,6 +22,7 @@ const errSerializer = function (err) {
 
 const Logger = (module.exports = {
   initialize(name) {
+    this.useMetadata = (process.env.USE_METADATA || '').toLowerCase() === 'true'
     this.isProduction =
       (process.env.NODE_ENV || '').toLowerCase() === 'production'
     this.defaultLevel =
@@ -42,52 +43,54 @@ const Logger = (module.exports = {
     return this
   },
 
-  checkLogLevelFile() {
-    fs.readFile('/logging/tracingEndTime', (error, end) => {
-      if (error || !end) {
-        this.logger.level(this.defaultLevel)
-        return
-      }
-      if (parseInt(end) > Date.now()) {
+  async checkLogLevelFile() {
+    try {
+      const end = await fs.promises.readFile('/logging/tracingEndTime')
+      if (!end) throw new Error("No end time found")
+      if (parseInt(end,10) > Date.now()) {
         this.logger.level('trace')
       } else {
         this.logger.level(this.defaultLevel)
       }
-    })
+    } catch (err) {
+      this.logger.level(this.defaultLevel)
+      return
+    }
   },
 
-  checkLogLevelMetadata() {
+async checkLogLevelMetadata() {
     const options = {
       headers: {
         'Metadata-Flavor': 'Google'
       }
     }
     const uri = `http://metadata.google.internal/computeMetadata/v1/project/attributes/${this.loggerName}-setLogLevelEndTime`
-    fetch.fetch(uri,options).then(res => res.text()).then(body => {
-      console.log("About to parse Int", body)
-      if (parseInt(body) > Date.now()) {
-        console.log("About to set logger level to trace")
-        console.log(this.logger)
-        this.logger.level('trace')
-      } else {
-        console.log("About to set logger level to default")
-        this.logger.level(this.defaultLevel)
-      }
-    }).catch(err => {
+    try {
+      const res = await fetch(uri,options)
+      if (!res.ok) throw new Error("Metadata not okay")
+        const body = await res.text()
+        console.log("About to parse Int", body)
+        if (parseInt(body) > Date.now()) {
+          console.log("About to set logger level to trace")
+          console.log(this.logger)
+          this.logger.level('trace')
+        } else {
+          console.log("About to set logger level to default")
+          this.logger.level(this.defaultLevel)
+        }
+    } catch (err) {
       console.log("ERROR: About to set logger level to default")
       this.logger.level(this.defaultLevel)
       return
-    })
+    }
   },
 
-  checkLogLevel() {
-    fs.access('/logging', (err) => {
-      if (err) {
-        this.checkLogLevelMetadata()
-      } else {
-        this.checkLogLevelFile()
-      }
-    })
+  async checkLogLevel() {
+    if (this.useMetadata) {
+      await this.checkLogLevelMetadata()
+    } else {
+      await this.checkLogLevelFile()
+    }
   },
 
   initializeErrorReporting(sentryDsn, options) {
diff --git a/libraries/logger/test/unit/loggingManagerTests.js b/libraries/logger/test/unit/loggingManagerTests.js
index d822432028..505928a9cb 100644
--- a/libraries/logger/test/unit/loggingManagerTests.js
+++ b/libraries/logger/test/unit/loggingManagerTests.js
@@ -45,9 +45,7 @@ describe('LoggingManager', function () {
     this.Raven = {
       Client: sinon.stub().returns(this.ravenClient)
     }
-    this.Fetch = {
-      fetch: sinon.stub().resolves(this.fetchResponse)
-    }
+    this.Fetch = sinon.stub().resolves(this.fetchResponse)
     this.Fs = {
       readFile: sinon.stub(),
       access: sinon.stub()
@@ -381,9 +379,14 @@ describe('LoggingManager', function () {
         })
       })
 
-      describe('when /logging does not exist', function () {
+      describe('when not running in GKE', function () {
         beforeEach(function () {
-          this.Fs.access.yields(new Error)
+          process.env.USE_METADATA = 'TRUE'
+          this.logger = this.LoggingManager.initialize(this.loggerName)
+        })
+
+        afterEach(function() {
+          process.env.USE_METADATA = undefined
         })
 
         describe('checkLogLevel', function() {
@@ -439,13 +442,14 @@ describe('LoggingManager', function () {
       
           describe('when time value returned that is more than current time', function() {
             describe('when level is already set', function() {
-              beforeEach(function() {
+              beforeEach(async function() {
                 this.bunyanLogger.level.returns(10)
                 //this.Request.yields(null, { statusCode: 200 }, this.start + 1000)
                 console.log("In test ", this.start + 1000)
                 this.fetchResponse.text = sinon.stub().resolves(this.start + 1000)
-                this.Fetch.fetch = sinon.stub().resolves(this.fetchResponse)
-                this.logger.checkLogLevel()
+                //this.Fetch = sinon.stub().resolves(this.fetchResponse)
+
+                await this.logger.checkLogLevel()
               })
       
               it.only('should set trace level', function() {