search

aspnet / Diagnostics

[Archived] Diagnostics middleware for reporting info and handling exceptions and errors in ASP.NET Core, and diagnosing Entity Framework Core migrations errors. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
213 stars 111 forks source link

Consider changing HealthStatus #513

Closed rynowak closed 5 years ago

rynowak commented 5 years ago

I'm still not totally happy with the design of health status. We should review this again and see if it's meeting all of our goals.

Design points:

https://github.com/aspnet/Diagnostics/pull/479#issuecomment-432478080

yelakhal commented 5 years ago

Hi @rynowak ,

Thank you for your efforts. We really need this kind of check.

We are implementing a healthCheck in our services and we will certainly replace it with the official .Net Core healthCheck but we have a need to customize the return json object.

Is should be useful to have a second Func parameter in which we can return a response based on the CompositeHealthCheckResult. Something like:

.UseHealthChecks("/HealthCheck", cr =>
            {
                var healthCheckResults = cr.Results.Where(r => r.Value is IHealthCheckResult);
                return new
                {
                    status = cr.CheckStatus.ToString(),
                    details = healthCheckResults.Select(r => new { scope = r.Key, message = r.Value.Description, status = r.Value.CheckStatus.ToString() })
                };
            });
steveoh commented 5 years ago

@youssefNettuno I think you can do at least part of that with a HealthCheck ResponseWriter

rynowak commented 5 years ago

@unaizorrilla - do you have any concerns if we go back to individual health checks returning healthy/degraded/unhealthy? I still want to retain a way for the app author to specify what the health check should return. I'll have a proposal soon.

unaizorrilla commented 5 years ago

Hi @rynowak - nope, let me known when you have the proposal.

rynowak commented 5 years ago

Done!

steveoh commented 5 years ago

What's the new proposal or solution? Is it doc'd or what's the store?

rynowak commented 5 years ago

Oops sorry the PR didn't get linked here: https://github.com/aspnet/Diagnostics/pull/520

It's pretty similar to what you saw in preview2. For an app author, return whatever status you want in custom checks.

The guidance for libraries is to use context.Registration.FailureStatus to respect what the user configurated.

alexsandro-xpt commented 5 years ago

+1