diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/Actors/DeadLetterMonitorActor.cs b/src/ZB.MOM.WW.ScadaBridge.Host/Actors/DeadLetterMonitorActor.cs index 87f86d8b..825a78d9 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/Actors/DeadLetterMonitorActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/Actors/DeadLetterMonitorActor.cs @@ -8,12 +8,31 @@ namespace ZB.MOM.WW.ScadaBridge.Host.Actors; /// /// Subscribes to Akka.NET dead letter events, logs them, and tracks count /// for health monitoring integration. +/// +/// Warning logging is rate-limited to per +/// : a failover/undeploy storm can produce thousands +/// of dead letters, and one Warning per letter is an unbounded log flood +/// (review 01 [Low]). Once the per-window cap is hit, per-letter warnings are +/// suppressed and a single summary of the suppressed count is logged when the +/// window rolls over. The health-metric count is never throttled — every +/// dead letter is counted. /// -public class DeadLetterMonitorActor : ReceiveActor +public class DeadLetterMonitorActor : ReceiveActor, IWithTimers { + /// Maximum per-letter Warning logs emitted within one . + public const int MaxWarningsPerWindow = 10; + + /// Rolling window over which applies. + internal static readonly TimeSpan WarningWindow = TimeSpan.FromMinutes(1); + private long _deadLetterCount; + private int _warningsThisWindow; + private long _suppressedThisWindow; private readonly ISiteHealthCollector? _healthCollector; + /// Akka timer scheduler (assigned by the runtime via ). + public ITimerScheduler Timers { get; set; } = null!; + /// /// Initializes the actor and registers dead-letter message handlers. /// @@ -25,13 +44,39 @@ public class DeadLetterMonitorActor : ReceiveActor Receive(dl => { + // Metric counting is never throttled. _deadLetterCount++; _healthCollector?.IncrementDeadLetter(); - logger.LogWarning( - "Dead letter: {MessageType} from {Sender} to {Recipient}", - dl.Message.GetType().Name, - dl.Sender, - dl.Recipient); + + if (_warningsThisWindow < MaxWarningsPerWindow) + { + _warningsThisWindow++; + logger.LogWarning( + "Dead letter: {MessageType} from {Sender} to {Recipient}", + dl.Message.GetType().Name, + dl.Sender, + dl.Recipient); + + if (_warningsThisWindow == MaxWarningsPerWindow) + logger.LogWarning( + "Dead-letter warning limit ({Limit}) reached — suppressing per-letter warnings for {Window}", + MaxWarningsPerWindow, WarningWindow); + } + else + { + _suppressedThisWindow++; + } + }); + + Receive(_ => + { + if (_suppressedThisWindow > 0) + logger.LogWarning( + "Suppressed {Count} dead-letter warnings in the last {Window}", + _suppressedThisWindow, WarningWindow); + + _warningsThisWindow = 0; + _suppressedThisWindow = 0; }); Receive(_ => Sender.Tell(new DeadLetterCountResponse(_deadLetterCount))); @@ -41,6 +86,7 @@ public class DeadLetterMonitorActor : ReceiveActor protected override void PreStart() { Context.System.EventStream.Subscribe(Self, typeof(DeadLetter)); + Timers.StartPeriodicTimer("dl-window", WindowTick.Instance, WarningWindow); } /// @@ -48,6 +94,13 @@ public class DeadLetterMonitorActor : ReceiveActor { Context.System.EventStream.Unsubscribe(Self, typeof(DeadLetter)); } + + /// Periodic self-message that rolls the warning-rate-limit window. + private sealed class WindowTick + { + public static readonly WindowTick Instance = new(); + private WindowTick() { } + } } /// diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/DeadLetterMonitorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/DeadLetterMonitorTests.cs index ae754e74..00ed2ae8 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/DeadLetterMonitorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/DeadLetterMonitorTests.cs @@ -1,8 +1,10 @@ +using System.Linq; using Akka.Actor; using Akka.Event; using Akka.TestKit.Xunit2; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; using ZB.MOM.WW.ScadaBridge.Host.Actors; namespace ZB.MOM.WW.ScadaBridge.Host.Tests; @@ -48,6 +50,33 @@ public class DeadLetterMonitorTests : TestKit }); } + [Fact] + public async Task DeadLetterStorm_LogsAtMostWindowLimit_ButCountsAll() + { + // Review 01 [Low]: a failover/undeploy storm produced one Warning per + // dead letter — unbounded log flood. Health metric counting is untouched. + var logger = Substitute.For>(); + var monitor = Sys.ActorOf(Props.Create(() => new DeadLetterMonitorActor(logger, null))); + + // Ensure the actor has started before the storm. + monitor.Tell(GetDeadLetterCount.Instance); + ExpectMsg(); + + for (var i = 0; i < 50; i++) + monitor.Tell(new DeadLetter($"m{i}", TestActor, TestActor)); + + var count = await monitor.Ask(GetDeadLetterCount.Instance); + Assert.Equal(50, count.Count); // every dead letter still counted + + var warnings = logger.ReceivedCalls().Count(c => + c.GetMethodInfo().Name == "Log" + && (Microsoft.Extensions.Logging.LogLevel)c.GetArguments()[0]! + == Microsoft.Extensions.Logging.LogLevel.Warning); + // The per-letter cap plus the one "limit reached" notice. + Assert.True(warnings <= DeadLetterMonitorActor.MaxWarningsPerWindow + 1, + $"expected <= {DeadLetterMonitorActor.MaxWarningsPerWindow + 1} warnings, got {warnings}"); + } + [Fact] public void DeadLetterMonitor_CountAccumulates() { diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ZB.MOM.WW.ScadaBridge.Host.Tests.csproj b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ZB.MOM.WW.ScadaBridge.Host.Tests.csproj index d970bf47..d6fb9a82 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ZB.MOM.WW.ScadaBridge.Host.Tests.csproj +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ZB.MOM.WW.ScadaBridge.Host.Tests.csproj @@ -18,6 +18,7 @@ +