Exposing Canary Rate Threshold to be configurable and usable (#593)

* fetching files

* add missing file

* missing utils

* [Gradle Release Plugin] - new version commit:  '3.30.6-snapshot'.

* [Gradle Release Plugin] - new version commit:  '3.31.0-snapshot'.

* release notes

* creating docs

* add refs

* setup factory

RELEASE-NOTES.md

@@ -1,3 +1,6 @@
+## 3.31.0
+* Canary Rate Threshold Circuit Breaker. #533
+
 ## 3.30.5
 * Bugfix: Treating npe at networks mapping when network hasn't a gateway ip #580.
 

docs/content/2-features/remote-solver-circuitbreaker/_index.en.md

@@ -0,0 +1,111 @@
+---
+title: Remote Solver Circuit Breaker
+---
+
+DPS use circuit breaker strategies to choose the most available Remote Server from the configured ones.
+
+## Static Threshold
+
+* Consider all remote servers circuits as **closed** on app start.
+* Opens and closes circuits based on fixed number of failures or successes.
+
+
+#### Configuration Example
+
+```json
+{
+  "version": 2,
+  "remoteDnsServers": [],
+  "envs": [],
+  "activeEnv": "",
+  "webServerPort": 5380,
+  "dnsServerPort": 53,
+  "logLevel": "INFO",
+  "logFile": "console",
+  "registerContainerNames": false,
+  "domain": "docker",
+  "dpsNetwork": false,
+  "dpsNetworkAutoConnect": false,
+  "defaultDns": true,
+  "hostMachineHostname": "host.docker",
+  "serverProtocol": "UDP_TCP",
+  "dockerHost": null,
+  "resolvConfOverrideNameServers": true,
+  "noRemoteServers": false,
+  "noEntriesResponseCode": 3,
+  "dockerSolverHostMachineFallbackActive": true,
+  "solverRemote": {
+    "circuitBreaker": {
+      "failureThreshold": 3,
+      "failureThresholdCapacity": 10,
+      "successThreshold": 5,
+      "testDelay": "PT20S"
+    }
+  }
+}
+```
+
+* **failureThreshold**: How many attempts before open the circuit?
+* **failureThresholdCapacity**: How many attempts store to the stack?
+* **successThreshold**: How many attempts before close the circuit?
+* **testDelay**: How much time to wait before test the circuit again?, see [Duration docs][1] for format explanation
+
+
+## Canary Rate Threshold
+
+* Consider all remote servers circuits as **open** on app start
+* Opens and closes circuits based on percentage of failure
+
+#### Consider all remote servers circuits as open on app start
+
+Test them on startup and add the healthy ones as HALF_OPEN this will evict to app get resolution fails right on the
+start because the first server on the remote servers list is offline.
+
+#### Configuration Example
+
+```json
+{
+  "version": 2,
+  "remoteDnsServers": [],
+  "envs": [],
+  "activeEnv": "",
+  "webServerPort": 5380,
+  "dnsServerPort": 53,
+  "logLevel": "INFO",
+  "logFile": "console",
+  "registerContainerNames": false,
+  "domain": "docker",
+  "dpsNetwork": false,
+  "dpsNetworkAutoConnect": false,
+  "defaultDns": true,
+  "hostMachineHostname": "host.docker",
+  "serverProtocol": "UDP_TCP",
+  "dockerHost": null,
+  "resolvConfOverrideNameServers": true,
+  "noRemoteServers": false,
+  "noEntriesResponseCode": 3,
+  "dockerSolverHostMachineFallbackActive": true,
+  "solverRemote": {
+    "circuitBreaker": {
+      "strategy": "CANARY_RATE_THRESHOLD",
+      "failureRateThreshold": 21,
+      "minimumNumberOfCalls": 50,
+      "permittedNumberOfCallsInHalfOpenState": 10
+    }
+  }
+}
+```
+
+* **failureRateThreshold**: If the failure rate is equal to or greater than this threshold, the CircuitBreaker will
+  transition to open. rules: values greater than 0 and not greater than 100.
+* **minimumNumberOfCalls**: Configures the minimum number of calls which are required (per sliding window period) before
+  the CircuitBreaker can calculate the error rate.
+* **permittedNumberOfCallsInHalfOpenState**: Configures the number of permitted calls when the CircuitBreaker is half
+  open.
+
+## Refs
+
+* [A more resilient circuit breaker strategy #533][2]
+
+[1]: https://docs.oracle.com/javase/8/docs/api/java/time/Duration.html#toString--
+[2]: https://github.com/mageddo/dns-proxy-server/issues/533

gradle.properties

@@ -1 +1 @@
-version=3.30.5-snapshot
+version=3.31.0-snapshot

src/main/java/com/mageddo/dnsproxyserver/config/dataprovider/mapper/ConfigJsonV2Mapper.java

@@ -1,15 +1,21 @@
 package com.mageddo.dnsproxyserver.config.dataprovider.mapper;
 
-import com.mageddo.dnsproxyserver.config.StaticThresholdCircuitBreakerStrategyConfig;
+import com.mageddo.dnsproxyserver.config.CanaryRateThresholdCircuitBreakerStrategyConfig;
+import com.mageddo.dnsproxyserver.config.CircuitBreakerStrategyConfig;
 import com.mageddo.dnsproxyserver.config.Config;
 import com.mageddo.dnsproxyserver.config.SolverRemote;
+import com.mageddo.dnsproxyserver.config.StaticThresholdCircuitBreakerStrategyConfig;
 import com.mageddo.dnsproxyserver.config.dataprovider.vo.ConfigJson;
 import com.mageddo.dnsproxyserver.config.dataprovider.vo.ConfigJsonV2;
+import com.mageddo.dnsproxyserver.config.dataprovider.vo.ConfigJsonV2.CanaryRateThresholdCircuitBreaker;
+import com.mageddo.dnsproxyserver.config.dataprovider.vo.ConfigJsonV2.StaticThresholdCircuitBreaker;
 import com.mageddo.dnsproxyserver.utils.Booleans;
+import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.ObjectUtils;
 
 import java.nio.file.Path;
 
+@Slf4j
 public class ConfigJsonV2Mapper {
 
   public static Config toConfig(ConfigJson json, Path configFileAbsolutePath) {
@@ -64,16 +70,36 @@ static SolverRemote buildCompleteSolverRemote(ConfigJson json, ConfigJsonV2.Circ
     return SolverRemote
       .builder()
       .active(Booleans.reverseWhenNotNull(json.getNoRemoteServers()))
-      // fixme #533 need to create a dynamic json parser for different strategies,
-      //      then a dynamic mapper to the solver remote
-      .circuitBreaker(StaticThresholdCircuitBreakerStrategyConfig
-        .builder()
-        .failureThreshold(circuitBreaker.getFailureThreshold())
-        .failureThresholdCapacity(circuitBreaker.getFailureThresholdCapacity())
-        .successThreshold(circuitBreaker.getSuccessThreshold())
-        .testDelay(circuitBreaker.getTestDelay())
-        .build()
-      )
+      .circuitBreaker(mapCircuitBreaker(circuitBreaker))
+      .build();
+  }
+
+  private static CircuitBreakerStrategyConfig mapCircuitBreaker(ConfigJsonV2.CircuitBreaker circuitBreaker) {
+    log.debug("circuitBreakerConfigStrategy={}", circuitBreaker.strategy());
+    return switch (circuitBreaker.strategy()){
+      case STATIC_THRESHOLD -> mapFromStaticCircuitBreaker((StaticThresholdCircuitBreaker) circuitBreaker);
+      case CANARY_RATE_THRESHOLD -> mapFromCanaryRateThresholdCircuitBreaker((CanaryRateThresholdCircuitBreaker) circuitBreaker);
+      default -> throw new UnsupportedOperationException("Unrecognized circuit breaker: " + circuitBreaker.strategy());
+    };
+  }
+
+  private static CircuitBreakerStrategyConfig mapFromCanaryRateThresholdCircuitBreaker(
+    CanaryRateThresholdCircuitBreaker circuitBreaker
+  ) {
+    return CanaryRateThresholdCircuitBreakerStrategyConfig.builder()
+      .failureRateThreshold(circuitBreaker.getFailureRateThreshold())
+      .minimumNumberOfCalls(circuitBreaker.getMinimumNumberOfCalls())
+      .permittedNumberOfCallsInHalfOpenState(circuitBreaker.getPermittedNumberOfCallsInHalfOpenState())
+      .build();
+  }
+
+  private static CircuitBreakerStrategyConfig mapFromStaticCircuitBreaker(StaticThresholdCircuitBreaker circuitBreaker) {
+    return StaticThresholdCircuitBreakerStrategyConfig
+      .builder()
+      .failureThreshold(circuitBreaker.getFailureThreshold())
+      .failureThresholdCapacity(circuitBreaker.getFailureThresholdCapacity())
+      .successThreshold(circuitBreaker.getSuccessThreshold())
+      .testDelay(circuitBreaker.getTestDelay())
       .build();
   }
 }

src/main/java/com/mageddo/dnsproxyserver/config/dataprovider/vo/ConfigJsonV2.java

@@ -2,10 +2,13 @@
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonSubTypes;
+import com.fasterxml.jackson.annotation.JsonTypeInfo;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.fasterxml.jackson.databind.annotation.JsonSerialize;
 import com.fasterxml.jackson.datatype.jsr310.deser.DurationDeserializer;
 import com.fasterxml.jackson.datatype.jsr310.ser.DurationSerializer;
+import com.mageddo.dnsproxyserver.config.CircuitBreakerStrategyConfig;
 import com.mageddo.dnsproxyserver.config.Config;
 import com.mageddo.dnsproxyserver.config.Config.Entry.Type;
 import com.mageddo.dnsproxyserver.config.dataprovider.mapper.ConfigJsonV2EnvsMapper;
@@ -167,8 +170,26 @@ public static class SolverRemote {
 
   }
 
+
+  @JsonTypeInfo(
+    use = JsonTypeInfo.Id.NAME,
+    include = JsonTypeInfo.As.PROPERTY,
+    property = "strategy",
+    defaultImpl = StaticThresholdCircuitBreaker.class
+  )
+  @JsonSubTypes({
+    @JsonSubTypes.Type(value = StaticThresholdCircuitBreaker.class, name = "STATIC_THRESHOLD"),
+    @JsonSubTypes.Type(value = CanaryRateThresholdCircuitBreaker.class, name = "CANARY_RATE_THRESHOLD")
+  })
+  public interface CircuitBreaker {
+
+    CircuitBreakerStrategyConfig.Name strategy();
+
+  }
+
+
   @Data
-  public static class CircuitBreaker {
+  public static class StaticThresholdCircuitBreaker implements CircuitBreaker {
 
     private Integer failureThreshold;
     private Integer failureThresholdCapacity;
@@ -177,5 +198,23 @@ public static class CircuitBreaker {
     @JsonSerialize(using = DurationSerializer.class)
     @JsonDeserialize(using = DurationDeserializer.class)
     private Duration testDelay;
+
+    @Override
+    public CircuitBreakerStrategyConfig.Name strategy() {
+      return CircuitBreakerStrategyConfig.Name.STATIC_THRESHOLD;
+    }
+  }
+
+  @Data
+  public static class CanaryRateThresholdCircuitBreaker implements CircuitBreaker {
+
+    private float failureRateThreshold;
+    private int minimumNumberOfCalls;
+    private int permittedNumberOfCallsInHalfOpenState;
+
+    @Override
+    public CircuitBreakerStrategyConfig.Name strategy() {
+      return CircuitBreakerStrategyConfig.Name.CANARY_RATE_THRESHOLD;
+    }
   }
 }

src/main/java/com/mageddo/dnsproxyserver/config/validator/CircuitBreakerValidator.java

@@ -1,16 +1,32 @@
 package com.mageddo.dnsproxyserver.config.validator;
 
+import com.mageddo.dnsproxyserver.config.CanaryRateThresholdCircuitBreakerStrategyConfig;
+import com.mageddo.dnsproxyserver.config.CircuitBreakerStrategyConfig;
 import com.mageddo.dnsproxyserver.config.StaticThresholdCircuitBreakerStrategyConfig;
 import org.apache.commons.lang3.Validate;
 
 import static com.mageddo.dnsproxyserver.utils.Numbers.positiveOrNull;
 
 public class CircuitBreakerValidator {
-  public static void validate(StaticThresholdCircuitBreakerStrategyConfig circuit) {
-    Validate.notNull(positiveOrNull(circuit.getFailureThreshold()), genMsg("failure theshold must be a positive number"));
-    Validate.notNull(positiveOrNull(circuit.getSuccessThreshold()), genMsg("success theshold must be positive number"));
-    Validate.notNull(positiveOrNull(circuit.getFailureThresholdCapacity()), genMsg("success theshold capacity must be positive number"));
-    Validate.notNull(circuit.getTestDelay(), genMsg("test delay must be not null"));
+  public static void validate(CircuitBreakerStrategyConfig config) {
+
+    switch (config.name()){
+      case STATIC_THRESHOLD -> validate((StaticThresholdCircuitBreakerStrategyConfig)config);
+      case CANARY_RATE_THRESHOLD -> validate((CanaryRateThresholdCircuitBreakerStrategyConfig)config);
+    }
+  }
+
+  static void validate(CanaryRateThresholdCircuitBreakerStrategyConfig config) {
+    Validate.notNull(positiveOrNull(config.getMinimumNumberOfCalls()), genMsg("failure threshold must be a positive number"));
+    Validate.notNull(positiveOrNull(config.getPermittedNumberOfCallsInHalfOpenState()), genMsg("success threshold must be positive number"));
+    Validate.notNull(positiveOrNull(config.getFailureRateThreshold()), genMsg("success thershold capacity must be positive number"));
+  }
+
+  static void validate(StaticThresholdCircuitBreakerStrategyConfig config) {
+    Validate.notNull(positiveOrNull(config.getFailureThreshold()), genMsg("failure threshold must be a positive number"));
+    Validate.notNull(positiveOrNull(config.getSuccessThreshold()), genMsg("success threshold must be positive number"));
+    Validate.notNull(positiveOrNull(config.getFailureThresholdCapacity()), genMsg("success thershold capacity must be positive number"));
+    Validate.notNull(config.getTestDelay(), genMsg("test delay must be not null"));
   }
 
   private static String genMsg(String msg) {

src/main/java/com/mageddo/dnsproxyserver/config/validator/ConfigValidator.java

@@ -1,7 +1,6 @@
 package com.mageddo.dnsproxyserver.config.validator;
 
 import com.mageddo.dnsproxyserver.config.Config;
-import com.mageddo.dnsproxyserver.config.StaticThresholdCircuitBreakerStrategyConfig;
 import org.apache.commons.lang3.Validate;
 
 public class ConfigValidator {
@@ -24,7 +23,6 @@ public static void validate(Config config) {
     Validate.notNull(config.getSolverRemote(), "Solver Remote");
     Validate.notNull(config.isSolverRemoteActive(), "Solver remote active");
 
-    // fixme #533 this could not work every time after new types be created, check it
-    CircuitBreakerValidator.validate((StaticThresholdCircuitBreakerStrategyConfig) config.getSolverRemoteCircuitBreakerStrategy());
+    CircuitBreakerValidator.validate(config.getSolverRemoteCircuitBreakerStrategy());
   }
 }

src/main/java/com/mageddo/dnsproxyserver/solver/remote/application/failsafe/CircuitBreakerFactory.java

@@ -1,6 +1,7 @@
 package com.mageddo.dnsproxyserver.solver.remote.application.failsafe;
 
 import com.mageddo.commons.lang.tuple.Pair;
+import com.mageddo.dnsproxyserver.config.CanaryRateThresholdCircuitBreakerStrategyConfig;
 import com.mageddo.dnsproxyserver.config.CircuitBreakerStrategyConfig;
 import com.mageddo.dnsproxyserver.config.StaticThresholdCircuitBreakerStrategyConfig;
 import com.mageddo.dnsproxyserver.config.application.ConfigService;
@@ -28,11 +29,17 @@
 @Slf4j
 @Singleton
 @RequiredArgsConstructor(onConstructor = @__({@Inject}))
+/**
+ * fixme #533 Move failsafe.CircuitBreakerFactory  to another package, this is not specific for failsafe
+ */
 public class CircuitBreakerFactory {
 
   private final Map<InetSocketAddress, CircuitBreakerDelegate> circuitBreakerMap = new ConcurrentHashMap<>();
   private final ConfigService configService;
+
+  // fixme #533 Delete CircuitBreakerPingCheckerService from CircuitBreakerFactory and related stuff
   private final CircuitBreakerPingCheckerService circuitBreakerCheckerService;
+
   private final FailsafeCircuitBreakerFactory failsafeCircuitBreakerFactory;
   private final com.mageddo.dnsproxyserver.solver.remote.circuitbreaker.canaryratethreshold.CircuitBreakerFactory canaryThresholdFactory;
 
@@ -41,28 +48,34 @@ public Result check(ResultSupplier sup) {
     return circuitBreaker.execute(sup);
   }
 
-  public CircuitBreakerDelegate findCircuitBreaker(IpAddr serverAddress) {
-    final var strategy = this.findCircuitBreakerHotLoad(serverAddress);
-    return this.circuitBreakerMap.computeIfAbsent(
-      ResolverMapper.toInetSocketAddress(serverAddress),
-      addr -> strategy
-    );
+  public CircuitBreakerDelegate findCircuitBreaker(IpAddr address) {
+    return this.findCircuitBreaker(ResolverMapper.toInetSocketAddress(address));
   }
 
-  CircuitBreakerDelegate findCircuitBreakerHotLoad(IpAddr address) {
+  public CircuitBreakerDelegate findCircuitBreaker(InetSocketAddress address) {
+    return this.circuitBreakerMap.computeIfAbsent(address, this::findCircuitBreakerHotLoad);
+  }
+
+  CircuitBreakerDelegate findCircuitBreakerHotLoad(InetSocketAddress address) {
     final var config = this.findCircuitBreakerConfig();
     return switch (config.name()) {
       case STATIC_THRESHOLD -> this.buildStaticThresholdFailSafeCircuitBreaker(address, config);
       case NON_RESILIENT -> new CircuitBreakerDelegateNonResilient();
+      case CANARY_RATE_THRESHOLD -> this.buildCanaryRateThreshold(config, address);
       default -> throw new UnsupportedOperationException();
     };
   }
 
+  CircuitBreakerDelegate buildCanaryRateThreshold(CircuitBreakerStrategyConfig config, InetSocketAddress address) {
+//    return this.canaryThresholdFactory.build(config, IpAddrs.from(address));
+    return this.canaryThresholdFactory.build((CanaryRateThresholdCircuitBreakerStrategyConfig) config);
+  }
+
   private CircuitBreakerDelegateStaticThresholdFailsafe buildStaticThresholdFailSafeCircuitBreaker(
-    IpAddr address, CircuitBreakerStrategyConfig config
+    InetSocketAddress address, CircuitBreakerStrategyConfig config
   ) {
     return new CircuitBreakerDelegateStaticThresholdFailsafe(this.failsafeCircuitBreakerFactory.build(
-      ResolverMapper.toInetSocketAddress(address),
+      address,
       (StaticThresholdCircuitBreakerStrategyConfig) config
     ));
   }

src/main/java/com/mageddo/dnsproxyserver/utils/Numbers.java

@@ -10,8 +10,8 @@ public static Integer positiveOrDefault(Integer v, Integer def) {
     return v;
   }
 
-  public static Integer positiveOrNull(Integer v) {
-    if (v == null || v <= 0) {
+  public static <T extends Number> T positiveOrNull(T v) {
+    if (v == null || v.intValue() < 0) {
       return null;
     }
     return v;