Fix ITelemetryModule singleton registration to support presence of ke…

…yed services (#2908) * Remove AddSingletonIfNotExists and call TryAddSingleton to fix bugs when keyed services are registered. * Test tweaks. * Tweaks and CHANGELOG update. --------- Co-authored-by: Timothy Mothra <[email protected]> Co-authored-by: Rajkumar Rangaraj <[email protected]>
microsoft · Feb 4, 2025 · 00f1e39 · 00f1e39
1 parent 183f4c7
commit 00f1e39
Show file tree

Hide file tree

Showing 7 changed files with 165 additions and 193 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## VNext
 - [Populate required field Message with "n/a" if it is empty](https://github.com/microsoft/ApplicationInsights-dotnet/issues/1066)
+- [Fix ITelemetryModule singleton registration to support presence of keyed services](https://github.com/microsoft/ApplicationInsights-dotnet/pull/2908)
 
 ## Version 2.22.0
 - no changes since beta.

diff --git a/NETCORE/src/Shared/Extensions/ApplicationInsightsExtensions.cs b/NETCORE/src/Shared/Extensions/ApplicationInsightsExtensions.cs
@@ -280,26 +280,6 @@ internal static void AddTelemetryConfiguration(
             }
         }
 
-        /// <summary>
-        /// The AddSingleton method will not check if a class has already been added as an ImplementationType.
-        /// This extension method is to encapsulate those checks.
-        /// </summary>
-        /// <remarks>
-        /// Must check all three properties to avoid duplicates or null ref exceptions.
-        /// </remarks>
-        /// <typeparam name="TService">The type of the service to add.</typeparam>
-        /// <typeparam name="TImplementation">The type of the implementation to use.</typeparam>
-        /// <param name="services">The Microsoft.Extensions.DependencyInjection.IServiceCollection to add the service to.</param>
-        internal static void AddSingletonIfNotExists<TService, TImplementation>(this IServiceCollection services)
-            where TService : class
-            where TImplementation : class, TService
-        {
-            if (!services.Any(o => o.ImplementationFactory == null && typeof(TImplementation).IsAssignableFrom(o.ImplementationType ?? o.ImplementationInstance.GetType())))
-            {
-                services.AddSingleton<TService, TImplementation>();
-            }
-        }
-
         private static bool TryGetValue(this IConfiguration config, string primaryKey, out string value, string backupKey = null)
         {
             value = config[primaryKey];
@@ -332,7 +312,7 @@ private static void AddCommonInitializers(IServiceCollection services)
         private static void AddCommonTelemetryModules(IServiceCollection services)
         {
             // Previously users were encouraged to manually add the DiagnosticsTelemetryModule.
-            services.AddSingletonIfNotExists<ITelemetryModule, DiagnosticsTelemetryModule>();
+            services.TryAddEnumerable(ServiceDescriptor.Singleton<ITelemetryModule, DiagnosticsTelemetryModule>());
 
             // These modules add properties to Heartbeat and expect the DiagnosticsTelemetryModule to be configured in DI.
             services.AddSingleton<ITelemetryModule, AppServicesHeartbeatTelemetryModule>();

diff --git a/...sts/Extensions/ApplicationInsightsExtensionsTests/AddApplicationInsightsTelemetryTests.cs b/...sts/Extensions/ApplicationInsightsExtensionsTests/AddApplicationInsightsTelemetryTests.cs
diff --git a/...ights.AspNetCore.Tests/Extensions/ApplicationInsightsExtensionsTests/AddSingletonTests.cs b/...ights.AspNetCore.Tests/Extensions/ApplicationInsightsExtensionsTests/AddSingletonTests.cs
diff --git a/...pplicationInsights.AspNetCore.Tests/Microsoft.ApplicationInsights.AspNetCore.Tests.csproj b/...pplicationInsights.AspNetCore.Tests/Microsoft.ApplicationInsights.AspNetCore.Tests.csproj
@@ -18,11 +18,12 @@
     </PackageReference>
     <PackageReference Include="xunit" Version="2.4.1" />
     <PackageReference Include="Moq" Version="4.16.1" />
+    <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="8.0.0" />
   </ItemGroup>
 
   <ItemGroup Condition="'$(IsNetCore)' == 'True'">
-    <!-- TODO: Can't switch to FrameworkReference yet; 
-         'IHostingEnvironment' is obsolete: 'This type is obsolete and will be removed in a future version. 
+    <!-- TODO: Can't switch to FrameworkReference yet;
+         'IHostingEnvironment' is obsolete: 'This type is obsolete and will be removed in a future version.
          The recommended alternative is Microsoft.AspNetCore.Hosting.IWebHostEnvironment.' -->
     <!--<FrameworkReference Include="Microsoft.AspNetCore.App" />-->