Skip to content
This repository has been archived by the owner on Mar 26, 2019. It is now read-only.

Allow auto registering of health checks in full framework #2

Closed
alhardy opened this issue Oct 16, 2017 · 6 comments
Closed

Allow auto registering of health checks in full framework #2

alhardy opened this issue Oct 16, 2017 · 6 comments

Comments

@alhardy
Copy link
Contributor

alhardy commented Oct 16, 2017

Dependency context does't seem to work when usign to find all health checks in referenced assemblies.

@alhardy alhardy created this issue from a note in 2.0.0 (TODO) Oct 16, 2017
@alhardy alhardy added the bug label Oct 16, 2017
@alhardy alhardy added this to the 2.0.0 milestone Oct 16, 2017
@alhardy alhardy moved this from TODO to In Progress in 2.0.0 Oct 20, 2017
@alhardy alhardy moved this from In Progress to Ready for Release in 2.0.0 Oct 20, 2017
@alhardy alhardy closed this as completed Oct 20, 2017
alhardy added a commit that referenced this issue Oct 20, 2017
alhardy added a commit to AppMetrics/Docs.V2.Hugo that referenced this issue Oct 20, 2017
alhardy added a commit to AppMetrics/AspNetCoreHealth that referenced this issue Feb 2, 2018
@alhardy alhardy moved this from Ready for Release to Preview 1 Released (Done) in 2.0.0 Feb 9, 2018
@relair
Copy link

relair commented Feb 9, 2018

This is closed but in App.Metrics.AspNetCore.Health.Endpoints version 2.0.0-preview1 the DependencyInjection and RegisterFromAssembly doesn't seem to work at all.
I have a public class Inheriting from HealthCheck that implements CheckAsync but it doesn't get picket up (healthBuilder.HealthChecks are empty).
As for details I have .Net Core 2 app targeted at full .Net framework.

@alhardy alhardy reopened this Feb 9, 2018
@alhardy
Copy link
Contributor Author

alhardy commented Feb 9, 2018

Hmm, OK, my unit test for this works but it's scanning the same assembly as the test itself :)

Any chance you'd be able to look into this one?

The code to look at would be be

@relair
Copy link

relair commented Feb 12, 2018

I have found what was wrong, in https://www.app-metrics.io/web-monitoring/aspnet-core/health/ the documentation is incorrect/outdated. It seems that RegisterFromAssembly doesn't work if you don't call BuildAndAddTo.
Unfortunately I have found even more issues after that.

  1. It seems that calling RegisterFromAssembly and BuildAndAddTo somehow tries to resolve all classes with HealthCheck in the name, regardless if they are inheriting from HealthCheck, causing potential errors on start.
  2. BuildAndAddTo registers healthchecks as singletons, so I cannot use my own scoped services with them.

For now I have the following workaround (this is for someone who stumbles upon this looking for workaround):

// in Configure
ApplicationServices = app.ApplicationServices;
// in ConfigureServices
services.AddScoped<DatabaseHealthCheck>();
IHealthBuilder healthBuilder = AppMetricsHealth.CreateDefaultBuilder();
healthBuilder.HealthChecks.AddCheck("Database Health Check", CheckDatabaseHealth);
services.AddHealth(healthBuilder);

// Method in Startup
private ValueTask<HealthCheckResult> CheckDatabaseHealth()
{
    using (IServiceScope scope = ApplicationServices.CreateScope())
    {
        var databaseHealthCheck = scope.ServiceProvider.GetService<DatabaseHealthCheck>();`
        return databaseHealthCheck.CheckDatabaseStatus();
    }
}

I would expect dependency injection with AppMetrics to work similar to what I have done - resolving my healthchecks with their dependencies each time healthcheck endpoint is called. Unfortunately it is not the case right now.

@alhardy
Copy link
Contributor Author

alhardy commented Feb 12, 2018

Thanks for the details and looking into this @relair

  1. It seems that calling RegisterFromAssembly and BuildAndAddTo somehow tries to resolve all classes with HealthCheck in the name, regardless if they are inheriting from HealthCheck, causing potential errors on start.

Ah that could be the case, I thought I changed this to check for class which inherit health check rather than ends with "HealthCheck". Will look into this one for the next release

  1. BuildAndAddTo registers healthchecks as singletons, so I cannot use my own scoped services with them.

Yep, was tricky to do otherwise. Will have to review again for the details as memory isn't serving me. It could be less verbose than your work around to inject a Func

@alhardy alhardy moved this from Preview 1 Released (Done) to In Progress in 2.0.0 Feb 12, 2018
alhardy added a commit to AppMetrics/Docs.V2.Hugo that referenced this issue Feb 12, 2018
@alhardy
Copy link
Contributor Author

alhardy commented Feb 12, 2018

Documentation updated

@alhardy
Copy link
Contributor Author

alhardy commented Feb 12, 2018

  1. It seems that calling RegisterFromAssembly and BuildAndAddTo somehow tries to resolve all classes with HealthCheck in the name...

Resolved

@alhardy alhardy closed this as completed Mar 29, 2018
@alhardy alhardy moved this from In Progress to Ready for Release in 2.0.0 Mar 29, 2018
@alhardy alhardy moved this from Ready for Release to Released (Done) in 2.0.0 Mar 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
2.0.0
Released (Done)
Development

No branches or pull requests

2 participants