From a11602cd7119fd779a54e314fb6bcd7fe78c7606 Mon Sep 17 00:00:00 2001
From: Max Duval <max.duval@theguardian.com>
Date: Wed, 3 Apr 2024 12:44:56 +0100
Subject: [PATCH] Add more logging to Deeply Read & reduce bugs (#27011)

* refactor(DeeplyRead): add recovery and logging

We recently stopped receiving data for Deeply Read,
but our current logging was insufficient to correctly
identify the source of the error, so we add more logging
to help future developers investigate where things may
have gone awry.

We also may have fixed a bug by flattening, rather than getting,
where Seq[Option[Trail]] was turned into Seq[Trail]. It is possible
that previously any issue in querying CAPI for any of the trail would
result in the entire Future to fail.

* chore(format): apply scalafmt

---------

Co-authored-by: Alina Boghiu <alina.boghiu@gmail.com>
---
 common/app/agents/DeeplyReadAgent.scala  | 76 ++++++++++++++----------
 facia/app/feed/DeeplyReadLifecycle.scala |  9 ++-
 2 files changed, 52 insertions(+), 33 deletions(-)

diff --git a/common/app/agents/DeeplyReadAgent.scala b/common/app/agents/DeeplyReadAgent.scala
index fd1e3b491848..30f4c4a03595 100644
--- a/common/app/agents/DeeplyReadAgent.scala
+++ b/common/app/agents/DeeplyReadAgent.scala
@@ -33,40 +33,54 @@ class DeeplyReadAgent(contentApiClient: ContentApiClient, ophanApi: OphanApi) ex
      */
     Future
       .sequence(Edition.allEditions.map { edition =>
-        ophanApi.getDeeplyRead(edition).flatMap {
-          ophanDeeplyReadItems =>
-            log.info(s"ophanItems updated with: ${ophanDeeplyReadItems.size} new items")
-            val constructedTrail: Seq[Future[Trail]] = ophanDeeplyReadItems.map {
-              ophanItem =>
-                log.info(s"CAPI lookup for Ophan deeply read item: ${ophanItem.toString}")
-                val path = removeStartingSlash(ophanItem.path)
-                log.info(s"CAPI Lookup for path: $path")
-                val capiRequest = contentApiClient
-                  .item(path)
-                  .showTags("all")
-                  .showFields("all")
-                  .showReferences("none")
-                  .showAtoms("none")
-                val trailFromCapiResponse = contentApiClient
-                  .getResponse(capiRequest)
-                  .map { res =>
-                    res.content.flatMap { capiData =>
-                      log.info(s"Retrieved CAPI data for Deeply Read item: ${path}")
-                      deeplyReadUrlToTrail(capiData)
+        ophanApi
+          .getDeeplyRead(edition)
+          .flatMap {
+            ophanDeeplyReadItems =>
+              log.info(s"Fetched ${ophanDeeplyReadItems.size} Deeply Read items for ${edition.displayName}")
+              val constructedTrail: Seq[Future[Option[Trail]]] = ophanDeeplyReadItems.map {
+                ophanItem =>
+                  log.info(s"CAPI lookup for Ophan deeply read item: ${ophanItem.toString}")
+                  val path = removeStartingSlash(ophanItem.path)
+                  log.info(s"CAPI Lookup for path: $path")
+                  val capiRequest = contentApiClient
+                    .item(path)
+                    .showTags("all")
+                    .showFields("all")
+                    .showReferences("none")
+                    .showAtoms("none")
+
+                  contentApiClient
+                    .getResponse(capiRequest)
+                    .map { res =>
+                      res.content.flatMap { capiData =>
+                        log.info(s"Retrieved CAPI data for Deeply Read item: ${path}")
+                        deeplyReadUrlToTrail(capiData)
+                      }
+                    }
+                    .recover {
+                      case NonFatal(e) =>
+                        log.error(s"Error retrieving CAPI data for Deeply Read item: ${path}. ${e.getMessage}")
+                        None
                     }
-                  }
-                  .recover {
-                    case NonFatal(e) =>
-                      log.error(s"Error retrieving CAPI data for Deeply Read item: ${path}. ${e.getMessage}")
-                      None
-                  }
-                trailFromCapiResponse.map(_.get)
-            }
-            Future.sequence(constructedTrail)
-        }
+              }
+              Future
+                .sequence(constructedTrail)
+                .map { maybeTrails =>
+                  (edition, maybeTrails.flatten)
+                }
+
+          }
+          .recover { e =>
+            log.error(s"Failed to fetch Deeply Read items for ${edition.displayName}. ${e.getMessage()}")
+            (edition, Seq.empty)
+          }
       })
       .map(trailsList => {
-        val map = Edition.allEditions.zip(trailsList).toMap
+        val map = trailsList.toMap
+        for {
+          (edition, list) <- map
+        } yield log.info(s"Deeply Read in ${edition.displayName}: ${list.map(_.url).toString()}")
         deeplyReadItems.alter(map)
       })
   }
diff --git a/facia/app/feed/DeeplyReadLifecycle.scala b/facia/app/feed/DeeplyReadLifecycle.scala
index c6ff9468b5fb..e1ea30c0b696 100644
--- a/facia/app/feed/DeeplyReadLifecycle.scala
+++ b/facia/app/feed/DeeplyReadLifecycle.scala
@@ -7,13 +7,15 @@ import play.api.inject.ApplicationLifecycle
 
 import java.util.concurrent.Executors
 import scala.concurrent.{ExecutionContext, ExecutionContextExecutorService, Future}
+import common.GuLogging
 
 class DeeplyReadLifecycle(
     appLifecycle: ApplicationLifecycle,
     jobs: JobScheduler,
     pekkoAsync: PekkoAsync,
     deeplyReadAgent: DeeplyReadAgent,
-) extends LifecycleComponent {
+) extends LifecycleComponent
+    with GuLogging {
 
   implicit val executionContext: ExecutionContextExecutorService =
     ExecutionContext.fromExecutorService(Executors.newSingleThreadExecutor())
@@ -33,7 +35,10 @@ class DeeplyReadLifecycle(
     descheduleAll()
 
     jobs.scheduleEveryNMinutes("DeeplyReadAgentsHighFrequencyRefreshJob", 5) {
-      deeplyReadAgent.refresh()
+      deeplyReadAgent.refresh().recover {
+        case e => log.error(s"Failed to refresh with, ${e.getMessage()}")
+      }
+
     }
 
     pekkoAsync.after1s {