From 8243f61e962e2163bab10daa3897cacde3c0d988 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 21 May 2026 13:46:34 -0400 Subject: [PATCH 1/2] feat(auditlog): per-script-execution correlation id on sync audit rows --- .../Middleware/AuditWriteMiddleware.cs | 4 ++ .../Scripts/AuditingDbCommand.cs | 7 ++- .../Scripts/AuditingDbConnection.cs | 4 ++ .../Scripts/ScriptRuntimeContext.cs | 33 ++++++++++- .../AuditWriteFailureSafetyTests.cs | 3 + .../DatabaseSyncEmissionEndToEndTests.cs | 1 + .../Middleware/AuditWriteMiddlewareTests.cs | 40 +++++++++++++ .../DatabaseCachedWriteEmissionTests.cs | 3 + .../Scripts/DatabaseSyncEmissionTests.cs | 40 ++++++++++++- .../ExternalSystemCachedCallEmissionTests.cs | 3 + .../ExternalSystemCallAuditEmissionTests.cs | 56 ++++++++++++++++++- 11 files changed, 188 insertions(+), 6 deletions(-) diff --git a/src/ScadaLink.InboundAPI/Middleware/AuditWriteMiddleware.cs b/src/ScadaLink.InboundAPI/Middleware/AuditWriteMiddleware.cs index eb0f9c9..2662f3f 100644 --- a/src/ScadaLink.InboundAPI/Middleware/AuditWriteMiddleware.cs +++ b/src/ScadaLink.InboundAPI/Middleware/AuditWriteMiddleware.cs @@ -145,6 +145,10 @@ public sealed class AuditWriteMiddleware OccurredAtUtc = DateTime.UtcNow, Channel = AuditChannel.ApiInbound, Kind = kind, + // Audit Log #23: a fresh per-request correlation id so the + // inbound row carries a request identifier (closes the design + // gap that inbound rows should be correlatable). + CorrelationId = Guid.NewGuid(), Actor = actor, Target = methodName, Status = status, diff --git a/src/ScadaLink.SiteRuntime/Scripts/AuditingDbCommand.cs b/src/ScadaLink.SiteRuntime/Scripts/AuditingDbCommand.cs index 223d294..a056214 100644 --- a/src/ScadaLink.SiteRuntime/Scripts/AuditingDbCommand.cs +++ b/src/ScadaLink.SiteRuntime/Scripts/AuditingDbCommand.cs @@ -37,6 +37,7 @@ internal sealed class AuditingDbCommand : DbCommand private readonly string _siteId; private readonly string _instanceName; private readonly string? _sourceScript; + private readonly Guid _correlationId; private readonly ILogger _logger; private DbConnection? _wrappingConnection; @@ -47,6 +48,7 @@ internal sealed class AuditingDbCommand : DbCommand string siteId, string instanceName, string? sourceScript, + Guid correlationId, ILogger logger) { _inner = inner ?? throw new ArgumentNullException(nameof(inner)); @@ -55,6 +57,7 @@ internal sealed class AuditingDbCommand : DbCommand _siteId = siteId ?? string.Empty; _instanceName = instanceName ?? string.Empty; _sourceScript = sourceScript; + _correlationId = correlationId; _logger = logger ?? throw new ArgumentNullException(nameof(logger)); } @@ -426,7 +429,9 @@ internal sealed class AuditingDbCommand : DbCommand OccurredAtUtc = DateTime.SpecifyKind(occurredAtUtc, DateTimeKind.Utc), Channel = AuditChannel.DbOutbound, Kind = AuditKind.DbWrite, - CorrelationId = null, + // Audit Log #23: the execution-wide correlation id, so all the + // sync ApiCall/DbWrite rows from one script run share an id. + CorrelationId = _correlationId, SourceSiteId = string.IsNullOrEmpty(_siteId) ? null : _siteId, SourceInstanceId = _instanceName, SourceScript = _sourceScript, diff --git a/src/ScadaLink.SiteRuntime/Scripts/AuditingDbConnection.cs b/src/ScadaLink.SiteRuntime/Scripts/AuditingDbConnection.cs index a1e6121..33edf62 100644 --- a/src/ScadaLink.SiteRuntime/Scripts/AuditingDbConnection.cs +++ b/src/ScadaLink.SiteRuntime/Scripts/AuditingDbConnection.cs @@ -36,6 +36,7 @@ internal sealed class AuditingDbConnection : DbConnection private readonly string _siteId; private readonly string _instanceName; private readonly string? _sourceScript; + private readonly Guid _correlationId; private readonly ILogger _logger; public AuditingDbConnection( @@ -45,6 +46,7 @@ internal sealed class AuditingDbConnection : DbConnection string siteId, string instanceName, string? sourceScript, + Guid correlationId, ILogger logger) { _inner = inner ?? throw new ArgumentNullException(nameof(inner)); @@ -53,6 +55,7 @@ internal sealed class AuditingDbConnection : DbConnection _siteId = siteId ?? string.Empty; _instanceName = instanceName ?? string.Empty; _sourceScript = sourceScript; + _correlationId = correlationId; _logger = logger ?? throw new ArgumentNullException(nameof(logger)); } @@ -92,6 +95,7 @@ internal sealed class AuditingDbConnection : DbConnection _siteId, _instanceName, _sourceScript, + _correlationId, _logger); } diff --git a/src/ScadaLink.SiteRuntime/Scripts/ScriptRuntimeContext.cs b/src/ScadaLink.SiteRuntime/Scripts/ScriptRuntimeContext.cs index 250dcc1..1708173 100644 --- a/src/ScadaLink.SiteRuntime/Scripts/ScriptRuntimeContext.cs +++ b/src/ScadaLink.SiteRuntime/Scripts/ScriptRuntimeContext.cs @@ -105,6 +105,21 @@ public class ScriptRuntimeContext /// private readonly ICachedCallTelemetryForwarder? _cachedForwarder; + /// + /// Audit Log #23: the execution-wide audit correlation id. Every sync + /// trust-boundary audit row emitted by this script execution + /// (ApiCall, DbWrite) is stamped with this id so all the + /// rows from one script run can be correlated together. + /// + private readonly Guid _correlationId; + + /// + /// Audit Log #23: the execution-wide audit correlation id. When omitted + /// (tag-change / timer-triggered executions) a fresh id is generated; an + /// inbound caller may supply one to tie the execution to an upstream + /// request. Stamped on the sync ApiCall/DbWrite audit rows + /// this execution emits. + /// public ScriptRuntimeContext( IActorRef instanceActor, IActorRef self, @@ -122,7 +137,8 @@ public class ScriptRuntimeContext string? sourceScript = null, IAuditWriter? auditWriter = null, IOperationTrackingStore? operationTrackingStore = null, - ICachedCallTelemetryForwarder? cachedForwarder = null) + ICachedCallTelemetryForwarder? cachedForwarder = null, + Guid? correlationId = null) { _instanceActor = instanceActor; _self = self; @@ -141,6 +157,7 @@ public class ScriptRuntimeContext _auditWriter = auditWriter; _operationTrackingStore = operationTrackingStore; _cachedForwarder = cachedForwarder; + _correlationId = correlationId ?? Guid.NewGuid(); } /// @@ -241,7 +258,7 @@ public class ScriptRuntimeContext /// ExternalSystem.CachedCall("systemName", "methodName", params) /// public ExternalSystemHelper ExternalSystem => new( - _externalSystemClient, _instanceName, _logger, _auditWriter, _siteId, _sourceScript, + _externalSystemClient, _instanceName, _logger, _correlationId, _auditWriter, _siteId, _sourceScript, // Audit Log #23 (M3 Bundle E — Task E3): emit CachedSubmit telemetry // on every ExternalSystem.CachedCall enqueue. _cachedForwarder); @@ -255,6 +272,7 @@ public class ScriptRuntimeContext _databaseGateway, _instanceName, _logger, + _correlationId, // Audit Log #23 (M4 Bundle A): wire the IAuditWriter so // Database.Connection(name) returns an auditing decorator that // emits one DbOutbound/DbWrite row per script-initiated @@ -362,6 +380,7 @@ public class ScriptRuntimeContext private readonly IExternalSystemClient? _client; private readonly string _instanceName; private readonly ILogger _logger; + private readonly Guid _correlationId; private readonly IAuditWriter? _auditWriter; private readonly string _siteId; private readonly string? _sourceScript; @@ -374,6 +393,7 @@ public class ScriptRuntimeContext IExternalSystemClient? client, string instanceName, ILogger logger, + Guid correlationId, IAuditWriter? auditWriter = null, string siteId = "", string? sourceScript = null, @@ -382,6 +402,7 @@ public class ScriptRuntimeContext _client = client; _instanceName = instanceName; _logger = logger; + _correlationId = correlationId; _auditWriter = auditWriter; _siteId = siteId; _sourceScript = sourceScript; @@ -882,7 +903,9 @@ public class ScriptRuntimeContext OccurredAtUtc = DateTime.SpecifyKind(occurredAtUtc, DateTimeKind.Utc), Channel = AuditChannel.ApiOutbound, Kind = AuditKind.ApiCall, - CorrelationId = null, + // Audit Log #23: the execution-wide correlation id, so all the + // sync ApiCall/DbWrite rows from one script run share an id. + CorrelationId = _correlationId, SourceSiteId = string.IsNullOrEmpty(_siteId) ? null : _siteId, SourceInstanceId = _instanceName, SourceScript = _sourceScript, @@ -949,6 +972,7 @@ public class ScriptRuntimeContext private readonly IDatabaseGateway? _gateway; private readonly string _instanceName; private readonly ILogger _logger; + private readonly Guid _correlationId; private readonly string _siteId; private readonly string? _sourceScript; private readonly ICachedCallTelemetryForwarder? _cachedForwarder; @@ -969,6 +993,7 @@ public class ScriptRuntimeContext IDatabaseGateway? gateway, string instanceName, ILogger logger, + Guid correlationId, IAuditWriter? auditWriter = null, string siteId = "", string? sourceScript = null, @@ -977,6 +1002,7 @@ public class ScriptRuntimeContext _gateway = gateway; _instanceName = instanceName; _logger = logger; + _correlationId = correlationId; _auditWriter = auditWriter; _siteId = siteId; _sourceScript = sourceScript; @@ -1011,6 +1037,7 @@ public class ScriptRuntimeContext siteId: _siteId, instanceName: _instanceName, sourceScript: _sourceScript, + correlationId: _correlationId, logger: _logger); } diff --git a/tests/ScadaLink.AuditLog.Tests/Integration/AuditWriteFailureSafetyTests.cs b/tests/ScadaLink.AuditLog.Tests/Integration/AuditWriteFailureSafetyTests.cs index 7c505a8..9399fb7 100644 --- a/tests/ScadaLink.AuditLog.Tests/Integration/AuditWriteFailureSafetyTests.cs +++ b/tests/ScadaLink.AuditLog.Tests/Integration/AuditWriteFailureSafetyTests.cs @@ -150,6 +150,7 @@ public class AuditWriteFailureSafetyTests : TestKit, IClassFixture + { + ctx.Response.StatusCode = 200; + return Task.CompletedTask; + }, writer); + + await mw.InvokeAsync(ctx); + + var evt = Assert.Single(writer.Events); + Assert.NotNull(evt.CorrelationId); + Assert.NotEqual(Guid.Empty, evt.CorrelationId!.Value); + } + + [Fact] + public async Task SeparateRequests_GetDistinct_CorrelationIds() + { + var writer = new RecordingAuditWriter(); + var mw = CreateMiddleware(hc => + { + hc.Response.StatusCode = 200; + return Task.CompletedTask; + }, writer); + + await mw.InvokeAsync(BuildContext()); + await mw.InvokeAsync(BuildContext()); + + Assert.Equal(2, writer.Events.Count); + Assert.NotEqual(writer.Events[0].CorrelationId, writer.Events[1].CorrelationId); + } + [Fact] public async Task DurationMs_IsRecorded() { diff --git a/tests/ScadaLink.SiteRuntime.Tests/Scripts/DatabaseCachedWriteEmissionTests.cs b/tests/ScadaLink.SiteRuntime.Tests/Scripts/DatabaseCachedWriteEmissionTests.cs index 991bbaf..042a031 100644 --- a/tests/ScadaLink.SiteRuntime.Tests/Scripts/DatabaseCachedWriteEmissionTests.cs +++ b/tests/ScadaLink.SiteRuntime.Tests/Scripts/DatabaseCachedWriteEmissionTests.cs @@ -47,6 +47,9 @@ public class DatabaseCachedWriteEmissionTests gateway, InstanceName, NullLogger.Instance, + // Audit Log #23: execution-wide correlation id. Cached rows keep + // CorrelationId = TrackedOperationId, so any value works here. + Guid.NewGuid(), siteId: SiteId, sourceScript: SourceScript, cachedForwarder: forwarder); diff --git a/tests/ScadaLink.SiteRuntime.Tests/Scripts/DatabaseSyncEmissionTests.cs b/tests/ScadaLink.SiteRuntime.Tests/Scripts/DatabaseSyncEmissionTests.cs index 2f6f9df..40f2986 100644 --- a/tests/ScadaLink.SiteRuntime.Tests/Scripts/DatabaseSyncEmissionTests.cs +++ b/tests/ScadaLink.SiteRuntime.Tests/Scripts/DatabaseSyncEmissionTests.cs @@ -48,14 +48,28 @@ public class DatabaseSyncEmissionTests private const string SourceScript = "ScriptActor:Sync"; private const string ConnectionName = "machineData"; + /// + /// Audit Log #23: a fixed execution-wide correlation id used by the + /// default + /// overload so assertions can compare against a known value. + /// + private static readonly Guid TestCorrelationId = Guid.NewGuid(); + private static ScriptRuntimeContext.DatabaseHelper CreateHelper( IDatabaseGateway gateway, IAuditWriter? auditWriter) + => CreateHelper(gateway, auditWriter, TestCorrelationId); + + private static ScriptRuntimeContext.DatabaseHelper CreateHelper( + IDatabaseGateway gateway, + IAuditWriter? auditWriter, + Guid correlationId) { return new ScriptRuntimeContext.DatabaseHelper( gateway, InstanceName, NullLogger.Instance, + correlationId, auditWriter: auditWriter, siteId: SiteId, sourceScript: SourceScript, @@ -268,10 +282,34 @@ public class DatabaseSyncEmissionTests Assert.Equal(SourceScript, evt.SourceScript); // Outbound channel: Actor carries the calling script identity. Assert.Equal(SourceScript, evt.Actor); - Assert.Null(evt.CorrelationId); + // Audit Log #23: the sync DbWrite row now carries the execution-wide + // correlation id the helper was constructed with. + Assert.Equal(TestCorrelationId, evt.CorrelationId); Assert.NotEqual(Guid.Empty, evt.EventId); } + [Fact] + public async Task SyncDbWrite_StampsExecutionCorrelationId() + { + using var keepAlive = new SqliteConnection("Data Source=kc;Mode=Memory;Cache=Shared"); + var inner = NewInMemoryDb(out var _); + var gateway = new Mock(); + gateway + .Setup(g => g.GetConnectionAsync(ConnectionName, It.IsAny())) + .ReturnsAsync(inner); + var writer = new CapturingAuditWriter(); + var correlationId = Guid.NewGuid(); + + var helper = CreateHelper(gateway.Object, writer, correlationId); + await using var conn = await helper.Connection(ConnectionName); + await using var cmd = conn.CreateCommand(); + cmd.CommandText = "INSERT INTO t (id, name) VALUES (7, 'eta')"; + await cmd.ExecuteNonQueryAsync(); + + var evt = Assert.Single(writer.Events); + Assert.Equal(correlationId, evt.CorrelationId); + } + [Fact] public async Task DurationMs_NonZero() { diff --git a/tests/ScadaLink.SiteRuntime.Tests/Scripts/ExternalSystemCachedCallEmissionTests.cs b/tests/ScadaLink.SiteRuntime.Tests/Scripts/ExternalSystemCachedCallEmissionTests.cs index 7516822..f2edfa8 100644 --- a/tests/ScadaLink.SiteRuntime.Tests/Scripts/ExternalSystemCachedCallEmissionTests.cs +++ b/tests/ScadaLink.SiteRuntime.Tests/Scripts/ExternalSystemCachedCallEmissionTests.cs @@ -49,6 +49,9 @@ public class ExternalSystemCachedCallEmissionTests client, InstanceName, NullLogger.Instance, + // Audit Log #23: execution-wide correlation id. Cached rows keep + // CorrelationId = TrackedOperationId, so any value works here. + Guid.NewGuid(), auditWriter: null, siteId: SiteId, sourceScript: SourceScript, diff --git a/tests/ScadaLink.SiteRuntime.Tests/Scripts/ExternalSystemCallAuditEmissionTests.cs b/tests/ScadaLink.SiteRuntime.Tests/Scripts/ExternalSystemCallAuditEmissionTests.cs index d021181..209946e 100644 --- a/tests/ScadaLink.SiteRuntime.Tests/Scripts/ExternalSystemCallAuditEmissionTests.cs +++ b/tests/ScadaLink.SiteRuntime.Tests/Scripts/ExternalSystemCallAuditEmissionTests.cs @@ -45,14 +45,28 @@ public class ExternalSystemCallAuditEmissionTests private const string InstanceName = "Plant.Pump42"; private const string SourceScript = "ScriptActor:CheckPressure"; + /// + /// Audit Log #23: a fixed execution-wide correlation id used by the + /// default + /// overload so assertions can compare against a known value. + /// + private static readonly Guid TestCorrelationId = Guid.NewGuid(); + private static ScriptRuntimeContext.ExternalSystemHelper CreateHelper( IExternalSystemClient client, IAuditWriter? auditWriter) + => CreateHelper(client, auditWriter, TestCorrelationId); + + private static ScriptRuntimeContext.ExternalSystemHelper CreateHelper( + IExternalSystemClient client, + IAuditWriter? auditWriter, + Guid correlationId) { return new ScriptRuntimeContext.ExternalSystemHelper( client, InstanceName, NullLogger.Instance, + correlationId, auditWriter, SiteId, SourceScript); @@ -211,7 +225,47 @@ public class ExternalSystemCallAuditEmissionTests Assert.Equal(SourceScript, evt.SourceScript); // Outbound channel: Actor carries the calling script identity. Assert.Equal(SourceScript, evt.Actor); - Assert.Null(evt.CorrelationId); + // Audit Log #23: the sync ApiCall row now carries the execution-wide + // correlation id the helper was constructed with. + Assert.Equal(TestCorrelationId, evt.CorrelationId); + } + + [Fact] + public async Task Call_SyncApiCall_StampsExecutionCorrelationId() + { + var client = new Mock(); + client + .Setup(c => c.CallAsync("ERP", "GetOrder", It.IsAny?>(), It.IsAny())) + .ReturnsAsync(new ExternalCallResult(true, "{}", null)); + var writer = new CapturingAuditWriter(); + var correlationId = Guid.NewGuid(); + + var helper = CreateHelper(client.Object, writer, correlationId); + await helper.Call("ERP", "GetOrder"); + + var evt = Assert.Single(writer.Events); + Assert.Equal(correlationId, evt.CorrelationId); + } + + [Fact] + public async Task Call_TwoCallsOnSameHelper_ShareTheSameCorrelationId() + { + var client = new Mock(); + client + .Setup(c => c.CallAsync(It.IsAny(), It.IsAny(), It.IsAny?>(), It.IsAny())) + .ReturnsAsync(new ExternalCallResult(true, "{}", null)); + var writer = new CapturingAuditWriter(); + var correlationId = Guid.NewGuid(); + + var helper = CreateHelper(client.Object, writer, correlationId); + await helper.Call("ERP", "GetOrder"); + await helper.Call("ERP", "GetCustomer"); + + Assert.Equal(2, writer.Events.Count); + // Both sync ApiCall rows from one execution carry the same id. + Assert.Equal(correlationId, writer.Events[0].CorrelationId); + Assert.Equal(correlationId, writer.Events[1].CorrelationId); + Assert.Equal(writer.Events[0].CorrelationId, writer.Events[1].CorrelationId); } [Fact] From aadb1fd72a75e304e822e02b824eee99ff6115c8 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 21 May 2026 13:57:17 -0400 Subject: [PATCH 2/2] refactor(auditlog): rename audit correlation field, add cross-helper tests --- .../Middleware/AuditWriteMiddleware.cs | 7 + .../Scripts/AuditingDbCommand.cs | 18 +- .../Scripts/AuditingDbConnection.cs | 15 +- .../Scripts/ScriptRuntimeContext.cs | 41 ++-- .../ExecutionCorrelationContextTests.cs | 179 ++++++++++++++++++ 5 files changed, 232 insertions(+), 28 deletions(-) create mode 100644 tests/ScadaLink.SiteRuntime.Tests/Scripts/ExecutionCorrelationContextTests.cs diff --git a/src/ScadaLink.InboundAPI/Middleware/AuditWriteMiddleware.cs b/src/ScadaLink.InboundAPI/Middleware/AuditWriteMiddleware.cs index 2662f3f..c271d03 100644 --- a/src/ScadaLink.InboundAPI/Middleware/AuditWriteMiddleware.cs +++ b/src/ScadaLink.InboundAPI/Middleware/AuditWriteMiddleware.cs @@ -148,6 +148,13 @@ public sealed class AuditWriteMiddleware // Audit Log #23: a fresh per-request correlation id so the // inbound row carries a request identifier (closes the design // gap that inbound rows should be correlatable). + // + // This id is intentionally request-local: it is NOT bridged to + // RouteHelper's routed-call correlation id or to + // HttpContext.TraceIdentifier. Threading an inbound request's + // correlation id through to the routed script execution (so an + // inbound call and the outbound API/DB rows it triggers share + // one id) is a deliberate future follow-up, out of scope here. CorrelationId = Guid.NewGuid(), Actor = actor, Target = methodName, diff --git a/src/ScadaLink.SiteRuntime/Scripts/AuditingDbCommand.cs b/src/ScadaLink.SiteRuntime/Scripts/AuditingDbCommand.cs index a056214..3e59ecf 100644 --- a/src/ScadaLink.SiteRuntime/Scripts/AuditingDbCommand.cs +++ b/src/ScadaLink.SiteRuntime/Scripts/AuditingDbCommand.cs @@ -37,10 +37,13 @@ internal sealed class AuditingDbCommand : DbCommand private readonly string _siteId; private readonly string _instanceName; private readonly string? _sourceScript; - private readonly Guid _correlationId; + private readonly Guid _auditCorrelationId; private readonly ILogger _logger; private DbConnection? _wrappingConnection; + // Parameter ordering: auditCorrelationId sits immediately after the ILogger, + // consistent with the other three audit-threaded ctors (ExternalSystemHelper, + // DatabaseHelper, AuditingDbConnection). public AuditingDbCommand( DbCommand inner, IAuditWriter auditWriter, @@ -48,8 +51,8 @@ internal sealed class AuditingDbCommand : DbCommand string siteId, string instanceName, string? sourceScript, - Guid correlationId, - ILogger logger) + ILogger logger, + Guid auditCorrelationId) { _inner = inner ?? throw new ArgumentNullException(nameof(inner)); _auditWriter = auditWriter ?? throw new ArgumentNullException(nameof(auditWriter)); @@ -57,8 +60,8 @@ internal sealed class AuditingDbCommand : DbCommand _siteId = siteId ?? string.Empty; _instanceName = instanceName ?? string.Empty; _sourceScript = sourceScript; - _correlationId = correlationId; _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _auditCorrelationId = auditCorrelationId; } // -- Forwarded surface ------------------------------------------------ @@ -429,9 +432,10 @@ internal sealed class AuditingDbCommand : DbCommand OccurredAtUtc = DateTime.SpecifyKind(occurredAtUtc, DateTimeKind.Utc), Channel = AuditChannel.DbOutbound, Kind = AuditKind.DbWrite, - // Audit Log #23: the execution-wide correlation id, so all the - // sync ApiCall/DbWrite rows from one script run share an id. - CorrelationId = _correlationId, + // Audit Log #23: the execution-wide correlation id, so this sync + // DbWrite row shares an id with the other sync trust-boundary rows + // from the same script run. + CorrelationId = _auditCorrelationId, SourceSiteId = string.IsNullOrEmpty(_siteId) ? null : _siteId, SourceInstanceId = _instanceName, SourceScript = _sourceScript, diff --git a/src/ScadaLink.SiteRuntime/Scripts/AuditingDbConnection.cs b/src/ScadaLink.SiteRuntime/Scripts/AuditingDbConnection.cs index 33edf62..a8d0ee0 100644 --- a/src/ScadaLink.SiteRuntime/Scripts/AuditingDbConnection.cs +++ b/src/ScadaLink.SiteRuntime/Scripts/AuditingDbConnection.cs @@ -36,9 +36,12 @@ internal sealed class AuditingDbConnection : DbConnection private readonly string _siteId; private readonly string _instanceName; private readonly string? _sourceScript; - private readonly Guid _correlationId; + private readonly Guid _auditCorrelationId; private readonly ILogger _logger; + // Parameter ordering: auditCorrelationId sits immediately after the ILogger, + // consistent with the other three audit-threaded ctors (ExternalSystemHelper, + // DatabaseHelper, AuditingDbCommand). public AuditingDbConnection( DbConnection inner, IAuditWriter auditWriter, @@ -46,8 +49,8 @@ internal sealed class AuditingDbConnection : DbConnection string siteId, string instanceName, string? sourceScript, - Guid correlationId, - ILogger logger) + ILogger logger, + Guid auditCorrelationId) { _inner = inner ?? throw new ArgumentNullException(nameof(inner)); _auditWriter = auditWriter ?? throw new ArgumentNullException(nameof(auditWriter)); @@ -55,8 +58,8 @@ internal sealed class AuditingDbConnection : DbConnection _siteId = siteId ?? string.Empty; _instanceName = instanceName ?? string.Empty; _sourceScript = sourceScript; - _correlationId = correlationId; _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _auditCorrelationId = auditCorrelationId; } // ConnectionString is settable on DbConnection — forward both halves. @@ -95,8 +98,8 @@ internal sealed class AuditingDbConnection : DbConnection _siteId, _instanceName, _sourceScript, - _correlationId, - _logger); + _logger, + _auditCorrelationId); } protected override void Dispose(bool disposing) diff --git a/src/ScadaLink.SiteRuntime/Scripts/ScriptRuntimeContext.cs b/src/ScadaLink.SiteRuntime/Scripts/ScriptRuntimeContext.cs index 1708173..ed643ef 100644 --- a/src/ScadaLink.SiteRuntime/Scripts/ScriptRuntimeContext.cs +++ b/src/ScadaLink.SiteRuntime/Scripts/ScriptRuntimeContext.cs @@ -111,9 +111,9 @@ public class ScriptRuntimeContext /// (ApiCall, DbWrite) is stamped with this id so all the /// rows from one script run can be correlated together. /// - private readonly Guid _correlationId; + private readonly Guid _auditCorrelationId; - /// + /// /// Audit Log #23: the execution-wide audit correlation id. When omitted /// (tag-change / timer-triggered executions) a fresh id is generated; an /// inbound caller may supply one to tie the execution to an upstream @@ -138,7 +138,7 @@ public class ScriptRuntimeContext IAuditWriter? auditWriter = null, IOperationTrackingStore? operationTrackingStore = null, ICachedCallTelemetryForwarder? cachedForwarder = null, - Guid? correlationId = null) + Guid? auditCorrelationId = null) { _instanceActor = instanceActor; _self = self; @@ -157,7 +157,7 @@ public class ScriptRuntimeContext _auditWriter = auditWriter; _operationTrackingStore = operationTrackingStore; _cachedForwarder = cachedForwarder; - _correlationId = correlationId ?? Guid.NewGuid(); + _auditCorrelationId = auditCorrelationId ?? Guid.NewGuid(); } /// @@ -258,7 +258,7 @@ public class ScriptRuntimeContext /// ExternalSystem.CachedCall("systemName", "methodName", params) /// public ExternalSystemHelper ExternalSystem => new( - _externalSystemClient, _instanceName, _logger, _correlationId, _auditWriter, _siteId, _sourceScript, + _externalSystemClient, _instanceName, _logger, _auditCorrelationId, _auditWriter, _siteId, _sourceScript, // Audit Log #23 (M3 Bundle E — Task E3): emit CachedSubmit telemetry // on every ExternalSystem.CachedCall enqueue. _cachedForwarder); @@ -272,7 +272,7 @@ public class ScriptRuntimeContext _databaseGateway, _instanceName, _logger, - _correlationId, + _auditCorrelationId, // Audit Log #23 (M4 Bundle A): wire the IAuditWriter so // Database.Connection(name) returns an auditing decorator that // emits one DbOutbound/DbWrite row per script-initiated @@ -380,7 +380,7 @@ public class ScriptRuntimeContext private readonly IExternalSystemClient? _client; private readonly string _instanceName; private readonly ILogger _logger; - private readonly Guid _correlationId; + private readonly Guid _auditCorrelationId; private readonly IAuditWriter? _auditWriter; private readonly string _siteId; private readonly string? _sourceScript; @@ -389,11 +389,18 @@ public class ScriptRuntimeContext // Internal constructor for tests living in ScadaLink.SiteRuntime.Tests // (via InternalsVisibleTo). Production sites resolve the helper through // ScriptRuntimeContext.ExternalSystem. + // + // Parameter ordering: auditCorrelationId sits immediately after the + // ILogger across all four audit-threaded ctors (ExternalSystemHelper, + // DatabaseHelper, AuditingDbConnection, AuditingDbCommand) — a required + // Guid cannot follow the optional provenance params without a + // required-after-optional compile error, so the post-logger slot is the + // one consistent position that compiles cleanly everywhere. internal ExternalSystemHelper( IExternalSystemClient? client, string instanceName, ILogger logger, - Guid correlationId, + Guid auditCorrelationId, IAuditWriter? auditWriter = null, string siteId = "", string? sourceScript = null, @@ -402,7 +409,7 @@ public class ScriptRuntimeContext _client = client; _instanceName = instanceName; _logger = logger; - _correlationId = correlationId; + _auditCorrelationId = auditCorrelationId; _auditWriter = auditWriter; _siteId = siteId; _sourceScript = sourceScript; @@ -905,7 +912,7 @@ public class ScriptRuntimeContext Kind = AuditKind.ApiCall, // Audit Log #23: the execution-wide correlation id, so all the // sync ApiCall/DbWrite rows from one script run share an id. - CorrelationId = _correlationId, + CorrelationId = _auditCorrelationId, SourceSiteId = string.IsNullOrEmpty(_siteId) ? null : _siteId, SourceInstanceId = _instanceName, SourceScript = _sourceScript, @@ -972,7 +979,7 @@ public class ScriptRuntimeContext private readonly IDatabaseGateway? _gateway; private readonly string _instanceName; private readonly ILogger _logger; - private readonly Guid _correlationId; + private readonly Guid _auditCorrelationId; private readonly string _siteId; private readonly string? _sourceScript; private readonly ICachedCallTelemetryForwarder? _cachedForwarder; @@ -989,11 +996,15 @@ public class ScriptRuntimeContext /// private readonly IAuditWriter? _auditWriter; + // Parameter ordering: auditCorrelationId sits immediately after the + // ILogger — see the note on ExternalSystemHelper's ctor for why the + // post-logger slot is the one consistent position across all four + // audit-threaded ctors. internal DatabaseHelper( IDatabaseGateway? gateway, string instanceName, ILogger logger, - Guid correlationId, + Guid auditCorrelationId, IAuditWriter? auditWriter = null, string siteId = "", string? sourceScript = null, @@ -1002,7 +1013,7 @@ public class ScriptRuntimeContext _gateway = gateway; _instanceName = instanceName; _logger = logger; - _correlationId = correlationId; + _auditCorrelationId = auditCorrelationId; _auditWriter = auditWriter; _siteId = siteId; _sourceScript = sourceScript; @@ -1037,8 +1048,8 @@ public class ScriptRuntimeContext siteId: _siteId, instanceName: _instanceName, sourceScript: _sourceScript, - correlationId: _correlationId, - logger: _logger); + logger: _logger, + auditCorrelationId: _auditCorrelationId); } /// diff --git a/tests/ScadaLink.SiteRuntime.Tests/Scripts/ExecutionCorrelationContextTests.cs b/tests/ScadaLink.SiteRuntime.Tests/Scripts/ExecutionCorrelationContextTests.cs new file mode 100644 index 0000000..13a54dc --- /dev/null +++ b/tests/ScadaLink.SiteRuntime.Tests/Scripts/ExecutionCorrelationContextTests.cs @@ -0,0 +1,179 @@ +using Akka.Actor; +using Microsoft.Data.Sqlite; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using ScadaLink.Commons.Entities.Audit; +using ScadaLink.Commons.Interfaces.Services; +using ScadaLink.Commons.Types.Enums; +using ScadaLink.SiteRuntime.Scripts; + +namespace ScadaLink.SiteRuntime.Tests.Scripts; + +/// +/// Audit Log #23 — execution-correlation tests exercised through a full +/// : +/// +/// +/// +/// The ?? Guid.NewGuid() fallback in the +/// ctor: when no audit correlation id is supplied (tag-change / timer-triggered +/// executions) a fresh, non-empty id is minted and stamped on the emitted rows. +/// +/// +/// The execution-wide contract: an ExternalSystem.Call and a sync +/// Database write performed through ONE context share a single +/// . +/// +/// +/// +public class ExecutionCorrelationContextTests +{ + /// + /// In-memory capturing every emitted event + /// (mirrors the CapturingAuditWriter stubs in + /// / + /// ). + /// + private sealed class CapturingAuditWriter : IAuditWriter + { + public List Events { get; } = new(); + + public Task WriteAsync(AuditEvent evt, CancellationToken ct = default) + { + Events.Add(evt); + return Task.CompletedTask; + } + } + + private const string InstanceName = "Plant.Pump42"; + private const string ConnectionName = "machineData"; + + /// + /// Builds a full wired with the external + /// system client, database gateway and audit writer the cross-helper test + /// needs. The actor refs are — the + /// integration helpers (ExternalSystem / Database) never touch them — and + /// defaults to null so the ctor's + /// ?? Guid.NewGuid() fallback is exercised unless a test supplies one. + /// + private static ScriptRuntimeContext CreateContext( + IExternalSystemClient? externalSystemClient, + IDatabaseGateway? databaseGateway, + IAuditWriter? auditWriter, + Guid? auditCorrelationId = null) + { + var compilationService = new ScriptCompilationService( + NullLogger.Instance); + var sharedScriptLibrary = new SharedScriptLibrary( + compilationService, NullLogger.Instance); + + return new ScriptRuntimeContext( + ActorRefs.Nobody, + ActorRefs.Nobody, + sharedScriptLibrary, + currentCallDepth: 0, + maxCallDepth: 10, + askTimeout: TimeSpan.FromSeconds(5), + instanceName: InstanceName, + logger: NullLogger.Instance, + externalSystemClient: externalSystemClient, + databaseGateway: databaseGateway, + storeAndForward: null, + siteCommunicationActor: null, + siteId: "site-77", + sourceScript: "ScriptActor:OnTick", + auditWriter: auditWriter, + operationTrackingStore: null, + cachedForwarder: null, + auditCorrelationId: auditCorrelationId); + } + + /// + /// Spin up a fresh in-memory SQLite database with a tiny single-table + /// schema. The keep-alive root must outlive any auditing wrapper the test + /// exercises (mirrors DatabaseSyncEmissionTests.NewInMemoryDb). + /// + private static SqliteConnection NewInMemoryDb(out SqliteConnection keepAlive) + { + var dbName = $"db-{Guid.NewGuid():N}"; + var connStr = $"Data Source={dbName};Mode=Memory;Cache=Shared"; + + keepAlive = new SqliteConnection(connStr); + keepAlive.Open(); + using (var seed = keepAlive.CreateCommand()) + { + seed.CommandText = + "CREATE TABLE t (id INTEGER PRIMARY KEY, name TEXT NOT NULL);"; + seed.ExecuteNonQuery(); + } + + var live = new SqliteConnection(connStr); + live.Open(); + return live; + } + + [Fact] + public async Task NoCorrelationIdSupplied_SyncCall_StampsFreshNonEmptyCorrelationId() + { + // No auditCorrelationId argument — the ScriptRuntimeContext ctor's + // `?? Guid.NewGuid()` fallback must mint one (this is the unsupplied-id + // branch every other audit test bypasses by passing an explicit id). + var client = new Mock(); + client + .Setup(c => c.CallAsync("ERP", "GetOrder", It.IsAny?>(), It.IsAny())) + .ReturnsAsync(new ExternalCallResult(true, "{}", null)); + var writer = new CapturingAuditWriter(); + + var context = CreateContext(client.Object, databaseGateway: null, writer); + await context.ExternalSystem.Call("ERP", "GetOrder"); + + var evt = Assert.Single(writer.Events); + Assert.NotNull(evt.CorrelationId); + Assert.NotEqual(Guid.Empty, evt.CorrelationId!.Value); + } + + [Fact] + public async Task SameContext_ApiCallAndDbWrite_ShareTheSameCorrelationId() + { + // The execution-wide contract: an ExternalSystem.Call AND a sync + // Database write performed through ONE ScriptRuntimeContext must both + // carry the same execution correlation id, so an audit reader can tie + // every trust-boundary action from one script run together. + using var keepAlive = new SqliteConnection("Data Source=ecc;Mode=Memory;Cache=Shared"); + var innerDb = NewInMemoryDb(out var _); + + var client = new Mock(); + client + .Setup(c => c.CallAsync("ERP", "GetOrder", It.IsAny?>(), It.IsAny())) + .ReturnsAsync(new ExternalCallResult(true, "{}", null)); + + var gateway = new Mock(); + gateway + .Setup(g => g.GetConnectionAsync(ConnectionName, It.IsAny())) + .ReturnsAsync(innerDb); + + var writer = new CapturingAuditWriter(); + var context = CreateContext(client.Object, gateway.Object, writer); + + // 1) outbound API call through the context's ExternalSystem helper. + await context.ExternalSystem.Call("ERP", "GetOrder"); + + // 2) sync DB write through the SAME context's Database helper. + await using (var conn = await context.Database.Connection(ConnectionName)) + await using (var cmd = conn.CreateCommand()) + { + cmd.CommandText = "INSERT INTO t (id, name) VALUES (1, 'alpha')"; + await cmd.ExecuteNonQueryAsync(); + } + + Assert.Equal(2, writer.Events.Count); + var apiRow = Assert.Single(writer.Events, e => e.Channel == AuditChannel.ApiOutbound); + var dbRow = Assert.Single(writer.Events, e => e.Channel == AuditChannel.DbOutbound); + + Assert.NotNull(apiRow.CorrelationId); + Assert.NotEqual(Guid.Empty, apiRow.CorrelationId!.Value); + // The ApiCall row and the DbWrite row, emitted by two different helpers + // resolved off one context, carry the identical execution correlation id. + Assert.Equal(apiRow.CorrelationId, dbRow.CorrelationId); + } +}