Skip to content

Commit

Permalink
Improve error handling / logging
Browse files Browse the repository at this point in the history
Log messages on Failures now include a summary of the underlying
cause, if present. This should improve error visibility across the
board in Security HQ.

We also add additional error handling to the cache service. On every
cache update we include a summary of which accounts have failures in
their cache data.
  • Loading branch information
adamnfish committed Jan 22, 2025
1 parent f2c5dfc commit a459616
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 5 deletions.
39 changes: 35 additions & 4 deletions hq/app/services/CacheService.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand All @@ -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)
}
}
Expand All @@ -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)
}
}
Expand Down Expand Up @@ -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)
}
}
}
}
3 changes: 2 additions & 1 deletion hq/app/utils/attempt/Failure.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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] = {
Expand Down

0 comments on commit a459616

Please sign in to comment.