From 7d3314afcd5ef204b75f75048e7c740f2d3e84b2 Mon Sep 17 00:00:00 2001 From: Bin Wu Date: Wed, 20 Mar 2019 21:58:52 -0400 Subject: [PATCH 1/6] Add quic_expect_bug_impl.h. Signed-off-by: Bin Wu --- bazel/external/quiche.BUILD | 9 +++-- .../quic_listeners/quiche/platform/BUILD | 14 ++++++- .../quiche/platform/quic_expect_bug_impl.h | 16 ++++++++ .../quiche/platform/quic_logging_impl.cc | 17 +++++++- .../quiche/platform/quic_logging_impl.h | 3 ++ .../quiche/platform/quic_mock_log_impl.h | 40 +++++++++++++++++++ .../quiche/platform/quic_platform_test.cc | 13 ++++++ 7 files changed, 106 insertions(+), 6 deletions(-) create mode 100644 source/extensions/quic_listeners/quiche/platform/quic_expect_bug_impl.h diff --git a/bazel/external/quiche.BUILD b/bazel/external/quiche.BUILD index decfa8ac748c3..cd4e1379fca0a 100644 --- a/bazel/external/quiche.BUILD +++ b/bazel/external/quiche.BUILD @@ -52,19 +52,20 @@ cc_library( name = "http2_platform", hdrs = [ "quiche/http2/platform/api/http2_arraysize.h", + "quiche/http2/platform/api/http2_bug_tracker.h", "quiche/http2/platform/api/http2_containers.h", "quiche/http2/platform/api/http2_estimate_memory_usage.h", "quiche/http2/platform/api/http2_export.h", "quiche/http2/platform/api/http2_flag_utils.h", + "quiche/http2/platform/api/http2_logging.h", "quiche/http2/platform/api/http2_macros.h", + "quiche/http2/platform/api/http2_mock_log.h", "quiche/http2/platform/api/http2_optional.h", "quiche/http2/platform/api/http2_ptr_util.h", "quiche/http2/platform/api/http2_string.h", "quiche/http2/platform/api/http2_string_piece.h", # TODO: uncomment the following files as implementations are added. - # "quiche/http2/platform/api/http2_bug_tracker.h", # "quiche/http2/platform/api/http2_flags.h", - # "quiche/http2/platform/api/http2_mock_log.h", # "quiche/http2/platform/api/http2_reconstruct_object.h", # "quiche/http2/platform/api/http2_test_helpers.h", ] + envoy_select_quiche( @@ -79,10 +80,12 @@ cc_library( name = "spdy_platform", hdrs = [ "quiche/spdy/platform/api/spdy_arraysize.h", + "quiche/spdy/platform/api/spdy_bug_tracker.h", "quiche/spdy/platform/api/spdy_containers.h", "quiche/spdy/platform/api/spdy_endianness_util.h", "quiche/spdy/platform/api/spdy_estimate_memory_usage.h", "quiche/spdy/platform/api/spdy_export.h", + "quiche/spdy/platform/api/spdy_logging.h", "quiche/spdy/platform/api/spdy_mem_slice.h", "quiche/spdy/platform/api/spdy_ptr_util.h", "quiche/spdy/platform/api/spdy_string.h", @@ -135,6 +138,7 @@ cc_library( "quiche/quic/platform/api/quic_containers.h", "quiche/quic/platform/api/quic_endian.h", "quiche/quic/platform/api/quic_estimate_memory_usage.h", + "quiche/quic/platform/api/quic_expect_bug.h", "quiche/quic/platform/api/quic_exported_stats.h", "quiche/quic/platform/api/quic_fallthrough.h", "quiche/quic/platform/api/quic_flag_utils.h", @@ -157,7 +161,6 @@ cc_library( "quiche/quic/platform/api/quic_thread.h", # TODO: uncomment the following files as implementations are added. # "quiche/quic/platform/api/quic_clock.h", - # "quiche/quic/platform/api/quic_expect_bug.h", # "quiche/quic/platform/api/quic_file_utils.h", # "quiche/quic/platform/api/quic_flags.h", # "quiche/quic/platform/api/quic_fuzzed_data_provider.h", diff --git a/source/extensions/quic_listeners/quiche/platform/BUILD b/source/extensions/quic_listeners/quiche/platform/BUILD index d571f9ab24888..efd691ff55eaa 100644 --- a/source/extensions/quic_listeners/quiche/platform/BUILD +++ b/source/extensions/quic_listeners/quiche/platform/BUILD @@ -42,7 +42,12 @@ envoy_cc_library( "http2_ptr_util_impl.h", "http2_string_impl.h", "http2_string_piece_impl.h", - ] + envoy_select_quiche(["http2_string_utils_impl.h"]), + ] + envoy_select_quiche([ + "http2_bug_tracker_impl.h", + "http2_logging_impl.h", + "http2_mock_log_impl.h", + "http2_string_utils_impl.h", + ]), external_deps = [ "abseil_base", "abseil_optional", @@ -80,6 +85,7 @@ envoy_cc_library( "quic_string_piece_impl.h", "quic_uint128_impl.h", ] + envoy_select_quiche([ + "quic_expect_bug_impl.h", "quic_logging_impl.h", "quic_mock_log_impl.h", "quic_stack_trace_impl.h", @@ -158,7 +164,11 @@ envoy_cc_library( "spdy_test_helpers_impl.h", "spdy_test_utils_prod_impl.h", "spdy_unsafe_arena_impl.h", - ] + envoy_select_quiche(["spdy_string_utils_impl.h"]), + ] + envoy_select_quiche([ + "spdy_bug_tracker_impl.h", + "spdy_logging_impl.h", + "spdy_string_utils_impl.h", + ]), external_deps = [ "abseil_base", "abseil_hash", diff --git a/source/extensions/quic_listeners/quiche/platform/quic_expect_bug_impl.h b/source/extensions/quic_listeners/quiche/platform/quic_expect_bug_impl.h new file mode 100644 index 0000000000000..213f8ab397a02 --- /dev/null +++ b/source/extensions/quic_listeners/quiche/platform/quic_expect_bug_impl.h @@ -0,0 +1,16 @@ +#pragma once + +// NOLINT(namespace-envoy) +// +// This file is part of the QUICHE platform implementation, and is not to be +// consumed or referenced directly by other Envoy code. It serves purely as a +// porting layer for QUICHE. + +#include "extensions/quic_listeners/quiche/platform/quic_logging_impl.h" +#include "extensions/quic_listeners/quiche/platform/quic_mock_log_impl.h" + +#define EXPECT_QUIC_BUG_IMPL(statement, regex) \ + EXPECT_QUIC_DFATAL_IMPL(statement, testing::ContainsRegex(regex)) + +#define EXPECT_QUIC_PEER_BUG_IMPL(statement, regex) \ + EXPECT_QUIC_LOG_IMPL(statement, ERROR, testing::ContainsRegex(regex)) diff --git a/source/extensions/quic_listeners/quiche/platform/quic_logging_impl.cc b/source/extensions/quic_listeners/quiche/platform/quic_logging_impl.cc index cee7e7decb32f..9a81d6104a59d 100644 --- a/source/extensions/quic_listeners/quiche/platform/quic_logging_impl.cc +++ b/source/extensions/quic_listeners/quiche/platform/quic_logging_impl.cc @@ -12,6 +12,7 @@ namespace quic { namespace { std::atomic g_verbosity_threshold; +std::atomic g_dfatal_exit_disabled; // Pointer to the global log sink, usually it is nullptr. // If not nullptr, as in some tests, the sink will receive a copy of the log message right after the @@ -39,7 +40,15 @@ QuicLogEmitter::~QuicLogEmitter() { } if (level_ == FATAL) { - abort(); +#ifdef NDEBUG + bool is_dfatal = false; +#else + bool is_dfatal = true; +#endif + + if (!(is_dfatal && g_dfatal_exit_disabled)) { + abort(); + } } } @@ -49,6 +58,12 @@ void SetVerbosityLogThreshold(int new_verbosity) { g_verbosity_threshold.store(new_verbosity, std::memory_order_relaxed); } +bool IsDFatalExitDisabled() { return g_dfatal_exit_disabled.load(std::memory_order_relaxed); } + +void SetDFatalExitDisabled(bool is_disabled) { + g_dfatal_exit_disabled.store(is_disabled, std::memory_order_relaxed); +} + QuicLogSink* SetLogSink(QuicLogSink* new_sink) { absl::MutexLock lock(&g_quic_log_sink_mutex); QuicLogSink* old_sink = g_quic_log_sink.load(std::memory_order_relaxed); diff --git a/source/extensions/quic_listeners/quiche/platform/quic_logging_impl.h b/source/extensions/quic_listeners/quiche/platform/quic_logging_impl.h index 41df60688b99c..a21d213994f8d 100644 --- a/source/extensions/quic_listeners/quiche/platform/quic_logging_impl.h +++ b/source/extensions/quic_listeners/quiche/platform/quic_logging_impl.h @@ -133,6 +133,9 @@ inline bool IsVerboseLogEnabled(int verbosity) { return IsLogLevelEnabled(INFO) && verbosity <= GetVerbosityLogThreshold(); } +bool IsDFatalExitDisabled(); +void SetDFatalExitDisabled(bool is_disabled); + // QuicLogSink is used to capture logs emitted from the QUIC_LOG... macros. class QuicLogSink { public: diff --git a/source/extensions/quic_listeners/quiche/platform/quic_mock_log_impl.h b/source/extensions/quic_listeners/quiche/platform/quic_mock_log_impl.h index 4f3c6d4e5c7a1..ed5298a07f2bc 100644 --- a/source/extensions/quic_listeners/quiche/platform/quic_mock_log_impl.h +++ b/source/extensions/quic_listeners/quiche/platform/quic_mock_log_impl.h @@ -47,6 +47,24 @@ class QuicEnvoyMockLog : public QuicLogSink { bool is_capturing_; }; +// ScopedDisableExitOnDFatal is used to disable exiting the program when we encounter a +// QUIC_LOG(DFATAL) within the current block. After we leave the current block, the previous +// behavior is restored. +class ScopedDisableExitOnDFatal { +public: + ScopedDisableExitOnDFatal() : previous_value_(IsDFatalExitDisabled()) { + SetDFatalExitDisabled(true); + } + + ScopedDisableExitOnDFatal(const ScopedDisableExitOnDFatal&) = delete; + ScopedDisableExitOnDFatal& operator=(const ScopedDisableExitOnDFatal&) = delete; + + ~ScopedDisableExitOnDFatal() { SetDFatalExitDisabled(previous_value_); } + +private: + const bool previous_value_; +}; + } // namespace quic using QuicMockLogImpl = quic::QuicEnvoyMockLog; @@ -57,3 +75,25 @@ using QuicMockLogImpl = quic::QuicEnvoyMockLog; #define EXPECT_QUIC_LOG_CALL_CONTAINS_IMPL(log, level, content) \ EXPECT_CALL(log, Log(quic::level, testing::HasSubstr(content))) + +// Not part of the api exposed by quic_mock_log.h. This is used by +// quic_expect_bug_impl.h. +#define EXPECT_QUIC_LOG_IMPL(statement, level, matcher) \ + do { \ + quic::QuicEnvoyMockLog mock_log; \ + EXPECT_CALL(mock_log, Log(quic::level, matcher)).Times(testing::AtLeast(1)); \ + mock_log.StartCapturingLogs(); \ + { statement; } \ + mock_log.StopCapturingLogs(); \ + if (!testing::Mock::VerifyAndClear(&mock_log)) { \ + GTEST_NONFATAL_FAILURE_(""); \ + } \ + } while (false) + +#define EXPECT_QUIC_DFATAL_IMPL(statement, matcher) \ + EXPECT_QUIC_LOG_IMPL( \ + { \ + quic::ScopedDisableExitOnDFatal disable_exit_on_dfatal; \ + statement; \ + }, \ + DFATAL, matcher) diff --git a/test/extensions/quic_listeners/quiche/platform/quic_platform_test.cc b/test/extensions/quic_listeners/quiche/platform/quic_platform_test.cc index 6328a7779f0cc..c18f1f90eaa99 100644 --- a/test/extensions/quic_listeners/quiche/platform/quic_platform_test.cc +++ b/test/extensions/quic_listeners/quiche/platform/quic_platform_test.cc @@ -12,6 +12,7 @@ #include "quiche/quic/platform/api/quic_containers.h" #include "quiche/quic/platform/api/quic_endian.h" #include "quiche/quic/platform/api/quic_estimate_memory_usage.h" +#include "quiche/quic/platform/api/quic_expect_bug.h" #include "quiche/quic/platform/api/quic_exported_stats.h" #include "quiche/quic/platform/api/quic_hostname_utils.h" #include "quiche/quic/platform/api/quic_logging.h" @@ -72,6 +73,18 @@ TEST(QuicPlatformTest, QuicClientStats) { quic::QuicClientSparseHistogram("my.sparse.histogram", 345); } +TEST(QuicPlatformTest, QuicExpectBug) { + auto bug = [](const char* error_message) { QUIC_BUG << error_message; }; + + auto peer_bug = [](const char* error_message) { QUIC_PEER_BUG << error_message; }; + + EXPECT_QUIC_BUG(bug("bug one is expected"), "bug one"); + EXPECT_QUIC_BUG(bug("bug two is expected"), "bug two"); + + EXPECT_QUIC_PEER_BUG(peer_bug("peer_bug_1 is expected"), "peer_bug_1"); + EXPECT_QUIC_PEER_BUG(peer_bug("peer_bug_2 is expected"), "peer_bug_2"); +} + TEST(QuicPlatformTest, QuicExportedStats) { // Just make sure they compile. QUIC_HISTOGRAM_ENUM("my.enum.histogram", TestEnum::ONE, TestEnum::COUNT, "doc"); From 113f47c6dc6e985da2119b126f56d6888f60106c Mon Sep 17 00:00:00 2001 From: Bin Wu Date: Wed, 20 Mar 2019 23:20:46 -0400 Subject: [PATCH 2/6] Add spdy_logging_impl.h and spdy_bug_tracker_impl.h. Signed-off-by: Bin Wu --- .../quic_listeners/quiche/platform/BUILD | 17 ++++++++-- .../quiche/platform/quic_bug_tracker_impl.h | 10 +++--- .../quiche/platform/spdy_bug_tracker_impl.h | 13 ++++++++ .../quiche/platform/spdy_logging_impl.h | 21 ++++++++++++ .../quic_listeners/quiche/platform/BUILD | 3 +- .../quiche/platform/spdy_platform_test.cc | 33 +++++++++++++++++++ 6 files changed, 88 insertions(+), 9 deletions(-) create mode 100644 source/extensions/quic_listeners/quiche/platform/spdy_bug_tracker_impl.h create mode 100644 source/extensions/quic_listeners/quiche/platform/spdy_logging_impl.h diff --git a/source/extensions/quic_listeners/quiche/platform/BUILD b/source/extensions/quic_listeners/quiche/platform/BUILD index efd691ff55eaa..e9def7deb1970 100644 --- a/source/extensions/quic_listeners/quiche/platform/BUILD +++ b/source/extensions/quic_listeners/quiche/platform/BUILD @@ -63,9 +63,20 @@ envoy_cc_library( visibility = ["//visibility:public"], ) +envoy_cc_library( + name = "quic_platform_logging_impl_lib", + srcs = ["quic_logging_impl.cc"], + hdrs = [ + "quic_bug_tracker_impl.h", + "quic_logging_impl.h", + "quic_mock_log_impl.h", + ], + visibility = ["//visibility:public"], + deps = ["//source/common/common:assert_lib"], +) + envoy_cc_library( name = "quic_platform_base_impl_lib", - srcs = envoy_select_quiche(["quic_logging_impl.cc"]), hdrs = [ "quic_aligned_impl.h", "quic_arraysize_impl.h", @@ -87,7 +98,6 @@ envoy_cc_library( ] + envoy_select_quiche([ "quic_expect_bug_impl.h", "quic_logging_impl.h", - "quic_mock_log_impl.h", "quic_stack_trace_impl.h", "quic_test_impl.h", "quic_thread_impl.h", @@ -103,6 +113,7 @@ envoy_cc_library( ], visibility = ["//visibility:public"], deps = ["@com_googlesource_quiche//:quic_platform_export"] + envoy_select_quiche([ + ":quic_platform_logging_impl_lib", "//include/envoy/thread:thread_interface", "//source/common/common:assert_lib", "//source/server:backtrace_lib", @@ -120,7 +131,6 @@ envoy_cc_library( "quic_mutex_impl.h", "quic_str_cat_impl.h", ] + envoy_select_quiche([ - "quic_bug_tracker_impl.h", "quic_hostname_utils_impl.h", "quic_string_utils_impl.h", "quic_test_output_impl.h", @@ -178,6 +188,7 @@ envoy_cc_library( ], visibility = ["//visibility:public"], deps = envoy_select_quiche([ + ":quic_platform_logging_impl_lib", ":string_utils_lib", "//source/common/common:assert_lib", ]), diff --git a/source/extensions/quic_listeners/quiche/platform/quic_bug_tracker_impl.h b/source/extensions/quic_listeners/quiche/platform/quic_bug_tracker_impl.h index 4f30441d1b9ac..4ddac84760dcf 100644 --- a/source/extensions/quic_listeners/quiche/platform/quic_bug_tracker_impl.h +++ b/source/extensions/quic_listeners/quiche/platform/quic_bug_tracker_impl.h @@ -6,9 +6,9 @@ // consumed or referenced directly by other Envoy code. It serves purely as a // porting layer for QUICHE. -#include "quiche/quic/platform/api/quic_logging.h" +#include "extensions/quic_listeners/quiche/platform/quic_logging_impl.h" -#define QUIC_BUG_IMPL QUIC_LOG(DFATAL) -#define QUIC_BUG_IF_IMPL(condition) QUIC_LOG_IF(DFATAL, condition) -#define QUIC_PEER_BUG_IMPL QUIC_LOG(ERROR) -#define QUIC_PEER_BUG_IF_IMPL(condition) QUIC_LOG_IF(ERROR, condition) +#define QUIC_BUG_IMPL QUIC_LOG_IMPL(DFATAL) +#define QUIC_BUG_IF_IMPL(condition) QUIC_LOG_IF_IMPL(DFATAL, condition) +#define QUIC_PEER_BUG_IMPL QUIC_LOG_IMPL(ERROR) +#define QUIC_PEER_BUG_IF_IMPL(condition) QUIC_LOG_IF_IMPL(ERROR, condition) diff --git a/source/extensions/quic_listeners/quiche/platform/spdy_bug_tracker_impl.h b/source/extensions/quic_listeners/quiche/platform/spdy_bug_tracker_impl.h new file mode 100644 index 0000000000000..93cb60e469695 --- /dev/null +++ b/source/extensions/quic_listeners/quiche/platform/spdy_bug_tracker_impl.h @@ -0,0 +1,13 @@ +#pragma once + +// NOLINT(namespace-envoy) +// +// This file is part of the QUICHE platform implementation, and is not to be +// consumed or referenced directly by other Envoy code. It serves purely as a +// porting layer for QUICHE. + +#include "extensions/quic_listeners/quiche/platform/quic_bug_tracker_impl.h" + +#define SPDY_BUG_IMPL QUIC_BUG_IMPL +#define SPDY_BUG_IF_IMPL QUIC_BUG_IF_IMPL +#define FLAGS_spdy_always_log_bugs_for_tests_impl true diff --git a/source/extensions/quic_listeners/quiche/platform/spdy_logging_impl.h b/source/extensions/quic_listeners/quiche/platform/spdy_logging_impl.h new file mode 100644 index 0000000000000..4a21b95ab34d6 --- /dev/null +++ b/source/extensions/quic_listeners/quiche/platform/spdy_logging_impl.h @@ -0,0 +1,21 @@ +#pragma once + +// NOLINT(namespace-envoy) +// +// This file is part of the QUICHE platform implementation, and is not to be +// consumed or referenced directly by other Envoy code. It serves purely as a +// porting layer for QUICHE. + +#include "extensions/quic_listeners/quiche/platform/quic_logging_impl.h" + +#define SPDY_LOG_IMPL(severity) QUIC_LOG_IMPL(severity) + +#define SPDY_VLOG_IMPL(verbose_level) QUIC_VLOG_IMPL(verbose_level) + +#define SPDY_DLOG_IMPL(severity) QUIC_DLOG_IMPL(severity) + +#define SPDY_DLOG_IF_IMPL(severity, condition) QUIC_DLOG_IF_IMPL(severity, condition) + +#define SPDY_DVLOG_IMPL(verbose_level) QUIC_DVLOG_IMPL(verbose_level) + +#define SPDY_DVLOG_IF_IMPL(verbose_level, condition) QUIC_DVLOG_IF_IMPL(verbose_level, condition) diff --git a/test/extensions/quic_listeners/quiche/platform/BUILD b/test/extensions/quic_listeners/quiche/platform/BUILD index c24563392a970..1b8f0fee2b2b7 100644 --- a/test/extensions/quic_listeners/quiche/platform/BUILD +++ b/test/extensions/quic_listeners/quiche/platform/BUILD @@ -39,9 +39,10 @@ envoy_cc_test( envoy_cc_test( name = "spdy_platform_test", - srcs = ["spdy_platform_test.cc"], + srcs = envoy_select_quiche(["spdy_platform_test.cc"]), external_deps = ["quiche_spdy_platform"], deps = [ + "//test/test_common:logging_lib", "//test/test_common:utility_lib", ], ) diff --git a/test/extensions/quic_listeners/quiche/platform/spdy_platform_test.cc b/test/extensions/quic_listeners/quiche/platform/spdy_platform_test.cc index 8c3571920a3e2..ac0895f1a09a5 100644 --- a/test/extensions/quic_listeners/quiche/platform/spdy_platform_test.cc +++ b/test/extensions/quic_listeners/quiche/platform/spdy_platform_test.cc @@ -1,10 +1,14 @@ #include +#include "test/test_common/logging.h" + #include "gtest/gtest.h" #include "quiche/spdy/platform/api/spdy_arraysize.h" +#include "quiche/spdy/platform/api/spdy_bug_tracker.h" #include "quiche/spdy/platform/api/spdy_containers.h" #include "quiche/spdy/platform/api/spdy_endianness_util.h" #include "quiche/spdy/platform/api/spdy_estimate_memory_usage.h" +#include "quiche/spdy/platform/api/spdy_logging.h" #include "quiche/spdy/platform/api/spdy_ptr_util.h" #include "quiche/spdy/platform/api/spdy_string.h" #include "quiche/spdy/platform/api/spdy_string_piece.h" @@ -26,6 +30,14 @@ TEST(SpdyPlatformTest, SpdyArraysize) { EXPECT_EQ(5, SPDY_ARRAYSIZE(array)); } +TEST(SpdyPlatformTest, SpdyBugTracker) { + EXPECT_DEBUG_DEATH(SPDY_BUG << "Here is a bug,", " bug"); + EXPECT_DEBUG_DEATH(SPDY_BUG_IF(true) << "There is a bug,", " bug"); + EXPECT_LOG_NOT_CONTAINS("error", "", SPDY_BUG_IF(false) << "A feature is not a bug."); + + EXPECT_EQ(true, FLAGS_spdy_always_log_bugs_for_tests); +} + TEST(SpdyPlatformTest, SpdyHashMap) { spdy::SpdyHashMap hmap; hmap.insert({"foo", 2}); @@ -51,6 +63,27 @@ TEST(SpdyPlatformTest, SpdyEstimateMemoryUsage) { EXPECT_EQ(0, spdy::SpdyEstimateMemoryUsage(s)); } +TEST(SpdyPlatformTest, SpdyLog) { + // SPDY_LOG macros are defined to QUIC_LOG macros, which is tested in + // QuicPlatformTest. Here we just make sure SPDY_LOG macros compile. + SPDY_LOG(INFO) << "INFO log may not show up by default."; + SPDY_LOG(ERROR) << "ERROR log should show up by default."; + + // VLOGs are only emitted if INFO is enabled and verbosity level is high enough. + SPDY_VLOG(1) << "VLOG(1)"; + + SPDY_DLOG(INFO) << "DLOG(INFO)"; + SPDY_DLOG(ERROR) << "DLOG(ERROR)"; + + SPDY_DLOG_IF(ERROR, true) << "DLOG_IF(ERROR, true)"; + SPDY_DLOG_IF(ERROR, false) << "DLOG_IF(ERROR, false)"; + + SPDY_DVLOG(2) << "DVLOG(2)"; + + SPDY_DVLOG_IF(3, true) << "DVLOG_IF(3, true)"; + SPDY_DVLOG_IF(4, false) << "DVLOG_IF(4, false)"; +} + TEST(SpdyPlatformTest, SpdyMakeUnique) { auto p = spdy::SpdyMakeUnique(4); EXPECT_EQ(4, *p); From 9bf4b264c9d558edbc6a6c44af261d2805b880c4 Mon Sep 17 00:00:00 2001 From: Bin Wu Date: Thu, 21 Mar 2019 00:02:59 -0400 Subject: [PATCH 3/6] Add http2_logging_impl.h and http2_bug_tracker_impl.h. Signed-off-by: Bin Wu --- bazel/external/quiche.BUILD | 1 - .../quic_listeners/quiche/platform/BUILD | 8 +++-- .../quiche/platform/http2_bug_tracker_impl.h | 13 +++++++ .../quiche/platform/http2_logging_impl.h | 23 ++++++++++++ .../quiche/platform/quic_logging_impl.h | 5 +++ .../quic_listeners/quiche/platform/BUILD | 3 +- .../quiche/platform/http2_platform_test.cc | 35 +++++++++++++++++++ 7 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 source/extensions/quic_listeners/quiche/platform/http2_bug_tracker_impl.h create mode 100644 source/extensions/quic_listeners/quiche/platform/http2_logging_impl.h diff --git a/bazel/external/quiche.BUILD b/bazel/external/quiche.BUILD index cd4e1379fca0a..97b1439e6aa3a 100644 --- a/bazel/external/quiche.BUILD +++ b/bazel/external/quiche.BUILD @@ -59,7 +59,6 @@ cc_library( "quiche/http2/platform/api/http2_flag_utils.h", "quiche/http2/platform/api/http2_logging.h", "quiche/http2/platform/api/http2_macros.h", - "quiche/http2/platform/api/http2_mock_log.h", "quiche/http2/platform/api/http2_optional.h", "quiche/http2/platform/api/http2_ptr_util.h", "quiche/http2/platform/api/http2_string.h", diff --git a/source/extensions/quic_listeners/quiche/platform/BUILD b/source/extensions/quic_listeners/quiche/platform/BUILD index e9def7deb1970..afbf9bbfba729 100644 --- a/source/extensions/quic_listeners/quiche/platform/BUILD +++ b/source/extensions/quic_listeners/quiche/platform/BUILD @@ -45,7 +45,6 @@ envoy_cc_library( ] + envoy_select_quiche([ "http2_bug_tracker_impl.h", "http2_logging_impl.h", - "http2_mock_log_impl.h", "http2_string_utils_impl.h", ]), external_deps = [ @@ -54,7 +53,10 @@ envoy_cc_library( "abseil_str_format", ], visibility = ["//visibility:public"], - deps = envoy_select_quiche([":string_utils_lib"]), + deps = envoy_select_quiche([ + ":quic_platform_logging_impl_lib", + ":string_utils_lib", + ]), ) envoy_cc_library( @@ -69,7 +71,6 @@ envoy_cc_library( hdrs = [ "quic_bug_tracker_impl.h", "quic_logging_impl.h", - "quic_mock_log_impl.h", ], visibility = ["//visibility:public"], deps = ["//source/common/common:assert_lib"], @@ -98,6 +99,7 @@ envoy_cc_library( ] + envoy_select_quiche([ "quic_expect_bug_impl.h", "quic_logging_impl.h", + "quic_mock_log_impl.h", "quic_stack_trace_impl.h", "quic_test_impl.h", "quic_thread_impl.h", diff --git a/source/extensions/quic_listeners/quiche/platform/http2_bug_tracker_impl.h b/source/extensions/quic_listeners/quiche/platform/http2_bug_tracker_impl.h new file mode 100644 index 0000000000000..58c7039d536bb --- /dev/null +++ b/source/extensions/quic_listeners/quiche/platform/http2_bug_tracker_impl.h @@ -0,0 +1,13 @@ +#pragma once + +// NOLINT(namespace-envoy) +// +// This file is part of the QUICHE platform implementation, and is not to be +// consumed or referenced directly by other Envoy code. It serves purely as a +// porting layer for QUICHE. + +#include "extensions/quic_listeners/quiche/platform/quic_bug_tracker_impl.h" + +#define HTTP2_BUG_IMPL QUIC_BUG_IMPL +#define HTTP2_BUG_IF_IMPL QUIC_BUG_IF_IMPL +#define FLAGS_http2_always_log_bugs_for_tests_IMPL true diff --git a/source/extensions/quic_listeners/quiche/platform/http2_logging_impl.h b/source/extensions/quic_listeners/quiche/platform/http2_logging_impl.h new file mode 100644 index 0000000000000..473c2d00d4bd4 --- /dev/null +++ b/source/extensions/quic_listeners/quiche/platform/http2_logging_impl.h @@ -0,0 +1,23 @@ +#pragma once + +// NOLINT(namespace-envoy) +// +// This file is part of the QUICHE platform implementation, and is not to be +// consumed or referenced directly by other Envoy code. It serves purely as a +// porting layer for QUICHE. + +#include "extensions/quic_listeners/quiche/platform/quic_logging_impl.h" + +#define HTTP2_LOG_IMPL(severity) QUIC_LOG_IMPL(severity) + +#define HTTP2_VLOG_IMPL(verbose_level) QUIC_VLOG_IMPL(verbose_level) + +#define HTTP2_DLOG_IMPL(severity) QUIC_DLOG_IMPL(severity) + +#define HTTP2_DLOG_IF_IMPL(severity, condition) QUIC_DLOG_IF_IMPL(severity, condition) + +#define HTTP2_DVLOG_IMPL(verbose_level) QUIC_DVLOG_IMPL(verbose_level) + +#define HTTP2_DVLOG_IF_IMPL(verbose_level, condition) QUIC_DVLOG_IF_IMPL(verbose_level, condition) + +#define HTTP2_DLOG_EVERY_N_IMPL(severity, n) QUIC_DLOG_EVERY_N_IMPL(severity, n) diff --git a/source/extensions/quic_listeners/quiche/platform/quic_logging_impl.h b/source/extensions/quic_listeners/quiche/platform/quic_logging_impl.h index a21d213994f8d..d3bc84bad7d22 100644 --- a/source/extensions/quic_listeners/quiche/platform/quic_logging_impl.h +++ b/source/extensions/quic_listeners/quiche/platform/quic_logging_impl.h @@ -45,6 +45,9 @@ // TODO(wub): Implement QUIC_LOG_FIRST_N_IMPL. #define QUIC_LOG_FIRST_N_IMPL(severity, n) QUIC_LOG_IMPL(severity) +// TODO(wub): Implement QUIC_LOG_EVERY_N_IMPL. +#define QUIC_LOG_EVERY_N_IMPL(severity, n) QUIC_LOG_IMPL(severity) + // TODO(wub): Implement QUIC_LOG_EVERY_N_SEC_IMPL. #define QUIC_LOG_EVERY_N_SEC_IMPL(severity, seconds) QUIC_LOG_IMPL(severity) @@ -64,6 +67,7 @@ #define QUIC_DLOG_IMPL(severity) QUIC_COMPILED_OUT_LOG() #define QUIC_DLOG_IF_IMPL(severity, condition) QUIC_COMPILED_OUT_LOG() #define QUIC_DLOG_INFO_IS_ON_IMPL() 0 +#define QUIC_DLOG_EVERY_N_IMPL(severity, n) QUIC_COMPILED_OUT_LOG() #define QUIC_NOTREACHED_IMPL() #else // Debug build @@ -72,6 +76,7 @@ #define QUIC_DLOG_IMPL(severity) QUIC_LOG_IMPL(severity) #define QUIC_DLOG_IF_IMPL(severity, condition) QUIC_LOG_IF_IMPL(severity, condition) #define QUIC_DLOG_INFO_IS_ON_IMPL() QUIC_LOG_INFO_IS_ON_IMPL() +#define QUIC_DLOG_EVERY_N_IMPL(severity, n) QUIC_LOG_EVERY_N_IMPL(severity, n) #define QUIC_NOTREACHED_IMPL() NOT_REACHED_GCOVR_EXCL_LINE #endif diff --git a/test/extensions/quic_listeners/quiche/platform/BUILD b/test/extensions/quic_listeners/quiche/platform/BUILD index 1b8f0fee2b2b7..16d008648f526 100644 --- a/test/extensions/quic_listeners/quiche/platform/BUILD +++ b/test/extensions/quic_listeners/quiche/platform/BUILD @@ -15,9 +15,10 @@ envoy_package() envoy_cc_test( name = "http2_platform_test", - srcs = ["http2_platform_test.cc"], + srcs = envoy_select_quiche(["http2_platform_test.cc"]), external_deps = ["quiche_http2_platform"], deps = [ + "//test/test_common:logging_lib", "//test/test_common:utility_lib", ], ) diff --git a/test/extensions/quic_listeners/quiche/platform/http2_platform_test.cc b/test/extensions/quic_listeners/quiche/platform/http2_platform_test.cc index f17b77ae9808c..2362870cae5b0 100644 --- a/test/extensions/quic_listeners/quiche/platform/http2_platform_test.cc +++ b/test/extensions/quic_listeners/quiche/platform/http2_platform_test.cc @@ -1,9 +1,13 @@ #include +#include "test/test_common/logging.h" + #include "gtest/gtest.h" #include "quiche/http2/platform/api/http2_arraysize.h" +#include "quiche/http2/platform/api/http2_bug_tracker.h" #include "quiche/http2/platform/api/http2_containers.h" #include "quiche/http2/platform/api/http2_estimate_memory_usage.h" +#include "quiche/http2/platform/api/http2_logging.h" #include "quiche/http2/platform/api/http2_optional.h" #include "quiche/http2/platform/api/http2_ptr_util.h" #include "quiche/http2/platform/api/http2_string.h" @@ -26,6 +30,14 @@ TEST(Http2PlatformTest, Http2Arraysize) { EXPECT_EQ(5, HTTP2_ARRAYSIZE(array)); } +TEST(Http2PlatformTest, Http2BugTracker) { + EXPECT_DEBUG_DEATH(HTTP2_BUG << "Here is a bug,", " bug"); + EXPECT_DEBUG_DEATH(HTTP2_BUG_IF(true) << "There is a bug,", " bug"); + EXPECT_LOG_NOT_CONTAINS("error", "", HTTP2_BUG_IF(false) << "A feature is not a bug."); + + EXPECT_EQ(true, FLAGS_http2_always_log_bugs_for_tests); +} + TEST(Http2PlatformTest, Http2Deque) { http2::Http2Deque deque; deque.push_back(10); @@ -38,6 +50,29 @@ TEST(Http2PlatformTest, Http2EstimateMemoryUsage) { EXPECT_EQ(0, http2::Http2EstimateMemoryUsage(s)); } +TEST(Http2PlatformTest, Http2Log) { + // HTTP2_LOG macros are defined to QUIC_LOG macros, which is tested in + // QuicPlatformTest. Here we just make sure HTTP2_LOG macros compile. + HTTP2_LOG(INFO) << "INFO log may not show up by default."; + HTTP2_LOG(ERROR) << "ERROR log should show up by default."; + + // VLOGs are only emitted if INFO is enabled and verbosity level is high enough. + HTTP2_VLOG(1) << "VLOG(1)"; + + HTTP2_DLOG(INFO) << "DLOG(INFO)"; + HTTP2_DLOG(ERROR) << "DLOG(ERROR)"; + + HTTP2_DLOG_IF(ERROR, true) << "DLOG_IF(ERROR, true)"; + HTTP2_DLOG_IF(ERROR, false) << "DLOG_IF(ERROR, false)"; + + HTTP2_DVLOG(2) << "DVLOG(2)"; + + HTTP2_DVLOG_IF(3, true) << "DVLOG_IF(3, true)"; + HTTP2_DVLOG_IF(4, false) << "DVLOG_IF(4, false)"; + + HTTP2_DLOG_EVERY_N(ERROR, 2) << "DLOG_EVERY_N(ERROR, 2)"; +} + TEST(Http2PlatformTest, Http2Optional) { http2::Http2Optional opt; EXPECT_FALSE(opt.has_value()); From c338d2f5f741757bae63802f64c6f3d554139798 Mon Sep 17 00:00:00 2001 From: Bin Wu Date: Thu, 21 Mar 2019 09:27:34 -0400 Subject: [PATCH 4/6] s/VLOGs/VLOG/ to make spelling checker happy. Signed-off-by: Bin Wu --- .../quic_listeners/quiche/platform/http2_platform_test.cc | 2 +- .../quic_listeners/quiche/platform/spdy_platform_test.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/extensions/quic_listeners/quiche/platform/http2_platform_test.cc b/test/extensions/quic_listeners/quiche/platform/http2_platform_test.cc index 2362870cae5b0..10b9b38787e1e 100644 --- a/test/extensions/quic_listeners/quiche/platform/http2_platform_test.cc +++ b/test/extensions/quic_listeners/quiche/platform/http2_platform_test.cc @@ -56,7 +56,7 @@ TEST(Http2PlatformTest, Http2Log) { HTTP2_LOG(INFO) << "INFO log may not show up by default."; HTTP2_LOG(ERROR) << "ERROR log should show up by default."; - // VLOGs are only emitted if INFO is enabled and verbosity level is high enough. + // VLOG are only emitted if INFO is enabled and verbosity level is high enough. HTTP2_VLOG(1) << "VLOG(1)"; HTTP2_DLOG(INFO) << "DLOG(INFO)"; diff --git a/test/extensions/quic_listeners/quiche/platform/spdy_platform_test.cc b/test/extensions/quic_listeners/quiche/platform/spdy_platform_test.cc index ac0895f1a09a5..6ac1874cad0e6 100644 --- a/test/extensions/quic_listeners/quiche/platform/spdy_platform_test.cc +++ b/test/extensions/quic_listeners/quiche/platform/spdy_platform_test.cc @@ -69,7 +69,7 @@ TEST(SpdyPlatformTest, SpdyLog) { SPDY_LOG(INFO) << "INFO log may not show up by default."; SPDY_LOG(ERROR) << "ERROR log should show up by default."; - // VLOGs are only emitted if INFO is enabled and verbosity level is high enough. + // VLOG is only emitted if INFO is enabled and verbosity level is high enough. SPDY_VLOG(1) << "VLOG(1)"; SPDY_DLOG(INFO) << "DLOG(INFO)"; From 105d1713ad42cb861497c8478e0782b34ec37f26 Mon Sep 17 00:00:00 2001 From: Bin Wu Date: Thu, 21 Mar 2019 13:30:38 -0400 Subject: [PATCH 5/6] Update per review comment. Signed-off-by: Bin Wu --- bazel/external/quiche.BUILD | 47 ++++++++++++------- .../quiche/platform/quic_logging_impl.cc | 10 ++-- 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/bazel/external/quiche.BUILD b/bazel/external/quiche.BUILD index 97b1439e6aa3a..7b60a1f072452 100644 --- a/bazel/external/quiche.BUILD +++ b/bazel/external/quiche.BUILD @@ -52,12 +52,10 @@ cc_library( name = "http2_platform", hdrs = [ "quiche/http2/platform/api/http2_arraysize.h", - "quiche/http2/platform/api/http2_bug_tracker.h", "quiche/http2/platform/api/http2_containers.h", "quiche/http2/platform/api/http2_estimate_memory_usage.h", "quiche/http2/platform/api/http2_export.h", "quiche/http2/platform/api/http2_flag_utils.h", - "quiche/http2/platform/api/http2_logging.h", "quiche/http2/platform/api/http2_macros.h", "quiche/http2/platform/api/http2_optional.h", "quiche/http2/platform/api/http2_ptr_util.h", @@ -68,7 +66,11 @@ cc_library( # "quiche/http2/platform/api/http2_reconstruct_object.h", # "quiche/http2/platform/api/http2_test_helpers.h", ] + envoy_select_quiche( - ["quiche/http2/platform/api/http2_string_utils.h"], + [ + "quiche/http2/platform/api/http2_bug_tracker.h", + "quiche/http2/platform/api/http2_logging.h", + "quiche/http2/platform/api/http2_string_utils.h", + ], "@envoy", ), visibility = ["//visibility:public"], @@ -79,12 +81,10 @@ cc_library( name = "spdy_platform", hdrs = [ "quiche/spdy/platform/api/spdy_arraysize.h", - "quiche/spdy/platform/api/spdy_bug_tracker.h", "quiche/spdy/platform/api/spdy_containers.h", "quiche/spdy/platform/api/spdy_endianness_util.h", "quiche/spdy/platform/api/spdy_estimate_memory_usage.h", "quiche/spdy/platform/api/spdy_export.h", - "quiche/spdy/platform/api/spdy_logging.h", "quiche/spdy/platform/api/spdy_mem_slice.h", "quiche/spdy/platform/api/spdy_ptr_util.h", "quiche/spdy/platform/api/spdy_string.h", @@ -92,7 +92,11 @@ cc_library( # TODO: uncomment the following files as implementations are added. # "quiche/spdy/platform/api/spdy_flags.h", ] + envoy_select_quiche( - ["quiche/spdy/platform/api/spdy_string_utils.h"], + [ + "quiche/spdy/platform/api/spdy_bug_tracker.h", + "quiche/spdy/platform/api/spdy_logging.h", + "quiche/spdy/platform/api/spdy_string_utils.h", + ], "@envoy", ), visibility = ["//visibility:public"], @@ -132,24 +136,19 @@ cc_library( hdrs = [ "quiche/quic/platform/api/quic_aligned.h", "quiche/quic/platform/api/quic_arraysize.h", - "quiche/quic/platform/api/quic_bug_tracker.h", "quiche/quic/platform/api/quic_client_stats.h", "quiche/quic/platform/api/quic_containers.h", "quiche/quic/platform/api/quic_endian.h", "quiche/quic/platform/api/quic_estimate_memory_usage.h", - "quiche/quic/platform/api/quic_expect_bug.h", "quiche/quic/platform/api/quic_exported_stats.h", "quiche/quic/platform/api/quic_fallthrough.h", "quiche/quic/platform/api/quic_flag_utils.h", "quiche/quic/platform/api/quic_iovec.h", - "quiche/quic/platform/api/quic_logging.h", "quiche/quic/platform/api/quic_map_util.h", - "quiche/quic/platform/api/quic_mock_log.h", "quiche/quic/platform/api/quic_prefetch.h", "quiche/quic/platform/api/quic_ptr_util.h", "quiche/quic/platform/api/quic_reference_counted.h", "quiche/quic/platform/api/quic_server_stats.h", - "quiche/quic/platform/api/quic_stack_trace.h", "quiche/quic/platform/api/quic_string.h", "quiche/quic/platform/api/quic_string_piece.h", "quiche/quic/platform/api/quic_string_utils.h", @@ -157,7 +156,6 @@ cc_library( "quiche/quic/platform/api/quic_test_output.h", "quiche/quic/platform/api/quic_text_utils.h", "quiche/quic/platform/api/quic_uint128.h", - "quiche/quic/platform/api/quic_thread.h", # TODO: uncomment the following files as implementations are added. # "quiche/quic/platform/api/quic_clock.h", # "quiche/quic/platform/api/quic_file_utils.h", @@ -176,7 +174,17 @@ cc_library( # "quiche/quic/platform/api/quic_test.h", # "quiche/quic/platform/api/quic_test_loopback.h", # "quiche/quic/platform/api/quic_test_mem_slice_vector.h", - ], + ] + envoy_select_quiche( + [ + "quiche/quic/platform/api/quic_bug_tracker.h", + "quiche/quic/platform/api/quic_expect_bug.h", + "quiche/quic/platform/api/quic_mock_log.h", + "quiche/quic/platform/api/quic_logging.h", + "quiche/quic/platform/api/quic_stack_trace.h", + "quiche/quic/platform/api/quic_thread.h", + ], + "@envoy", + ), visibility = ["//visibility:public"], deps = [ ":quic_platform_export", @@ -221,11 +229,14 @@ envoy_cc_test( envoy_cc_test( name = "quic_platform_test", - srcs = [ - "quiche/quic/platform/api/quic_reference_counted_test.cc", - "quiche/quic/platform/api/quic_string_utils_test.cc", - "quiche/quic/platform/api/quic_text_utils_test.cc", - ], + srcs = envoy_select_quiche( + [ + "quiche/quic/platform/api/quic_reference_counted_test.cc", + "quiche/quic/platform/api/quic_string_utils_test.cc", + "quiche/quic/platform/api/quic_text_utils_test.cc", + ], + "@envoy", + ), repository = "@envoy", deps = [":quic_platform"], ) diff --git a/source/extensions/quic_listeners/quiche/platform/quic_logging_impl.cc b/source/extensions/quic_listeners/quiche/platform/quic_logging_impl.cc index 9a81d6104a59d..578fbffc28335 100644 --- a/source/extensions/quic_listeners/quiche/platform/quic_logging_impl.cc +++ b/source/extensions/quic_listeners/quiche/platform/quic_logging_impl.cc @@ -41,14 +41,14 @@ QuicLogEmitter::~QuicLogEmitter() { if (level_ == FATAL) { #ifdef NDEBUG - bool is_dfatal = false; + // Release mode. + abort(); #else - bool is_dfatal = true; -#endif - - if (!(is_dfatal && g_dfatal_exit_disabled)) { + // Debug mode. + if (!g_dfatal_exit_disabled) { abort(); } +#endif } } From f9f7cde414ea4c97a046b7a37dc66c26065c146b Mon Sep 17 00:00:00 2001 From: Bin Wu Date: Mon, 25 Mar 2019 11:32:03 -0400 Subject: [PATCH 6/6] Add a TODO for QUIC_BUG to implement exponential back off. Signed-off-by: Bin Wu --- .../quic_listeners/quiche/platform/quic_bug_tracker_impl.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/source/extensions/quic_listeners/quiche/platform/quic_bug_tracker_impl.h b/source/extensions/quic_listeners/quiche/platform/quic_bug_tracker_impl.h index 4ddac84760dcf..050bd385d8818 100644 --- a/source/extensions/quic_listeners/quiche/platform/quic_bug_tracker_impl.h +++ b/source/extensions/quic_listeners/quiche/platform/quic_bug_tracker_impl.h @@ -8,6 +8,8 @@ #include "extensions/quic_listeners/quiche/platform/quic_logging_impl.h" +// TODO(wub): Implement exponential back off to avoid performance problems due +// to excessive QUIC_BUG. #define QUIC_BUG_IMPL QUIC_LOG_IMPL(DFATAL) #define QUIC_BUG_IF_IMPL(condition) QUIC_LOG_IF_IMPL(DFATAL, condition) #define QUIC_PEER_BUG_IMPL QUIC_LOG_IMPL(ERROR)