Merge pull request guardian#3554 from guardian/group-migration-failures

introduce new grouped view of migration failures (aka 'overview')

thrall/app/controllers/ThrallController.scala

@@ -58,7 +58,21 @@ class ThrallController(
     }
   }
 
-  def migrationFailures(maybePage: Option[Int]): Action[AnyContent] = withLoginRedirectAsync {
+  def migrationFailuresOverview(): Action[AnyContent] = withLoginRedirectAsync {
+    es.migrationStatus match {
+      case running: Running =>
+        es.getMigrationFailuresOverview(es.imagesCurrentAlias, running.migrationIndexName).map(failuresOverview =>
+          Ok(views.html.migrationFailuresOverview(
+            failuresOverview,
+            apiBaseUrl = services.apiBaseUri,
+            uiBaseUrl = services.kahunaBaseUri,
+          ))
+        )
+      case _ => Future.successful(Ok("No current migration"))
+    }
+  }
+
+  def migrationFailures(filter: String, maybePage: Option[Int]): Action[AnyContent] = withLoginRedirectAsync {
     val pageSize = 250
     // pages are indexed from 1
     val page = maybePage.getOrElse(1)
@@ -68,12 +82,13 @@ class ThrallController(
     } else {
       es.migrationStatus match {
         case running: Running =>
-          es.getMigrationFailures(es.imagesCurrentAlias, running.migrationIndexName, from, pageSize).map(failures =>
+          es.getMigrationFailures(es.imagesCurrentAlias, running.migrationIndexName, from, pageSize, filter).map(failures =>
             Ok(views.html.migrationFailures(
+              failures,
               apiBaseUrl = services.apiBaseUri,
               uiBaseUrl = services.kahunaBaseUri,
-              page = page,
-              failures = failures
+              filter,
+              page
             ))
           )
         case _ => Future.successful(Ok("No current migration"))

thrall/app/lib/FailedMigrationDetails.scala

@@ -1,5 +1,9 @@
 package lib
 
-final case class FailedMigrationDetails(imageId: String, cause: String)
+final case class FailedMigrationDetails(imageId: String, lastModified: String, crops: String, usages: String)
 
-final case class FailedMigrationSummary(totalFailed: Long, totalFailedRelation: String, returned: Long, details: Seq[FailedMigrationDetails])
+final case class FailedMigrationSummary(totalFailed: Long, details: Seq[FailedMigrationDetails])
+
+final case class FailedMigrationsGrouping(message: String, count: Long, exampleIDs: Seq[String])
+
+final case class FailedMigrationsOverview(totalFailed: Long, grouped: Seq[FailedMigrationsGrouping])

thrall/app/lib/elasticsearch/ThrallMigrationClient.scala

@@ -2,11 +2,14 @@ package lib.elasticsearch
 
 import com.gu.mediaservice.lib.elasticsearch.{ElasticSearchClient, InProgress, MigrationAlreadyRunningError, MigrationStatusProvider, NotRunning, Paused}
 import com.gu.mediaservice.lib.logging.{LogMarker, MarkerMap}
+import com.gu.mediaservice.model.Image
 import com.sksamuel.elastic4s.ElasticApi.{existsQuery, matchQuery, not}
 import com.sksamuel.elastic4s.ElasticDsl
 import com.sksamuel.elastic4s.ElasticDsl._
 import com.sksamuel.elastic4s.requests.searches.SearchHit
-import lib.{FailedMigrationDetails, FailedMigrationSummary}
+import com.sksamuel.elastic4s.requests.searches.aggs.responses.bucket.Terms
+import com.sksamuel.elastic4s.requests.searches.aggs.responses.metrics.TopHits
+import lib.{FailedMigrationDetails, FailedMigrationSummary, FailedMigrationsGrouping, FailedMigrationsOverview}
 import play.api.libs.json.{JsError, JsSuccess, Json, Reads, __}
 
 import scala.concurrent.{ExecutionContext, Future}
@@ -81,34 +84,64 @@ trait ThrallMigrationClient extends MigrationStatusProvider {
     } yield ()
   }
 
+  def getMigrationFailuresOverview(
+    currentIndexName: String, migrationIndexName: String
+  )(implicit ec: ExecutionContext, logMarker: LogMarker = MarkerMap()): Future[FailedMigrationsOverview] = {
+
+    val examplesSubAggregation = topHitsAgg("examples")
+      .fetchSource(false)
+      .size(3)
+
+    val aggregateOnFailureMessage =
+      termsAgg("failures", s"esInfo.migration.failures.$migrationIndexName.keyword")
+        .size(1000)
+        .subAggregations(examplesSubAggregation)
+
+    val aggSearch = ElasticDsl.search(currentIndexName).trackTotalHits(true) query must(
+      existsQuery(s"esInfo.migration.failures.$migrationIndexName"),
+      not(matchQuery("esInfo.migration.migratedTo", migrationIndexName))
+    ) aggregations aggregateOnFailureMessage
+
+    executeAndLog(aggSearch, s"retrieving grouped overview of migration failures").map { response =>
+      FailedMigrationsOverview(
+        totalFailed = response.result.hits.total.value,
+        grouped = response.result.aggregations.result[Terms](aggregateOnFailureMessage.name).buckets.map { bucket  =>
+          FailedMigrationsGrouping(
+            message = bucket.key,
+            count = bucket.docCount,
+            exampleIDs = bucket.result[TopHits](examplesSubAggregation.name).hits.map(_.id)
+          )
+        }
+      )
+    }
+  }
+
   def getMigrationFailures(
-    currentIndexName: String, migrationIndexName: String, from: Int, pageSize: Int
+    currentIndexName: String, migrationIndexName: String, from: Int, pageSize: Int, filter: String
   )(implicit ec: ExecutionContext, logMarker: LogMarker = MarkerMap()): Future[FailedMigrationSummary] = {
-    val search = ElasticDsl.search(currentIndexName).from(from).size(pageSize) query must(
+    val search = ElasticDsl.search(currentIndexName).trackTotalHits(true).from(from).size(pageSize) query must(
       existsQuery(s"esInfo.migration.failures.$migrationIndexName"),
+      termQuery(s"esInfo.migration.failures.$migrationIndexName.keyword", filter),
       not(matchQuery("esInfo.migration.migratedTo", migrationIndexName))
-    )
+    ) sortByFieldDesc "lastModified"
     executeAndLog(search, s"retrieving list of migration failures")
       .map { resp =>
-        logger.info(logMarker, s"failed migrations - got ${resp.result.hits.size} hits")
         val failedMigrationDetails: Seq[FailedMigrationDetails] = resp.result.hits.hits.map { hit =>
-            logger.info(logMarker, s"failed migrations - got hit $hit.id")
-            val source = hit.sourceAsString
-            val cause = Json.parse(source).validate(Json.reads[EsInfoContainer]) match {
-              case JsSuccess(EsInfoContainer(EsInfo(Some(MigrationInfo(Some(failures), _)))), _) =>
-                failures.getOrElse(migrationIndexName, "UNKNOWN - NO FAILURE MATCHING MIGRATION INDEX NAME")
-              case JsError(errors) =>
-                logger.error(logMarker, s"Could not parse EsInfo for ${hit.id} - $errors")
-                "Could not extract migration info from ES due to parsing failure"
-              case _ => "UNKNOWN - NO FAILURE MATCHING MIGRATION INDEX NAME"
-            }
-            FailedMigrationDetails(imageId = hit.id, cause = cause)
+
+          Json.parse(hit.sourceAsString).asOpt[Image].fold(
+            FailedMigrationDetails(hit.id, "ES DOC CORRUPT", "ES DOC CORRUPT", "ES DOC CORRUPT")
+          )( image =>
+            FailedMigrationDetails(
+              imageId = hit.id,
+              lastModified = image.lastModified.fold("")(_.toString),
+              crops = image.exports.size.toString,
+              usages = image.usages.size.toString
+            )
+          )
         }
 
         FailedMigrationSummary(
           totalFailed = resp.result.hits.total.value,
-          totalFailedRelation = resp.result.hits.total.relation,
-          returned = resp.result.hits.hits.length,
           details = failedMigrationDetails
         )
       }

thrall/app/views/index.scala.html

@@ -80,7 +80,7 @@ <h2>MigrationStatus (from the MigrationStatusProvider, a.k.a. 'migration status
     <em>Note that the above represents the value at the point this page was loaded.</em>
 
     @if(migrationStatus.isInstanceOf[Running]) {
-        <p><a href="@routes.ThrallController.migrationFailures(None)">View images that have failed to migrate and retry.</a></p>
+        <p><a href="@routes.ThrallController.migrationFailuresOverview()">View images that have failed to migrate and retry.</a></p>
     }
 </body>
 </html>

thrall/app/views/migrationFailures.scala.html

@@ -5,41 +5,40 @@
     failures: FailedMigrationSummary,
     apiBaseUrl: String,
     uiBaseUrl: String,
+    filter: String,
     page: Int,
 )
 
 @navigation = {
     @if(page > 1) {
-        <a href="?page=@(page - 1)">Previous page</a>
+        <a href="@routes.ThrallController.migrationFailures(filter, Some(page - 1))">Previous page</a>
     }
-    <a href="?page=@(page + 1)">Next page</a>
-}
-
-@totalFailures = @{
-    (failures.totalFailedRelation match {
-        case "lte" | "lt" => "Less than "
-        case "gte" | "gt" => "More than "
-        case _ => ""
-    }) + failures.totalFailed + " images failed in total!"
+    <a href="@routes.ThrallController.migrationFailures(filter, Some(page + 1))">Next page</a>
 }
 
 <!DOCTYPE html>
 <html lang="en">
 <head>
     <meta charset="UTF-8">
-    <title>Migration Failures - Page @(page)</title>
+    <title>Migration Failures - Page @(page) - @filter</title>
     <link rel="stylesheet" href="@routes.Assets.versioned("stylesheets/main.css")" />
 </head>
 <body>
-    <h1>
-        Migration Failures - Page @(page)
-    </h1>
-    <p>@totalFailures</p>
-    @navigation
+    <div class="sticky headingSection">
+        <a href="@routes.ThrallController.migrationFailuresOverview()">&lt; back to Migration Failures Overview</a>
+        <h1>
+            Migration Failures - Page @(page) - <code>@filter</code>
+        </h1>
+        <p>@failures.totalFailed images failed with the above message!</p>
+        @navigation
+    </div>
+
     <table>
-        <tr>
+        <tr class="sticky headingRow">
             <th>Image ID</th>
-            <th>Failure cause</th>
+            <th>Last Modified</th>
+            <th>Crop Count</th>
+            <th>Usage Count</th>
             <th>Click to reattempt migration</th>
         </tr>
         @for(failure <- failures.details) {
@@ -48,7 +47,15 @@ <h1>
                     <a href="@uiBaseUrl/images/@failure.imageId" target="_blank">[Grid]</a>
                     <a href="@apiBaseUrl/images/@failure.imageId" target="_blank">[API]</a>
                 </td>
-                <td>@failure.cause</td>
+                <td>@failure.lastModified</td>
+                <td>@failure.crops</td>
+                <td>
+                    @failure.usages
+                    <br/>
+                    <a href="https://content.guardianapis.com/[email protected]&format=json&api-key=[KEY]">
+                        Full Usage Search in CAPI
+                    </a>
+                </td>
                 <td>@form(action = routes.ThrallController.migrateSingleImage){
                     <label for="id" class="hidden">Image ID:</label>
                     <input type="text" id="id" value="@failure.imageId" name="id" class="hidden">

thrall/app/views/migrationFailuresOverview.scala.html

@@ -0,0 +1,51 @@
+@import lib.FailedMigrationsOverview
+@(
+    failuresOverview: FailedMigrationsOverview,
+    apiBaseUrl: String,
+    uiBaseUrl: String
+)
+
+<!DOCTYPE html>
+<html lang="en">
+<head>
+    <meta charset="UTF-8">
+    <title>Migration Failures Overview</title>
+    <link rel="stylesheet" href="@routes.Assets.versioned("stylesheets/main.css")" />
+</head>
+<body>
+    <div class="sticky headingSection">
+        <h1>
+            Migration Failures Overview
+        </h1>
+        <p>@failuresOverview.totalFailed images failed in total!</p>
+    </div>
+    <table>
+        <tr>
+            <th>Failure cause</th>
+            <th>Count</th>
+            <th>Examples</th>
+        </tr>
+        @for(failureGrouping <- failuresOverview.grouped) {
+            <tr>
+                <td>
+                    <a href="@routes.ThrallController.migrationFailures(failureGrouping.message, None)">
+                        @failureGrouping.message
+                    </a>
+                </td>
+                <td>@failureGrouping.count</td>
+                <td>
+                    <ul>
+                    @for(failureExampleID <- failureGrouping.exampleIDs) {
+                        <li>
+                            <pre class="imageId">@failureExampleID</pre>
+                            <a href="@uiBaseUrl/images/@failureExampleID" target="_blank">[Grid]</a>
+                            <a href="@apiBaseUrl/images/@failureExampleID" target="_blank">[API]</a>
+                        </li>
+                    }
+                    </ul>
+                </td>
+            </tr>
+        }
+    </table>
+</body>
+</html>

thrall/conf/routes

@@ -3,7 +3,8 @@
 # ~~~~
 
 GET     /                                             controllers.ThrallController.index
-GET     /migrationFailures                            controllers.ThrallController.migrationFailures(page: Option[Int])
+GET     /migrationFailuresOverview                     controllers.ThrallController.migrationFailuresOverview()
+GET     /migrationFailures                            controllers.ThrallController.migrationFailures(filter: String, page: Option[Int])
 
 +nocsrf
 POST    /startMigration                               controllers.ThrallController.startMigration

thrall/public/stylesheets/main.css

@@ -1,5 +1,6 @@
 table, td, th {
   border: 1px solid black;
+  margin: 8px;
 }
 
 td, th {
@@ -13,3 +14,21 @@ td, th {
 .imageId {
   display: inline;
 }
+
+body {
+  margin: 0;
+}
+
+.sticky {
+  position: sticky;
+  background: white;
+}
+
+.headingSection {
+  padding: 8px;
+  top: 0;
+}
+
+.headingRow {
+  top: 166px;
+}