Skip to content

Add timestamp to HealthCheckEvent definition #4119

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mattklein123 merged 1 commit into from
Aug 16, 2018
Merged

Add timestamp to HealthCheckEvent definition #4119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions api/envoy/data/core/v2alpha/health_check_event.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions docs/root/intro/version_history.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,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
Expand Down
4 changes: 4 additions & 0 deletions source/common/upstream/health_checker_base_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
6 changes: 4 additions & 2 deletions source/common/upstream/health_checker_base_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -142,6 +143,7 @@ class HealthCheckEventLoggerImpl : public HealthCheckEventLogger {
const HostDescriptionConstSharedPtr& host, bool first_check) override;

private:
SystemTimeSource& system_time_source_;
Filesystem::FileSharedPtr file_;
};

Expand Down
4 changes: 2 additions & 2 deletions source/common/upstream/health_checker_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
14 changes: 10 additions & 4 deletions test/common/upstream/health_checker_impl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"}));
Copy link
Contributor

@danielhochman danielhochman Aug 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's too bad there's not a reasonable way to have the timestamp written as the first field in the json, just to make it slightly more human readable

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);
}

Expand Down