65b19516c8
QueueDepthGaugeTests failed intermittently whenever a sibling test class started a StoreAndForwardService concurrently. `scadabridge.store_and_forward.queue.depth` is an ObservableGauge on ScadaBridgeTelemetry's static meter reading ONE process-global provider slot; every StartAsync writes it, last registrant wins. A concurrent StartAsync clobbered the gauge mid-assertion and the test read the sibling's depth (observed: expected 0, actual 6 — 2 runs in 5 with parallel collections enabled). The global slot cannot be swapped for a per-test meter: three of the six facts assert the slot-ownership semantics themselves (compare-and-clear identity, stop/restart re-seed), so the slot IS their subject. The correct isolation is mutual exclusion — QueueDepthGaugeTests, StoreAndForwardServiceTests and CachedCallAttemptEmissionTests (the three classes that call StartAsync) now share the QueueDepthGaugeCollection, which xUnit runs serially; the other eight classes stay parallel. With real isolation in place the assembly-wide `parallelizeTestCollections: false` blanket switch is no longer needed and is flipped back on, so the isolation is actually exercised rather than papered over. QueueDepthGaugeCollectionGuardTests is a staleness self-test: it scans this project's sources (anchored by [CallerFilePath], not the output dir) for files that both construct and start a service, and fails if one omits the collection attribute — so a new test class cannot silently reintroduce the flake. Negative-controlled: dropping the attribute from StoreAndForwardServiceTests makes the guard fail with the offending file name, and the original gauge flake reappears in the same run. Verification: 8 consecutive full-suite runs green with parallel collections ON (135 tests), plus 5 more after the negative control was reverted. Test-only change; no src/ involvement.
342 lines
15 KiB
C#
342 lines
15 KiB
C#
using System.Diagnostics.Metrics;
|
|
using Microsoft.Extensions.Logging.Abstractions;
|
|
using ZB.MOM.WW.ScadaBridge.Commons.Observability;
|
|
using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
|
|
using ZB.MOM.WW.ScadaBridge.TestSupport;
|
|
|
|
namespace ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests;
|
|
|
|
/// <summary>
|
|
/// WP-14 (telemetry follow-on): verifies the cached buffered-message counter that
|
|
/// backs the <c>scadabridge.store_and_forward.queue.depth</c> observable gauge tracks
|
|
/// the live (Pending) queue across the existing enqueue / drain / park / requeue paths,
|
|
/// and that the sync gauge callback reports it.
|
|
///
|
|
/// The gauge is read the way the OpenTelemetry collector reads it — via a
|
|
/// <see cref="MeterListener"/> that forces an observation (the callback is synchronous
|
|
/// and does no I/O, which is the whole point of caching the count). <see cref="StartAsync"/>
|
|
/// seeds the counter from storage and registers the provider against this service
|
|
/// instance, so the gauge resolves to this test's counter.
|
|
///
|
|
/// Register row 32: that provider slot is process-global, so this class joins
|
|
/// <see cref="QueueDepthGaugeCollection"/> along with every other class in this assembly
|
|
/// that starts a service — see that type for why mutual exclusion (rather than a per-test
|
|
/// meter) is the correct isolation here.
|
|
/// </summary>
|
|
[Collection(QueueDepthGaugeCollection.Name)]
|
|
public class QueueDepthGaugeTests : IAsyncLifetime, IDisposable
|
|
{
|
|
private readonly TestLocalDb _localDb;
|
|
private readonly StoreAndForwardStorage _storage;
|
|
private readonly StoreAndForwardService _service;
|
|
|
|
public QueueDepthGaugeTests()
|
|
{
|
|
_localDb = TestLocalDb.CreateTemp("QueueDepthTests");
|
|
|
|
_storage = new StoreAndForwardStorage(_localDb.Db, NullLogger<StoreAndForwardStorage>.Instance);
|
|
|
|
var options = new StoreAndForwardOptions
|
|
{
|
|
DefaultRetryInterval = TimeSpan.Zero,
|
|
DefaultMaxRetries = 3,
|
|
// Long interval so no background sweep fires on its own during the test;
|
|
// sweeps are driven explicitly via RetryPendingMessagesAsync.
|
|
RetryTimerInterval = TimeSpan.FromMinutes(10)
|
|
};
|
|
|
|
_service = new StoreAndForwardService(
|
|
_storage, options, NullLogger<StoreAndForwardService>.Instance);
|
|
}
|
|
|
|
public async Task InitializeAsync()
|
|
{
|
|
await _storage.InitializeAsync();
|
|
// StartAsync seeds _bufferedCount from the (empty) store and registers the
|
|
// queue-depth provider against this service instance.
|
|
await _service.StartAsync();
|
|
}
|
|
|
|
public async Task DisposeAsync() => await _service.StopAsync();
|
|
|
|
public void Dispose()
|
|
{
|
|
var path = _localDb.Path;
|
|
_localDb.Dispose();
|
|
TestLocalDb.DeleteFiles(path);
|
|
}
|
|
|
|
/// <summary>
|
|
/// Reads the current value of the <c>scadabridge.store_and_forward.queue.depth</c>
|
|
/// gauge by forcing a synchronous observation through a transient MeterListener —
|
|
/// exactly the path the Prometheus/OTLP collector exercises on each scrape.
|
|
/// </summary>
|
|
private static long ReadQueueDepthGauge()
|
|
{
|
|
long observed = -1;
|
|
using var listener = new MeterListener
|
|
{
|
|
InstrumentPublished = (instrument, l) =>
|
|
{
|
|
if (instrument.Meter.Name == ScadaBridgeTelemetry.MeterName &&
|
|
instrument.Name == "scadabridge.store_and_forward.queue.depth")
|
|
{
|
|
l.EnableMeasurementEvents(instrument);
|
|
}
|
|
}
|
|
};
|
|
listener.SetMeasurementEventCallback<long>((_, measurement, _, _) => observed = measurement);
|
|
listener.Start();
|
|
listener.RecordObservableInstruments();
|
|
return observed;
|
|
}
|
|
|
|
[Fact]
|
|
public async Task Gauge_TracksBufferedDepth_AcrossEnqueueDrainAndPark()
|
|
{
|
|
// Empty store seeded at StartAsync → gauge reports 0.
|
|
Assert.Equal(0, ReadQueueDepthGauge());
|
|
|
|
// A handler that fails transiently so each enqueue buffers a Pending row
|
|
// (immediate attempt 0 throws → BufferAsync → +1).
|
|
var deliver = false;
|
|
_service.RegisterDeliveryHandler(StoreAndForwardCategory.ExternalSystem,
|
|
_ =>
|
|
{
|
|
if (!deliver) throw new HttpRequestException("transient");
|
|
return Task.FromResult(true);
|
|
});
|
|
|
|
// Enqueue 3 → cached depth = 3 → gauge reports 3.
|
|
for (var i = 0; i < 3; i++)
|
|
{
|
|
var r = await _service.EnqueueAsync(
|
|
StoreAndForwardCategory.ExternalSystem, "api", """{}""");
|
|
Assert.True(r.WasBuffered);
|
|
}
|
|
Assert.Equal(3, ReadQueueDepthGauge());
|
|
|
|
// Drain: handler now succeeds → the retry sweep removes all 3 Pending rows → depth 0.
|
|
deliver = true;
|
|
await _service.RetryPendingMessagesAsync();
|
|
Assert.Equal(0, ReadQueueDepthGauge());
|
|
|
|
// Park path: buffer one more, then make it park (maxRetries:1 parks after one
|
|
// sweep). Pending→Parked leaves the live queue → depth back to 0.
|
|
deliver = false;
|
|
var parkResult = await _service.EnqueueAsync(
|
|
StoreAndForwardCategory.ExternalSystem, "api", """{}""", maxRetries: 1);
|
|
Assert.True(parkResult.WasBuffered);
|
|
Assert.Equal(1, ReadQueueDepthGauge());
|
|
|
|
await _service.RetryPendingMessagesAsync();
|
|
var parked = await _storage.GetMessageByIdAsync(parkResult.MessageId);
|
|
Assert.Equal(StoreAndForwardMessageStatus.Parked, parked!.Status);
|
|
Assert.Equal(0, ReadQueueDepthGauge());
|
|
|
|
// Operator requeue: Parked→Pending re-adds to the live queue → depth 1.
|
|
Assert.True(await _service.RetryParkedMessageAsync(parkResult.MessageId));
|
|
Assert.Equal(1, ReadQueueDepthGauge());
|
|
}
|
|
|
|
/// <summary>
|
|
/// StoreAndForward-025: after a graceful <see cref="StoreAndForwardService.StopAsync"/>
|
|
/// the service must deregister its queue-depth provider from the process-global gauge
|
|
/// slot, so the gauge stops reporting the stopped instance's (now-frozen) depth and the
|
|
/// provider closure no longer pins the dead service. With the provider cleared the gauge
|
|
/// falls back to 0.
|
|
/// </summary>
|
|
[Fact]
|
|
public async Task StopAsync_ClearsQueueDepthProvider_GaugeFallsBackToZero()
|
|
{
|
|
var fresh = new StoreAndForwardService(
|
|
_storage,
|
|
new StoreAndForwardOptions { RetryTimerInterval = TimeSpan.FromMinutes(10) },
|
|
NullLogger<StoreAndForwardService>.Instance);
|
|
|
|
// Register a Pending row this instance owns, then start so the instance registers
|
|
// its provider and seeds the cached count to 1 → gauge reports 1.
|
|
await _storage.EnqueueAsync(new StoreAndForwardMessage
|
|
{
|
|
Id = Guid.NewGuid().ToString("N"),
|
|
Category = StoreAndForwardCategory.ExternalSystem,
|
|
Target = "api",
|
|
PayloadJson = "{}",
|
|
Status = StoreAndForwardMessageStatus.Pending,
|
|
CreatedAt = DateTimeOffset.UtcNow,
|
|
MaxRetries = 3
|
|
});
|
|
await fresh.StartAsync();
|
|
Assert.Equal(1, ReadQueueDepthGauge());
|
|
|
|
// Graceful stop must deregister the provider; the gauge falls back to 0 rather
|
|
// than reporting this dead instance's frozen depth of 1.
|
|
await fresh.StopAsync();
|
|
Assert.Equal(0, ReadQueueDepthGauge());
|
|
}
|
|
|
|
/// <summary>
|
|
/// StoreAndForward-025 (compare-and-clear): when a newer instance has already
|
|
/// registered its provider into the process-global slot, a late
|
|
/// <see cref="StoreAndForwardService.StopAsync"/> of an older instance must NOT clear
|
|
/// the slot — the identity-checked clear only removes the slot when it still holds the
|
|
/// stopping instance's own delegate. After the late stop the gauge must still report
|
|
/// the newer instance's depth, not 0.
|
|
/// </summary>
|
|
[Fact]
|
|
public async Task StopAsync_DoesNotClobberNewerInstanceProvider()
|
|
{
|
|
// Old instance: starts over an empty store, registers its provider (gauge → 0),
|
|
// then takes a single buffered message so it would report 1 if it stayed live.
|
|
var older = new StoreAndForwardService(
|
|
_storage,
|
|
new StoreAndForwardOptions { RetryTimerInterval = TimeSpan.FromMinutes(10) },
|
|
NullLogger<StoreAndForwardService>.Instance);
|
|
await older.StartAsync();
|
|
older.TestOnly_IncrementBufferedCount(); // older's depth would be 1
|
|
Assert.Equal(1, ReadQueueDepthGauge());
|
|
|
|
// New instance: starts and re-registers into the same global slot, winning it.
|
|
// It seeds from the (empty) store and stands in two buffered messages → depth 2.
|
|
var newer = new StoreAndForwardService(
|
|
_storage,
|
|
new StoreAndForwardOptions { RetryTimerInterval = TimeSpan.FromMinutes(10) },
|
|
NullLogger<StoreAndForwardService>.Instance);
|
|
await newer.StartAsync();
|
|
newer.TestOnly_IncrementBufferedCount();
|
|
newer.TestOnly_IncrementBufferedCount();
|
|
Assert.Equal(2, ReadQueueDepthGauge());
|
|
|
|
// Late stop of the OLDER instance: compare-and-clear must fail the identity check
|
|
// (the slot now holds the newer instance's delegate), so the newer provider stays.
|
|
await older.StopAsync();
|
|
Assert.Equal(2, ReadQueueDepthGauge());
|
|
|
|
// Cleanup: stop the newer instance, which legitimately clears its own provider.
|
|
await newer.StopAsync();
|
|
Assert.Equal(0, ReadQueueDepthGauge());
|
|
}
|
|
|
|
/// <summary>
|
|
/// StoreAndForward-028: a same-instance Stop→Start must re-seed the cached depth from a
|
|
/// clean base. <see cref="StoreAndForwardService.StopAsync"/> resets the one-time
|
|
/// registration guard so a later <see cref="StoreAndForwardService.StartAsync"/>
|
|
/// re-registers and re-seeds <c>_bufferedCount</c> from the durable Pending count; if
|
|
/// StopAsync did not also reset <c>_bufferedCount</c>, the restart would ADD the re-read
|
|
/// count on top of the leftover in-memory value, double-counting the gauge to ~2N.
|
|
/// </summary>
|
|
[Fact]
|
|
public async Task StartAsync_AfterStop_ReseedsFromCleanBase_NoDoubleCount()
|
|
{
|
|
// One durable Pending row that survives the stop/restart in SQLite.
|
|
await _storage.EnqueueAsync(new StoreAndForwardMessage
|
|
{
|
|
Id = Guid.NewGuid().ToString("N"),
|
|
Category = StoreAndForwardCategory.ExternalSystem,
|
|
Target = "api",
|
|
PayloadJson = "{}",
|
|
Status = StoreAndForwardMessageStatus.Pending,
|
|
CreatedAt = DateTimeOffset.UtcNow,
|
|
MaxRetries = 3
|
|
});
|
|
|
|
var svc = new StoreAndForwardService(
|
|
_storage,
|
|
new StoreAndForwardOptions { RetryTimerInterval = TimeSpan.FromMinutes(10) },
|
|
NullLogger<StoreAndForwardService>.Instance);
|
|
|
|
// First start seeds the cached count from the durable store → 1.
|
|
await svc.StartAsync();
|
|
Assert.Equal(1, ReadQueueDepthGauge());
|
|
|
|
// Graceful stop resets the registration guard AND the cached count (the fix).
|
|
await svc.StopAsync();
|
|
|
|
// Restart the SAME instance: the guard was reset so StartAsync re-seeds from the
|
|
// store (still 1 Pending). With the _bufferedCount reset the gauge reports 1, not 2;
|
|
// without it the seed would ADD onto the leftover 1 → 2 (the double-count bug).
|
|
await svc.StartAsync();
|
|
Assert.Equal(1, ReadQueueDepthGauge());
|
|
|
|
await svc.StopAsync();
|
|
}
|
|
|
|
[Fact]
|
|
public async Task Gauge_SeedsFromExistingPendingRows_OnStart()
|
|
{
|
|
// Pre-seed two Pending rows directly in storage *before* a fresh service starts,
|
|
// simulating a process restart over a non-empty buffer. StartAsync must seed the
|
|
// cached counter from the store so the gauge does not under-report on restart.
|
|
await _storage.EnqueueAsync(new StoreAndForwardMessage
|
|
{
|
|
Id = Guid.NewGuid().ToString("N"),
|
|
Category = StoreAndForwardCategory.ExternalSystem,
|
|
Target = "api",
|
|
PayloadJson = "{}",
|
|
Status = StoreAndForwardMessageStatus.Pending,
|
|
CreatedAt = DateTimeOffset.UtcNow,
|
|
MaxRetries = 3
|
|
});
|
|
await _storage.EnqueueAsync(new StoreAndForwardMessage
|
|
{
|
|
Id = Guid.NewGuid().ToString("N"),
|
|
Category = StoreAndForwardCategory.Notification,
|
|
Target = "list",
|
|
PayloadJson = "{}",
|
|
Status = StoreAndForwardMessageStatus.Pending,
|
|
CreatedAt = DateTimeOffset.UtcNow,
|
|
MaxRetries = 3
|
|
});
|
|
|
|
var fresh = new StoreAndForwardService(
|
|
_storage,
|
|
new StoreAndForwardOptions { RetryTimerInterval = TimeSpan.FromMinutes(10) },
|
|
NullLogger<StoreAndForwardService>.Instance);
|
|
try
|
|
{
|
|
await fresh.StartAsync();
|
|
// The fresh service registered itself as the global provider and seeded 2.
|
|
Assert.Equal(2, ReadQueueDepthGauge());
|
|
}
|
|
finally
|
|
{
|
|
await fresh.StopAsync();
|
|
}
|
|
}
|
|
|
|
/// <summary>
|
|
/// Review finding (FINDING 1): the startup seed must ADD to whatever the counter
|
|
/// already holds, not overwrite it. A concurrent <c>BufferAsync</c> can
|
|
/// <c>Interlocked.Increment</c> <c>_bufferedCount</c> in the window between
|
|
/// <c>StartAsync</c>'s async <c>COUNT(*)</c> returning and the seed running; with an
|
|
/// <c>Interlocked.Exchange</c> seed that increment would be clobbered (lost +1). This
|
|
/// pre-increments the in-memory counter (standing in for that concurrent enqueue),
|
|
/// then starts the service over an empty store and asserts the pre-increment survives.
|
|
/// </summary>
|
|
[Fact]
|
|
public async Task Gauge_SeedAddsToConcurrentPreSeedIncrement_NotClobber()
|
|
{
|
|
// Store is empty (StartAsync's pending COUNT(*) = 0), so the only contribution
|
|
// is the simulated concurrent pre-seed enqueue increment.
|
|
var fresh = new StoreAndForwardService(
|
|
_storage,
|
|
new StoreAndForwardOptions { RetryTimerInterval = TimeSpan.FromMinutes(10) },
|
|
NullLogger<StoreAndForwardService>.Instance);
|
|
|
|
// Stand in for a BufferAsync increment that landed before StartAsync seeded.
|
|
fresh.TestOnly_IncrementBufferedCount();
|
|
|
|
try
|
|
{
|
|
await fresh.StartAsync();
|
|
// Add(0 seed) over the pre-existing +1 → 1. An Exchange(0) seed would clobber
|
|
// it to 0, losing the concurrent enqueue — the bug this fix prevents.
|
|
Assert.Equal(1, ReadQueueDepthGauge());
|
|
}
|
|
finally
|
|
{
|
|
await fresh.StopAsync();
|
|
}
|
|
}
|
|
}
|