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

Ideas for future health check features #464

Closed ThomasArdal closed 6 years ago

ThomasArdal commented 6 years ago

I love your ideas with the new health check middleware. I typically have something homemade brewed into all API's and websites I'm creating. My health checks (/isalive in most instances) are pinged by Pingdom and elmah.io Uptime Monitoring (disclaimer: I'm the founder of the latter). I definitely want to switch to health checks, when 2.2 will be available.

When thinking about health checks in general, I have some ideas, that could be baked into Diagnostics for sure. Here they are:

Better control of the response

I see that I can implement my own ResponseWriter. Would be awesome with some built-in writers. The obvious one would serialize all checks and their result to JSON. One (ok me) could even write a parser of this and present it in some nice way.

Another writer could serialize the result to problem details (RFC 7807).

Dependency injection

I'm sure you are already focusing on this and I cannot seem to figure out how much is supported in the current version.

Hooks

I would be interested in hooking into the situation where a health check fails. I know that a client like Pingdom would complain when the health check fails, but I would like more details only available on the server. Could be something like this:

services
    .AddHealthChecks()
    .AddCheck(new MyCustomCheck())
    .AddNotification(new MyNotification());

Don't really know if I like the way of doing this, but I'm sure you get the point. Ideally, I may want to develop an elmah.io integration for this:

services
    .AddHealthChecks()
    .AddCheck(new MyCustomCheck())
    .AddElmahIo(...);

All health check errors would be logged to elmah.io, but that could be anything really (Microsoft.Extensions.Logging, Serilog, you name it).

ASP.NET / MVC / Web API Support

Don't know what your plans are for non-Core? Would make sense to create a version of this for normal ASP.NET * websites.

mkArtakMSFT commented 6 years ago

Thanks for the feedback, @ThomasArdal. @rynowak FYI

mkArtakMSFT commented 6 years ago

@glennc, assigning to you so you can incorporate this into our future plans as necessary.

jasonwadsworth commented 6 years ago

The dependency injection is a must. Without that it can be difficult to construct the object(s) required for the health check(s).

lmcarreiro commented 6 years ago

+1 for Dependency Injection, please!!!

We could call services.AddCheck<MyCustomCheck>() instead of services.AddCheck(new MyCustomCheck()).

Is there any workaround?

Already created a question on stackoverflow and an issue here: https://github.com/dotnet-architecture/HealthChecks/issues/90

rynowak commented 6 years ago

@ThomasArdal Thanks for the feedback, responding to your comments inline.

Would be awesome with some built-in writers. The obvious one would serialize all checks and their result to JSON

We hadn't planned to create a default response writer for JSON because there isn't a broad consensus on what formats are useful. For instance most container monitoring systems don't inspect the body at all. If a consensus develops that we could define a format that's useful for everyone then I think we would be interested.

If ELMAH is interested in parsing the response does it make sense for you to ship your own response writer? It sounds like you'd use this for out of process monitoring and what in a different solution for logging in process yes?

I'm sure you are already focusing on this and I cannot seem to figure out how much is supported in the current version.

In preview 1 we resolve IHealthCheck instances as singleton services.. this isn't great and we know it needs to be improved.

We're making changes in preview2 that allow you to register health checks with any DI lifetime. You'll also be able to register health checks like builder.AddHealthCheck<T> - @lmcarreiro I believe you were looking for that.

I would like more details only available on the server.

Right now we log all health check failures to Microsoft.Extensions.Logging. Each health check can additionally include property data when it reports status. Right now we don't log this extra data, but if we did would that give you what you want?

I'm interested to know more about what your requirements are and exactly what you would do with the data.

Don't know what your plans are for non-Core?

All of the Microsoft.Extensions packages are usable on non-Core. We don't have any immediate plans to ship an http handler for non-Core, but that's what would be required. It would be pretty easy for someone to build, since most of the real logic is in the service.

glennc commented 6 years ago

In general for non-core we tell people to use GenericHost to construct all the pieces and get DI, it's not immediately obvious to people that you can do that.

ThomasArdal commented 6 years ago

@rynowak Here's a quick elaboration.

does it make sense for you to ship your own response writer? It sounds like you'd use this for out of process monitoring and what in a different solution for logging in process yes?

Makes total sense. elmah.io is an error logging for in process, but our uptime monitoring system is indeed out of process. It's for ping like tests, that I see a huge benefit in being able to tell a user that "your endpoint /healthcheck failed because health check x failed with the following error: y".

Right now we don't log this extra data, but if we did would that give you what you want?

I believe that a custom ResponseWriter would be able to return all of the needed information to the client. I'm not sure that I would want the full HTTP context on each fail anyway, and the application could potentially leak configuration info that I would never send to the client (connection strings, etc.). But again, the writer could return relevant info to the client for sure.

Right now we log all health check failures to Microsoft.Extensions.Logging.

I wasn't aware of the logging of each health check, but makes total sense. Will check that out for sure. Thx.

I'm interested to know more about what your requirements are and exactly what you would do with the data

My challenge here is that I don't really know yet :smile: I just see some potential of integration health checks even better with the clients actually requesting the health check endpoint. In the end, getting a "website failed because of a, b and c" is better than "website failed".

rynowak commented 6 years ago

@ThomasArdal - have you had a chance to get hands on with this yet? Do you need any follow up from us?

ThomasArdal commented 6 years ago

I haven't and I don't believe this is something I will look at before 2.2 is released. This issue was meant for input and ideas only. I don't need any more information from you guys :)

rynowak commented 6 years ago

Great, I'm going to close this then since I think we've discussed everything here.