From ea81b1eca0d4fb10fe56e7594bfd72de9a0941b7 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Tue, 16 Jun 2026 17:29:35 +0200 Subject: [PATCH 1/2] fix(backend): normalize V2 ingest event data to prevent DeviceSnapshot corruption The V2 telemetry ingest path (TelemetryPayloadParser) skipped the Newtonsoft-JToken -> native normalization the V1 NDJSON path applied. Nested values (e.g. hardware_spec.disks/gpus) stayed as JObject/JArray and reached System.Text.Json downstream in UpsertDeviceSnapshotAsync, serializing to corrupt nested-empty-array output (e.g. "disks":[[[[]],[[]]]]). Verified on live data. - Extract shared EventDataNormalizer (Normalize + NormalizeMap); both ingest paths and the read path (TableStorageService.DeserializeEventData) now use it, consolidating 3 duplicate copies of the same logic into one. - TelemetryPayloadParser.ParseEvent now normalizes after deserialize. - DetectHasSSD: exact Equals("SSD"/"NVMe") -> case-insensitive Contains. The agent reports composite media types ("NVMe SSD") so the exact compare matched nothing and hasSSD was false on every row, independent of the corruption. - New EventDataNormalizationTests cover nested normalization, STJ round-trip safety, the read-path map, and DetectHasSSD matching. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../EventDataNormalizationTests.cs | 118 ++++++++++++++++++ .../Functions/Ingest/EventDataNormalizer.cs | 61 +++++++++ .../Functions/Ingest/NdjsonParser.cs | 28 +---- .../Ingest/TelemetryPayloadParser.cs | 5 + .../Services/TableStorageService.AgentApi.cs | 8 +- .../Services/TableStorageService.cs | 38 +----- 6 files changed, 192 insertions(+), 66 deletions(-) create mode 100644 src/Backend/AutopilotMonitor.Functions.Tests/EventDataNormalizationTests.cs create mode 100644 src/Backend/AutopilotMonitor.Functions/Functions/Ingest/EventDataNormalizer.cs diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/EventDataNormalizationTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/EventDataNormalizationTests.cs new file mode 100644 index 00000000..dbef76f9 --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/EventDataNormalizationTests.cs @@ -0,0 +1,118 @@ +using System.Collections.Generic; +using AutopilotMonitor.Functions.Functions.Ingest; +using AutopilotMonitor.Functions.Services; +using AutopilotMonitor.Shared.Models; +using Newtonsoft.Json; +using Stj = System.Text.Json.JsonSerializer; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Regression tests for the V2 ingest data-corruption bug: the telemetry payload parser used to +/// skip the Newtonsoft-JToken → native normalization the V1 NDJSON path applied, so nested +/// arrays/objects (e.g. hardware_spec.disks) reached System.Text.Json downstream as JObject/JArray +/// and serialized to corrupt nested-empty-array output. Also covers DetectHasSSD, which used an +/// exact mediaType compare that never matched the agent's composite values ("NVMe SSD"). +/// +public class EventDataNormalizationTests +{ + private const string TenantId = "a1b2c3d4-e5f6-7890-abcd-ef1234567890"; + private const string SessionId = "b2c3d4e5-f6a7-8901-bcde-f12345678901"; + + private static TelemetryItemDto EventDto(string payloadJson) + => new TelemetryItemDto + { + Kind = "Event", + PartitionKey = $"{TenantId}_{SessionId}", + RowKey = "0000000001", + TelemetryItemId = 1, + PayloadJson = payloadJson, + EnqueuedAtUtc = new System.DateTime(2026, 4, 21, 10, 0, 0, System.DateTimeKind.Utc), + }; + + // hardware_spec shape the agent actually emits — a nested object array (disks) and a nested + // object array (gpus), plus flat scalars. + private const string HardwareSpecPayload = + "{\"EventType\":\"hardware_spec\",\"Data\":{" + + "\"systemModel\":\"HP EliteBook 840 G8\"," + + "\"ramTotalGB\":7.7," + + "\"disks\":[{\"model\":\"KIOXIA\",\"sizeGB\":238,\"mediaType\":\"NVMe SSD\",\"busType\":\"NVMe\"}]," + + "\"gpus\":[{\"name\":\"Intel Iris Xe\",\"adapterRAMGB\":2}]" + + "}}"; + + [Fact] + public void ParseEvent_normalizes_nested_objects_to_native_types() + { + var evt = TelemetryPayloadParser.ParseEvent(EventDto(HardwareSpecPayload), TenantId, SessionId); + + Assert.NotNull(evt); + // Nested array must be a native List of Dictionary, NOT a JArray/JObject. + var disks = Assert.IsType>(evt!.Data["disks"]); + var disk = Assert.IsType>(disks[0]); + Assert.Equal("NVMe SSD", disk["mediaType"]?.ToString()); + } + + [Fact] + public void ParseEvent_output_is_SystemTextJson_safe() + { + // The actual corruption manifests when the downstream DeviceSnapshot writer re-serializes + // Data with System.Text.Json. Before the fix this produced "disks":[[[[]],...]]; now the + // real fields must survive a round-trip. + var evt = TelemetryPayloadParser.ParseEvent(EventDto(HardwareSpecPayload), TenantId, SessionId); + Assert.NotNull(evt); + + var json = Stj.Serialize(evt!.Data); + + Assert.Contains("\"mediaType\":\"NVMe SSD\"", json); + Assert.Contains("\"model\":\"KIOXIA\"", json); + Assert.Contains("\"name\":\"Intel Iris Xe\"", json); + // The corruption signature: nested empty arrays where objects should be. + Assert.DoesNotContain("[[[", json); + } + + [Fact] + public void NormalizeMap_converts_newtonsoft_tokens_from_stored_json() + { + // Mirrors the read path: storage JSON deserialized by Newtonsoft into object values. + var deserialized = JsonConvert.DeserializeObject>( + "{\"disks\":[{\"mediaType\":\"NVMe SSD\"}],\"flat\":\"x\"}"); + + var normalized = EventDataNormalizer.NormalizeMap(deserialized); + + Assert.IsType>(normalized["disks"]); + Assert.Equal("x", normalized["flat"]?.ToString()); + Assert.DoesNotContain("[[[", Stj.Serialize(normalized)); + } + + [Fact] + public void NormalizeMap_returns_empty_for_null() + => Assert.Empty(EventDataNormalizer.NormalizeMap(null)); + + // ============================================================ DetectHasSSD + + private static Dictionary DisksWith(params string[] mediaTypes) + { + var disks = new List(); + foreach (var mt in mediaTypes) + disks.Add(new Dictionary { ["mediaType"] = mt }); + return new Dictionary { ["disks"] = disks }; + } + + [Theory] + [InlineData("NVMe SSD", true)] // composite value the exact-equals bug missed + [InlineData("NVMe", true)] + [InlineData("SSD", true)] + [InlineData("nvme ssd", true)] // case-insensitive + [InlineData("HDD", false)] + [InlineData("Unknown", false)] + public void DetectHasSSD_matches_composite_and_case_insensitive(string mediaType, bool expected) + => Assert.Equal(expected, TableStorageService.DetectHasSSD(DisksWith(mediaType))); + + [Fact] + public void DetectHasSSD_true_when_any_disk_is_solid_state() + => Assert.Equal(true, TableStorageService.DetectHasSSD(DisksWith("Unknown", "NVMe SSD"))); + + [Fact] + public void DetectHasSSD_null_when_no_disks_key() + => Assert.Null(TableStorageService.DetectHasSSD(new Dictionary())); +} diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Ingest/EventDataNormalizer.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Ingest/EventDataNormalizer.cs new file mode 100644 index 00000000..fdebff19 --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Ingest/EventDataNormalizer.cs @@ -0,0 +1,61 @@ +using System.Collections.Generic; +using System.Linq; +using AutopilotMonitor.Shared.Models; +using Newtonsoft.Json.Linq; + +namespace AutopilotMonitor.Functions.Functions.Ingest +{ + /// + /// Normalizes an graph produced by Newtonsoft into plain + /// .NET types (, , primitives). + /// + /// Both ingest paths deserialize the agent payload with Newtonsoft into a + /// Dictionary<string, object>; for NESTED values Newtonsoft leaves + /// / instances in place. Downstream storage + /// (e.g. UpsertDeviceSnapshotAsync) re-serializes Data with System.Text.Json, + /// which cannot represent a Newtonsoft JToken and emits corrupt output + /// (e.g. "disks":[[[[]],[[]]]]). Converting the graph to native types first makes + /// the value System.Text.Json-safe and keeps both ingest paths behaviourally identical. + /// + internal static class EventDataNormalizer + { + /// + /// Replaces 's Data with a graph of native .NET types. + /// No-op when there is nothing to normalize. + /// + internal static void Normalize(EnrollmentEvent evt) + { + if (evt.Data == null || evt.Data.Count == 0) return; + evt.Data = NormalizeMap(evt.Data); + } + + /// + /// Returns a new map with every value converted to native .NET types. + /// Used by the read path when re-hydrating stored event data. + /// + internal static Dictionary NormalizeMap(Dictionary? data) + { + var normalized = new Dictionary(); + if (data == null) return normalized; + foreach (var kvp in data) + normalized[kvp.Key] = ConvertJTokenToNative(kvp.Value); + return normalized; + } + + private static object ConvertJTokenToNative(object value) + { + if (value is JArray jArray) + return jArray.Select(item => ConvertJTokenToNative(item)).ToList(); + if (value is JObject jObject) + { + var dict = new Dictionary(); + foreach (var prop in jObject.Properties()) + dict[prop.Name] = ConvertJTokenToNative(prop.Value); + return dict; + } + if (value is JValue jValue) + return jValue.Value ?? string.Empty; + return value; + } + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Ingest/NdjsonParser.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Ingest/NdjsonParser.cs index 85a57d55..961b1fc8 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Ingest/NdjsonParser.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Ingest/NdjsonParser.cs @@ -1,7 +1,6 @@ using System.Text; using AutopilotMonitor.Shared.Models; using Newtonsoft.Json; -using Newtonsoft.Json.Linq; namespace AutopilotMonitor.Functions.Functions.Ingest { @@ -63,7 +62,7 @@ namespace AutopilotMonitor.Functions.Functions.Ingest var evt = JsonConvert.DeserializeObject(lines[i]); if (evt != null) { - NormalizeEventData(evt); + EventDataNormalizer.Normalize(evt); events.Add(evt); } } @@ -76,30 +75,5 @@ namespace AutopilotMonitor.Functions.Functions.Ingest return (metadata.SessionId, metadata.TenantId, events); } - - internal static void NormalizeEventData(EnrollmentEvent evt) - { - if (evt.Data == null || evt.Data.Count == 0) return; - var normalized = new Dictionary(); - foreach (var kvp in evt.Data) - normalized[kvp.Key] = ConvertJTokenToNative(kvp.Value); - evt.Data = normalized; - } - - private static object ConvertJTokenToNative(object value) - { - if (value is JArray jArray) - return jArray.Select(item => ConvertJTokenToNative(item)).ToList(); - if (value is JObject jObject) - { - var dict = new Dictionary(); - foreach (var prop in jObject.Properties()) - dict[prop.Name] = ConvertJTokenToNative(prop.Value); - return dict; - } - if (value is JValue jValue) - return jValue.Value ?? string.Empty; - return value; - } } } diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Ingest/TelemetryPayloadParser.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Ingest/TelemetryPayloadParser.cs index 9aae2f42..429caec3 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Ingest/TelemetryPayloadParser.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Ingest/TelemetryPayloadParser.cs @@ -25,6 +25,11 @@ namespace AutopilotMonitor.Functions.Functions.Ingest { var evt = JsonConvert.DeserializeObject(item.PayloadJson!); if (evt == null) return null; + // Convert Newtonsoft JObject/JArray nodes in nested Data values to native .NET + // types — same contract as the V1 NDJSON path. Without this, nested values reach + // System.Text.Json downstream (e.g. UpsertDeviceSnapshotAsync) as JToken and + // serialize to corrupt output (e.g. "disks":[[[[]],[[]]]]). + EventDataNormalizer.Normalize(evt); // Tenant/Session stamped by the function after return; here we only deserialise. return evt; } diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.AgentApi.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.AgentApi.cs index fe6dd20e..7ca220d0 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.AgentApi.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.AgentApi.cs @@ -209,7 +209,7 @@ namespace AutopilotMonitor.Functions.Services } } - private static bool? DetectHasSSD(Dictionary data) + internal static bool? DetectHasSSD(Dictionary data) { try { @@ -222,9 +222,11 @@ namespace AutopilotMonitor.Functions.Services if (diskDict.TryGetValue("mediaType", out var mt)) { + // The agent reports composite media types such as "NVMe SSD", so match on + // substring rather than equality (an exact compare missed every real disk). var mtStr = mt?.ToString() ?? ""; - if (mtStr.Equals("SSD", StringComparison.OrdinalIgnoreCase) || - mtStr.Equals("NVMe", StringComparison.OrdinalIgnoreCase)) + if (mtStr.IndexOf("SSD", StringComparison.OrdinalIgnoreCase) >= 0 || + mtStr.IndexOf("NVMe", StringComparison.OrdinalIgnoreCase) >= 0) return true; } } diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.cs index 8179c589..dbd97273 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.cs @@ -266,16 +266,8 @@ namespace AutopilotMonitor.Functions.Services try { var deserialized = JsonConvert.DeserializeObject>(dataJson); - if (deserialized == null) - return new Dictionary(); - - // Convert all JToken values to native types - var result = new Dictionary(); - foreach (var kvp in deserialized) - { - result[kvp.Key] = ConvertJTokenToNative(kvp.Value); - } - return result; + // Convert all JToken values to native types (shared with the ingest paths). + return Functions.Ingest.EventDataNormalizer.NormalizeMap(deserialized); } catch { @@ -288,32 +280,6 @@ namespace AutopilotMonitor.Functions.Services } } - /// - /// Converts JToken objects (JArray, JObject) to native .NET types - /// This fixes the issue where Newtonsoft.Json deserialization creates JToken objects - /// that get serialized incorrectly as nested empty arrays - /// - private object ConvertJTokenToNative(object value) - { - if (value is JArray jArray) - { - return jArray.Select(item => ConvertJTokenToNative(item)).ToList(); - } - else if (value is JObject jObject) - { - var dict = new Dictionary(); - foreach (var prop in jObject.Properties()) - { - dict[prop.Name] = ConvertJTokenToNative(prop.Value); - } - return dict; - } - else if (value is JValue jValue) - { - return jValue.Value ?? string.Empty; - } - return value; - } } } From f67341dd70d394bc47c33649f60aab8bf8de7bb3 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Tue, 16 Jun 2026 17:30:11 +0200 Subject: [PATCH 2/2] perf(backend): derive maintenance tenant list from TenantConfiguration, not a Sessions scan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GetAllTenantIdsAsync answered "which tenants exist?" by scanning the entire Sessions table (PK projection) — an O(corpus) cost paid multiple times per 2h maintenance run by all 6 callers (stalled/excessive sweeps, retention fanout, platform-stats recompute, distress-report cleanup). It grows with session history rather than tenant count. Read the TenantConfiguration table instead (one "config" row per tenant, same proven pattern as TableConfigRepository). Same return shape, so all callers benefit with no interface or DI change. Trade-off (documented in code): a fully-offboarded tenant whose config row was deleted but whose old sessions still linger is no longer visited by maintenance sweeps. Acceptable — the offboarding cascade removes those sessions anyway, and a merely disabled tenant keeps its config row and is still processed. Not bundled here: day-windowing RecomputePlatformStatsAsync — its counters are cumulative/all-time and need separate semantic review. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Services/TableStorageService.Maintenance.cs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Maintenance.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Maintenance.cs index dfdbf80f..8fb8c4d1 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Maintenance.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Maintenance.cs @@ -616,14 +616,24 @@ namespace AutopilotMonitor.Functions.Services } /// - /// Gets all unique tenant IDs from sessions table + /// Gets all known tenant IDs from the TenantConfiguration table (one "config" row per + /// tenant). This replaces an O(corpus) scan of the entire Sessions table — the maintenance + /// pass only needs the tenant set, not every session, and the tenant set grows with the + /// number of onboarded tenants, not the session history. + /// + /// Trade-off: a tenant whose config row was deleted (full offboarding) but whose old + /// sessions still linger will no longer be visited by maintenance sweeps. That is + /// acceptable — the offboarding cascade removes those sessions anyway, and a merely + /// disabled tenant keeps its config row and is still processed. /// public async Task> GetAllTenantIdsAsync() { try { - var tableClient = _tableServiceClient.GetTableClient(Constants.TableNames.Sessions); - var query = tableClient.QueryAsync(select: new[] { "PartitionKey" }); + var tableClient = _tableServiceClient.GetTableClient(Constants.TableNames.TenantConfiguration); + var query = tableClient.QueryAsync( + filter: "RowKey eq 'config'", + select: new[] { "PartitionKey" }); var tenantIds = new HashSet(); await foreach (var entity in query)