Add multi provider support #151

Arkatufus · 2022-12-14T17:01:28Z

Add multi provider support with backward compatibility

…adotnet-healthcheck into add-multi-provider-support

Aaronontheweb

Overall, a very well thought out design and a very clean + backwards compat experience, but we do need the transports themselves before integrating this.

Aaronontheweb · 2022-12-15T14:10:49Z

src/Akka.HealthCheck.Cluster.Tests/Akka.HealthCheck.Cluster.Tests.csproj

@@ -9,6 +9,7 @@
  <ItemGroup>
    <PackageReference Include="Akka.TestKit.Xunit" Version="$(AkkaVersion)" />
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="$(TestSdkVersion)" />
+    <PackageReference Include="FluentAssertions" Version="5.10.3" />


Ideally we should upgrade to the latest here and reference this from common.props / Directory.Build.props but we can do that later

Aaronontheweb · 2022-12-15T14:11:14Z

src/Akka.HealthCheck/AkkaHealthCheck.cs

-            return new AkkaHealthCheck(new HealthCheckSettings(system), system);
+            return new AkkaHealthCheck(system);


Aaronontheweb · 2022-12-15T14:22:40Z

src/Akka.HealthCheck/Configuration/akka.healthcheck.conf

+	liveness {
+	    # List of liveness probe providers. 
+	    # Custom end-user provider can be created by implementing the IProbeProvider interface.
+	    providers {


Got it, that makes a lot of sense

So users can add their own custom healthcheck providers? How would they go about doing that?

All of the probe providers and probe actors themselves are exposed as an ImmutableDictionary inside in the HealthCheck extension, each are keyed to the property name inside akka.healthcheck.liveness.providers or akka.healthcheck.readiness.providers.

To override an implementation, an end user need to implement the IProbeProvider interface, then assign it to the name they wanted. For example, to override the default provider, the user can set akka.healthcheck.readiness.providers.default = "Custom.Package.ProbeProvider, Custom.Package", or even have their custom probe living side by side with the default one:

akka.healthcheck.readiness.providers { default : "Akka.HealthCheck.Readiness.DefaultReadinessProvider, Akka.HealthCheck" custom : "Custom.Package.ProbeProvider, Custom.Package" }

The "custom" provider can be accessed by using

var provider = HealthCheck.Get(system).ReadinessProviders["custom"];

The "custom" probe actor can be accessed by using

var readinessActor = HealthCheck.Get(system).ReadinessProbes["custom"]

same thing can be done with the default provider.

Aaronontheweb · 2022-12-15T14:27:15Z

src/Akka.HealthCheck/Transports/LivenessTransportActor.cs

        {
            _statusTransport = statusTransport;
-            _livenessProbe = livenessProbe;
+            _livenessProbes = livenessProbes.Values.ToList();
            _logInfo = log;

            ReceiveAsync<LivenessStatus>(async status =>


So one question - if we have multiple healthchecks:

Cluster

Remote

Persistence

Suppose persistence fails but cluster and remoting are still sending happy thoughts back to the LivenessTransportActor - do the happy thoughts from the two live probes override the "things are bad" signal from the probe that is failing? I.e. do we create a false negative here when that happens?

If so, shouldn't it be that we need some more robust boolean logic here:

If any of the probes are in a failing state, the entire liveness check is failed

If none of the probes are in a failing state, we're live

We're also going to need to apply this to readiness checks

Got it, I'll implement this.

Aaronontheweb · 2022-12-15T14:28:11Z

src/Akka.HealthCheck/Transports/ReadinessTransportActor.cs

@@ -18,16 +21,16 @@ namespace Akka.HealthCheck.Transports
    /// </summary>
    public sealed class ReadinessTransportActor : ReceiveActor


Same comments I made about the LivenessTransportActor also apply here

Aaronontheweb · 2022-12-15T14:30:09Z

src/Akka.HealthCheck.Cluster.Tests/SettingsSpecs.cs

+        }
+
+        [Fact(DisplayName = "HealthCheck should load settings properly (compat)")]
+        public void HealthCheck_Should_load_settings_test_compat()


Aaronontheweb · 2022-12-15T14:30:51Z

src/Akka.HealthCheck.Tests/AkkaHealthCheckSpecs.cs

+                akka.healthcheck.readiness.providers.default = ""Akka.HealthCheck.Tests.AkkaHealthCheckSpecs+FakeName, Akka.HealthCheck.Tests""
+            ";
+
+            using (var system = ActorSystem.Create("foo", config))


Aaronontheweb · 2022-12-15T14:31:16Z

src/Akka.HealthCheck.Tests/AkkaHealthCheckSpecs.cs

@@ -96,7 +128,35 @@ public async Task Should_load_default_AkkaHealthCheck()
        }

        [Fact(DisplayName = "Should load AkkaHealthCheck plugin and settings with custom providers")]
-        public async Task Should_load_misconfigured_AkkaHealthCheck()
+        public async Task Should_load_custom_AkkaHealthCheck()


Aaronontheweb · 2022-12-15T14:31:54Z

src/Akka.HealthCheck.Tests/AkkaHealthCheckSpecs.cs

+                }
+            ";
+
+            using (var system = ActorSystem.Create("foo", config))


Arkatufus

Self review for changes request.

Arkatufus · 2022-12-15T18:20:07Z

src/Akka.HealthCheck/Transports/LivenessTransportActor.cs

        {
            _statusTransport = statusTransport;
-            _livenessProbe = livenessProbe;
+            var probeReverseLookup = livenessProbes.ToImmutableDictionary(kvp => kvp.Value, kvp => kvp.Key);


Reverse lookup dictionary

Arkatufus · 2022-12-15T18:22:45Z

src/Akka.HealthCheck/Transports/LivenessTransportActor.cs

            _logInfo = log;

            ReceiveAsync<LivenessStatus>(async status =>
            {
+                var probeName = probeReverseLookup[Sender];


Lookup the probe key name

Arkatufus · 2022-12-15T18:23:24Z

src/Akka.HealthCheck/Transports/LivenessTransportActor.cs

+                }
+                finally
+                {
+                    cts.Dispose();


Added a CTS dispose and timeout check

Arkatufus · 2022-12-15T18:24:07Z

src/Akka.HealthCheck/Transports/LivenessTransportActor.cs

+                try
+                {
+                    if (status.IsLive)
+                        writeStatus = await _statusTransport.Go($"[{probeName}] {status.StatusMessage}", cts.Token);


Tagged the status message with the probe name. Will this be good enough formatting wise?

Aaronontheweb · 2022-12-15T18:25:27Z

@Arkatufus mind looking at the failing spec here?

Arkatufus added 2 commits December 14, 2022 23:59

Add multi provider support

c860ed4

Add back compat code

98d18af

Arkatufus marked this pull request as ready for review December 14, 2022 17:30

Aaronontheweb and others added 6 commits December 14, 2022 13:36

Merge branch 'dev' into add-multi-provider-support

77aea98

Add backward compatibility test for persistence

4b65a5e

Merge branch 'add-multi-provider-support' of github.com:Arkatufus/akk…

22f3719

…adotnet-healthcheck into add-multi-provider-support

Add top level hocon injection

f05fd50

Add cluster healthcheck setting spec

db2a3ea

Simplify persistence settings test

5f903e6

Aaronontheweb requested changes Dec 15, 2022

View reviewed changes

Fix the transport actor to support multi provider

dae5de3

Arkatufus commented Dec 15, 2022

View reviewed changes

Aaronontheweb approved these changes Dec 15, 2022

View reviewed changes

Aaronontheweb merged commit bdf56b4 into petabridge:dev Dec 15, 2022

Aaronontheweb mentioned this pull request Dec 15, 2022

How can you perform multiple health checks ? #108

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add multi provider support #151

Add multi provider support #151

Arkatufus commented Dec 14, 2022 •

edited

Loading

Aaronontheweb left a comment

Aaronontheweb Dec 15, 2022

Aaronontheweb Dec 15, 2022

Aaronontheweb Dec 15, 2022

Aaronontheweb Dec 15, 2022

Arkatufus Dec 15, 2022 •

edited

Loading

Aaronontheweb Dec 15, 2022

Aaronontheweb Dec 15, 2022

Arkatufus Dec 15, 2022

Aaronontheweb Dec 15, 2022

Aaronontheweb Dec 15, 2022

Aaronontheweb Dec 15, 2022

Aaronontheweb Dec 15, 2022

Aaronontheweb Dec 15, 2022

Arkatufus left a comment

Arkatufus Dec 15, 2022

Arkatufus Dec 15, 2022

Arkatufus Dec 15, 2022

Arkatufus Dec 15, 2022

Aaronontheweb commented Dec 15, 2022

		return new AkkaHealthCheck(new HealthCheckSettings(system), system);
		return new AkkaHealthCheck(system);

		@@ -18,16 +21,16 @@ namespace Akka.HealthCheck.Transports
		/// </summary>
		public sealed class ReadinessTransportActor : ReceiveActor

Add multi provider support #151

Add multi provider support #151

Conversation

Arkatufus commented Dec 14, 2022 • edited Loading

Aaronontheweb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Arkatufus Dec 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Arkatufus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aaronontheweb commented Dec 15, 2022

Arkatufus commented Dec 14, 2022 •

edited

Loading

Arkatufus Dec 15, 2022 •

edited

Loading