Merge pull request #1193 from guardian/add-more-logging-to-iam-creden…

…tials-report-code Improve error handling / logging
guardian · Jan 22, 2025 · 5dd17e9 · 5dd17e9
2 parents 15c9da5 + 36d57a2
commit 5dd17e9
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 5 deletions.
diff --git a/hq/app/services/CacheService.scala b/hq/app/services/CacheService.scala
@@ -120,8 +120,7 @@ class CacheService(
         regions
       )
     } yield {
-      logger.info("Sending the refreshed data to the Credentials Box")
-
+      logCacheDataStatus("Credentials", updatedCredentialReports)
       credentialsBox.send(updatedCredentialReports.toMap)
     }
   }
@@ -135,7 +134,7 @@ class CacheService(
         s3Clients
       )
     } yield {
-      logger.info("Sending the refreshed data to the Public Buckets Box")
+      logCacheDataStatus("Public buckets", allPublicBuckets)
       publicBucketsBox.send(allPublicBuckets.toMap)
     }
   }
@@ -148,7 +147,7 @@ class CacheService(
         taClients
       )
     } yield {
-      logger.info("Sending the refreshed data to the Exposed Keys Box")
+      logCacheDataStatus("Exposed Keys", allExposedKeys)
       exposedKeysBox.send(allExposedKeys.toMap)
     }
   }
@@ -183,4 +182,36 @@ class CacheService(
       Future.successful(())
     }
   }
+
+  /**
+   * Prints an overview of this cache data.
+   *
+   * If everything succeeded then we say as much. If the cache data contains failures
+   * we log a warning that shows which accounts are affected and give one failure
+   * as the underlying cause, if available.
+   */
+  def logCacheDataStatus[A](cacheName: String, data: Seq[(AwsAccount, Either[FailedAttempt, A])]): Unit = {
+    val (successful, failed) = data.partition { case (_, result) => result.isRight }
+
+    if (failed.isEmpty) {
+      logger.info(s"$cacheName updated: All ${data.size} accounts successful")
+    } else {
+      val failedAccountsDetails = failed.flatMap {
+        case (account, Left(failedAttempt)) =>
+          Some(s"${account.name}: ${failedAttempt.logMessage}")
+        case _ => None
+      }.mkString(", ")
+      val logMessage = s"$cacheName updated: ${successful.size}/${data.size} accounts succeeded. Failed accounts: $failedAccountsDetails"
+      failed.flatMap {
+        case (_, Left(failedAttempt)) =>
+          failedAttempt.firstException
+        case _ => None
+      }.headOption match {
+        case None =>
+          logger.warn(logMessage)
+        case Some(exampleCausedBy) =>
+          logger.warn(s"$logMessage - see stacktrace for an example cause", exampleCausedBy)
+      }
+    }
+  }
 }
diff --git a/hq/app/utils/attempt/Failure.scala b/hq/app/utils/attempt/Failure.scala
@@ -7,7 +7,8 @@ case class FailedAttempt(failures: List[Failure]) {
 
   def logMessage: String = failures.map { failure =>
     val context = failure.context.fold("")(c => s" ($c)")
-    s"${failure.message}$context"
+    val causedBy = firstException.fold("")(err => s" caused by: ${err.getMessage}")
+    s"${failure.message}$context$causedBy"
   }.mkString(", ")
 
   def firstException: Option[Throwable] = {