Merge pull request #984 from Project-MONAI/nds-stats-fix

adding failure reason to executionStats

Author: neildsouth

src/WorkflowManager/Contracts/Migrations/M001_ExecutionStats_addVersion.cs

@@ -22,7 +22,7 @@ namespace Monai.Deploy.WorkflowManager.Common.Contracts.Migrations
 {
     public class M001_ExecutionStats_addVersion : DocumentMigration<ExecutionStats>
     {
-        public M001_ExecutionStats_addVersion() : base("1.0.0") { }
+        public M001_ExecutionStats_addVersion() : base("1.0.1") { }
 
         public override void Up(BsonDocument document)
         {

src/WorkflowManager/Contracts/Migrations/M002_ExecutionStats_addWorkflowId.cs

@@ -22,7 +22,7 @@ namespace Monai.Deploy.WorkflowManager.Common.Contracts.Migrations
 {
     public class M002_ExecutionStats_addWorkflowId : DocumentMigration<ExecutionStats>
     {
-        public M002_ExecutionStats_addWorkflowId() : base("1.0.1") { }
+        public M002_ExecutionStats_addWorkflowId() : base("1.0.2") { }
 
         public override void Up(BsonDocument document)
         {

src/WorkflowManager/Contracts/Migrations/M003_ExecutionStats_addFailureReason.cs

@@ -0,0 +1,42 @@
+/*
+ * Copyright 2022 MONAI Consortium
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+using Monai.Deploy.WorkflowManager.Common.Contracts.Models;
+using Mongo.Migration.Migrations.Document;
+using MongoDB.Bson;
+
+namespace Monai.Deploy.WorkflowManager.Common.Contracts.Migrations
+{
+    public class M003_ExecutionStats_addFailureReason : DocumentMigration<ExecutionStats>
+    {
+        public M003_ExecutionStats_addFailureReason() : base("1.0.3") { }
+
+        public override void Up(BsonDocument document)
+        {
+            // empty, but this will make all objects re-saved with a reason
+        }
+        public override void Down(BsonDocument document)
+        {
+            try
+            {
+                document.Remove("Reason");
+            }
+            catch
+            {  // can ignore we dont want failures stopping startup !
+            }
+        }
+    }
+}

src/WorkflowManager/Contracts/Models/ExecutionStats.cs

@@ -17,7 +17,6 @@
 using System;
 using System.ComponentModel.DataAnnotations;
 using Monai.Deploy.WorkflowManager.Common.Contracts.Migrations;
-using Ardalis.GuardClauses;
 using Monai.Deploy.Messaging.Events;
 using Mongo.Migration.Documents;
 using Mongo.Migration.Documents.Attributes;
@@ -26,7 +25,7 @@
 
 namespace Monai.Deploy.WorkflowManager.Common.Contracts.Models
 {
-    [CollectionLocation("ExecutionStats"), RuntimeVersion("1.0.1")]
+    [CollectionLocation("ExecutionStats"), RuntimeVersion("1.0.3")]
     public class ExecutionStats : IDocument
     {
         /// <summary>
@@ -40,7 +39,7 @@ public class ExecutionStats : IDocument
         /// Gets or sets Db version.
         /// </summary>
         [JsonConverter(typeof(DocumentVersionConvert)), BsonSerializer(typeof(DocumentVersionConverBson))]
-        public DocumentVersion Version { get; set; } = new DocumentVersion(1, 0, 1);
+        public DocumentVersion Version { get; set; } = new DocumentVersion(1, 0, 2);
 
         /// <summary>
         /// the correlationId of the event
@@ -110,6 +109,12 @@ public class ExecutionStats : IDocument
         [JsonProperty(PropertyName = "status")]
         public string Status { get; set; } = TaskExecutionStatus.Created.ToString();
 
+        /// <summary>
+        /// Gets or sets the failure reason.
+        /// </summary>
+        [JsonProperty(PropertyName = "reason")]
+        public FailureReason Reason { get; set; }
+
         /// <summary>
         /// Gets or sets the duration, difference between startedAt and CompletedAt time.
         /// </summary>
@@ -134,6 +139,7 @@ public ExecutionStats(TaskExecution execution, string workflowId, string correla
             StartedUTC = execution.TaskStartTime.ToUniversalTime();
             Status = execution.Status.ToString();
             WorkflowId = workflowId;
+            Reason = execution.Reason;
         }
 
         public ExecutionStats(TaskUpdateEvent taskUpdateEvent, string workflowId)
@@ -145,6 +151,7 @@ public ExecutionStats(TaskUpdateEvent taskUpdateEvent, string workflowId)
             TaskId = taskUpdateEvent.TaskId;
             Status = taskUpdateEvent.Status.ToString();
             WorkflowId = workflowId;
+            Reason = taskUpdateEvent.Reason;
         }
 
         public ExecutionStats(TaskCancellationEvent taskCanceledEvent, string workflowId, string correlationId)
@@ -156,6 +163,7 @@ public ExecutionStats(TaskCancellationEvent taskCanceledEvent, string workflowId
             TaskId = taskCanceledEvent.TaskId;
             Status = TaskExecutionStatus.Failed.ToString();
             WorkflowId = workflowId;
+            Reason = taskCanceledEvent.Reason;
         }
     }
 }

src/WorkflowManager/Database/Repositories/TaskExecutionStatsRepository.cs

@@ -110,6 +110,7 @@ await _taskExecutionStatsCollection.UpdateOneAsync(o =>
                         .Set(w => w.CompletedAtUTC, updateMe.CompletedAtUTC)
                         .Set(w => w.ExecutionTimeSeconds, updateMe.ExecutionTimeSeconds)
                         .Set(w => w.DurationSeconds, duration)
+                        .Set(w => w.Reason, taskUpdateEvent.Reason)
 
                     , new UpdateOptions { IsUpsert = true }).ConfigureAwait(false);
             }
@@ -132,6 +133,7 @@ await _taskExecutionStatsCollection.UpdateOneAsync(o =>
                         o.ExecutionId == updateMe.ExecutionId,
                     Builders<ExecutionStats>.Update
                         .Set(w => w.Status, updateMe.Status)
+                        .Set(w => w.Reason, taskCanceledEvent.Reason)
                         .Set(w => w.LastUpdatedUTC, DateTime.UtcNow)
                         .Set(w => w.CompletedAtUTC, updateMe.CompletedAtUTC)
                         .Set(w => w.DurationSeconds, duration)

src/WorkflowManager/WorkflowManager/Controllers/TaskStatsController.cs

@@ -136,13 +136,15 @@ public async Task<IActionResult> GetDailyStatsAsync([FromQuery] TimeFilter filte
                     {
                         Date = DateOnly.FromDateTime(g.Key.Date),
                         TotalExecutions = g.Count(),
-                        TotalFailures = g.Count(i => string.Compare(i.Status, "Failed", true) == 0),
-                        TotalApprovals = g.Count(i => string.Compare(i.Status, ApplicationReviewStatus.Approved.ToString(), true) == 0),
-                        TotalRejections = g.Count(i => string.Compare(i.Status, ApplicationReviewStatus.Rejected.ToString(), true) == 0),
-                        TotalCancelled = g.Count(i => string.Compare(i.Status, ApplicationReviewStatus.Cancelled.ToString(), true) == 0),
+                        TotalFailures = g.Count(i => string.Compare(i.Status, "Failed", true) == 0 && i.Reason != FailureReason.TimedOut && i.Reason != FailureReason.Rejected),
+                        TotalApprovals = g.Count(i => string.Compare(i.Status, "Succeeded", true) == 0 && i.Reason == FailureReason.None),
+                        TotalRejections = g.Count(i => string.Compare(i.Status, "Failed", true) == 0 && i.Reason == FailureReason.Rejected),
+                        TotalCancelled = g.Count(i => string.Compare(i.Status, "Failed", true) == 0 && i.Reason == FailureReason.TimedOut),
                         TotalAwaitingReview = g.Count(i => string.Compare(i.Status, ApplicationReviewStatus.AwaitingReview.ToString(), true) == 0),
                     });
 
+
+
                 var pagedStats = statsDto.Skip((filter.PageNumber - 1) * pageSize).Take(pageSize);
 
                 var res = CreateStatsPagedResponse(pagedStats, validFilter, statsDto.Count(), _uriService, route);
@@ -152,7 +154,7 @@ public async Task<IActionResult> GetDailyStatsAsync([FromQuery] TimeFilter filte
                 res.PeriodEnd = filter.EndTime;
                 res.TotalExecutions = allStats.Count();
                 res.TotalSucceeded = statsDto.Sum(s => s.TotalApprovals);
-                res.TotalFailures = statsDto.Sum(s => s.TotalFailures);
+                res.TotalFailures = statsDto.Sum(s => s.TotalFailures + s.TotalCancelled + s.TotalRejections);
                 res.TotalInprogress = statsDto.Sum(s => s.TotalAwaitingReview);
                 res.AverageTotalExecutionSeconds = Math.Round(avgTotalExecution, 2);
                 res.AverageArgoExecutionSeconds = Math.Round(avgArgoExecution, 2);

tests/UnitTests/WorkflowManager.Tests/Controllers/TaskExecutionStatsControllerTests.cs

@@ -336,6 +336,102 @@ public async Task GetAllStatsAsync_Pass_All_Arguments_To_GetStatsAsync_In_Repo()
                 It.Is<string>(s => s.Equals("")))
             );
         }
+
+        [Fact]
+        public async Task GetAllStatsAsync_Get_Correct_Reject_Count()
+        {
+            var startTime = new DateTime(2023, 4, 4);
+            var endTime = new DateTime(2023, 4, 5);
+            const int pageNumber = 1;
+            const int pageSize = 10;
+
+            var executionStats = new ExecutionStats[]
+            {
+                new ExecutionStats
+                            {
+                                ExecutionId = Guid.NewGuid().ToString(),
+                                StartedUTC = _startTime,
+                                WorkflowInstanceId= "workflow",
+                                TaskId = "task",
+                                Status = "Failed",
+                                Reason = Messaging.Events.FailureReason.Rejected,
+                            },
+            };
+
+            _repo.Setup(w => w.GetAllStatsAsync(It.IsAny<DateTime>(), It.IsAny<DateTime>(), It.IsAny<string>(), It.IsAny<string>())).ReturnsAsync(executionStats);
+
+            var result = await StatsController.GetDailyStatsAsync(new TimeFilter { StartTime = startTime, EndTime = endTime, PageNumber = pageNumber, PageSize = pageSize }, "workflow");
+
+            var resultCollection = result.As<OkObjectResult>().Value.As<StatsPagedResponse<IEnumerable<ExecutionStatDayOverview>>>().Data;
+
+            Assert.Equal(1, resultCollection.First().TotalExecutions);
+            Assert.Equal(1, resultCollection.First().TotalRejections);
+            Assert.Equal(0, resultCollection.First().TotalFailures);
+        }
+
+        [Fact]
+        public async Task GetAllStatsAsync_Get_Correct_Canceled_Count()
+        {
+            var startTime = new DateTime(2023, 4, 4);
+            var endTime = new DateTime(2023, 4, 5);
+            const int pageNumber = 1;
+            const int pageSize = 10;
+
+            var executionStats = new ExecutionStats[]
+            {
+                new ExecutionStats
+                            {
+                                ExecutionId = Guid.NewGuid().ToString(),
+                                StartedUTC = _startTime,
+                                WorkflowInstanceId= "workflow",
+                                TaskId = "task",
+                                Status = "Failed",
+                                Reason = Messaging.Events.FailureReason.TimedOut,
+                            },
+            };
+
+            _repo.Setup(w => w.GetAllStatsAsync(It.IsAny<DateTime>(), It.IsAny<DateTime>(), It.IsAny<string>(), It.IsAny<string>())).ReturnsAsync(executionStats);
+
+            var result = await StatsController.GetDailyStatsAsync(new TimeFilter { StartTime = startTime, EndTime = endTime, PageNumber = pageNumber, PageSize = pageSize }, "workflow");
+
+            var resultCollection = result.As<OkObjectResult>().Value.As<StatsPagedResponse<IEnumerable<ExecutionStatDayOverview>>>().Data;
+
+            Assert.Equal(1, resultCollection.First().TotalExecutions);
+            Assert.Equal(1, resultCollection.First().TotalCancelled);
+            Assert.Equal(0, resultCollection.First().TotalFailures);
+        }
+
+        [Fact]
+        public async Task GetAllStatsAsync_Get_Correct_Accepted_Count()
+        {
+            var startTime = new DateTime(2023, 4, 4);
+            var endTime = new DateTime(2023, 4, 5);
+            const int pageNumber = 1;
+            const int pageSize = 10;
+
+            var executionStats = new ExecutionStats[]
+            {
+                new ExecutionStats
+                            {
+                                ExecutionId = Guid.NewGuid().ToString(),
+                                StartedUTC = _startTime,
+                                WorkflowInstanceId= "workflow",
+                                TaskId = "task",
+                                Status = "Succeeded",
+                                Reason = Messaging.Events.FailureReason.None,
+                            },
+            };
+
+            _repo.Setup(w => w.GetAllStatsAsync(It.IsAny<DateTime>(), It.IsAny<DateTime>(), It.IsAny<string>(), It.IsAny<string>())).ReturnsAsync(executionStats);
+
+            var result = await StatsController.GetDailyStatsAsync(new TimeFilter { StartTime = startTime, EndTime = endTime, PageNumber = pageNumber, PageSize = pageSize }, "workflow");
+
+            var resultCollection = result.As<OkObjectResult>().Value.As<StatsPagedResponse<IEnumerable<ExecutionStatDayOverview>>>().Data;
+
+            Assert.Equal(1, resultCollection.First().TotalExecutions);
+            Assert.Equal(1, resultCollection.First().TotalApprovals);
+            Assert.Equal(0, resultCollection.First().TotalFailures);
+        }
     }
 #pragma warning restore CS8604 // Possible null reference argument.
 #pragma warning restore CS8602 // Dereference of a possibly null reference.