fix(historian): validate non-positive drain/capacity/retention knobs (review) + log prefix

This commit is contained in:
Joseph Doherty
2026-06-11 13:09:13 -04:00
parent f215982b93
commit 5ea6e9d7d9
3 changed files with 34 additions and 2 deletions
@@ -1,5 +1,6 @@
using System.Collections.Generic;
using System.IO;
using ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian;
namespace ZB.MOM.WW.OtOpcUa.Runtime.Historian;
@@ -38,7 +39,7 @@ public sealed class AlarmHistorianOptions
/// <summary>Maximum queued rows before the sink evicts the oldest. Defaults to 1,000,000
/// (matches <c>SqliteStoreAndForwardSink</c>'s <c>DefaultCapacity</c>).</summary>
public long Capacity { get; init; } = 1_000_000;
public long Capacity { get; init; } = SqliteStoreAndForwardSink.DefaultCapacity;
/// <summary>Days to retain dead-lettered rows before purge. Defaults to 30.</summary>
public int DeadLetterRetentionDays { get; init; } = 30;
@@ -54,6 +55,12 @@ public sealed class AlarmHistorianOptions
warnings.Add("AlarmHistorian:SharedSecret is empty while the historian is enabled — the Wonderware sidecar Hello frame will carry an empty secret.");
if (!Path.IsPathRooted(DatabasePath))
warnings.Add($"AlarmHistorian:DatabasePath '{DatabasePath}' is relative — it resolves against the process working directory (e.g. System32 for a Windows service). Set an absolute path.");
if (DrainIntervalSeconds <= 0)
warnings.Add($"AlarmHistorian:DrainIntervalSeconds is {DrainIntervalSeconds} — must be > 0; the drain timer will throw or spin at startup.");
if (Capacity <= 0)
warnings.Add($"AlarmHistorian:Capacity is {Capacity} — must be > 0; the sink constructor will throw at startup.");
if (DeadLetterRetentionDays <= 0)
warnings.Add($"AlarmHistorian:DeadLetterRetentionDays is {DeadLetterRetentionDays} — must be > 0; dead-lettered rows would be purged on every drain tick.");
return warnings;
}
}
@@ -73,7 +73,7 @@ public static class ServiceCollectionExtensions
if (opts is not { Enabled: true }) return services; // leave the Null default from AddOtOpcUaRuntime
foreach (var warning in opts.Validate())
Serilog.Log.Logger.ForContext<SqliteStoreAndForwardSink>().Warning("{HistorianConfigWarning}", warning);
Serilog.Log.Logger.ForContext<SqliteStoreAndForwardSink>().Warning("Historian config: {HistorianConfigWarning}", warning);
services.AddSingleton<IAlarmHistorianSink>(sp =>
{
@@ -137,4 +137,29 @@ public sealed class AlarmHistorianRegistrationTests
{
new AlarmHistorianOptions { Enabled = false, SharedSecret = "" }.Validate().ShouldBeEmpty();
}
[Fact]
public void Validate_warns_on_non_positive_drain_interval()
{
var opts = new AlarmHistorianOptions { Enabled = true, SharedSecret = "s", DatabasePath = "/abs/h.db", DrainIntervalSeconds = 0 };
opts.Validate().ShouldContain(w => w.Contains("DrainIntervalSeconds"));
}
[Fact]
public void Validate_warns_on_non_positive_capacity()
{
var opts = new AlarmHistorianOptions { Enabled = true, SharedSecret = "s", DatabasePath = "/abs/h.db", Capacity = 0 };
opts.Validate().ShouldContain(w => w.Contains("Capacity"));
}
[Fact]
public void Validate_accumulates_multiple_warnings()
{
// relative path + empty secret ⇒ both warnings, not short-circuited on the first.
var opts = new AlarmHistorianOptions { Enabled = true, SharedSecret = "", DatabasePath = "alarm-historian.db" };
var warnings = opts.Validate();
warnings.ShouldContain(w => w.Contains("SharedSecret"));
warnings.ShouldContain(w => w.Contains("DatabasePath"));
warnings.Count.ShouldBeGreaterThanOrEqualTo(2);
}
}