From d7e178a96de8842b0639d5738a200103598aa80d Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Sun, 24 May 2026 14:33:08 -0700 Subject: [PATCH 1/3] Serialize CLI host tests to fix sample_rate race (#17450) Aspire.Cli.Tests.CliSmokeTests.MainReturnsExpectedExitCode(args: [], expectedExitCode: 1) intermittently failed in CI with: System.InvalidOperationException : 'The collection already contains item with same key microsoft.sample_rate' at System.Diagnostics.ActivityTagsCollection.Add(String, Object) at OpenTelemetry.Trace.TracerProviderSdk.ComputeActivitySamplingResult(...) at System.Diagnostics.ActivitySource.CreateActivity(...) at Aspire.Cli.Telemetry.AspireCliTelemetry.StartReportedActivity(...) Root cause: Azure.Monitor.OpenTelemetry.Exporter uses RateLimitedSampler by default, which (once its adaptive state matures ~200ms after creation) returns a SamplingResult with a 'microsoft.sample_rate' attribute. OpenTelemetry's TracerProviderSdk writes those attributes into the shared ActivityCreationOptions.SamplingTags via a hard Add (no TryAdd), and ActivitySource.CreateActivity reuses the same options across every registered listener. When two listeners on the 'Aspire.Cli.Reported' source both run such samplers, the second Add throws the duplicate-key exception. Aspire CLI production only ever has one live TelemetryManager (DI singleton), so the bug is invisible at runtime. xUnit v3 runs test classes in parallel by default, however, which allows two in-process host-building tests to race. Serialize them via a new CliHostTestCollection (DisableParallelization = true), mirroring the existing EnvVarMutatingTestCollection pattern. Apply [Collection] to: * CliSmokeTests (in-process Program.Main([])) * CliBootstrapTests (Program.BuildApplicationAsync) * TelemetryConfigurationTests (direct new TelemetryManager + host) SdkDumpCommandTests only invokes Program.Main inside RemoteExecutor.Invoke (separate process), so it does not contribute to the in-process race. Verified locally: * Repro with two live TelemetryManager instances + Thread.Sleep(300ms) deterministically reproduces the exact CI stack trace before the fix. * Full Aspire.Cli.Tests suite (3,660 tests) passes after the fix. * MainReturnsExpectedExitCode passes across 5 stress runs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/Aspire.Cli.Tests/CliBootstrapTests.cs | 1 + .../Aspire.Cli.Tests/CliHostTestCollection.cs | 44 +++++++++++++++++++ tests/Aspire.Cli.Tests/CliSmokeTests.cs | 1 + .../Telemetry/TelemetryConfigurationTests.cs | 1 + 4 files changed, 47 insertions(+) create mode 100644 tests/Aspire.Cli.Tests/CliHostTestCollection.cs diff --git a/tests/Aspire.Cli.Tests/CliBootstrapTests.cs b/tests/Aspire.Cli.Tests/CliBootstrapTests.cs index fee54f57f68..d35e73d21f7 100644 --- a/tests/Aspire.Cli.Tests/CliBootstrapTests.cs +++ b/tests/Aspire.Cli.Tests/CliBootstrapTests.cs @@ -18,6 +18,7 @@ namespace Aspire.Cli.Tests; /// , registered in DI by /// . /// +[Collection(CliHostTestCollection.Name)] public class CliBootstrapTests(ITestOutputHelper outputHelper) { private static readonly string[] s_fixedChannels = ["stable", "staging", "daily", "local"]; diff --git a/tests/Aspire.Cli.Tests/CliHostTestCollection.cs b/tests/Aspire.Cli.Tests/CliHostTestCollection.cs new file mode 100644 index 00000000000..c032dcd3e7d --- /dev/null +++ b/tests/Aspire.Cli.Tests/CliHostTestCollection.cs @@ -0,0 +1,44 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Aspire.Cli.Tests; + +/// +/// Collection definition that disables parallel execution for tests which +/// build the real Aspire CLI host (via +/// or ) or otherwise +/// create a live with the +/// Azure Monitor exporter enabled in-process. +/// +/// +/// +/// Tracks . +/// +/// +/// Azure.Monitor.OpenTelemetry.Exporter uses RateLimitedSampler by default, +/// which returns a SamplingResult containing a microsoft.sample_rate +/// attribute once its adaptive sampling state has matured (~200ms after the +/// sampler is constructed). OpenTelemetry's TracerProviderSdk then writes +/// those attributes into ActivityCreationOptions.SamplingTags using a hard +/// +/// (no TryAdd). ActivitySource.CreateActivity reuses the SAME +/// ActivityCreationOptions instance across every registered listener, so +/// when two listeners on the Aspire.Cli.Reported source both run samplers +/// that emit microsoft.sample_rate, the second Add throws +/// ("The collection already +/// contains item with same key 'microsoft.sample_rate'"). +/// +/// +/// Aspire CLI production never has more than one live +/// (registered as a DI singleton), so the bug is invisible at runtime. xUnit v3 +/// runs test classes in parallel by default, however, which lets two host-building +/// tests race in-process. Placing every such test class in this collection +/// serializes their execution and keeps at most one Azure Monitor TracerProvider +/// alive at a time, eliminating the duplicate sampling-tag race. +/// +/// +[CollectionDefinition(Name, DisableParallelization = true)] +public sealed class CliHostTestCollection +{ + public const string Name = "CliHostTests"; +} diff --git a/tests/Aspire.Cli.Tests/CliSmokeTests.cs b/tests/Aspire.Cli.Tests/CliSmokeTests.cs index 32b0f8ccbaf..d047ce9c457 100644 --- a/tests/Aspire.Cli.Tests/CliSmokeTests.cs +++ b/tests/Aspire.Cli.Tests/CliSmokeTests.cs @@ -6,6 +6,7 @@ namespace Aspire.Cli.Tests; +[Collection(CliHostTestCollection.Name)] public class CliSmokeTests(ITestOutputHelper outputHelper) { private static readonly RemoteInvokeOptions s_remoteInvokeOptions = new() diff --git a/tests/Aspire.Cli.Tests/Telemetry/TelemetryConfigurationTests.cs b/tests/Aspire.Cli.Tests/Telemetry/TelemetryConfigurationTests.cs index 9eaadbe2137..7a0a7549752 100644 --- a/tests/Aspire.Cli.Tests/Telemetry/TelemetryConfigurationTests.cs +++ b/tests/Aspire.Cli.Tests/Telemetry/TelemetryConfigurationTests.cs @@ -15,6 +15,7 @@ namespace Aspire.Cli.Tests.Telemetry; +[Collection(CliHostTestCollection.Name)] public class TelemetryConfigurationTests { private static async Task BuildHostAsync(Dictionary? config = null) From a16248d565ef962c6c17cd4554f42b659670c65a Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Sun, 24 May 2026 17:26:00 -0700 Subject: [PATCH 2/3] Switch CLI host test serialization to xunit.runner.json Per JamesNK and follow-up feedback on #17451, replace the in-code [Collection(CliHostTestCollection.Name)] approach with a project-local xunit.runner.json that disables test-collection parallelization for Aspire.Cli.Tests. This matches the existing repo convention used by Aspire.Cli.EndToEnd.Tests, Aspire.Templates.Tests, and others, and removes the need for any C# code annotations or environment-variable opt-out to work around the OpenTelemetry/Azure Monitor in-process 'microsoft.sample_rate' duplicate-key race (#17450). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/Aspire.Cli.Tests/CliBootstrapTests.cs | 1 - .../Aspire.Cli.Tests/CliHostTestCollection.cs | 44 ------------------- tests/Aspire.Cli.Tests/CliSmokeTests.cs | 1 - .../Telemetry/TelemetryConfigurationTests.cs | 1 - tests/Aspire.Cli.Tests/xunit.runner.json | 5 +++ 5 files changed, 5 insertions(+), 47 deletions(-) delete mode 100644 tests/Aspire.Cli.Tests/CliHostTestCollection.cs create mode 100644 tests/Aspire.Cli.Tests/xunit.runner.json diff --git a/tests/Aspire.Cli.Tests/CliBootstrapTests.cs b/tests/Aspire.Cli.Tests/CliBootstrapTests.cs index d35e73d21f7..fee54f57f68 100644 --- a/tests/Aspire.Cli.Tests/CliBootstrapTests.cs +++ b/tests/Aspire.Cli.Tests/CliBootstrapTests.cs @@ -18,7 +18,6 @@ namespace Aspire.Cli.Tests; /// , registered in DI by /// . /// -[Collection(CliHostTestCollection.Name)] public class CliBootstrapTests(ITestOutputHelper outputHelper) { private static readonly string[] s_fixedChannels = ["stable", "staging", "daily", "local"]; diff --git a/tests/Aspire.Cli.Tests/CliHostTestCollection.cs b/tests/Aspire.Cli.Tests/CliHostTestCollection.cs deleted file mode 100644 index c032dcd3e7d..00000000000 --- a/tests/Aspire.Cli.Tests/CliHostTestCollection.cs +++ /dev/null @@ -1,44 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace Aspire.Cli.Tests; - -/// -/// Collection definition that disables parallel execution for tests which -/// build the real Aspire CLI host (via -/// or ) or otherwise -/// create a live with the -/// Azure Monitor exporter enabled in-process. -/// -/// -/// -/// Tracks . -/// -/// -/// Azure.Monitor.OpenTelemetry.Exporter uses RateLimitedSampler by default, -/// which returns a SamplingResult containing a microsoft.sample_rate -/// attribute once its adaptive sampling state has matured (~200ms after the -/// sampler is constructed). OpenTelemetry's TracerProviderSdk then writes -/// those attributes into ActivityCreationOptions.SamplingTags using a hard -/// -/// (no TryAdd). ActivitySource.CreateActivity reuses the SAME -/// ActivityCreationOptions instance across every registered listener, so -/// when two listeners on the Aspire.Cli.Reported source both run samplers -/// that emit microsoft.sample_rate, the second Add throws -/// ("The collection already -/// contains item with same key 'microsoft.sample_rate'"). -/// -/// -/// Aspire CLI production never has more than one live -/// (registered as a DI singleton), so the bug is invisible at runtime. xUnit v3 -/// runs test classes in parallel by default, however, which lets two host-building -/// tests race in-process. Placing every such test class in this collection -/// serializes their execution and keeps at most one Azure Monitor TracerProvider -/// alive at a time, eliminating the duplicate sampling-tag race. -/// -/// -[CollectionDefinition(Name, DisableParallelization = true)] -public sealed class CliHostTestCollection -{ - public const string Name = "CliHostTests"; -} diff --git a/tests/Aspire.Cli.Tests/CliSmokeTests.cs b/tests/Aspire.Cli.Tests/CliSmokeTests.cs index d047ce9c457..32b0f8ccbaf 100644 --- a/tests/Aspire.Cli.Tests/CliSmokeTests.cs +++ b/tests/Aspire.Cli.Tests/CliSmokeTests.cs @@ -6,7 +6,6 @@ namespace Aspire.Cli.Tests; -[Collection(CliHostTestCollection.Name)] public class CliSmokeTests(ITestOutputHelper outputHelper) { private static readonly RemoteInvokeOptions s_remoteInvokeOptions = new() diff --git a/tests/Aspire.Cli.Tests/Telemetry/TelemetryConfigurationTests.cs b/tests/Aspire.Cli.Tests/Telemetry/TelemetryConfigurationTests.cs index 7a0a7549752..9eaadbe2137 100644 --- a/tests/Aspire.Cli.Tests/Telemetry/TelemetryConfigurationTests.cs +++ b/tests/Aspire.Cli.Tests/Telemetry/TelemetryConfigurationTests.cs @@ -15,7 +15,6 @@ namespace Aspire.Cli.Tests.Telemetry; -[Collection(CliHostTestCollection.Name)] public class TelemetryConfigurationTests { private static async Task BuildHostAsync(Dictionary? config = null) diff --git a/tests/Aspire.Cli.Tests/xunit.runner.json b/tests/Aspire.Cli.Tests/xunit.runner.json new file mode 100644 index 00000000000..dd80f43a679 --- /dev/null +++ b/tests/Aspire.Cli.Tests/xunit.runner.json @@ -0,0 +1,5 @@ +{ + "$schema": "https://xunit.net/schema/current/xunit.runner.schema.json", + "parallelizeAssembly": false, + "parallelizeTestCollections": false +} From f249f95b8e276861ec33c7b7b23f19f7909c54ba Mon Sep 17 00:00:00 2001 From: David Fowler Date: Sun, 24 May 2026 18:02:37 -0700 Subject: [PATCH 3/3] Opt CLI tests out of Azure Monitor to fix sample_rate race (#17450) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Aspire.Cli.Tests assembly now sets ASPIRE_CLI_TELEMETRY_OPTOUT=true process-wide via a [ModuleInitializer], so each in-process CliHost built by a test skips Azure Monitor by default. Tests that need to exercise the Azure-Monitor-enabled branch override the env-var-derived opt-out via the in-memory configuration passed to Program.BuildApplicationAsync (which is layered on top of AddEnvironmentVariables and therefore wins). This replaces the xunit.runner.json serialization workaround. With Azure Monitor disabled by default, only the few tests in TelemetryConfigurationTests build a TracerProvider — and those run serially within a single class — so the duplicate "microsoft.sample_rate" Add into the shared ActivityCreationOptions.SamplingTags can no longer occur across parallel test classes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Telemetry/TelemetryConfigurationTests.cs | 33 +++++++++++++++---- .../Aspire.Cli.Tests/TestTelemetryDefaults.cs | 33 +++++++++++++++++++ tests/Aspire.Cli.Tests/xunit.runner.json | 5 --- 3 files changed, 59 insertions(+), 12 deletions(-) create mode 100644 tests/Aspire.Cli.Tests/TestTelemetryDefaults.cs delete mode 100644 tests/Aspire.Cli.Tests/xunit.runner.json diff --git a/tests/Aspire.Cli.Tests/Telemetry/TelemetryConfigurationTests.cs b/tests/Aspire.Cli.Tests/Telemetry/TelemetryConfigurationTests.cs index 9eaadbe2137..6d9d74fbd8c 100644 --- a/tests/Aspire.Cli.Tests/Telemetry/TelemetryConfigurationTests.cs +++ b/tests/Aspire.Cli.Tests/Telemetry/TelemetryConfigurationTests.cs @@ -17,6 +17,23 @@ namespace Aspire.Cli.Tests.Telemetry; public class TelemetryConfigurationTests { + // The Aspire.Cli.Tests assembly opts out of Azure Monitor telemetry by default + // (see TestTelemetryDefaults). Tests that need the Azure Monitor branch override + // the env-var-derived opt-out by passing this in their in-memory configuration — + // AddInMemoryCollection is added AFTER AddEnvironmentVariables in + // Program.BuildApplicationAsync, so the in-memory value wins. + private static readonly KeyValuePair s_telemetryOptInOverride = + new(AspireCliTelemetry.TelemetryOptOutConfigKey, "false"); + + private static Dictionary WithTelemetryOptIn(Dictionary? config = null) + { + var result = config is null + ? new Dictionary() + : new Dictionary(config); + result[s_telemetryOptInOverride.Key] = s_telemetryOptInOverride.Value; + return result; + } + private static async Task BuildHostAsync(Dictionary? config = null) { var loggingOptions = Program.ParseLoggingOptions([]); @@ -30,9 +47,11 @@ private static async Task BuildHostAsync(Dictionary? con [Fact] public async Task AzureMonitor_Enabled_ByDefault() { - // The Application Insights connection string is now hardcoded, so Azure Monitor - // should be enabled by default when telemetry is not opted out - using var host = await BuildHostAsync(); + // The Application Insights connection string is hardcoded, so Azure Monitor + // should be enabled when telemetry is not opted out. The test process opts out + // by default (see TestTelemetryDefaults); we explicitly opt back in here to + // exercise the Azure Monitor branch. + using var host = await BuildHostAsync(WithTelemetryOptIn()); var telemetryManager = host.Services.GetService(); Assert.NotNull(telemetryManager); @@ -59,10 +78,10 @@ public async Task AzureMonitor_Disabled_WhenOptOutSetToTrueValues(string optOutV [Fact] public async Task OtlpExporter_WithoutProfiling_EnablesOnlyDebugDiagnostics_WhenEndpointProvided() { - var config = new Dictionary + var config = WithTelemetryOptIn(new Dictionary { [AspireCliTelemetry.OtlpExporterEndpointConfigKey] = "http://localhost:4317" - }; + }); using var host = await BuildHostAsync(config); @@ -122,11 +141,11 @@ public async Task OtlpExporter_WithProfiling_EnablesProfilingProviderWhenTelemet [Fact] public async Task OtlpExporter_WithProfiling_KeepsReportedTelemetryAndProfilingSeparate() { - var config = new Dictionary + var config = WithTelemetryOptIn(new Dictionary { [AspireCliTelemetry.OtlpExporterEndpointConfigKey] = "http://localhost:4317", [Aspire.Hosting.KnownConfigNames.ProfilingEnabled] = "true" - }; + }); using var host = await BuildHostAsync(config); diff --git a/tests/Aspire.Cli.Tests/TestTelemetryDefaults.cs b/tests/Aspire.Cli.Tests/TestTelemetryDefaults.cs new file mode 100644 index 00000000000..ab6892be1ed --- /dev/null +++ b/tests/Aspire.Cli.Tests/TestTelemetryDefaults.cs @@ -0,0 +1,33 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.CompilerServices; +using Aspire.Cli.Telemetry; + +namespace Aspire.Cli.Tests; + +/// +/// Opts the CLI out of Azure Monitor telemetry for the entire Aspire.Cli.Tests +/// process. Tests that need to exercise the Azure Monitor branch override this via the +/// in-memory configuration passed to , +/// which is layered on top of AddEnvironmentVariables() and therefore wins. +/// +/// Why opt out by default in tests: see https://github.com/microsoft/aspire/issues/17450. +/// Azure Monitor's default RateLimitedSampler emits a microsoft.sample_rate +/// attribute via ActivityCreationOptions.SamplingTags.Add (no TryAdd). When +/// xUnit v3 runs test classes in parallel and more than one TelemetryManager +/// builds a TracerProvider in the same process, two listeners are registered on the +/// Aspire.Cli.Reported ActivitySource and both samplers fire on the same shared +/// ActivityCreationOptions, so the second Add throws +/// InvalidOperationException("The collection already contains item with same key 'microsoft.sample_rate'"). +/// Defaulting the test process to opted-out keeps Azure Monitor out of the pipeline +/// except in the focused tests that explicitly need it. +/// +internal static class TestTelemetryDefaults +{ + [ModuleInitializer] + internal static void OptOutByDefault() + { + Environment.SetEnvironmentVariable(AspireCliTelemetry.TelemetryOptOutConfigKey, "true"); + } +} diff --git a/tests/Aspire.Cli.Tests/xunit.runner.json b/tests/Aspire.Cli.Tests/xunit.runner.json deleted file mode 100644 index dd80f43a679..00000000000 --- a/tests/Aspire.Cli.Tests/xunit.runner.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "$schema": "https://xunit.net/schema/current/xunit.runner.schema.json", - "parallelizeAssembly": false, - "parallelizeTestCollections": false -}