Add timestamp to HealthCheckEvent definition (envoyproxy#4119)

This PR adds timestamp field to the HealthCheckEvent message to allow having it rendered inside the JSON serialized log of a health check event. Signed-off-by: Dhi Aurrahman <dio@tetrate.io>
rshriram · Aug 16, 2018 · 8d0680f · 8d0680f
1 parent 497efb9
commit 8d0680f
Show file tree

Hide file tree

Showing 6 changed files with 26 additions and 8 deletions.
diff --git a/api/envoy/data/core/v2alpha/health_check_event.proto b/api/envoy/data/core/v2alpha/health_check_event.proto
@@ -6,6 +6,7 @@ import "envoy/api/v2/core/address.proto";
 import "envoy/api/v2/core/base.proto";
 
 import "google/protobuf/duration.proto";
+import "google/protobuf/timestamp.proto";
 import "google/protobuf/wrappers.proto";
 
 import "validate/validate.proto";
@@ -30,6 +31,9 @@ message HealthCheckEvent {
     // Host addition.
     HealthCheckAddHealthy add_healthy_event = 5;
   }
+
+  // Timestamp for event.
+  google.protobuf.Timestamp timestamp = 6 [(gogoproto.stdtime) = true];
 }
 
 enum HealthCheckFailureType {

diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst
@@ -19,6 +19,8 @@ Version history
 * health check: added support for :ref:`custom health check <envoy_api_field_core.HealthCheck.custom_health_check>`.
 * health check: added support for :ref:`specifying jitter as a percentage <envoy_api_field_core.HealthCheck.interval_jitter_percent>`.
 * health_check: added support for :ref:`health check event logging <arch_overview_health_check_logging>`.
+* health_check: added :ref:`timestamp <envoy_api_field_data.core.v2alpha.HealthCheckEvent.timestamp>`
+  to the :ref:`health check event <envoy_api_msg_data.core.v2alpha.HealthCheckEvent>` definition.
 * health_check: added support for specifying :ref:`custom request headers <config_http_conn_man_headers_custom_request_headers>`
   to HTTP health checker requests.
 * http: added support for a per-stream idle timeout. This applies at both :ref:`connection manager

diff --git a/source/common/upstream/health_checker_base_impl.cc b/source/common/upstream/health_checker_base_impl.cc
@@ -286,6 +286,8 @@ void HealthCheckEventLoggerImpl::logEjectUnhealthy(
   *event.mutable_host() = std::move(address);
   event.set_cluster_name(host->cluster().name());
   event.mutable_eject_unhealthy_event()->set_failure_type(failure_type);
+  TimestampUtil::systemClockToTimestamp(system_time_source_.currentTime(),
+                                        *event.mutable_timestamp());
   // Make sure the type enums make it into the JSON
   const auto json = MessageUtil::getJsonStringFromMessage(event, /* pretty_print */ false,
                                                           /* always_print_primitive_fields */ true);
@@ -302,6 +304,8 @@ void HealthCheckEventLoggerImpl::logAddHealthy(
   *event.mutable_host() = std::move(address);
   event.set_cluster_name(host->cluster().name());
   event.mutable_add_healthy_event()->set_first_check(first_check);
+  TimestampUtil::systemClockToTimestamp(system_time_source_.currentTime(),
+                                        *event.mutable_timestamp());
   // Make sure the type enums make it into the JSON
   const auto json = MessageUtil::getJsonStringFromMessage(event, /* pretty_print */ false,
                                                           /* always_print_primitive_fields */ true);

diff --git a/source/common/upstream/health_checker_base_impl.h b/source/common/upstream/health_checker_base_impl.h
@@ -132,8 +132,9 @@ class HealthCheckerImplBase : public HealthChecker,
 
 class HealthCheckEventLoggerImpl : public HealthCheckEventLogger {
 public:
-  HealthCheckEventLoggerImpl(AccessLog::AccessLogManager& log_manager, const std::string& file_name)
-      : file_(log_manager.createAccessLog(file_name)) {}
+  HealthCheckEventLoggerImpl(AccessLog::AccessLogManager& log_manager,
+                             SystemTimeSource& system_time_source, const std::string& file_name)
+      : system_time_source_(system_time_source), file_(log_manager.createAccessLog(file_name)) {}
 
   void logEjectUnhealthy(envoy::data::core::v2alpha::HealthCheckerType health_checker_type,
                          const HostDescriptionConstSharedPtr& host,
@@ -142,6 +143,7 @@ class HealthCheckEventLoggerImpl : public HealthCheckEventLogger {
                      const HostDescriptionConstSharedPtr& host, bool first_check) override;
 
 private:
+  SystemTimeSource& system_time_source_;
   Filesystem::FileSharedPtr file_;
 };
 

diff --git a/source/common/upstream/health_checker_impl.cc b/source/common/upstream/health_checker_impl.cc
@@ -49,8 +49,8 @@ HealthCheckerFactory::create(const envoy::api::v2::core::HealthCheck& hc_config,
                              AccessLog::AccessLogManager& log_manager) {
   HealthCheckEventLoggerPtr event_logger;
   if (!hc_config.event_log_path().empty()) {
-    event_logger =
-        std::make_unique<HealthCheckEventLoggerImpl>(log_manager, hc_config.event_log_path());
+    event_logger = std::make_unique<HealthCheckEventLoggerImpl>(
+        log_manager, ProdSystemTimeSource::instance_, hc_config.event_log_path());
   }
   switch (hc_config.health_checker_case()) {
   case envoy::api::v2::core::HealthCheck::HealthCheckerCase::kHttpHealthCheck:

diff --git a/test/common/upstream/health_checker_impl_test.cc b/test/common/upstream/health_checker_impl_test.cc
@@ -3164,22 +3164,28 @@ TEST(HealthCheckEventLoggerImplTest, All) {
   NiceMock<MockClusterInfo> cluster;
   ON_CALL(*host, cluster()).WillByDefault(ReturnRef(cluster));
 
-  HealthCheckEventLoggerImpl event_logger(log_manager, "foo");
+  NiceMock<MockSystemTimeSource> system_time_source;
+  EXPECT_CALL(system_time_source, currentTime())
+      // This is rendered as "2009-02-13T23:31:31.234Z".
+      .WillRepeatedly(Return(SystemTime(std::chrono::milliseconds(1234567891234))));
+
+  HealthCheckEventLoggerImpl event_logger(log_manager, system_time_source, "foo");
 
   EXPECT_CALL(*file, write(absl::string_view{
                          "{\"health_checker_type\":\"HTTP\",\"host\":{\"socket_address\":{"
                          "\"protocol\":\"TCP\",\"address\":\"10.0.0.1\",\"resolver_name\":\"\","
                          "\"ipv4_compat\":false,\"port_value\":443}},\"cluster_name\":\"fake_"
-                         "cluster\",\"eject_unhealthy_event\":{\"failure_type\":\"ACTIVE\"}}\n"}));
+                         "cluster\",\"eject_unhealthy_event\":{\"failure_type\":\"ACTIVE\"},"
+                         "\"timestamp\":\"2009-02-13T23:31:31.234Z\"}\n"}));
   event_logger.logEjectUnhealthy(envoy::data::core::v2alpha::HealthCheckerType::HTTP, host,
                                  envoy::data::core::v2alpha::HealthCheckFailureType::ACTIVE);
 
   EXPECT_CALL(*file, write(absl::string_view{
                          "{\"health_checker_type\":\"HTTP\",\"host\":{\"socket_address\":{"
                          "\"protocol\":\"TCP\",\"address\":\"10.0.0.1\",\"resolver_name\":\"\","
                          "\"ipv4_compat\":false,\"port_value\":443}},\"cluster_name\":\"fake_"
-                         "cluster\",\"add_healthy_event\":{\"first_check\":false}}\n"}));
-
+                         "cluster\",\"add_healthy_event\":{\"first_check\":false},\"timestamp\":"
+                         "\"2009-02-13T23:31:31.234Z\"}\n"}));
   event_logger.logAddHealthy(envoy::data::core::v2alpha::HealthCheckerType::HTTP, host, false);
 }