Skip to content

Conversation

@mitchdenny
Copy link
Member

@mitchdenny mitchdenny commented Sep 2, 2024

Description

This PR integrates WaitFor with health checks from the .NET hosting model. Extension methods like AddRedis(...) register a health check in DI which uses the connection string which is produced when the resource is provisioned.

A new annotation HealthCheckAnnotation keeps track of the health check names associated with each resource and a health check publisher implementation ResourceNotificationHealthCheckPublisher transcribes health check status from the health check report to a new property on the ResourceEvent (HealthStatus).

In WaitFor we determine whether a resources's dependencies has a HealthCheckAnnotation and if it does it waits for the health status to return healthy.

This PR also introduces a new event .... ConnectionStringAvailableEvent which is used by AddRedis(...) and AddPostgres(...) to detect when the connection string is available so we can store it in a captured string which is used by the health check. This is significant because it allows Azure hosted resources to participate in the health-check enabled WaitFor(...) experience.

In the case of DCP hosted resource sthe ConnectionStringAvailableEvent is fired at the same time as BeforeResourceStartedEvent - but in the case of the AzureProvisioner it is fired after the resource is online in Azure (so the order is roughly opposite) - hence why we needed another event (Azure hosted resources don't get endpoint allocated events).

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?
    • Yes
      • Link to aspire-docs issue:
    • No
Microsoft Reviewers: Open in CodeFlow

@ghost ghost added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Sep 2, 2024
@mitchdenny mitchdenny requested a review from davidfowl September 2, 2024 02:22
@mitchdenny mitchdenny self-assigned this Sep 2, 2024
@mitchdenny mitchdenny added this to the 9.0 milestone Sep 2, 2024
@mitchdenny mitchdenny requested a review from davidfowl September 4, 2024 06:01
@mitchdenny mitchdenny changed the title DRAFT: WaitFor integration with Health Check Service WaitFor integration with Health Check Service Sep 4, 2024
@davidfowl
Copy link
Member

This needs tests

@mitchdenny
Copy link
Member Author

This needs tests

Yes it does! I didn't think you'd like me #YOLO merge this one.

@mitchdenny
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@davidfowl davidfowl merged commit 87bcc24 into main Sep 6, 2024
@davidfowl davidfowl deleted the mitchdenny/healthcheckintegration branch September 6, 2024 15:28
@github-actions github-actions bot locked and limited conversation to collaborators Oct 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants