[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #5046: Add health checker module

[GitBox <gi...@apache.org>]

kezhenxu94 commented on a change in pull request #5046:
URL: https://github.com/apache/skywalking/pull/5046#discussion_r451242194



##########
File path: oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/h2/H2StorageProvider.java
##########
@@ -139,6 +146,7 @@ public void start() throws ServiceNotProvidedException, ModuleStartException {
         } catch (StorageException e) {
             throw new ModuleStartException(e.getMessage(), e);
         }
+        healthChecker.setValue(1);

Review comment:
       ```suggestion
           healthChecker.setValue(0);
   ```

##########
File path: oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/h2/H2StorageProvider.java
##########
@@ -131,6 +135,9 @@ public void prepare() throws ServiceNotProvidedException, ModuleStartException {
 
     @Override
     public void start() throws ServiceNotProvidedException, ModuleStartException {
+        MetricsCreator metricCreator = getManager().find(TelemetryModule.NAME).provider().getService(MetricsCreator.class);
+        GaugeMetrics healthChecker = metricCreator.createHealthCheckerGauge("storage_h2", MetricsTag.EMPTY_KEY, MetricsTag.EMPTY_VALUE);
+        healthChecker.setValue(1);

Review comment:
       > Why always `1`? According to your doc, 1 should mean unhealthy? I think you missed some codes.
   
   This may be the initial status "unhealthy", and after the connection is established (line 142-145), the value should be set to `0`, so I reckon the line at 149 should be `healthChecker.setValue(0);`? @hanahmily 

##########
File path: docs/en/setup/backend/backend-health-check.md
##########
@@ -0,0 +1,62 @@
+# Health Check
+
+Health check intends to provide a unique approach to check the healthy status of OAP server. It includes the health status
+of modules, GraphQL and gRPC services readiness.
+
+## Health Checker Module.
+
+Health Checker module could solute how to observe the health status of modules. We can active it by below:
+```yaml
+health-checker:
+  selector: ${SW_HEALTH_CHECKER:default}
+  default:
+    checkIntervalSeconds: ${SW_HEALTH_CHECKER_INTERVAL_SECONDS:5}
+```
+Notice, we should enable `prometheus` telemetry at the same time.

Review comment:
       +1 "Any module should not depend on another module's implementation. "




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org