Discard fixed stack events that don't regress #1507

Discard fixed stack events that don't regress

Author: niemyjski

src/Exceptionless.Core/Pipeline/030_CheckForRegressionAction.cs

@@ -23,7 +23,7 @@ public CheckForRegressionAction(IStackRepository stackRepository, SemanticVersio
     public override async Task ProcessBatchAsync(ICollection<EventContext> contexts)
     {
         var stacks = contexts
-            .Where(c => c.Stack is not null && c.Stack.Status != StackStatus.Regressed && c.Stack.DateFixed.HasValue)
+            .Where(c => c.Stack is { Status: StackStatus.Fixed, DateFixed: not null })
             .OrderBy(c => c.Event.Date)
             .GroupBy(c => c.Event.StackId);
 
@@ -52,7 +52,16 @@ public override async Task ProcessBatchAsync(ICollection<EventContext> contexts)
                     {
                         var version = _semanticVersionParser.Parse(versionGroup.Key, versionCache) ?? _semanticVersionParser.Default;
                         if (version < fixedInVersion)
+                        {
+                            foreach (var ctx in stackGroup.Where(s => s.Organization.HasPremiumFeatures))
+                            {
+                                _logger.LogDebug("Discarding fixed stack event: Version {Version} is older than fixed in version {FixedInVersion}", version, fixedInVersion);
+                                ctx.IsDiscarded = true;
+                                ctx.IsCancelled = true;
+                            }
+
                             continue;
+                        }
 
                         regressedVersion = version;
                         regressedContext = versionGroup.First();
@@ -63,7 +72,7 @@ public override async Task ProcessBatchAsync(ICollection<EventContext> contexts)
                 if (regressedContext is null)
                     return;
 
-                _logger.LogTrace("Marking stack and events as regressed in version: {Version}", regressedVersion);
+                _logger.LogDebug("Marking stack and events as regressed in version: {Version}", regressedVersion);
                 stack.Status = StackStatus.Regressed;
                 await _stackRepository.MarkAsRegressedAsync(stack.Id);
 
@@ -79,7 +88,10 @@ public override async Task ProcessBatchAsync(ICollection<EventContext> contexts)
                     {
                         cont = HandleError(ex, context);
                     }
-                    catch { }
+                    catch (Exception hex)
+                    {
+                        _logger.LogError(hex, "Error calling HandleError: {Message}", ex.Message);
+                    }
 
                     if (!cont)
                         context.SetError(ex.Message, ex);

tests/Exceptionless.Tests/Pipeline/EventPipelineTests.cs

@@ -12,6 +12,7 @@
 using Exceptionless.Core.Queues.Models;
 using Exceptionless.Core.Repositories;
 using Exceptionless.Core.Repositories.Configuration;
+using Exceptionless.Core.Utility;
 using Exceptionless.DateTimeExtensions;
 using Exceptionless.Tests.Utility;
 using Foundatio.Repositories;
@@ -929,7 +930,7 @@ public async Task WillHandleDiscardedStack()
         var organization = OrganizationData.GenerateSampleOrganization(_billingManager, _plans);
         var project = ProjectData.GenerateSampleProject();
 
-        var ev = EventData.GenerateEvent(organizationId: TestConstants.OrganizationId, projectId: TestConstants.ProjectId, type: Event.KnownTypes.Log, source: "test", occurrenceDate: SystemClock.OffsetNow);
+        var ev = EventData.GenerateEvent(organizationId: organization.Id, projectId: project.Id, type: Event.KnownTypes.Log, source: "test", occurrenceDate: SystemClock.OffsetNow);
         var context = await _pipeline.RunAsync(ev, organization, project);
         Assert.True(context.IsProcessed);
         Assert.False(context.HasError);
@@ -944,22 +945,101 @@ public async Task WillHandleDiscardedStack()
         stack.Status = StackStatus.Discarded;
         stack = await _stackRepository.SaveAsync(stack, o => o.ImmediateConsistency());
 
-
-        ev = EventData.GenerateEvent(organizationId: TestConstants.OrganizationId, projectId: TestConstants.ProjectId, stackId: ev.StackId, type: Event.KnownTypes.Log, source: "test", occurrenceDate: SystemClock.OffsetNow);
+        ev = EventData.GenerateEvent(organizationId: organization.Id, projectId: project.Id, type: Event.KnownTypes.Log, source: "test", occurrenceDate: SystemClock.OffsetNow);
         context = await _pipeline.RunAsync(ev, organization, project);
         Assert.False(context.IsProcessed);
         Assert.False(context.HasError);
         Assert.True(context.IsCancelled);
         Assert.True(context.IsDiscarded);
         await RefreshDataAsync();
 
-        ev = EventData.GenerateEvent(organizationId: TestConstants.OrganizationId, projectId: TestConstants.ProjectId, type: Event.KnownTypes.Log, source: "test", occurrenceDate: SystemClock.OffsetNow);
+        ev = EventData.GenerateEvent(organizationId: organization.Id, projectId: project.Id, type: Event.KnownTypes.Log, source: "test", occurrenceDate: SystemClock.OffsetNow);
         context = await _pipeline.RunAsync(ev, organization, project);
         Assert.False(context.IsProcessed);
         Assert.False(context.HasError);
         Assert.True(context.IsCancelled);
         Assert.True(context.IsDiscarded);
+    }
+
+    [Theory]
+    [InlineData(StackStatus.Regressed, false, null, null)]
+    [InlineData(StackStatus.Fixed, true, "1.0.0", null)] // A fixed stack should not be marked as regressed if the event has no version.
+    [InlineData(StackStatus.Regressed, false, null, "1.0.0")]
+    [InlineData(StackStatus.Regressed, false, "1.0.0", "1.0.0")] // A fixed stack should not be marked as regressed if the event has the same version.
+    [InlineData(StackStatus.Fixed, true, "2.0.0", "1.0.0")]
+    [InlineData(StackStatus.Regressed, false, null, "1.0.1")]
+    [InlineData(StackStatus.Regressed, false, "1.0.0", "1.0.1")]
+    public async Task CanDiscardStackEventsBasedOnEventVersion(StackStatus expectedStatus, bool expectedDiscard, string? stackFixedInVersion, string? eventSemanticVersion)
+    {
+        var organization = await _organizationRepository.GetByIdAsync(TestConstants.OrganizationId, o => o.Cache());
+        var project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId, o => o.Cache());
+
+        var ev = EventData.GenerateEvent(organizationId: organization.Id, projectId: project.Id, type: Event.KnownTypes.Log, source: "test", occurrenceDate: SystemClock.OffsetNow);
+        var context = await _pipeline.RunAsync(ev, organization, project);
+
+        var stack = context.Stack;
+        Assert.NotNull(stack);
+        Assert.Equal(StackStatus.Open, stack.Status);
+
+        Assert.True(context.IsProcessed);
+        Assert.False(context.HasError);
+        Assert.False(context.IsCancelled);
+        Assert.False(context.IsDiscarded);
+
+        var semanticVersionParser = GetService<SemanticVersionParser>();
+        var fixedInVersion = semanticVersionParser.Parse(stackFixedInVersion);
+        stack.MarkFixed(fixedInVersion);
+        await _stackRepository.SaveAsync(stack, o => o.ImmediateConsistency());
+
+        await RefreshDataAsync();
+        ev = EventData.GenerateEvent(organizationId: organization.Id, projectId: project.Id, type: Event.KnownTypes.Log, source: "test", occurrenceDate: SystemClock.OffsetNow, semver: eventSemanticVersion);
+        context = await _pipeline.RunAsync(ev, organization, project);
+
+        stack = context.Stack;
+        Assert.NotNull(stack);
+        Assert.Equal(expectedStatus, stack.Status);
+        Assert.Equal(expectedDiscard, context.IsCancelled);
+        Assert.Equal(expectedDiscard, context.IsDiscarded);
+    }
+
+    [Theory]
+    [InlineData("1.0.0", null)] // A fixed stack should not be marked as regressed if the event has no version.
+    [InlineData("2.0.0", "1.0.0")]
+    public async Task WillNotDiscardStackEventsBasedOnEventVersionWithFreePlan(string stackFixedInVersion, string? eventSemanticVersion)
+    {
+        var organization = await _organizationRepository.GetByIdAsync(TestConstants.OrganizationId3, o => o.Cache());
+
+        var plans = GetService<BillingPlans>();
+        Assert.Equal(plans.FreePlan.Id, organization.PlanId);
+
+        var project = await _projectRepository.AddAsync(ProjectData.GenerateProject(organizationId: organization.Id), o => o.ImmediateConsistency().Cache());
+
+        var ev = EventData.GenerateEvent(organizationId: organization.Id, projectId: project.Id, type: Event.KnownTypes.Log, source: "test", occurrenceDate: SystemClock.OffsetNow);
+        var context = await _pipeline.RunAsync(ev, organization, project);
+
+        var stack = context.Stack;
+        Assert.NotNull(stack);
+        Assert.Equal(StackStatus.Open, stack.Status);
+
+        Assert.True(context.IsProcessed);
+        Assert.False(context.HasError);
+        Assert.False(context.IsCancelled);
+        Assert.False(context.IsDiscarded);
+
+        var semanticVersionParser = GetService<SemanticVersionParser>();
+        var fixedInVersion = semanticVersionParser.Parse(stackFixedInVersion);
+        stack.MarkFixed(fixedInVersion);
+        await _stackRepository.SaveAsync(stack, o => o.ImmediateConsistency());
+
         await RefreshDataAsync();
+        ev = EventData.GenerateEvent(organizationId: organization.Id, projectId: project.Id, type: Event.KnownTypes.Log, source: "test", occurrenceDate: SystemClock.OffsetNow, semver: eventSemanticVersion);
+        context = await _pipeline.RunAsync(ev, organization, project);
+
+        stack = context.Stack;
+        Assert.NotNull(stack);
+        Assert.Equal(StackStatus.Fixed, stack.Status);
+        Assert.False(context.IsCancelled);
+        Assert.False(context.IsDiscarded);
     }
 
     [Theory]