Skip to content
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

Support modeling health probes into the AppHost model for use in dashboard, deployment manifest, etc. #890

Open
DamianEdwards opened this issue Nov 16, 2023 · 14 comments
Labels
area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication
Milestone

Comments

@DamianEdwards
Copy link
Member

DamianEdwards commented Nov 16, 2023

We should consider adding support for modeling health probes into the AppHost app model so that resources' health checks endpoints can be integrated into the dashboard (#889) and emitted into the deployment manifest for potential auto-configuration by deployment tools, e.g. azd could configure HTTP health probes when deploying Aspire apps to ACA.

@DamianEdwards DamianEdwards added area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication User Story labels Nov 16, 2023
@DamianEdwards DamianEdwards changed the title Support modeling health checks into the AppHost model for use in dashboard, deployment manifest, etc. Support modeling health probes into the AppHost model for use in dashboard, deployment manifest, etc. Nov 16, 2023
@davidfowl davidfowl added this to the preview 3 (Jan) milestone Nov 24, 2023
@mitchdenny
Copy link
Member

Some API ideas:

var builder = DistributedApplication.CreateBuilder(args);
var app = builder.AddProject<Projects.Foo>(...);
app.WithProbe(app.GetEndpoint("http"), "/health");

Signature would look like:

public static IResourceBuilder<T> WithProbe<T>(this IResourceBuilder<T> builder, EndpointReference endpoint, string path) where T: IResourceWithHealthProbes

This would ultimately result in a HealthProbeAnnotation being added.

@davidfowl
Copy link
Member

We need the manifest model as well. We also need to update the shared defaults to read the endpoint from config (today we hard code it)

@mitchdenny
Copy link
Member

@davidfowl @DamianEdwards confirming this is still in for GA ... if so, this would be a good one to jump on @JamesNK.

@davidfowl
Copy link
Member

No, this is post GA

@GMouaad
Copy link
Contributor

GMouaad commented May 10, 2024

I started some draft work on this issue on #4151 if someone would like to take a look at and provide some feedback.

Open points:

  • Probe endpoint types:
    The draft supports TCP and http Probes. Is supporting commands necessary? it would make stuff more complicated.
  • Additional parameters (InitialDelay and Period) and their defaults
    I added some additional parameters to be used by some target platforms (K8s, ACA, etc.). Any thoughts on these and their defaults (5 Seconds on both)
  • Extension with a callback
    Can be quickly added if needed (?).
  • Which resources should already implement the IResourceWithHealthProbes
    In the tests I only use a dummy resource for now to showcase the functionality.
    I can implement it on an actual resource as a starting point maybe. Any suggestions on which one?
  • How to address the security on these probes?
    First thought is to add a headers property on the annotation.

CC @mitchdenny @davidfowl Feel free to give feedback ;)

PS: I still have to figure out how the the tooling should work with it, i.e. how the manifest model should look like. I'll try to lean about it (any hints to sources/docs would be great :D ) and give some thoughts soon and come up with a draft.

@mitchdenny
Copy link
Member

@davidfowl @DamianEdwards @vhvb1989 @prom3theu5

Interested in your thoughts on the manifest structure here? We could introduce another field at the resource level for probes. But health probes are heavily correlated with endpoints/bindings so perhaps it makes more sense to make it a sub-property which might make for easier correlation by deployment tools?

To that end, I am wondering whether the probe API surface in the app model should be tied to endpoints as well (so more than just a manifest serialization issue).

@DamianEdwards
Copy link
Member Author

While I understand the motivation, tying it to endpoints so closely feels a bit too limiting/messy. In the manifest, it's easy enough to use an expression to refer to the endpoint the probe utilizes, but I think there will likely be a few other properties required to properly model a probe such that keeping them separate is warranted, e.g. probe kind (health, liveness, readiness), mechanism (HTTP, TCP, executable, etc.), delay, HTTP-specific properties like path, status code, headers, body text, etc.

@wertzui
Copy link

wertzui commented Jul 24, 2024

Has there been any progress on this? I would really like to reflect my health probes for my projects in the Dashboard.

@thompson-tomo
Copy link

thompson-tomo commented Jul 24, 2024

I am of the thought that we should try and standardise health probes to leverage the OTEL data model to capture the result of the health checks.

I have raised open-telemetry/opentelemetry-dotnet-contrib#1855 to discuss the concept of capturing the probe results as an event in OTEL. This approach would mean that aspire & other tools would simply provide a visualisation of those events. I have also raised dotnet/aspnetcore#53670 for dotnet to provide an implementation which performs a periodic check & log the result.

@mitchdenny
Copy link
Member

@samsp-msft

@julioct
Copy link
Contributor

julioct commented Sep 2, 2024

Any news on this one? Assuming I have enabled the health endpoints for Prod in Service Defaults, how do I tell my AppHost to turn on the liveness and readiness probes in ACA for my projects?

image

@davidfowl
Copy link
Member

We’re working on health checks at the moment but there’s no active plan to work on health probes

@DamianEdwards
Copy link
Member Author

Any news on this one? Assuming I have enabled the health endpoints for Prod in Service Defaults, how do I tell my AppHost to turn on the liveness and readiness probes in ACA for my projects?

Today you'd have to do that manually. As @davidfowl said we haven't got active plans for modeling health probes into the app as yet, but as we make progress on the story for customizing ACA configuration from the App Host C# code, something might emerge.

@julioct
Copy link
Contributor

julioct commented Sep 5, 2024

Yes, I ended up having to azd infra synth and add this block to each of my 7 microservices:

  template:
    containers:
      - image: {{ .Image }}
        name: catalog-service
        probes:
          - type: liveness
            httpGet:
              path: "/health/alive"
              port: 8081
              scheme: HTTP
            initialDelaySeconds: 10
            periodSeconds: 10
          - type: readiness
            httpGet:
              path: "/health/ready"
              port: 8081
              scheme: HTTP
            initialDelaySeconds: 10
            periodSeconds: 5

Sad, but works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication
Projects
None yet
Development

No branches or pull requests

7 participants