Address LanguageWorkerOptions null in FunctionMetadataManager (#10550)

* Address language options null by consuming from parent services

* Update unit tests to calculate LanguageWorkerOptions

* Address merge issue

* Update languageWorkerOptions ref on host change

* Revert to single monitoring of LanguageWorkerOptions

* Remove LanguageWorkerOptions from ScriptStartupTypeLocator

* Mark readonly

Author: jviau

src/WebJobs.Script/DependencyInjection/ScriptStartupTypeLocator.cs

@@ -38,20 +38,18 @@ public class ScriptStartupTypeLocator : IWebJobsStartupTypeLocator
         private readonly IFunctionMetadataManager _functionMetadataManager;
         private readonly IMetricsLogger _metricsLogger;
         private readonly Lazy<IEnumerable<Type>> _startupTypes;
-        private readonly IOptionsMonitor<LanguageWorkerOptions> _languageWorkerOptions;
         private readonly IOptions<ExtensionRequirementOptions> _extensionRequirementOptions;
         private static string[] _builtinExtensionAssemblies = GetBuiltinExtensionAssemblies();
 
         public ScriptStartupTypeLocator(string rootScriptPath, ILogger<ScriptStartupTypeLocator> logger, IExtensionBundleManager extensionBundleManager,
-            IFunctionMetadataManager functionMetadataManager, IMetricsLogger metricsLogger, IOptionsMonitor<LanguageWorkerOptions> languageWorkerOptions, IOptions<ExtensionRequirementOptions> extensionRequirementOptions)
+            IFunctionMetadataManager functionMetadataManager, IMetricsLogger metricsLogger, IOptions<ExtensionRequirementOptions> extensionRequirementOptions)
         {
             _rootScriptPath = rootScriptPath ?? throw new ArgumentNullException(nameof(rootScriptPath));
             _extensionBundleManager = extensionBundleManager ?? throw new ArgumentNullException(nameof(extensionBundleManager));
             _logger = logger;
             _functionMetadataManager = functionMetadataManager;
             _metricsLogger = metricsLogger;
             _startupTypes = new Lazy<IEnumerable<Type>>(() => GetExtensionsStartupTypesAsync().ConfigureAwait(false).GetAwaiter().GetResult());
-            _languageWorkerOptions = languageWorkerOptions;
             _extensionRequirementOptions = extensionRequirementOptions;
         }
 
@@ -84,18 +82,17 @@ public async Task<IEnumerable<Type>> GetExtensionsStartupTypesAsync()
             bool isPrecompiledFunctionApp = false;
 
             // dotnet app precompiled -> Do not use bundles
-            var workerConfigs = _languageWorkerOptions.CurrentValue.WorkerConfigs;
             ExtensionRequirementsInfo extensionRequirements = GetExtensionRequirementsInfo();
             ImmutableArray<FunctionMetadata> functionMetadataCollection = ImmutableArray<FunctionMetadata>.Empty;
             if (bundleConfigured)
             {
                 ExtensionBundleDetails bundleDetails = await _extensionBundleManager.GetExtensionBundleDetails();
                 ValidateBundleRequirements(bundleDetails, extensionRequirements);
 
-                functionMetadataCollection = _functionMetadataManager.GetFunctionMetadata(forceRefresh: true, includeCustomProviders: false, workerConfigs: workerConfigs);
+                functionMetadataCollection = _functionMetadataManager.GetFunctionMetadata(forceRefresh: true, includeCustomProviders: false);
                 bindingsSet = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
 
-                // Generate a Hashset of all the binding types used in the function app
+                // Generate a HashSet of all the binding types used in the function app
                 foreach (var functionMetadata in functionMetadataCollection)
                 {
                     foreach (var binding in functionMetadata.Bindings)
@@ -113,7 +110,7 @@ public async Task<IEnumerable<Type>> GetExtensionsStartupTypesAsync()
             if (SystemEnvironment.Instance.IsPlaceholderModeEnabled())
             {
                 // Do not move this.
-                // Calling this log statement in the placeholder mode to avoid jitting during specializtion
+                // Calling this log statement in the placeholder mode to avoid jitting during specialization
                 _logger.ScriptStartNotLoadingExtensionBundle("WARMUP_LOG_ONLY", bundleConfigured, isPrecompiledFunctionApp, isLegacyExtensionBundle, isDotnetIsolatedApp, isLogicApp);
             }
 

src/WebJobs.Script/Host/FunctionMetadataManager.cs

@@ -7,40 +7,44 @@
 using System.Collections.Immutable;
 using System.IO;
 using System.Linq;
-using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Azure.WebJobs.Logging;
-using Microsoft.Azure.WebJobs.Script.Configuration;
 using Microsoft.Azure.WebJobs.Script.Description;
 using Microsoft.Azure.WebJobs.Script.Diagnostics.Extensions;
-using Microsoft.Azure.WebJobs.Script.Workers;
 using Microsoft.Azure.WebJobs.Script.Workers.Http;
 using Microsoft.Azure.WebJobs.Script.Workers.Rpc;
-using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 
 namespace Microsoft.Azure.WebJobs.Script
 {
-    public class FunctionMetadataManager : IFunctionMetadataManager
+    public sealed class FunctionMetadataManager : IFunctionMetadataManager, IDisposable
     {
         private const string FunctionConfigurationErrorMessage = "Unable to determine the primary function script. Make sure at least one script file is present. Try renaming your entry point script to 'run' or alternatively you can specify the name of the entry point script explicitly by adding a 'scriptFile' property to your function metadata.";
         private const string MetadataProviderName = "Custom";
         private readonly IServiceProvider _serviceProvider;
-        private IFunctionMetadataProvider _functionMetadataProvider;
+        private readonly IFunctionMetadataProvider _functionMetadataProvider;
+        private readonly IEnvironment _environment;
+
+        private readonly IOptionsMonitor<LanguageWorkerOptions> _languageOptions;
+        private IDisposable _onChangeSubscription;
+        private IOptions<ScriptJobHostOptions> _scriptOptions;
+        private ILogger _logger;
         private bool _isHttpWorker;
-        private IEnvironment _environment;
         private bool _servicesReset = false;
-        private ILogger _logger;
-        private IOptions<ScriptJobHostOptions> _scriptOptions;
         private ImmutableArray<FunctionMetadata> _functionMetadataArray;
         private Dictionary<string, ICollection<string>> _functionErrors = new Dictionary<string, ICollection<string>>();
         private ConcurrentDictionary<string, FunctionMetadata> _functionMetadataMap = new ConcurrentDictionary<string, FunctionMetadata>(StringComparer.OrdinalIgnoreCase);
 
-        public FunctionMetadataManager(IOptions<ScriptJobHostOptions> scriptOptions, IFunctionMetadataProvider functionMetadataProvider,
-            IOptions<HttpWorkerOptions> httpWorkerOptions, IScriptHostManager scriptHostManager, ILoggerFactory loggerFactory,
-            IEnvironment environment)
+        public FunctionMetadataManager(
+            IOptions<ScriptJobHostOptions> scriptOptions,
+            IFunctionMetadataProvider functionMetadataProvider,
+            IOptions<HttpWorkerOptions> httpWorkerOptions,
+            IScriptHostManager scriptHostManager,
+            ILoggerFactory loggerFactory,
+            IEnvironment environment,
+            IOptionsMonitor<LanguageWorkerOptions> languageOptions)
         {
             _scriptOptions = scriptOptions;
             _serviceProvider = scriptHostManager as IServiceProvider;
@@ -49,6 +53,9 @@ public FunctionMetadataManager(IOptions<ScriptJobHostOptions> scriptOptions, IFu
             _isHttpWorker = httpWorkerOptions?.Value?.Description != null;
             _environment = environment;
 
+            _languageOptions = languageOptions;
+            _onChangeSubscription = languageOptions.OnChange(_ => _servicesReset = true);
+
             // Every time script host is re-initializing, we also need to re-initialize
             // services that change with the scope of the script host.
             scriptHostManager.ActiveHostChanged += (s, e) =>
@@ -60,8 +67,10 @@ public FunctionMetadataManager(IOptions<ScriptJobHostOptions> scriptOptions, IFu
             };
         }
 
+        /// <inheritdoc />
         public ImmutableDictionary<string, ImmutableArray<string>> Errors { get; private set; }
 
+        /// <inheritdoc />
         public bool TryGetFunctionMetadata(string functionName, out FunctionMetadata functionMetadata, bool forceRefresh)
         {
             if (forceRefresh)
@@ -85,18 +94,21 @@ public bool TryGetFunctionMetadata(string functionName, out FunctionMetadata fun
         /// <param name="applyAllowList">Apply functions allow list filter.</param>
         /// <param name="includeCustomProviders">Include any metadata provided by IFunctionProvider when loading the metadata.</param>
         /// <returns> An Immutable array of FunctionMetadata.</returns>
-        public ImmutableArray<FunctionMetadata> GetFunctionMetadata(bool forceRefresh, bool applyAllowList = true, bool includeCustomProviders = true, IList<RpcWorkerConfig> workerConfigs = null)
+        public ImmutableArray<FunctionMetadata> GetFunctionMetadata(bool forceRefresh, bool applyAllowList = true, bool includeCustomProviders = true)
         {
             if (forceRefresh || _servicesReset || _functionMetadataArray.IsDefaultOrEmpty)
             {
-                _functionMetadataArray = LoadFunctionMetadata(forceRefresh, includeCustomProviders, workerConfigs: workerConfigs);
+                _functionMetadataArray = LoadFunctionMetadata(forceRefresh, includeCustomProviders);
                 _logger.FunctionMetadataManagerFunctionsLoaded(ApplyAllowList(_functionMetadataArray).Count());
                 _servicesReset = false;
             }
 
             return applyAllowList ? ApplyAllowList(_functionMetadataArray) : _functionMetadataArray;
         }
 
+        /// <inheritdoc />
+        public void Dispose() => _onChangeSubscription.Dispose();
+
         private ImmutableArray<FunctionMetadata> ApplyAllowList(ImmutableArray<FunctionMetadata> metadataList)
         {
             var allowList = _scriptOptions.Value?.Functions;
@@ -119,25 +131,16 @@ private void InitializeServices()
             // Resetting the logger switches the logger scope to Script Host level,
             // also making the logs available to Application Insights
             _logger = _serviceProvider?.GetService<ILoggerFactory>().CreateLogger(LogCategories.Startup);
-            _servicesReset = true;
-        }
 
-        /// <summary>
-        /// This is the worker configuration created in the jobhost scope during placeholder initialization
-        /// This is used as a fallback incase the config is not passed down from previous method call.
-        /// </summary>
-        private IList<RpcWorkerConfig> GetFallbackWorkerConfig()
-        {
-            return _serviceProvider.GetService<IOptionsMonitor<LanguageWorkerOptions>>().CurrentValue.WorkerConfigs;
+            _servicesReset = true;
         }
 
         /// <summary>
         /// Read all functions and populate function metadata.
         /// </summary>
-        internal ImmutableArray<FunctionMetadata> LoadFunctionMetadata(bool forceRefresh = false, bool includeCustomProviders = true, IFunctionInvocationDispatcher dispatcher = null, IList<RpcWorkerConfig> workerConfigs = null)
+        internal ImmutableArray<FunctionMetadata> LoadFunctionMetadata(bool forceRefresh = false, bool includeCustomProviders = true)
         {
-            workerConfigs ??= GetFallbackWorkerConfig();
-
+            var workerConfigs = _languageOptions.CurrentValue.WorkerConfigs;
             _functionMetadataMap.Clear();
 
             ICollection<string> functionsAllowList = _scriptOptions?.Value?.Functions;

src/WebJobs.Script/Host/IFunctionMetadataManager.cs

@@ -1,19 +1,16 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
-using System.Collections.Generic;
 using System.Collections.Immutable;
 using Microsoft.Azure.WebJobs.Script.Description;
-using Microsoft.Azure.WebJobs.Script.Workers;
-using Microsoft.Azure.WebJobs.Script.Workers.Rpc;
 
 namespace Microsoft.Azure.WebJobs.Script
 {
     public interface IFunctionMetadataManager
     {
         ImmutableDictionary<string, ImmutableArray<string>> Errors { get; }
 
-        ImmutableArray<FunctionMetadata> GetFunctionMetadata(bool forceRefresh = false, bool applyAllowlist = true, bool includeCustomProviders = true, IList<RpcWorkerConfig> workerConfigs = null);
+        ImmutableArray<FunctionMetadata> GetFunctionMetadata(bool forceRefresh = false, bool applyAllowlist = true, bool includeCustomProviders = true);
 
         bool TryGetFunctionMetadata(string functionName, out FunctionMetadata functionMetadata, bool forceRefresh = false);
     }

src/WebJobs.Script/Host/ScriptHost.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Collections.ObjectModel;
 using System.Diagnostics;
 using System.Globalization;
@@ -373,13 +374,10 @@ private void LogHostFunctionErrors()
         /// <returns>A metadata collection of functions and proxies configured.</returns>
         private IEnumerable<FunctionMetadata> GetFunctionsMetadata()
         {
-            IEnumerable<FunctionMetadata> functionMetadata;
-
-            functionMetadata = _functionMetadataManager.GetFunctionMetadata(forceRefresh: false, workerConfigs: _languageWorkerOptions.CurrentValue.WorkerConfigs);
-
+            ImmutableArray<FunctionMetadata> functionMetadata = _functionMetadataManager.GetFunctionMetadata(forceRefresh: false);
             foreach (var error in _functionMetadataManager.Errors)
             {
-                FunctionErrors.Add(error.Key, error.Value.ToArray());
+                FunctionErrors.Add(error.Key, error.Value);
             }
 
             return functionMetadata;

src/WebJobs.Script/ScriptHostBuilderExtensions.cs

@@ -141,9 +141,8 @@ public static IHostBuilder AddScriptHost(this IHostBuilder builder,
 
                 var bundleManager = new ExtensionBundleManager(extensionBundleOptions, SystemEnvironment.Instance, loggerFactory, configOption);
                 var metadataServiceManager = applicationOptions.RootServiceProvider.GetService<IFunctionMetadataManager>();
-                var languageWorkerOptions = applicationOptions.RootServiceProvider.GetService<IOptionsMonitor<LanguageWorkerOptions>>();
 
-                var locator = new ScriptStartupTypeLocator(applicationOptions.ScriptPath, loggerFactory.CreateLogger<ScriptStartupTypeLocator>(), bundleManager, metadataServiceManager, metricsLogger, languageWorkerOptions, extensionRequirementOptions);
+                var locator = new ScriptStartupTypeLocator(applicationOptions.ScriptPath, loggerFactory.CreateLogger<ScriptStartupTypeLocator>(), bundleManager, metadataServiceManager, metricsLogger, extensionRequirementOptions);
 
                 // The locator (and thus the bundle manager) need to be created now in order to configure app configuration.
                 // Store them so they do not need to be re-created later when configuring services.

test/WebJobs.Script.Tests.Integration/TestFunctionHost.cs

@@ -35,6 +35,7 @@
 using Microsoft.Extensions.Logging.Abstractions;
 using Microsoft.Extensions.Options;
 using Microsoft.WebJobs.Script.Tests;
+using Moq;
 using Newtonsoft.Json.Linq;
 using IApplicationLifetime = Microsoft.AspNetCore.Hosting.IApplicationLifetime;
 
@@ -502,12 +503,16 @@ private FunctionMetadataManager GetMetadataManager(IOptionsMonitor<ScriptApplica
                 WorkerConfigs = TestHelpers.GetTestWorkerConfigs()
             };
 
+            var mockOptions = new Mock<IOptionsMonitor<LanguageWorkerOptions>>();
+            mockOptions.Setup(o => o.CurrentValue).Returns(workerOptions);
+            mockOptions.Setup(o => o.OnChange(It.IsAny<Action<LanguageWorkerOptions, string>>())).Returns(Mock.Of<IDisposable>());
+
             var managerServiceProvider = manager as IServiceProvider;
 
             var metadataProvider = new HostFunctionMetadataProvider(optionsMonitor, NullLogger<HostFunctionMetadataProvider>.Instance, new TestMetricsLogger(), SystemEnvironment.Instance);
             var defaultProvider = new FunctionMetadataProvider(NullLogger<FunctionMetadataProvider>.Instance, null, metadataProvider, new OptionsWrapper<FunctionsHostingConfigOptions>(new FunctionsHostingConfigOptions()), SystemEnvironment.Instance);
             var metadataManager = new FunctionMetadataManager(managerServiceProvider.GetService<IOptions<ScriptJobHostOptions>>(), defaultProvider,
-                managerServiceProvider.GetService<IOptions<HttpWorkerOptions>>(), manager, factory, environment);
+                managerServiceProvider.GetService<IOptions<HttpWorkerOptions>>(), manager, factory, environment, mockOptions.Object);
 
             return metadataManager;
         }

test/WebJobs.Script.Tests.Shared/TestFunctionMetadataManager.cs

@@ -3,14 +3,10 @@
 
 using System;
 using System.Collections.Generic;
-using System.IO;
 using Microsoft.Azure.WebJobs.Script.Description;
-using Microsoft.Azure.WebJobs.Script.Diagnostics;
-using Microsoft.Azure.WebJobs.Script.Grpc;
 using Microsoft.Azure.WebJobs.Script.Workers.Http;
 using Microsoft.Azure.WebJobs.Script.Workers.Rpc;
 using Microsoft.Extensions.Configuration;
-using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 using Moq;
@@ -64,7 +60,7 @@ public static FunctionMetadataManager GetFunctionMetadataManager(IOptions<Script
             var source = new TestChangeTokenSource<ScriptApplicationHostOptions>();
             var changeTokens = new[] { source };
             var optionsMonitor = new OptionsMonitor<ScriptApplicationHostOptions>(factory, changeTokens, factory);
-            return new FunctionMetadataManager(jobHostOptions, functionMetadataProvider, httpOptions, managerMock.Object, loggerFactory, SystemEnvironment.Instance);
+            return new FunctionMetadataManager(jobHostOptions, functionMetadataProvider, httpOptions, managerMock.Object, loggerFactory, SystemEnvironment.Instance, languageWorkerOptions);
         }
 
         public static FunctionMetadataManager GetFunctionMetadataManagerWithDefaultHostConfig(IOptions<ScriptJobHostOptions> jobHostOptions,
@@ -98,7 +94,7 @@ public static FunctionMetadataManager GetFunctionMetadataManagerWithDefaultHostC
             var source = new TestChangeTokenSource<ScriptApplicationHostOptions>();
             var changeTokens = new[] { source };
             var optionsMonitor = new OptionsMonitor<ScriptApplicationHostOptions>(factory, changeTokens, factory);
-            return new FunctionMetadataManager(jobHostOptions, functionMetadataProvider, httpOptions, managerMock.Object, loggerFactory, SystemEnvironment.Instance);
+            return new FunctionMetadataManager(jobHostOptions, functionMetadataProvider, httpOptions, managerMock.Object, loggerFactory, SystemEnvironment.Instance, languageWorkerOptions);
         }
     }
 }