From fdb3a6c388bf66d0d710f662c7ad037995c32972 Mon Sep 17 00:00:00 2001 From: Tom Richards Date: Thu, 25 Nov 2021 17:14:35 +0000 Subject: [PATCH 1/2] introduce new grouped view of migration failures (aka 'overview') Co-Authored-By: Andrew Nowak --- thrall/app/controllers/ThrallController.scala | 23 ++++++-- thrall/app/lib/FailedMigrationDetails.scala | 8 ++- .../elasticsearch/ThrallMigrationClient.scala | 56 +++++++++++++------ thrall/app/views/index.scala.html | 2 +- thrall/app/views/migrationFailures.scala.html | 33 +++++------ .../migrationFailuresOverview.scala.html | 49 ++++++++++++++++ thrall/conf/routes | 3 +- 7 files changed, 130 insertions(+), 44 deletions(-) create mode 100644 thrall/app/views/migrationFailuresOverview.scala.html diff --git a/thrall/app/controllers/ThrallController.scala b/thrall/app/controllers/ThrallController.scala index dab6dedbb0..b39c279155 100644 --- a/thrall/app/controllers/ThrallController.scala +++ b/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")) diff --git a/thrall/app/lib/FailedMigrationDetails.scala b/thrall/app/lib/FailedMigrationDetails.scala index 516e87fe89..9536a7480a 100644 --- a/thrall/app/lib/FailedMigrationDetails.scala +++ b/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) -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]) diff --git a/thrall/app/lib/elasticsearch/ThrallMigrationClient.scala b/thrall/app/lib/elasticsearch/ThrallMigrationClient.scala index 6966a0a5a2..3ecee26c22 100644 --- a/thrall/app/lib/elasticsearch/ThrallMigrationClient.scala +++ b/thrall/app/lib/elasticsearch/ThrallMigrationClient.scala @@ -6,7 +6,9 @@ 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 +83,54 @@ 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)) ) 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) + FailedMigrationDetails(imageId = hit.id) } FailedMigrationSummary( totalFailed = resp.result.hits.total.value, - totalFailedRelation = resp.result.hits.total.relation, - returned = resp.result.hits.hits.length, details = failedMigrationDetails ) } diff --git a/thrall/app/views/index.scala.html b/thrall/app/views/index.scala.html index b42dd74e7d..abbfc9616e 100644 --- a/thrall/app/views/index.scala.html +++ b/thrall/app/views/index.scala.html @@ -80,7 +80,7 @@

MigrationStatus (from the MigrationStatusProvider, a.k.a. 'migration status Note that the above represents the value at the point this page was loaded. @if(migrationStatus.isInstanceOf[Running]) { -

View images that have failed to migrate and retry.

+

View images that have failed to migrate and retry.

} diff --git a/thrall/app/views/migrationFailures.scala.html b/thrall/app/views/migrationFailures.scala.html index c45b860eb0..9c9a59499e 100644 --- a/thrall/app/views/migrationFailures.scala.html +++ b/thrall/app/views/migrationFailures.scala.html @@ -5,41 +5,37 @@ failures: FailedMigrationSummary, apiBaseUrl: String, uiBaseUrl: String, + filter: String, page: Int, ) @navigation = { @if(page > 1) { - Previous page + Previous page } - Next page -} - -@totalFailures = @{ - (failures.totalFailedRelation match { - case "lte" | "lt" => "Less than " - case "gte" | "gt" => "More than " - case _ => "" - }) + failures.totalFailed + " images failed in total!" + Next page } - Migration Failures - Page @(page) + Migration Failures - Page @(page) - @filter -

- Migration Failures - Page @(page) -

-

@totalFailures

- @navigation +
+ < back to Migration Failures Overview +

+ Migration Failures - Page @(page) - @filter +

+

@failures.totalFailed images failed with the above message!

+ @navigation +
+ - + - @for(failure <- failures.details) { @@ -48,7 +44,6 @@

[Grid] [API] -

Image IDFailure cause Click to reattempt migration
@failure.cause @form(action = routes.ThrallController.migrateSingleImage){ diff --git a/thrall/app/views/migrationFailuresOverview.scala.html b/thrall/app/views/migrationFailuresOverview.scala.html new file mode 100644 index 0000000000..bb624be1bc --- /dev/null +++ b/thrall/app/views/migrationFailuresOverview.scala.html @@ -0,0 +1,49 @@ +@import lib.FailedMigrationsOverview +@( + failuresOverview: FailedMigrationsOverview, + apiBaseUrl: String, + uiBaseUrl: String +) + + + + + + Migration Failures Overview + + + +

+ Migration Failures Overview +

+

@failuresOverview.totalFailed images failed in total!

+ + + + + + + @for(failureGrouping <- failuresOverview.grouped) { + + + + + + } +
Failure causeCountExamples
+ + @failureGrouping.message + + @failureGrouping.count +
    + @for(failureExampleID <- failureGrouping.exampleIDs) { +
  • + 
    @failureExampleID
    + [Grid] + [API] +
    • + } +
+
+ + diff --git a/thrall/conf/routes b/thrall/conf/routes index cb3913fd96..01c16321b1 100644 --- a/thrall/conf/routes +++ b/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 From 6571c992e6fbc685f4600359e7c0a7ffe3e65e17 Mon Sep 17 00:00:00 2001 From: Tom Richards Date: Fri, 26 Nov 2021 10:07:17 +0000 Subject: [PATCH 2/2] add lastModified, usage count and crop count to migration failure detail view --- thrall/app/lib/FailedMigrationDetails.scala | 2 +- .../elasticsearch/ThrallMigrationClient.scala | 15 +++++++++++++-- thrall/app/views/migrationFailures.scala.html | 16 ++++++++++++++-- .../migrationFailuresOverview.scala.html | 10 ++++++---- thrall/public/stylesheets/main.css | 19 +++++++++++++++++++ 5 files changed, 53 insertions(+), 9 deletions(-) diff --git a/thrall/app/lib/FailedMigrationDetails.scala b/thrall/app/lib/FailedMigrationDetails.scala index 9536a7480a..b452467f92 100644 --- a/thrall/app/lib/FailedMigrationDetails.scala +++ b/thrall/app/lib/FailedMigrationDetails.scala @@ -1,6 +1,6 @@ package lib -final case class FailedMigrationDetails(imageId: String) +final case class FailedMigrationDetails(imageId: String, lastModified: String, crops: String, usages: String) final case class FailedMigrationSummary(totalFailed: Long, details: Seq[FailedMigrationDetails]) diff --git a/thrall/app/lib/elasticsearch/ThrallMigrationClient.scala b/thrall/app/lib/elasticsearch/ThrallMigrationClient.scala index 3ecee26c22..bd424c0132 100644 --- a/thrall/app/lib/elasticsearch/ThrallMigrationClient.scala +++ b/thrall/app/lib/elasticsearch/ThrallMigrationClient.scala @@ -2,6 +2,7 @@ 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._ @@ -122,11 +123,21 @@ trait ThrallMigrationClient extends MigrationStatusProvider { 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 => val failedMigrationDetails: Seq[FailedMigrationDetails] = resp.result.hits.hits.map { hit => - FailedMigrationDetails(imageId = hit.id) + + 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( diff --git a/thrall/app/views/migrationFailures.scala.html b/thrall/app/views/migrationFailures.scala.html index 9c9a59499e..d2ae70198d 100644 --- a/thrall/app/views/migrationFailures.scala.html +++ b/thrall/app/views/migrationFailures.scala.html @@ -24,7 +24,7 @@ -
+
< back to Migration Failures Overview

Migration Failures - Page @(page) - @filter @@ -34,8 +34,11 @@

- + + + + @for(failure <- failures.details) { @@ -44,6 +47,15 @@

[Grid] [API] +

+ +
Image IDLast ModifiedCrop CountUsage Count Click to reattempt migration
@failure.lastModified@failure.crops + @failure.usages +
+ + Full Usage Search in CAPI + + 		@form(action = routes.ThrallController.migrateSingleImage){ diff --git a/thrall/app/views/migrationFailuresOverview.scala.html b/thrall/app/views/migrationFailuresOverview.scala.html index bb624be1bc..a04efc4c6a 100644 --- a/thrall/app/views/migrationFailuresOverview.scala.html +++ b/thrall/app/views/migrationFailuresOverview.scala.html @@ -13,10 +13,12 @@ -

- Migration Failures Overview -

-

@failuresOverview.totalFailed images failed in total!

+
+

+ Migration Failures Overview +

+

@failuresOverview.totalFailed images failed in total!

+
diff --git a/thrall/public/stylesheets/main.css b/thrall/public/stylesheets/main.css index 7f1595518a..bffc133904 100644 --- a/thrall/public/stylesheets/main.css +++ b/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; +}
Failure cause