-
Notifications
You must be signed in to change notification settings - Fork 470
Include HealthChecks from ScriptHost scope #11410
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR integrates health checks from the ScriptHost scope into the overall health check system by implementing a DynamicHealthCheckService that combines health checks from both WebHost and ScriptHost scopes. The implementation ensures that health checks run in their correct service scope contexts rather than being incorrectly executed in the WebHost scope.
Key changes:
- Added a new
Servicesproperty toIScriptHostManagerinterface to expose the ScriptHost service provider - Implemented
DynamicHealthCheckServicethat wraps and coordinates two health check services (WebHost and ScriptHost) - Enhanced service provider extensions with a
CreateInstancemethod to support dynamic service instantiation
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/WebJobs.Script/Host/IScriptHostManager.cs | Added Services property to expose ScriptHost service provider |
| src/WebJobs.Script/Extensions/IServiceProviderExtensions.cs | Added CreateInstance method for dynamic service instantiation |
| src/WebJobs.Script/Diagnostics/HealthChecks/HealthCheckExtensions.cs | Added UseDynamicHealthCheckService extension method |
| src/WebJobs.Script/Diagnostics/HealthChecks/DynamicHealthCheckService.cs | New service that coordinates WebHost and ScriptHost health checks |
| src/WebJobs.Script.WebHost/DependencyInjection/ServiceProviderExtensions.cs | Updated child container ignored types to include health check services |
| test/WebJobs.Script.Tests/Diagnostics/HealthChecks/DynamicHealthCheckServiceTests.cs | Comprehensive test suite for the new DynamicHealthCheckService |
| test/WebJobs.Script.Tests/Diagnostics/HealthChecks/HealthCheckExtensionsTests.cs | Tests for the new UseDynamicHealthCheckService extension |
| test/WebJobs.Script.Tests/Extensions/ServiceProviderExtensionsTests.cs | Tests for the new CreateInstance extension method |
| test/WebJobs.Script.Tests/Metrics/LinuxContainerLegionMetricsPublisherTests.cs | Updated test mock to implement new Services property |
| test/WebJobs.Script.Tests.Shared/TestHelpers.cs | Updated test helper to implement new Services property |
src/WebJobs.Script/Diagnostics/HealthChecks/DynamicHealthCheckService.cs
Outdated
Show resolved
Hide resolved
src/WebJobs.Script/Diagnostics/HealthChecks/DynamicHealthCheckService.cs
Outdated
Show resolved
Hide resolved
…m/Azure/azure-functions-host into u/jviau/script-host-health-checks
Issue describing the changes in this PR
resolves #11403
Pull request checklist
IMPORTANT: Currently, changes must be backported to the
in-procbranch to be included in Core Tools and non-Flex deployments.in-procbranch is not requiredrelease_notes.mdAdditional information
This PR integrates
IHealthCheckimplementations from the ScriptHost scope. This is done by inserting a newDynamicHealthCheckService, which is effectively a wrapper around 2 health check services. (1) The existing WebJobs scoped health check service, and (2) the current active ScriptHost health check service (if one exists).This approach is taken to ensure all health checks from the ScriptHost are ran in the correct service scope. An initial attempt was made to pull health check registrations from the active script host into the WebHost directly, but that will run them in the incorrect context and lead to wrong DI setup for them.