From 55e3e4091dd90b1549b2fa438a889cfba2c897ec Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Mon, 14 Sep 2026 08:07:17 +0200 Subject: [PATCH] fix(diagnostics): honor MaxRecentLogQuerySize for relational Take (#8147) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(diagnostics): honor MaxRecentLogQuerySize for relational Take Relational recent-log queries used a hard-coded null→100 default and 1000 clamp, so enabling Sqlite silently dropped the page size from the InMemory contract of StructuredLogsOptions.MaxRecentLogQuerySize. Co-authored-by: Sipke Schoorstra * test(diagnostics): disambiguate Options.Create in SQL builder tests Co-authored-by: Sipke Schoorstra * fix(diagnostics): normalize negative MaxRecentLogQuerySize before Clamp Math.Clamp throws when the configured ceiling is negative. Both InMemory and Relational now share ClampRecentLogQueryTake, which treats a negative max as zero so query construction cannot fail. Co-authored-by: Sipke Schoorstra --------- Co-authored-by: Cursor Agent --- doc/wiki/diagnostics-structured-logs.md | 2 + ...ructuredLogsServiceCollectionExtensions.cs | 2 + .../RelationalStructuredLogSqlBuilder.cs | 6 ++- .../Options/StructuredLogsOptions.cs | 10 +++++ .../InMemory/InMemoryStructuredLogStore.cs | 2 +- .../RelationalStructuredLogSqlBuilderTests.cs | 41 +++++++++++++++++-- .../InMemoryStructuredLogProviderTests.cs | 11 +++++ 7 files changed, 68 insertions(+), 6 deletions(-) diff --git a/doc/wiki/diagnostics-structured-logs.md b/doc/wiki/diagnostics-structured-logs.md index 2d211faf9..49ac0a754 100644 --- a/doc/wiki/diagnostics-structured-logs.md +++ b/doc/wiki/diagnostics-structured-logs.md @@ -140,6 +140,8 @@ services.AddElsa(elsa => The write buffer uses a bounded queue. If the queue is full, newest events are dropped and the dropped-write count is reported through storage diagnostics. +Recent-log queries honor `StructuredLogsOptions.MaxRecentLogQuerySize` for both the default `Take` and the upper clamp, matching the in-memory store. + ## SQLite Provider Boundary [AddSqliteStructuredLogPersistence](../../src/modules/Elsa.Diagnostics.StructuredLogs.Persistence.Sqlite/Extensions/SqliteStructuredLogsModuleExtensions.cs) supplies provider-specific services: diff --git a/src/modules/Elsa.Diagnostics.StructuredLogs.Persistence.Relational/Extensions/RelationalStructuredLogsServiceCollectionExtensions.cs b/src/modules/Elsa.Diagnostics.StructuredLogs.Persistence.Relational/Extensions/RelationalStructuredLogsServiceCollectionExtensions.cs index d9e346e7b..f79ba9f83 100644 --- a/src/modules/Elsa.Diagnostics.StructuredLogs.Persistence.Relational/Extensions/RelationalStructuredLogsServiceCollectionExtensions.cs +++ b/src/modules/Elsa.Diagnostics.StructuredLogs.Persistence.Relational/Extensions/RelationalStructuredLogsServiceCollectionExtensions.cs @@ -1,4 +1,5 @@ using Elsa.Diagnostics.StructuredLogs.Contracts; +using Elsa.Diagnostics.StructuredLogs.Options; using Elsa.Diagnostics.StructuredLogs.Persistence.Relational.Contracts; using Elsa.Diagnostics.StructuredLogs.Persistence.Relational.Options; using Elsa.Diagnostics.StructuredLogs.Persistence.Relational.Services; @@ -17,6 +18,7 @@ public static class RelationalStructuredLogsServiceCollectionExtensions services.Configure(configureOptions); services.AddOptions(); + services.AddOptions(); services.TryAddSingleton(); services.TryAddSingleton(); services.TryAddSingleton(); diff --git a/src/modules/Elsa.Diagnostics.StructuredLogs.Persistence.Relational/Services/RelationalStructuredLogSqlBuilder.cs b/src/modules/Elsa.Diagnostics.StructuredLogs.Persistence.Relational/Services/RelationalStructuredLogSqlBuilder.cs index 88e3d0e57..c573260f7 100644 --- a/src/modules/Elsa.Diagnostics.StructuredLogs.Persistence.Relational/Services/RelationalStructuredLogSqlBuilder.cs +++ b/src/modules/Elsa.Diagnostics.StructuredLogs.Persistence.Relational/Services/RelationalStructuredLogSqlBuilder.cs @@ -1,9 +1,11 @@ using Elsa.Diagnostics.StructuredLogs.Models; +using Elsa.Diagnostics.StructuredLogs.Options; using Elsa.Diagnostics.StructuredLogs.Persistence.Relational.Contracts; +using Microsoft.Extensions.Options; namespace Elsa.Diagnostics.StructuredLogs.Persistence.Relational.Services; -public class RelationalStructuredLogSqlBuilder(IRelationalStructuredLogDialect dialect) +public class RelationalStructuredLogSqlBuilder(IRelationalStructuredLogDialect dialect, IOptions options) { private readonly string _table = dialect.QuoteIdentifier("StructuredLogEvents"); @@ -20,7 +22,7 @@ public class RelationalStructuredLogSqlBuilder(IRelationalStructuredLogDialect d var parameters = new Dictionary(); var predicates = BuildFilterPredicates(filter, parameters); var where = predicates.Count == 0 ? "" : $" WHERE {string.Join(" AND ", predicates)}"; - var limit = Math.Clamp(filter.Take ?? 100, 0, 1000); + var limit = options.Value.ClampRecentLogQueryTake(filter.Take); var sql = $"SELECT {string.Join(", ", Columns.Select(dialect.QuoteIdentifier))} FROM {_table}{where} ORDER BY {dialect.QuoteIdentifier("Timestamp")} DESC, {dialect.QuoteIdentifier("ReceivedAt")} DESC, {dialect.QuoteIdentifier("Sequence")} DESC, {dialect.QuoteIdentifier("Id")} DESC"; sql = dialect.ApplyLimit(sql, limit); return new(sql, parameters); diff --git a/src/modules/Elsa.Diagnostics.StructuredLogs/Options/StructuredLogsOptions.cs b/src/modules/Elsa.Diagnostics.StructuredLogs/Options/StructuredLogsOptions.cs index 54e56a39b..e4375057f 100644 --- a/src/modules/Elsa.Diagnostics.StructuredLogs/Options/StructuredLogsOptions.cs +++ b/src/modules/Elsa.Diagnostics.StructuredLogs/Options/StructuredLogsOptions.cs @@ -4,6 +4,10 @@ public class StructuredLogsOptions { public int RecentLogCapacity { get; set; } = 5_000; public int SubscriberChannelCapacity { get; set; } = 1_000; + /// + /// Default Take and upper clamp for recent-log queries on every IStructuredLogStore implementation. + /// Negative values are treated as zero so query construction never throws. + /// public int MaxRecentLogQuerySize { get; set; } = 1_000; public TimeSpan SourceHeartbeatTimeout { get; set; } = TimeSpan.FromSeconds(30); public bool IncludeStructuredLogsInternalLogs { get; set; } @@ -27,4 +31,10 @@ public class StructuredLogsOptions "(?i)(password|secret|token|api[-_]?key)\\s*[=:]\\s*[^\\s,;]+", "(?i)(AccountKey|SharedAccessKey)=([^;\\s]+)" ]; + + public int ClampRecentLogQueryTake(int? take) + { + var maxTake = Math.Max(0, MaxRecentLogQuerySize); + return Math.Clamp(take ?? maxTake, 0, maxTake); + } } diff --git a/src/modules/Elsa.Diagnostics.StructuredLogs/Providers/InMemory/InMemoryStructuredLogStore.cs b/src/modules/Elsa.Diagnostics.StructuredLogs/Providers/InMemory/InMemoryStructuredLogStore.cs index f33739540..859a26e84 100644 --- a/src/modules/Elsa.Diagnostics.StructuredLogs/Providers/InMemory/InMemoryStructuredLogStore.cs +++ b/src/modules/Elsa.Diagnostics.StructuredLogs/Providers/InMemory/InMemoryStructuredLogStore.cs @@ -28,7 +28,7 @@ public class InMemoryStructuredLogStore : IStructuredLogStore public ValueTask QueryAsync(StructuredLogFilter filter, CancellationToken cancellationToken = default) { - var take = Math.Clamp(filter.Take ?? _options.MaxRecentLogQuerySize, 0, _options.MaxRecentLogQuerySize); + var take = _options.ClampRecentLogQueryTake(filter.Take); var items = _recentLogs .Snapshot() .Where(x => StructuredLogFilterEvaluator.Matches(x, filter)) diff --git a/test/unit/Elsa.Diagnostics.StructuredLogs.Persistence.Relational.UnitTests/RelationalStructuredLogSqlBuilderTests.cs b/test/unit/Elsa.Diagnostics.StructuredLogs.Persistence.Relational.UnitTests/RelationalStructuredLogSqlBuilderTests.cs index ead94bed4..ea8abb853 100644 --- a/test/unit/Elsa.Diagnostics.StructuredLogs.Persistence.Relational.UnitTests/RelationalStructuredLogSqlBuilderTests.cs +++ b/test/unit/Elsa.Diagnostics.StructuredLogs.Persistence.Relational.UnitTests/RelationalStructuredLogSqlBuilderTests.cs @@ -1,12 +1,14 @@ using Elsa.Diagnostics.StructuredLogs.Models; +using Elsa.Diagnostics.StructuredLogs.Options; using Elsa.Diagnostics.StructuredLogs.Persistence.Relational.Contracts; using Elsa.Diagnostics.StructuredLogs.Persistence.Relational.Services; +using MicrosoftOptions = Microsoft.Extensions.Options.Options; namespace Elsa.Diagnostics.StructuredLogs.Persistence.Relational.UnitTests; public class RelationalStructuredLogSqlBuilderTests { - private readonly RelationalStructuredLogSqlBuilder _builder = new(new FakeDialect()); + private readonly RelationalStructuredLogSqlBuilder _builder = CreateBuilder(); [Fact] public void BuildInsert_UsesDialectQuotingAndParameters() @@ -61,18 +63,42 @@ public class RelationalStructuredLogSqlBuilderTests } [Theory] - [InlineData(null, "FETCH 100")] + [InlineData(null, "FETCH 1000")] [InlineData(-5, "FETCH 0")] [InlineData(-1, "FETCH 0")] [InlineData(2000, "FETCH 1000")] [InlineData(5000, "FETCH 1000")] - public void BuildQuery_ClampsTakeToSupportedRange(int? take, string expectedLimit) + public void BuildQuery_ClampsTakeToMaxRecentLogQuerySize(int? take, string expectedLimit) { var query = _builder.BuildQuery(new() { Take = take }); Assert.Contains(expectedLimit, query.Sql, StringComparison.Ordinal); } + [Theory] + [InlineData(null, "FETCH 50")] + [InlineData(25, "FETCH 25")] + [InlineData(200, "FETCH 50")] + public void BuildQuery_UsesConfiguredMaxRecentLogQuerySize(int? take, string expectedLimit) + { + var builder = CreateBuilder(maxRecentLogQuerySize: 50); + var query = builder.BuildQuery(new() { Take = take }); + + Assert.Contains(expectedLimit, query.Sql, StringComparison.Ordinal); + } + + [Theory] + [InlineData(null)] + [InlineData(-5)] + [InlineData(25)] + public void BuildQuery_WhenMaxRecentLogQuerySizeIsNegative_UsesZeroLimit(int? take) + { + var builder = CreateBuilder(maxRecentLogQuerySize: -10); + var query = builder.BuildQuery(new() { Take = take }); + + Assert.Contains("FETCH 0", query.Sql, StringComparison.Ordinal); + } + [Fact] public void BuildListSources_GroupsBySourceAndOrdersBySource() { @@ -118,6 +144,15 @@ public class RelationalStructuredLogSqlBuilderTests Assert.DoesNotContain("LIMIT -1", query.Sql, StringComparison.Ordinal); } + private static RelationalStructuredLogSqlBuilder CreateBuilder(int? maxRecentLogQuerySize = null) + { + var options = new StructuredLogsOptions(); + if (maxRecentLogQuerySize is { } maxTake) + options.MaxRecentLogQuerySize = maxTake; + + return new(new FakeDialect(), MicrosoftOptions.Create(options)); + } + private class FakeDialect : IRelationalStructuredLogDialect { public string ProviderName => "Fake"; diff --git a/test/unit/Elsa.Diagnostics.StructuredLogs.UnitTests/InMemory/InMemoryStructuredLogProviderTests.cs b/test/unit/Elsa.Diagnostics.StructuredLogs.UnitTests/InMemory/InMemoryStructuredLogProviderTests.cs index afc3546eb..418443d79 100644 --- a/test/unit/Elsa.Diagnostics.StructuredLogs.UnitTests/InMemory/InMemoryStructuredLogProviderTests.cs +++ b/test/unit/Elsa.Diagnostics.StructuredLogs.UnitTests/InMemory/InMemoryStructuredLogProviderTests.cs @@ -43,6 +43,17 @@ public class InMemoryStructuredLogProviderTests Assert.Equal([2, 4], result.Items.Select(x => x.Sequence)); } + [Fact] + public async Task GetRecentAsync_WhenMaxRecentLogQuerySizeIsNegative_ReturnsEmptyWithoutThrowing() + { + _options.MaxRecentLogQuerySize = -10; + await _provider.PublishAsync(CreateLog(1, StructuredLogLevel.Information)); + + var result = await _provider.GetRecentAsync(new() { Take = 25 }); + + Assert.Empty(result.Items); + } + [Fact] public async Task SubscribeAsync_YieldsOnlyMatchingLiveEvents() {