From 8058e2ea8adae0e1bbafac59f22aa3315e682c87 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Fri, 9 Oct 2026 16:19:54 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Do=20not=20share=20the=20broker?= =?UTF-8?q?=20data=20dictionary=20between=20usage=20reports?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- ...utCollector_Report_BrokerMetadata_Tests.cs | 33 +++++++++++++++++++ .../ThroughputCollector.cs | 3 +- .../Implementation/LicensingDataStore.cs | 3 +- .../Throughput/LicensingDataStore.cs | 3 +- 4 files changed, 37 insertions(+), 5 deletions(-) create mode 100644 src/Particular.LicensingComponent.UnitTests/ThroughputCollector/ThroughputCollector_Report_BrokerMetadata_Tests.cs diff --git a/src/Particular.LicensingComponent.UnitTests/ThroughputCollector/ThroughputCollector_Report_BrokerMetadata_Tests.cs b/src/Particular.LicensingComponent.UnitTests/ThroughputCollector/ThroughputCollector_Report_BrokerMetadata_Tests.cs new file mode 100644 index 0000000000..bcdb41edc2 --- /dev/null +++ b/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; + +/// +/// 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. +/// +[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 { ["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"); + } + } +} diff --git a/src/Particular.LicensingComponent/ThroughputCollector.cs b/src/Particular.LicensingComponent/ThroughputCollector.cs index e2186f6dda..31bc9744fd 100644 --- a/src/Particular.LicensingComponent/ThroughputCollector.cs +++ b/src/Particular.LicensingComponent/ThroughputCollector.cs @@ -168,7 +168,8 @@ public async Task 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(brokerMetaData.Data) } }; var auditThroughput = queueThroughputs.SelectMany(w => w.DailyThroughputFromAudit); diff --git a/src/ServiceControl.Persistence.EFCore/Implementation/LicensingDataStore.cs b/src/ServiceControl.Persistence.EFCore/Implementation/LicensingDataStore.cs index 2683c5069c..ba4d6ec45a 100644 --- a/src/ServiceControl.Persistence.EFCore/Implementation/LicensingDataStore.cs +++ b/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> GetAllEndpoints(bool includePlatformEndpoints, CancellationToken cancellationToken = default) => ExecuteWithDbContext(async (context, token) => @@ -291,7 +290,7 @@ Task IsThereThroughput(int days, ThroughputSource? throughputSource, bool public Task GetBrokerMetadata(CancellationToken cancellationToken = default) => ExecuteWithDbContext(async (context, token) => - await context.GetSetting(SettingKeys.BrokerMetadata, token) ?? DefaultBrokerMetadata, cancellationToken); + await context.GetSetting(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); diff --git a/src/ServiceControl.Persistence.RavenDB/Throughput/LicensingDataStore.cs b/src/ServiceControl.Persistence.RavenDB/Throughput/LicensingDataStore.cs index 6e14a20845..ad0a9e2b64 100644 --- a/src/ServiceControl.Persistence.RavenDB/Throughput/LicensingDataStore.cs +++ b/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> GetAllEndpoints(bool includePlatformEndpoints, CancellationToken cancellationToken = default) @@ -289,7 +288,7 @@ public async Task GetBrokerMetadata(CancellationToken cancellati var store = await storeProvider.GetDocumentStore(cancellationToken); using IAsyncDocumentSession session = store.OpenAsyncSession(databaseConfiguration.Name); - return await session.LoadAsync(BrokerMetadataDocumentId, cancellationToken) ?? DefaultBrokerMetadata; + return await session.LoadAsync(BrokerMetadataDocumentId, cancellationToken) ?? new BrokerMetadata(null, []); } public async Task SaveBrokerMetadata(BrokerMetadata brokerMetadata, CancellationToken cancellationToken = default)