Skip to content

Commit

Permalink
tech debt: simplify access log code by adding a ALL_RESPONSE_STRING_F…
Browse files Browse the repository at this point in the history 
…LAGS (#15580)

Signed-off-by: Yuchen Dai <silentdai@gmail.com>
  • Loading branch information
@lambdai
lambdai authored Apr 9, 2021
1 parent 575fe8a commit 56d3f8a
Show file tree
Hide file tree
Showing 9 changed files with 125 additions and 302 deletions.
197 changes: 28 additions & 169 deletions source/common/stream_info/utility.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,180 +5,39 @@
namespace Envoy {
namespace StreamInfo {

const std::string ResponseFlagUtils::NONE = "-";
const std::string ResponseFlagUtils::DOWNSTREAM_CONNECTION_TERMINATION = "DC";
const std::string ResponseFlagUtils::FAILED_LOCAL_HEALTH_CHECK = "LH";
const std::string ResponseFlagUtils::NO_HEALTHY_UPSTREAM = "UH";
const std::string ResponseFlagUtils::UPSTREAM_REQUEST_TIMEOUT = "UT";
const std::string ResponseFlagUtils::LOCAL_RESET = "LR";
const std::string ResponseFlagUtils::UPSTREAM_REMOTE_RESET = "UR";
const std::string ResponseFlagUtils::UPSTREAM_CONNECTION_FAILURE = "UF";
const std::string ResponseFlagUtils::UPSTREAM_CONNECTION_TERMINATION = "UC";
const std::string ResponseFlagUtils::UPSTREAM_OVERFLOW = "UO";
const std::string ResponseFlagUtils::UPSTREAM_RETRY_LIMIT_EXCEEDED = "URX";
const std::string ResponseFlagUtils::NO_ROUTE_FOUND = "NR";
const std::string ResponseFlagUtils::DELAY_INJECTED = "DI";
const std::string ResponseFlagUtils::FAULT_INJECTED = "FI";
const std::string ResponseFlagUtils::RATE_LIMITED = "RL";
const std::string ResponseFlagUtils::UNAUTHORIZED_EXTERNAL_SERVICE = "UAEX";
const std::string ResponseFlagUtils::RATELIMIT_SERVICE_ERROR = "RLSE";
const std::string ResponseFlagUtils::STREAM_IDLE_TIMEOUT = "SI";
const std::string ResponseFlagUtils::INVALID_ENVOY_REQUEST_HEADERS = "IH";
const std::string ResponseFlagUtils::DOWNSTREAM_PROTOCOL_ERROR = "DPE";
const std::string ResponseFlagUtils::UPSTREAM_MAX_STREAM_DURATION_REACHED = "UMSDR";
const std::string ResponseFlagUtils::RESPONSE_FROM_CACHE_FILTER = "RFCF";
const std::string ResponseFlagUtils::NO_FILTER_CONFIG_FOUND = "NFCF";
const std::string ResponseFlagUtils::DURATION_TIMEOUT = "DT";
const std::string ResponseFlagUtils::UPSTREAM_PROTOCOL_ERROR = "UPE";
const std::string ResponseFlagUtils::NO_CLUSTER_FOUND = "NC";

void ResponseFlagUtils::appendString(std::string& result, const std::string& append) {
if (result.empty()) {
result = append;
} else {
result += "," + append;
}
}

const std::string ResponseFlagUtils::toShortString(const StreamInfo& stream_info) {
std::string result;

static_assert(ResponseFlag::LastFlag == 0x1000000, "A flag has been added. Fix this code.");

if (stream_info.hasResponseFlag(ResponseFlag::FailedLocalHealthCheck)) {
appendString(result, FAILED_LOCAL_HEALTH_CHECK);
}

if (stream_info.hasResponseFlag(ResponseFlag::NoHealthyUpstream)) {
appendString(result, NO_HEALTHY_UPSTREAM);
}

if (stream_info.hasResponseFlag(ResponseFlag::UpstreamRequestTimeout)) {
appendString(result, UPSTREAM_REQUEST_TIMEOUT);
}

if (stream_info.hasResponseFlag(ResponseFlag::LocalReset)) {
appendString(result, LOCAL_RESET);
}

if (stream_info.hasResponseFlag(ResponseFlag::UpstreamRemoteReset)) {
appendString(result, UPSTREAM_REMOTE_RESET);
}

if (stream_info.hasResponseFlag(ResponseFlag::UpstreamConnectionFailure)) {
appendString(result, UPSTREAM_CONNECTION_FAILURE);
}

if (stream_info.hasResponseFlag(ResponseFlag::UpstreamConnectionTermination)) {
appendString(result, UPSTREAM_CONNECTION_TERMINATION);
}

if (stream_info.hasResponseFlag(ResponseFlag::UpstreamOverflow)) {
appendString(result, UPSTREAM_OVERFLOW);
}

if (stream_info.hasResponseFlag(ResponseFlag::NoRouteFound)) {
appendString(result, NO_ROUTE_FOUND);
}

if (stream_info.hasResponseFlag(ResponseFlag::DelayInjected)) {
appendString(result, DELAY_INJECTED);
}

if (stream_info.hasResponseFlag(ResponseFlag::FaultInjected)) {
appendString(result, FAULT_INJECTED);
}

if (stream_info.hasResponseFlag(ResponseFlag::RateLimited)) {
appendString(result, RATE_LIMITED);
}

if (stream_info.hasResponseFlag(ResponseFlag::UnauthorizedExternalService)) {
appendString(result, UNAUTHORIZED_EXTERNAL_SERVICE);
}

if (stream_info.hasResponseFlag(ResponseFlag::RateLimitServiceError)) {
appendString(result, RATELIMIT_SERVICE_ERROR);
}

if (stream_info.hasResponseFlag(ResponseFlag::DownstreamConnectionTermination)) {
appendString(result, DOWNSTREAM_CONNECTION_TERMINATION);
}

if (stream_info.hasResponseFlag(ResponseFlag::UpstreamRetryLimitExceeded)) {
appendString(result, UPSTREAM_RETRY_LIMIT_EXCEEDED);
}

if (stream_info.hasResponseFlag(ResponseFlag::StreamIdleTimeout)) {
appendString(result, STREAM_IDLE_TIMEOUT);
}

if (stream_info.hasResponseFlag(ResponseFlag::InvalidEnvoyRequestHeaders)) {
appendString(result, INVALID_ENVOY_REQUEST_HEADERS);
}
if (stream_info.hasResponseFlag(ResponseFlag::DownstreamProtocolError)) {
appendString(result, DOWNSTREAM_PROTOCOL_ERROR);
}

if (stream_info.hasResponseFlag(ResponseFlag::UpstreamMaxStreamDurationReached)) {
appendString(result, UPSTREAM_MAX_STREAM_DURATION_REACHED);
}

if (stream_info.hasResponseFlag(ResponseFlag::ResponseFromCacheFilter)) {
appendString(result, RESPONSE_FROM_CACHE_FILTER);
}

if (stream_info.hasResponseFlag(ResponseFlag::NoFilterConfigFound)) {
appendString(result, NO_FILTER_CONFIG_FOUND);
}

if (stream_info.hasResponseFlag(ResponseFlag::DurationTimeout)) {
appendString(result, DURATION_TIMEOUT);
}

if (stream_info.hasResponseFlag(ResponseFlag::UpstreamProtocolError)) {
appendString(result, UPSTREAM_PROTOCOL_ERROR);
}
// We don't expect more than 4 flags are set. Relax to 16 since the vector is allocated on stack
// anyway.
absl::InlinedVector<absl::string_view, 16> flag_strings;
for (const auto& [flag_string, flag] : ALL_RESPONSE_STRING_FLAGS) {
if (stream_info.hasResponseFlag(flag)) {
flag_strings.push_back(flag_string);
}
}
if (flag_strings.empty()) {
return std::string(NONE);
}
return absl::StrJoin(flag_strings, ",");
}

if (stream_info.hasResponseFlag(ResponseFlag::NoClusterFound)) {
appendString(result, NO_CLUSTER_FOUND);
absl::flat_hash_map<std::string, ResponseFlag> ResponseFlagUtils::getFlagMap() {
static_assert(ResponseFlag::LastFlag == 0x1000000,
"A flag has been added. Add the new flag to ALL_RESPONSE_STRING_FLAGS.");
absl::flat_hash_map<std::string, ResponseFlag> res;
for (auto [str, flag] : ResponseFlagUtils::ALL_RESPONSE_STRING_FLAGS) {
res.emplace(str, flag);
}
return result.empty() ? NONE : result;
return res;
}

absl::optional<ResponseFlag> ResponseFlagUtils::toResponseFlag(const std::string& flag) {
static const std::map<std::string, ResponseFlag> map = {
{ResponseFlagUtils::FAILED_LOCAL_HEALTH_CHECK, ResponseFlag::FailedLocalHealthCheck},
{ResponseFlagUtils::NO_HEALTHY_UPSTREAM, ResponseFlag::NoHealthyUpstream},
{ResponseFlagUtils::UPSTREAM_REQUEST_TIMEOUT, ResponseFlag::UpstreamRequestTimeout},
{ResponseFlagUtils::LOCAL_RESET, ResponseFlag::LocalReset},
{ResponseFlagUtils::UPSTREAM_REMOTE_RESET, ResponseFlag::UpstreamRemoteReset},
{ResponseFlagUtils::UPSTREAM_CONNECTION_FAILURE, ResponseFlag::UpstreamConnectionFailure},
{ResponseFlagUtils::UPSTREAM_CONNECTION_TERMINATION,
ResponseFlag::UpstreamConnectionTermination},
{ResponseFlagUtils::UPSTREAM_OVERFLOW, ResponseFlag::UpstreamOverflow},
{ResponseFlagUtils::NO_ROUTE_FOUND, ResponseFlag::NoRouteFound},
{ResponseFlagUtils::DELAY_INJECTED, ResponseFlag::DelayInjected},
{ResponseFlagUtils::FAULT_INJECTED, ResponseFlag::FaultInjected},
{ResponseFlagUtils::RATE_LIMITED, ResponseFlag::RateLimited},
{ResponseFlagUtils::UNAUTHORIZED_EXTERNAL_SERVICE, ResponseFlag::UnauthorizedExternalService},
{ResponseFlagUtils::RATELIMIT_SERVICE_ERROR, ResponseFlag::RateLimitServiceError},
{ResponseFlagUtils::DOWNSTREAM_CONNECTION_TERMINATION,
ResponseFlag::DownstreamConnectionTermination},
{ResponseFlagUtils::UPSTREAM_RETRY_LIMIT_EXCEEDED, ResponseFlag::UpstreamRetryLimitExceeded},
{ResponseFlagUtils::STREAM_IDLE_TIMEOUT, ResponseFlag::StreamIdleTimeout},
{ResponseFlagUtils::INVALID_ENVOY_REQUEST_HEADERS, ResponseFlag::InvalidEnvoyRequestHeaders},
{ResponseFlagUtils::DOWNSTREAM_PROTOCOL_ERROR, ResponseFlag::DownstreamProtocolError},
{ResponseFlagUtils::UPSTREAM_MAX_STREAM_DURATION_REACHED,
ResponseFlag::UpstreamMaxStreamDurationReached},
{ResponseFlagUtils::RESPONSE_FROM_CACHE_FILTER, ResponseFlag::ResponseFromCacheFilter},
{ResponseFlagUtils::NO_FILTER_CONFIG_FOUND, ResponseFlag::NoFilterConfigFound},
{ResponseFlagUtils::DURATION_TIMEOUT, ResponseFlag::DurationTimeout},
{ResponseFlagUtils::UPSTREAM_PROTOCOL_ERROR, ResponseFlag::UpstreamProtocolError},
{ResponseFlagUtils::NO_CLUSTER_FOUND, ResponseFlag::NoClusterFound},
};
const auto& it = map.find(flag);
if (it != map.end()) {
absl::optional<ResponseFlag> ResponseFlagUtils::toResponseFlag(absl::string_view flag) {
// This `MapType` is introduce because CONSTRUCT_ON_FIRST_USE doesn't like template.
using MapType = absl::flat_hash_map<std::string, ResponseFlag>;
const auto& flag_map = []() {
CONSTRUCT_ON_FIRST_USE(MapType, ResponseFlagUtils::getFlagMap());
}();
const auto& it = flag_map.find(flag);
if (it != flag_map.end()) {
return absl::make_optional<ResponseFlag>(it->second);
}
return absl::nullopt;
Expand Down
91 changes: 62 additions & 29 deletions source/common/stream_info/utility.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,38 +14,71 @@ namespace StreamInfo {
class ResponseFlagUtils {
public:
static const std::string toShortString(const StreamInfo& stream_info);
static absl::optional<ResponseFlag> toResponseFlag(const std::string& response_flag);
static absl::optional<ResponseFlag> toResponseFlag(absl::string_view response_flag);

using FlagStringAndEnum = std::pair<const absl::string_view, ResponseFlag>;

constexpr static absl::string_view NONE = "-";
constexpr static absl::string_view DOWNSTREAM_CONNECTION_TERMINATION = "DC";
constexpr static absl::string_view FAILED_LOCAL_HEALTH_CHECK = "LH";
constexpr static absl::string_view NO_HEALTHY_UPSTREAM = "UH";
constexpr static absl::string_view UPSTREAM_REQUEST_TIMEOUT = "UT";
constexpr static absl::string_view LOCAL_RESET = "LR";
constexpr static absl::string_view UPSTREAM_REMOTE_RESET = "UR";
constexpr static absl::string_view UPSTREAM_CONNECTION_FAILURE = "UF";
constexpr static absl::string_view UPSTREAM_CONNECTION_TERMINATION = "UC";
constexpr static absl::string_view UPSTREAM_OVERFLOW = "UO";
constexpr static absl::string_view UPSTREAM_RETRY_LIMIT_EXCEEDED = "URX";
constexpr static absl::string_view NO_ROUTE_FOUND = "NR";
constexpr static absl::string_view DELAY_INJECTED = "DI";
constexpr static absl::string_view FAULT_INJECTED = "FI";
constexpr static absl::string_view RATE_LIMITED = "RL";
constexpr static absl::string_view UNAUTHORIZED_EXTERNAL_SERVICE = "UAEX";
constexpr static absl::string_view RATELIMIT_SERVICE_ERROR = "RLSE";
constexpr static absl::string_view STREAM_IDLE_TIMEOUT = "SI";
constexpr static absl::string_view INVALID_ENVOY_REQUEST_HEADERS = "IH";
constexpr static absl::string_view DOWNSTREAM_PROTOCOL_ERROR = "DPE";
constexpr static absl::string_view UPSTREAM_MAX_STREAM_DURATION_REACHED = "UMSDR";
constexpr static absl::string_view RESPONSE_FROM_CACHE_FILTER = "RFCF";
constexpr static absl::string_view NO_FILTER_CONFIG_FOUND = "NFCF";
constexpr static absl::string_view DURATION_TIMEOUT = "DT";
constexpr static absl::string_view UPSTREAM_PROTOCOL_ERROR = "UPE";
constexpr static absl::string_view NO_CLUSTER_FOUND = "NC";

static constexpr std::array ALL_RESPONSE_STRING_FLAGS{
FlagStringAndEnum{FAILED_LOCAL_HEALTH_CHECK, ResponseFlag::FailedLocalHealthCheck},
FlagStringAndEnum{NO_HEALTHY_UPSTREAM, ResponseFlag::NoHealthyUpstream},
FlagStringAndEnum{UPSTREAM_REQUEST_TIMEOUT, ResponseFlag::UpstreamRequestTimeout},
FlagStringAndEnum{LOCAL_RESET, ResponseFlag::LocalReset},
FlagStringAndEnum{UPSTREAM_REMOTE_RESET, ResponseFlag::UpstreamRemoteReset},
FlagStringAndEnum{UPSTREAM_CONNECTION_FAILURE, ResponseFlag::UpstreamConnectionFailure},
FlagStringAndEnum{UPSTREAM_CONNECTION_TERMINATION,
ResponseFlag::UpstreamConnectionTermination},
FlagStringAndEnum{UPSTREAM_OVERFLOW, ResponseFlag::UpstreamOverflow},
FlagStringAndEnum{NO_ROUTE_FOUND, ResponseFlag::NoRouteFound},
FlagStringAndEnum{DELAY_INJECTED, ResponseFlag::DelayInjected},
FlagStringAndEnum{FAULT_INJECTED, ResponseFlag::FaultInjected},
FlagStringAndEnum{RATE_LIMITED, ResponseFlag::RateLimited},
FlagStringAndEnum{UNAUTHORIZED_EXTERNAL_SERVICE, ResponseFlag::UnauthorizedExternalService},
FlagStringAndEnum{RATELIMIT_SERVICE_ERROR, ResponseFlag::RateLimitServiceError},
FlagStringAndEnum{DOWNSTREAM_CONNECTION_TERMINATION,
ResponseFlag::DownstreamConnectionTermination},
FlagStringAndEnum{UPSTREAM_RETRY_LIMIT_EXCEEDED, ResponseFlag::UpstreamRetryLimitExceeded},
FlagStringAndEnum{STREAM_IDLE_TIMEOUT, ResponseFlag::StreamIdleTimeout},
FlagStringAndEnum{INVALID_ENVOY_REQUEST_HEADERS, ResponseFlag::InvalidEnvoyRequestHeaders},
FlagStringAndEnum{DOWNSTREAM_PROTOCOL_ERROR, ResponseFlag::DownstreamProtocolError},
FlagStringAndEnum{UPSTREAM_MAX_STREAM_DURATION_REACHED,
ResponseFlag::UpstreamMaxStreamDurationReached},
FlagStringAndEnum{RESPONSE_FROM_CACHE_FILTER, ResponseFlag::ResponseFromCacheFilter},
FlagStringAndEnum{NO_FILTER_CONFIG_FOUND, ResponseFlag::NoFilterConfigFound},
FlagStringAndEnum{DURATION_TIMEOUT, ResponseFlag::DurationTimeout},
FlagStringAndEnum{UPSTREAM_PROTOCOL_ERROR, ResponseFlag::UpstreamProtocolError},
FlagStringAndEnum{NO_CLUSTER_FOUND, ResponseFlag::NoClusterFound},
};

private:
ResponseFlagUtils();
static void appendString(std::string& result, const std::string& append);

const static std::string NONE;
const static std::string DOWNSTREAM_CONNECTION_TERMINATION;
const static std::string FAILED_LOCAL_HEALTH_CHECK;
const static std::string NO_HEALTHY_UPSTREAM;
const static std::string UPSTREAM_REQUEST_TIMEOUT;
const static std::string LOCAL_RESET;
const static std::string UPSTREAM_REMOTE_RESET;
const static std::string UPSTREAM_CONNECTION_FAILURE;
const static std::string UPSTREAM_CONNECTION_TERMINATION;
const static std::string UPSTREAM_OVERFLOW;
const static std::string UPSTREAM_RETRY_LIMIT_EXCEEDED;
const static std::string NO_ROUTE_FOUND;
const static std::string DELAY_INJECTED;
const static std::string FAULT_INJECTED;
const static std::string RATE_LIMITED;
const static std::string UNAUTHORIZED_EXTERNAL_SERVICE;
const static std::string RATELIMIT_SERVICE_ERROR;
const static std::string STREAM_IDLE_TIMEOUT;
const static std::string INVALID_ENVOY_REQUEST_HEADERS;
const static std::string DOWNSTREAM_PROTOCOL_ERROR;
const static std::string UPSTREAM_PROTOCOL_ERROR;
const static std::string UPSTREAM_MAX_STREAM_DURATION_REACHED;
const static std::string RESPONSE_FROM_CACHE_FILTER;
const static std::string NO_FILTER_CONFIG_FOUND;
const static std::string DURATION_TIMEOUT;
const static std::string NO_CLUSTER_FOUND;
static absl::flat_hash_map<std::string, ResponseFlag> getFlagMap();
};

/**
Expand Down
1 change: 1 addition & 0 deletions test/common/access_log/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ envoy_cc_test(
srcs = ["access_log_impl_test.cc"],
deps = [
"//source/common/access_log:access_log_lib",
"//source/common/stream_info:utility_lib",
"//source/extensions/access_loggers/file:config",
"//source/extensions/access_loggers/grpc:http_config",
"//source/extensions/access_loggers/grpc:tcp_config",
Expand Down
43 changes: 8 additions & 35 deletions test/common/access_log/access_log_impl_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "common/config/utility.h"
#include "common/protobuf/message_validator_impl.h"
#include "common/runtime/runtime_impl.h"
#include "common/stream_info/utility.h"

#include "test/common/stream_info/test_util.h"
#include "test/common/upstream/utility.h"
Expand Down Expand Up @@ -993,40 +994,12 @@ name: accesslog
path: /dev/null
)EOF";

static_assert(StreamInfo::ResponseFlag::LastFlag == 0x1000000,
"A flag has been added. Fix this code.");

const std::vector<StreamInfo::ResponseFlag> all_response_flags = {
StreamInfo::ResponseFlag::FailedLocalHealthCheck,
StreamInfo::ResponseFlag::NoHealthyUpstream,
StreamInfo::ResponseFlag::UpstreamRequestTimeout,
StreamInfo::ResponseFlag::LocalReset,
StreamInfo::ResponseFlag::UpstreamRemoteReset,
StreamInfo::ResponseFlag::UpstreamConnectionFailure,
StreamInfo::ResponseFlag::UpstreamConnectionTermination,
StreamInfo::ResponseFlag::UpstreamOverflow,
StreamInfo::ResponseFlag::NoRouteFound,
StreamInfo::ResponseFlag::DelayInjected,
StreamInfo::ResponseFlag::FaultInjected,
StreamInfo::ResponseFlag::RateLimited,
StreamInfo::ResponseFlag::UnauthorizedExternalService,
StreamInfo::ResponseFlag::RateLimitServiceError,
StreamInfo::ResponseFlag::DownstreamConnectionTermination,
StreamInfo::ResponseFlag::UpstreamRetryLimitExceeded,
StreamInfo::ResponseFlag::StreamIdleTimeout,
StreamInfo::ResponseFlag::InvalidEnvoyRequestHeaders,
StreamInfo::ResponseFlag::DownstreamProtocolError,
StreamInfo::ResponseFlag::UpstreamMaxStreamDurationReached,
StreamInfo::ResponseFlag::ResponseFromCacheFilter,
StreamInfo::ResponseFlag::NoFilterConfigFound,
StreamInfo::ResponseFlag::DurationTimeout,
StreamInfo::ResponseFlag::UpstreamProtocolError,
StreamInfo::ResponseFlag::NoClusterFound,
};

InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_);

for (const auto response_flag : all_response_flags) {
for (const auto& [flag_string, response_flag] :
StreamInfo::ResponseFlagUtils::ALL_RESPONSE_STRING_FLAGS) {
UNREFERENCED_PARAMETER(flag_string);

TestStreamInfo stream_info;
stream_info.setResponseFlag(response_flag);
EXPECT_CALL(*file_, write(_));
Expand Down Expand Up @@ -1231,11 +1204,11 @@ name: accesslog
{"PERMISSION_DENIED", 403}, {"UNAVAILABLE", 429}, {"UNIMPLEMENTED", 404},
{"UNAVAILABLE", 502}, {"UNAVAILABLE", 503}, {"UNAVAILABLE", 504}};

for (const auto& pair : statusMapping) {
stream_info_.response_code_ = pair.second;
for (const auto& [response_string, response_code] : statusMapping) {
stream_info_.response_code_ = response_code;

const InstanceSharedPtr log = AccessLogFactory::fromProto(
parseAccessLogFromV3Yaml(fmt::format(yaml_template, pair.first)), context_);
parseAccessLogFromV3Yaml(fmt::format(yaml_template, response_string)), context_);

EXPECT_CALL(*file_, write(_));
log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_);
Expand Down
1 change: 1 addition & 0 deletions test/common/stream_info/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ envoy_cc_test(
"//include/envoy/upstream:host_description_interface",
"//source/common/stream_info:stream_info_lib",
"//test/mocks/router:router_mocks",
"//test/mocks/ssl:ssl_mocks",
"//test/mocks/upstream:cluster_info_mocks",
"//test/mocks/upstream:host_mocks",
"//test/test_common:utility_lib",
Expand Down
Loading

0 comments on commit 56d3f8a

Please sign in to comment.