From befb0e380506eb03322e3a9edddd708b1de28355 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 9 Sep 2026 11:26:05 -0400 Subject: [PATCH 1/2] feat(commons): FlowTraceCollector low-priority lane for log lines (D7) --- .../Types/Debugging/FlowTraceCollector.cs | 54 +++++++++++++-- .../Debugging/FlowTraceCollectorTests.cs | 67 +++++++++++++++++++ 2 files changed, 117 insertions(+), 4 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Debugging/FlowTraceCollector.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Debugging/FlowTraceCollector.cs index 37980852..84042689 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Debugging/FlowTraceCollector.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Debugging/FlowTraceCollector.cs @@ -27,12 +27,33 @@ namespace ZB.MOM.WW.ScadaBridge.Commons.Types.Debugging; /// protobuf framing, and 1 048 576 chars is 1 MB of UTF-8 for ASCII payloads but up to ~3 MB for /// non-ASCII ones. That worst case still fits the 4 MB limit with the rest of the reply, which is /// what the budget exists to guarantee — raise it only with a real serialized-size measurement. +/// Step-logs addendum (D7, 2026-09-09): a ScriptLog row per script log line rides the same +/// sink via , a lower-priority lane that shares the same +/// / accounting but stops +/// events short of the count cap and at of the byte budget — so +/// the step rows a run emits at the very end (the ScriptRun anchor is written last) always +/// have room, even on a chatty run whose log lines alone would otherwise fill the collector. A +/// dropped low-priority row sets instead of , +/// because losing inline log lines does not affect step-row fidelity the way losing a step row +/// would — the page reports it separately ("log lines truncated inline — Replay is complete"). /// public sealed class FlowTraceCollector { /// Default inline cap per run (design §5.4 MaxTraceEventsPerRun). public const int DefaultMaxEvents = 500; + /// + /// Events reserved for (step rows) — (log + /// lines) stops this many events short of the event cap (D7, 2026-09-09). + /// + public const int LowPriorityReserveEvents = 50; + + /// + /// Share of the byte budget (log lines) may consume — the + /// remainder is reserved for (step rows) (D7, 2026-09-09). + /// + public const double LowPriorityByteShare = 0.75; + /// /// Default inline byte budget per run — the summed redacted /// length in UTF-16 chars (see the type remarks: ~1 MB of UTF-8 for ASCII payloads, ~3 MB worst @@ -53,6 +74,13 @@ public sealed class FlowTraceCollector /// public bool Truncated { get; private set; } + /// + /// True once an (log line) was dropped because the low-priority + /// event or byte share was reached — reported separately from because + /// step-row fidelity is unaffected (D7, 2026-09-09). + /// + public bool LogsTruncated { get; private set; } + /// /// True once ran — the run's reply snapshot has been taken and /// later calls are ignored (they never count as ). @@ -77,10 +105,22 @@ public sealed class FlowTraceCollector } /// - /// Appends the REDACTED copy of an event, or marks the collector truncated once the event - /// cap or the byte budget is reached. A no-op after . + /// Appends the REDACTED copy of a step event, or marks the collector truncated once the + /// event cap or the byte budget is reached. A no-op after . Full + /// priority — never yields to a log line queued via . /// - public void Add(AuditEvent evt) + public void Add(AuditEvent evt) => AddCore(evt, lowPriority: false); + + /// + /// Appends the REDACTED copy of a ScriptLog event at LOW priority (D7, 2026-09-09): + /// it stops events short of the event cap and at + /// of the byte budget, so log lines never crowd out the + /// step rows a run emits at the end. A dropped row sets , not + /// . A no-op after . + /// + public void AddLowPriority(AuditEvent evt) => AddCore(evt, lowPriority: true); + + private void AddCore(AuditEvent evt, bool lowPriority) { ArgumentNullException.ThrowIfNull(evt); // Redaction runs OUTSIDE the lock (arch-review F1): IAuditRedactor is a pure function @@ -91,7 +131,13 @@ public sealed class FlowTraceCollector lock (_gate) { if (Completed) return; - if (_events.Count >= _max || _bytes + size > _maxBytes) { Truncated = true; return; } + var maxCount = lowPriority ? Math.Max(1, _max - LowPriorityReserveEvents) : _max; + var maxBytes = lowPriority ? (int)(_maxBytes * LowPriorityByteShare) : _maxBytes; + if (_events.Count >= maxCount || _bytes + size > maxBytes) + { + if (lowPriority) LogsTruncated = true; else Truncated = true; + return; + } _events.Add(filtered); _bytes += size; } diff --git a/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Debugging/FlowTraceCollectorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Debugging/FlowTraceCollectorTests.cs index d27d3176..6d226fcc 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Debugging/FlowTraceCollectorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Debugging/FlowTraceCollectorTests.cs @@ -1,5 +1,7 @@ using ZB.MOM.WW.Audit; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Audit; using ZB.MOM.WW.ScadaBridge.Commons.Types.Debugging; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; namespace ZB.MOM.WW.ScadaBridge.Commons.Tests.Debugging; @@ -16,6 +18,12 @@ public class FlowTraceCollectorTests Target = i.ToString(), }; + /// Low-priority-lane helper (D7, 2026-09-09): a canonical row built through the + /// factory, so DetailsJson carries a real envelope around . + private static AuditEvent Evt(string? details = null) => ScadaBridgeAuditEventFactory.Create( + AuditChannel.ScriptTrace, AuditKind.ScriptRun, AuditStatus.Delivered, + target: "S", requestSummary: details); + [Fact] public void Add_BeyondCap_KeepsFirstN_AndFlagsTruncated() { @@ -128,6 +136,65 @@ public class FlowTraceCollectorTests Assert.Throws(() => new FlowTraceCollector(maxBytes: 0)); } + /// D7 (2026-09-09): the low-priority lane stops short of the event cap so the + /// step rows a run emits at the end always have room. + [Fact] + public void AddLowPriority_StopsBeforeTheReserve_AndSetsLogsTruncated() + { + var c = new FlowTraceCollector(maxEvents: 60, maxBytes: 1_000_000); // reserve = 50 → low-priority stops at 10 + for (var i = 0; i < 12; i++) c.AddLowPriority(Evt()); + + Assert.Equal(10, c.Count); + Assert.True(c.LogsTruncated); + Assert.False(c.Truncated); + + c.Add(Evt()); // a step row still fits + + Assert.Equal(11, c.Count); + } + + /// D7 (2026-09-09): the low-priority lane also stops short of the byte budget. + [Fact] + public void AddLowPriority_RespectsByteShare() + { + var c = new FlowTraceCollector(maxEvents: 500, maxBytes: 1_000); // 75% = 750 chars + c.AddLowPriority(Evt(details: new string('x', 600))); + c.AddLowPriority(Evt(details: new string('x', 200))); + + Assert.Equal(1, c.Count); + Assert.True(c.LogsTruncated); + } + + /// D7 (2026-09-09): the reserve floors at 1, so a tiny cap still admits one log line. + [Fact] + public void AddLowPriority_WithTinyCap_StillAllowsOne() + { + var c = new FlowTraceCollector(maxEvents: 20, maxBytes: 1_000_000); // max(1, 20-50) = 1 + + c.AddLowPriority(Evt()); + Assert.Equal(1, c.Count); + Assert.False(c.LogsTruncated); + + c.AddLowPriority(Evt()); + Assert.Equal(1, c.Count); + Assert.True(c.LogsTruncated); + } + + /// D7 (2026-09-09): closes the low-priority + /// lane too — a late log line is dropped silently, not counted as truncated. + [Fact] + public void AddLowPriority_IsNoOpAfterComplete() + { + var c = new FlowTraceCollector(); + c.AddLowPriority(Evt()); + c.Complete(); + + c.AddLowPriority(Evt()); + + Assert.Equal(1, c.Count); + Assert.False(c.LogsTruncated); + } + /// Upper-cases DetailsJson so a test can see the filtered copy land. private sealed class UpperRedactor : IAuditRedactor { From b73962831ca0f6aa347b67cfbdf30106431cdf68 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 9 Sep 2026 11:28:01 -0400 Subject: [PATCH 2/2] refactor(commons): FlowTraceCollector.Add(lowPriority:) instead of a fourth public method (Commons method-count constraint) --- .../Types/Debugging/FlowTraceCollector.cs | 42 ++++++++----------- .../Debugging/FlowTraceCollectorTests.cs | 22 +++++----- 2 files changed, 29 insertions(+), 35 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Debugging/FlowTraceCollector.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Debugging/FlowTraceCollector.cs index 84042689..10074ae4 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Debugging/FlowTraceCollector.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Debugging/FlowTraceCollector.cs @@ -28,10 +28,10 @@ namespace ZB.MOM.WW.ScadaBridge.Commons.Types.Debugging; /// non-ASCII ones. That worst case still fits the 4 MB limit with the rest of the reply, which is /// what the budget exists to guarantee — raise it only with a real serialized-size measurement. /// Step-logs addendum (D7, 2026-09-09): a ScriptLog row per script log line rides the same -/// sink via , a lower-priority lane that shares the same -/// / accounting but stops -/// events short of the count cap and at of the byte budget — so -/// the step rows a run emits at the very end (the ScriptRun anchor is written last) always +/// sink via 's lowPriority parameter — a lower-priority lane that shares +/// the same event list/byte accounting but stops events +/// short of the count cap and at of the byte budget — so the +/// step rows a run emits at the very end (the ScriptRun anchor is written last) always /// have room, even on a chatty run whose log lines alone would otherwise fill the collector. A /// dropped low-priority row sets instead of , /// because losing inline log lines does not affect step-row fidelity the way losing a step row @@ -43,14 +43,14 @@ public sealed class FlowTraceCollector public const int DefaultMaxEvents = 500; /// - /// Events reserved for (step rows) — (log - /// lines) stops this many events short of the event cap (D7, 2026-09-09). + /// Events reserved for full-priority step rows — a low-priority (log line) + /// stops this many events short of the event cap (D7, 2026-09-09). /// public const int LowPriorityReserveEvents = 50; /// - /// Share of the byte budget (log lines) may consume — the - /// remainder is reserved for (step rows) (D7, 2026-09-09). + /// Share of the byte budget a low-priority (log line) may consume — the + /// remainder is reserved for full-priority step rows (D7, 2026-09-09). /// public const double LowPriorityByteShare = 0.75; @@ -75,7 +75,7 @@ public sealed class FlowTraceCollector public bool Truncated { get; private set; } /// - /// True once an (log line) was dropped because the low-priority + /// True once a low-priority (log line) was dropped because the low-priority /// event or byte share was reached — reported separately from because /// step-row fidelity is unaffected (D7, 2026-09-09). /// @@ -105,22 +105,16 @@ public sealed class FlowTraceCollector } /// - /// Appends the REDACTED copy of a step event, or marks the collector truncated once the - /// event cap or the byte budget is reached. A no-op after . Full - /// priority — never yields to a log line queued via . + /// Appends the REDACTED copy of an event, or marks the collector truncated once the event + /// cap or the byte budget is reached. A no-op after . /// - public void Add(AuditEvent evt) => AddCore(evt, lowPriority: false); - - /// - /// Appends the REDACTED copy of a ScriptLog event at LOW priority (D7, 2026-09-09): - /// it stops events short of the event cap and at - /// of the byte budget, so log lines never crowd out the - /// step rows a run emits at the end. A dropped row sets , not - /// . A no-op after . - /// - public void AddLowPriority(AuditEvent evt) => AddCore(evt, lowPriority: true); - - private void AddCore(AuditEvent evt, bool lowPriority) + /// The event to append. + /// + /// True for a log-line row — stops at the reserve/byte share and sets + /// instead of , so the step rows emitted + /// at run end are never crowded out (D7). False (default) for a full-priority step row. + /// + public void Add(AuditEvent evt, bool lowPriority = false) { ArgumentNullException.ThrowIfNull(evt); // Redaction runs OUTSIDE the lock (arch-review F1): IAuditRedactor is a pure function diff --git a/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Debugging/FlowTraceCollectorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Debugging/FlowTraceCollectorTests.cs index 6d226fcc..b78404b5 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Debugging/FlowTraceCollectorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Debugging/FlowTraceCollectorTests.cs @@ -139,10 +139,10 @@ public class FlowTraceCollectorTests /// D7 (2026-09-09): the low-priority lane stops short of the event cap so the /// step rows a run emits at the end always have room. [Fact] - public void AddLowPriority_StopsBeforeTheReserve_AndSetsLogsTruncated() + public void Add_LowPriority_StopsBeforeTheReserve_AndSetsLogsTruncated() { var c = new FlowTraceCollector(maxEvents: 60, maxBytes: 1_000_000); // reserve = 50 → low-priority stops at 10 - for (var i = 0; i < 12; i++) c.AddLowPriority(Evt()); + for (var i = 0; i < 12; i++) c.Add(Evt(), lowPriority: true); Assert.Equal(10, c.Count); Assert.True(c.LogsTruncated); @@ -155,11 +155,11 @@ public class FlowTraceCollectorTests /// D7 (2026-09-09): the low-priority lane also stops short of the byte budget. [Fact] - public void AddLowPriority_RespectsByteShare() + public void Add_LowPriority_RespectsByteShare() { var c = new FlowTraceCollector(maxEvents: 500, maxBytes: 1_000); // 75% = 750 chars - c.AddLowPriority(Evt(details: new string('x', 600))); - c.AddLowPriority(Evt(details: new string('x', 200))); + c.Add(Evt(details: new string('x', 600)), lowPriority: true); + c.Add(Evt(details: new string('x', 200)), lowPriority: true); Assert.Equal(1, c.Count); Assert.True(c.LogsTruncated); @@ -167,15 +167,15 @@ public class FlowTraceCollectorTests /// D7 (2026-09-09): the reserve floors at 1, so a tiny cap still admits one log line. [Fact] - public void AddLowPriority_WithTinyCap_StillAllowsOne() + public void Add_LowPriority_WithTinyCap_StillAllowsOne() { var c = new FlowTraceCollector(maxEvents: 20, maxBytes: 1_000_000); // max(1, 20-50) = 1 - c.AddLowPriority(Evt()); + c.Add(Evt(), lowPriority: true); Assert.Equal(1, c.Count); Assert.False(c.LogsTruncated); - c.AddLowPriority(Evt()); + c.Add(Evt(), lowPriority: true); Assert.Equal(1, c.Count); Assert.True(c.LogsTruncated); } @@ -183,13 +183,13 @@ public class FlowTraceCollectorTests /// D7 (2026-09-09): closes the low-priority /// lane too — a late log line is dropped silently, not counted as truncated. [Fact] - public void AddLowPriority_IsNoOpAfterComplete() + public void Add_LowPriority_IsNoOpAfterComplete() { var c = new FlowTraceCollector(); - c.AddLowPriority(Evt()); + c.Add(Evt(), lowPriority: true); c.Complete(); - c.AddLowPriority(Evt()); + c.Add(Evt(), lowPriority: true); Assert.Equal(1, c.Count); Assert.False(c.LogsTruncated);