From bcd307d8a494585e03348661e66ccc5e1f523ce2 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Tue, 30 Jun 2026 16:15:47 +0200 Subject: [PATCH 1/3] perf(backend): project index scans + bound AppInstall reads to window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two classes of read-efficiency fixes on hot/aggregation paths. No response shape or value changes — DTOs untouched, all 2541 backend tests pass. P2 — select projections on full-mirror SessionsIndex scans (bandwidth only): - GetMetricsSummaryAsync: cross-partition status tally now projects [PartitionKey, Status] instead of the ~40-column mirror row (incl. the large FailureSnapshotJson) -> ~10-20x less transfer. - QuickSearchSessionsAsync: typeahead scan projects only the matched fields (+ RowKey for the SessionId fallback) instead of the full row, for up to QuickSearchMaxScannedRows rows per request. P3 — push the existing sinceUtc window floor server-side at 8 AppInstall call sites across 5 files (previously each dematerialized the whole table / partition and filtered to the window in memory): - UsageMetricsService (global + tenant): now.AddDays(-days) - MaintenanceService.Aggregation (global + tenant, per aggregated day): targetDate - SlaBreachEvaluationService (current ISO week): now.AddDays(-8) margin - SlaMetricsService: startDate (matches the in-memory lower bound exactly) - Get(Global)GeographicLocationSessionsFunction: cutoff Each floor equals the session-window floor the apps are joined against (app.StartedAt >= session.StartedAt >= floor), so results are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Metrics/GetGeographicLocationSessionsFunction.cs | 3 ++- .../GetGlobalGeographicLocationSessionsFunction.cs | 5 +++-- .../Services/MaintenanceService.Aggregation.cs | 7 +++++-- .../Services/SlaBreachEvaluationService.cs | 5 ++++- .../Services/SlaMetricsService.cs | 4 +++- .../Services/TableStorageService.AgentApi.cs | 12 ++++++++++-- .../Services/UsageMetricsService.cs | 9 +++++++-- 7 files changed, 34 insertions(+), 11 deletions(-) diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Metrics/GetGeographicLocationSessionsFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Metrics/GetGeographicLocationSessionsFunction.cs index 792957e7..f66b2fd9 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Metrics/GetGeographicLocationSessionsFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Metrics/GetGeographicLocationSessionsFunction.cs @@ -69,7 +69,8 @@ namespace AutopilotMonitor.Functions.Functions.Metrics ? FilterSessionsByLocation(sessions, locationKey, groupBy) : FilterSessionsByFields(sessions, country!, query["region"], query["city"]); - var appSummaries = await _metricsRepo.GetAppInstallSummariesByTenantAsync(tenantId); + // Same window as the sessions above (cutoff); BuildRows joins apps to those sessions. + var appSummaries = await _metricsRepo.GetAppInstallSummariesByTenantAsync(tenantId, cutoff); var rows = BuildRows(filtered, appSummaries); var response = req.CreateResponse(HttpStatusCode.OK); diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Metrics/GetGlobalGeographicLocationSessionsFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Metrics/GetGlobalGeographicLocationSessionsFunction.cs index 00bc2d45..5b35e28d 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Metrics/GetGlobalGeographicLocationSessionsFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Metrics/GetGlobalGeographicLocationSessionsFunction.cs @@ -67,9 +67,10 @@ namespace AutopilotMonitor.Functions.Functions.Metrics ? GetGeographicLocationSessionsFunction.FilterSessionsByLocation(sessions, locationKey, groupBy) : GetGeographicLocationSessionsFunction.FilterSessionsByFields(sessions, country!, query["region"], query["city"]); + // Same window as the sessions above (cutoff); BuildRows joins apps to those sessions. var appSummaries = string.IsNullOrEmpty(filterTenantId) - ? await _metricsRepo.GetAllAppInstallSummariesAsync() - : await _metricsRepo.GetAppInstallSummariesByTenantAsync(filterTenantId); + ? await _metricsRepo.GetAllAppInstallSummariesAsync(cutoff) + : await _metricsRepo.GetAppInstallSummariesByTenantAsync(filterTenantId, cutoff); var rows = GetGeographicLocationSessionsFunction.BuildRows(filtered, appSummaries); var response = req.CreateResponse(HttpStatusCode.OK); diff --git a/src/Backend/AutopilotMonitor.Functions/Services/MaintenanceService.Aggregation.cs b/src/Backend/AutopilotMonitor.Functions/Services/MaintenanceService.Aggregation.cs index 083423da..16c930da 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/MaintenanceService.Aggregation.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/MaintenanceService.Aggregation.cs @@ -139,10 +139,13 @@ namespace AutopilotMonitor.Functions.Services // App metrics: count apps per session from AppInstallSummaries table var sessionIdSet = new HashSet(sessions.Select(s => s.SessionId)); List appSummaries; + // Bound the AppInstall scan to this snapshot's day. The session set is already + // [targetDate, targetDate+1) and apps install during their session (StartedAt >= targetDate), + // so relevantApps is unchanged; without sinceUtc this scanned the whole table per aggregated day. if (tenantId == "global") - appSummaries = await _metricsRepo.GetAllAppInstallSummariesAsync(); + appSummaries = await _metricsRepo.GetAllAppInstallSummariesAsync(targetDate); else - appSummaries = await _metricsRepo.GetAppInstallSummariesByTenantAsync(tenantId); + appSummaries = await _metricsRepo.GetAppInstallSummariesByTenantAsync(tenantId, targetDate); var relevantApps = appSummaries.Where(a => sessionIdSet.Contains(a.SessionId)).ToList(); var appsPerSession = relevantApps.GroupBy(a => a.SessionId).Select(g => g.Count()).ToList(); diff --git a/src/Backend/AutopilotMonitor.Functions/Services/SlaBreachEvaluationService.cs b/src/Backend/AutopilotMonitor.Functions/Services/SlaBreachEvaluationService.cs index 9bee75df..8b1068de 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/SlaBreachEvaluationService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/SlaBreachEvaluationService.cs @@ -186,7 +186,10 @@ namespace AutopilotMonitor.Functions.Services if (config.SlaNotifyOnAppInstallBreach && config.SlaTargetAppInstallSuccessRate.HasValue) { var currentWeekKey = SlaMetricsService.GetIsoWeekKey(now); - var raw = await _metricsRepo.GetAppInstallSummariesByTenantAsync(config.TenantId); + // Bound to a safe margin before the current ISO week (an ISO week spans <=7 days, so + // now-8d never excludes a current-week row). The exact week match happens in-memory + // below; this just stops the query from scanning the tenant's entire history every 2h. + var raw = await _metricsRepo.GetAppInstallSummariesByTenantAsync(config.TenantId, now.AddDays(-8)); appInstalls = raw .Where(a => (a.Status == "Succeeded" || a.Status == "Failed") && SlaMetricsService.GetIsoWeekKey(a.StartedAt) == currentWeekKey) diff --git a/src/Backend/AutopilotMonitor.Functions/Services/SlaMetricsService.cs b/src/Backend/AutopilotMonitor.Functions/Services/SlaMetricsService.cs index bcb1a467..003171d8 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/SlaMetricsService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/SlaMetricsService.cs @@ -125,7 +125,9 @@ namespace AutopilotMonitor.Functions.Services List? appInstalls = null; if (config?.SlaTargetAppInstallSuccessRate != null) { - appInstalls = await _metricsRepo.GetAppInstallSummariesByTenantAsync(tenantId); + // Push the window's lower bound server-side (startDate); the in-memory filter below + // still applies the upper bound. Without sinceUtc this read the tenant's full history. + appInstalls = await _metricsRepo.GetAppInstallSummariesByTenantAsync(tenantId, startDate); // Filter to the time window appInstalls = appInstalls .Where(a => a.StartedAt >= startDate && a.StartedAt <= endDate) diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.AgentApi.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.AgentApi.cs index f0803ac6..998564d5 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.AgentApi.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.AgentApi.cs @@ -338,7 +338,11 @@ namespace AutopilotMonitor.Functions.Services : windowBound; var scannedRows = 0; - await foreach (var entity in indexTableClient.QueryAsync(filter: filter)) + // Project to only the typeahead-matched fields (+ RowKey for the SessionId fallback at + // ExtractSessionIdFromIndexRowKey). Avoids pulling the full ~40-column SessionsIndex + // mirror for up to QuickSearchMaxScannedRows rows on every keystroke-driven request. + await foreach (var entity in indexTableClient.QueryAsync( + filter: filter, select: new[] { "RowKey", "SessionId", "SerialNumber", "DeviceName", "Status", "StartedAt" })) { if (++scannedRows > QuickSearchMaxScannedRows) break; @@ -1261,7 +1265,11 @@ namespace AutopilotMonitor.Functions.Services // longer vanish from the summary). See SessionStatusBuckets in MetricsMath.cs. var groups = new Dictionary(StringComparer.OrdinalIgnoreCase); - await foreach (var entity in indexTableClient.QueryAsync(filter: oDataFilter)) + // Project to the two columns the tally consumes. The SessionsIndex row is a full + // ~40-column mirror (incl. the large FailureSnapshotJson), so reading it whole for a + // status count multiplied transfer ~10-20x for nothing on this cross-partition scan. + await foreach (var entity in indexTableClient.QueryAsync( + filter: oDataFilter, select: new[] { "PartitionKey", "Status" })) { var pk = entity.PartitionKey; var statusStr = entity.GetString("Status") ?? "InProgress"; diff --git a/src/Backend/AutopilotMonitor.Functions/Services/UsageMetricsService.cs b/src/Backend/AutopilotMonitor.Functions/Services/UsageMetricsService.cs index 26efcb80..a0b3438c 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/UsageMetricsService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/UsageMetricsService.cs @@ -217,7 +217,11 @@ namespace AutopilotMonitor.Functions.Services }; // App & Script Metrics - var allAppSummaries = await _metricsRepo.GetAllAppInstallSummariesAsync(); + // Bound the AppInstall scan to the same window as the sessions above — relevantApps is + // filtered to sessionIdSet anyway, and an app installs during its session so its StartedAt + // is >= the session's. Without sinceUtc this dematerialized the entire AppInstallSummaries + // table on every cache miss just to discard everything outside the window. + var allAppSummaries = await _metricsRepo.GetAllAppInstallSummariesAsync(DateTime.UtcNow.AddDays(-days)); var sessionIdSet = new HashSet(allSessions.Select(s => s.SessionId)); var relevantApps = allAppSummaries.Where(a => sessionIdSet.Contains(a.SessionId)).ToList(); var appsPerSessionList = relevantApps.GroupBy(a => a.SessionId).Select(g => g.Count()).ToList(); @@ -366,7 +370,8 @@ namespace AutopilotMonitor.Functions.Services }; // App & Script Metrics - var tenantAppSummaries = await _metricsRepo.GetAppInstallSummariesByTenantAsync(tenantId); + // Bound to the window (see ComputeUsageMetricsInternalAsync) — joined to tenantSessionIdSet below. + var tenantAppSummaries = await _metricsRepo.GetAppInstallSummariesByTenantAsync(tenantId, DateTime.UtcNow.AddDays(-days)); var tenantSessionIdSet = new HashSet(tenantSessions.Select(s => s.SessionId)); var relevantTenantApps = tenantAppSummaries.Where(a => tenantSessionIdSet.Contains(a.SessionId)).ToList(); var tenantAppsPerSession = relevantTenantApps.GroupBy(a => a.SessionId).Select(g => g.Count()).ToList(); From b41796344e1a6f0ffe73673d754a025c38de4f81 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Tue, 30 Jun 2026 16:24:59 +0200 Subject: [PATCH 2/3] perf(backend): project GetSessionStatsAsync drain to the columns it tallies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GetSessionStatsAsync drained the entire window via GetSessionsAsync, which materializes every ~40-column SessionsIndex mirror row (incl. the large FailureSnapshotJson, all Os*/Geo*/PendingActionsJson columns) — but AggregateSessionStats only reads Status, StartedAt and DurationSeconds. Drain through FetchSessionsPageInternalAsync with a narrow projection { PartitionKey, RowKey, SessionId, Status, StartedAt, CompletedAt, DurationSeconds }: - PartitionKey/RowKey/SessionId: TenantId map + page-cursor derivation. - CompletedAt: ComputeEffectiveDuration's fallback for a Succeeded session with no stored DurationSeconds — kept so the tally value is identical. - IsPreProvisioned/ResumedAt deliberately omitted: they only feed the WhiteGlove Part-2 duration branch, which is gated on status==InProgress, and AggregateSessionStats never reads DurationSeconds for InProgress sessions — so dropping them cannot change any reported value. FetchSessionsPageInternalAsync gains an optional `select` param defaulting to null (= full row), so its only other caller (GetSessionsPageAsync) is unchanged. Cursor/HasMore/pre-migration fallback mechanics are untouched. DurationSeconds has a history of display/calc bugs, so equivalence is pinned by SessionStatsProjectionEquivalenceTests: it reduces a full mirror row to exactly SessionStatsProjection (derived from the production array, so a future column drop fails the test), maps both shapes through the real production mapper, and asserts identical DurationSeconds + identical SessionStats across a mixed fleet — including a WhiteGlove Succeeded (drops IsPreProvisioned/ ResumedAt) and a CompletedAt-fallback Succeeded. Verified the test bites: removing CompletedAt from the projection fails it. All 2544 backend tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../SessionStatsProjectionEquivalenceTests.cs | 179 ++++++++++++++++++ .../Services/TableStorageService.Sessions.cs | 40 +++- 2 files changed, 215 insertions(+), 4 deletions(-) create mode 100644 src/Backend/AutopilotMonitor.Functions.Tests/SessionStatsProjectionEquivalenceTests.cs diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/SessionStatsProjectionEquivalenceTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/SessionStatsProjectionEquivalenceTests.cs new file mode 100644 index 00000000..1ee9762d --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/SessionStatsProjectionEquivalenceTests.cs @@ -0,0 +1,179 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Azure.Data.Tables; +using AutopilotMonitor.Functions.Services; +using AutopilotMonitor.Shared.Models; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Pins that the column-projected SessionsIndex scan driving GetSessionStatsAsync produces +/// byte-identical dashboard stats to the old full-row drain — in particular that the omitted +/// columns can never shift the (historically fragile) DurationSeconds calculation. +/// +/// The stats drain reads only SessionStatsProjection +/// { PartitionKey, RowKey, SessionId, Status, StartedAt, CompletedAt, DurationSeconds }. In real +/// Azure Table Storage a $select returns ONLY those properties; every other column is +/// genuinely absent from the entity. These tests reproduce that faithfully: the "projected" row is +/// built with the projected keys ONLY (no IsPreProvisioned / ResumedAt / noise), so a getter for an +/// omitted column returns null exactly as it would against the live service. We then map both shapes +/// through the production mapper () +/// and through and assert equivalence. +/// +public class SessionStatsProjectionEquivalenceTests +{ + private const string TenantId = "00000000-0000-0000-0000-000000000abc"; + + private static readonly TableStorageService Sut = + new(new Mock().Object, NullLogger.Instance); + + // SessionsIndex RowKey = inverted-tick prefix + "_" + sessionId (see ComputeIndexRowKey). + private static string IndexRowKey(DateTime startedAt, string sessionId) + => $"{(DateTime.MaxValue.Ticks - startedAt.Ticks):D19}_{sessionId}"; + + /// Full ~40-column mirror row as the list drain would read it (no projection). + private static TableEntity FullRow( + string sessionId, string status, DateTime startedAt, + int? durationSeconds, DateTime? completedAt, + bool isPreProvisioned, DateTime? resumedAt) + { + var e = new TableEntity(TenantId, IndexRowKey(startedAt, sessionId)) + { + ["SessionId"] = sessionId, + ["Status"] = status, + ["StartedAt"] = new DateTimeOffset(startedAt), + ["IsPreProvisioned"] = isPreProvisioned, + // Representative noise that the stats never read — present on a real mirror row, absent + // on the projected one. Proves the projection drops them without changing the outcome. + ["SerialNumber"] = "SN-FULL", + ["DeviceName"] = "PC-FULL", + ["Manufacturer"] = "Contoso", + ["Model"] = "Model-X", + ["OsName"] = "Windows 11", + ["GeoCountry"] = "DE", + ["EventCount"] = 123, + ["PlatformScriptCount"] = 4, + ["FailureSnapshotJson"] = "{\"big\":\"" + new string('x', 2000) + "\"}", + }; + if (durationSeconds.HasValue) e["DurationSeconds"] = durationSeconds.Value; + if (completedAt.HasValue) e["CompletedAt"] = new DateTimeOffset(completedAt.Value); + if (resumedAt.HasValue) e["ResumedAt"] = new DateTimeOffset(resumedAt.Value); + return e; + } + + /// + /// Reduces a full mirror row to exactly what Azure returns under the production + /// $select = SessionStatsProjection: the projected data columns plus the structural + /// PartitionKey/RowKey; every other property is genuinely absent. Deriving the keep-set from + /// the production array (rather than hardcoding it) means dropping a column there — e.g. + /// CompletedAt — would immediately fail the fallback test below. + /// + private static TableEntity Project(TableEntity full) + { + var keep = new HashSet(TableStorageService.SessionStatsProjection, StringComparer.Ordinal); + var projected = new TableEntity(full.PartitionKey, full.RowKey); + foreach (var kv in full) + { + // PartitionKey/RowKey/Timestamp/ETag are structural (set via ctor / system-managed). + if (kv.Key is "PartitionKey" or "RowKey" or "Timestamp" or "odata.etag") continue; + if (keep.Contains(kv.Key)) projected[kv.Key] = kv.Value; + } + return projected; + } + + // ── DurationSeconds equivalence — the historically fragile field ───────────── + + [Fact] + public void Succeeded_whiteglove_with_stored_duration_maps_identically_despite_dropped_columns() + { + // The decisive case: a WhiteGlove (IsPreProvisioned + ResumedAt) Succeeded session. The full + // row carries both columns; the projected row drops them. They feed ONLY the Part-2 branch, + // which is gated on status==InProgress, so a Succeeded session must resolve to the stored + // duration on BOTH shapes. If the projection were unsafe this is where it would break. + var started = DateTime.UtcNow.AddHours(-3); + var sid = Guid.NewGuid().ToString(); + + var fullRow = FullRow( + sid, "Succeeded", started, durationSeconds: 1800, completedAt: started.AddSeconds(1800), + isPreProvisioned: true, resumedAt: started.AddMinutes(20)); + var full = Sut.MapIndexEntityToSessionSummary(fullRow); + var projected = Sut.MapIndexEntityToSessionSummary(Project(fullRow)); + + Assert.Equal(1800, full.DurationSeconds); + Assert.Equal(full.DurationSeconds, projected.DurationSeconds); + Assert.Equal(full.Status, projected.Status); + Assert.Equal(full.StartedAt, projected.StartedAt); + Assert.Equal(sid, projected.SessionId); + } + + [Fact] + public void Succeeded_without_stored_duration_falls_back_to_completedAt_on_both_shapes() + { + // No stored DurationSeconds → ComputeEffectiveDuration uses (CompletedAt - StartedAt). + // CompletedAt IS in the projection precisely so this fallback stays identical; this test + // would fail if CompletedAt were ever dropped from SessionStatsProjection. + var started = DateTime.UtcNow.AddHours(-2); + var completed = started.AddSeconds(1200); + var sid = Guid.NewGuid().ToString(); + + var fullRow = FullRow( + sid, "Succeeded", started, durationSeconds: null, completedAt: completed, + isPreProvisioned: false, resumedAt: null); + var full = Sut.MapIndexEntityToSessionSummary(fullRow); + var projected = Sut.MapIndexEntityToSessionSummary(Project(fullRow)); + + Assert.Equal(1200, full.DurationSeconds); + Assert.Equal(full.DurationSeconds, projected.DurationSeconds); + } + + // ── End-to-end: projected drain yields identical SessionStats ──────────────── + + [Fact] + public void AggregateSessionStats_is_identical_for_full_vs_projected_fleet() + { + var now = DateTime.UtcNow; + // A representative mixed fleet covering every status the cards count, including a WhiteGlove + // Succeeded (drops IsPreProvisioned/ResumedAt), a CompletedAt-fallback Succeeded, and an + // InProgress WhiteGlove whose per-row duration DOES diverge between shapes but which the + // tally never reads — so the aggregate must still match exactly. + var fleet = new (string Status, DateTime Started, int? Dur, DateTime? Completed, bool Wg, DateTime? Resumed)[] + { + ("Succeeded", now.AddHours(-1), 1800, now.AddHours(-1).AddSeconds(1800), false, null), + ("Succeeded", now.AddHours(-2), 600, now.AddHours(-2).AddSeconds(600), true, now.AddHours(-2).AddMinutes(15)), // WhiteGlove + ("Succeeded", now.AddHours(-3), null, now.AddHours(-3).AddSeconds(900), false, null), // CompletedAt fallback + ("Failed", now.AddHours(-4), 7200, now.AddHours(-4).AddSeconds(7200), false, null), // duration ignored for Failed + ("InProgress", now.AddMinutes(-30), 60, null, true, now.AddMinutes(-10)), // WG in-flight: per-row dur diverges, ignored + ("Pending", now.AddDays(-2), null, null, true, null), + ("Stalled", now.AddHours(-9), null, null, false, null), + ("Succeeded", now.Date.AddHours(2), 1200, now.Date.AddHours(2).AddSeconds(1200), false, null), // today + ("Failed", now.Date.AddHours(3), null, null, false, null), // today + }; + + var fullSummaries = new List(); + var projectedSummaries = new List(); + foreach (var (status, started, dur, completed, wg, resumed) in fleet) + { + var sid = Guid.NewGuid().ToString(); + var fullRow = FullRow(sid, status, started, dur, completed, wg, resumed); + fullSummaries.Add(Sut.MapIndexEntityToSessionSummary(fullRow)); + projectedSummaries.Add(Sut.MapIndexEntityToSessionSummary(Project(fullRow))); + } + + var full = TableStorageService.AggregateSessionStats(fullSummaries, days: 30); + var projected = TableStorageService.AggregateSessionStats(projectedSummaries, days: 30); + + Assert.Equal(full.Days, projected.Days); + Assert.Equal(full.ActiveCount, projected.ActiveCount); + Assert.Equal(full.TotalLastNDays, projected.TotalLastNDays); + Assert.Equal(full.SucceededLastNDays, projected.SucceededLastNDays); + Assert.Equal(full.FailedLastNDays, projected.FailedLastNDays); + Assert.Equal(full.SuccessRatePct, projected.SuccessRatePct); + Assert.Equal(full.AvgDurationMinutes, projected.AvgDurationMinutes); // the duration-sensitive card + Assert.Equal(full.TotalToday, projected.TotalToday); + Assert.Equal(full.FailedToday, projected.FailedToday); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs index 95bfbd71..fabbc513 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs @@ -293,7 +293,9 @@ namespace AutopilotMonitor.Functions.Services /// The key difference from the primary-table mapping: SessionId comes from a stored property /// instead of RowKey (which contains the inverted-tick key in the index). /// - private SessionSummary MapIndexEntityToSessionSummary(TableEntity entity) + // internal (not private) so SessionStatsProjectionEquivalenceTests can pin that a row + // carrying only SessionStatsProjection maps to the same stat-relevant fields as a full row. + internal SessionSummary MapIndexEntityToSessionSummary(TableEntity entity) => MapToSessionSummary(entity, entity.GetString("SessionId") ?? ExtractSessionIdFromIndexRowKey(entity.RowKey)); // ===== SESSION MANAGEMENT METHODS ===== @@ -788,7 +790,7 @@ namespace AutopilotMonitor.Functions.Services /// (single page). /// private async Task<(List Sessions, bool HasMore, string? NextCursor)> FetchSessionsPageInternalAsync( - string tenantId, int maxResults, string? cursor, int? days) + string tenantId, int maxResults, string? cursor, int? days, IEnumerable? select = null) { SecurityValidator.EnsureValidGuid(tenantId, nameof(tenantId)); @@ -817,9 +819,12 @@ namespace AutopilotMonitor.Functions.Services // mechanics are independent of the date window so a tenant with more // than `maxResults` matching sessions remains fully reachable across // multiple calls (was: silently capped at 10k pre-fix). + // select == null → full mirror row (every existing caller). The stats drain passes a + // narrow projection so it doesn't materialize the ~40-column row just to tally statuses. var query = indexTableClient.QueryAsync( filter: filter, - maxPerPage: Math.Min(maxResults + 1, 1000) + maxPerPage: Math.Min(maxResults + 1, 1000), + select: select ); var sessions = new List(); @@ -961,12 +966,39 @@ namespace AutopilotMonitor.Functions.Services /// Computed server-side so the cards don't depend on whatever the client has /// paginated into memory. /// + // Columns AggregateSessionStats (and the ComputeEffectiveDuration it relies on for Succeeded + // sessions) actually reads, plus PartitionKey/RowKey/SessionId for the TenantId map + page + // cursor. Everything else on the ~40-column SessionsIndex mirror (FailureSnapshotJson, Os*, + // Geo*, PendingActionsJson, …) is never consumed by the tally, so the stats drain skips it. + // Crucially IsPreProvisioned/ResumedAt are omitted on purpose: they only feed the WhiteGlove + // Part-2 duration branch, which is gated on status==InProgress, and the stats never read + // DurationSeconds for InProgress sessions — so dropping them cannot change any reported value. + // Equivalence is pinned by SessionStatsProjectionEquivalenceTests. Non-projected columns map + // to null/default via the Safe*/`?? ` getters in MapToSessionSummary — no missing-column throw. + // internal so the equivalence test derives its simulated projected row from this exact set + // (any future column drop here is automatically reflected in the test's faithfulness). + internal static readonly string[] SessionStatsProjection = + { "PartitionKey", "RowKey", "SessionId", "Status", "StartedAt", "CompletedAt", "DurationSeconds" }; + public async Task GetSessionStatsAsync(string tenantId, int days) { SecurityValidator.EnsureValidGuid(tenantId, nameof(tenantId)); if (days < 1) throw new ArgumentOutOfRangeException(nameof(days)); - var sessions = await GetSessionsAsync(tenantId, days); + // Stats are a pure status/duration tally, so drain the window with a projected + // SessionsIndex scan instead of GetSessionsAsync's full-row materialization. Same + // cursor/HasMore/fallback mechanics as the list drain (via FetchSessionsPageInternalAsync), + // just a narrower column set — the returned partial summaries never leave this method. + var sessions = new List(); + string? cursor = null; + do + { + var page = await FetchSessionsPageInternalAsync( + tenantId, maxResults: 1000, cursor: cursor, days: days, select: SessionStatsProjection); + sessions.AddRange(page.Sessions); + cursor = page.HasMore ? page.NextCursor : null; + } while (!string.IsNullOrEmpty(cursor)); + return AggregateSessionStats(sessions, days); } From a7c25aa7de191a589765d66f4025585e8f570cd1 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Tue, 30 Jun 2026 17:06:58 +0200 Subject: [PATCH 3/3] fix(metrics): share one window start between session and AppInstall bounds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the sinceUtc bounding (commit "project index scans + bound AppInstall reads to window"): the AppInstall cutoff was computed with a fresh DateTime.UtcNow.AddDays(-days) AFTER the session window had already been queried and several aggregates had run. That second snapshot sits a few ms later than the session query's, so the AppInstall lower bound was strictly later than the session lower bound. A session included right at the window's lower boundary could then keep its row while app summaries whose StartedAt fell into the gap between the two cutoffs were dropped — undercounting AppScripts.AvgAppsPerSession and TotalUniqueApps for boundary sessions. Capture a single `now`/`windowStart` snapshot at the top of both ComputeUsageMetricsInternalAsync and ComputeTenantUsageMetricsInternalAsync and reuse it for GetSessionsByDateRangeAsync(windowStart, …) and Get(All)AppInstallSummariesAsync(windowStart). Now both bounds are identical. UsageMetricsWindowConsistencyTests pins it: it captures the start arg passed to the session repo and the cutoff passed to the app repo and asserts they are equal (and thus <=), on both the platform and tenant paths. Verified the test bites — reverting either call to a fresh UtcNow fails it. All 2546 backend tests pass. Reported-by: Codex review Co-Authored-By: Claude Opus 4.8 (1M context) --- .../UsageMetricsWindowConsistencyTests.cs | 85 +++++++++++++++++++ .../Services/UsageMetricsService.cs | 36 +++++--- 2 files changed, 107 insertions(+), 14 deletions(-) create mode 100644 src/Backend/AutopilotMonitor.Functions.Tests/UsageMetricsWindowConsistencyTests.cs diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/UsageMetricsWindowConsistencyTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/UsageMetricsWindowConsistencyTests.cs new file mode 100644 index 00000000..fb1e0841 --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/UsageMetricsWindowConsistencyTests.cs @@ -0,0 +1,85 @@ +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using AutopilotMonitor.Functions.Services; +using AutopilotMonitor.Shared.DataAccess; +using AutopilotMonitor.Shared.Models; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Regression guard for the AppInstall window-consistency bug: the session window and the AppInstall +/// sinceUtc cutoff must derive from ONE shared windowStart, not two separate DateTime.UtcNow snapshots +/// taken moments apart (the second sits slightly later because the in-between aggregates take time). +/// If the app cutoff drifted later than the session start, a session included at the lower boundary +/// could keep its row while its app summaries fell into the gap and were dropped — undercounting +/// AppScripts.AvgAppsPerSession and TotalUniqueApps. These tests assert the app repo receives a +/// cutoff identical to (and therefore <=) the session repo's window start, on both code paths. +/// +public class UsageMetricsWindowConsistencyTests +{ + private const string TenantId = "00000000-0000-0000-0000-000000000777"; + + [Fact] + public async Task Global_app_cutoff_equals_session_window_start() + { + DateTime? sessionStart = null; + DateTime? appCutoff = null; + + var maintenance = new Mock(); + maintenance + .Setup(m => m.GetSessionsByDateRangeAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((start, _, _) => sessionStart = start) + .ReturnsAsync(new List()); + + var metrics = new Mock(); + metrics + .Setup(m => m.GetAllAppInstallSummariesAsync(It.IsAny())) + .Callback(c => appCutoff = c) + .ReturnsAsync(new List()); + metrics.Setup(m => m.GetAllUserActivityMetricsAsync()).ReturnsAsync(new UserActivityMetrics()); + metrics.Setup(m => m.GetPlatformStatsAsync()).ReturnsAsync((PlatformStats?)null); + + var sut = new UsageMetricsService(metrics.Object, maintenance.Object, NullLogger.Instance); + + await sut.ComputeUsageMetricsInternalAsync(days: 30); + + Assert.NotNull(sessionStart); + Assert.NotNull(appCutoff); + Assert.Equal(sessionStart, appCutoff); // identical lower bound (one shared windowStart) + Assert.True(appCutoff <= sessionStart); // ... and never later than the session start + } + + [Fact] + public async Task Tenant_app_cutoff_equals_session_window_start() + { + DateTime? sessionStart = null; + DateTime? appCutoff = null; + + var maintenance = new Mock(); + maintenance + .Setup(m => m.GetSessionsByDateRangeAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((start, _, _) => sessionStart = start) + .ReturnsAsync(new List()); + + var metrics = new Mock(); + metrics + .Setup(m => m.GetAppInstallSummariesByTenantAsync(It.IsAny(), It.IsAny())) + .Callback((_, c) => appCutoff = c) + .ReturnsAsync(new List()); + metrics.Setup(m => m.GetUserActivityMetricsAsync(It.IsAny())).ReturnsAsync(new UserActivityMetrics()); + metrics.Setup(m => m.GetPlatformStatsAsync()).ReturnsAsync((PlatformStats?)null); + + var sut = new UsageMetricsService(metrics.Object, maintenance.Object, NullLogger.Instance); + + await sut.ComputeTenantUsageMetricsInternalAsync(TenantId, days: 30); + + Assert.NotNull(sessionStart); + Assert.NotNull(appCutoff); + Assert.Equal(sessionStart, appCutoff); + Assert.True(appCutoff <= sessionStart); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/Services/UsageMetricsService.cs b/src/Backend/AutopilotMonitor.Functions/Services/UsageMetricsService.cs index a0b3438c..5936cff2 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/UsageMetricsService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/UsageMetricsService.cs @@ -104,14 +104,19 @@ namespace AutopilotMonitor.Functions.Services return days; } - private async Task ComputeUsageMetricsInternalAsync(int days) + internal async Task ComputeUsageMetricsInternalAsync(int days) { // Query sessions over the requested window (days). Today/7d/30d sub-aggregates remain // meaningful only when they fit inside the window; clients can read WindowDays to know // the actual scope. "Total" counters use PlatformStats (cumulative, tracked separately). - var allSessions = await _maintenanceRepo.GetSessionsByDateRangeAsync(DateTime.UtcNow.AddDays(-days), DateTime.UtcNow.AddDays(1)); - + // One UtcNow snapshot for the whole computation: the session window lower bound and the + // AppInstall cutoff (below) MUST share an identical windowStart. A fresh UtcNow for the + // app query would sit milliseconds later and could drop a boundary session's app rows + // (undercounting AvgAppsPerSession / TotalUniqueApps). Pinned by UsageMetricsWindowConsistencyTests. var now = DateTime.UtcNow; + var windowStart = now.AddDays(-days); + var allSessions = await _maintenanceRepo.GetSessionsByDateRangeAsync(windowStart, now.AddDays(1)); + var today = now.Date; var last7Days = now.AddDays(-7); var last30Days = now.AddDays(-30); @@ -217,11 +222,11 @@ namespace AutopilotMonitor.Functions.Services }; // App & Script Metrics - // Bound the AppInstall scan to the same window as the sessions above — relevantApps is - // filtered to sessionIdSet anyway, and an app installs during its session so its StartedAt - // is >= the session's. Without sinceUtc this dematerialized the entire AppInstallSummaries - // table on every cache miss just to discard everything outside the window. - var allAppSummaries = await _metricsRepo.GetAllAppInstallSummariesAsync(DateTime.UtcNow.AddDays(-days)); + // Reuse windowStart (NOT a fresh UtcNow) so the AppInstall lower bound is identical to the + // session window's — see the snapshot note above. relevantApps is filtered to sessionIdSet + // anyway, and an app installs during its session (StartedAt >= the session's), so bounding + // here only drops rows already outside the window instead of scanning the whole table. + var allAppSummaries = await _metricsRepo.GetAllAppInstallSummariesAsync(windowStart); var sessionIdSet = new HashSet(allSessions.Select(s => s.SessionId)); var relevantApps = allAppSummaries.Where(a => sessionIdSet.Contains(a.SessionId)).ToList(); var appsPerSessionList = relevantApps.GroupBy(a => a.SessionId).Select(g => g.Count()).ToList(); @@ -268,12 +273,15 @@ namespace AutopilotMonitor.Functions.Services }; } - private async Task ComputeTenantUsageMetricsInternalAsync(string tenantId, int days) + internal async Task ComputeTenantUsageMetricsInternalAsync(string tenantId, int days) { - // Query sessions for specific tenant over the requested window. - var tenantSessions = await _maintenanceRepo.GetSessionsByDateRangeAsync(DateTime.UtcNow.AddDays(-days), DateTime.UtcNow.AddDays(1), tenantId); - + // Query sessions for specific tenant over the requested window. One UtcNow snapshot so the + // session window lower bound and the AppInstall cutoff (below) share an identical + // windowStart — see ComputeUsageMetricsInternalAsync for why this must not drift. var now = DateTime.UtcNow; + var windowStart = now.AddDays(-days); + var tenantSessions = await _maintenanceRepo.GetSessionsByDateRangeAsync(windowStart, now.AddDays(1), tenantId); + var today = now.Date; var last7Days = now.AddDays(-7); var last30Days = now.AddDays(-30); @@ -370,8 +378,8 @@ namespace AutopilotMonitor.Functions.Services }; // App & Script Metrics - // Bound to the window (see ComputeUsageMetricsInternalAsync) — joined to tenantSessionIdSet below. - var tenantAppSummaries = await _metricsRepo.GetAppInstallSummariesByTenantAsync(tenantId, DateTime.UtcNow.AddDays(-days)); + // Reuse windowStart (identical to the session window lower bound) — joined to tenantSessionIdSet below. + var tenantAppSummaries = await _metricsRepo.GetAppInstallSummariesByTenantAsync(tenantId, windowStart); var tenantSessionIdSet = new HashSet(tenantSessions.Select(s => s.SessionId)); var relevantTenantApps = tenantAppSummaries.Where(a => tenantSessionIdSet.Contains(a.SessionId)).ToList(); var tenantAppsPerSession = relevantTenantApps.GroupBy(a => a.SessionId).Select(g => g.Count()).ToList();