improving idempotency of registration methods for AspNet and App Insights (#3273)

Author: brettsam

extensions/Worker.Extensions.Http.AspNetCore/src/FunctionsHostBuilderExtensions.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
 using System;
@@ -18,6 +18,8 @@ namespace Microsoft.Extensions.Hosting
     /// </summary>
     public static class FunctionsHostBuilderExtensions
     {
+        private const string AspNetCoreIntegrationConfiguredKey = "__FunctionsAspNetCoreConfigured";
+
         /// <summary>
         /// Configures the worker to use the ASP.NET Core integration, enabling advanced HTTP features.
         /// </summary>
@@ -63,6 +65,16 @@ public static IHostBuilder ConfigureFunctionsWebApplication(this IHostBuilder bu
 
         internal static IHostBuilder ConfigureAspNetCoreIntegration(this IHostBuilder builder)
         {
+            if (builder.Properties.TryGetValue(AspNetCoreIntegrationConfiguredKey, out var alreadyConfiguredObj) &&
+                alreadyConfiguredObj is bool alreadyConfigured &&
+                alreadyConfigured)
+            {
+                // Already configured, don't do it twice
+                return builder;
+            }
+
+            builder.Properties[AspNetCoreIntegrationConfiguredKey] = true;
+
             builder.ConfigureServices(services =>
             {
                 services.AddSingleton<FunctionsEndpointDataSource>();

extensions/Worker.Extensions.Http.AspNetCore/src/WorkerBuilderExtensions.cs

@@ -1,7 +1,8 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
 using System;
+using System.Linq;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.Azure.Functions.Worker.Extensions.Http.AspNetCore;
 using Microsoft.Extensions.DependencyInjection;
@@ -22,9 +23,12 @@ internal static class WorkerBuilderExtensions
         /// <exception cref="ArgumentNullException"></exception>
         internal static IFunctionsWorkerApplicationBuilder UseAspNetCoreIntegration(this IFunctionsWorkerApplicationBuilder builder)
         {
-            if (builder is null)
+            ArgumentNullException.ThrowIfNull(builder, nameof(builder));
+
+            // Check if already configured by looking for our middleware
+            if (builder.Services.Any(d => d.ImplementationType == typeof(FunctionsHttpProxyingMiddleware)))
             {
-                throw new ArgumentNullException(nameof(builder));
+                return builder;
             }
 
             builder.UseMiddleware<FunctionsHttpProxyingMiddleware>();
@@ -36,7 +40,7 @@ internal static IFunctionsWorkerApplicationBuilder UseAspNetCoreIntegration(this
             builder.Services.Configure<WorkerOptions>((workerOption) =>
             {
                 workerOption.InputConverters.RegisterAt<HttpContextConverter>(0);
-                workerOption.Capabilities.Add(Constants.HttpUriCapability, HttpUriProvider.HttpUriString);
+                workerOption.Capabilities[Constants.HttpUriCapability] = HttpUriProvider.HttpUriString;
             });
 
             return builder;

src/DotNetWorker.ApplicationInsights/FunctionsApplicationInsightsExtensions.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
 using System;
@@ -26,6 +26,12 @@ public static IServiceCollection ConfigureFunctionsApplicationInsights(this ISer
                 throw new ArgumentNullException(nameof(services));
             }
 
+            // Check if already configured by looking for our validation service
+            if (services.Any(d => d.ImplementationType == typeof(ApplicationInsightsValidationService)))
+            {
+                return services;
+            }
+
             services.ConfigureOptions<TelemetryConfigurationSetup>();
             services.AddSingleton<IConfigureOptions<AppServiceOptions>, AppServiceOptionsInitializer>();
             services.AddSingleton<AppServiceEnvironmentVariableMonitor>();

src/DotNetWorker.ApplicationInsights/release_notes.md

@@ -3,4 +3,5 @@
 ### Microsoft.Azure.Functions.Worker.ApplicationInsights <version>
 
 - Updating `Azure.Identity` from 1.12.0 to 1.17.0
-- Updating `Microsoft.ApplicationInsights.PerfCounterCollector` from 2.22.0 to 2.23.0
+- Updating `Microsoft.ApplicationInsights.PerfCounterCollector` from 2.22.0 to 2.23.0
+- Improved idempotency of service registration calls (#3273)

test/DotNetWorker.Tests/ApplicationInsights/ApplicationInsightsConfigurationTests.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using Microsoft.ApplicationInsights.Extensibility;
@@ -62,6 +62,8 @@ private static void Verify(IHostBuilder builder)
 
         builder.Build();
         Assert.Contains(typeof(FunctionsTelemetryInitializer), initializers);
+        Assert.Equal(6, initializers.Count());
         Assert.Contains(typeof(FunctionsTelemetryModule), modules);
+        Assert.Equal(8, modules.Count());
     }
 }

test/extensions/Worker.Extensions.Http.AspNetCore.Tests/FunctionsHostBuilderExtensionsTests.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
 using Microsoft.Azure.Functions.Worker;
@@ -58,6 +58,44 @@ public void ConfigureFunctionsWebApplication_ShouldConfigureFunctionsWebApplicat
             VerifyRegistrationOfAspNetCoreIntegrationServices(host);
         }
 
+        [Fact]
+        public void ConfigureFunctionsWebApplication_CalledTwice_ShouldBeIdempotent()
+        {
+            var builder = new HostBuilder();
+
+            int callbackCount = 0;
+
+            // Call ConfigureFunctionsWebApplication twice but should call callbacks each time
+            builder.ConfigureFunctionsWebApplication((_, _) => callbackCount++);
+            builder.ConfigureFunctionsWebApplication((_, _) => callbackCount++);
+
+            IServiceCollection serviceCollection = default!;
+            builder.ConfigureServices(services =>
+            {
+                serviceCollection = services;
+            });
+
+            var host = builder.Build();
+
+            // Verify services are registered
+            VerifyRegistrationOfAspNetCoreIntegrationServices(host);
+
+            Assert.Equal(2, callbackCount);
+
+            ValidateSingleServiceType(serviceCollection, typeof(FunctionsHttpProxyingMiddleware));
+            ValidateSingleServiceType(serviceCollection, typeof(IHttpCoordinator));
+            ValidateSingleServiceType(serviceCollection, typeof(FunctionsEndpointDataSource));
+
+            static void ValidateSingleServiceType(IServiceCollection services, Type type)
+            {
+                Assert.NotNull(services);
+                var coordinatorDescriptors = services
+                    .Where(d => d.ServiceType == type)
+                    .ToList();
+                Assert.Single(coordinatorDescriptors);
+            }
+        }
+
         private static void VerifyRegistrationOfCustomMiddleware(IHost host)
         {
             Assert.NotNull(host.Services.GetService<TestMiddleware>());