diff --git a/.bazelrc b/.bazelrc index f80346d54..23ca82ff4 100644 --- a/.bazelrc +++ b/.bazelrc @@ -134,23 +134,24 @@ build:coverage --strategy=CoverageReport=sandboxed,local build:coverage --experimental_use_llvm_covmap build:coverage --collect_code_coverage build:coverage --test_tag_filters=-nocoverage -build:coverage --instrumentation_filter="//source(?!/common/chromium_url|/extensions/quic_listeners/quiche/platform)[/:],//include[/:]" -coverage:test-coverage --test_arg="--log-path /dev/null" +build:coverage --instrumentation_filter="//source(?!/extensions/quic_listeners/quiche/platform)[/:],//include[/:]" coverage:test-coverage --test_arg="-l trace" -coverage:fuzz-coverage --config=asan-fuzzer +coverage:fuzz-coverage --config=plain-fuzzer coverage:fuzz-coverage --run_under=@envoy//bazel/coverage:fuzz_coverage_wrapper.sh # Remote execution: https://docs.bazel.build/versions/master/remote-execution.html -build:rbe-toolchain --host_platform=@envoy_build_tools//toolchains:rbe_ubuntu_clang_platform -build:rbe-toolchain --platforms=@envoy_build_tools//toolchains:rbe_ubuntu_clang_platform build:rbe-toolchain --action_env=BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN=1 build:rbe-toolchain-clang --config=rbe-toolchain +build:rbe-toolchain-clang --platforms=@rbe_ubuntu_clang//config:platform +build:rbe-toolchain-clang --host_platform=@rbe_ubuntu_clang//config:platform build:rbe-toolchain-clang --crosstool_top=@rbe_ubuntu_clang//cc:toolchain build:rbe-toolchain-clang --extra_toolchains=@rbe_ubuntu_clang//config:cc-toolchain build:rbe-toolchain-clang --action_env=CC=clang --action_env=CXX=clang++ --action_env=PATH=/usr/sbin:/usr/bin:/sbin:/bin:/opt/llvm/bin build:rbe-toolchain-clang-libc++ --config=rbe-toolchain +build:rbe-toolchain-clang-libc++ --platforms=@rbe_ubuntu_clang_libcxx//config:platform +build:rbe-toolchain-clang-libc++ --host_platform=@rbe_ubuntu_clang_libcxx//config:platform build:rbe-toolchain-clang-libc++ --crosstool_top=@rbe_ubuntu_clang_libcxx//cc:toolchain build:rbe-toolchain-clang-libc++ --extra_toolchains=@rbe_ubuntu_clang_libcxx//config:cc-toolchain build:rbe-toolchain-clang-libc++ --action_env=CC=clang --action_env=CXX=clang++ --action_env=PATH=/usr/sbin:/usr/bin:/sbin:/bin:/opt/llvm/bin @@ -163,6 +164,8 @@ build:rbe-toolchain-msan --linkopt=-Wl,-rpath,/opt/libcxx_msan/lib build:rbe-toolchain-msan --config=clang-msan build:rbe-toolchain-gcc --config=rbe-toolchain +build:rbe-toolchain-gcc --platforms=@rbe_ubuntu_gcc//config:platform +build:rbe-toolchain-gcc --host_platform=@rbe_ubuntu_gcc//config:platform build:rbe-toolchain-gcc --crosstool_top=@rbe_ubuntu_gcc//cc:toolchain build:rbe-toolchain-gcc --extra_toolchains=@rbe_ubuntu_gcc//config:cc-toolchain diff --git a/.circleci/config.yml b/.circleci/config.yml index f24261777..505a8ff5b 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -24,13 +24,13 @@ jobs: steps: - checkout - run: ci/do_ci.sh clang_tidy - test_with_valgrind: + test_gcc: docker: - image: *envoy-build-image resource_class: xlarge steps: - checkout - - run: ci/do_ci.sh test_with_valgrind + - run: ci/do_ci.sh test_gcc coverage: docker: - image: *envoy-build-image @@ -57,7 +57,9 @@ jobs: resource_class: xlarge steps: - checkout - - run: ci/do_ci.sh asan + - run: + command: ci/do_ci.sh asan + no_output_timeout: 30m tsan: docker: - image: *envoy-build-image @@ -88,8 +90,8 @@ workflows: jobs: - build - test + - test_gcc - clang_tidy - # - test_with_valgrind - coverage - asan - tsan diff --git a/.gitignore b/.gitignore index a099a0c5f..2970d2e8a 100644 --- a/.gitignore +++ b/.gitignore @@ -17,4 +17,4 @@ tools/pyformat test/coverage/BUILD *.bak default.profraw -tmp-* \ No newline at end of file +tmp-* diff --git a/BUILD b/BUILD index 7b12a5738..7c0820a4b 100644 --- a/BUILD +++ b/BUILD @@ -30,6 +30,7 @@ envoy_cc_binary( name = "nighthawk_test_server", repository = "@envoy", deps = [ + "//source/server:http_dynamic_delay_filter_config", "//source/server:http_test_server_filter_config", "@envoy//source/exe:envoy_main_entry_lib", ], diff --git a/api/server/response_options.proto b/api/server/response_options.proto index 69b3bd7d6..0644d3d0e 100644 --- a/api/server/response_options.proto +++ b/api/server/response_options.proto @@ -5,8 +5,19 @@ package nighthawk.server; import "google/protobuf/wrappers.proto"; import "validate/validate.proto"; import "envoy/api/v2/core/base.proto"; +import "google/protobuf/duration.proto"; -// Options that control the test server response. +message ConcurrencyBasedLinearDelay { + // Minimal delay to add to replies. + google.protobuf.Duration minimal_delay = 1 [(validate.rules).duration.gte.nanos = 0]; + // Factor to use when adding latency as concurrency increases. + google.protobuf.Duration concurrency_delay_factor = 2 [(validate.rules).duration.gte.nanos = 0]; +} + +// Options that control the test server response. Can be provided via request +// headers as well as via static file-based configuration. In case both are +// provided, a merge will happen, in which case the header-provided +// configuration will override. message ResponseOptions { // List of additional response headers. repeated envoy.api.v2.core.HeaderValueOption response_headers = 1; @@ -14,4 +25,11 @@ message ResponseOptions { uint32 response_body_size = 2 [(validate.rules).uint32 = {lte: 4194304}]; // If true, then echo request headers in the response body. bool echo_request_headers = 3; + + oneof oneof_delay_options { + // Static delay duration. + google.protobuf.Duration static_delay = 4 [(validate.rules).duration.gte.nanos = 0]; + // Concurrency based linear delay configuration. + ConcurrencyBasedLinearDelay concurrency_based_linear_delay = 5; + } } diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index c3dc11867..ed06d2f7b 100644 --- a/bazel/repositories.bzl +++ b/bazel/repositories.bzl @@ -1,10 +1,10 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") -ENVOY_COMMIT = "f6b86a58b264b46a57d71a9b3b0989b2969df408" # July 2nd, 2020 -ENVOY_SHA = "5c802266f0cdc5193b6e0247a0f5f20f39f6cc36b688b194e60a853148ba438a" +ENVOY_COMMIT = "7abb0e0bbed4f6b6304403b93762614ad385f80d" # July 14th, 2020 +ENVOY_SHA = "13fd08f9478e3dee5289581eb8f5f85dfc53fa5ac21555e0e86af536e5a200d8" -HDR_HISTOGRAM_C_VERSION = "0.9.13" # Feb 22nd, 2020 -HDR_HISTOGRAM_C_SHA = "2bd4a4631b64f2f8cf968ef49dd03ff3c51b487c3c98a01217ae4cf4a35b8310" +HDR_HISTOGRAM_C_VERSION = "0.11.0" # July 14th, 2020 +HDR_HISTOGRAM_C_SHA = "c00696b3d81776675aa2bc62d3642e31bd8a48cc9619c9bd7d4a78762896e353" def nighthawk_dependencies(): http_archive( diff --git a/ci/do_ci.sh b/ci/do_ci.sh index 39c618052..92658d437 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -7,31 +7,25 @@ set -u export BUILDIFIER_BIN="${BUILDIFIER_BIN:=/usr/local/bin/buildifier}" export BUILDOZER_BIN="${BUILDOZER_BIN:=/usr/local/bin/buildozer}" export NUM_CPUS=${NUM_CPUS:=$(grep -c ^processor /proc/cpuinfo)} -export CIRCLECI=${CIRCLECI:="")} +export CIRCLECI=${CIRCLECI:=""} export BAZEL_EXTRA_TEST_OPTIONS=${BAZEL_EXTRA_TEST_OPTIONS:=""} export BAZEL_OPTIONS=${BAZEL_OPTIONS:=""} export BAZEL_BUILD_EXTRA_OPTIONS=${BAZEL_BUILD_EXTRA_OPTIONS:=""} export SRCDIR=${SRCDIR:="${PWD}"} +export CLANG_FORMAT=clang-format function do_build () { - bazel build $BAZEL_BUILD_OPTIONS --verbose_failures=true //:nighthawk + bazel build $BAZEL_BUILD_OPTIONS //:nighthawk tools/update_cli_readme_documentation.sh --mode check } function do_opt_build () { - bazel build $BAZEL_BUILD_OPTIONS -c opt --verbose_failures=true //:nighthawk + bazel build $BAZEL_BUILD_OPTIONS -c opt //:nighthawk } function do_test() { - bazel test $BAZEL_BUILD_OPTIONS $BAZEL_TEST_OPTIONS \ - --test_output=all \ - //test/... -} - -function do_test_with_valgrind() { - apt-get update && apt-get install valgrind && \ - bazel build $BAZEL_BUILD_OPTIONS -c dbg //test/... && \ - nighthawk/tools/valgrind-tests.sh + bazel build $BAZEL_BUILD_OPTIONS //test/... + bazel test $BAZEL_TEST_OPTIONS --test_output=all //test/... } function do_clang_tidy() { @@ -41,15 +35,17 @@ function do_clang_tidy() { function do_coverage() { export TEST_TARGETS="//test/..." echo "bazel coverage build with tests ${TEST_TARGETS}" - - # Reduce the amount of memory Bazel tries to use to prevent it from launching too many subprocesses. - # This should prevent the system from running out of memory and killing tasks. See discussion on - # https://github.com/envoyproxy/envoy/pull/5611. - [ -z "$CIRCLECI" ] || export BAZEL_BUILD_OPTIONS="${BAZEL_BUILD_OPTIONS} --local_ram_resources=12288" test/run_nighthawk_bazel_coverage.sh ${TEST_TARGETS} exit 0 } +function setup_gcc_toolchain() { + export CC=gcc + export CXX=g++ + export BAZEL_COMPILER=gcc + echo "$CC/$CXX toolchain configured" +} + function setup_clang_toolchain() { export PATH=/opt/llvm/bin:$PATH export CC=clang @@ -93,7 +89,10 @@ function do_tsan() { echo "bazel TSAN debug build with tests" echo "Building and testing envoy tests..." cd "${SRCDIR}" - [ -z "$CIRCLECI" ] || export BAZEL_BUILD_OPTIONS="${BAZEL_TEST_OPTIONS} --local_ram_resources=12288" + run_bazel build ${BAZEL_TEST_OPTIONS} -c dbg --config=clang-tsan -- //source/exe/... && \ + run_bazel build ${BAZEL_TEST_OPTIONS} -c dbg --config=clang-tsan -- //source/server/... && \ + run_bazel build ${BAZEL_TEST_OPTIONS} -c dbg --config=clang-tsan -- //test/mocks/... && \ + run_bazel build ${BAZEL_TEST_OPTIONS} -c dbg --config=clang-tsan -- //test/... && \ run_bazel test ${BAZEL_TEST_OPTIONS} -c dbg --config=clang-tsan //test/... } @@ -150,21 +149,6 @@ function do_fix_format() { ./tools/format_python_tools.sh fix } -if [ -n "$CIRCLECI" ]; then - if [[ -f "${HOME:-/root}/.gitconfig" ]]; then - mv "${HOME:-/root}/.gitconfig" "${HOME:-/root}/.gitconfig_save" - echo 1 - fi - # We constrain parallelism in CI to avoid running out of memory. - NUM_CPUS=8 - if [[ "$1" == "asan" ]]; then - NUM_CPUS=5 - fi - if [[ "$1" == "coverage" ]]; then - NUM_CPUS=3 - fi -fi - if grep 'docker\|lxc' /proc/1/cgroup; then # Create a fake home. Python site libs tries to do getpwuid(3) if we don't and the CI # Docker image gets confused as it has no passwd entry when running non-root @@ -190,65 +174,91 @@ fi export BAZEL_EXTRA_TEST_OPTIONS="--test_env=ENVOY_IP_TEST_VERSIONS=v4only ${BAZEL_EXTRA_TEST_OPTIONS}" export BAZEL_BUILD_OPTIONS=" \ --verbose_failures ${BAZEL_OPTIONS} --action_env=HOME --action_env=PYTHONUSERBASE \ ---jobs=${NUM_CPUS} --show_task_finish --experimental_generate_json_trace_profile ${BAZEL_BUILD_EXTRA_OPTIONS}" +--experimental_local_memory_estimate \ +--show_task_finish --experimental_generate_json_trace_profile ${BAZEL_BUILD_EXTRA_OPTIONS}" + +if [ -n "$CIRCLECI" ]; then + if [[ -f "${HOME:-/root}/.gitconfig" ]]; then + mv "${HOME:-/root}/.gitconfig" "${HOME:-/root}/.gitconfig_save" + echo 1 + fi + + # Asan has huge memory requirements in its link steps. + # As of the new coverage methodology introduced in Envoy, that has grown memory requirements too. + # Hence we heavily reduce parallellism, to avoid being OOM killed. + if [[ "$1" == "coverage" ]]; then + NUM_CPUS=4 + elif [[ "$1" == "asan" ]]; then + NUM_CPUS=3 + else + NUM_CPUS=8 + fi +fi +echo "Running with ${NUM_CPUS} cpus" +BAZEL_BUILD_OPTIONS="${BAZEL_BUILD_OPTIONS} --jobs=${NUM_CPUS}" + export BAZEL_TEST_OPTIONS="${BAZEL_BUILD_OPTIONS} --test_env=HOME --test_env=PYTHONUSERBASE \ --test_env=UBSAN_OPTIONS=print_stacktrace=1 \ --cache_test_results=no --test_output=all ${BAZEL_EXTRA_TEST_OPTIONS}" -setup_clang_toolchain -export CLANG_FORMAT=clang-format - case "$1" in build) + setup_clang_toolchain do_build exit 0 ;; test) + setup_clang_toolchain do_test exit 0 ;; - test_with_valgrind) - do_test_with_valgrind + test_gcc) + setup_gcc_toolchain + do_test exit 0 ;; clang_tidy) - if [ -n "$CIRCLECI" ]; then - # Decrease parallelism to avoid running out of memory - NUM_CPUS=7 - fi + setup_clang_toolchain do_clang_tidy exit 0 ;; coverage) + setup_clang_toolchain do_coverage exit 0 ;; asan) + setup_clang_toolchain do_asan exit 0 ;; tsan) + setup_clang_toolchain do_tsan exit 0 ;; docker) + setup_clang_toolchain do_docker exit 0 ;; check_format) + setup_clang_toolchain do_check_format exit 0 ;; fix_format) + setup_clang_toolchain do_fix_format exit 0 ;; benchmark_with_own_binaries) + setup_clang_toolchain do_benchmark_with_own_binaries exit 0 ;; *) - echo "must be one of [build,test,clang_tidy,test_with_valgrind,coverage,asan,tsan,benchmark_with_own_binaries,docker,check_format,fix_format]" + echo "must be one of [build,test,clang_tidy,coverage,asan,tsan,benchmark_with_own_binaries,docker,check_format,fix_format,test_gcc]" exit 1 ;; esac diff --git a/requirements.txt b/requirements.txt index 4ce0db090..9823650f6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,3 +3,5 @@ pytest pytest-dependency pytest-xdist pyyaml +zipp +importlib_metadata \ No newline at end of file diff --git a/source/client/BUILD b/source/client/BUILD index e61a60beb..19c7ecda1 100644 --- a/source/client/BUILD +++ b/source/client/BUILD @@ -46,6 +46,7 @@ envoy_cc_library( "//include/nighthawk/common:base_includes", "//source/common:request_source_impl_lib", "//source/common:nighthawk_common_lib", + "@envoy//source/common/common:random_generator_lib_with_external_headers", "@envoy//source/common/access_log:access_log_manager_lib_with_external_headers", "@envoy//source/common/api:api_lib_with_external_headers", "@envoy//source/common/common:cleanup_lib_with_external_headers", diff --git a/source/client/benchmark_client_impl.h b/source/client/benchmark_client_impl.h index a40e90a19..e835f58f3 100644 --- a/source/client/benchmark_client_impl.h +++ b/source/client/benchmark_client_impl.h @@ -15,6 +15,7 @@ #include "nighthawk/common/statistic.h" #include "external/envoy/source/common/common/logger.h" +#include "external/envoy/source/common/common/random_generator.h" #include "external/envoy/source/common/http/http1/conn_pool.h" #include "external/envoy/source/common/http/http2/conn_pool.h" #include "external/envoy/source/common/runtime/runtime_impl.h" @@ -129,7 +130,7 @@ class BenchmarkClientHttpImpl : public BenchmarkClient, uint32_t max_active_requests_{UINT32_MAX}; uint32_t max_requests_per_connection_{UINT32_MAX}; Envoy::Event::TimerPtr timer_; - Envoy::Runtime::RandomGeneratorImpl generator_; + Envoy::Random::RandomGeneratorImpl generator_; uint64_t requests_completed_{}; uint64_t requests_initiated_{}; bool measure_latencies_{}; diff --git a/source/client/process_impl.h b/source/client/process_impl.h index 11d265fb8..ebb41990d 100644 --- a/source/client/process_impl.h +++ b/source/client/process_impl.h @@ -16,6 +16,7 @@ #include "external/envoy/source/common/access_log/access_log_manager_impl.h" #include "external/envoy/source/common/common/logger.h" +#include "external/envoy/source/common/common/random_generator.h" #include "external/envoy/source/common/event/real_time_system.h" #include "external/envoy/source/common/grpc/context_impl.h" #include "external/envoy/source/common/http/context_impl.h" @@ -133,7 +134,7 @@ class ProcessImpl : public Process, public Envoy::Logger::Loggable(); + const auto& stat = dynamic_cast(statistic); + hist_accumulate(combined->histogram_, &this->histogram_, /*cnt=*/1); + hist_accumulate(combined->histogram_, &stat.histogram_, /*cnt=*/1); + + combined->min_ = std::min(this->min(), stat.min()); + combined->max_ = std::max(this->max(), stat.max()); + combined->count_ = this->count() + stat.count(); + return combined; +} + +StatisticPtr CircllhistStatistic::createNewInstanceOfSameType() const { + return std::make_unique(); +} + +nighthawk::client::Statistic CircllhistStatistic::toProto(SerializationDomain domain) const { + nighthawk::client::Statistic proto = StatisticImpl::toProto(domain); + if (count() == 0) { + return proto; + } + + // List of quantiles is based on hdr_proto_json.gold. + const std::vector quantiles{0, 0.1, 0.2, 0.3, 0.4, 0.5, 0.55, 0.6, + 0.65, 0.7, 0.75, 0.775, 0.8, 0.825, 0.85, 0.875, + 0.90, 0.925, 0.95, 0.975, 0.99, 0.995, 0.999, 1}; + std::vector computed_quantiles(quantiles.size(), 0.0); + hist_approx_quantile(histogram_, quantiles.data(), quantiles.size(), computed_quantiles.data()); + for (size_t i = 0; i < quantiles.size(); i++) { + nighthawk::client::Percentile* percentile = proto.add_percentiles(); + if (domain == Statistic::SerializationDomain::DURATION) { + setDurationFromNanos(*percentile->mutable_duration(), + static_cast(computed_quantiles[i])); + } else { + percentile->set_raw_value(computed_quantiles[i]); + } + percentile->set_percentile(quantiles[i]); + percentile->set_count(hist_approx_count_below(histogram_, computed_quantiles[i])); + } + + return proto; +} + +SinkableStatistic::SinkableStatistic(Envoy::Stats::Scope& scope, absl::optional worker_id) + : Envoy::Stats::HistogramImplHelper(scope.symbolTable()), scope_(scope), worker_id_(worker_id) { +} + +SinkableStatistic::~SinkableStatistic() { + // We must explicitly free the StatName here in order to supply the + // SymbolTable reference. + MetricImpl::clear(scope_.symbolTable()); +} + +Envoy::Stats::Histogram::Unit SinkableStatistic::unit() const { + return Envoy::Stats::Histogram::Unit::Unspecified; +} + +Envoy::Stats::SymbolTable& SinkableStatistic::symbolTable() { return scope_.symbolTable(); } + +SinkableHdrStatistic::SinkableHdrStatistic(Envoy::Stats::Scope& scope, + absl::optional worker_id) + : SinkableStatistic(scope, worker_id) {} + +void SinkableHdrStatistic::recordValue(uint64_t value) { + addValue(value); + // Currently in Envoy Scope implementation, deliverHistogramToSinks() will flush the histogram + // value directly to stats Sinks. + scope_.deliverHistogramToSinks(*this, value); +} + +SinkableCircllhistStatistic::SinkableCircllhistStatistic(Envoy::Stats::Scope& scope, + absl::optional worker_id) + : SinkableStatistic(scope, worker_id) {} + +void SinkableCircllhistStatistic::recordValue(uint64_t value) { + addValue(value); + // Currently in Envoy Scope implementation, deliverHistogramToSinks() will flush the histogram + // value directly to stats Sinks. + scope_.deliverHistogramToSinks(*this, value); +} + +} // namespace Nighthawk diff --git a/source/common/statistic_impl.h b/source/common/statistic_impl.h index 981133b44..d7e1f240f 100644 --- a/source/common/statistic_impl.h +++ b/source/common/statistic_impl.h @@ -7,6 +7,7 @@ #include "external/dep_hdrhistogram_c/src/hdr_histogram.h" #include "external/envoy/source/common/common/logger.h" +#include "external/envoy/source/common/stats/histogram_impl.h" #include "common/frequency.h" @@ -150,4 +151,94 @@ class HdrStatistic : public StatisticImpl { struct hdr_histogram* histogram_; }; -} // namespace Nighthawk \ No newline at end of file +/** + * CircllhistStatistic uses Circllhist under the hood to compute statistics. + * Circllhist is used in the implementation of Envoy Histograms, compared to HdrHistogram it trades + * precision for fast performance in merge and insertion. For more info, please see + * https://github.com/circonus-labs/libcircllhist + */ +class CircllhistStatistic : public StatisticImpl { +public: + CircllhistStatistic(); + ~CircllhistStatistic() override; + + void addValue(uint64_t value) override; + double mean() const override; + double pvariance() const override; + double pstdev() const override; + StatisticPtr combine(const Statistic& statistic) const override; + // circllhist has low significant digit precision as a result of base 10 + // algorithm. + uint64_t significantDigits() const override { return 1; } + StatisticPtr createNewInstanceOfSameType() const override; + nighthawk::client::Statistic toProto(SerializationDomain domain) const override; + +private: + histogram_t* histogram_; +}; + +/** + * In order to be able to flush a histogram value to downstream Envoy stats Sinks, abstract class + * SinkableStatistic takes the Scope reference in the constructor and wraps the + * Envoy::Stats::HistogramHelper interface. Implementation of sinkable Nighthawk Statistic class + * will inherit from this class. + */ +class SinkableStatistic : public Envoy::Stats::HistogramImplHelper { +public: + // Calling HistogramImplHelper(SymbolTable& symbol_table) constructor to construct an empty + // MetricImpl. This is to bypass the complicated logic of setting up SymbolTable/StatName in + // Envoy. + SinkableStatistic(Envoy::Stats::Scope& scope, absl::optional worker_id); + ~SinkableStatistic() override; + + // Currently Envoy Histogram Unit supports {Unspecified, Bytes, Microseconds, Milliseconds}. By + // default, Nighthawk::Statistic uses nanosecond as the unit of latency histograms, so Unspecified + // is returned here to isolate Nighthawk Statistic from Envoy Histogram Unit. + Envoy::Stats::Histogram::Unit unit() const override; + Envoy::Stats::SymbolTable& symbolTable() override; + // Return the id of the worker where this statistic is defined. Per worker + // statistic should always set worker_id. Return absl::nullopt when the + // statistic is not defined per worker. + const absl::optional worker_id() { return worker_id_; } + +protected: + // This is used in child class for delivering the histogram data to sinks. + Envoy::Stats::Scope& scope_; + +private: + // worker_id can be used in downstream stats Sinks as the stats tag. + absl::optional worker_id_; +}; + +// Implementation of sinkable Nighthawk Statistic with HdrHistogram. +class SinkableHdrStatistic : public SinkableStatistic, public HdrStatistic { +public: + // The constructor takes the Scope reference which is used to flush a histogram value to + // downstream stats Sinks through deliverHistogramToSinks(). + SinkableHdrStatistic(Envoy::Stats::Scope& scope, absl::optional worker_id = absl::nullopt); + + // Envoy::Stats::Histogram + void recordValue(uint64_t value) override; + bool used() const override { return count() > 0; } + // Overriding name() to return Nighthawk::Statistic::id(). + std::string name() const override { return id(); } + std::string tagExtractedName() const override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } +}; + +// Implementation of sinkable Nighthawk Statistic with Circllhist Histogram. +class SinkableCircllhistStatistic : public SinkableStatistic, public CircllhistStatistic { +public: + // The constructor takes the Scope reference which is used to flush a histogram value to + // downstream stats Sinks through deliverHistogramToSinks(). + SinkableCircllhistStatistic(Envoy::Stats::Scope& scope, + absl::optional worker_id = absl::nullopt); + + // Envoy::Stats::Histogram + void recordValue(uint64_t value) override; + bool used() const override { return count() > 0; } + // Overriding name() to return Nighthawk::Statistic::id(). + std::string name() const override { return id(); } + std::string tagExtractedName() const override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } +}; + +} // namespace Nighthawk diff --git a/source/server/BUILD b/source/server/BUILD index 1b304eeb8..faf3288bf 100644 --- a/source/server/BUILD +++ b/source/server/BUILD @@ -8,16 +8,53 @@ licenses(["notice"]) # Apache 2 envoy_package() +envoy_cc_library( + name = "well_known_headers_lib", + hdrs = ["well_known_headers.h"], + repository = "@envoy", + deps = [ + "@envoy//source/common/http:headers_lib_with_external_headers", + "@envoy//source/common/singleton:const_singleton_with_external_headers", + ], +) + +envoy_cc_library( + name = "configuration_lib", + srcs = ["configuration.cc"], + hdrs = ["configuration.h"], + repository = "@envoy", + deps = [ + "//api/server:response_options_proto_cc_proto", + "@envoy//source/common/protobuf:message_validator_lib_with_external_headers", + "@envoy//source/common/protobuf:utility_lib_with_external_headers", + "@envoy//source/common/singleton:const_singleton_with_external_headers", + ], +) + envoy_cc_library( name = "http_test_server_filter_lib", srcs = ["http_test_server_filter.cc"], hdrs = ["http_test_server_filter.h"], repository = "@envoy", deps = [ + ":configuration_lib", + ":well_known_headers_lib", + "//api/server:response_options_proto_cc_proto", + "@envoy//source/exe:envoy_common_lib_with_external_headers", + ], +) + +envoy_cc_library( + name = "http_dynamic_delay_filter_lib", + srcs = ["http_dynamic_delay_filter.cc"], + hdrs = ["http_dynamic_delay_filter.h"], + repository = "@envoy", + deps = [ + ":configuration_lib", + ":well_known_headers_lib", "//api/server:response_options_proto_cc_proto", - "@envoy//source/common/protobuf:message_validator_lib_with_external_headers", - "@envoy//source/common/protobuf:utility_lib_with_external_headers", "@envoy//source/exe:envoy_common_lib_with_external_headers", + "@envoy//source/extensions/filters/http/fault:fault_filter_lib", ], ) @@ -30,3 +67,13 @@ envoy_cc_library( "@envoy//include/envoy/server:filter_config_interface_with_external_headers", ], ) + +envoy_cc_library( + name = "http_dynamic_delay_filter_config", + srcs = ["http_dynamic_delay_filter_config.cc"], + repository = "@envoy", + deps = [ + ":http_dynamic_delay_filter_lib", + "@envoy//include/envoy/server:filter_config_interface_with_external_headers", + ], +) diff --git a/source/server/README.md b/source/server/README.md index 85d2d23a0..e65b2768a 100644 --- a/source/server/README.md +++ b/source/server/README.md @@ -16,48 +16,46 @@ bazel build -c opt :nighthawk_test_server ## Configuring the test server - `test-server.yaml` sample content ```yaml static_resources: listeners: - # define an origin server on :10000 that always returns "lorem ipsum..." - - address: - socket_address: - address: 0.0.0.0 - port_value: 10000 - filter_chains: - - filters: - - name: envoy.http_connection_manager - config: - generate_request_id: false - codec_type: auto - stat_prefix: ingress_http - route_config: - name: local_route - virtual_hosts: - - name: service - domains: - - "*" - http_filters: - - name: envoy.fault - config: - max_active_faults: 100 - delay: - header_delay: {} - percentage: - numerator: 100 - - name: test-server # before envoy.router because order matters! - config: - response_body_size: 10 - response_headers: - - { header: { key: "foo", value: "bar"} } - - { header: { key: "foo", value: "bar2"}, append: true } - - { header: { key: "x-nh", value: "1"}} - - name: envoy.router - config: - dynamic_stats: false + # define an origin server on :10000 that always returns "lorem ipsum..." + - address: + socket_address: + address: 0.0.0.0 + port_value: 10000 + filter_chains: + - filters: + - name: envoy.http_connection_manager + config: + generate_request_id: false + codec_type: auto + stat_prefix: ingress_http + route_config: + name: local_route + virtual_hosts: + - name: service + domains: + - "*" + http_filters: + - name: dynamic-delay + config: + static_delay: 0.5s + - name: test-server # before envoy.router because order matters! + config: + response_body_size: 10 + response_headers: + - { header: { key: "foo", value: "bar" } } + - { + header: { key: "foo", value: "bar2" }, + append: true, + } + - { header: { key: "x-nh", value: "1" } } + - name: envoy.router + config: + dynamic_stats: false admin: access_log_path: /tmp/envoy.log address: @@ -68,21 +66,23 @@ admin: ## Response Options config -The ResponseOptions proto can be used in the test-server filter config or passed in `x-nighthawk-test-server-config`` -request header. +The [ResponseOptions proto](/api/server/response_options.proto) is shared by +the `Test Server` and `Dynamic Delay` filter extensions. Each filter will +interpret the parts that are relevant to it. This allows specifying what +a response should look like in a single message, which can be done at request +time via the optional `x-nighthawk-test-server-config` request-header. -The following parameters are available: +### Test Server -* `response_body_size` - number of 'a' characters repeated in the response body. -* `response_headers` - list of headers to add to response. If `append` is set to +- `response_body_size` - number of 'a' characters repeated in the response body. +- `response_headers` - list of headers to add to response. If `append` is set to `true`, then the header is appended. -* `echo_request_headers` - if set to `true`, then append the dump of request headers to the response +- `echo_request_headers` - if set to `true`, then append the dump of request headers to the response body. -The response options could be used to test and debug proxy or server configuration, for -example, to verify request headers that are added by intermediate proxy: +The response options above could be used to test and debug proxy or server configuration, for example, to verify request headers that are added by intermediate proxy: -``` +```bash $ curl -6 -v [::1]:8080/nighthawk * Trying ::1:8080... @@ -122,15 +122,28 @@ Request Headers: This example shows that intermediate proxy has added `x-forwarded-proto` and `x-forwarded-for` request headers. -## Running the test server +### Dynamic Delay + +The Dynamic Delay interprets the `oneof_delay_options` part in the [ResponseOptions proto](/api/server/response_options.proto). If specified, it can be used to: + +- Configure a static delay via `static_delay`. +- Configure a delay which linearly increase as the number of active requests grows, representing a simplified model of an overloaded server, via `concurrency_based_linear_delay`. +All delays have a millisecond-level granularity. + +At the time of writing this, there is a [known issue](https://github.com/envoyproxy/nighthawk/issues/392) with merging configuration provided via +request headers into the statically configured configuration. The current recommendation is to +use either static, or dynamic configuration (delivered per request header), but not both at the +same time. + +## Running the test server ``` # If you already have Envoy running, you might need to set --base-id to allow the test-server to start. ➜ /bazel-bin/nighthawk/source/server/server --config-path /path/to/test-server-server.yaml # Verify the test server with a curl command similar to: -➜ curl -H "x-envoy-fault-delay-request: 1000" -H "x-nighthawk-test-server-config: {response_body_size:20}" -vv 127.0.0.1:10000 +➜ curl -H "x-nighthawk-test-server-config: {response_body_size:20, static_delay: \"0s\"}" -vv 127.0.0.1:10000 ``` ```bash @@ -144,9 +157,7 @@ bazel-bin/nighthawk_test_server [--disable-extensions ] [--use-fake-symbol-table ] [--cpuset-threads] [--enable-mutex-tracing] -[--disable-hot-restart] -[--max-obj-name-len ] -[--max-stats ] [--mode +[--disable-hot-restart] [--mode ] [--parent-shutdown-time-s ] [--drain-strategy ] [--drain-time-s ] @@ -193,12 +204,6 @@ Enable mutex contention tracing functionality --disable-hot-restart Disable hot restart functionality ---max-obj-name-len -Deprecated and unused; please do not specify. - ---max-stats -Deprecated and unused; please do not specify. - --mode One of 'serve' (default; validate configs and then serve traffic normally) or 'validate' (validate configs and exit). diff --git a/source/server/configuration.cc b/source/server/configuration.cc new file mode 100644 index 000000000..8ddc92fb6 --- /dev/null +++ b/source/server/configuration.cc @@ -0,0 +1,45 @@ +#include "server/configuration.h" + +#include + +#include "external/envoy/source/common/protobuf/message_validator_impl.h" +#include "external/envoy/source/common/protobuf/utility.h" + +#include "api/server/response_options.pb.validate.h" + +#include "absl/strings/numbers.h" + +namespace Nighthawk { +namespace Server { +namespace Configuration { + +bool mergeJsonConfig(absl::string_view json, nighthawk::server::ResponseOptions& config, + std::string& error_message) { + error_message = ""; + try { + nighthawk::server::ResponseOptions json_config; + auto& validation_visitor = Envoy::ProtobufMessage::getStrictValidationVisitor(); + Envoy::MessageUtil::loadFromJson(std::string(json), json_config, validation_visitor); + config.MergeFrom(json_config); + Envoy::MessageUtil::validate(config, validation_visitor); + } catch (const Envoy::EnvoyException& exception) { + error_message = fmt::format("Error merging json config: {}", exception.what()); + } + return error_message == ""; +} + +void applyConfigToResponseHeaders(Envoy::Http::ResponseHeaderMap& response_headers, + nighthawk::server::ResponseOptions& response_options) { + for (const auto& header_value_option : response_options.response_headers()) { + const auto& header = header_value_option.header(); + auto lower_case_key = Envoy::Http::LowerCaseString(header.key()); + if (!header_value_option.append().value()) { + response_headers.remove(lower_case_key); + } + response_headers.addCopy(lower_case_key, header.value()); + } +} + +} // namespace Configuration +} // namespace Server +} // namespace Nighthawk diff --git a/source/server/configuration.h b/source/server/configuration.h new file mode 100644 index 000000000..ec1f77165 --- /dev/null +++ b/source/server/configuration.h @@ -0,0 +1,36 @@ +#pragma once + +#include + +#include "envoy/http/header_map.h" + +#include "api/server/response_options.pb.h" + +namespace Nighthawk { +namespace Server { +namespace Configuration { + +/** + * Merges a json string containing configuration into a ResponseOptions instance. + * + * @param json Json-formatted seralization of ResponseOptions to merge into the configuration. + * @param config The target that the json string should be merged into. + * @param error_message Set to an error message if one occurred, else set to an empty string. + * @return bool false if an error occurred. + */ +bool mergeJsonConfig(absl::string_view json, nighthawk::server::ResponseOptions& config, + std::string& error_message); + +/** + * Applies ResponseOptions onto a HeaderMap containing response headers. + * + * @param response_headers Response headers to transform to reflect the passed in response + * options. + * @param response_options Configuration specifying how to transform the header map. + */ +void applyConfigToResponseHeaders(Envoy::Http::ResponseHeaderMap& response_headers, + nighthawk::server::ResponseOptions& response_options); + +} // namespace Configuration +} // namespace Server +} // namespace Nighthawk diff --git a/source/server/http_dynamic_delay_filter.cc b/source/server/http_dynamic_delay_filter.cc new file mode 100644 index 000000000..542cedefa --- /dev/null +++ b/source/server/http_dynamic_delay_filter.cc @@ -0,0 +1,113 @@ +#include "server/http_dynamic_delay_filter.h" + +#include + +#include "envoy/server/filter_config.h" + +#include "server/configuration.h" +#include "server/well_known_headers.h" + +#include "absl/strings/str_cat.h" + +namespace Nighthawk { +namespace Server { + +HttpDynamicDelayDecoderFilterConfig::HttpDynamicDelayDecoderFilterConfig( + nighthawk::server::ResponseOptions proto_config, Envoy::Runtime::Loader& runtime, + const std::string& stats_prefix, Envoy::Stats::Scope& scope, Envoy::TimeSource& time_source) + : server_config_(std::move(proto_config)), runtime_(runtime), + stats_prefix_(absl::StrCat(stats_prefix, "dynamic-delay.")), scope_(scope), + time_source_(time_source) {} + +HttpDynamicDelayDecoderFilter::HttpDynamicDelayDecoderFilter( + HttpDynamicDelayDecoderFilterConfigSharedPtr config) + : Envoy::Extensions::HttpFilters::Fault::FaultFilter( + translateOurConfigIntoFaultFilterConfig(*config)), + config_(std::move(config)) { + config_->incrementFilterInstanceCount(); +} + +HttpDynamicDelayDecoderFilter::~HttpDynamicDelayDecoderFilter() { + RELEASE_ASSERT(destroyed_, "onDestroy() not called"); +} + +void HttpDynamicDelayDecoderFilter::onDestroy() { + destroyed_ = true; + config_->decrementFilterInstanceCount(); + Envoy::Extensions::HttpFilters::Fault::FaultFilter::onDestroy(); +} + +Envoy::Http::FilterHeadersStatus +HttpDynamicDelayDecoderFilter::decodeHeaders(Envoy::Http::RequestHeaderMap& headers, + bool end_stream) { + response_options_ = config_->server_config(); + std::string error_message; + if (!computeResponseOptions(headers, error_message)) { + decoder_callbacks_->sendLocalReply( + static_cast(500), + fmt::format("dynamic-delay didn't understand the request: {}", error_message), nullptr, + absl::nullopt, ""); + return Envoy::Http::FilterHeadersStatus::StopIteration; + } + const absl::optional delay_ms = + computeDelayMs(response_options_, config_->approximateFilterInstances()); + maybeRequestFaultFilterDelay(delay_ms, headers); + return Envoy::Extensions::HttpFilters::Fault::FaultFilter::decodeHeaders(headers, end_stream); +} + +bool HttpDynamicDelayDecoderFilter::computeResponseOptions( + const Envoy::Http::RequestHeaderMap& headers, std::string& error_message) { + response_options_ = config_->server_config(); + const auto* request_config_header = headers.get(TestServer::HeaderNames::get().TestServerConfig); + if (request_config_header) { + if (!Configuration::mergeJsonConfig(request_config_header->value().getStringView(), + response_options_, error_message)) { + return false; + } + } + return true; +} + +absl::optional HttpDynamicDelayDecoderFilter::computeDelayMs( + const nighthawk::server::ResponseOptions& response_options, const uint64_t concurrency) { + absl::optional delay_ms; + if (response_options.has_static_delay()) { + delay_ms = + Envoy::Protobuf::util::TimeUtil::DurationToMilliseconds(response_options.static_delay()); + } else if (response_options.has_concurrency_based_linear_delay()) { + const nighthawk::server::ConcurrencyBasedLinearDelay& concurrency_config = + response_options.concurrency_based_linear_delay(); + delay_ms = computeConcurrencyBasedLinearDelayMs(concurrency, concurrency_config.minimal_delay(), + concurrency_config.concurrency_delay_factor()); + } + return delay_ms; +} + +void HttpDynamicDelayDecoderFilter::maybeRequestFaultFilterDelay( + const absl::optional delay_ms, Envoy::Http::RequestHeaderMap& headers) { + if (delay_ms.has_value() && delay_ms > 0) { + // Emit header to communicate the delay we desire to the fault filter extension. + const Envoy::Http::LowerCaseString key("x-envoy-fault-delay-request"); + headers.setCopy(key, absl::StrCat(*delay_ms)); + } +} + +Envoy::Extensions::HttpFilters::Fault::FaultFilterConfigSharedPtr +HttpDynamicDelayDecoderFilter::translateOurConfigIntoFaultFilterConfig( + HttpDynamicDelayDecoderFilterConfig& config) { + envoy::extensions::filters::http::fault::v3::HTTPFault fault_config; + fault_config.mutable_max_active_faults()->set_value(UINT32_MAX); + fault_config.mutable_delay()->mutable_percentage()->set_numerator(100); + fault_config.mutable_delay()->mutable_header_delay(); + return std::make_shared( + fault_config, config.runtime(), config.stats_prefix(), config.scope(), config.time_source()); +} + +void HttpDynamicDelayDecoderFilter::setDecoderFilterCallbacks( + Envoy::Http::StreamDecoderFilterCallbacks& callbacks) { + decoder_callbacks_ = &callbacks; + Envoy::Extensions::HttpFilters::Fault::FaultFilter::setDecoderFilterCallbacks(callbacks); +} + +} // namespace Server +} // namespace Nighthawk diff --git a/source/server/http_dynamic_delay_filter.h b/source/server/http_dynamic_delay_filter.h new file mode 100644 index 000000000..031ec9499 --- /dev/null +++ b/source/server/http_dynamic_delay_filter.h @@ -0,0 +1,187 @@ +#pragma once + +#include +#include + +#include "envoy/server/filter_config.h" + +#include "api/server/response_options.pb.h" + +#include "extensions/filters/http/fault/fault_filter.h" + +namespace Nighthawk { +namespace Server { + +/** + * Filter configuration container class for the dynamic delay extension. + * Instances of this class will be shared accross instances of HttpDynamicDelayDecoderFilter. + * The methods for getting and manipulating (global) active filter instance counts are thread safe. + */ +class HttpDynamicDelayDecoderFilterConfig { + +public: + /** + * Constructs a new HttpDynamicDelayDecoderFilterConfig instance. + * + * @param proto_config The proto configuration of the filter. Will be tranlated internally into + * the right configuration for the base class structure (the failt filter and config). + * @param runtime Envoy runtime to be used by the filter. + * @param stats_prefix Prefix to use by the filter when it names statistics. E.g. + * dynamic-delay.fault.delays_injected: 1 + * @param scope Statistics scope to be used by the filter. + * @param time_source Time source to be used by the filter. + */ + HttpDynamicDelayDecoderFilterConfig(nighthawk::server::ResponseOptions proto_config, + Envoy::Runtime::Loader& runtime, + const std::string& stats_prefix, Envoy::Stats::Scope& scope, + Envoy::TimeSource& time_source); + + /** + * @return const nighthawk::server::ResponseOptions& read-only reference to the proto config + * object. + */ + const nighthawk::server::ResponseOptions& server_config() const { return server_config_; } + + /** + * Increments the number of globally active filter instances. + */ + void incrementFilterInstanceCount() { instances()++; } + + /** + * Decrements the number of globally active filter instances. + */ + void decrementFilterInstanceCount() { instances()--; } + + /** + * @return uint64_t the approximate number of globally active HttpDynamicDelayDecoderFilter + * instances. Approximate, because by the time the value is consumed it might have changed. + */ + uint64_t approximateFilterInstances() const { return instances(); } + + /** + * @return Envoy::Runtime::Loader& to be used by filter instantiations associated to this. + */ + Envoy::Runtime::Loader& runtime() { return runtime_; } + + /** + * @return Envoy::Stats::Scope& to be used by filter instantiations associated to this. + */ + Envoy::Stats::Scope& scope() { return scope_; } + + /** + * @return Envoy::TimeSource& to be used by filter instantiations associated to this. + */ + Envoy::TimeSource& time_source() { return time_source_; } + + /** + * @return std::string to be used by filter instantiations associated to this. + */ + std::string stats_prefix() { return stats_prefix_; } + +private: + const nighthawk::server::ResponseOptions server_config_; + static std::atomic& instances() { + // We lazy-init the atomic to avoid static initialization / a fiasco. + MUTABLE_CONSTRUCT_ON_FIRST_USE(std::atomic, 0); // NOLINT + } + + Envoy::Runtime::Loader& runtime_; + const std::string stats_prefix_; + Envoy::Stats::Scope& scope_; + Envoy::TimeSource& time_source_; +}; + +using HttpDynamicDelayDecoderFilterConfigSharedPtr = + std::shared_ptr; + +/** + * Extension that controls the fault filter extension by supplying it with a request + * header that triggers it to induce a delay under the hood. + * In the future, we may look into injecting the fault filter ourselves with the right + * configuration, either directly/verbatim, or by including a derivation of it in + * this code base, thereby making it all transparant to the user / eliminating the need + * to configure the fault filter and make NH take full ownership at the feature level. + */ +class HttpDynamicDelayDecoderFilter : public Envoy::Extensions::HttpFilters::Fault::FaultFilter { +public: + HttpDynamicDelayDecoderFilter(HttpDynamicDelayDecoderFilterConfigSharedPtr); + ~HttpDynamicDelayDecoderFilter() override; + + // Http::StreamFilterBase + void onDestroy() override; + + // Http::StreamDecoderFilter + Envoy::Http::FilterHeadersStatus decodeHeaders(Envoy::Http::RequestHeaderMap&, bool) override; + void setDecoderFilterCallbacks(Envoy::Http::StreamDecoderFilterCallbacks&) override; + + /** + * Compute the response options based on the static configuration and optional configuration + * provided via the request headers. After a successfull call the response_options_ field will + * have been modified to reflect request-level configuration. + * + * @param request_headers The request headers set to inspect for configuration. + * @param error_message Set to an error message if the request-level configuration couldn't be + * interpreted. + * @return true iff the configuration was successfully computed. + */ + bool computeResponseOptions(const Envoy::Http::RequestHeaderMap& request_headers, + std::string& error_message); + + /** + * Compute the concurrency based linear delay in milliseconds. + * + * @param concurrency indicating the number of concurrently active requests. + * @param minimal_delay gets unconditionally included in the return value. + * @param delay_factor added for each increase in the number of active requests. + * @return int64_t the computed delay in milliseconds. + */ + static int64_t + computeConcurrencyBasedLinearDelayMs(const uint64_t concurrency, + const Envoy::ProtobufWkt::Duration& minimal_delay, + const Envoy::ProtobufWkt::Duration& delay_factor) { + return std::round(Envoy::Protobuf::util::TimeUtil::DurationToNanoseconds( + minimal_delay + (concurrency * delay_factor)) / + 1e6); + } + + /** + * Compute the delay in milliseconds, based on provided response options and number of active + * requests. + * + * @param response_options Response options configuration. + * @param concurrency The number of concurrenct active requests. + * @return absl::optional The computed delay in milliseconds, if any. + */ + static absl::optional + computeDelayMs(const nighthawk::server::ResponseOptions& response_options, + const uint64_t concurrency); + + /** + * Communicate to the fault filter, which should be running after this filter, that a delay should + * be inserted. The request is only made when the passed delay is set to a value > 0. + * + * @param delay_ms The delay in milliseconds that should be propagated, if any. When not set or <= + * 0, the call will be a no-op. + * @param request_headers The request headers that will be modified to instruct the fault filter. + */ + static void maybeRequestFaultFilterDelay(const absl::optional delay_ms, + Envoy::Http::RequestHeaderMap& request_headers); + + /** + * Translates our options into a configuration of the fault filter base class where needed. + * + * @param config Dynamic delay configuration. + * @return Envoy::Extensions::HttpFilters::Fault::FaultFilterConfigSharedPtr + */ + static Envoy::Extensions::HttpFilters::Fault::FaultFilterConfigSharedPtr + translateOurConfigIntoFaultFilterConfig(HttpDynamicDelayDecoderFilterConfig& config); + +private: + const HttpDynamicDelayDecoderFilterConfigSharedPtr config_; + Envoy::Http::StreamDecoderFilterCallbacks* decoder_callbacks_; + nighthawk::server::ResponseOptions response_options_; + bool destroyed_{false}; +}; + +} // namespace Server +} // namespace Nighthawk diff --git a/source/server/http_dynamic_delay_filter_config.cc b/source/server/http_dynamic_delay_filter_config.cc new file mode 100644 index 000000000..336a5da7b --- /dev/null +++ b/source/server/http_dynamic_delay_filter_config.cc @@ -0,0 +1,60 @@ +#include + +#include "envoy/registry/registry.h" + +#include "external/envoy/source/common/protobuf/message_validator_impl.h" + +#include "api/server/response_options.pb.h" +#include "api/server/response_options.pb.validate.h" + +#include "server/http_dynamic_delay_filter.h" + +namespace Nighthawk { +namespace Server { +namespace Configuration { +namespace { + +class HttpDynamicDelayDecoderFilterConfigFactory + : public Envoy::Server::Configuration::NamedHttpFilterConfigFactory { +public: + Envoy::Http::FilterFactoryCb + createFilterFactoryFromProto(const Envoy::Protobuf::Message& proto_config, const std::string&, + Envoy::Server::Configuration::FactoryContext& context) override { + + auto& validation_visitor = Envoy::ProtobufMessage::getStrictValidationVisitor(); + return createFilter( + Envoy::MessageUtil::downcastAndValidate( + proto_config, validation_visitor), + context); + } + + Envoy::ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return Envoy::ProtobufTypes::MessagePtr{new nighthawk::server::ResponseOptions()}; + } + + std::string name() const override { return "dynamic-delay"; } + +private: + Envoy::Http::FilterFactoryCb createFilter(const nighthawk::server::ResponseOptions& proto_config, + Envoy::Server::Configuration::FactoryContext& context) { + Nighthawk::Server::HttpDynamicDelayDecoderFilterConfigSharedPtr config = + std::make_shared( + Nighthawk::Server::HttpDynamicDelayDecoderFilterConfig( + proto_config, context.runtime(), "" /*stats_prefix*/, context.scope(), + context.timeSource())); + + return [config](Envoy::Http::FilterChainFactoryCallbacks& callbacks) -> void { + auto* filter = new Nighthawk::Server::HttpDynamicDelayDecoderFilter(config); + callbacks.addStreamDecoderFilter(Envoy::Http::StreamDecoderFilterSharedPtr{filter}); + }; + } +}; + +} // namespace + +static Envoy::Registry::RegisterFactory + register_; +} // namespace Configuration +} // namespace Server +} // namespace Nighthawk diff --git a/source/server/http_test_server_filter.cc b/source/server/http_test_server_filter.cc index b43c413ed..cf01105cf 100644 --- a/source/server/http_test_server_filter.cc +++ b/source/server/http_test_server_filter.cc @@ -4,10 +4,8 @@ #include "envoy/server/filter_config.h" -#include "external/envoy/source/common/protobuf/message_validator_impl.h" -#include "external/envoy/source/common/protobuf/utility.h" - -#include "api/server/response_options.pb.validate.h" +#include "server/configuration.h" +#include "server/well_known_headers.h" #include "absl/strings/numbers.h" @@ -24,37 +22,8 @@ HttpTestServerDecoderFilter::HttpTestServerDecoderFilter( void HttpTestServerDecoderFilter::onDestroy() {} -bool HttpTestServerDecoderFilter::mergeJsonConfig(absl::string_view json, - nighthawk::server::ResponseOptions& config, - absl::optional& error_message) { - error_message = absl::nullopt; - try { - nighthawk::server::ResponseOptions json_config; - auto& validation_visitor = Envoy::ProtobufMessage::getStrictValidationVisitor(); - Envoy::MessageUtil::loadFromJson(std::string(json), json_config, validation_visitor); - config.MergeFrom(json_config); - Envoy::MessageUtil::validate(config, validation_visitor); - } catch (const Envoy::EnvoyException& exception) { - error_message.emplace(fmt::format("Error merging json config: {}", exception.what())); - } - return error_message == absl::nullopt; -} - -void HttpTestServerDecoderFilter::applyConfigToResponseHeaders( - Envoy::Http::ResponseHeaderMap& response_headers, - nighthawk::server::ResponseOptions& response_options) { - for (const auto& header_value_option : response_options.response_headers()) { - const auto& header = header_value_option.header(); - auto lower_case_key = Envoy::Http::LowerCaseString(header.key()); - if (!header_value_option.append().value()) { - response_headers.remove(lower_case_key); - } - response_headers.addCopy(lower_case_key, header.value()); - } -} - void HttpTestServerDecoderFilter::sendReply() { - if (error_message_ == absl::nullopt) { + if (!json_merge_error_) { std::string response_body(base_config_.response_body_size(), 'a'); if (request_headers_dump_.has_value()) { response_body += *request_headers_dump_; @@ -62,13 +31,13 @@ void HttpTestServerDecoderFilter::sendReply() { decoder_callbacks_->sendLocalReply( static_cast(200), response_body, [this](Envoy::Http::ResponseHeaderMap& direct_response_headers) { - applyConfigToResponseHeaders(direct_response_headers, base_config_); + Configuration::applyConfigToResponseHeaders(direct_response_headers, base_config_); }, absl::nullopt, ""); } else { decoder_callbacks_->sendLocalReply( static_cast(500), - fmt::format("test-server didn't understand the request: {}", *error_message_), nullptr, + fmt::format("test-server didn't understand the request: {}", error_message_), nullptr, absl::nullopt, ""); } } @@ -80,7 +49,8 @@ HttpTestServerDecoderFilter::decodeHeaders(Envoy::Http::RequestHeaderMap& header base_config_ = config_->server_config(); const auto* request_config_header = headers.get(TestServer::HeaderNames::get().TestServerConfig); if (request_config_header) { - mergeJsonConfig(request_config_header->value().getStringView(), base_config_, error_message_); + json_merge_error_ = !Configuration::mergeJsonConfig( + request_config_header->value().getStringView(), base_config_, error_message_); } if (base_config_.echo_request_headers()) { std::stringstream headers_dump; diff --git a/source/server/http_test_server_filter.h b/source/server/http_test_server_filter.h index f7ba094a2..6f8d2ace1 100644 --- a/source/server/http_test_server_filter.h +++ b/source/server/http_test_server_filter.h @@ -9,17 +9,6 @@ namespace Nighthawk { namespace Server { -namespace TestServer { - -class HeaderNameValues { -public: - const Envoy::Http::LowerCaseString TestServerConfig{"x-nighthawk-test-server-config"}; -}; - -using HeaderNames = Envoy::ConstSingleton; - -} // namespace TestServer - // Basically this is left in as a placeholder for further configuration. class HttpTestServerDecoderFilterConfig { public: @@ -46,33 +35,13 @@ class HttpTestServerDecoderFilter : public Envoy::Http::StreamDecoderFilter { Envoy::Http::FilterTrailersStatus decodeTrailers(Envoy::Http::RequestTrailerMap&) override; void setDecoderFilterCallbacks(Envoy::Http::StreamDecoderFilterCallbacks&) override; - /** - * Merges a json string containing configuration into a ResponseOptions instance. - * - * @param json Json-formatted seralization of ResponseOptions to merge into the configuration. - * @param config The target that the json string should be merged into. - * @param error_message Will contain an error message iff an error occurred. - * @return bool false iff an error occurred. - */ - bool mergeJsonConfig(absl::string_view json, nighthawk::server::ResponseOptions& config, - absl::optional& error_message); - - /** - * Applies ResponseOptions onto a HeaderMap containing response headers. - * - * @param response_headers Response headers to transform to reflect the passed in response - * options. - * @param response_options Configuration specifying how to transform the header map. - */ - void applyConfigToResponseHeaders(Envoy::Http::ResponseHeaderMap& response_headers, - nighthawk::server::ResponseOptions& response_options); - private: void sendReply(); const HttpTestServerDecoderFilterConfigSharedPtr config_; Envoy::Http::StreamDecoderFilterCallbacks* decoder_callbacks_; nighthawk::server::ResponseOptions base_config_; - absl::optional error_message_; + bool json_merge_error_{false}; + std::string error_message_; absl::optional request_headers_dump_; }; diff --git a/source/server/well_known_headers.h b/source/server/well_known_headers.h new file mode 100644 index 000000000..e64eb6c82 --- /dev/null +++ b/source/server/well_known_headers.h @@ -0,0 +1,22 @@ +#pragma once + +#include + +#include "envoy/http/header_map.h" + +#include "external/envoy/source/common/singleton/const_singleton.h" + +namespace Nighthawk { +namespace Server { +namespace TestServer { + +class HeaderNameValues { +public: + const Envoy::Http::LowerCaseString TestServerConfig{"x-nighthawk-test-server-config"}; +}; + +using HeaderNames = Envoy::ConstSingleton; + +} // namespace TestServer +} // namespace Server +} // namespace Nighthawk diff --git a/test/BUILD b/test/BUILD index 868394027..ac7cdcacc 100644 --- a/test/BUILD +++ b/test/BUILD @@ -195,13 +195,17 @@ envoy_cc_test( envoy_cc_test( name = "statistic_test", srcs = ["statistic_test.cc"], - data = ["test_data/hdr_proto_json.gold"], + data = [ + "test_data/circllhist_proto_json.gold", + "test_data/hdr_proto_json.gold", + ], repository = "@envoy", deps = [ "//source/common:nighthawk_common_lib", "//test/test_common:environment_lib", "@envoy//source/common/protobuf:utility_lib_with_external_headers", "@envoy//source/common/stats:isolated_store_lib_with_external_headers", + "@envoy//test/mocks/stats:stats_mocks", ], ) @@ -238,6 +242,7 @@ envoy_cc_test( repository = "@envoy", deps = [ "//source/common:nighthawk_common_lib", + "@envoy//source/common/common:random_generator_lib_with_external_headers", "@envoy//source/common/stats:isolated_store_lib_with_external_headers", "@envoy//test/mocks/local_info:local_info_mocks", "@envoy//test/mocks/protobuf:protobuf_mocks", diff --git a/test/benchmark_http_client_test.cc b/test/benchmark_http_client_test.cc index 0539f487b..f6d67efe2 100644 --- a/test/benchmark_http_client_test.cc +++ b/test/benchmark_http_client_test.cc @@ -1,5 +1,6 @@ #include +#include "external/envoy/source/common/common/random_generator.h" #include "external/envoy/source/common/http/header_map_impl.h" #include "external/envoy/source/common/network/utility.h" #include "external/envoy/source/common/runtime/runtime_impl.h" @@ -146,7 +147,7 @@ class BenchmarkClientHttpTest : public Test { Envoy::Stats::IsolatedStoreImpl store_; Envoy::Api::ApiPtr api_; Envoy::Event::DispatcherPtr dispatcher_; - Envoy::Runtime::RandomGeneratorImpl generator_; + Envoy::Random::RandomGeneratorImpl generator_; NiceMock tls_; NiceMock runtime_; std::unique_ptr client_; diff --git a/test/client_test.cc b/test/client_test.cc index 0840c2c3c..6cde01f27 100644 --- a/test/client_test.cc +++ b/test/client_test.cc @@ -26,6 +26,7 @@ class ClientTest : public testing::Test {}; TEST_F(ClientTest, NormalRun) { Main program(Nighthawk::Client::TestUtility::createOptionsImpl( "foo --duration 1 --rps 10 http://localhost:63657/")); + EXPECT_FALSE(program.run()); } diff --git a/test/client_worker_test.cc b/test/client_worker_test.cc index 4a7b59517..c2a7a9dae 100644 --- a/test/client_worker_test.cc +++ b/test/client_worker_test.cc @@ -3,6 +3,7 @@ #include "envoy/upstream/cluster_manager.h" +#include "external/envoy/source/common/common/random_generator.h" #include "external/envoy/source/common/runtime/runtime_impl.h" #include "external/envoy/source/common/stats/isolated_store_impl.h" #include "external/envoy/test/mocks/local_info/mocks.h" @@ -90,7 +91,7 @@ class ClientWorkerTest : public Test { MockBenchmarkClient* benchmark_client_; MockSequencer* sequencer_; MockRequestSource* request_generator_; - Envoy::Runtime::RandomGeneratorImpl rand_; + Envoy::Random::RandomGeneratorImpl rand_; NiceMock dispatcher_; std::unique_ptr loader_; NiceMock local_info_; diff --git a/test/integration/configurations/nighthawk_http_origin.yaml b/test/integration/configurations/nighthawk_http_origin.yaml index cf39a6ffc..08247b781 100644 --- a/test/integration/configurations/nighthawk_http_origin.yaml +++ b/test/integration/configurations/nighthawk_http_origin.yaml @@ -23,6 +23,7 @@ static_resources: domains: - "*" http_filters: + - name: dynamic-delay - name: envoy.fault config: max_active_faults: 100 diff --git a/test/run_nighthawk_bazel_coverage.sh b/test/run_nighthawk_bazel_coverage.sh index 8b48f4eec..0512e3b23 100755 --- a/test/run_nighthawk_bazel_coverage.sh +++ b/test/run_nighthawk_bazel_coverage.sh @@ -43,7 +43,7 @@ COVERAGE_VALUE=${COVERAGE_VALUE%?} if [ "$VALIDATE_COVERAGE" == "true" ] then - COVERAGE_THRESHOLD=98.6 + COVERAGE_THRESHOLD=98.4 COVERAGE_FAILED=$(echo "${COVERAGE_VALUE}<${COVERAGE_THRESHOLD}" | bc) if test ${COVERAGE_FAILED} -eq 1; then echo Code coverage ${COVERAGE_VALUE} is lower than limit of ${COVERAGE_THRESHOLD} @@ -52,4 +52,4 @@ then echo Code coverage ${COVERAGE_VALUE} is good and higher than limit of ${COVERAGE_THRESHOLD} fi fi -echo "HTML coverage report is in ${COVERAGE_DIR}/index.html" \ No newline at end of file +echo "HTML coverage report is in ${COVERAGE_DIR}/index.html" diff --git a/test/server/BUILD b/test/server/BUILD index e1cec75f4..23723c3ac 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -19,3 +19,14 @@ envoy_cc_test( "@envoy//test/integration:http_integration_lib", ], ) + +envoy_cc_test( + name = "http_dynamic_delay_filter_integration_test", + srcs = ["http_dynamic_delay_filter_integration_test.cc"], + repository = "@envoy", + deps = [ + "//source/server:http_dynamic_delay_filter_config", + "@envoy//source/common/api:api_lib_with_external_headers", + "@envoy//test/integration:http_integration_lib", + ], +) diff --git a/test/server/http_dynamic_delay_filter_integration_test.cc b/test/server/http_dynamic_delay_filter_integration_test.cc new file mode 100644 index 000000000..c4a38e429 --- /dev/null +++ b/test/server/http_dynamic_delay_filter_integration_test.cc @@ -0,0 +1,184 @@ +#include + +#include "external/envoy/test/integration/http_integration.h" + +#include "api/server/response_options.pb.h" + +#include "server/configuration.h" +#include "server/http_dynamic_delay_filter.h" + +#include "gtest/gtest.h" + +namespace Nighthawk { + +/** + * Support class for testing the dynamic delay filter. We rely on the fault filter for + * inducing the actual delay, so this aims to prove that: + * - The computations are correct. + * - Static/file-based configuration is handled as expected. + * - Request level configuration is handled as expected. + * - Failure modes work. + * - TODO(#393): An end to end test which proves that the interaction between this filter + * and the fault filter work as expected. + */ +class HttpDynamicDelayIntegrationTest + : public Envoy::HttpIntegrationTest, + public testing::TestWithParam { +protected: + HttpDynamicDelayIntegrationTest() + : HttpIntegrationTest(Envoy::Http::CodecClient::Type::HTTP1, GetParam()), + request_headers_({{":method", "GET"}, {":path", "/"}, {":authority", "host"}}), + delay_header_string_(Envoy::Http::LowerCaseString("x-envoy-fault-delay-request")) {} + + // We don't override SetUp(): tests in this file will call setup() instead to avoid having to + // create a fixture per filter configuration. + void setup(const std::string& config) { + config_helper_.addFilter(config); + HttpIntegrationTest::initialize(); + } + + // Fetches a response with request-level configuration set in the request header. + Envoy::IntegrationStreamDecoderPtr getResponse(absl::string_view request_level_config, + bool setup_for_upstream_request = true) { + const Envoy::Http::LowerCaseString key("x-nighthawk-test-server-config"); + Envoy::Http::TestRequestHeaderMapImpl request_headers = request_headers_; + request_headers.setCopy(key, request_level_config); + return getResponse(request_headers, setup_for_upstream_request); + } + + // Fetches a response with the default request headers, expecting the fake upstream to supply + // the response. + Envoy::IntegrationStreamDecoderPtr getResponse() { return getResponse(request_headers_); } + + // Fetches a response using the provided request headers. When setup_for_upstream_request + // is true, the expectation will be that an upstream request will be needed to provide a + // response. If it is set to false, the extension is expected to supply the response, and + // no upstream request ought to occur. + Envoy::IntegrationStreamDecoderPtr + getResponse(const Envoy::Http::TestRequestHeaderMapImpl& request_headers, + bool setup_for_upstream_request = true) { + cleanupUpstreamAndDownstream(); + codec_client_ = makeHttpConnection(lookupPort("http")); + Envoy::IntegrationStreamDecoderPtr response; + if (setup_for_upstream_request) { + response = sendRequestAndWaitForResponse(request_headers, 0, default_response_headers_, 0); + } else { + response = codec_client_->makeHeaderOnlyRequest(request_headers); + response->waitForEndStream(); + } + return response; + } + + const Envoy::Http::TestRequestHeaderMapImpl request_headers_; + const Envoy::Http::LowerCaseString delay_header_string_; +}; + +INSTANTIATE_TEST_SUITE_P(IpVersions, HttpDynamicDelayIntegrationTest, + testing::ValuesIn(Envoy::TestEnvironment::getIpVersionsForTest())); + +// Verify expectations with an empty dynamic-delay configuration. +TEST_P(HttpDynamicDelayIntegrationTest, NoStaticConfiguration) { + setup(R"( +name: dynamic-delay +typed_config: + "@type": type.googleapis.com/nighthawk.server.ResponseOptions +)"); + // Don't send any config request header + getResponse(); + EXPECT_EQ(upstream_request_->headers().get(delay_header_string_), nullptr); + // Send a config request header with an empty / default config. Should be a no-op. + getResponse("{}"); + EXPECT_EQ(upstream_request_->headers().get(delay_header_string_), nullptr); + // Send a config request header, this should become effective. + getResponse("{static_delay: \"1.6s\"}"); + EXPECT_EQ(upstream_request_->headers().get(delay_header_string_)->value().getStringView(), + "1600"); + + // Send a malformed config request header. This ought to shortcut and directly reply, + // hence we don't expect an upstream request. + auto response = getResponse("bad_json", false); + EXPECT_EQ(Envoy::Http::Utility::getResponseStatus(response->headers()), 500); + EXPECT_EQ( + response->body(), + "dynamic-delay didn't understand the request: Error merging json config: Unable to parse " + "JSON as proto (INVALID_ARGUMENT:Unexpected token.\nbad_json\n^): bad_json"); + // Send an empty config header, which ought to trigger failure mode as well. + response = getResponse("", false); + EXPECT_EQ(Envoy::Http::Utility::getResponseStatus(response->headers()), 500); + EXPECT_EQ( + response->body(), + "dynamic-delay didn't understand the request: Error merging json config: Unable to " + "parse JSON as proto (INVALID_ARGUMENT:Unexpected end of string. Expected a value.\n\n^): "); +} + +// Verify expectations with static/file-based static_delay configuration. +TEST_P(HttpDynamicDelayIntegrationTest, StaticConfigurationStaticDelay) { + setup(R"EOF( +name: dynamic-delay +typed_config: + "@type": type.googleapis.com/nighthawk.server.ResponseOptions + static_delay: 1.33s +)EOF"); + getResponse(); + EXPECT_EQ(upstream_request_->headers().get(delay_header_string_)->value().getStringView(), + "1330"); + getResponse("{}"); + EXPECT_EQ(upstream_request_->headers().get(delay_header_string_)->value().getStringView(), + "1330"); + getResponse("{static_delay: \"0.2s\"}"); + // TODO(#392): This fails, because the duration is a two-field message: it would make here to see + // both the number of seconds and nanoseconds to be overridden. + // However, the seconds part is set to '0', which equates to the default of the underlying int + // type, and the fact that we are using proto3, which doesn't merge default values. + // Hence the following expectation will fail, as it yields 1200 instead of the expected 200. + // EXPECT_EQ(upstream_request_->headers().get(delay_header_string_)->value().getStringView(), + // "200"); + getResponse("{static_delay: \"2.2s\"}"); + EXPECT_EQ(upstream_request_->headers().get(delay_header_string_)->value().getStringView(), + "2200"); +} + +// Verify expectations with static/file-based concurrency_based_linear_delay configuration. +TEST_P(HttpDynamicDelayIntegrationTest, StaticConfigurationConcurrentDelay) { + setup(R"EOF( +name: dynamic-delay +typed_config: + "@type": type.googleapis.com/nighthawk.server.ResponseOptions + concurrency_based_linear_delay: + minimal_delay: 0.05s + concurrency_delay_factor: 0.01s +)EOF"); + getResponse(); + EXPECT_EQ(upstream_request_->headers().get(delay_header_string_)->value().getStringView(), "60"); +} + +class ComputeTest : public testing::Test { +public: + int64_t compute(uint64_t concurrency, uint64_t minimal_delay_seconds, + uint64_t minimal_delay_nanos, uint64_t delay_factor_seconds, + uint64_t delay_factor_nanos) { + Envoy::ProtobufWkt::Duration minimal_delay; + Envoy::ProtobufWkt::Duration delay_factor; + minimal_delay.set_seconds(minimal_delay_seconds); + minimal_delay.set_nanos(minimal_delay_nanos); + delay_factor.set_seconds(delay_factor_seconds); + delay_factor.set_nanos(delay_factor_nanos); + return Server::HttpDynamicDelayDecoderFilter::computeConcurrencyBasedLinearDelayMs( + concurrency, minimal_delay, delay_factor); + } +}; + +// Test that the delay looks as expected with various parameterizations. +TEST_F(ComputeTest, ComputeConcurrencyBasedLinearDelayMs) { + EXPECT_EQ(compute(1, 1, 0, 0, 0), 1000); + EXPECT_EQ(compute(2, 1, 0, 0, 0), 1000); + EXPECT_EQ(compute(1, 2, 0, 0, 0), 2000); + EXPECT_EQ(compute(2, 2, 0, 0, 0), 2000); + EXPECT_EQ(compute(1, 0, 500000, 0, 500000), 1); + EXPECT_EQ(compute(2, 0, 500000, 0, 500000), 2); + EXPECT_EQ(compute(3, 0, 500000, 0, 500000), 2); + EXPECT_EQ(compute(4, 0, 500000, 0, 500000), 3); + EXPECT_EQ(compute(4, 1, 500000, 1, 500000), 5003); +} + +} // namespace Nighthawk diff --git a/test/server/http_test_server_filter_integration_test.cc b/test/server/http_test_server_filter_integration_test.cc index 1946f3abc..702b77c3c 100644 --- a/test/server/http_test_server_filter_integration_test.cc +++ b/test/server/http_test_server_filter_integration_test.cc @@ -7,7 +7,9 @@ #include "api/server/response_options.pb.h" #include "api/server/response_options.pb.validate.h" +#include "server/configuration.h" #include "server/http_test_server_filter.h" +#include "server/well_known_headers.h" #include "gtest/gtest.h" @@ -310,7 +312,7 @@ TEST_F(HttpTestServerDecoderFilterTest, HeaderMerge) { Server::HttpTestServerDecoderFilterConfigSharedPtr config = std::make_shared(initial_options); Server::HttpTestServerDecoderFilter f(config); - absl::optional error_message; + std::string error_message; nighthawk::server::ResponseOptions options = config->server_config(); EXPECT_EQ(1, options.response_headers_size()); @@ -320,40 +322,40 @@ TEST_F(HttpTestServerDecoderFilterTest, HeaderMerge) { EXPECT_EQ(false, options.response_headers(0).append().value()); Envoy::Http::TestResponseHeaderMapImpl header_map{{":status", "200"}, {"foo", "bar"}}; - f.applyConfigToResponseHeaders(header_map, options); + Server::Configuration::applyConfigToResponseHeaders(header_map, options); EXPECT_TRUE(Envoy::TestUtility::headerMapEqualIgnoreOrder( header_map, Envoy::Http::TestResponseHeaderMapImpl{{":status", "200"}, {"foo", "bar1"}})); - EXPECT_TRUE(f.mergeJsonConfig( + EXPECT_TRUE(Server::Configuration::mergeJsonConfig( R"({response_headers: [ { header: { key: "foo", value: "bar2"}, append: false } ]})", options, error_message)); - EXPECT_EQ(absl::nullopt, error_message); + EXPECT_EQ("", error_message); EXPECT_EQ(2, options.response_headers_size()); EXPECT_EQ("foo", options.response_headers(1).header().key()); EXPECT_EQ("bar2", options.response_headers(1).header().value()); EXPECT_EQ(false, options.response_headers(1).append().value()); - f.applyConfigToResponseHeaders(header_map, options); + Server::Configuration::applyConfigToResponseHeaders(header_map, options); EXPECT_TRUE(Envoy::TestUtility::headerMapEqualIgnoreOrder( header_map, Envoy::Http::TestRequestHeaderMapImpl{{":status", "200"}, {"foo", "bar2"}})); - EXPECT_TRUE(f.mergeJsonConfig( + EXPECT_TRUE(Server::Configuration::mergeJsonConfig( R"({response_headers: [ { header: { key: "foo2", value: "bar3"}, append: true } ]})", options, error_message)); - EXPECT_EQ(absl::nullopt, error_message); + EXPECT_EQ("", error_message); EXPECT_EQ(3, options.response_headers_size()); EXPECT_EQ("foo2", options.response_headers(2).header().key()); EXPECT_EQ("bar3", options.response_headers(2).header().value()); EXPECT_EQ(true, options.response_headers(2).append().value()); - f.applyConfigToResponseHeaders(header_map, options); + Server::Configuration::applyConfigToResponseHeaders(header_map, options); EXPECT_TRUE(Envoy::TestUtility::headerMapEqualIgnoreOrder( header_map, Envoy::Http::TestResponseHeaderMapImpl{ {":status", "200"}, {"foo", "bar2"}, {"foo2", "bar3"}})); - EXPECT_FALSE(f.mergeJsonConfig(kBadJson, options, error_message)); + EXPECT_FALSE(Server::Configuration::mergeJsonConfig(kBadJson, options, error_message)); EXPECT_EQ("Error merging json config: Unable to parse JSON as proto (INVALID_ARGUMENT:Unexpected " "token.\nbad_json\n^): bad_json", error_message); diff --git a/test/statistic_test.cc b/test/statistic_test.cc index c73e1d6b8..b72d76043 100644 --- a/test/statistic_test.cc +++ b/test/statistic_test.cc @@ -2,10 +2,12 @@ #include #include +#include #include // std::bad_cast #include "external/envoy/source/common/protobuf/utility.h" #include "external/envoy/source/common/stats/isolated_store_impl.h" +#include "external/envoy/test/mocks/stats/mocks.h" #include "external/envoy/test/test_common/file_system_for_test.h" #include "external/envoy/test/test_common/utility.h" @@ -20,7 +22,8 @@ using namespace testing; namespace Nighthawk { -using MyTypes = Types; +using MyTypes = Types; template class TypedStatisticTest : public Test {}; @@ -116,15 +119,15 @@ TYPED_TEST(TypedStatisticTest, SingleAndDoubleValue) { a.addValue(1); EXPECT_EQ(1, a.count()); - EXPECT_DOUBLE_EQ(1, a.mean()); + Helper::expectNear(1.0, a.mean(), a.significantDigits()); EXPECT_DOUBLE_EQ(0, a.pvariance()); EXPECT_DOUBLE_EQ(0, a.pstdev()); a.addValue(2); EXPECT_EQ(2, a.count()); - EXPECT_DOUBLE_EQ(1.5, a.mean()); - EXPECT_DOUBLE_EQ(0.25, a.pvariance()); - EXPECT_DOUBLE_EQ(0.5, a.pstdev()); + Helper::expectNear(1.5, a.mean(), a.significantDigits()); + Helper::expectNear(0.25, a.pvariance(), a.significantDigits()); + Helper::expectNear(0.5, a.pstdev(), a.significantDigits()); } TYPED_TEST(TypedStatisticTest, CatastrophicalCancellation) { @@ -220,6 +223,15 @@ TYPED_TEST(TypedStatisticTest, StringOutput) { } } +TYPED_TEST(TypedStatisticTest, IdFieldWorks) { + TypeParam statistic; + std::string id = "fooid"; + + EXPECT_EQ("", statistic.id()); + statistic.setId(id); + EXPECT_EQ(id, statistic.id()); +} + class StatisticTest : public Test {}; // Note that we explicitly subject SimpleStatistic to the large @@ -269,6 +281,20 @@ TEST(StatisticTest, StreamingStatProtoOutputLargeValues) { EXPECT_EQ(proto.pstdev().nanos(), 0); } +TEST(StatisticTest, CircllhistStatisticProtoOutputLargeValues) { + CircllhistStatistic statistic; + uint64_t value = 100ul + 0xFFFFFFFF; + statistic.addValue(value); + statistic.addValue(value); + const nighthawk::client::Statistic proto = + statistic.toProto(Statistic::SerializationDomain::DURATION); + + EXPECT_EQ(proto.count(), 2); + Helper::expectNear(Envoy::Protobuf::util::TimeUtil::DurationToNanoseconds(proto.mean()), value, + statistic.significantDigits()); + EXPECT_EQ(Envoy::Protobuf::util::TimeUtil::DurationToNanoseconds(proto.pstdev()), 0); +} + TEST(StatisticTest, HdrStatisticPercentilesProto) { nighthawk::client::Statistic parsed_json_proto; HdrStatistic statistic; @@ -281,33 +307,51 @@ TEST(StatisticTest, HdrStatisticPercentilesProto) { util.loadFromJson(Envoy::Filesystem::fileSystemForTest().fileReadToEnd( TestEnvironment::runfilesPath("test/test_data/hdr_proto_json.gold")), parsed_json_proto, Envoy::ProtobufMessage::getStrictValidationVisitor()); - // Instead of comparing proto's, we perform a string-based comparison, because that emits a - // helpful diff when this fails. const std::string json = util.getJsonStringFromMessage( statistic.toProto(Statistic::SerializationDomain::DURATION), true, true); const std::string golden_json = util.getJsonStringFromMessage(parsed_json_proto, true, true); - EXPECT_EQ(json, golden_json); + EXPECT_THAT(statistic.toProto(Statistic::SerializationDomain::DURATION), + Envoy::ProtoEq(parsed_json_proto)) + << json << "\n" + << "is not equal to golden file:\n" + << golden_json; +} + +TEST(StatisticTest, CircllhistStatisticPercentilesProto) { + nighthawk::client::Statistic parsed_json_proto; + CircllhistStatistic statistic; + + for (int i = 1; i <= 10; i++) { + statistic.addValue(i); + } + + Envoy::MessageUtil util; + util.loadFromJson(Envoy::Filesystem::fileSystemForTest().fileReadToEnd( + TestEnvironment::runfilesPath("test/test_data/circllhist_proto_json.gold")), + parsed_json_proto, Envoy::ProtobufMessage::getStrictValidationVisitor()); + const std::string json = util.getJsonStringFromMessage( + statistic.toProto(Statistic::SerializationDomain::DURATION), true, true); + const std::string golden_json = util.getJsonStringFromMessage(parsed_json_proto, true, true); + EXPECT_THAT(statistic.toProto(Statistic::SerializationDomain::DURATION), + Envoy::ProtoEq(parsed_json_proto)) + << json << "\n" + << "is not equal to golden file:\n" + << golden_json; } TEST(StatisticTest, CombineAcrossTypesFails) { HdrStatistic a; InMemoryStatistic b; StreamingStatistic c; + CircllhistStatistic d; EXPECT_THROW(a.combine(b), std::bad_cast); EXPECT_THROW(a.combine(c), std::bad_cast); EXPECT_THROW(b.combine(a), std::bad_cast); EXPECT_THROW(b.combine(c), std::bad_cast); EXPECT_THROW(c.combine(a), std::bad_cast); EXPECT_THROW(c.combine(b), std::bad_cast); -} - -TEST(StatisticTest, IdFieldWorks) { - StreamingStatistic c; - std::string id = "fooid"; - - EXPECT_EQ("", c.id()); - c.setId(id); - EXPECT_EQ(id, c.id()); + EXPECT_THROW(c.combine(d), std::bad_cast); + EXPECT_THROW(d.combine(a), std::bad_cast); } TEST(StatisticTest, HdrStatisticOutOfRange) { @@ -319,13 +363,73 @@ TEST(StatisticTest, HdrStatisticOutOfRange) { TEST(StatisticTest, NullStatistic) { NullStatistic stat; EXPECT_EQ(0, stat.count()); + std::string id = "fooid"; + stat.setId(id); + EXPECT_EQ(id, stat.id()); stat.addValue(1); + EXPECT_EQ(0, stat.count()); + EXPECT_EQ(0, stat.max()); + EXPECT_EQ(UINT64_MAX, stat.min()); EXPECT_EQ(0, stat.mean()); EXPECT_EQ(0, stat.pvariance()); EXPECT_EQ(0, stat.pstdev()); EXPECT_NE(nullptr, stat.combine(stat)); EXPECT_EQ(0, stat.significantDigits()); EXPECT_NE(nullptr, stat.createNewInstanceOfSameType()); + const nighthawk::client::Statistic proto = stat.toProto(Statistic::SerializationDomain::RAW); + EXPECT_EQ(id, proto.id()); + EXPECT_EQ(0, proto.count()); + EXPECT_EQ(0, proto.raw_mean()); + EXPECT_EQ(0, proto.raw_pstdev()); + EXPECT_EQ(0, proto.raw_max()); + EXPECT_EQ(UINT64_MAX, proto.raw_min()); +} + +using SinkableTypes = Types; + +template class SinkableStatisticTest : public Test {}; + +TYPED_TEST_SUITE(SinkableStatisticTest, SinkableTypes); + +TYPED_TEST(SinkableStatisticTest, EmptySinkableStatistic) { + Envoy::Stats::MockIsolatedStatsStore mock_store; + TypeParam stat(mock_store); + EXPECT_EQ(0, stat.count()); + EXPECT_TRUE(std::isnan(stat.mean())); + EXPECT_TRUE(std::isnan(stat.pvariance())); + EXPECT_TRUE(std::isnan(stat.pstdev())); + EXPECT_EQ(stat.min(), UINT64_MAX); + EXPECT_EQ(stat.max(), 0); + EXPECT_EQ(Envoy::Stats::Histogram::Unit::Unspecified, stat.unit()); + EXPECT_FALSE(stat.used()); + EXPECT_EQ("", stat.name()); + EXPECT_DEATH(stat.tagExtractedName(), ".*"); + EXPECT_EQ(absl::nullopt, stat.worker_id()); +} + +TYPED_TEST(SinkableStatisticTest, SimpleSinkableStatistic) { + Envoy::Stats::MockIsolatedStatsStore mock_store; + const int worker_id = 0; + TypeParam stat(mock_store, worker_id); + const uint64_t sample_value = 123; + const std::string stat_name = "stat_name"; + + EXPECT_CALL(mock_store, deliverHistogramToSinks(_, sample_value)).Times(1); + stat.recordValue(sample_value); + stat.setId(stat_name); + + EXPECT_EQ(1, stat.count()); + Helper::expectNear(123.0, stat.mean(), stat.significantDigits()); + EXPECT_DOUBLE_EQ(0, stat.pvariance()); + EXPECT_DOUBLE_EQ(0, stat.pstdev()); + EXPECT_EQ(123, stat.min()); + EXPECT_EQ(123, stat.max()); + EXPECT_EQ(Envoy::Stats::Histogram::Unit::Unspecified, stat.unit()); + EXPECT_TRUE(stat.used()); + EXPECT_EQ(stat_name, stat.name()); + EXPECT_DEATH(stat.tagExtractedName(), ".*"); + EXPECT_TRUE(stat.worker_id().has_value()); + EXPECT_EQ(worker_id, stat.worker_id().value()); } } // namespace Nighthawk diff --git a/test/stream_decoder_test.cc b/test/stream_decoder_test.cc index 93cb5333f..a18cabc10 100644 --- a/test/stream_decoder_test.cc +++ b/test/stream_decoder_test.cc @@ -1,9 +1,9 @@ #include +#include "external/envoy/source/common/common/random_generator.h" #include "external/envoy/source/common/event/dispatcher_impl.h" #include "external/envoy/source/common/http/header_map_impl.h" #include "external/envoy/source/common/network/utility.h" -#include "external/envoy/source/common/runtime/runtime_impl.h" #include "external/envoy/source/common/stats/isolated_store_impl.h" #include "external/envoy/test/mocks/http/mocks.h" #include "external/envoy/test/mocks/stream_info/mocks.h" @@ -49,7 +49,7 @@ class StreamDecoderTest : public Test, public StreamDecoderCompletionCallback { HeaderMapPtr request_headers_; uint64_t stream_decoder_completion_callbacks_{0}; uint64_t pool_failures_{0}; - Envoy::Runtime::RandomGeneratorImpl random_generator_; + Envoy::Random::RandomGeneratorImpl random_generator_; Envoy::Tracing::HttpTracerSharedPtr http_tracer_; Envoy::Http::ResponseHeaderMapPtr test_header_; Envoy::Http::ResponseTrailerMapPtr test_trailer_; diff --git a/test/test_data/circllhist_proto_json.gold b/test/test_data/circllhist_proto_json.gold new file mode 100644 index 000000000..4262b4fd2 --- /dev/null +++ b/test/test_data/circllhist_proto_json.gold @@ -0,0 +1,130 @@ +{ + "count": "10", + "id": "", + "percentiles": [ + { + "percentile": 0, + "count": "0", + "duration": "0.000000001s" + }, + { + "percentile": 0.1, + "count": "1", + "duration": "0.000000001s" + }, + { + "percentile": 0.2, + "count": "2", + "duration": "0.000000002s" + }, + { + "percentile": 0.3, + "count": "3", + "duration": "0.000000003s" + }, + { + "percentile": 0.4, + "count": "4", + "duration": "0.000000004s" + }, + { + "percentile": 0.5, + "count": "5", + "duration": "0.000000005s" + }, + { + "percentile": 0.55, + "count": "5", + "duration": "0.000000006s" + }, + { + "percentile": 0.6, + "count": "6", + "duration": "0.000000006s" + }, + { + "percentile": 0.65, + "count": "6", + "duration": "0.000000007s" + }, + { + "percentile": 0.7, + "count": "7", + "duration": "0.000000007s" + }, + { + "percentile": 0.75, + "count": "7", + "duration": "0.000000008s" + }, + { + "percentile": 0.775, + "count": "7", + "duration": "0.000000008s" + }, + { + "percentile": 0.8, + "count": "8", + "duration": "0.000000008s" + }, + { + "percentile": 0.825, + "count": "8", + "duration": "0.000000009s" + }, + { + "percentile": 0.85, + "count": "8", + "duration": "0.000000009s" + }, + { + "percentile": 0.875, + "count": "8", + "duration": "0.000000009s" + }, + { + "percentile": 0.9, + "count": "9", + "duration": "0.000000009s" + }, + { + "percentile": 0.925, + "count": "9", + "duration": "0.000000010s" + }, + { + "percentile": 0.95, + "count": "9", + "duration": "0.000000010s" + }, + { + "percentile": 0.975, + "count": "9", + "duration": "0.000000010s" + }, + { + "percentile": 0.99, + "count": "9", + "duration": "0.000000010s" + }, + { + "percentile": 0.995, + "count": "9", + "duration": "0.000000010s" + }, + { + "percentile": 0.999, + "count": "9", + "duration": "0.000000010s" + }, + { + "percentile": 1, + "count": "10", + "duration": "0.000000011s" + } + ], + "mean": "0.000000006s", + "pstdev": "0.000000003s", + "min": "0.000000001s", + "max": "0.000000010s" +} diff --git a/test/worker_test.cc b/test/worker_test.cc index 30263f0af..c30a8b323 100644 --- a/test/worker_test.cc +++ b/test/worker_test.cc @@ -1,5 +1,6 @@ #include +#include "external/envoy/source/common/common/random_generator.h" #include "external/envoy/source/common/runtime/runtime_impl.h" #include "external/envoy/source/common/stats/isolated_store_impl.h" #include "external/envoy/test/mocks/local_info/mocks.h" @@ -36,7 +37,7 @@ class WorkerTest : public Test { Envoy::Api::ApiPtr api_; Envoy::ThreadLocal::MockInstance tls_; Envoy::Stats::IsolatedStoreImpl test_store_; - Envoy::Runtime::RandomGeneratorImpl rand_; + Envoy::Random::RandomGeneratorImpl rand_; NiceMock local_info_; NiceMock validation_visitor_; }; diff --git a/tools/check_envoy_includes.py b/tools/check_envoy_includes.py index a121485c4..f7ca1f550 100755 --- a/tools/check_envoy_includes.py +++ b/tools/check_envoy_includes.py @@ -32,6 +32,9 @@ def inspect_line(bazel_output_base, file_path, line): alternative_found = os.path.isfile(potential_envoy_path) if alternative_found: + # TODO(#399): remove after extension includes are available at the proper location. + if "extensions/filters/http/fault/fault_filter.h" == path: + return True sys.stderr.writelines("Bad include in file " + str(file_path) + ": " + path) sys.stderr.writelines(" (Possible fixed path: " + potential_envoy_path[len(bazel_output_base) + 1:] + ")\n") diff --git a/tools/valgrind-suppressions.txt b/tools/valgrind-suppressions.txt deleted file mode 100644 index dce24197c..000000000 --- a/tools/valgrind-suppressions.txt +++ /dev/null @@ -1,12 +0,0 @@ -{ - - Memcheck:Leak - match-leak-kinds: definite - fun:_Znam - fun:InitModule - fun:_ZN15MallocExtension8RegisterEPS_ - fun:__cxx_global_var_init.2 - fun:_GLOBAL__sub_I_tcmalloc.cc - fun:__libc_csu_init - fun:(below main) -} \ No newline at end of file diff --git a/tools/valgrind-tests.sh b/tools/valgrind-tests.sh deleted file mode 100755 index 0b00311eb..000000000 --- a/tools/valgrind-tests.sh +++ /dev/null @@ -1,4 +0,0 @@ -export TEST_WORKSPACE=. -export TEST_SRCDIR="$(pwd)" -export ENVOY_IP_TEST_VERSIONS="v4only" -valgrind --leak-check=full --track-origins=yes --gen-suppressions=all --suppressions=nighthawk/envoy/tools/valgrind-suppressions.txt --suppressions=nighthawk/tools/valgrind-suppressions.txt bazel-bin/test/nighthawk_test diff --git a/tools/valgrind.sh b/tools/valgrind.sh deleted file mode 100755 index 049b2dacb..000000000 --- a/tools/valgrind.sh +++ /dev/null @@ -1 +0,0 @@ -valgrind --leak-check=full --track-origins=yes --gen-suppressions=all --suppressions=nighthawk/envoy/tools/valgrind-suppressions.txt --suppressions=nighthawk/tools/valgrind-suppressions.txt bazel-bin/nighthawk_client --rps 2 http://127.0.0.1:10000/