Skip to content
This repository has been archived by the owner on Dec 8, 2018. It is now read-only.

Consider changing HealthStatus #513

Closed
rynowak opened this issue Oct 24, 2018 · 8 comments
Closed

Consider changing HealthStatus #513

rynowak opened this issue Oct 24, 2018 · 8 comments
Assignees
Labels
3 - Done enhancement PRI: 1 - Required Must be handled in a reasonable time
Milestone

Comments

@rynowak
Copy link
Member

rynowak commented Oct 24, 2018

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:

  • app developers want to control the status reported from health checks they didn't author
  • health check library authors generally will report healthy or unhealthy
  • user-authored health checks often use degraded
  • I'm still really unhappy with Failed and what it does

#479 (comment)

@yelakhal
Copy link

yelakhal commented Oct 24, 2018

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
Copy link

steveoh commented Oct 24, 2018

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

@mkArtakMSFT mkArtakMSFT added this to the 2.2.0 milestone Oct 24, 2018
@mkArtakMSFT mkArtakMSFT added enhancement 1 - Ready PRI: 1 - Required Must be handled in a reasonable time labels Oct 24, 2018
@rynowak
Copy link
Member Author

rynowak commented Oct 27, 2018

@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
Copy link
Contributor

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

@rynowak
Copy link
Member Author

rynowak commented Oct 31, 2018

Done!

@rynowak rynowak closed this as completed Oct 31, 2018
@steveoh
Copy link

steveoh commented Oct 31, 2018

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

@rynowak
Copy link
Member Author

rynowak commented Oct 31, 2018

Oops sorry the PR didn't get linked here: #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
Copy link

+1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3 - Done enhancement PRI: 1 - Required Must be handled in a reasonable time
Projects
None yet
Development

No branches or pull requests

6 participants