From b57c7e9fc3bf66f0dc7ee4636c5b3dc254746113 Mon Sep 17 00:00:00 2001 From: Tanvi Jagtap <139093547+tanvi-jagtap@users.noreply.github.com> Date: Sat, 15 Jun 2024 05:14:03 -0700 Subject: [PATCH] [grpc][Gpr_To_Absl_Logging] Migrating from gpr to absl logging - gpr_log (#36879) [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 #36879 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36879 from tanvi-jagtap:test_folder_gpr_log 40eeeaf987875bda793ecc1def8acc532bb7bb1b PiperOrigin-RevId: 643593896 --- .../parse_address_with_named_scope_id_test.cc | 4 ++-- test/core/client_channel/BUILD | 5 ++++- test/core/client_channel/client_channel_test.cc | 3 ++- test/core/end2end/fixtures/http_proxy_fixture.cc | 2 +- test/core/iomgr/fd_posix_test.cc | 12 ++++-------- test/core/slice/slice_test.cc | 10 +++++++--- 6 files changed, 20 insertions(+), 16 deletions(-) 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 eb76adef43d..19b21f35027 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 @@ -51,7 +51,7 @@ static void test_grpc_parse_ipv6_parity_with_getaddrinfo( grpc_core::ExecCtx exec_ctx; absl::StatusOr uri = grpc_core::URI::Parse(target); 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; @@ -75,7 +75,7 @@ static void test_grpc_parse_ipv6_parity_with_getaddrinfo( struct sockaddr_in6 resolve_with_gettaddrinfo(const char* uri_text) { 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(); EXPECT_TRUE(uri.ok()); } std::string host; diff --git a/test/core/client_channel/BUILD b/test/core/client_channel/BUILD index eaeccd3b414..fab2857ce6c 100644 --- a/test/core/client_channel/BUILD +++ b/test/core/client_channel/BUILD @@ -57,7 +57,10 @@ grpc_yodel_simple_test( grpc_yodel_simple_test( name = "load_balanced_call_destination", srcs = ["load_balanced_call_destination_test.cc"], - external_deps = ["gtest"], + external_deps = [ + "absl/log:log", + "gtest", + ], deps = [ "//:grpc_client_channel", "//test/core/call/yodel:yodel_test", diff --git a/test/core/client_channel/client_channel_test.cc b/test/core/client_channel/client_channel_test.cc index a280eae8a71..4c8b2919207 100644 --- a/test/core/client_channel/client_channel_test.cc +++ b/test/core/client_channel/client_channel_test.cc @@ -17,6 +17,7 @@ #include #include +#include "absl/log/log.h" #include "absl/strings/string_view.h" #include "gtest/gtest.h" @@ -118,7 +119,7 @@ class ClientChannelTest : public YodelTest { RefCountedPtr CreateSubchannel( const grpc_resolved_address& address, const ChannelArgs& args) override { - gpr_log(GPR_INFO, "CreateSubchannel: args=%s", args.ToString().c_str()); + LOG(INFO) << "CreateSubchannel: args=" << args.ToString(); return Subchannel::Create(MakeOrphanable(), address, args); } }; diff --git a/test/core/end2end/fixtures/http_proxy_fixture.cc b/test/core/end2end/fixtures/http_proxy_fixture.cc index 4875b2591b3..f0ffe5519a5 100644 --- a/test/core/end2end/fixtures/http_proxy_fixture.cc +++ b/test/core/end2end/fixtures/http_proxy_fixture.cc @@ -208,7 +208,7 @@ static void on_read_request_done_locked(void* arg, grpc_error_handle error); static void proxy_connection_failed(proxy_connection* conn, failure_type failure, const char* prefix, grpc_error_handle error) { - gpr_log(GPR_INFO, "%s: %s", prefix, grpc_core::StatusToString(error).c_str()); + LOG(INFO) << prefix << ": " << grpc_core::StatusToString(error); // Decide whether we should shut down the client and server. bool shutdown_client = false; bool shutdown_server = false; diff --git a/test/core/iomgr/fd_posix_test.cc b/test/core/iomgr/fd_posix_test.cc index 8497b62591a..8395b75dec2 100644 --- a/test/core/iomgr/fd_posix_test.cc +++ b/test/core/iomgr/fd_posix_test.cc @@ -164,8 +164,7 @@ static void session_read_cb(void* arg, // session // before notify_on_read is called. grpc_fd_notify_on_read(se->em_fd, &se->session_read_closure); } else { - grpc_core::Crash(absl::StrFormat("Unhandled read error %s", - grpc_core::StrError(errno).c_str())); + LOG(FATAL) << "Unhandled read error " << grpc_core::StrError(errno); } } } @@ -330,8 +329,7 @@ static void client_session_write(void* arg, // client } gpr_mu_unlock(g_mu); } else { - grpc_core::Crash(absl::StrFormat("unknown errno %s", - grpc_core::StrError(errno).c_str())); + LOG(FATAL) << "unknown errno " << grpc_core::StrError(errno).c_str(); } } @@ -348,12 +346,10 @@ static void client_start(client* cl, int port) { pfd.events = POLLOUT; pfd.revents = 0; if (poll(&pfd, 1, -1) == -1) { - gpr_log(GPR_ERROR, "poll() failed during connect; errno=%d", errno); - abort(); + LOG(FATAL) << "poll() failed during connect; errno=" << errno; } } else { - grpc_core::Crash( - absl::StrFormat("Failed to connect to the server (errno=%d)", errno)); + LOG(FATAL) << "Failed to connect to the server (errno=" << errno << ")"; } } diff --git a/test/core/slice/slice_test.cc b/test/core/slice/slice_test.cc index 44c139d8c07..3b1977145bb 100644 --- a/test/core/slice/slice_test.cc +++ b/test/core/slice/slice_test.cc @@ -33,7 +33,6 @@ #include "gtest/gtest.h" #include -#include #include #include "src/core/lib/gprpp/memory.h" @@ -395,14 +394,19 @@ size_t SumSlice(const Slice& slice) { TEST(SliceTest, ExternalAsOwned) { auto external_string = std::make_unique(RandomString(1024)); Slice slice = Slice::FromExternalString(*external_string); - const auto initial_sum = SumSlice(slice); + const size_t initial_sum = SumSlice(slice); Slice owned = slice.AsOwned(); EXPECT_EQ(initial_sum, SumSlice(owned)); external_string.reset(); // In ASAN (where we can be sure that it'll crash), go ahead and read the // bytes we just deleted. if (BuiltUnderAsan()) { - ASSERT_DEATH({ gpr_log(GPR_DEBUG, "%" PRIdPTR, SumSlice(slice)); }, ""); + ASSERT_DEATH( + { + size_t sum = SumSlice(slice); + VLOG(2) << sum; + }, + ""); } EXPECT_EQ(initial_sum, SumSlice(owned)); }