Apply the documented EventRetentionPeriod setting to event retention

docs/eventlog-design.md

@@ -44,7 +44,7 @@ Timestamps are when the thing happened, so an item can land in the middle of the
 
 ## Retention
 
-Items age out after `EventsRetentionPeriod`, 14 days by default. Users cannot remove an item, and no API exists for it. Enforcement is left to each storage backend and is invisible through the seam, so a change to the setting applies retrospectively on some backends and to new items only on others.
+Items age out on their own after `EventRetentionPeriod`, 14 days by default. An install that still sets the older `EventsRetentionPeriod` spelling keeps that value, and `EventRetentionPeriod` takes precedence when both are set. Nothing a user does removes one and there is no API to try. Enforcement is left to each storage backend and is invisible through the seam, so a change to the setting applies retrospectively on some backends and to new items only on others.
 
 ## Failure behavior
 

src/ServiceControl.Persistence.EFCore/Abstractions/EFPersistenceConfigurationBase.cs

@@ -26,7 +26,6 @@ public abstract class EFPersistenceConfigurationBase : PersistenceConfiguration,
     internal const string MinBodySizeForCompressionKey = "MessageBody/MinCompressionSize";
     const string MaxBodySizeToStoreKey = "MaxBodySizeToStore";
     const string ErrorRetentionPeriodKey = "ErrorRetentionPeriod";
-    const string EventsRetentionPeriodKey = "EventsRetentionPeriod";
     internal const string SubscriptionCacheDurationKey = "SubscriptionCacheDuration";
     const string ExternalIntegrationsDispatchingBatchSizeKey = "ExternalIntegrationsDispatchingBatchSize";
 
@@ -41,7 +40,7 @@ public PersistenceSettings CreateSettings(SettingsRootNamespace settingsRootName
         settings.Schema = ReadSchema(settingsRootNamespace);
         settings.CommandTimeout = SettingsReader.Read(settingsRootNamespace, CommandTimeoutKey, EFPersisterSettings.DefaultCommandTimeout);
         settings.ErrorRetentionPeriod = GetRequiredSetting<TimeSpan>(settingsRootNamespace, ErrorRetentionPeriodKey);
-        settings.EventsRetentionPeriod = SettingsReader.Read(settingsRootNamespace, EventsRetentionPeriodKey, EFPersisterSettings.DefaultEventsRetentionPeriod);
+        settings.EventsRetentionPeriod = ReadEventsRetentionPeriod(settingsRootNamespace, EFPersisterSettings.DefaultEventsRetentionPeriod);
         settings.SubscriptionCacheDuration = SettingsReader.Read(settingsRootNamespace, SubscriptionCacheDurationKey, EFPersisterSettings.DefaultSubscriptionCacheDuration);
         settings.ExternalIntegrationsDispatchingBatchSize = ReadExternalIntegrationsDispatchingBatchSize(settingsRootNamespace);
         settings.QueryTimeout = QueryTimeLimit.Read(settingsRootNamespace, LoggerUtil.CreateStaticLogger<EFPersistenceConfigurationBase>());

src/ServiceControl.Persistence.RavenDB/RavenPersistenceConfiguration.cs

@@ -14,7 +14,6 @@ class RavenPersistenceConfiguration : PersistenceConfiguration, IPersistenceConf
         public const string DataSpaceRemainingThresholdKey = "DataSpaceRemainingThreshold";
         const string AuditRetentionPeriodKey = "AuditRetentionPeriod";
         const string ErrorRetentionPeriodKey = "ErrorRetentionPeriod";
-        const string EventsRetentionPeriodKey = "EventsRetentionPeriod";
         const string ExternalIntegrationsDispatchingBatchSizeKey = "ExternalIntegrationsDispatchingBatchSize";
         const string MaintenanceModeKey = "MaintenanceMode";
 
@@ -38,7 +37,7 @@ public PersistenceSettings CreateSettings(SettingsRootNamespace settingsRootName
                 MinimumStorageLeftRequiredForIngestion = SettingsReader.Read(settingsRootNamespace, RavenBootstrapper.MinimumStorageLeftRequiredForIngestionKey, CheckMinimumStorageRequiredForIngestion.MinimumStorageLeftRequiredForIngestionDefault),
                 DataSpaceRemainingThreshold = SettingsReader.Read(settingsRootNamespace, DataSpaceRemainingThresholdKey, CheckFreeDiskSpace.DataSpaceRemainingThresholdDefault),
                 ErrorRetentionPeriod = GetRequiredSetting<TimeSpan>(settingsRootNamespace, ErrorRetentionPeriodKey),
-                EventsRetentionPeriod = SettingsReader.Read(settingsRootNamespace, EventsRetentionPeriodKey, TimeSpan.FromDays(14)),
+                EventsRetentionPeriod = ReadEventsRetentionPeriod(settingsRootNamespace, TimeSpan.FromDays(14)),
                 AuditRetentionPeriod = SettingsReader.Read(settingsRootNamespace, AuditRetentionPeriodKey, TimeSpan.Zero),
                 ExternalIntegrationsDispatchingBatchSize = ReadExternalIntegrationsDispatchingBatchSize(settingsRootNamespace),
                 QueryTimeout = QueryTimeLimit.Read(settingsRootNamespace, LoggerUtil.CreateStaticLogger<RavenPersistenceConfiguration>()),

src/ServiceControl.Persistence.Tests.RavenDB/EventsRetentionConfigurationTests.cs

@@ -0,0 +1,55 @@
+namespace ServiceControl.Persistence.Tests;
+
+using System;
+using NUnit.Framework;
+using ServiceControl.Configuration;
+using ServiceControl.Persistence.RavenDB;
+
+[TestFixture]
+[NonParallelizable]
+class RavenEventsRetentionConfigurationTests
+{
+    const string DocumentedVariable = "SERVICECONTROL_EVENTRETENTIONPERIOD";
+    const string LegacyVariable = "SERVICECONTROL_EVENTSRETENTIONPERIOD";
+    const string ErrorRetentionVariable = "SERVICECONTROL_ERRORRETENTIONPERIOD";
+
+    [SetUp]
+    public void SetUp() => Environment.SetEnvironmentVariable(ErrorRetentionVariable, "10.00:00:00");
+
+    [TearDown]
+    public void TearDown()
+    {
+        foreach (var variable in new[] { DocumentedVariable, LegacyVariable, ErrorRetentionVariable })
+        {
+            Environment.SetEnvironmentVariable(variable, null);
+        }
+    }
+
+    [Test]
+    public void Applies_the_documented_setting()
+    {
+        Environment.SetEnvironmentVariable(DocumentedVariable, "3.00:00:00");
+
+        Assert.That(CreateSettings().EventsRetentionPeriod, Is.EqualTo(TimeSpan.FromDays(3)));
+    }
+
+    [Test]
+    public void Applies_the_spelling_existing_installs_use()
+    {
+        Environment.SetEnvironmentVariable(LegacyVariable, "5.00:00:00");
+
+        Assert.That(CreateSettings().EventsRetentionPeriod, Is.EqualTo(TimeSpan.FromDays(5)));
+    }
+
+    [Test]
+    public void Prefers_the_documented_setting_over_the_existing_spelling()
+    {
+        Environment.SetEnvironmentVariable(DocumentedVariable, "3.00:00:00");
+        Environment.SetEnvironmentVariable(LegacyVariable, "5.00:00:00");
+
+        Assert.That(CreateSettings().EventsRetentionPeriod, Is.EqualTo(TimeSpan.FromDays(3)));
+    }
+
+    static RavenPersisterSettings CreateSettings() =>
+        (RavenPersisterSettings)new RavenPersistenceConfiguration().CreateSettings(new SettingsRootNamespace("ServiceControl"));
+}

src/ServiceControl.Persistence.Tests/EFCore/EventsRetentionConfigurationTests.cs

@@ -0,0 +1,85 @@
+namespace ServiceControl.Persistence.Tests;
+
+using System;
+using NUnit.Framework;
+using ServiceControl.Configuration;
+using ServiceControl.Persistence.EFCore.Abstractions;
+
+[TestFixture]
+[NonParallelizable]
+class EventsRetentionConfigurationTests
+{
+    static readonly SettingsRootNamespace TestNamespace = new("ServiceControl");
+
+    const string DocumentedVariable = "SERVICECONTROL_EVENTRETENTIONPERIOD";
+    const string LegacyVariable = "SERVICECONTROL_EVENTSRETENTIONPERIOD";
+
+    static readonly string[] Keys =
+    [
+        DocumentedVariable,
+        LegacyVariable,
+        "SERVICECONTROL_DATABASE_CONNECTIONSTRING",
+        "SERVICECONTROL_ERRORRETENTIONPERIOD",
+        "SERVICECONTROL_MESSAGEBODY_STORAGETYPE",
+        "SERVICECONTROL_MESSAGEBODY_FILESYSTEM_STORAGEPATH"
+    ];
+
+    [SetUp]
+    public void SetUp()
+    {
+        ClearKeys();
+        Environment.SetEnvironmentVariable("SERVICECONTROL_DATABASE_CONNECTIONSTRING", "Server=nowhere");
+        Environment.SetEnvironmentVariable("SERVICECONTROL_ERRORRETENTIONPERIOD", "10.00:00:00");
+        Environment.SetEnvironmentVariable("SERVICECONTROL_MESSAGEBODY_STORAGETYPE", "FileSystem");
+        Environment.SetEnvironmentVariable("SERVICECONTROL_MESSAGEBODY_FILESYSTEM_STORAGEPATH", "/var/bodies");
+    }
+
+    [TearDown]
+    public void TearDown() => ClearKeys();
+
+    static void ClearKeys()
+    {
+        foreach (var key in Keys)
+        {
+            Environment.SetEnvironmentVariable(key, null);
+        }
+    }
+
+    [Test]
+    public void Applies_the_documented_setting()
+    {
+        Environment.SetEnvironmentVariable(DocumentedVariable, "3.00:00:00");
+
+        Assert.That(CreateSettings().EventsRetentionPeriod, Is.EqualTo(TimeSpan.FromDays(3)));
+    }
+
+    [Test]
+    public void Applies_the_spelling_existing_installs_use()
+    {
+        Environment.SetEnvironmentVariable(LegacyVariable, "5.00:00:00");
+
+        Assert.That(CreateSettings().EventsRetentionPeriod, Is.EqualTo(TimeSpan.FromDays(5)));
+    }
+
+    [Test]
+    public void Prefers_the_documented_setting_over_the_existing_spelling()
+    {
+        Environment.SetEnvironmentVariable(DocumentedVariable, "3.00:00:00");
+        Environment.SetEnvironmentVariable(LegacyVariable, "5.00:00:00");
+
+        Assert.That(CreateSettings().EventsRetentionPeriod, Is.EqualTo(TimeSpan.FromDays(3)));
+    }
+
+    static EFPersisterSettings CreateSettings() =>
+        (EFPersisterSettings)new TestPersistenceConfiguration().CreateSettings(TestNamespace);
+
+    sealed class TestPersistenceConfiguration : EFPersistenceConfigurationBase
+    {
+        public override IPersistence Create(PersistenceSettings settings) => throw new NotSupportedException();
+
+        protected override EFPersisterSettings CreateSettings(string connectionString, BodyStorageSettings bodyStorage) =>
+            new TestPersisterSettings { ConnectionString = connectionString, BodyStorage = bodyStorage };
+    }
+
+    sealed class TestPersisterSettings : EFPersisterSettings;
+}

src/ServiceControl.Persistence/PersistenceConfiguration.cs

@@ -5,6 +5,9 @@ namespace ServiceControl.Persistence;
 
 public abstract class PersistenceConfiguration
 {
+    const string EventRetentionPeriodKey = "EventRetentionPeriod";
+    const string LegacyEventsRetentionPeriodKey = "EventsRetentionPeriod";
+
     protected static T GetRequiredSetting<T>(SettingsRootNamespace settingsRootNamespace, string key)
     {
         if (SettingsReader.TryRead<T>(settingsRootNamespace, key, out var value))
@@ -14,4 +17,9 @@ protected static T GetRequiredSetting<T>(SettingsRootNamespace settingsRootNames
 
         throw new Exception($"Setting {key} of type {typeof(T)} is required");
     }
+
+    protected static TimeSpan ReadEventsRetentionPeriod(SettingsRootNamespace settingsRootNamespace, TimeSpan defaultValue) =>
+        TimeSpan.TryParse(SettingsReader.Read<string>(settingsRootNamespace, EventRetentionPeriodKey), out var period)
+            ? period
+            : SettingsReader.Read(settingsRootNamespace, LegacyEventsRetentionPeriodKey, defaultValue);
 }

src/ServiceControl.UnitTests/Infrastructure/Settings/EventRetentionPeriodSettingsTests.cs

@@ -0,0 +1,37 @@
+namespace ServiceControl.UnitTests.Infrastructure.Settings;
+
+using System;
+using NUnit.Framework;
+using ServiceBus.Management.Infrastructure.Settings;
+
+[TestFixture]
+[NonParallelizable]
+class EventRetentionPeriodSettingsTests
+{
+    const string DocumentedVariable = "SERVICECONTROL_EVENTRETENTIONPERIOD";
+    const string LegacyVariable = "SERVICECONTROL_EVENTSRETENTIONPERIOD";
+
+    [TearDown]
+    public void TearDown()
+    {
+        Environment.SetEnvironmentVariable(DocumentedVariable, null);
+        Environment.SetEnvironmentVariable(LegacyVariable, null);
+    }
+
+    [Test]
+    public void Reports_the_spelling_existing_installs_use()
+    {
+        Environment.SetEnvironmentVariable(LegacyVariable, "5.00:00:00");
+
+        Assert.That(new Settings().EventsRetentionPeriod, Is.EqualTo(TimeSpan.FromDays(5)));
+    }
+
+    [Test]
+    public void Prefers_the_documented_setting_over_the_existing_spelling()
+    {
+        Environment.SetEnvironmentVariable(DocumentedVariable, "3.00:00:00");
+        Environment.SetEnvironmentVariable(LegacyVariable, "5.00:00:00");
+
+        Assert.That(new Settings().EventsRetentionPeriod, Is.EqualTo(TimeSpan.FromDays(3)));
+    }
+}

src/ServiceControl/Infrastructure/Settings/Settings.cs

@@ -302,7 +302,7 @@ TimeSpan GetEventRetentionPeriod()
                 }
             }
 
-            return TimeSpan.FromDays(14);
+            return SettingsReader.Read(SettingsRootNamespace, "EventsRetentionPeriod", TimeSpan.FromDays(14));
         }
 
         TimeSpan GetErrorRetentionPeriod()