From dace4fdd7135a3832b45dd7a06067ea14dcc8fe1 Mon Sep 17 00:00:00 2001
From: Jeff Ching <chingor@google.com>
Date: Fri, 4 Nov 2022 12:02:23 -0700
Subject: [PATCH 1/3] fix: attach PR information as context to logs

---
 packages/merge-on-green/src/merge-on-green.ts | 22 ++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/packages/merge-on-green/src/merge-on-green.ts b/packages/merge-on-green/src/merge-on-green.ts
index 2f6fb3043dd..e44a0305db6 100644
--- a/packages/merge-on-green/src/merge-on-green.ts
+++ b/packages/merge-on-green/src/merge-on-green.ts
@@ -433,7 +433,19 @@ handler.checkPRMergeability = async function checkPRMergeability(
     const work = watchedPRs.splice(0, WORKER_SIZE);
     await Promise.all(
       work.map(async wp => {
-        logger.info(`checking ${wp.url}, ${wp.installationId}`);
+        // deep copy base logger bindings
+        const bindings = JSON.parse(JSON.stringify(logger.getBindings()));
+        if (!bindings.trigger) {
+          bindings.trigger = {};
+        }
+        bindings.trigger.trigger_source_repo = {
+          owner: wp.owner,
+          repo_name: wp.repo,
+          url: `https://github.com/${wp.owner}/${wp.repo}`,
+        };
+        bindings.pull_request_url = wp.url;
+        const prLogger = logger.child(bindings);
+        prLogger.info(`checking ${wp.url}, ${wp.installationId}`);
         try {
           const remove = await mergeOnGreen(
             wp.owner,
@@ -445,10 +457,10 @@ handler.checkPRMergeability = async function checkPRMergeability(
             wp.label,
             wp.author,
             octokit,
-            logger
+            prLogger
           );
           if (remove || wp.state === 'stop') {
-            await handler.removePR(wp.url, logger);
+            await handler.removePR(wp.url, prLogger);
             try {
               await handler.cleanUpPullRequest(
                 wp.owner,
@@ -459,7 +471,7 @@ handler.checkPRMergeability = async function checkPRMergeability(
                 octokit
               );
             } catch (err) {
-              logger.warn(
+              prLogger.warn(
                 `Failed to delete reaction and label on ${wp.owner}/${wp.repo}/${wp.number}`
               );
             }
@@ -467,7 +479,7 @@ handler.checkPRMergeability = async function checkPRMergeability(
         } catch (e) {
           const err = e as Error;
           err.message = `Error in merge-on-green: \n\n${err.message}`;
-          logger.error(err);
+          prLogger.error(err);
         }
       })
     );

From 9f56f5359613e75037a090d315a6b9fe2391e9a4 Mon Sep 17 00:00:00 2001
From: Jeff Ching <chingor@google.com>
Date: Fri, 4 Nov 2022 12:06:59 -0700
Subject: [PATCH 2/3] fix: extract logic to add context to a new function

---
 packages/merge-on-green/src/merge-on-green.ts | 34 ++++++++++++-------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/packages/merge-on-green/src/merge-on-green.ts b/packages/merge-on-green/src/merge-on-green.ts
index e44a0305db6..5e7b8f37d92 100644
--- a/packages/merge-on-green/src/merge-on-green.ts
+++ b/packages/merge-on-green/src/merge-on-green.ts
@@ -417,6 +417,27 @@ handler.cleanDatastoreTable = async function cleanDatastoreTable(
   }
 };
 
+/**
+ * Returns a new child logger with added PR contexst.
+ * @param {GCFLogger} logger The base logger
+ * @param {DatastorePR} watchedPR The pull request
+ * @returns {GCFLogger} New logger with added context
+ */
+function addPullRequestLoggerContext(logger: GCFLogger, watchedPR: DatastorePR): GCFLogger {
+  // deep copy base logger bindings
+  const bindings = JSON.parse(JSON.stringify(logger.getBindings()));
+  if (!bindings.trigger) {
+    bindings.trigger = {};
+  }
+  bindings.trigger.trigger_source_repo = {
+    owner: watchedPR.owner,
+    repo_name: watchedPR.repo,
+    url: `https://github.com/${watchedPR.owner}/${watchedPR.repo}`,
+  };
+  bindings.pull_request_url = watchedPR.url;
+  return logger.child(bindings);
+}
+
 /**
  * Calls the main MOG logic, either deletes or keeps that PR in the Datastore table
  * @param watchedPRs array of watched PRs
@@ -433,18 +454,7 @@ handler.checkPRMergeability = async function checkPRMergeability(
     const work = watchedPRs.splice(0, WORKER_SIZE);
     await Promise.all(
       work.map(async wp => {
-        // deep copy base logger bindings
-        const bindings = JSON.parse(JSON.stringify(logger.getBindings()));
-        if (!bindings.trigger) {
-          bindings.trigger = {};
-        }
-        bindings.trigger.trigger_source_repo = {
-          owner: wp.owner,
-          repo_name: wp.repo,
-          url: `https://github.com/${wp.owner}/${wp.repo}`,
-        };
-        bindings.pull_request_url = wp.url;
-        const prLogger = logger.child(bindings);
+        const prLogger = addPullRequestLoggerContext(logger, wp);
         prLogger.info(`checking ${wp.url}, ${wp.installationId}`);
         try {
           const remove = await mergeOnGreen(

From fb1bf87696058fe24d6df044cd286a93947a889d Mon Sep 17 00:00:00 2001
From: Jeff Ching <chingor@google.com>
Date: Fri, 4 Nov 2022 12:09:28 -0700
Subject: [PATCH 3/3] fix lint

---
 packages/merge-on-green/src/merge-on-green.ts | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/packages/merge-on-green/src/merge-on-green.ts b/packages/merge-on-green/src/merge-on-green.ts
index 5e7b8f37d92..7dd1dcbda16 100644
--- a/packages/merge-on-green/src/merge-on-green.ts
+++ b/packages/merge-on-green/src/merge-on-green.ts
@@ -423,7 +423,10 @@ handler.cleanDatastoreTable = async function cleanDatastoreTable(
  * @param {DatastorePR} watchedPR The pull request
  * @returns {GCFLogger} New logger with added context
  */
-function addPullRequestLoggerContext(logger: GCFLogger, watchedPR: DatastorePR): GCFLogger {
+function addPullRequestLoggerContext(
+  logger: GCFLogger,
+  watchedPR: DatastorePR
+): GCFLogger {
   // deep copy base logger bindings
   const bindings = JSON.parse(JSON.stringify(logger.getBindings()));
   if (!bindings.trigger) {