Migrate Nighthawk off the proto DebugString method. (#1078)

The `DebugString` method will be deprecated shortly. Migrating to `absl::StrCat` on the proto messages instead.

Signed-off-by: Jakub Sobon <[email protected]>

source/adaptive_load/adaptive_load_client_main.cc

@@ -20,6 +20,7 @@
 #include "source/common/utility.h"
 #include "source/common/version_info.h"
 
+#include "absl/strings/str_cat.h"
 #include "fmt/ranges.h"
 #include "google/rpc/status.pb.h"
 #include "tclap/CmdLine.h"
@@ -125,7 +126,7 @@ uint32_t AdaptiveLoadClientMain::Run() {
     return 1;
   }
   nighthawk::adaptive_load::AdaptiveLoadSessionOutput output = output_or.value();
-  WriteFileOrThrow(filesystem_, output_filename_, output.DebugString());
+  WriteFileOrThrow(filesystem_, output_filename_, absl::StrCat(output));
   return 0;
 }
 

source/adaptive_load/adaptive_load_controller_impl.cc

@@ -27,6 +27,7 @@
 
 #include "absl/container/flat_hash_map.h"
 #include "absl/status/status.h"
+#include "absl/strings/str_cat.h"
 #include "absl/strings/str_format.h"
 #include "absl/strings/str_join.h"
 
@@ -87,7 +88,7 @@ void LogGlobalResultExcludingStatistics(const nighthawk::client::ExecutionRespon
   ENVOY_LOG_MISC(info,
                  "Got result (stripped to just the global result excluding "
                  "statistics): {}",
-                 stripped.DebugString());
+                 absl::StrCat(stripped));
 }
 
 /**
@@ -137,7 +138,7 @@ absl::StatusOr<BenchmarkResult> AdaptiveLoadControllerImpl::PerformAndAnalyzeNig
   // or testing stage.
   *command_line_options.mutable_duration() = std::move(duration);
 
-  ENVOY_LOG_MISC(info, "Sending load: {}", command_line_options.DebugString());
+  ENVOY_LOG_MISC(info, "Sending load: {}", absl::StrCat(command_line_options));
   Envoy::SystemTime start_time = time_source_.systemTime();
   absl::StatusOr<nighthawk::client::ExecutionResponse> nighthawk_response_or =
       nighthawk_service_client_.PerformNighthawkBenchmark(nighthawk_service_stub,
@@ -165,7 +166,7 @@ absl::StatusOr<BenchmarkResult> AdaptiveLoadControllerImpl::PerformAndAnalyzeNig
   Envoy::TimestampUtil::systemClockToTimestamp(end_time, *benchmark_result.mutable_end_time());
 
   for (const MetricEvaluation& evaluation : benchmark_result.metric_evaluations()) {
-    ENVOY_LOG_MISC(info, "Evaluation: {}", evaluation.DebugString());
+    ENVOY_LOG_MISC(info, "Evaluation: {}", absl::StrCat(evaluation));
   }
   step_controller.UpdateAndRecompute(benchmark_result);
   return benchmark_result;
@@ -206,7 +207,7 @@ absl::StatusOr<AdaptiveLoadSessionOutput> AdaptiveLoadControllerImpl::PerformAda
 
     if (spec.has_benchmark_cooldown_duration()) {
       ENVOY_LOG_MISC(info, "Cooling down before the next benchmark for duration: {}",
-                     spec.benchmark_cooldown_duration().ShortDebugString());
+                     absl::StrCat(spec.benchmark_cooldown_duration()));
       uint64_t sleep_time_ms = Envoy::Protobuf::util::TimeUtil::DurationToMilliseconds(
           spec.benchmark_cooldown_duration());
       absl::SleepFor(absl::Milliseconds(sleep_time_ms));

source/client/process_impl.cc

@@ -37,6 +37,7 @@
 
 #include "source/client/process_bootstrap.h"
 
+#include "absl/strings/str_cat.h"
 #include "absl/strings/str_replace.h"
 #include "absl/types/optional.h"
 
@@ -839,7 +840,7 @@ bool ProcessImpl::runInternal(OutputCollector& collector, const UriPtr& tracing_
       setupTracingImplementation(bootstrap_, *tracing_uri);
       addTracingCluster(bootstrap_, *tracing_uri);
     }
-    ENVOY_LOG(debug, "Computed configuration: {}", bootstrap_.DebugString());
+    ENVOY_LOG(debug, "Computed configuration: {}", absl::StrCat(bootstrap_));
     cluster_manager_ = cluster_manager_factory_->clusterManagerFromProto(bootstrap_);
     maybeCreateTracingDriver(bootstrap_.tracing());
     cluster_manager_->setInitializedCb(

source/client/service_impl.cc

@@ -9,6 +9,8 @@
 #include "source/client/output_collector_impl.h"
 #include "source/common/request_source_impl.h"
 
+#include "absl/strings/str_cat.h"
+
 namespace Nighthawk {
 namespace Client {
 
@@ -78,7 +80,7 @@ void ServiceImpl::handleExecutionRequest(const nighthawk::client::ExecutionReque
 }
 
 void ServiceImpl::writeResponse(const nighthawk::client::ExecutionResponse& response) {
-  ENVOY_LOG(debug, "Write response: {}", response.DebugString());
+  ENVOY_LOG(debug, "Write response: {}", absl::StrCat(response));
   if (!stream_->Write(response)) {
     ENVOY_LOG(warn, "Failed to write response to the stream");
   }
@@ -107,7 +109,7 @@ grpc::Status ServiceImpl::ExecutionStream(
   stream_ = stream;
 
   while (stream->Read(&request)) {
-    ENVOY_LOG(debug, "Read ExecutionRequest data {}", request.DebugString());
+    ENVOY_LOG(debug, "Read ExecutionRequest data {}", absl::StrCat(request));
     if (request.has_start_request()) {
       // If busy_lock_ is held we can't start a new benchmark run because one is active already.
       if (busy_lock_.tryLock()) {
@@ -153,7 +155,7 @@ grpc::Status RequestSourceServiceImpl::RequestStream(
   nighthawk::request_source::RequestStreamRequest request;
   bool ok = true;
   while (stream->Read(&request)) {
-    ENVOY_LOG(trace, "Inbound RequestStreamRequest {}", request.DebugString());
+    ENVOY_LOG(trace, "Inbound RequestStreamRequest {}", absl::StrCat(request));
 
     // TODO(oschaaf): this is useful for integration testing purposes, but sending
     // these nearly empty headers will basically be a near no-op (note that the client will merge

source/common/request_stream_grpc_client_impl.cc

@@ -14,6 +14,8 @@
 
 #include "source/common/request_impl.h"
 
+#include "absl/strings/str_cat.h"
+
 namespace Nighthawk {
 
 using ::nighthawk::request_source::RequestSpecifier;
@@ -41,7 +43,7 @@ void RequestStreamGrpcClientImpl::trySendRequest() {
     request.set_quantity(header_buffer_length_);
     stream_->sendMessage(request, false);
     in_flight_headers_ = header_buffer_length_;
-    ENVOY_LOG(trace, "send request: {}", request.DebugString());
+    ENVOY_LOG(trace, "send request: {}", absl::StrCat(request));
   }
 }
 
@@ -142,7 +144,7 @@ RequestPtr RequestStreamGrpcClientImpl::maybeDequeue() {
 
 void RequestStreamGrpcClientImpl::emplaceMessage(
     std::unique_ptr<nighthawk::request_source::RequestStreamResponse>&& message) {
-  ENVOY_LOG(trace, "message received: {}", message->DebugString());
+  ENVOY_LOG(trace, "message received: {}", absl::StrCat(*message));
   messages_.emplace(std::move(message));
 }
 

source/common/statistic_impl.cc

@@ -9,6 +9,7 @@
 #include "external/envoy/source/common/common/assert.h"
 #include "external/envoy/source/common/protobuf/utility.h"
 
+#include "absl/strings/str_cat.h"
 #include "internal_proto/statistic/statistic.pb.h"
 
 namespace Nighthawk {
@@ -29,7 +30,7 @@ static void setDurationFromNanos(Envoy::ProtobufWkt::Duration& mutable_duration,
 } // namespace
 
 std::string StatisticImpl::toString() const {
-  return toProto(SerializationDomain::RAW).DebugString();
+  return absl::StrCat(toProto(SerializationDomain::RAW));
 }
 
 nighthawk::client::Statistic StatisticImpl::toProto(SerializationDomain domain) const {

source/distributor/nighthawk_distributor_client_impl.cc

@@ -2,6 +2,8 @@
 
 #include "external/envoy/source/common/common/assert.h"
 
+#include "absl/strings/str_cat.h"
+
 namespace Nighthawk {
 
 absl::StatusOr<nighthawk::DistributedResponse> NighthawkDistributorClientImpl::DistributedRequest(
@@ -11,7 +13,7 @@ absl::StatusOr<nighthawk::DistributedResponse> NighthawkDistributorClientImpl::D
   std::shared_ptr<::grpc::ClientReaderWriterInterface<nighthawk::DistributedRequest,
                                                       nighthawk::DistributedResponse>>
       stream(nighthawk_distributor_stub.DistributedRequestStream(&context));
-  ENVOY_LOG_MISC(trace, "Write {}", distributed_request.DebugString());
+  ENVOY_LOG_MISC(trace, "Write {}", absl::StrCat(distributed_request));
   if (!stream->Write(distributed_request)) {
     return absl::UnavailableError(
         "Failed to write request to the Nighthawk Distributor gRPC channel.");
@@ -25,7 +27,7 @@ absl::StatusOr<nighthawk::DistributedResponse> NighthawkDistributorClientImpl::D
     RELEASE_ASSERT(!got_response,
                    "Distributor Service has started responding with more than one message.");
     got_response = true;
-    ENVOY_LOG_MISC(trace, "Read {}", response.DebugString());
+    ENVOY_LOG_MISC(trace, "Read {}", absl::StrCat(response));
   }
   if (!got_response) {
     return absl::InternalError("Distributor Service did not send a gRPC response.");

source/distributor/service_impl.cc

@@ -10,6 +10,8 @@
 
 #include "api/distributor/distributor.pb.validate.h"
 
+#include "absl/strings/str_cat.h"
+
 namespace Nighthawk {
 namespace {
 
@@ -87,7 +89,7 @@ grpc::Status NighthawkDistributorServiceImpl::DistributedRequestStream(
   nighthawk::DistributedRequest request;
   grpc::Status status = grpc::Status::OK;
   while (status.ok() && stream->Read(&request)) {
-    ENVOY_LOG(trace, "Inbound DistributedRequest {}", request.DebugString());
+    ENVOY_LOG(trace, "Inbound DistributedRequest {}", absl::StrCat(request));
     status = validateRequest(request);
     if (status.ok()) {
       std::tuple<grpc::Status, nighthawk::DistributedResponse> status_and_response =
@@ -99,7 +101,7 @@ grpc::Status NighthawkDistributorServiceImpl::DistributedRequestStream(
         status = grpc::Status(grpc::StatusCode::INTERNAL,
                               std::string("Failed to write DistributedResponse."));
       } else {
-        ENVOY_LOG(trace, "Wrote DistributedResponse {}", response.DebugString());
+        ENVOY_LOG(trace, "Wrote DistributedResponse {}", absl::StrCat(response));
       }
     } else {
       ENVOY_LOG(error, "DistributedRequest invalid: ({}) '{}'", status.error_code(),

source/server/configuration.cc

@@ -11,6 +11,7 @@
 #include "api/server/response_options.pb.validate.h"
 
 #include "absl/strings/numbers.h"
+#include "absl/strings/str_cat.h"
 
 namespace Nighthawk {
 namespace Server {
@@ -75,7 +76,7 @@ void validateResponseOptions(const nighthawk::server::ResponseOptions& response_
     throw Envoy::EnvoyException(
         absl::StrCat("invalid configuration in nighthawk::server::ResponseOptions ",
                      "cannot specify both response_headers and v3_response_headers ",
-                     "configuration was: ", response_options.ShortDebugString()));
+                     "configuration was: ", absl::StrCat(response_options)));
   }
 }
 

source/sink/service_impl.cc

@@ -9,6 +9,8 @@
 #include "source/sink/nighthawk_sink_client_impl.h"
 #include "source/sink/sink_impl.h"
 
+#include "absl/strings/str_cat.h"
+
 namespace Nighthawk {
 
 using ::Envoy::Protobuf::util::MessageDifferencer;
@@ -21,7 +23,7 @@ grpc::Status SinkServiceImpl::StoreExecutionResponseStream(
   RELEASE_ASSERT(request_reader != nullptr, "stream == nullptr");
   nighthawk::StoreExecutionRequest request;
   while (request_reader->Read(&request)) {
-    ENVOY_LOG(trace, "StoreExecutionResponseStream request {}", request.DebugString());
+    ENVOY_LOG(trace, "StoreExecutionResponseStream request {}", absl::StrCat(request));
     const absl::Status status = sink_->StoreExecutionResultPiece(request.execution_response());
     if (!status.ok()) {
       return abslStatusToGrpcStatus(status);
@@ -44,7 +46,7 @@ grpc::Status SinkServiceImpl::SinkRequestStream(
   nighthawk::SinkRequest request;
   RELEASE_ASSERT(stream != nullptr, "stream == nullptr");
   while (stream->Read(&request)) {
-    ENVOY_LOG(trace, "Inbound SinkRequest {}", request.DebugString());
+    ENVOY_LOG(trace, "Inbound SinkRequest {}", absl::StrCat(request));
     absl::StatusOr<std::vector<nighthawk::client::ExecutionResponse>> execution_responses =
         sink_->LoadExecutionResult(request.execution_id());
     if (!execution_responses.status().ok()) {
@@ -80,14 +82,14 @@ absl::Status mergeOutput(const nighthawk::client::Output& input_to_merge,
     if (!MessageDifferencer::Equivalent(input_to_merge.options(), merge_target.options())) {
       return absl::Status{absl::StatusCode::kInternal,
                           fmt::format("Options divergence detected: {} vs {}.",
-                                      merge_target.options().DebugString(),
-                                      input_to_merge.options().DebugString())};
+                                      absl::StrCat(merge_target.options()),
+                                      absl::StrCat(input_to_merge.options()))};
     }
     if (!MessageDifferencer::Equivalent(input_to_merge.version(), merge_target.version())) {
       return absl::Status{absl::StatusCode::kInternal,
                           fmt::format("Version divergence detected: {} vs {}.",
-                                      merge_target.version().DebugString(),
-                                      input_to_merge.version().DebugString())};
+                                      absl::StrCat(merge_target.version()),
+                                      absl::StrCat(input_to_merge.version()))};
     }
   }
   // Append all input results into our own results.

test/adaptive_load/adaptive_load_client_main_test.cc

@@ -18,6 +18,7 @@
 #include "test/mocks/adaptive_load/mock_adaptive_load_controller.h"
 #include "test/test_common/environment.h"
 
+#include "absl/strings/str_cat.h"
 #include "absl/strings/string_view.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -327,7 +328,7 @@ TEST(AdaptiveLoadClientMainTest, WritesOutputProtoToFile) {
                                 .value();
   nighthawk::adaptive_load::AdaptiveLoadSessionOutput golden_proto;
   Envoy::Protobuf::TextFormat::ParseFromString(golden_text, &golden_proto);
-  EXPECT_EQ(actual_outfile_contents, golden_proto.DebugString());
+  EXPECT_EQ(actual_outfile_contents, absl::StrCat(golden_proto));
 }
 
 TEST(AdaptiveLoadClientMainTest, DefaultsToInsecureConnection) {

test/adaptive_load/adaptive_load_controller_test.cc

@@ -42,6 +42,7 @@
 
 #include "absl/container/flat_hash_map.h"
 #include "absl/status/status.h"
+#include "absl/strings/str_cat.h"
 #include "absl/strings/str_join.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -171,7 +172,7 @@ TEST_F(AdaptiveLoadControllerImplFixture, CopiesThresholdSpecsIntoOutput) {
   MetricSpecWithThreshold actual_spec_with_threshold = output_or.value().metric_thresholds(0);
   EXPECT_TRUE(
       MessageDifferencer::Equivalent(actual_spec_with_threshold, spec.metric_thresholds(0)));
-  EXPECT_EQ(actual_spec_with_threshold.DebugString(), spec.metric_thresholds(0).DebugString());
+  EXPECT_EQ(absl::StrCat(actual_spec_with_threshold), absl::StrCat(spec.metric_thresholds(0)));
 }
 
 TEST_F(AdaptiveLoadControllerImplFixture, TimesOutIfNeverConverged) {

test/test_common/proto_matchers.h

@@ -4,6 +4,7 @@
 
 #include "external/envoy/source/common/protobuf/protobuf.h"
 
+#include "absl/strings/str_cat.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -26,9 +27,9 @@ MATCHER_P(EqualsProto, expected_proto, "is equal to the expected_proto") {
   if (!equal) {
     *result_listener << "\n"
                      << "=======================Expected proto:===========================\n"
-                     << expected_proto.DebugString()
+                     << absl::StrCat(expected_proto)
                      << "------------------is not equal to actual proto:------------------\n"
-                     << arg.DebugString()
+                     << absl::StrCat(arg)
                      << "------------------------the diff is:-----------------------------\n"
                      << diff
                      << "=================================================================\n";