Skip to content

Commit

Permalink
feat: healthcheck (#401)
Browse files Browse the repository at this point in the history
* feat: add /health/live servlet

- adds liveliness endpoint for healthcheck

* feat: add DatabaseHealthCheck and HealthService

- adds DatabaseHealthCheck to check db is live and ready + first tests
- adds HealthService to check on all registered HealthChecks

* feat: make DbConnection AutoClosable

* chore: add negative test cases + refactor DatabaseHealthCheck

* chore: javadoc and refactor
- java doc HealthCheck interface

* chore: refactor

* test:  HealthServletTest do not check JSON body yet

* test: HealthServletTest add MariaDB container

* test: HealthServletTest use container user password

* feat: HealthService add isLive() + tests

* feat: HealthServlet add ready/live routing

- adds also DbPool startup in HealthServletTest

* feat: DatabaseService check db connection for both live and ready

- add tests for DatabaseService on live

* test: HealthServlet test 200/500

- add test for HealthServlet when db started/not started

* feat: add /health endpoint with dependencies

- adds HealthResponse POJO for health JSON response
- document DbPool getDatabaseConnection instance-level method
- small refactors in methods naming and variables

* refactor: rename HealthService.java to HealthUseCase

* refactor: rename DatabaseService.java.java to DatabaseServiceDependency

* refactor: DatabaseServiceDependency check ready/live after interval

- update HealthServletTest to use mariadb 10.4.31

* chore: update javadoc

* fix: refactor DatabaseServiceDependency and tests

- fix doCheckStatus to return correct status(was returning true always)
- simplify canConnectToDatabase and make it private
- align tests with changes

* chore: refactor HealthResponse

- improve builder
- add javadoc

* chore: refactor HealthServlet

- add logging
- simplify response creation
- handle unknown routes (return 404)
- improve test readability

* fix: fix dependency issues

* feat: set content type for health endpoint response

* refactor: extract polling logic to ServiceDependency class for possible reuse

- cleanup DatabaseServiceDependency

* chore: register dependencies as provided

* chore: avoid declaring jna for all projects

- found jna is needed by zmconfigd Jython and no others, directly. Was updated beacuse needed by TestContainers

* chore: use new healthcheck in service

* chore: avoid fork

* test: HealthServletTest use container host as mysql bind address

* refactor: bring back caching logic to DatabaseServiceDependency until needed by other use health checks

- avoid putting caching logic on abstract class (YAGNI)

* chore: JettyServerFactory add final to maps fields

* chore: JettyServerFactory local host until override needed by others

* test: DatabaseServiceDependencyTest remove unneeded SQLException in test

* refactor: database check logic

* refactor: mark response builder methods as public

* refactor: simplify dependency declaration in HealthServletModule

* test: add empty dependencies list test

* refactor: change test names

* refactor: general review and refactoring of DatabaseServiceDependencyTest

* refactor: extract handler methods in HealthServlet

* refactor: move dependency health summary logic in HealthUseCase

* refactor: change Dependency response type

* refactor: rename Dependency dto in DependencyResponse

* refactor: introduce withDb utility in HealthServletTest

---------

Co-authored-by: Keshav Bhatt <keshavnrj@gmail.com>
Co-authored-by: Matteo Baglini <matteo.baglini@gmail.com>
  • Loading branch information
3 people authored Dec 12, 2023
1 parent 3f5fe6b commit 237ee66
Show file tree
Hide file tree
Showing 18 changed files with 904 additions and 33 deletions.
3 changes: 2 additions & 1 deletion carbonio-jetty-libs/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -373,9 +373,11 @@
<artifactId>eddsa</artifactId>
<version>0.3.0</version>
</dependency>
<!-- Needed by jython -->
<dependency>
<groupId>net.java.dev.jna</groupId>
<artifactId>jna</artifactId>
<version>3.4.0</version>
</dependency>
<dependency>
<groupId>net.sf.ehcache</groupId>
Expand Down Expand Up @@ -664,7 +666,6 @@
<dependency>
<groupId>org.mariadb.jdbc</groupId>
<artifactId>mariadb-java-client</artifactId>
<version>2.7.3</version>
</dependency>
<dependency>
<groupId>org.owasp.antisamy</groupId>
Expand Down
2 changes: 1 addition & 1 deletion packages/appserver-service/carbonio-mailbox.hcl
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
services {
check {
tcp = "localhost:8080"
http = "http://localhost:8080/service/health/ready"
timeout = "1s"
interval = "5s"
}
Expand Down
12 changes: 6 additions & 6 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -361,11 +361,6 @@
<artifactId>apache-jsp</artifactId>
<version>${jetty.version}</version>
</dependency>
<dependency>
<groupId>net.java.dev.jna</groupId>
<artifactId>jna</artifactId>
<version>3.4.0</version>
</dependency>
<dependency>
<groupId>org.dom4j</groupId>
<artifactId>dom4j</artifactId>
Expand Down Expand Up @@ -836,6 +831,11 @@
<artifactId>mockserver-client-java</artifactId>
<version>${mockserver.version}</version>
</dependency>
<dependency>
<groupId>org.mariadb.jdbc</groupId>
<artifactId>mariadb-java-client</artifactId>
<version>${mariadb-java-client.version}</version>
</dependency>
</dependencies>
</dependencyManagement>

Expand Down Expand Up @@ -878,6 +878,7 @@
<!-- Maven revision: https://maven.apache.org/maven-ci-friendly.html -->
<revision>24.1.0-SNAPSHOT</revision>
<mockserver.version>5.13.0</mockserver.version>
<mariadb-java-client.version>2.7.3</mariadb-java-client.version>
</properties>
<version>${revision}</version>
<profiles>
Expand Down Expand Up @@ -995,7 +996,6 @@
<artifactId>maven-failsafe-plugin</artifactId>
<version>3.0.0-M7</version>
<configuration>
<forkMode>always</forkMode>
<!-- argline preserves other plugin args, like JaCoCo -->
<argLine>@{argLine} -Dserver.dir=${project.basedir}
-Dzimbra.config=${project.basedir}/src/test/resources/localconfig-test.xml
Expand Down
55 changes: 38 additions & 17 deletions store/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<artifactId>zm-store</artifactId>

<parent>
<artifactId>zm-mailbox</artifactId>
<groupId>zextras</groupId>
<version>${revision}</version>
</parent>

<!-- To sort out dependencies take a look at: https://wiki.eclipse.org/Jetty/Reference/Dependencies -->
<dependencies>
<!-- Must be provided to avoid collision -->
Expand Down Expand Up @@ -64,19 +70,11 @@
<scope>test</scope>
</dependency>


<dependency>
<artifactId>zm-ews-stub</artifactId>
<groupId>zimbra</groupId>
</dependency>

<!-- TODO: figure out if we require this as well, since zimbra already have one-->
<!-- <dependency>-->
<!-- <artifactId>ews_2010</artifactId>-->
<!-- <groupId>ews_2010</groupId>-->
<!-- <version>1.0</version>-->
<!-- </dependency>
-->
<dependency>
<artifactId>jjwt</artifactId>
<groupId>io.jsonwebtoken</groupId>
Expand Down Expand Up @@ -109,6 +107,30 @@
<artifactId>junit-jupiter-params</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.testcontainers</groupId>
<artifactId>testcontainers</artifactId>
<version>1.17.6</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mariadb.jdbc</groupId>
<artifactId>mariadb-java-client</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.testcontainers</groupId>
<artifactId>mariadb</artifactId>
<version>1.17.6</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>com.github.docker-java</groupId>
<artifactId>docker-java-api</artifactId>
<version>3.2.13</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-library</artifactId>
Expand Down Expand Up @@ -504,18 +526,17 @@
<version>1.6.14</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.testcontainers</groupId>
<artifactId>junit-jupiter</artifactId>
<version>1.19.0</version>
<scope>test</scope>
</dependency>
</dependencies>

<modelVersion>4.0.0</modelVersion>
<packaging>war</packaging>

<parent>
<artifactId>zm-mailbox</artifactId>
<groupId>zextras</groupId>
<version>${revision}</version>
</parent>

<build>
<plugins>
<plugin>
Expand Down Expand Up @@ -759,16 +780,16 @@
<dependency>
<groupId>org.mariadb.jdbc</groupId>
<artifactId>mariadb-java-client</artifactId>
<version>2.7.3</version>
<version>${mariadb-java-client.version}</version>
</dependency>
<dependency>
<groupId>log4j</groupId>
<artifactId>apache-log4j-extras</artifactId>
<version>1.0</version>
</dependency>
</dependencies>

</plugin>

<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// SPDX-FileCopyrightText: 2023 Zextras <https://www.zextras.com>
//
// SPDX-License-Identifier: AGPL-3.0-only

package com.zextras.mailbox.health;

import com.zimbra.common.service.ServiceException;
import com.zimbra.cs.db.DbPool;
import com.zimbra.cs.db.DbPool.DbConnection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.function.Supplier;

/** Class represents MariaDB service dependency of mailbox */
public class DatabaseServiceDependency extends ServiceDependency {

private final DbPool dbPool;
private final int cacheIntervalMillis;
private final Supplier<Long> currentTimeProvider;
private Long lastExecMillis;
private boolean lastHealthCheckedValue = false;

public DatabaseServiceDependency(DbPool dbPool, Supplier<Long> currentTimeProvider) {
this(dbPool, 5000, currentTimeProvider);
}

public DatabaseServiceDependency(
DbPool dbPool, int cacheIntervalMillis, Supplier<Long> currentTimeProvider) {
super("MariaDb", ServiceType.REQUIRED);
this.dbPool = dbPool;
this.cacheIntervalMillis = cacheIntervalMillis;
this.currentTimeProvider = currentTimeProvider;
}

@Override
public boolean isReady() {
return canConnectToDatabase();
}

@Override
public boolean isLive() {
return this.canConnectToDatabase();
}

private boolean doCheckStatus() {
try (DbConnection connection = dbPool.getDatabaseConnection();
PreparedStatement preparedStatement = connection.prepareStatement("SELECT 1");
ResultSet resultSet = preparedStatement.executeQuery()) {
resultSet.next();
return true;
} catch (ServiceException | SQLException e) {
return false;
}
}

private boolean canConnectToDatabase() {
final long currentTime = currentTimeProvider.get();

if (lastExecMillis == null || currentTime > lastExecMillis + cacheIntervalMillis) {
lastHealthCheckedValue = doCheckStatus();
lastExecMillis = currentTime;
}

return lastHealthCheckedValue;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package com.zextras.mailbox.health;

public class DependencyHealthResult {
private final String name;
private final ServiceDependency.ServiceType type;
private final boolean ready;
private final boolean live;

public DependencyHealthResult(
String name, ServiceDependency.ServiceType type, boolean ready, boolean live) {
this.name = name;
this.type = type;
this.ready = ready;
this.live = live;
}

public String getName() {
return name;
}

public ServiceDependency.ServiceType getType() {
return type;
}

public boolean isReady() {
return ready;
}

public boolean isLive() {
return live;
}
}
27 changes: 27 additions & 0 deletions store/src/main/java/com/zextras/mailbox/health/HealthStatus.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// SPDX-FileCopyrightText: 2023 Zextras <https://www.zextras.com>
//
// SPDX-License-Identifier: AGPL-3.0-only

package com.zextras.mailbox.health;

/**
* Contract for checking the health status of a service.
*/
public interface HealthStatus {

/**
* Checks if the service is ready to process requests.
*
* @return {@code true} if the service is ready to receive requests, otherwise {@code false}
*/
boolean isReady();

/**
* Checks if the service is live and responsive. If the liveliness check fails, it indicates that
* the service is unhealthy or dead and should be restarted.
*
* @return {@code true} if the service is healthy and responsive, otherwise {@code false}
*/
boolean isLive();
}

37 changes: 37 additions & 0 deletions store/src/main/java/com/zextras/mailbox/health/HealthUseCase.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// SPDX-FileCopyrightText: 2023 Zextras <https://www.zextras.com>
//
// SPDX-License-Identifier: AGPL-3.0-only

package com.zextras.mailbox.health;

import java.util.List;
import java.util.stream.Collectors;
import javax.inject.Inject;

public class HealthUseCase {

private final List<ServiceDependency> serviceDependencies;

@Inject
public HealthUseCase(List<ServiceDependency> serviceDependencies) {
this.serviceDependencies = serviceDependencies;
}

public boolean isReady() {
return serviceDependencies.stream().allMatch(ServiceDependency::isReady);
}

public List<DependencyHealthResult> dependenciesHealthSummary() {
return serviceDependencies.stream()
.map(x -> createHealthResult(x))
.collect(Collectors.toList());
}

public boolean isLive() {
return serviceDependencies.stream().allMatch(ServiceDependency::isLive);
}

private DependencyHealthResult createHealthResult(ServiceDependency x) {
return new DependencyHealthResult(x.getName(), x.getType(), x.isReady(), x.isLive());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// SPDX-FileCopyrightText: 2023 Zextras <https://www.zextras.com>
//
// SPDX-License-Identifier: AGPL-3.0-only

package com.zextras.mailbox.health;

/**
* Abstract class representing service as dependency of another service.
*/
public abstract class ServiceDependency implements HealthStatus {

private final String name;
private final ServiceType type;


protected ServiceDependency(String name,
ServiceType type) {
this.name = name;
this.type = type;
}

public String getName() {
return name;
}

public ServiceType getType() {
return type;
}

public enum ServiceType {
OPTIONAL,
REQUIRED
}
}


Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class GuiceMailboxServletConfig extends GuiceServletContextListener {
* on how to solve possible dependencies
*/
private static Collection<AbstractModule> getInjectionModules() {
return Set.of(new MetricsServletModule());
return Set.of(new MetricsServletModule(), new HealthServletModule());
}

/**
Expand Down
Loading

0 comments on commit 237ee66

Please sign in to comment.