Fix a bug that prevented health checks from becoming degraded

[pmachapman]

Fixes #836

---

This change is 

[codecov-commenter]

Codecov Report

❌ Patch coverage is 14.81481% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.42%. Comparing base (a82894e) to head (8e61dc2).

Files with missing lines	Patch %	Lines
...rc/Serval.Machine.Shared/Services/S3HealthCheck.cs	0.00%	12 Missing ⚠️
...rval.Machine.Shared/Services/ClearMLHealthCheck.cs	0.00%	6 Missing ⚠️
...c/SIL.ServiceToolkit/Services/OutboxHealthCheck.cs	42.85%	3 Missing and 1 partial ⚠️
...ared/Configuration/IServiceCollectionExtensions.cs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #858      +/-   ##
==========================================
- Coverage   66.42%   66.42%   -0.01%     
==========================================
  Files         382      382              
  Lines       20785    20784       -1     
  Branches     2717     2717              
==========================================
- Hits        13807    13805       -2     
- Misses       6008     6009       +1     
  Partials      970      970              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Enkidu93]

I can't believe we missed this! I was sure this was working in the past for at least the s3 health check 🤔. I'm noticing, Peter, that the S3HealthCheck is added as a singleton in Machine.Shared.IServiceCollectionExtensions.AddMachine() whereas the other health checks are not and have been moved to SIL.ServiceToolkit.Configuration.IHealthChecksBuilderExtensions. Is adding the S3HealthCheck as a singleton not 'taking' for some reason? If adding the health checks as singletons works, is that a better option? Or do you think using the memory cache is more appropriate? Regardless, I think it would make sense for you to move the health-check adding logic to SIL.ServiceToolkit.Configuration.IHealthChecksBuilderExtensions.

@Enkidu93 reviewed 7 files and all commit messages, and made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ddaspit).

[ddaspit]

I had the same thought. The health check is a singleton. That would make me think that it can hold state. Maybe there is something that we don't understand about how the health check works.

@ddaspit reviewed 7 files and all commit messages, and made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @pmachapman).

[pmachapman]

The OutboxHealthCheck service consumes the IRepository<OutboxMessage> service which is scoped. This means that OutboxHealthCheck cannot be a singleton.

I updated the ClearMLHealthCheck and S3HealthCheck services to use the same IMemoryCache implementation as OutboxHealthCheck mostly for consistency reasons, and to prevent this bug from unintentionally being introduced to those services in future, if they for some reason stop being singleton (i.e. because they have to consume a scoped service in a future update/modification). If you think that's overkill, I can just write a comment in those two health checks to that effect?

@pmachapman made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @pmachapman).