refactor(commons): FlowTraceCollector.Add(lowPriority:) instead of a fourth public method (Commons method-count constraint)

This commit is contained in:
Joseph Doherty
2026-09-09 11:28:01 -04:00
parent befb0e3805
commit b73962831c
2 changed files with 29 additions and 35 deletions
@@ -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 /// 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. /// what the budget exists to guarantee — raise it only with a real serialized-size measurement.
/// Step-logs addendum (D7, 2026-09-09): a <c>ScriptLog</c> row per script log line rides the same /// Step-logs addendum (D7, 2026-09-09): a <c>ScriptLog</c> row per script log line rides the same
/// sink via <see cref="AddLowPriority"/>, a lower-priority lane that shares the same /// sink via <see cref="Add"/>'s <c>lowPriority</c> parameter — a lower-priority lane that shares
/// <see cref="_events"/>/<see cref="_bytes"/> accounting but stops <see cref="LowPriorityReserveEvents"/> /// the same event list/byte accounting but stops <see cref="LowPriorityReserveEvents"/> events
/// events short of the count cap and at <see cref="LowPriorityByteShare"/> of the byte budget — so /// short of the count cap and at <see cref="LowPriorityByteShare"/> of the byte budget — so the
/// the step rows a run emits at the very end (the <c>ScriptRun</c> anchor is written last) always /// step rows a run emits at the very end (the <c>ScriptRun</c> anchor is written last) always
/// have room, even on a chatty run whose log lines alone would otherwise fill the collector. A /// have room, even on a chatty run whose log lines alone would otherwise fill the collector. A
/// dropped low-priority row sets <see cref="LogsTruncated"/> instead of <see cref="Truncated"/>, /// dropped low-priority row sets <see cref="LogsTruncated"/> instead of <see cref="Truncated"/>,
/// because losing inline log lines does not affect step-row fidelity the way losing a step row /// 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; public const int DefaultMaxEvents = 500;
/// <summary> /// <summary>
/// Events reserved for <see cref="Add"/> (step rows) — <see cref="AddLowPriority"/> (log /// Events reserved for full-priority step rows — a low-priority (log line) <see cref="Add"/>
/// lines) stops this many events short of the event cap (D7, 2026-09-09). /// stops this many events short of the event cap (D7, 2026-09-09).
/// </summary> /// </summary>
public const int LowPriorityReserveEvents = 50; public const int LowPriorityReserveEvents = 50;
/// <summary> /// <summary>
/// Share of the byte budget <see cref="AddLowPriority"/> (log lines) may consume — the /// Share of the byte budget a low-priority (log line) <see cref="Add"/> may consume — the
/// remainder is reserved for <see cref="Add"/> (step rows) (D7, 2026-09-09). /// remainder is reserved for full-priority step rows (D7, 2026-09-09).
/// </summary> /// </summary>
public const double LowPriorityByteShare = 0.75; public const double LowPriorityByteShare = 0.75;
@@ -75,7 +75,7 @@ public sealed class FlowTraceCollector
public bool Truncated { get; private set; } public bool Truncated { get; private set; }
/// <summary> /// <summary>
/// True once an <see cref="AddLowPriority"/> (log line) was dropped because the low-priority /// True once a low-priority (log line) <see cref="Add"/> was dropped because the low-priority
/// event or byte share was reached — reported separately from <see cref="Truncated"/> because /// event or byte share was reached — reported separately from <see cref="Truncated"/> because
/// step-row fidelity is unaffected (D7, 2026-09-09). /// step-row fidelity is unaffected (D7, 2026-09-09).
/// </summary> /// </summary>
@@ -105,22 +105,16 @@ public sealed class FlowTraceCollector
} }
/// <summary> /// <summary>
/// Appends the REDACTED copy of a step event, or marks the collector truncated once the /// Appends the REDACTED copy of an event, or marks the collector truncated once the event
/// event cap or the byte budget is reached. A no-op after <see cref="Complete"/>. Full /// cap or the byte budget is reached. A no-op after <see cref="Complete"/>.
/// priority — never yields to a log line queued via <see cref="AddLowPriority"/>.
/// </summary> /// </summary>
public void Add(AuditEvent evt) => AddCore(evt, lowPriority: false); /// <param name="evt">The event to append.</param>
/// <param name="lowPriority">
/// <summary> /// True for a log-line row — stops at the reserve/byte share and sets
/// Appends the REDACTED copy of a <c>ScriptLog</c> event at LOW priority (D7, 2026-09-09): /// <see cref="LogsTruncated"/> instead of <see cref="Truncated"/>, so the step rows emitted
/// it stops <see cref="LowPriorityReserveEvents"/> events short of the event cap and at /// at run end are never crowded out (D7). False (default) for a full-priority step row.
/// <see cref="LowPriorityByteShare"/> of the byte budget, so log lines never crowd out the /// </param>
/// step rows a run emits at the end. A dropped row sets <see cref="LogsTruncated"/>, not public void Add(AuditEvent evt, bool lowPriority = false)
/// <see cref="Truncated"/>. A no-op after <see cref="Complete"/>.
/// </summary>
public void AddLowPriority(AuditEvent evt) => AddCore(evt, lowPriority: true);
private void AddCore(AuditEvent evt, bool lowPriority)
{ {
ArgumentNullException.ThrowIfNull(evt); ArgumentNullException.ThrowIfNull(evt);
// Redaction runs OUTSIDE the lock (arch-review F1): IAuditRedactor is a pure function // Redaction runs OUTSIDE the lock (arch-review F1): IAuditRedactor is a pure function
@@ -139,10 +139,10 @@ public class FlowTraceCollectorTests
/// <summary>D7 (2026-09-09): the low-priority lane stops short of the event cap so the /// <summary>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.</summary> /// step rows a run emits at the end always have room.</summary>
[Fact] [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 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.Equal(10, c.Count);
Assert.True(c.LogsTruncated); Assert.True(c.LogsTruncated);
@@ -155,11 +155,11 @@ public class FlowTraceCollectorTests
/// <summary>D7 (2026-09-09): the low-priority lane also stops short of the byte budget.</summary> /// <summary>D7 (2026-09-09): the low-priority lane also stops short of the byte budget.</summary>
[Fact] [Fact]
public void AddLowPriority_RespectsByteShare() public void Add_LowPriority_RespectsByteShare()
{ {
var c = new FlowTraceCollector(maxEvents: 500, maxBytes: 1_000); // 75% = 750 chars var c = new FlowTraceCollector(maxEvents: 500, maxBytes: 1_000); // 75% = 750 chars
c.AddLowPriority(Evt(details: new string('x', 600))); c.Add(Evt(details: new string('x', 600)), lowPriority: true);
c.AddLowPriority(Evt(details: new string('x', 200))); c.Add(Evt(details: new string('x', 200)), lowPriority: true);
Assert.Equal(1, c.Count); Assert.Equal(1, c.Count);
Assert.True(c.LogsTruncated); Assert.True(c.LogsTruncated);
@@ -167,15 +167,15 @@ public class FlowTraceCollectorTests
/// <summary>D7 (2026-09-09): the reserve floors at 1, so a tiny cap still admits one log line.</summary> /// <summary>D7 (2026-09-09): the reserve floors at 1, so a tiny cap still admits one log line.</summary>
[Fact] [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 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.Equal(1, c.Count);
Assert.False(c.LogsTruncated); Assert.False(c.LogsTruncated);
c.AddLowPriority(Evt()); c.Add(Evt(), lowPriority: true);
Assert.Equal(1, c.Count); Assert.Equal(1, c.Count);
Assert.True(c.LogsTruncated); Assert.True(c.LogsTruncated);
} }
@@ -183,13 +183,13 @@ public class FlowTraceCollectorTests
/// <summary>D7 (2026-09-09): <see cref="FlowTraceCollector.Complete"/> closes the low-priority /// <summary>D7 (2026-09-09): <see cref="FlowTraceCollector.Complete"/> closes the low-priority
/// lane too — a late log line is dropped silently, not counted as truncated.</summary> /// lane too — a late log line is dropped silently, not counted as truncated.</summary>
[Fact] [Fact]
public void AddLowPriority_IsNoOpAfterComplete() public void Add_LowPriority_IsNoOpAfterComplete()
{ {
var c = new FlowTraceCollector(); var c = new FlowTraceCollector();
c.AddLowPriority(Evt()); c.Add(Evt(), lowPriority: true);
c.Complete(); c.Complete();
c.AddLowPriority(Evt()); c.Add(Evt(), lowPriority: true);
Assert.Equal(1, c.Count); Assert.Equal(1, c.Count);
Assert.False(c.LogsTruncated); Assert.False(c.LogsTruncated);