From e6c15250ce352ff5769ae4656d8935a3da925f40 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 17 Jun 2026 20:00:43 -0400 Subject: [PATCH] =?UTF-8?q?refactor(kpi):=20K2/K6/K7=20review=20fixups=20?= =?UTF-8?q?=E2=80=94=20empty-batch=20guard=20+=20sealed=20repo=20+=20unifo?= =?UTF-8?q?rm=20TryAddEnumerable=20+=20KPI-age=20doc=20fidelity=20+=20cove?= =?UTF-8?q?rage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../KpiSampleEntityTypeConfiguration.cs | 2 ++ .../Repositories/KpiHistoryRepository.cs | 8 +++++++- .../Kpi/NotificationOutboxKpiSampleSource.cs | 4 +++- .../ServiceCollectionExtensions.cs | 5 ++++- .../Kpi/SiteCallAuditKpiSampleSource.cs | 12 ++++++++---- .../ServiceCollectionExtensions.cs | 5 ++++- .../Repositories/KpiHistoryRepositoryTests.cs | 18 ++++++++++++++++++ 7 files changed, 46 insertions(+), 8 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Configurations/KpiSampleEntityTypeConfiguration.cs b/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Configurations/KpiSampleEntityTypeConfiguration.cs index abcb5ef6..e8fc015b 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Configurations/KpiSampleEntityTypeConfiguration.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Configurations/KpiSampleEntityTypeConfiguration.cs @@ -45,6 +45,8 @@ public class KpiSampleEntityTypeConfiguration : IEntityTypeConfiguration new { s.Source, s.Metric, s.Scope, s.ScopeKey, s.CapturedAtUtc }) .HasDatabaseName("IX_KpiSample_Series"); diff --git a/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/KpiHistoryRepository.cs b/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/KpiHistoryRepository.cs index bf06bfcf..bbc4d55d 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/KpiHistoryRepository.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/KpiHistoryRepository.cs @@ -10,7 +10,7 @@ namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Repositories; /// KpiSample table (M6 "KPI History & Trends"). See the interface for the /// contract; this class adds notes on the data-access strategy per method. /// -public class KpiHistoryRepository : IKpiHistoryRepository +public sealed class KpiHistoryRepository : IKpiHistoryRepository { private readonly ScadaBridgeDbContext _context; @@ -29,6 +29,12 @@ public class KpiHistoryRepository : IKpiHistoryRepository { ArgumentNullException.ThrowIfNull(samples); + // Avoid a no-op SaveChanges round-trip on quiet sampling ticks. + if (samples.Count == 0) + { + return; + } + // Bulk-insert one sampling pass. AddRange + a single SaveChanges keeps the // whole batch in one round-trip; the store assigns each row's identity. _context.KpiSamples.AddRange(samples); diff --git a/src/ZB.MOM.WW.ScadaBridge.NotificationOutbox/Kpi/NotificationOutboxKpiSampleSource.cs b/src/ZB.MOM.WW.ScadaBridge.NotificationOutbox/Kpi/NotificationOutboxKpiSampleSource.cs index e678a116..e0b0174b 100644 --- a/src/ZB.MOM.WW.ScadaBridge.NotificationOutbox/Kpi/NotificationOutboxKpiSampleSource.cs +++ b/src/ZB.MOM.WW.ScadaBridge.NotificationOutbox/Kpi/NotificationOutboxKpiSampleSource.cs @@ -20,7 +20,9 @@ namespace ZB.MOM.WW.ScadaBridge.NotificationOutbox.Kpi; /// capturedAtUtc rather than wall-clock now: the stuck cutoff is /// capturedAtUtc - and the /// delivered window is capturedAtUtc - . -/// So a sample captured at the same instant equals the live tile. +/// The COUNT metrics (queueDepth, stuckCount, parkedCount, deliveredLastInterval) equal the +/// live tile at the same instant; the oldestPendingAgeSeconds metric is computed against +/// the repository's internal clock and may differ from the live tile by the query-execution latency. /// /// /// Emits Global (ScopeKey == null), per-Site (ScopeKey == SourceSiteId), and diff --git a/src/ZB.MOM.WW.ScadaBridge.NotificationOutbox/ServiceCollectionExtensions.cs b/src/ZB.MOM.WW.ScadaBridge.NotificationOutbox/ServiceCollectionExtensions.cs index 8d8912b8..7aedcb95 100644 --- a/src/ZB.MOM.WW.ScadaBridge.NotificationOutbox/ServiceCollectionExtensions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.NotificationOutbox/ServiceCollectionExtensions.cs @@ -1,4 +1,5 @@ using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Kpi; using ZB.MOM.WW.ScadaBridge.NotificationOutbox.Delivery; using ZB.MOM.WW.ScadaBridge.NotificationOutbox.Kpi; @@ -50,7 +51,9 @@ public static class ServiceCollectionExtensions // KPI history (M6): the recorder singleton enumerates every IKpiSampleSource each // sampling pass to snapshot the outbox delivery KPIs into the central history store. - services.AddScoped(); + // TryAddEnumerable is idempotent — no double-registration if AddNotificationOutbox + // is ever called more than once, matching the AuditLog (K8) idiom. + services.TryAddEnumerable(ServiceDescriptor.Scoped()); return services; } diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteCallAudit/Kpi/SiteCallAuditKpiSampleSource.cs b/src/ZB.MOM.WW.ScadaBridge.SiteCallAudit/Kpi/SiteCallAuditKpiSampleSource.cs index 11031152..3accce76 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteCallAudit/Kpi/SiteCallAuditKpiSampleSource.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteCallAudit/Kpi/SiteCallAuditKpiSampleSource.cs @@ -20,9 +20,12 @@ namespace ZB.MOM.WW.ScadaBridge.SiteCallAudit.Kpi; /// The cutoffs are derived from exactly as the /// live SiteCallAuditActor KPI handlers derive them /// (stuckCutoff = capturedAtUtc - StuckAgeThreshold, -/// intervalSince = capturedAtUtc - KpiInterval) so a sampled value equals -/// the live tile computed at the same instant. The recorder's -/// capturedAtUtc is the single anchor for both cutoffs. +/// intervalSince = capturedAtUtc - KpiInterval). The COUNT metrics +/// (buffered, parked, failedLastInterval, deliveredLastInterval, stuck) equal the +/// live tile at the same instant; the oldestPendingAgeSeconds metric is +/// computed against the repository's internal clock and may differ from the live +/// tile by the query-execution latency. The recorder's capturedAtUtc is +/// the single anchor for both cutoffs. /// /// /// Registered DI-scoped (next to the rest of the Site Call Audit composition) so @@ -33,12 +36,13 @@ namespace ZB.MOM.WW.ScadaBridge.SiteCallAudit.Kpi; public sealed class SiteCallAuditKpiSampleSource : IKpiSampleSource { // ── Metric catalog (the M6-agreed metric-name strings for this source) ── + // Declaration order matches the emission order in AddSnapshot. private const string MetricBuffered = "buffered"; private const string MetricParked = "parked"; private const string MetricFailedLastInterval = "failedLastInterval"; private const string MetricDeliveredLastInterval = "deliveredLastInterval"; - private const string MetricOldestPendingAgeSeconds = "oldestPendingAgeSeconds"; private const string MetricStuck = "stuck"; + private const string MetricOldestPendingAgeSeconds = "oldestPendingAgeSeconds"; private readonly ISiteCallAuditRepository _repository; private readonly SiteCallAuditOptions _options; diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteCallAudit/ServiceCollectionExtensions.cs b/src/ZB.MOM.WW.ScadaBridge.SiteCallAudit/ServiceCollectionExtensions.cs index 9e604cae..58dbaa4c 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteCallAudit/ServiceCollectionExtensions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteCallAudit/ServiceCollectionExtensions.cs @@ -1,4 +1,5 @@ using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Kpi; using ZB.MOM.WW.ScadaBridge.SiteCallAudit.Kpi; @@ -44,7 +45,9 @@ public static class ServiceCollectionExtensions // KPIs (global + per-site + per-node) into the KpiSample history store. // Scoped so each pass resolves a fresh ISiteCallAuditRepository scope, // mirroring the actor's scope-per-message repository access. - services.AddScoped(); + // TryAddEnumerable is idempotent — no double-registration if AddSiteCallAudit + // is ever called more than once, matching the AuditLog (K8) idiom. + services.TryAddEnumerable(ServiceDescriptor.Scoped()); return services; } diff --git a/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/Repositories/KpiHistoryRepositoryTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/Repositories/KpiHistoryRepositoryTests.cs index a9a7d2cc..cfdfc85c 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/Repositories/KpiHistoryRepositoryTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/Repositories/KpiHistoryRepositoryTests.cs @@ -106,6 +106,24 @@ public class KpiHistoryRepositoryTests Assert.Equal(new[] { 1d, 2d }, series.Select(p => p.Value).ToArray()); } + [Fact] + public async Task RecordSamplesAsync_EmptyBatch_IsNoOp_AndGetRawSeriesAsync_EmptyTable_ReturnsEmpty() + { + await using var ctx = NewContext(); + var repo = new KpiHistoryRepository(ctx); + + // Empty batch must not throw (early-return guard, no SaveChanges round-trip). + await repo.RecordSamplesAsync(Array.Empty()); + + // With nothing written, GetRawSeriesAsync must return a non-null empty list. + var series = await repo.GetRawSeriesAsync( + "NotificationOutbox", "queueDepth", "Global", scopeKey: null, + fromUtc: Base, toUtc: Base.AddMinutes(60)); + + Assert.NotNull(series); + Assert.Empty(series); + } + [Fact] public async Task PurgeOlderThanAsync_DeletesOnlyRowsOlderThanCutoff_AndReturnsCount() {