WIP: Update Envoy to 72bf41f (Jun 4th 2021).

[mum4k]

Blocked until envoyproxy/envoy#16829 or an alternative gets merged upstream (or an alternative gets implemented here).

• modified include paths to match recent change done in the envoy repository (envoyproxy/envoy@0fae697), specifically:
  1. Move include/nighthawk up one level by running git mv include/nighthawk nighthawk.
  2. Use full paths on all #include statements referring to packages under the source/ directory.
  3. Fixing incorrect include paths (pre-existing) that were referring to envoy headers without the external/ prefix. This was reported by check_format after the above changes.
• Changing a bool to envoy::config::core::v3::DnsResolverOptions in a call to Envoy::Event::Dispatcher::createDnsResolver to match a recent Envoy change (envoyproxy/envoy@2c0e1f2).
• changing how the HTTP connection pool is accessed after it was moved behind a new class HttpPoolData in envoy (envoyproxy/envoy@ff2c0e5).
• after envoy switched to the true abseil type of string_view (envoyproxy/envoy@72bf41f), we can no longer assign absl::string_view to std::string. Changing Nighthawk code accordingly.

All the large volume changes (renames and import changes) are purely mechanical. To simplify the review, here is a list of files with non-mechanical changes:

• source/client/benchmark_client_impl.h
• source/client/process_impl.cc
• source/common/uri_impl.cc
• source/sink/service_impl.cc
• source/sink/service_impl.h
• test/adaptive_load/adaptive_load_client_main_test.cc
• test/benchmark_http_client_test.cc
• test/request_source/request_source_plugin_test.cc

Signed-off-by: Jakub Sobon mumak@google.com

[mum4k]

@dubious90 @oschaaf while this PR isn't expected to build until we resolve HTTP pool access upstream (in the Envoy repository, see the PR description), I would appreciate if we could start the review considering the large scope of the changes related to import paths.

Specifically we need to discuss and decide if we are going to follow Envoy and perform git mv include/nighthawk nighthawk or break away from the Envoy structure. This PR assumes that we are going to follow it in order to prevent possible build issues in the future.

[mum4k]

Note - this is a lot to process in a single PR. If you prefer, we can break this out into four distinct PRs, advancing the Envoy commit gradually. However if we choose to do this, we have to hold ourselves to relatively fast review cycles, so that all the PRs make it in before we start the import process mid next week. Feel free to let me know if you would prefer this.

[mum4k]

In case you prefer the breakdown I went ahead and created the first partial PR #692.

[oschaaf]

Ugh this is quite painful. Thanks for splitting out the first PR; maybe separating out the include changes helps reduce clutter a bit. But if it is a lot of work please don't as far as I am concerned. I can swallow this one, just requires a little more focus because of the github UI.

[mum4k]

@oschaaf question of splitting is not about the amount of work, personally I always prioritize comfort of the reviewer as compared to the author. It is only a question of timely reviews, so that we get it all in in time for the next import.

We are onto a good start, since the first PR is already in and @yanavlasov is helping us with the majority of the import changes. I am going to send the next small PR shortly and discuss with @yanavlasov about the import changes.

Please don't invest into reviewing this PR yet, it may no go in as is. I noticed that @yanavlasov decided not to modify the directory structure in Nighthawk, which diverges it from Envoy. I think we will want to understand that choice and also why Envoy decided to do it before we move forward.

[mum4k]

fyi, majority of the changes in this PR were already merged, it was split into the following PRs:

• Update Envoy to 2c0e1f2 (Jun 2nd 2021). #692
• Update Envoy to ff2c0e5 (Jun 2 2021). #694
• Update Envoy to 151aa0ce (Jun 04, 2021) #693
• Update Envoy to 72bf41f (Jun 04, 2021). #696

As for the directory structure - we have decided not to follow Envoy. We aren't going to move include/nighthawk up one level, mostly because we don't have to. Reasoning:

• The issue that Envoy was solving by doing this (gcc build is failing for test targets that depend on //source/exe:envoy_common_lib envoy#16196) isn't going to affect Nighthawk.
• Doing such rename in Nighthawk would cause breakage of existing clients and while this isn't part of our public API, it would be annoying, so we should have a good reason.
• The likelihood of this divergence causing issues in the future is relatively small and if it does, we can always update / create our own build macros to fix the issue.

/cc @dubious90 @oschaaf