NLog / NLog.Web

NLog integration for ASP.NET & ASP.NET Core 2-8
https://nlog-project.org
BSD 3-Clause "New" or "Revised" License
319 stars 166 forks source link

Add support for IHealthCheck for netcoreapp3.1 + net6.0 #658

Open snakefoot opened 3 years ago

snakefoot commented 3 years ago

Something similar to these:

https://github.com/Xabaril/AspNetCore.Diagnostics.HealthChecks/tree/master/src

Maybe something like this:

bakgerman commented 1 year ago

is this still desired? I am preparing pull requests for AspNetCore.Diagnostics.HealthChecks for IBM DB2 and MQ so I am somewhat familiar. I can try and build a prototype.

snakefoot commented 1 year ago

I think it is still interesting to have diagnostics from the NLog-targets, and maybe also basic-statistics about logged-errors-per-minute-by-host-application.

bakgerman commented 1 year ago

There are several locations to place the health check classes

One is in NLog.Web.AspNetCore itself, this will require 1 or 2 additional NuGet package dependencies added, for the IHealthCheck and related interfaces. But as these are official Microsoft NuGet packages this may be OK.

One is in a separate project named NLog.Web.AspNetCore.HealthChecks. This will have only the 1 or 2 NuGet dependencies, but will also have very few classes (an extension class, and a class to implement the actual check) so not sure it deserves a project.

The last one is to host the NLog.Web.AspNetCore.HealthChecks in the https://github.com/Xabaril/AspNetCore.Diagnostics.HealthChecks repo, where there are many others...but then we will not be master maintainer of the project and cannot makes PR and release ourselves.

As the HealthCheck NuGet packages state they support net std 2.0 and net 4.6.1, we 'may' be able to support that as well, but certainly core 3.1/5.0/6.0 they support.

snakefoot commented 1 year ago

Are the HealthChecks-dependencies not included by default when project is using:

<FrameworkReference Include="Microsoft.AspNetCore.App" />

I'm ok with only adding HealthCheck-support for only some platforms like NET6, can see the following screenshot:

image

bakgerman commented 1 year ago

Good point, yes if we skip netstd 20/4.6.1 then is auto included. then we can leave in the project with no changes.