-
Notifications
You must be signed in to change notification settings - Fork 471
[HealthChecks] Add AzureWebJobsStorage health check #11471
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
base: dev
Are you sure you want to change the base?
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 adds a new health check for monitoring connectivity to the AzureWebJobsStorage account used by WebJobs. The health check runs periodically in the background to avoid expensive synchronous connectivity checks during health check requests.
Key changes:
- Implements
WebJobsStorageHealthCheckwith background refresh mechanism to cache health check results - Adds supporting infrastructure including
HealthCheckDatahelper class for structured health check metadata - Refactors
HostAzureBlobStorageProviderto extractCreateBlobServiceClientmethod for improved testability - Registers the new health check in the health check pipeline with appropriate tags
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/WebJobs.Script/Diagnostics/HealthChecks/WebJobsStorageHealthCheck.cs | New health check implementation with background refresh logic for AzureWebJobsStorage connectivity |
| src/WebJobs.Script/Diagnostics/HealthChecks/HealthCheckData.cs | Helper class for providing structured metadata with health check results |
| src/WebJobs.Script/Diagnostics/HealthChecks/HealthCheckExtensions.cs | Registers the new WebJobsStorageHealthCheck in the health check builder pipeline |
| src/WebJobs.Script/Diagnostics/HealthChecks/HealthCheckNames.cs | Adds constant for WebJobsStorage health check name |
| src/WebJobs.Script/Diagnostics/HealthChecks/HealthCheckTags.cs | Adds WebJobsStorage tag and refactors prefix constants |
| src/WebJobs.Script/StorageProvider/HostAzureBlobStorageProvider.cs | Extracts CreateBlobServiceClient method for improved testability and reusability |
| test/WebJobs.Script.Tests/Diagnostics/HealthChecks/WebJobsStorageHealthCheckTests.cs | Comprehensive test coverage for the new health check implementation |
| test/WebJobs.Script.Tests/Diagnostics/HealthChecks/HealthCheckExtensionsTests.cs | Tests for registration of the new health check |
test/WebJobs.Script.Tests/Diagnostics/HealthChecks/HealthCheckExtensionsTests.cs
Outdated
Show resolved
Hide resolved
src/WebJobs.Script/Diagnostics/HealthChecks/WebJobsStorageHealthCheck.cs
Outdated
Show resolved
Hide resolved
src/WebJobs.Script/Diagnostics/HealthChecks/WebJobsStorageHealthCheck.cs
Show resolved
Hide resolved
src/WebJobs.Script/Diagnostics/HealthChecks/WebJobsStorageHealthCheck.cs
Outdated
Show resolved
Hide resolved
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.
A good area of discussion for this PR is the names of these properties I have here.
- What style should they have? Currently it is pascal case (because it gets the name of the property)
- Any names that should be changed?
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.
I think this is fine as is. The only one where I'm like I wish there was a word more descriptive is "Area" but I also can't think of anything
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.
I bounced around on that naming a few times. I had Source and Cause before. Maybe Category? or ErrorCategory?
f7f9504 to
3445c74
Compare
Issue describing the changes in this PR
resolves #11461
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 adds a check for
AzureWebJobsStorage, which is needed for basic app functionality (the functioning connection). The implementation takes a background-refresh approach. To avoid high latency health check calls, the health will be checked in the background, with the latest value being stored and immediately returned toCheckHealthAsynccallers. An exception is the first call, which will block until a health result is available.This PR also introduces a
HealthCheckDatatype, which is a syntactic sugar wrapper aroundIReadOnlyDictionary<string, object>(which is what the health checks APIs use). The motivation is to provide a strongly-typed way to enforce a consistent schema for health check data. Having our checks follow a known schema will help automation consume our checks. This wrapper lets us do that via CSharp properties, which can enforce both key and value in one idiomatic way. This is much likeHeaderDictionary, which wraps a dictionary but exposes many well-known headers as easy to use properties (that are just part of the dictionary under the hood).