Enrich internal custom checks with flag showing them as such

docs/data-versioning-design.md

@@ -12,6 +12,8 @@ This is the primary (error) instance only. The audit instance still carries a `s
 
 **If a field the response renders can change without the version changing, a client caches that page for ever and nothing reveals it.** No log line, no exception, no failing test.
 
+The rule covers fields that can change on their own. A field that is a **pure function of a covered field** cannot: it moves only when its source does, and the source already moves the version, so the page is never stale on the field's own account. `CustomCheckView.Internal` is that case — it is a computed, get-only property classified out of `CustomCheckId` at read time (see `InternalCustomCheckClassification`), which is itself a version term, and the view rather than the stored `CustomCheck` is what `/api/customchecks` renders. Being get-only, it cannot be assigned at all, so the reflection test above never sees it as a field that could drift. The one residual window is a ServiceControl upgrade that reclassifies while a client holds a pre-upgrade tag, and it closes itself: internal checks re-report every 5s to 1h, which moves `ReportedAt` and therefore the version.
+
 The promise is scoped to **one URL**, because a client only ever sends a validator back to the URL that issued it. So what must never happen is one URL answering `304` when its own body would have differed. Two different URLs sharing a value is harmless: an HTTP cache is keyed on the whole URL.
 
 That scoping is what makes a backend's own token usable. RavenDB's result etag stands for the state of the index behind the query, so it moves on any write the query could see, but it says nothing about which page was asked for: every `/api/errors` URL shares one value, whatever the page, sort or filter. The EF Core persisters compose over the rows they returned, so theirs differ per page. Both satisfy the rule.

src/ServiceControl.AcceptanceTests.RavenDB/DiagPath.cs

@@ -0,0 +1,45 @@
+namespace ServiceControl.AcceptanceTests.RavenDB
+{
+    using System;
+    using System.Diagnostics;
+    using NUnit.Framework;
+
+    [TestFixture]
+    class DiagPath
+    {
+        static void Run(string label, string file, string args)
+        {
+            Console.WriteLine($"=== {label}: {file} {args} ===");
+            var psi = new ProcessStartInfo(file, args) { RedirectStandardOutput = true, UseShellExecute = false, CreateNoWindow = true };
+            try
+            {
+                using var p = Process.Start(psi)!;
+                var output = p.StandardOutput.ReadToEnd();
+                p.WaitForExit();
+                foreach (var line in output.Split('\n'))
+                {
+                    if (line.Contains("NETCore.App") || line.Contains("runtimes installed") || line.Contains("Base Path"))
+                    {
+                        Console.WriteLine("  " + line.Trim());
+                    }
+                }
+            }
+            catch (Exception e)
+            {
+                Console.WriteLine("  FAILED: " + e.Message);
+            }
+        }
+
+        [Test]
+        public void PrintPathAndDotnetInfo()
+        {
+            Console.WriteLine("DIAG PATH=" + Environment.GetEnvironmentVariable("PATH"));
+            Console.WriteLine("DIAG CWD=" + Environment.CurrentDirectory);
+            Run("PATH-dotnet", "dotnet", "--info");
+            Run("tmp-wrap", "/tmp/wrap/dotnet", "--info");
+            Run("tmp-dotnet8", "/tmp/dotnet8/dotnet", "--info");
+            Run("home-dotnet", "/home/piuser/.dotnet/dotnet", "--info");
+            Assert.Pass();
+        }
+    }
+}

src/ServiceControl.AcceptanceTests.RavenDB/Monitoring/CustomChecks/When_a_persister_check_fails.cs

@@ -0,0 +1,67 @@
+namespace ServiceControl.AcceptanceTests.RavenDB.Monitoring.CustomChecks
+{
+    using System;
+    using System.Linq;
+    using System.Threading.Tasks;
+    using AcceptanceTesting;
+    using AcceptanceTesting.EndpointTemplates;
+    using NServiceBus;
+    using NServiceBus.AcceptanceTesting;
+    using NUnit.Framework;
+    using Operations;
+    using ServiceBus.Management.Infrastructure.Settings;
+    using CustomCheckView = global::ServiceControl.Contracts.CustomChecks.CustomCheckView;
+    using CheckStatus = global::ServiceControl.Persistence.Status;
+
+    // Sibling of When_critical_storage_threshold_reached: proves a
+    // persister-implemented internal check that is forced to fail comes back classified through the API.
+    // "ServiceControl database" cannot be forced to fail in this environment (the shared embedded server means
+    // UseEmbeddedServer is false, so CheckFreeDiskSpace always passes) — see plan §8.6.
+    [TestFixture]
+    class When_a_persister_check_fails : AcceptanceTest
+    {
+        [SetUp]
+        public void SetupIngestion() =>
+            SetSettings = static s =>
+            {
+                s.DisableHealthChecks = false;
+            };
+
+        RavenPersisterSettings PersisterSettings => (RavenPersisterSettings)Settings.PersisterSpecificSettings;
+
+        [Test]
+        public async Task Forced_failure_is_classified_internal()
+        {
+            CustomCheckView ingestionCheck = null;
+
+            await Define<ScenarioContext>()
+                .WithEndpoint<Sender>(b => b
+                    .When(context => context.Logs.ToArray().Any(i => i.Message.StartsWith(ErrorIngestion.LogMessages.StartedInfrastructure)),
+                        (_, _) =>
+                        {
+                            PersisterSettings.MinimumStorageLeftRequiredForIngestion = 100;
+                            PersisterSettings.DatabasePath = TestContext.CurrentContext.TestDirectory;
+                            return Task.CompletedTask;
+                        }))
+                .Done(async c =>
+                {
+                    var result = await this.TryGetSingle<CustomCheckView>("/api/customchecks", x => x.CustomCheckId == "Message Ingestion Process" && x.Status == CheckStatus.Fail);
+                    ingestionCheck = result;
+                    return result;
+                })
+                .Run();
+
+            using (Assert.EnterMultipleScope())
+            {
+                Assert.That(ingestionCheck, Is.Not.Null, "the forced storage-threshold failure never showed up");
+                Assert.That(ingestionCheck.Internal, Is.True);
+            }
+        }
+
+        public class Sender : EndpointConfigurationBuilder
+        {
+            public Sender() =>
+                EndpointSetup<DefaultServerWithoutAudit>(c => c.ReportCustomChecksTo(Settings.DEFAULT_INSTANCE_NAME, TimeSpan.FromSeconds(1)));
+        }
+    }
+}

src/ServiceControl.AcceptanceTests.RavenDB/ServiceControl.AcceptanceTests.RavenDB.csproj

@@ -36,6 +36,9 @@
 
     <!-- Error ingestion only mode is supported on SQL Server and PostgreSQL storage only. -->
     <Compile Remove="..\ServiceControl.AcceptanceTests\Recoverability\When_hosting_error_ingestion_only.cs" />
+
+    <!-- The file system body storage custom check is an EF Core persister feature; RavenDB stores bodies in the database. -->
+    <Compile Remove="..\ServiceControl.AcceptanceTests\Monitoring\CustomChecks\When_the_body_storage_check_is_reported.cs" />
   </ItemGroup>
 
   <ItemGroup>

src/ServiceControl.AcceptanceTests/Monitoring/CustomChecks/When_a_failing_custom_check_is_dismissed.cs

@@ -6,13 +6,14 @@ namespace ServiceControl.AcceptanceTests.Monitoring.CustomChecks
     using System.Threading.Tasks;
     using AcceptanceTesting;
     using AcceptanceTesting.EndpointTemplates;
+    using Contracts.CustomChecks;
     using NServiceBus;
     using NServiceBus.AcceptanceTesting;
     using NServiceBus.CustomChecks;
     using NUnit.Framework;
     using ServiceBus.Management.Infrastructure.Settings;
-    using CustomCheckView = global::ServiceControl.Contracts.CustomChecks.CustomCheck;
     using CheckStatus = global::ServiceControl.Persistence.Status;
+    using CustomCheck = NServiceBus.CustomChecks.CustomCheck;
 
     class When_a_failing_custom_check_is_dismissed : AcceptanceTest
     {

src/ServiceControl.AcceptanceTests/Monitoring/CustomChecks/When_custom_checks_are_classified.cs

@@ -0,0 +1,124 @@
+namespace ServiceControl.AcceptanceTests.Monitoring.CustomChecks
+{
+    using System;
+    using System.Linq;
+    using System.Threading;
+    using System.Threading.Tasks;
+    using AcceptanceTesting;
+    using AcceptanceTesting.EndpointTemplates;
+    using NServiceBus;
+    using NServiceBus.AcceptanceTesting;
+    using NServiceBus.CustomChecks;
+    using NUnit.Framework;
+    using ServiceBus.Management.Infrastructure.Settings;
+    using CustomCheckView = global::ServiceControl.Contracts.CustomChecks.CustomCheckView;
+    using CheckStatus = global::ServiceControl.Persistence.Status;
+
+    [TestFixture]
+    class When_custom_checks_are_classified : AcceptanceTest
+    {
+        // Runs at startup with TimeSpan.Zero, so acceptance tests can assert on it without waiting an interval.
+        const string InternalId = "ServiceControl Primary Instance";
+
+        [Test]
+        public async Task Internal_checks_are_flagged_internal_and_endpoint_checks_are_not()
+        {
+            // The acceptance test runner disables internal custom checks by default; this test needs them.
+            SetSettings = settings => { settings.DisableHealthChecks = false; };
+
+            CustomCheckView internalCheck = null;
+            CustomCheckView endpointCheck = null;
+            string wireBody = null;
+
+            await Define<Context>()
+                .WithEndpoint<EndpointWithFailingCustomCheck>()
+                .Done(async c =>
+                {
+                    var checks = await this.TryGetMany<CustomCheckView>("/api/customchecks");
+
+                    internalCheck ??= checks.Items.SingleOrDefault(x => x.CustomCheckId == InternalId);
+                    endpointCheck ??= checks.Items.SingleOrDefault(x => x.CustomCheckId == "MyCustomCheckId" && x.Status == CheckStatus.Fail);
+
+                    // The view computes Internal from the check id, so deserializing alone would not
+                    // prove the endpoint emits it. Grab the raw payload once and assert on the wire itself.
+                    if (internalCheck != null && endpointCheck != null && wireBody == null)
+                    {
+                        var raw = await this.GetRaw("/api/customchecks");
+                        wireBody = await raw.Content.ReadAsStringAsync();
+                    }
+
+                    return internalCheck != null && endpointCheck != null && wireBody != null;
+                })
+                .Run();
+
+            using (Assert.EnterMultipleScope())
+            {
+                Assert.That(internalCheck, Is.Not.Null, "primary internal checks report at startup; nothing was found");
+                Assert.That(internalCheck.Internal, Is.True);
+
+                Assert.That(endpointCheck, Is.Not.Null);
+                Assert.That(endpointCheck.Internal, Is.False);
+
+                // What the wire actually carries:
+                Assert.That(wireBody, Does.Contain("\"internal\":true"), "internal checks must render internal:true on the wire");
+                Assert.That(wireBody, Does.Contain("\"internal\":false"), "endpoint checks must render internal:false on the wire");
+            }
+        }
+
+        [Test]
+        public async Task Every_expected_internal_check_is_flagged_internal()
+        {
+            // The acceptance test runner disables internal custom checks by default; this test needs them.
+            SetSettings = settings => { settings.DisableHealthChecks = false; };
+
+            var expectedIds = new[]
+            {
+                "ServiceControl Primary Instance",
+                "ServiceControl Remotes",
+                "Saga Audit Configuration",
+                // RavenDB persister checks also assert here on the RavenDB acceptance variant:
+                "Error Message Ingestion Process",
+                "Error Message Ingestion",
+            };
+
+            var seen = new System.Collections.Generic.List<CustomCheckView>();
+
+            await Define<Context>()
+                .Done(async c =>
+                {
+                    var checks = await this.TryGetMany<CustomCheckView>("/api/customchecks");
+                    foreach (var item in checks.Items)
+                    {
+                        // The Done predicate polls, so keep one row per check id
+                        if (seen.All(s => s.Id != item.Id))
+                        {
+                            seen.Add(item);
+                        }
+                    }
+
+                    return expectedIds.All(e => seen.Any(s => s.CustomCheckId == e));
+                })
+                .Run();
+
+            foreach (var id in expectedIds)
+            {
+                var check = seen.Single(s => s.CustomCheckId == id);
+                Assert.That(check.Internal, Is.True, id);
+            }
+        }
+
+        class Context : ScenarioContext;
+
+        public class EndpointWithFailingCustomCheck : EndpointConfigurationBuilder
+        {
+            public EndpointWithFailingCustomCheck() =>
+                EndpointSetup<DefaultServerWithoutAudit>(c => c.ReportCustomChecksTo(Settings.DEFAULT_INSTANCE_NAME, TimeSpan.FromSeconds(1)));
+
+            class FailingCustomCheck() : CustomCheck("MyCustomCheckId", "MyCategory", TimeSpan.FromSeconds(1))
+            {
+                public override Task<CheckResult> PerformCheck(CancellationToken cancellationToken = default) =>
+                    Task.FromResult(CheckResult.Failed("Some reason"));
+            }
+        }
+    }
+}

src/ServiceControl.AcceptanceTests/Monitoring/CustomChecks/When_email_notifications_are_configured.cs

@@ -10,14 +10,15 @@ namespace ServiceControl.AcceptanceTests.Monitoring.CustomChecks
     using System.Threading.Tasks;
     using AcceptanceTesting;
     using AcceptanceTesting.EndpointTemplates;
+    using Contracts.CustomChecks;
     using NServiceBus;
     using NServiceBus.AcceptanceTesting;
     using NServiceBus.CustomChecks;
     using NUnit.Framework;
     using ServiceBus.Management.Infrastructure.Settings;
     using ServiceControl.Notifications;
-    using CustomCheckView = global::ServiceControl.Contracts.CustomChecks.CustomCheck;
     using CheckStatus = global::ServiceControl.Persistence.Status;
+    using CustomCheck = NServiceBus.CustomChecks.CustomCheck;
 
     class When_email_notifications_are_configured : AcceptanceTest
     {

src/ServiceControl.AcceptanceTests/Monitoring/CustomChecks/When_the_body_storage_check_is_reported.cs

@@ -0,0 +1,37 @@
+namespace ServiceControl.AcceptanceTests.Monitoring.CustomChecks
+{
+    using System.Threading.Tasks;
+    using AcceptanceTesting;
+    using NServiceBus.AcceptanceTesting;
+    using NUnit.Framework;
+    using CustomCheckView = global::ServiceControl.Contracts.CustomChecks.CustomCheckView;
+
+    [TestFixture]
+    class When_the_body_storage_check_is_reported : AcceptanceTest
+    {
+        [SetUp]
+        public void EnableInternalChecks() =>
+            SetSettings = static s => s.DisableHealthChecks = false;
+
+        [Test]
+        public async Task Should_be_classified_internal()
+        {
+            CustomCheckView bodyStorageCheck = null;
+
+            await Define<ScenarioContext>()
+                .Done(async c =>
+                {
+                    var result = await this.TryGetSingle<CustomCheckView>("/api/customchecks", x => x.CustomCheckId == "ServiceControl body storage");
+                    bodyStorageCheck = result;
+                    return result;
+                })
+                .Run();
+
+            using (Assert.EnterMultipleScope())
+            {
+                Assert.That(bodyStorageCheck, Is.Not.Null, "the EF Core body storage check never reported");
+                Assert.That(bodyStorageCheck.Internal, Is.True);
+            }
+        }
+    }
+}

src/ServiceControl.Audit.Persistence.Tests.RavenDB/ServiceControl.Audit.Persistence.Tests.RavenDB.csproj

@@ -23,6 +23,9 @@
   </ItemGroup>
 
   <ItemGroup>
+    <!-- The InternalCustomCheckClassification code does not exist in Audit, including it here allows an 
+         approval test instead of a very expensive acceptance test. -->
+    <Compile Include="..\ServiceControl.Persistence\InternalCustomCheckClassification.cs" />
     <Compile Include="..\ServiceControl.Audit.Persistence.Tests\*.cs" LinkBase="Shared" />
     <Compile Remove="..\ServiceControl.Audit.Persistence.Tests\PersistenceManifestLibraryTests.cs" />
   </ItemGroup>

src/ServiceControl.Audit.Persistence.Tests/CustomCheckTests.cs

@@ -2,6 +2,7 @@
 {
     using System;
     using System.Linq;
+    using Contracts.CustomChecks;
     using Microsoft.Extensions.DependencyInjection;
     using NServiceBus.CustomChecks;
     using NUnit.Framework;
@@ -17,8 +18,8 @@ public void VerifyCustomChecks() =>
                 string.Join(Environment.NewLine,
                     from check in ServiceProvider.GetServices<ICustomCheck>()
                     orderby check.Category, check.Id
-                    select $"{check.Category}: {check.Id}"
+                    select $"{check.Category}: {check.Id}{(InternalCustomCheckClassification.IsInternal(check.Id) ? "" : " - MISSING FROM InternalCustomCheckClassification")}"
                 )
             );
     }
-}
+}

src/ServiceControl.Audit.Persistence.Tests/ServiceControl.Audit.Persistence.Tests.csproj

@@ -24,6 +24,9 @@
   </ItemGroup>
 
   <ItemGroup>
+    <!-- The InternalCustomCheckClassification code does not exist in Audit, including it here allows an 
+         approval test instead of a very expensive acceptance test. -->
+    <Compile Include="..\ServiceControl.Persistence\InternalCustomCheckClassification.cs" />
     <Compile Include="..\ServiceControl.UnitTests\NUnitParallelRunnerSettings.cs" />
   </ItemGroup>
 </Project>