diff --git a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt index 6a13cf62..1e6fd033 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt @@ -1 +1 @@ -443 +444 diff --git a/src/Agent/AutopilotMonitor.Agent/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent/buildcounter.txt index f8d65c31..b0536459 100644 --- a/src/Agent/AutopilotMonitor.Agent/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent/buildcounter.txt @@ -1 +1 @@ -1204 +1205 diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/RuleEngineCorrelationEvidenceSlimTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/RuleEngineCorrelationEvidenceSlimTests.cs new file mode 100644 index 00000000..545c03a9 --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/RuleEngineCorrelationEvidenceSlimTests.cs @@ -0,0 +1,174 @@ +using AutopilotMonitor.Functions.Services; +using AutopilotMonitor.Shared.DataAccess; +using AutopilotMonitor.Shared.Models; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Pins the slim-evidence contract for event_correlation rules. Before this fix the +/// per-pair evidence carried eventA_message / eventB_message plus a free-text +/// errorDetail field, and allMatches was uncapped — long-running stuck sessions +/// could push RuleResult.MatchedConditionsJson past Table Storage's 64KB property limit +/// and the persist would silently return false. The queue worker (post-fix) caught the false +/// and triggered a retry, but the underlying problem was still oversized evidence. +/// +public class RuleEngineCorrelationEvidenceSlimTests +{ + private const string TenantId = "a1b2c3d4-e5f6-7890-abcd-ef1234567890"; + private const string SessionId = "b2c3d4e5-f6a7-8901-bcde-f12345678901"; + + [Fact] + public async Task Correlation_evidence_omits_message_and_errorDetail_to_stay_slim() + { + var rule = BuildCorrelationRule(); + var baseTime = new DateTime(2026, 4, 28, 12, 0, 0, DateTimeKind.Utc); + var events = new List + { + BuildEvent("evt-A", "app_install_failed", appId: "abc", sequence: 1, timestamp: baseTime, + errorDetail: new string('x', 8000), message: "long error string " + new string('y', 8000)), + BuildEvent("evt-B", "app_install_completed", appId: "abc", sequence: 2, timestamp: baseTime.AddSeconds(5), + errorDetail: null, message: "completion details " + new string('z', 8000)), + }; + + var (engine, _) = SutWithRule(rule, events); + var outcome = await engine.AnalyzeSessionAsync(TenantId, SessionId); + + Assert.Single(outcome.Results); + var primary = (Dictionary)outcome.Results[0].MatchedConditions["enforcement_resolved"]; + + Assert.False(primary.ContainsKey("eventA_message"), "free-text message must not leak into evidence"); + Assert.False(primary.ContainsKey("eventB_message"), "free-text message must not leak into evidence"); + Assert.False(primary.ContainsKey("eventA_errorDetail"), "errorDetail (often stack trace) must not leak into evidence"); + Assert.False(primary.ContainsKey("eventB_errorDetail"), "errorDetail (often stack trace) must not leak into evidence"); + + // Identifying fields stay — they're short and necessary for the explanation. + Assert.Equal("evt-A", primary["eventA_eventId"]); + Assert.Equal("evt-B", primary["eventB_eventId"]); + Assert.Equal("abc", primary["eventA_appId"]); + Assert.Equal(1, primary["totalMatches"]); + } + + [Fact] + public async Task Correlation_with_many_pairs_caps_allMatches_at_ten_and_flags_truncated() + { + var rule = BuildCorrelationRule(); + var baseTime = new DateTime(2026, 4, 28, 12, 0, 0, DateTimeKind.Utc); + var events = new List(); + // 25 distinct app failures + 25 matching completions — uncapped this would be 25 pairs. + for (var i = 0; i < 25; i++) + { + events.Add(BuildEvent($"evt-A-{i}", "app_install_failed", appId: $"app-{i}", sequence: i * 2, timestamp: baseTime.AddSeconds(i * 10))); + events.Add(BuildEvent($"evt-B-{i}", "app_install_completed", appId: $"app-{i}", sequence: i * 2 + 1, timestamp: baseTime.AddSeconds(i * 10 + 5))); + } + + var (engine, _) = SutWithRule(rule, events); + var outcome = await engine.AnalyzeSessionAsync(TenantId, SessionId); + + Assert.Single(outcome.Results); + var primary = (Dictionary)outcome.Results[0].MatchedConditions["enforcement_resolved"]; + + Assert.Equal(25, primary["totalMatches"]); + Assert.True(primary.ContainsKey("allMatches")); + var allMatches = (List>)primary["allMatches"]; + Assert.Equal(10, allMatches.Count); + Assert.True((bool)primary["matchesTruncated"]); + } + + [Fact] + public async Task Correlation_with_few_pairs_does_not_truncate() + { + var rule = BuildCorrelationRule(); + var baseTime = new DateTime(2026, 4, 28, 12, 0, 0, DateTimeKind.Utc); + var events = new List(); + for (var i = 0; i < 3; i++) + { + events.Add(BuildEvent($"evt-A-{i}", "app_install_failed", appId: $"app-{i}", sequence: i * 2, timestamp: baseTime.AddSeconds(i * 10))); + events.Add(BuildEvent($"evt-B-{i}", "app_install_completed", appId: $"app-{i}", sequence: i * 2 + 1, timestamp: baseTime.AddSeconds(i * 10 + 5))); + } + + var (engine, _) = SutWithRule(rule, events); + var outcome = await engine.AnalyzeSessionAsync(TenantId, SessionId); + + Assert.Single(outcome.Results); + var primary = (Dictionary)outcome.Results[0].MatchedConditions["enforcement_resolved"]; + + Assert.Equal(3, primary["totalMatches"]); + var allMatches = (List>)primary["allMatches"]; + Assert.Equal(3, allMatches.Count); + Assert.False((bool)primary["matchesTruncated"]); + } + + // ============================================================ helpers + + private static (RuleEngine engine, Mock ruleRepo) SutWithRule(AnalyzeRule rule, List events) + { + var ruleRepo = new Mock(); + // The "global" partition holds built-in/community rules; the tenant-specific partition + // holds custom rules. AnalyzeRuleService merges both. We park the rule under "global" + // and return empty for the tenant partition so it doesn't show up twice. + ruleRepo.Setup(r => r.GetAnalyzeRulesAsync("global")).ReturnsAsync(new List { rule }); + ruleRepo.Setup(r => r.GetAnalyzeRulesAsync(It.Is(s => s != "global"))).ReturnsAsync(new List()); + ruleRepo.Setup(r => r.GetRuleStatesAsync(It.IsAny())).ReturnsAsync(new Dictionary()); + ruleRepo.Setup(r => r.GetRuleResultsAsync(It.IsAny(), It.IsAny())).ReturnsAsync(new List()); + + var sessionRepo = new Mock(); + sessionRepo.Setup(s => s.GetSessionEventsAsync(TenantId, SessionId, It.IsAny())).ReturnsAsync(events); + + var ruleService = new AnalyzeRuleService(ruleRepo.Object, NullLogger.Instance); + var engine = new RuleEngine(ruleService, ruleRepo.Object, sessionRepo.Object, NullLogger.Instance); + + return (engine, ruleRepo); + } + + private static AnalyzeRule BuildCorrelationRule() => new() + { + RuleId = "TEST-CORR-001", + Title = "Test correlation rule", + Severity = "warning", + Category = "apps", + Trigger = "correlation", + Enabled = true, + BaseConfidence = 80, + IsBuiltIn = false, + IsCommunity = false, + Conditions = new List + { + new() + { + Signal = "enforcement_resolved", + Source = "event_correlation", + EventType = "app_install_failed", + CorrelateEventType = "app_install_completed", + JoinField = "appId", + Required = true, + } + }, + ConfidenceFactors = new List(), + }; + + private static EnrollmentEvent BuildEvent(string eventId, string eventType, string appId, int sequence, DateTime timestamp, string? errorDetail = null, string? message = null) + { + var data = new Dictionary + { + ["appId"] = appId, + ["appName"] = $"App-{appId}", + ["errorPatternId"] = "IME-ERROR-ENFORCEMENT", + }; + if (errorDetail != null) + data["errorDetail"] = errorDetail; + + return new EnrollmentEvent + { + EventId = eventId, + TenantId = TenantId, + SessionId = SessionId, + EventType = eventType, + Sequence = sequence, + Timestamp = timestamp, + Message = message ?? string.Empty, + Data = data, + }; + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GetRuleResultsFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GetRuleResultsFunction.cs index 08126f07..87c7ee9f 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GetRuleResultsFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GetRuleResultsFunction.cs @@ -57,6 +57,13 @@ namespace AutopilotMonitor.Functions.Functions.Rules var reanalyze = string.Equals(req.Query["reanalyze"], "true", StringComparison.OrdinalIgnoreCase); + // Rules whose StoreRuleResultAsync returned false during the on-demand reanalyze. + // Reported back to the caller so the UI can render a warning banner without losing + // the rules that DID persist. Mirrors the queue-path's throw-on-false retry trigger, + // but stays partial-success here because the user is on a synchronous HTTP wait + // and a 500 would blank the page even when most rules persisted fine. + var persistFailureRuleIds = new List(); + if (reanalyze) { try @@ -69,10 +76,26 @@ namespace AutopilotMonitor.Functions.Functions.Rules foreach (var result in outcome.Results) { - await _ruleRepo.StoreRuleResultAsync(result); + var stored = await _ruleRepo.StoreRuleResultAsync(result); + if (!stored) + { + persistFailureRuleIds.Add(result.RuleId); + } } - _logger.LogInformation($"On-demand re-analysis for session {sessionId}: {outcome.Results.Count} issue(s) detected"); + if (persistFailureRuleIds.Count > 0) + { + _logger.LogError( + "On-demand re-analysis for session {SessionId}: {FailedCount} of {TotalCount} rule result(s) failed to persist: [{FailedRuleIds}]", + sessionId, + persistFailureRuleIds.Count, + outcome.Results.Count, + string.Join(", ", persistFailureRuleIds)); + } + else + { + _logger.LogInformation($"On-demand re-analysis for session {sessionId}: {outcome.Results.Count} issue(s) detected"); + } } catch (Exception ex) { @@ -85,13 +108,19 @@ namespace AutopilotMonitor.Functions.Functions.Rules var response = req.CreateResponse(HttpStatusCode.OK); await response.WriteAsJsonAsync(new { - success = true, + // success=false when any rule failed to persist during the reanalyze loop above. + // UI keeps showing the persisted results; consumers can branch on persistFailureCount + // to render a warning banner. persistFailureRuleIds is omitted entirely (null) when + // there were no failures, so the existing UI contract stays unchanged for the happy path. + success = persistFailureRuleIds.Count == 0, sessionId, results, totalIssues = results.Count, criticalCount = results.Count(r => r.Severity == "critical"), highCount = results.Count(r => r.Severity == "high"), - warningCount = results.Count(r => r.Severity == "warning") + warningCount = results.Count(r => r.Severity == "warning"), + persistFailureCount = persistFailureRuleIds.Count, + persistFailureRuleIds = persistFailureRuleIds.Count > 0 ? persistFailureRuleIds : null }); return response; } diff --git a/src/Backend/AutopilotMonitor.Functions/Services/RuleEngine.ConditionEvaluators.cs b/src/Backend/AutopilotMonitor.Functions/Services/RuleEngine.ConditionEvaluators.cs index 4f1bc077..6e2cb213 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/RuleEngine.ConditionEvaluators.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/RuleEngine.ConditionEvaluators.cs @@ -632,7 +632,13 @@ namespace AutopilotMonitor.Functions.Services continue; matchedJoinKeys.Add(bKey); - // Build evidence for this correlated pair + // Build evidence for this correlated pair. Keep it slim — store only event + // references (id/type/sequence/timestamp) plus the join key and a small set of + // short identifying data fields. The UI lazy-loads the full event via /events/{id} + // when the user expands the evidence row, so we don't need to embed Message or + // multi-KB stack-traces here. This caps per-pair evidence at ~500 bytes and keeps + // RuleResult.MatchedConditionsJson well below Table Storage's 64KB property limit + // even when many pairs match (e.g. ANALYZE-APP-012 on long-running stuck sessions). var pair = new Dictionary { ["joinField"] = condition.JoinField, @@ -641,16 +647,14 @@ namespace AutopilotMonitor.Functions.Services ["eventA_eventType"] = matchingA.EventType ?? string.Empty, ["eventA_timestamp"] = matchingA.Timestamp, ["eventA_sequence"] = matchingA.Sequence, - ["eventA_message"] = matchingA.Message ?? string.Empty, ["eventB_eventId"] = eventB.EventId ?? string.Empty, ["eventB_eventType"] = eventB.EventType ?? string.Empty, ["eventB_timestamp"] = eventB.Timestamp, ["eventB_sequence"] = eventB.Sequence, - ["eventB_message"] = eventB.Message ?? string.Empty, ["timeDeltaSeconds"] = (eventB.Timestamp - matchingA.Timestamp).TotalSeconds }; - // Add well-known data fields from both events + // Add slim identifying fields from both events (no free-text fields). AddDataFieldsToEvidence(pair, matchingA, "eventA_", condition.JoinField); AddDataFieldsToEvidence(pair, eventB, "eventB_", condition.JoinField); @@ -660,24 +664,41 @@ namespace AutopilotMonitor.Functions.Services if (!matchedPairs.Any()) return (false, "no correlated event pairs found"); - // Single match: flat dictionary; multiple: first as primary + allMatches list + // Single match: flat dictionary; multiple: first as primary + capped allMatches list. + // Cap protects against runaway evidence size — sessions with hundreds of correlated + // pairs would otherwise produce MatchedConditionsJson > 64KB and fail Table Storage + // persistence (see also defense-in-depth check in TableStorageService.StoreRuleResultAsync). + const int MaxAllMatches = 10; var primary = matchedPairs[0]; primary["totalMatches"] = matchedPairs.Count; if (matchedPairs.Count > 1) { - primary["allMatches"] = matchedPairs; + if (matchedPairs.Count > MaxAllMatches) + { + primary["allMatches"] = matchedPairs.Take(MaxAllMatches).ToList(); + primary["matchesTruncated"] = true; + } + else + { + primary["allMatches"] = matchedPairs; + primary["matchesTruncated"] = false; + } } return (true, primary); } /// - /// Adds well-known data fields from an event to the evidence dictionary with a prefix. + /// Adds slim identifying data fields from an event to the evidence dictionary with a prefix. + /// Free-text fields like errorDetail (often a stack trace) and message are + /// deliberately excluded to keep below Table + /// Storage's 64KB property limit. UI fetches the full event on demand via /events/{id}. /// private void AddDataFieldsToEvidence(Dictionary evidence, EnrollmentEvent evt, string prefix, string joinField) { if (evt.Data == null) return; - var knownFields = new[] { "appId", "appName", "errorPatternId", "errorDetail", "errorCode", "exitCode", "status" }; + // Whitelist: short identifiers only. errorDetail/message removed (can be multi-KB). + var knownFields = new[] { "appId", "appName", "errorPatternId", "errorCode", "exitCode", "status" }; foreach (var field in knownFields) { diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Rules.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Rules.cs index 0939dbb0..3091551e 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Rules.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Rules.cs @@ -14,9 +14,24 @@ namespace AutopilotMonitor.Functions.Services { // ===== RULE RESULTS METHODS ===== + /// + /// Azure Table Storage hard-limits each property to 64 KiB (UTF-16). We trip the + /// guard rail at 60 KiB to leave headroom for the property-name prefix + framing + /// bytes the storage layer adds on the wire. + /// + private const int MatchedConditionsJsonByteLimit = 60 * 1024; + /// /// Stores a rule evaluation result /// PartitionKey: {TenantId}_{SessionId}, RowKey: RuleId + /// + /// Defense-in-depth: rule authors should keep + /// under bytes (correlation evidence + /// already caps and slims its pairs). If a future rule still produces oversized + /// output, we substitute a truncation marker rather than letting the entire write + /// fail — the rule still fires, the UI sees the explanation + flag, and we get an + /// observable warning instead of an opaque store failure. + /// /// public async Task StoreRuleResultAsync(RuleResult result) { @@ -25,6 +40,22 @@ namespace AutopilotMonitor.Functions.Services var tableClient = _tableServiceClient.GetTableClient(Constants.TableNames.RuleResults); var partitionKey = $"{result.TenantId}_{result.SessionId}"; + var matchedJson = JsonConvert.SerializeObject(result.MatchedConditions ?? new Dictionary()); + if (matchedJson.Length > MatchedConditionsJsonByteLimit) + { + _logger.LogWarning( + "Rule result {RuleId} (session {SessionId}) MatchedConditionsJson is {Bytes} bytes, exceeds {Limit} byte guard. Substituting truncation marker so the rule still persists.", + result.RuleId, result.SessionId, matchedJson.Length, MatchedConditionsJsonByteLimit); + + var truncated = new Dictionary + { + ["_truncated"] = true, + ["_originalBytes"] = matchedJson.Length, + ["_reason"] = "MatchedConditions exceeded Table Storage 64KB property limit" + }; + matchedJson = JsonConvert.SerializeObject(truncated); + } + var entity = new TableEntity(partitionKey, result.RuleId) { ["ResultId"] = result.ResultId, @@ -38,7 +69,7 @@ namespace AutopilotMonitor.Functions.Services ["Explanation"] = result.Explanation ?? string.Empty, ["RemediationJson"] = JsonConvert.SerializeObject(result.Remediation ?? new List()), ["RelatedDocsJson"] = JsonConvert.SerializeObject(result.RelatedDocs ?? new List()), - ["MatchedConditionsJson"] = JsonConvert.SerializeObject(result.MatchedConditions ?? new Dictionary()), + ["MatchedConditionsJson"] = matchedJson, ["DetectedAt"] = result.DetectedAt };