Switch attribution on and off without a restart

CHANGELOG.md

@@ -10,6 +10,17 @@ first.
 
 ### Added
 
+- `/pulse`, a server command behind the `controlserver` privilege, so per-mod tick attribution no
+  longer needs a restart to switch. `/pulse attribution on` and `off` drive the duty cycle on the
+  running server without writing `pulse.json`, `/pulse attribution status` reports the cycle in use
+  and the ticks it has profiled, and `/pulse reload` re-reads the config file and applies the
+  `Attribution` block live, naming any other key whose value in the file has drifted from what the
+  server is running. A file that fails to parse leaves everything as it was and the reply carries
+  the parse error. This is the shape a profiler wants: the moment you need attribution is while the
+  server is struggling, and a restart erases what you wanted to look at. The four families and the
+  frame profiler priming are now registered whether or not `Attribution.Enabled` is set, which is
+  what makes a later switch-on safe rather than merely likely to work; an instrument nothing has
+  recorded into is not a series, so an idle server serves the exposition it always did.
 - Config files are brought up to date at startup instead of only on first boot. Each mod compares
   the file on disk against the keys it knows and writes the missing ones back with their defaults,
   keeping every value already in the file, so a server upgrading from 0.1.0 gets the `Attribution`

Pulse.Scenarios/ConfigReloadScenarios.cs

@@ -0,0 +1,69 @@
+using Atlas.Api;
+using Atlas.XUnit;
+using Xunit;
+
+namespace Pulse.Scenarios;
+
+/// <summary><c>/pulse reload</c> against a real server: the file on disk is rewritten under a
+/// running server and read back through the same loader startup uses.</summary>
+[AtlasDataFiles("data/reload/pulse.json", TargetPath = "ModConfig")]
+public class ConfigReloadScenarios : AtlasScenarioBase
+{
+    private const int Port = 39474;
+
+    private static string Config(int port, bool attribution, int burstTicks) =>
+        $$"""
+        {
+          "Enabled": true,
+          "Bind": "127.0.0.1",
+          "Port": {{port}},
+          "RuntimeMetrics": false,
+          "ChunksRefreshSeconds": 30,
+          "Attribution": {
+            "Enabled": {{(attribution ? "true" : "false")}},
+            "BurstTicks": {{burstTicks}},
+            "IntervalSeconds": 1
+          }
+        }
+        """;
+
+    [AtlasScenario]
+    public async Task Reload_Applies_TheAttributionBlock_AndNamesWhatItCannot()
+    {
+        await World.Ticks(5);
+        string path = Path.Combine(World.Api.GetOrCreateDataPath("ModConfig"), "pulse.json");
+
+        File.WriteAllText(path, Config(Port, attribution: true, burstTicks: 7));
+        CommandResult applied = await World.ExecuteCommand("/pulse reload");
+
+        Assert.True(applied.Ok, applied.Message);
+        Assert.Equal(
+            "Reloaded pulse.json. Attribution is on: bursts of 7 ticks every 1s. "
+                + "Nothing else in the file differs from what the server is running.",
+            applied.Message);
+
+        // Not the tick count: the server is ticking while this reads, so a burst may well have
+        // landed between the reload and the question.
+        CommandResult status = await World.ExecuteCommand("/pulse attribution status");
+        Assert.StartsWith("Attribution is on: bursts of 7 ticks every 1s,", status.Message);
+
+        // A key that was wired into a socket at startup. Reload cannot move it, and the reply has
+        // to say which one rather than leave the operator wondering why nothing happened.
+        File.WriteAllText(path, Config(19999, attribution: true, burstTicks: 7));
+        CommandResult needsRestart = await World.ExecuteCommand("/pulse reload");
+
+        Assert.EndsWith("Port differs from what the server is running and needs a restart.", needsRestart.Message);
+        Assert.Contains("pulse_server_ticks_total", await Scrape.Metrics(Port));
+
+        // And a file nobody can read changes nothing at all.
+        File.WriteAllText(path, "this is not json");
+        CommandResult broken = await World.ExecuteCommand("/pulse reload");
+
+        Assert.False(broken.Ok);
+        Assert.StartsWith("Pulse could not read pulse.json (", broken.Message);
+        Assert.EndsWith("Nothing changed: the server is still running the config it booted with.", broken.Message);
+
+        CommandResult unchanged = await World.ExecuteCommand("/pulse attribution status");
+        Assert.StartsWith("Attribution is on: bursts of 7 ticks every 1s,", unchanged.Message);
+    }
+}

Pulse.Scenarios/HotToggleScenarios.cs

@@ -0,0 +1,97 @@
+using Atlas.Api;
+using Atlas.XUnit;
+using Xunit;
+
+namespace Pulse.Scenarios;
+
+/// <summary>Switching attribution on and off against a real engine, which is the only place the
+/// interesting half can be proven: that a server which booted with attribution off can still turn
+/// the engine's frame profiler on part-way through its life without dying, and turn it back off
+/// again. The unit suite can check the state machine; only a live server can check that the
+/// priming done at startup is what makes the later switch safe.
+/// <para>One scenario rather than several, because scenarios in a class share a world and this one
+/// is a sequence: off, on, measuring, off again.</para></summary>
+[AtlasDataFiles("data/hottoggle/pulse.json", TargetPath = "ModConfig")]
+public class HotToggleScenarios : AtlasScenarioBase
+{
+    private const int Port = 39473;
+
+    [AtlasScenario]
+    public async Task Attribution_SwitchesOnAndOff_WithoutARestart()
+    {
+        await World.Ticks(5);
+
+        // Armed, not running. The four families are registered so the switch has something to
+        // record into, and an instrument nothing has recorded into is not a series: a server that
+        // never asks for attribution serves the exposition it always did.
+        string idle = await Scrape.Metrics(Port);
+        Assert.DoesNotContain("pulse_mod_tick_share", idle);
+        Assert.DoesNotContain("pulse_attribution_ticks_total", idle);
+
+        CommandResult off = await World.ExecuteCommand("/pulse attribution status");
+        Assert.True(off.Ok, off.Message);
+        Assert.Equal(
+            "Attribution is off. It would run bursts of 5 ticks every 1s; 0 ticks profiled so far.",
+            off.Message);
+
+        CommandResult on = await World.ExecuteCommand("/pulse attribution on");
+        Assert.True(on.Ok, on.Message);
+        Assert.Equal(
+            "Attribution is on: bursts of 5 ticks every 1s. pulse.json was not changed, so the "
+                + "file decides again after a restart.",
+            on.Message);
+
+        // Seeded the moment it is switched on, rather than a burst later: a family that shows up
+        // mid-scrape is a family no dashboard plots.
+        string armed = await Scrape.Metrics(Port);
+        Assert.Contains("pulse_mod_tick_share{modid=\"engine\"} ", armed);
+        Assert.Equal(0, Scrape.Value(armed, "pulse_attribution_ticks_total"));
+
+        string body = await Burst();
+
+        // The server survived a profiler switched on mid-life, the marks parsed, and Pulse found
+        // itself in its own numbers.
+        Assert.InRange(Share(body, "pulse"), double.Epsilon, 1.0);
+
+        CommandResult stop = await World.ExecuteCommand("/pulse attribution off");
+        Assert.True(stop.Ok, stop.Message);
+        Assert.StartsWith("Attribution is off, and the engine's frame profiler with it.", stop.Message);
+
+        await World.Ticks(5);
+
+        // The flag actually went back down. Leaving it up would charge every later tick a few
+        // percent for a tree nobody reads.
+        Assert.False(World.Api.World.FrameProfiler.Enabled);
+
+        long profiled = (long)Scrape.Value(await Scrape.Metrics(Port), "pulse_attribution_ticks_total");
+        await World.Ticks(300);
+        Assert.Equal(profiled, (long)Scrape.Value(await Scrape.Metrics(Port), "pulse_attribution_ticks_total"));
+
+        CommandResult after = await World.ExecuteCommand("/pulse attribution status");
+        Assert.Equal(
+            $"Attribution is off. It would run bursts of 5 ticks every 1s; {profiled} ticks profiled so far.",
+            after.Message);
+    }
+
+    /// <summary>Ticks until a burst has completed, or gives up and fails with the body it last
+    /// saw. A burst needs its interval, then a discarded sample, then five profiled ticks.</summary>
+    private async Task<string> Burst()
+    {
+        string body = string.Empty;
+        for (int attempt = 0; attempt < 20; attempt++)
+        {
+            await World.Ticks(30);
+            body = await Scrape.Metrics(Port);
+            if (Scrape.Value(body, "pulse_attribution_ticks_total") > 0)
+            {
+                return body;
+            }
+        }
+
+        Assert.Fail("no burst ever completed:\n" + body);
+        return body;
+    }
+
+    private static double Share(string exposition, string modid)
+        => Scrape.Value(exposition, $"pulse_mod_tick_share{{modid=\"{modid}\"}}");
+}

Pulse.Scenarios/data/hottoggle/pulse.json

@@ -0,0 +1,12 @@
+{
+  "Enabled": true,
+  "Bind": "127.0.0.1",
+  "Port": 39473,
+  "RuntimeMetrics": false,
+  "ChunksRefreshSeconds": 30,
+  "Attribution": {
+    "Enabled": false,
+    "BurstTicks": 5,
+    "IntervalSeconds": 1
+  }
+}

Pulse.Scenarios/data/reload/pulse.json

@@ -0,0 +1,12 @@
+{
+  "Enabled": true,
+  "Bind": "127.0.0.1",
+  "Port": 39474,
+  "RuntimeMetrics": false,
+  "ChunksRefreshSeconds": 30,
+  "Attribution": {
+    "Enabled": false,
+    "BurstTicks": 5,
+    "IntervalSeconds": 1
+  }
+}

Pulse.Tests/PulseCommandsTests.cs

@@ -0,0 +1,126 @@
+using Xunit;
+
+namespace Pulse.Tests;
+
+/// <summary>The replies an operator reads off their console, and the comparison behind the one
+/// sentence that could quietly mislead them: which keys the file has moved away from and a reload
+/// cannot do anything about.</summary>
+public class PulseCommandsTests
+{
+    private static PulseConfig Config() => new()
+    {
+        Enabled = true,
+        Bind = "127.0.0.1",
+        Port = 9464,
+        RuntimeMetrics = true,
+        ChunksRefreshSeconds = 30,
+    };
+
+    [Fact]
+    public void RestartKeys_Reports_Nothing_WhenTheFileMatchesTheRunningServer()
+        => Assert.Empty(PulseCommands.RestartKeys(Config(), Config()));
+
+    /// <summary>The Attribution block is the one part a reload does apply, so a change there is
+    /// not a reason to restart and must not be named as one.</summary>
+    [Fact]
+    public void RestartKeys_Ignores_TheAttributionBlock()
+    {
+        PulseConfig loaded = Config();
+        loaded.Attribution = new AttributionConfig { Enabled = true, BurstTicks = 7, IntervalSeconds = 2 };
+
+        Assert.Empty(PulseCommands.RestartKeys(Config(), loaded));
+    }
+
+    [Fact]
+    public void RestartKeys_Names_EveryKeyThatIsReadOnlyAtStartup()
+    {
+        PulseConfig loaded = Config();
+        loaded.Enabled = false;
+        loaded.Bind = "0.0.0.0";
+        loaded.Port = 9999;
+        loaded.RuntimeMetrics = false;
+        loaded.ChunksRefreshSeconds = 5;
+
+        Assert.Equal(
+            ["Enabled", "Bind", "Port", "RuntimeMetrics", "ChunksRefreshSeconds"],
+            PulseCommands.RestartKeys(Config(), loaded));
+    }
+
+    [Fact]
+    public void RestartKeys_Names_OnlyTheKeysThatActuallyDiffer()
+    {
+        PulseConfig loaded = Config();
+        loaded.Port = 9999;
+
+        Assert.Equal(["Port"], PulseCommands.RestartKeys(Config(), loaded));
+    }
+
+    /// <summary>Switching attribution deliberately never writes the file, and the reply has to say
+    /// so: an operator who took a ten minute look must not find it still running next month.</summary>
+    [Fact]
+    public void Switched_Says_TheFileWasNotTouched()
+    {
+        Assert.Contains("pulse.json was not changed", PulseCommands.Switched(true, 30, 10));
+        Assert.Contains("pulse.json was not changed", PulseCommands.Switched(false, 30, 10));
+    }
+
+    [Fact]
+    public void Switched_Reports_TheDutyCycleItStarted()
+        => Assert.StartsWith("Attribution is on: bursts of 30 ticks every 10s.", PulseCommands.Switched(true, 30, 10));
+
+    [Fact]
+    public void Switched_Reports_TheProfilerGoingOffToo()
+        => Assert.StartsWith("Attribution is off, and the engine's frame profiler with it.", PulseCommands.Switched(false, 30, 10));
+
+    [Fact]
+    public void Status_Reports_TheCycle_TheTicksProfiled_AndTheBurstInProgress()
+        => Assert.Equal(
+            "Attribution is on: bursts of 30 ticks every 10s, 120 ticks profiled so far, profiling right now.",
+            PulseCommands.Status(true, 30, 10, 120, inBurst: true));
+
+    [Fact]
+    public void Status_Distinguishes_IdlingFromProfiling()
+        => Assert.Equal(
+            "Attribution is on: bursts of 30 ticks every 10s, 120 ticks profiled so far, waiting for the next burst.",
+            PulseCommands.Status(true, 30, 10, 120, inBurst: false));
+
+    /// <summary>Off, the cycle is what it would use, not what it is using, and the count is what a
+    /// previous stretch of profiling left behind.</summary>
+    [Fact]
+    public void Status_Reports_TheCycleItWouldUse_WhenItIsOff()
+        => Assert.Equal(
+            "Attribution is off. It would run bursts of 5 ticks every 1s; 40 ticks profiled so far.",
+            PulseCommands.Status(false, 5, 1, 40, inBurst: false));
+
+    [Fact]
+    public void Reloaded_Says_WhatItApplied_AndThatNothingElseMoved()
+        => Assert.Equal(
+            "Reloaded pulse.json. Attribution is on: bursts of 7 ticks every 2s. "
+                + "Nothing else in the file differs from what the server is running.",
+            PulseCommands.Reloaded(true, 7, 2, []));
+
+    [Fact]
+    public void Reloaded_Names_TheOneKeyThatNeedsARestart()
+        => Assert.EndsWith(
+            "Port differs from what the server is running and needs a restart.",
+            PulseCommands.Reloaded(false, 30, 10, ["Port"]));
+
+    [Fact]
+    public void Reloaded_Lists_SeveralKeysThatNeedARestart()
+        => Assert.EndsWith(
+            "Port, Bind differ from what the server is running and need a restart.",
+            PulseCommands.Reloaded(false, 30, 10, ["Port", "Bind"]));
+
+    /// <summary>The engine runs a command reply through string.Format on its way to whoever asked,
+    /// so the parse error goes in as a parameter. A sentence that interpolated it would throw on
+    /// the very typo it is reporting: a stray brace in the JSON.</summary>
+    [Fact]
+    public void ReloadFailed_Keeps_TheErrorAsAFormatParameter()
+    {
+        Assert.Contains("{0}", PulseCommands.ReloadFailed);
+        Assert.Equal(
+            "Pulse could not read pulse.json (Unexpected character: }). Nothing changed: the server "
+                + "is still running the config it booted with.",
+            string.Format(PulseCommands.ReloadFailed, "Unexpected character: }"));
+    }
+}