From 863029b7fe2a4971312c29a98a93f0861eb81176 Mon Sep 17 00:00:00 2001 From: Tanvi Jagtap <139093547+tanvi-jagtap@users.noreply.github.com> Date: Fri, 10 May 2024 03:29:32 -0700 Subject: [PATCH] [grpc][Gpr_To_Absl_Logging] Migrating from gpr to absl logging - gpr_log (#36577) [grpc][Gpr_To_Absl_Logging] Migrating from gpr to absl logging - gpr_log In this CL we are migrating from gRPCs own gpr logging mechanism to absl logging mechanism. The intention is to deprecate gpr_log in the future. We have the following mapping 1. gpr_log(GPR_INFO,...) -> LOG(INFO) 2. gpr_log(GPR_ERROR,...) -> LOG(ERROR) 3. gpr_log(GPR_DEBUG,...) -> VLOG(2) Reviewers need to check : 1. If the above mapping is correct. 2. The content of the log is as before. gpr_log format strings did not use string_view or std::string . absl LOG accepts these. So there will be some elimination of string_view and std::string related conversions. This is expected. Closes #36577 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36577 from tanvi-jagtap:test_core_all 9887d74d81ceffcedf8c9712056763e822db7b35 PiperOrigin-RevId: 632441576 --- test/core/address_utils/BUILD | 6 +++++- test/core/address_utils/parse_address_test.cc | 14 +++++++------- .../parse_address_with_named_scope_id_test.cc | 13 +++++-------- test/core/bad_client/bad_client.cc | 15 +++++++-------- test/core/bad_client/generate_tests.bzl | 7 ++++--- test/core/bad_connection/BUILD | 5 ++++- test/core/bad_connection/close_fd_test.cc | 14 +++++++------- test/core/bad_ssl/generate_tests.bzl | 1 + test/core/bad_ssl/server_common.cc | 4 ++-- test/core/channel/BUILD | 2 ++ test/core/channel/channel_args_test.cc | 6 +++--- test/core/channel/metrics_test.cc | 13 +++++++------ test/core/compression/BUILD | 10 ++++++++-- test/core/compression/compression_test.cc | 10 +++++----- test/core/compression/message_compress_test.cc | 1 - 15 files changed, 67 insertions(+), 54 deletions(-) diff --git a/test/core/address_utils/BUILD b/test/core/address_utils/BUILD index 15f41c3f9d5..ceb5a12dbb6 100644 --- a/test/core/address_utils/BUILD +++ b/test/core/address_utils/BUILD @@ -41,6 +41,7 @@ grpc_cc_test( name = "parse_address_test", srcs = ["parse_address_test.cc"], external_deps = [ + "absl/log:log", "absl/strings", "gtest", ], @@ -69,7 +70,10 @@ grpc_fuzzer( grpc_cc_test( name = "parse_address_with_named_scope_id_test", srcs = ["parse_address_with_named_scope_id_test.cc"], - external_deps = ["gtest"], + external_deps = [ + "absl/log:log", + "gtest", + ], language = "C++", tags = ["no_windows"], uses_event_engine = False, diff --git a/test/core/address_utils/parse_address_test.cc b/test/core/address_utils/parse_address_test.cc index d94370556e5..41b9df4bd5d 100644 --- a/test/core/address_utils/parse_address_test.cc +++ b/test/core/address_utils/parse_address_test.cc @@ -34,12 +34,12 @@ #include +#include "absl/log/log.h" #include "absl/status/status.h" #include "absl/strings/match.h" #include "gtest/gtest.h" #include -#include #include "src/core/lib/iomgr/exec_ctx.h" #include "src/core/lib/iomgr/port.h" @@ -53,7 +53,7 @@ static void test_grpc_parse_unix(const char* uri_text, const char* pathname) { grpc_core::ExecCtx exec_ctx; absl::StatusOr uri = grpc_core::URI::Parse(uri_text); if (!uri.ok()) { - gpr_log(GPR_ERROR, "%s", uri.status().ToString().c_str()); + LOG(ERROR) << uri.status(); ASSERT_TRUE(uri.ok()); } grpc_resolved_address addr; @@ -70,7 +70,7 @@ static void test_grpc_parse_unix_abstract(const char* uri_text, grpc_core::ExecCtx exec_ctx; absl::StatusOr uri = grpc_core::URI::Parse(uri_text); if (!uri.ok()) { - gpr_log(GPR_ERROR, "%s", uri.status().ToString().c_str()); + LOG(ERROR) << uri.status(); ASSERT_TRUE(uri.ok()); } grpc_resolved_address addr; @@ -98,7 +98,7 @@ static void test_grpc_parse_vsock(const char* uri_text, uint32_t cid, grpc_core::ExecCtx exec_ctx; absl::StatusOr uri = grpc_core::URI::Parse(uri_text); if (!uri.ok()) { - gpr_log(GPR_ERROR, "%s", uri.status().ToString().c_str()); + LOG(ERROR) << uri.status(); ASSERT_TRUE(uri.ok()); } grpc_resolved_address addr; @@ -122,7 +122,7 @@ static void test_grpc_parse_ipv4(const char* uri_text, const char* host, grpc_core::ExecCtx exec_ctx; absl::StatusOr uri = grpc_core::URI::Parse(uri_text); if (!uri.ok()) { - gpr_log(GPR_ERROR, "%s", uri.status().ToString().c_str()); + LOG(ERROR) << uri.status(); ASSERT_TRUE(uri.ok()); } grpc_resolved_address addr; @@ -142,7 +142,7 @@ static void test_grpc_parse_ipv6(const char* uri_text, const char* host, grpc_core::ExecCtx exec_ctx; absl::StatusOr uri = grpc_core::URI::Parse(uri_text); if (!uri.ok()) { - gpr_log(GPR_ERROR, "%s", uri.status().ToString().c_str()); + LOG(ERROR) << uri.status(); ASSERT_TRUE(uri.ok()); } grpc_resolved_address addr; @@ -162,7 +162,7 @@ static void test_grpc_parse_ipv6_invalid(const char* uri_text) { grpc_core::ExecCtx exec_ctx; absl::StatusOr uri = grpc_core::URI::Parse(uri_text); if (!uri.ok()) { - gpr_log(GPR_ERROR, "%s", uri.status().ToString().c_str()); + LOG(ERROR) << uri.status(); ASSERT_TRUE(uri.ok()); } grpc_resolved_address addr; diff --git a/test/core/address_utils/parse_address_with_named_scope_id_test.cc b/test/core/address_utils/parse_address_with_named_scope_id_test.cc index a69fa0072b8..eb76adef43d 100644 --- a/test/core/address_utils/parse_address_with_named_scope_id_test.cc +++ b/test/core/address_utils/parse_address_with_named_scope_id_test.cc @@ -28,13 +28,13 @@ #include +#include "absl/log/log.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/str_format.h" #include "gtest/gtest.h" #include -#include #include "src/core/lib/address_utils/parse_address.h" #include "src/core/lib/gprpp/crash.h" @@ -114,10 +114,8 @@ TEST(ParseAddressWithNamedScopeIdTest, MainTest) { // system recognizes, and then use that for the test. for (size_t i = 1; i < 65536; i++) { if (if_indextoname(i, arbitrary_interface_name) != nullptr) { - gpr_log(GPR_DEBUG, - "Found interface at index %" PRIuPTR - " named %s. Will use this for the test", - i, arbitrary_interface_name); + VLOG(2) << "Found interface at index " << i << " named " + << arbitrary_interface_name << ". Will use this for the test"; break; } } @@ -127,9 +125,8 @@ TEST(ParseAddressWithNamedScopeIdTest, MainTest) { struct sockaddr_in6 result_from_getaddrinfo = resolve_with_gettaddrinfo(target.c_str()); // Run the test - gpr_log(GPR_DEBUG, - "Run test_grpc_parse_ipv6_parity_with_getaddrinfo with target: %s", - target.c_str()); + VLOG(2) << "Run test_grpc_parse_ipv6_parity_with_getaddrinfo with target: " + << target; test_grpc_parse_ipv6_parity_with_getaddrinfo(target.c_str(), result_from_getaddrinfo); // Cleanup diff --git a/test/core/bad_client/bad_client.cc b/test/core/bad_client/bad_client.cc index ee7e1ee4d39..f11000ead0f 100644 --- a/test/core/bad_client/bad_client.cc +++ b/test/core/bad_client/bad_client.cc @@ -22,11 +22,11 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" #include #include #include -#include #include #include @@ -111,11 +111,11 @@ void grpc_run_client_side_validator(grpc_bad_client_arg* arg, uint32_t flags, hex = gpr_dump(arg->client_payload, arg->client_payload_length, GPR_DUMP_HEX | GPR_DUMP_ASCII); // Add a debug log - gpr_log(GPR_INFO, "TEST: %s", hex); + LOG(INFO) << "TEST: " << hex; gpr_free(hex); } else { - gpr_log(GPR_INFO, "TEST: (%" PRIdPTR " byte long string)", - arg->client_payload_length); + LOG(INFO) << "TEST: (" << arg->client_payload_length + << " byte long string)"; } grpc_slice slice = grpc_slice_from_copied_buffer(arg->client_payload, @@ -171,9 +171,8 @@ void grpc_run_client_side_validator(grpc_bad_client_arg* arg, uint32_t flags, .type == GRPC_QUEUE_TIMEOUT); } while (!gpr_event_get(&read_done_event)); if (arg->client_validator(&incoming, arg->client_validator_arg)) break; - gpr_log(GPR_INFO, - "client validator failed; trying additional read " - "in case we didn't get all the data"); + LOG(INFO) << "client validator failed; trying additional read " + "in case we didn't get all the data"; } grpc_slice_buffer_destroy(&incoming); } @@ -317,7 +316,7 @@ bool rst_stream_client_validator(grpc_slice_buffer* incoming, void* /*arg*/) { *p++ == 0 || *p++ == 0 || *p++ == 0 || *p == 0 || *p == 11; if (!success) { - gpr_log(GPR_INFO, "client expected RST_STREAM frame, not found"); + LOG(INFO) << "client expected RST_STREAM frame, not found"; } grpc_slice_buffer_destroy(&last_frame_buffer); diff --git a/test/core/bad_client/generate_tests.bzl b/test/core/bad_client/generate_tests.bzl index 620fb5a114e..2ae6c1b3c42 100755 --- a/test/core/bad_client/generate_tests.bzl +++ b/test/core/bad_client/generate_tests.bzl @@ -44,6 +44,10 @@ def grpc_bad_client_tests(): hdrs = ["bad_client.h"], language = "C++", testonly = 1, + external_deps = [ + "absl/log:check", + "absl/log:log", + ], deps = [ "//test/core/test_util:grpc_test_util", "//:grpc", @@ -51,9 +55,6 @@ def grpc_bad_client_tests(): "//test/core/end2end:cq_verifier", "//:grpc_http_filters", ], - external_deps = [ - "absl/log:check", - ], ) for t, topt in BAD_CLIENT_TESTS.items(): grpc_cc_test( diff --git a/test/core/bad_connection/BUILD b/test/core/bad_connection/BUILD index d424ba888ac..5c674673037 100644 --- a/test/core/bad_connection/BUILD +++ b/test/core/bad_connection/BUILD @@ -23,7 +23,10 @@ grpc_cc_binary( srcs = [ "close_fd_test.cc", ], - external_deps = ["absl/log:check"], + external_deps = [ + "absl/log:check", + "absl/log:log", + ], language = "C++", tags = ["no_windows"], deps = [ diff --git a/test/core/bad_connection/close_fd_test.cc b/test/core/bad_connection/close_fd_test.cc index a51f141270e..d3d69c8c068 100644 --- a/test/core/bad_connection/close_fd_test.cc +++ b/test/core/bad_connection/close_fd_test.cc @@ -24,6 +24,7 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" #include "absl/status/statusor.h" #include "absl/strings/str_format.h" @@ -53,7 +54,6 @@ #include #include #include -#include #include "src/core/ext/transport/chttp2/transport/chttp2_transport.h" #include "src/core/lib/gprpp/crash.h" @@ -225,8 +225,8 @@ static void _test_close_before_server_recv(fd_type fdtype) { grpc_raw_byte_buffer_create(&request_payload_slice, 1); grpc_byte_buffer* response_payload = grpc_raw_byte_buffer_create(&response_payload_slice, 1); - gpr_log(GPR_INFO, "Running test: test_close_%s_before_server_recv", - fd_type_str(fdtype)); + LOG(INFO) << "Running test: test_close_" << fd_type_str(fdtype) + << "_before_server_recv"; test_init(); grpc_op ops[6]; @@ -399,8 +399,8 @@ static void _test_close_before_server_send(fd_type fdtype) { grpc_raw_byte_buffer_create(&request_payload_slice, 1); grpc_byte_buffer* response_payload = grpc_raw_byte_buffer_create(&response_payload_slice, 1); - gpr_log(GPR_INFO, "Running test: test_close_%s_before_server_send", - fd_type_str(fdtype)); + LOG(INFO) << "Running test: test_close_" << fd_type_str(fdtype) + << "_before_server_send"; test_init(); grpc_op ops[6]; @@ -596,8 +596,8 @@ static void _test_close_before_client_send(fd_type fdtype) { grpc_raw_byte_buffer_create(&request_payload_slice, 1); grpc_byte_buffer* response_payload = grpc_raw_byte_buffer_create(&response_payload_slice, 1); - gpr_log(GPR_INFO, "Running test: test_close_%s_before_client_send", - fd_type_str(fdtype)); + LOG(INFO) << "Running test: test_close_" << fd_type_str(fdtype) + << "_before_client_send"; test_init(); grpc_op ops[6]; diff --git a/test/core/bad_ssl/generate_tests.bzl b/test/core/bad_ssl/generate_tests.bzl index 3ce7de49171..ddd3b3c97bf 100755 --- a/test/core/bad_ssl/generate_tests.bzl +++ b/test/core/bad_ssl/generate_tests.bzl @@ -35,6 +35,7 @@ def grpc_bad_ssl_tests(): hdrs = ["server_common.h"], external_deps = [ "absl/log:check", + "absl/log:log", ], deps = [ "//test/core/test_util:grpc_test_util", diff --git a/test/core/bad_ssl/server_common.cc b/test/core/bad_ssl/server_common.cc index 1f9c0a69f5f..e76708b1969 100644 --- a/test/core/bad_ssl/server_common.cc +++ b/test/core/bad_ssl/server_common.cc @@ -21,8 +21,8 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" -#include #include #include "test/core/test_util/cmdline.h" @@ -74,7 +74,7 @@ void bad_ssl_run(grpc_server* server) { signal(SIGINT, sigint_handler); while (!shutdown_finished) { if (got_sigint && !shutdown_started) { - gpr_log(GPR_INFO, "Shutting down due to SIGINT"); + LOG(INFO) << "Shutting down due to SIGINT"; shutdown_cq = grpc_completion_queue_create_for_pluck(nullptr); grpc_server_shutdown_and_notify(server, shutdown_cq, nullptr); CHECK(grpc_completion_queue_pluck(shutdown_cq, nullptr, diff --git a/test/core/channel/BUILD b/test/core/channel/BUILD index 10831f5090c..b54cc023fa3 100644 --- a/test/core/channel/BUILD +++ b/test/core/channel/BUILD @@ -37,6 +37,7 @@ grpc_cc_test( srcs = ["channel_args_test.cc"], external_deps = [ "absl/log:check", + "absl/log:log", "gtest", ], language = "C++", @@ -149,6 +150,7 @@ grpc_cc_test( name = "metrics_test", srcs = ["metrics_test.cc"], external_deps = [ + "absl/log:log", "gtest", ], language = "C++", diff --git a/test/core/channel/channel_args_test.cc b/test/core/channel/channel_args_test.cc index 3e59d01bfa0..1fe9f0937b1 100644 --- a/test/core/channel/channel_args_test.cc +++ b/test/core/channel/channel_args_test.cc @@ -21,6 +21,7 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" #include "gtest/gtest.h" #include @@ -28,7 +29,6 @@ #include #include #include -#include #include "src/core/lib/gpr/useful.h" #include "src/core/lib/gprpp/notification.h" @@ -272,7 +272,7 @@ struct fake_class { }; static void* fake_pointer_arg_copy(void* arg) { - gpr_log(GPR_DEBUG, "fake_pointer_arg_copy"); + VLOG(2) << "fake_pointer_arg_copy"; fake_class* fc = static_cast(arg); fake_class* new_fc = static_cast(gpr_malloc(sizeof(fake_class))); new_fc->foo = fc->foo; @@ -280,7 +280,7 @@ static void* fake_pointer_arg_copy(void* arg) { } static void fake_pointer_arg_destroy(void* arg) { - gpr_log(GPR_DEBUG, "fake_pointer_arg_destroy"); + VLOG(2) << "fake_pointer_arg_destroy"; fake_class* fc = static_cast(arg); gpr_free(fc); } diff --git a/test/core/channel/metrics_test.cc b/test/core/channel/metrics_test.cc index 20b915c7904..3d57b32d2ab 100644 --- a/test/core/channel/metrics_test.cc +++ b/test/core/channel/metrics_test.cc @@ -16,6 +16,7 @@ #include +#include "absl/log/log.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -217,7 +218,7 @@ TEST_F(MetricsTest, Int64CallbackGauge) { auto plugin3 = MakeStatsPluginForTarget(kDomain1To4); // Register two callbacks that set the same metric but with different // label values. The callbacks get used only by plugin1. - gpr_log(GPR_INFO, "testing callbacks for: plugin1"); + LOG(INFO) << "testing callbacks for: plugin1"; auto group1 = GlobalStatsPluginRegistry::GetStatsPluginsForChannel( StatsPluginChannelScope(kDomain3To4, "")); auto callback1 = group1.RegisterCallback( @@ -278,7 +279,7 @@ TEST_F(MetricsTest, Int64CallbackGauge) { callback1.reset(); callback2.reset(); // Now register callbacks that hit both plugin1 and plugin2. - gpr_log(GPR_INFO, "testing callbacks for: plugin1, plugin2"); + LOG(INFO) << "testing callbacks for: plugin1, plugin2"; auto group2 = GlobalStatsPluginRegistry::GetStatsPluginsForChannel( StatsPluginChannelScope(kDomain2To4, "")); callback1 = group2.RegisterCallback( @@ -339,7 +340,7 @@ TEST_F(MetricsTest, Int64CallbackGauge) { callback1.reset(); callback2.reset(); // Now register callbacks that hit all three plugins. - gpr_log(GPR_INFO, "testing callbacks for: plugin1, plugin2, plugin3"); + LOG(INFO) << "testing callbacks for: plugin1, plugin2, plugin3"; auto group3 = GlobalStatsPluginRegistry::GetStatsPluginsForChannel( StatsPluginChannelScope(kDomain1To4, "")); callback1 = group3.RegisterCallback( @@ -422,7 +423,7 @@ TEST_F(MetricsTest, DoubleCallbackGauge) { auto plugin3 = MakeStatsPluginForTarget(kDomain1To4); // Register two callbacks that set the same metric but with different // label values. The callbacks get used only by plugin1. - gpr_log(GPR_INFO, "testing callbacks for: plugin1"); + LOG(INFO) << "testing callbacks for: plugin1"; auto group1 = GlobalStatsPluginRegistry::GetStatsPluginsForChannel( StatsPluginChannelScope(kDomain3To4, "")); auto callback1 = group1.RegisterCallback( @@ -483,7 +484,7 @@ TEST_F(MetricsTest, DoubleCallbackGauge) { callback1.reset(); callback2.reset(); // Now register callbacks that hit both plugin1 and plugin2. - gpr_log(GPR_INFO, "testing callbacks for: plugin1, plugin2"); + LOG(INFO) << "testing callbacks for: plugin1, plugin2"; auto group2 = GlobalStatsPluginRegistry::GetStatsPluginsForChannel( StatsPluginChannelScope(kDomain2To4, "")); callback1 = group2.RegisterCallback( @@ -544,7 +545,7 @@ TEST_F(MetricsTest, DoubleCallbackGauge) { callback1.reset(); callback2.reset(); // Now register callbacks that hit all three plugins. - gpr_log(GPR_INFO, "testing callbacks for: plugin1, plugin2, plugin3"); + LOG(INFO) << "testing callbacks for: plugin1, plugin2, plugin3"; auto group3 = GlobalStatsPluginRegistry::GetStatsPluginsForChannel( StatsPluginChannelScope(kDomain1To4, "")); callback1 = group3.RegisterCallback( diff --git a/test/core/compression/BUILD b/test/core/compression/BUILD index d6825f63e41..21746ffd776 100644 --- a/test/core/compression/BUILD +++ b/test/core/compression/BUILD @@ -26,7 +26,10 @@ licenses(["notice"]) grpc_cc_test( name = "compression_test", srcs = ["compression_test.cc"], - external_deps = ["gtest"], + external_deps = [ + "absl/log:log", + "gtest", + ], language = "C++", uses_event_engine = False, uses_polling = False, @@ -67,7 +70,10 @@ grpc_fuzzer( grpc_cc_test( name = "message_compress_test", srcs = ["message_compress_test.cc"], - external_deps = ["gtest"], + external_deps = [ + "absl/log:log", + "gtest", + ], language = "C++", uses_event_engine = False, uses_polling = False, diff --git a/test/core/compression/compression_test.cc b/test/core/compression/compression_test.cc index 2297e5c8303..569a1d7e2ff 100644 --- a/test/core/compression/compression_test.cc +++ b/test/core/compression/compression_test.cc @@ -21,11 +21,11 @@ #include +#include "absl/log/log.h" #include "gtest/gtest.h" #include #include -#include #include "src/core/lib/gpr/useful.h" #include "test/core/test_util/test_config.h" @@ -40,7 +40,7 @@ TEST(CompressionTest, CompressionAlgorithmParse) { }; const char* invalid_names[] = {"gzip2", "foo", "", "2gzip"}; - gpr_log(GPR_DEBUG, "test_compression_algorithm_parse"); + VLOG(2) << "test_compression_algorithm_parse"; for (i = 0; i < GPR_ARRAY_SIZE(valid_names); i++) { const char* valid_name = valid_names[i]; @@ -73,7 +73,7 @@ TEST(CompressionTest, CompressionAlgorithmName) { GRPC_COMPRESS_DEFLATE, }; - gpr_log(GPR_DEBUG, "test_compression_algorithm_name"); + VLOG(2) << "test_compression_algorithm_name"; for (i = 0; i < GPR_ARRAY_SIZE(valid_algorithms); i++) { success = grpc_compression_algorithm_name(valid_algorithms[i], &name); @@ -88,7 +88,7 @@ TEST(CompressionTest, CompressionAlgorithmName) { } TEST(CompressionTest, CompressionAlgorithmForLevel) { - gpr_log(GPR_DEBUG, "test_compression_algorithm_for_level"); + VLOG(2) << "test_compression_algorithm_for_level"; { // accept only identity (aka none) @@ -211,7 +211,7 @@ TEST(CompressionTest, CompressionEnableDisableAlgorithm) { grpc_compression_options options; grpc_compression_algorithm algorithm; - gpr_log(GPR_DEBUG, "test_compression_enable_disable_algorithm"); + VLOG(2) << "test_compression_enable_disable_algorithm"; grpc_compression_options_init(&options); for (algorithm = GRPC_COMPRESS_NONE; diff --git a/test/core/compression/message_compress_test.cc b/test/core/compression/message_compress_test.cc index b3cf8ba7e47..5666ed3e477 100644 --- a/test/core/compression/message_compress_test.cc +++ b/test/core/compression/message_compress_test.cc @@ -28,7 +28,6 @@ #include #include -#include #include "src/core/lib/gpr/useful.h" #include "src/core/lib/iomgr/exec_ctx.h"