🐛 Do not share the broker data dictionary between usage reports (#5983)

The usage report used the broker data dictionary from the data store as
its environment data, and then added its own keys to it. When no broker
data is stored, the EF Core and RavenDB data stores return one static
default instance. All reports in the process then write into the same
dictionary. Two reports that are generated at the same time can show
values of the other report.

This made When_reporting_the_environment fail at random. The acceptance
tests run in parallel, so the report of one test could contain the
redirect count, the report mask count or the heartbeat values of another
test.

- The report copies the broker data into a new dictionary.
- The EF Core and RavenDB data stores return a new default instance.

Author: ramonsmits

src/Particular.LicensingComponent.UnitTests/ThroughputCollector/ThroughputCollector_Report_BrokerMetadata_Tests.cs

@@ -0,0 +1,33 @@
+namespace Particular.LicensingComponent.UnitTests;
+
+using System.Collections.Generic;
+using System.Threading.Tasks;
+using NUnit.Framework;
+using Particular.LicensingComponent.Contracts;
+using Particular.LicensingComponent.UnitTests.Infrastructure;
+
+/// <summary>
+/// The report adds its environment data to the broker data. The data store can return a shared
+/// instance, so the report must not change the broker data that the store holds. Otherwise two
+/// reports that are generated at the same time write into the same dictionary.
+/// </summary>
+[TestFixture]
+class ThroughputCollector_Report_BrokerMetadata_Tests : ThroughputCollectorTestFixture
+{
+    [Test]
+    public async Task Should_not_change_the_stored_broker_data()
+    {
+        await DataStore.SaveBrokerMetadata(new BrokerMetadata("scope", new Dictionary<string, string> { ["Broker.Key"] = "value" }));
+
+        var report = await ThroughputCollector.GenerateThroughputReport("1.2.3", null);
+
+        var stored = await DataStore.GetBrokerMetadata();
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(report.ReportData.EnvironmentInformation.EnvironmentData, Does.ContainKey("Broker.Key"), "The report includes the broker data");
+            Assert.That(report.ReportData.EnvironmentInformation.EnvironmentData, Does.ContainKey(EnvironmentDataType.ServicePulseVersion.ToString()));
+            Assert.That(stored.Data.Keys, Is.EquivalentTo(new[] { "Broker.Key" }), "The report must not add its keys to the stored broker data");
+        }
+    }
+}

src/Particular.LicensingComponent/ThroughputCollector.cs

@@ -168,7 +168,8 @@ public async Task<SignedReport> GenerateThroughputReport(string spVersion, DateT
             IgnoredQueues = [.. ignoredQueueNames],
             Queues = [.. queueThroughputs],
             TotalQueues = queueThroughputs.Count,
-            EnvironmentInformation = new EnvironmentInformation { AuditServicesData = new AuditServicesData(auditServiceMetadata.Versions, auditServiceMetadata.Transports), EnvironmentData = brokerMetaData.Data }
+            // A copy, because the report adds its own keys. The data store can return a shared instance.
+            EnvironmentInformation = new EnvironmentInformation { AuditServicesData = new AuditServicesData(auditServiceMetadata.Versions, auditServiceMetadata.Transports), EnvironmentData = new Dictionary<string, string>(brokerMetaData.Data) }
         };
 
         var auditThroughput = queueThroughputs.SelectMany(w => w.DailyThroughputFromAudit);

src/ServiceControl.Persistence.EFCore/Implementation/LicensingDataStore.cs

@@ -12,7 +12,6 @@ class LicensingDataStore(IServiceScopeFactory scopeFactory, TimeProvider timePro
 {
     static readonly string PlatformEndpointIndicator = EndpointIndicator.PlatformEndpoint.ToString();
     static readonly AuditServiceMetadata DefaultAuditServiceMetadata = new([], []);
-    static readonly BrokerMetadata DefaultBrokerMetadata = new(null, []);
 
     public Task<IEnumerable<Endpoint>> GetAllEndpoints(bool includePlatformEndpoints, CancellationToken cancellationToken = default) =>
         ExecuteWithDbContext(async (context, token) =>
@@ -291,7 +290,7 @@ Task<bool> IsThereThroughput(int days, ThroughputSource? throughputSource, bool
 
     public Task<BrokerMetadata> GetBrokerMetadata(CancellationToken cancellationToken = default) =>
         ExecuteWithDbContext(async (context, token) =>
-            await context.GetSetting<BrokerMetadata>(SettingKeys.BrokerMetadata, token) ?? DefaultBrokerMetadata, cancellationToken);
+            await context.GetSetting<BrokerMetadata>(SettingKeys.BrokerMetadata, token) ?? new BrokerMetadata(null, []), cancellationToken);
 
     public Task SaveBrokerMetadata(BrokerMetadata brokerMetadata, CancellationToken cancellationToken = default) =>
         ExecuteWithDbContext((context, token) => context.StoreSetting(SettingKeys.BrokerMetadata, brokerMetadata, token), cancellationToken);

src/ServiceControl.Persistence.RavenDB/Throughput/LicensingDataStore.cs

@@ -27,7 +27,6 @@ class LicensingDataStore(
     const string LicencedEndpointDetailsDocumentId = "LicensedEndpointDetails";
 
     static readonly AuditServiceMetadata DefaultAuditServiceMetadata = new([], []);
-    static readonly BrokerMetadata DefaultBrokerMetadata = new(null, []);
     static readonly ReportConfigurationDocument DefaultReportConfiguration = new();
 
     public async Task<IEnumerable<Endpoint>> GetAllEndpoints(bool includePlatformEndpoints, CancellationToken cancellationToken = default)
@@ -289,7 +288,7 @@ public async Task<BrokerMetadata> GetBrokerMetadata(CancellationToken cancellati
         var store = await storeProvider.GetDocumentStore(cancellationToken);
         using IAsyncDocumentSession session = store.OpenAsyncSession(databaseConfiguration.Name);
 
-        return await session.LoadAsync<BrokerMetadata>(BrokerMetadataDocumentId, cancellationToken) ?? DefaultBrokerMetadata;
+        return await session.LoadAsync<BrokerMetadata>(BrokerMetadataDocumentId, cancellationToken) ?? new BrokerMetadata(null, []);
     }
 
     public async Task SaveBrokerMetadata(BrokerMetadata brokerMetadata, CancellationToken cancellationToken = default)