Refactor code to move the current logic to search for WorkerConfigs to a default worker configuration resolver #11229

surgupta-msft · 2025-08-01T19:04:01Z

Issue describing the changes in this PR

resolves #11218

Pull request checklist

IMPORTANT: Currently, changes must be backported to the in-proc branch to be included in Core Tools and non-Flex deployments.

Backporting to the in-proc branch is not required
- Otherwise: Link to backporting PR
My changes do not require documentation changes
- Otherwise: Documentation issue linked to PR
My changes should not be added to the release notes for the next release
- Otherwise: I've added my notes to release_notes.md
My changes do not need to be backported to a previous version
- Otherwise: Backport tracked by issue/PR #issue_or_pr
My changes do not require diagnostic events changes
- Otherwise: I have added/updated all related diagnostic events and their documentation (Documentation issue linked to PR)
I have added all required tests (Unit tests, E2E tests)

Additional information

Additional PR information

Refactor code to move the current logic to search for WorkerConfigs to a default worker configuration resolver. The current logic finds workerConfigs by scanning workers directory within the Host. This resolver should use Options pattern to get the required input values.

These changes should make it easier to add a new Resolver in future which will resolve workerConfigs from specified worker probing paths.

…workerconfigs by using resolver Options

Copilot

Pull Request Overview

This PR refactors the worker configuration resolution logic by extracting it from RpcWorkerConfigFactory into a dedicated DefaultWorkerConfigurationResolver class. The refactoring introduces the Options pattern for configuration management and enables easier extension with additional resolvers in the future.

Key changes include:

Extract worker configuration discovery logic into a new resolver pattern
Implement Options pattern for worker configuration resolver settings
Update dependency injection to support the new resolver architecture

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`IWorkerConfigurationResolver.cs`	New interface defining worker configuration resolution contract
`DefaultWorkerConfigurationResolver.cs`	New implementation that handles worker config path discovery from workers directory
`WorkerConfigurationResolverOptions.cs`	Options class for resolver configuration with JSON serialization support
`WorkerConfigurationResolverOptionsSetup.cs`	Setup class for configuring resolver options using Options pattern
`RpcWorkerConfigFactory.cs`	Refactored to use the new resolver instead of inline logic
Various test files	Updated to accommodate new dependency injection requirements

src/WebJobs.Script/Workers/Rpc/Configuration/WorkerConfigurationResolverOptions.cs

test/WebJobs.Script.Tests/Workers/Rpc/WorkerConfigurationResolverTestsHelper.cs

src/WebJobs.Script/Workers/Rpc/Configuration/DefaultWorkerConfigurationResolver.cs

src/WebJobs.Script/Workers/Rpc/Configuration/WorkerConfigurationResolverOptionsSetup.cs

surgupta-msft · 2025-08-01T20:58:38Z

src/WebJobs.Script.WebHost/WebHostServiceCollectionExtensions.cs

+                var workerConfigurationResolverOptions = p.GetService<IOptionsMonitor<WorkerConfigurationResolverOptions>>();
+                var loggerFactory = p.GetService<ILoggerFactory>();
+                return new DefaultWorkerConfigurationResolver(loggerFactory, workerConfigurationResolverOptions);
+            });


The DI of WorkerConfigurationResolverOptionsSetup has been done similar to LanguageWorkerOptionsSetup because the ultimate purpose of both is to get WorkerConfigs and LanguageWorkerOptionsSetup uses WorkerConfigurationResolverOptions as input.

surgupta-msft · 2025-08-01T21:02:02Z

src/WebJobs.Script.WebHost/WebJobsScriptHostService.cs

+                if (_workerConfigResolverOptionsChangeTokenSource is HostBuiltChangeTokenSource<WorkerConfigurationResolverOptions> { } hostBuiltChangeToken)
+                {
+                    hostBuiltChangeToken.TriggerChange();
+                }


Lines 385-388 are implemented similarly to lines 390-393

I filed #11208 to make this easier in the future.

surgupta-msft · 2025-08-01T21:02:39Z

src/WebJobs.Script/ScriptHostBuilderExtensions.cs

+                    var workerResolverOptions = applicationHostOptions.RootServiceProvider.GetService<IOptionsMonitor<WorkerConfigurationResolverOptions>>();
+                    services.AddSingleton(workerResolverOptions);
+                    services.AddSingleton<IOptions<WorkerConfigurationResolverOptions>>(s => new OptionsWrapper<WorkerConfigurationResolverOptions>(workerResolverOptions.CurrentValue));
+


Lines 342-345 are implemented similarly to lines 347-350

If you remove this, does it work? I think we did this with the LanguageWorkerOptions below b/c it was a complex setup. This new one is not. I think the registration will be copied to the child container automatically and all just work.

src/WebJobs.Script/Workers/Rpc/Configuration/DefaultWorkerConfigurationResolver.cs

surgupta-msft · 2025-08-01T21:09:40Z

src/WebJobs.Script/Workers/Rpc/Configuration/RpcWorkerConfigFactory.cs

-            {
-                WorkersDirPath = workersDirectorySection.Value;
-            }
+            WorkersDirPath = _workerConfigurationResolverOptions.CurrentValue.WorkersDirPath;


Lines 51-57 and lines 71-84 moved to WorkerConfigurationResolverOptionsSetup class.

src/WebJobs.Script/Workers/Rpc/Configuration/WorkerConfigurationResolverOptionsSetup.cs

src/WebJobs.Script/Workers/Rpc/IWorkerConfigurationResolver.cs

src/WebJobs.Script/Workers/Rpc/Configuration/WorkerConfigurationResolverOptionsSetup.cs

src/WebJobs.Script/Workers/Rpc/Configuration/DefaultWorkerConfigurationResolver.cs

src/WebJobs.Script/Workers/Rpc/Configuration/RpcWorkerConfigFactory.cs

src/WebJobs.Script/Workers/Rpc/IWorkerConfigurationResolver.cs

src/WebJobs.Script/Workers/Rpc/Configuration/WorkerConfigurationResolverOptionsSetup.cs

src/WebJobs.Script.WebHost/WebHostServiceCollectionExtensions.cs

src/WebJobs.Script/Workers/Rpc/Configuration/DefaultWorkerConfigurationResolver.cs

src/WebJobs.Script/Workers/Rpc/Configuration/WorkerConfigurationResolutionInfo.cs

src/WebJobs.Script/Workers/Rpc/Configuration/WorkerConfigurationResolverOptions.cs

src/WebJobs.Script/Workers/Rpc/Configuration/WorkerConfigurationResolverOptionsSetup.cs

…lverOptionsSetup

src/WebJobs.Script/Workers/Rpc/Configuration/WorkerConfigurationResolverOptionsSetup.cs

src/WebJobs.Script/Workers/Rpc/Configuration/WorkerConfigurationResolutionInfo.cs

brettsam · 2025-08-12T16:26:43Z

src/WebJobs.Script/Workers/Rpc/Configuration/RpcWorkerConfigFactory.cs

        private readonly JsonSerializerOptions _jsonSerializerOptions = new()
        {
            PropertyNameCaseInsensitive = true
        };

        private Dictionary<string, RpcWorkerConfig> _workerDescriptionDictionary = new Dictionary<string, RpcWorkerConfig>();
+        private WorkerConfigurationResolutionInfo _workerConfigurationResolutionInfo;


Remove this. It's not really needed.

brettsam · 2025-08-12T16:27:59Z

src/WebJobs.Script/Workers/Rpc/Configuration/RpcWorkerConfigFactory.cs

+
+            foreach (var workerConfig in workerConfigs)
+            {
+                AddProvider(workerConfig);


Pass the root worker dir path into this method instead of storing it in a field.

brettsam · 2025-08-12T16:28:24Z

src/WebJobs.Script/Workers/Rpc/Configuration/RpcWorkerConfigFactory.cs

@@ -122,13 +106,15 @@ internal void AddProvider(string workerDir)
            {
                try
                {
+                    string workersDirPath = _workerConfigurationResolutionInfo.WorkersDirPath;


Remove this. It should be passed in instead of being a field.

brettsam · 2025-08-12T16:40:08Z

src/WebJobs.Script/Workers/Rpc/Configuration/WorkerConfigurationResolutionInfo.cs

+        public readonly string WorkersDirPath => WorkersDirectoryPath;
+
+        public readonly IReadOnlyList<string> WorkerConfigPaths => WorkerConfigPathsList;


I'm pretty sure you don't need these properties. Records auto-generate properties based on the constructor params. This (note that I also removed struct) generates what we want, for example (init-only properties are auto-created):

internal record WorkerConfigurationResolutionInfo(string WorkersDirectoryPath, IReadOnlyList<string> WorkerConfigPathsList);

brettsam · 2025-08-12T16:44:50Z

src/WebJobs.Script/Workers/Rpc/Configuration/WorkerConfigurationResolverOptions.cs

+        /// <summary>
+        /// Gets or sets the workers directory path within the Host or defined by IConfiguration.
+        /// </summary>
+        public string WorkersDirPath { get; set; }


I think renaming this to WorkersPathRoot would be clearer for what this is giving us.

I think I prefer WorkerRootDir - multiple workers is already implied

brettsam · 2025-08-12T16:48:46Z

src/WebJobs.Script/Workers/Rpc/IWorkerConfigurationResolver.cs

+    /// </summary>
+    internal interface IWorkerConfigurationResolver
+    {
+        WorkerConfigurationResolutionInfo GetWorkerConfigPaths();


Rename to GetConfigurationInfo()? -- also maybe drop the Resolution from that record to make it a little shorter?

I also prefer WorkerConfigurationInfo here

brettsam · 2025-08-12T16:53:09Z

src/WebJobs.Script/Workers/Rpc/Configuration/WorkerConfigurationResolutionInfo.cs

+
+namespace Microsoft.Azure.WebJobs.Script.Workers.Rpc.Configuration
+{
+    internal record struct WorkerConfigurationResolutionInfo(string WorkersDirectoryPath, IReadOnlyList<string> WorkerConfigPathsList)


I'd name the first parameter here WorkersPathRoot. Secone one WorkerConfigurationFiles? I think it's giving us the actual full file paths, right?

brettsam · 2025-08-12T17:09:32Z

src/WebJobs.Script.WebHost/WebHostServiceCollectionExtensions.cs

@@ -217,6 +219,7 @@ public static void AddWebJobsScriptHost(this IServiceCollection services, IConfi
            services.ConfigureOptions<ScriptApplicationHostOptionsSetup>();
            services.AddSingleton<IOptionsChangeTokenSource<ScriptApplicationHostOptions>, ScriptApplicationHostOptionsChangeTokenSource>();
            services.ConfigureOptions<StandbyOptionsSetup>();
+            services.ConfigureOptions<WorkerConfigurationResolverOptionsSetup>();


I don't believe this (or the next line, actually) are required. If you look at what ConfigureOptionsWithChangeTokenSource does -- it does this already.

brettsam · 2025-08-12T17:19:53Z

src/WebJobs.Script/ScriptHostBuilderExtensions.cs

+                    var workerResolverOptions = applicationHostOptions.RootServiceProvider.GetService<IOptionsMonitor<WorkerConfigurationResolverOptions>>();
+                    services.AddSingleton(workerResolverOptions);
+                    services.AddSingleton<IOptions<WorkerConfigurationResolverOptions>>(s => new OptionsWrapper<WorkerConfigurationResolverOptions>(workerResolverOptions.CurrentValue));
+


If you remove this, does it work? I think we did this with the LanguageWorkerOptions below b/c it was a complex setup. This new one is not. I think the registration will be copied to the child container automatically and all just work.

liliankasem · 2025-08-12T17:30:38Z

src/WebJobs.Script/Diagnostics/Extensions/LoggerExtension.cs

+        private static readonly Action<ILogger, string, Exception> _defaultWorkersDirectoryPath =
+           LoggerMessage.Define<string>(LogLevel.Debug,
+           new EventId(343, nameof(DefaultWorkersDirectoryPath)),
+           "Workers Directory set to: {workersDirPath}");


Suggested change

"Workers Directory set to: {workersDirPath}"); "Worker directory set to: {workerDirPath}");

Use “Worker Directory” — in noun–noun phrases, the first noun is usually singular unless it’s a fixed name, so “worker” is grammatically cleaner than “workers” here. I know its named "workers" because multiple workers would be found here, but "Worker directory” means “a directory for workers” — the fact that it holds multiple workers is already implied.

liliankasem · 2025-08-12T17:36:16Z

src/WebJobs.Script/Workers/Rpc/Configuration/RpcWorkerConfigFactory.cs

-                {
-                    AddProvider(workerDir);
-                }
+                _logger.LogTrace("No worker configs found.");


What happens upstream if no worker configs are found? We just return here so I am wondering if we should throw, is this a bad state?

liliankasem · 2025-08-12T17:42:11Z

src/WebJobs.Script/Workers/Rpc/IWorkerConfigurationResolver.cs

+    /// </summary>
+    internal interface IWorkerConfigurationResolver
+    {
+        WorkerConfigurationResolutionInfo GetWorkerConfigPaths();


I also prefer WorkerConfigurationInfo here

liliankasem · 2025-08-12T17:45:00Z

src/WebJobs.Script/Workers/Rpc/Configuration/WorkerConfigurationResolverOptions.cs

+        /// <summary>
+        /// Gets or sets the workers directory path within the Host or defined by IConfiguration.
+        /// </summary>
+        public string WorkersDirPath { get; set; }


I think I prefer WorkerRootDir - multiple workers is already implied

surgupta-msft added 7 commits July 30, 2025 13:49

Refactoring to add DefaultWorkerConfigurationResolver which resolves …

5cf51b1

…workerconfigs by using resolver Options

Addressing Copilot PR feedback

e644d3c

Updating release notes

6dfc898

Removed obsolete code

1c98a16

DI fix in tests

f20fa53

Merge with dev

b0dc43f

Implemented IOptionsFormatter for WorkerConfigurationResolverOptions

ad13ac2

Copilot AI review requested due to automatic review settings August 1, 2025 19:04

surgupta-msft requested a review from a team as a code owner August 1, 2025 19:04

Copilot AI reviewed Aug 1, 2025

View reviewed changes

surgupta-msft mentioned this pull request Aug 1, 2025

Implement changes needed in the Host to decouple workers from the Host release #11111

Draft

9 tasks

surgupta-msft added 2 commits August 1, 2025 15:22

Addressing PR feedback

a566a9f

Build fix

76ccdc9

surgupta-msft commented Aug 1, 2025

View reviewed changes

src/WebJobs.Script/Workers/Rpc/Configuration/DefaultWorkerConfigurationResolver.cs Show resolved Hide resolved

surgupta-msft commented Aug 1, 2025

View reviewed changes

src/WebJobs.Script/Workers/Rpc/Configuration/WorkerConfigurationResolverOptionsSetup.cs Outdated Show resolved Hide resolved

kshyju reviewed Aug 1, 2025

View reviewed changes

surgupta-msft added 4 commits August 2, 2025 15:26

Addressing PR feedback

0de1312

Remove _workerConfigResolverOptions from LanguageWorkerOptionsSetup

b036344

Taking IOptionsMonitor as input in resolver

190531d

Added more tests in class WorkerConfigurationResolverOptionsSetupTests

a484c96

surgupta-msft requested a review from kshyju August 4, 2025 16:12

kshyju reviewed Aug 4, 2025

View reviewed changes

src/WebJobs.Script/Workers/Rpc/IWorkerConfigurationResolver.cs Outdated Show resolved Hide resolved

surgupta-msft added 2 commits August 4, 2025 22:02

Rebase with main

ad6a445

Update interface IWorkerConfigurationResolver

6317c0b

kshyju reviewed Aug 5, 2025

View reviewed changes

src/WebJobs.Script/Workers/Rpc/Configuration/WorkerConfigurationResolverOptionsSetup.cs Outdated Show resolved Hide resolved

jviau reviewed Aug 5, 2025

View reviewed changes

surgupta-msft added 3 commits August 5, 2025 23:51

Addressing PR feedback

0daca28

Adding FileSystem in TestHostBuilderExtensions

c53f0fc

Fix failing test

95bf08f

surgupta-msft requested review from kshyju and jviau August 6, 2025 19:27

surgupta-msft added 3 commits August 6, 2025 21:08

Adding only required configuration section in WorkerConfigurationReso…

70040bd

…lverOptionsSetup

Minor fix in test

b80e363

Fixing GetDefaultWorkersDirectory_Returns_Expected test

7324929

kshyju reviewed Aug 9, 2025

View reviewed changes

src/WebJobs.Script/Workers/Rpc/Configuration/WorkerConfigurationResolverOptionsSetup.cs Outdated Show resolved Hide resolved

kshyju reviewed Aug 9, 2025

View reviewed changes

src/WebJobs.Script/Workers/Rpc/Configuration/WorkerConfigurationResolutionInfo.cs Show resolved Hide resolved

surgupta-msft added 3 commits August 10, 2025 22:26

Updating WorkerConfigurationResolverOptionsSetup

ec1929d

Adding WorkerConfigPaths to WorkerConfigurationResolutionInfo

c816553

Rebase with dev

fe19a38

surgupta-msft requested a review from kshyju August 12, 2025 15:20

brettsam reviewed Aug 12, 2025

View reviewed changes

liliankasem reviewed Aug 12, 2025

View reviewed changes

		public readonly string WorkersDirPath => WorkersDirectoryPath;

		public readonly IReadOnlyList<string> WorkerConfigPaths => WorkerConfigPathsList;

Refactor code to move the current logic to search for WorkerConfigs to a default worker configuration resolver #11229

Are you sure you want to change the base?

Refactor code to move the current logic to search for WorkerConfigs to a default worker configuration resolver #11229

Conversation

surgupta-msft commented Aug 1, 2025

Issue describing the changes in this PR

Pull request checklist

Additional information

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment