Pull latest into my fork #1
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- `SetSessionSpecDefaults()`: Returns a copy of the `AdaptiveLoadSessionSpec` with default values added. - `CheckSessionSpec()`: Checks an `AdaptiveLoadSessionSpec` for illegal values, invalid plugin references, and invalid plugin configs. Part 6 of splitting PR #483. Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>
- Add dubious90 to the list of owners. - Also add in recently discussed conventions about maintaining and contributing to nighthawk. Signed-off-by: Nathan Perry <nbperry@google.com>
Our own dynamic delay extension owns this now. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
- Add missing `const` to helper methods. - Add a missing build dep - Add a missing `#pragma once ` Part 7 of splitting PR #483. Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>
Split out from #512 Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Make the end-to-end test for POST handling cover all extensions. Split out from #512 Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
- MockNighthawkServiceClient - MockMetricsEvaluator - MockAdaptiveLoadSessionSpecProtoHelper Part 8 of splitting PR #483. Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>
This is helpful to assist with building Nighthawk's production ready artifacts via docker build images. For example: ```bash DOCKER_IMAGE=envoyproxy/envoy-build-[centos|ubuntu] \ ci/run_envoy_docker.sh ci/do_ci.sh opt_build ``` Related issue: #525 Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Now that we have a benchmark docker image build, it would be good to make sure that keeps working in CI. This also makes it easy to start pushing this image in CI in a follow-up. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Prelude to #512, which includes it and provides a means to see the end goal. - Extracts shared needs between tested extensions. - Sanitized api, better naming. - Doc comments. - Support for testing POST request methods and entity bodies to check the alternative flow that will trigger in extensions when using that. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Adds a post-processing step to massage the fortio output formatting to more exactly reproduce Fortio's output, while leaving the original output formatter intact for backwards compatibility purposes. Fixes known issues #422 and #514. Experimental until we have some confirmation that this can be marked as final / all issues have been resolved. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
formatter factory when to construct the new formatter. Sorry! - Amends a pre-existing test to avoid regression when adding new output formats in the future - Fix the issue Fixes #543 Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
- Update to Envoy 9eeba8fd427d9bd0ef947ec14a3157083cc7bf0e - Update HdrHistogram_c from 0.11.0 -> 0.11.1 Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
This is the main function of the Adaptive Load Controller library: - Check the input proto for errors - Apply default input values - Adjusting Stage loop: - Get the latest dynamically generated CommandLineOptions from the StepController - Run a short benchmark with the Nighthawk Service - Obtain values from MetricsPlugins - Score metrics using ScoringFunction plugins - Report scores back to the StepController, which recalculates the load for the next iteration - Check for convergence deadline exceeded - Check for StepController convergence - Check for StepController doom - Testing Stage: Run one long benchmark on the Nighthawk Service at the converged load Fixes #485. Part 9 of splitting PR #483. Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>
Test basic behavioural properties of all test-server extensions. Any new extensions may piggy-back on this by enlisting themselves. In a follow up we'll purge code from the pre-existing tests that can now be deprecated. Split out from #512 Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
By chance weird behaviour was observed for when specifying large jitter values. This warranted analysis, and upon investigation the root problem seems to be that when configuring `--uniform-jitter` to a value that would allow the adjusted value(s) to overlap with the pacing of the underlying rate limiter, the new overlapping value could overwrite the old one that was scheduled. This modifies behaviour and tests to fix and consolidate, by modifying `DistributionSamplingRateLimiter` so that it can queue adjusted release timings instead of storing a single one. This will allow it to accumulate multiple release timings that are adjusted for jitter. This ensures that the pacing of the rate limiter that modifies timings to add jitter gets properly disconnected from the pacing of the underlying rate limiter. NOTE: There are no known consumers running into problems / inaccuracies because of this. To tease the problem out, either one needs to: - configure `--jitter-uniform` in a way that it would adjust timings to overlap with the frequency imposed by `--rps`, or - configure any combination of `--jitter-uniform > 0s` and `--burst-size > 0`. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Reported via Slack; confirmed to fix a problem when executing tests on some systems. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Small refactor that makes the client rely on system time instead of monotonic time for scheduling the start of worker executions. System clocks can be synchronized across machines, and this may come in handy when we start facilitating horizontal scaling. Note: `SequencerImpl` gets modified to re-use the execution duration that the `RateLimiter` it uses already tracks, in favour of its own tracking. This is a small clean up. Apart from the actual switching from monotonic time to wall clock time, this should be a mechanical change. This change will make things easier if we would like to add an option to schedule the time at which an execution will start. This in turn could be useful when directing clients running on multiple machines to start, as a means to have them start at approximately the same time. (the approximation would mostly depend on how well the wall clock time is synchronised across machines that are involved). Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Update overview doc to include user-specified logging. Signed-off-by: jiajunye <jiajunye@google.com>
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Adding config factories for RequestSourcePlugins. Specifically starting with FileBasedRequestSourcePlugin. These will be provided as options to be used inside RequestSourceFactory. This is part of a series of PRs for providing the ability to use statically linked requestSources. Signed-off-by: William Juan <66322422+wjuan-AFK@users.noreply.github.com>
…ies (#512) - Make the test server use the shared configuration handling code. - Convert its tests to use the new facilities that are shared accross extensions. Last in a series of PRs to fix #498 This is on par with what we did for the dynamic delay and timing extensions, but this extension was slightly more complex to begin with, so saving the best for last ;-) Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
- Update Envoy to 2097fe908f2abb718757dbd4087d793c861d7c5a - Update HdrHistogram_c to 0.11.2 Adds `--define tcmalloc=gperftools` to the opt build we use to push images, to (continue to) allow cpu profiling. We need this because of envoyproxy/envoy@4c0d2d2 Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Notable changes: - Handle the new `Envoy::Http::StreamResetReason:: ConnectError` & amend related test - Handle that `Envoy::Http::HeaderMap::get()` now returns a `Envoy::Http::HeaderMap::GetResult`. - Squelch Envoy's format check error that scans for urls in `bazel/repositories.bzl` Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
This is a more usable version of the Request Source Config Factory similar to the FIle Based Config Factory which is easier to pass through the CLI. Signed-off-by: William Juan <66322422+wjuan-AFK@users.noreply.github.com>
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
- Handle return value of encodeHeaders() now that there is one. - MetricSnapshotImpl now needs a TimeSource in its constructor. - Avoid assert, use valid request header in StreamDecoderTest Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Adding implementation in the factory_impl class for loading request source plugins. This also introduces python tests for the previous PR and undoes the temporary hack to reduce test_integration_coverage threshold in issue #564. Signed-off-by: William Juan <66322422+wjuan-AFK@users.noreply.github.com>
Signed-off-by: Yan Avlasov <yavlasov@google.com>
Verified that all integration tests pass after updating Envoy to commit `588d9344b31e6544869547c4bcd359b3b0f1d4cf`, so this PR unblocks #575. Our next steps will be adding a good warning and a compatibility flag for users of Nighthawk. If they do send configs with Envoy API v2, we will break by default, but allow them to continue with the compatibility flag. Summary of performed changes: - changing `config` to `typed_config` and listing the correct type. - migrating from deprecated field `tls_context` to `transport_socket`. - changing filter names to ones that match extension names in [extensions_build_config.bzl](https://github.com/envoyproxy/nighthawk/blob/master/extensions_build_config.bzl). - cosmetic changes of enum value from `auto` to `AUTO`. Also: - updating README and help displayed by the CLI in regards to passing in the `--tls-context` flag since this behavior is mirrored by one of the edited integration tests. - Adding the `test_request_source_plugin.py` integration test as a dependency of the `integration_test` py_binary which was forgotten before. Works on #580 Signed-off-by: Jakub Sobon <mumak@google.com>
We don't run integration tests in IPv6 mode on CI (See #578). As a result some of the test expectations became invalid. Also: - Pytest now displays more logging on test failures including the stderr and stdout of the started nighthawk test server. - fixing the default IPv6 address, as per `man 3 inet_pton`, the address `::/0` isn't valid, while `::` is. Signed-off-by: Jakub Sobon <mumak@google.com>
Signed-off-by: Jakub Sobon <mumak@google.com>
* chore: add issue and pull request templates Signed-off-by: Kush Trivedi <kushthedude@gmail.com> * add link to contributing.md Signed-off-by: Kush Trivedi <kushthedude@gmail.com>
Adds a new schedule option to the proto api., which allows specifying a date/time at which execution should start. Should also fix #569 by reducing reliance on `Envoy::SystemTime` to a minimum: Based on the value schedule option, a monotonic time will be computed and propagated. This makes us use `Envoy::MonotonicTime` again in places where sub ms resolution is a must have. (The fix will need confirmation, as I couldn't get `--config=libc++` to work for other reasons.) Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
- Adds a new proto/cli option, which allows requesting continued use of the about to be deprecated v2 api's. - Adds a new fixture / integration test for the test server, as a sanity check for making sure that when we update Envoy to the revision that deprecated v2 things will keep working as expected, while documenting how to set up the runtime configuration. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
- Fixes to accommodate upstream connection pool changes. - Fixes to accommodate upstream cluster related changes. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Verified that an Envoy built after `588d9344b31e6544869547c4bcd359b3b0f1d4cf` can load the configuration from the documents. Also: - fixing one typo in the documentation. - cosmetic changes of enum values to uppercase form. Works on #580 Signed-off-by: Jakub Sobon <mumak@google.com>
…ers. (#588) Adds a new field `v3_response_headers` alongside the old v2 `response_headers` in `api/server/response_options.proto` and marks the old field as deprecated. The old field retains its functionality for backward compatibility. Since these are repeated fields, we cannot use a oneof. Instead validation is added to ensure the filters report an error on configuration that has both the v2 and the v3 fields set. `Envoy::EnvoyException` is thrown on validation errors to comply with the `Envoy::Server::Configuration::NamedHttpFilterConfigFactory` interface. Also: - sorting imports in `response_options.proto ` alphabetically. - adding missing anonymous namespace in `http_dynamic_delay_filter_integration_test.cc`. Works on #580. Signed-off-by: Jakub Sobon <mumak@google.com>
…urce. (#589) Converting the current field into a `oneof` and marking the `v2` primitive as deprecated. The old field retains its functionality for backward compatibility. Also: - sorting imports in `api/request_source/service.proto`. - migrating our dummy request source to using the `v3` primitive. - improving error message emitted on test failures in `test/request_stream_grpc_client_test.cc`, the current message just dumps byte representation of the two compared header objects. Fixes #580. Signed-off-by: Jakub Sobon mumak@google.com
Fails the integration tests if any unknown warnings or errors are found in the logs of the Nighthawk test server. Includes an ignore list that (for now) accepts known warnings found in the logs, the ignore list can have different content per each test case. Also: - moving the `Attributes:` docstring section on `IntegrationTestBase` into its class docstring. `__init__` is just a method and should only have the `Args:` section. Fixes #577 Signed-off-by: Jakub Sobon <mumak@google.com>
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
List of changes: - Update Envoy to 8188e232a9e0b15111d30f4724cbc7bf77d3964a (Dec 8th) - Sync .bazelrc - Modify extensions_build_config.bzl, add required DISABLED_BY_DEFAULT_EXTENSIONS - Rework our h1 connection pool derivation to keep our own prefetching feature working. Note: We enable the allow_prefetch runtime feature for this. - Changes to reflect changed Envoy constructors and method signatures: OnPoolReady(), ProdClusterManagerFactory(), allocateConnPool() - Modified log filtering in the integration test: `SIGTERM` -> `ENVOY_SIGTERM`. - Dropped include of `ares.h` as small cleanup, we no longer need that. - In `options_impl.cc` there's a small change to how we define `elapsed_since_epoch` to unbreak building with `libc++`: a regression of #569. Filed #594 to avoid regression. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
- The way we can obtain a http connection pool changed. Amend. - Cleanup by fix_format: strip superfluous .Times(1) in tests. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
…598) The character literal overload is more efficient. Signed-off-by: Yan Avlasov <yavlasov@google.com>
- Unbreak changed allocateConnPool() method usage: changes with respect to ALPN allow negotiation of a protocol, which is backed in Envoy by a pool which supports >1 protocols. This update leaves a code-level comment plus a RELEASE_ASSERT when a multi-protocol pool is allocated. - Avoid MOCK_METHODn as per new check_format objections: Change our mocks to use MOCK_METHOD instead. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
- Transitively updates the grpc dependency, which eliminated the grpc_impl namespace. s/grpc_impl::/grpc::/ - Envoy::LocalInfo::LocalInfoImpl constructor now requires passing a symbol table. Pass that in. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Fixes a bug observed while working on horizontal scaling support: SignalHandler would fire on destruction. Fix & add tests. Note: pre-emptive, I don't think this is an issue in the current state of master. But some of the CI errors in #600 seemed familiar and hopefully merging this in there helps. (Split out from https://github.com/oschaaf/nighthawk/tree/horizontal-scaling) Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
- `Envoy::StreamInfo::StreamInfoImpl` no longer contains methods that set local and remote socket addresses. It instead requires a `Envoy::Network::SocketAddressProviderSharedPtr` with the addresses already set. Pass in a `Envoy::Network::SocketAddressSetterImpl` so that we can continue setting the remote address for tests. - Copied `.bazelversion` and `.bazelrc` from Envoy. Signed-off-by: Jakub Sobon <mumak@google.com>
Updates the Envoy dependency to revision: 17e815122ff53d0ac6cb2d64cdbf1bfc547bb7e8 Non mechanical change: - LocalinfoImpl constructor changed Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
This fixes the broken docker ci workflow. Signed-off-by: Jakub Sobon <mumak@google.com>
- tryCreateNewConnection now returns a ConnPoolImplBase::ConnectionResult instead of a bool. Signed-off-by: Jakub Sobon <mumak@google.com>
* Updating references to branch name master->main. The branch was renamed recently. Signed-off-by: Jakub Sobon <mumak@google.com>
Changing github repository paths from oschaaf to envoyproxy. Signed-off-by: Jakub Sobon <mumak@google.com>
Signed-off-by: Otto van der Schaaf <ovanders@redhat.com>
A mock for AdaptiveLoadController for use in unit tests of the upcoming adaptive load CLI executable. Part of #416 Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.