Skip to content

Commit

Permalink
[grpc][Gpr_To_Absl_Logging] Migrating from gpr to absl logging - gpr_…
Browse files Browse the repository at this point in the history
…log (grpc#36701)

[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 grpc#36701

COPYBARA_INTEGRATE_REVIEW=grpc#36701 from tanvi-jagtap:test_core_gpr_log 1d8c69e
PiperOrigin-RevId: 636850577
  • Loading branch information
tanvi-jagtap authored and copybara-github committed May 24, 2024
1 parent 0ed0950 commit 8881bde
Show file tree
Hide file tree
Showing 28 changed files with 151 additions and 171 deletions.
12 changes: 6 additions & 6 deletions test/core/compression/message_compress_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include <memory>

#include "absl/log/log.h"
#include "gtest/gtest.h"

#include <grpc/compression.h>
Expand Down Expand Up @@ -56,12 +57,11 @@ static void assert_passthrough(grpc_slice value,
const char* algorithm_name;

ASSERT_NE(grpc_compression_algorithm_name(algorithm, &algorithm_name), 0);
gpr_log(GPR_INFO,
"assert_passthrough: value_length=%" PRIuPTR
" algorithm='%s' uncompressed_split='%s' compressed_split='%s'",
GRPC_SLICE_LENGTH(value), algorithm_name,
grpc_slice_split_mode_name(uncompressed_split_mode),
grpc_slice_split_mode_name(compressed_split_mode));
LOG(INFO) << "assert_passthrough: value_length=" << GRPC_SLICE_LENGTH(value)
<< " algorithm='" << algorithm_name << "' uncompressed_split='"
<< grpc_slice_split_mode_name(uncompressed_split_mode)
<< "' compressed_split='"
<< grpc_slice_split_mode_name(compressed_split_mode) << "'";

grpc_slice_buffer_init(&input);
grpc_slice_buffer_init(&compressed_raw);
Expand Down
13 changes: 6 additions & 7 deletions test/core/event_engine/event_engine_test_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <utility>

#include "absl/log/check.h"
#include "absl/log/log.h"
#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "absl/strings/str_cat.h"
Expand All @@ -34,7 +35,6 @@
#include <grpc/event_engine/slice.h>
#include <grpc/event_engine/slice_buffer.h>
#include <grpc/slice_buffer.h>
#include <grpc/support/log.h>

#include "src/core/lib/event_engine/channel_args_endpoint_config.h"
#include "src/core/lib/event_engine/tcp_socket_utils.h"
Expand Down Expand Up @@ -159,8 +159,8 @@ absl::Status SendValidatePayload(absl::string_view data,
// Check if data written == data read
std::string data_read = ExtractSliceBufferIntoString(&read_store_buf);
if (data != data_read) {
gpr_log(GPR_INFO, "Data written = %s", data.data());
gpr_log(GPR_INFO, "Data read = %s", data_read.c_str());
LOG(INFO) << "Data written = " << data;
LOG(INFO) << "Data read = " << data_read;
return absl::CancelledError("Data read != Data written");
}
return absl::OkStatus();
Expand Down Expand Up @@ -201,8 +201,8 @@ absl::Status ConnectionManager::BindAndStartListener(
for (auto& addr : addrs) {
auto bind_status = listener->Bind(*URIToResolvedAddress(addr));
if (!bind_status.ok()) {
gpr_log(GPR_ERROR, "Binding listener failed: %s",
bind_status.status().ToString().c_str());
LOG(ERROR) << "Binding listener failed: "
<< bind_status.status().ToString();
return bind_status.status();
}
}
Expand Down Expand Up @@ -230,8 +230,7 @@ ConnectionManager::CreateConnection(std::string target_addr,
event_engine->Connect(
[this](absl::StatusOr<std::unique_ptr<EventEngine::Endpoint>> status) {
if (!status.ok()) {
gpr_log(GPR_ERROR, "Connect failed: %s",
status.status().ToString().c_str());
LOG(ERROR) << "Connect failed: " << status.status().ToString();
last_in_progress_connection_.SetClientEndpoint(nullptr);
} else {
last_in_progress_connection_.SetClientEndpoint(std::move(*status));
Expand Down
9 changes: 4 additions & 5 deletions test/core/event_engine/forkable_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,10 @@

#include <memory>

#include "absl/log/log.h"
#include "absl/types/optional.h"
#include "gtest/gtest.h"

#include <grpc/support/log.h>

#include "src/core/lib/config/config_vars.h"
#include "src/core/lib/gprpp/no_destruct.h"

Expand Down Expand Up @@ -94,14 +93,14 @@ TEST_F(ForkableTest, BasicPthreadAtForkOperations) {
int child_pid = fork();
ASSERT_NE(child_pid, -1);
if (child_pid == 0) {
gpr_log(GPR_DEBUG, "I am child pid: %d", getpid());
VLOG(2) << "I am child pid: " << getpid();
forkable->CheckChild();
exit(testing::Test::HasFailure());
} else {
gpr_log(GPR_DEBUG, "I am parent pid: %d", getpid());
VLOG(2) << "I am parent pid: " << getpid();
forkable->CheckParent();
int status;
gpr_log(GPR_DEBUG, "Waiting for child pid: %d", child_pid);
VLOG(2) << "Waiting for child pid: " << child_pid;
do {
// retry on EINTR, and fail otherwise
if (waitpid(child_pid, &status, 0) != -1) break;
Expand Down
10 changes: 5 additions & 5 deletions test/core/event_engine/posix/event_poller_posix_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@
#include <sys/socket.h>
#include <unistd.h>

#include "absl/log/log.h"
#include "absl/status/status.h"

#include <grpc/support/alloc.h>
#include <grpc/support/log.h>
#include <grpc/support/sync.h>

#include "src/core/lib/event_engine/common_closures.h"
Expand Down Expand Up @@ -235,8 +235,8 @@ void ListenCb(server* sv, absl::Status status) {
listen_em_fd->NotifyOnRead(sv->listen_closure);
return;
} else if (fd < 0) {
gpr_log(GPR_ERROR, "Failed to acceot a connection, returned error: %s",
grpc_core::StrError(errno).c_str());
LOG(ERROR) << "Failed to acceot a connection, returned error: "
<< grpc_core::StrError(errno);
}
EXPECT_GE(fd, 0);
EXPECT_LT(fd, FD_SETSIZE);
Expand Down Expand Up @@ -349,7 +349,7 @@ void ClientStart(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);
LOG(ERROR) << "poll() failed during connect; errno=" << errno;
abort();
}
} else {
Expand Down Expand Up @@ -389,7 +389,7 @@ class EventPollerTest : public ::testing::Test {
EXPECT_NE(engine_, nullptr);
scheduler_->ChangeCurrentEventEngine(engine_.get());
if (g_event_poller != nullptr) {
gpr_log(GPR_INFO, "Using poller: %s", g_event_poller->Name().c_str());
LOG(INFO) << "Using poller: " << g_event_poller->Name();
}
}

Expand Down
3 changes: 2 additions & 1 deletion test/core/event_engine/posix/posix_endpoint_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <vector>

#include "absl/log/check.h"
#include "absl/log/log.h"
#include "absl/status/statusor.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/str_split.h"
Expand Down Expand Up @@ -203,7 +204,7 @@ class PosixEndpointTest : public ::testing::TestWithParam<bool> {
EXPECT_NE(posix_ee_, nullptr);
scheduler_->ChangeCurrentEventEngine(posix_ee_.get());
if (poller_ != nullptr) {
gpr_log(GPR_INFO, "Using poller: %s", poller_->Name().c_str());
LOG(INFO) << "Using poller: " << poller_->Name();
}
}

Expand Down
13 changes: 6 additions & 7 deletions test/core/event_engine/posix/posix_engine_listener_utils_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@

#include <ifaddrs.h>

#include <grpc/support/log.h>
#include "absl/log/log.h"

#include "src/core/lib/event_engine/channel_args_endpoint_config.h"
#include "src/core/lib/event_engine/posix_engine/posix_engine_listener_utils.h"
Expand Down Expand Up @@ -108,9 +108,8 @@ TEST(PosixEngineListenerUtils, ListenerContainerAddAllLocalAddressesTest) {
struct ifaddrs* ifa_it;
if (getifaddrs(&ifa) != 0 || ifa == nullptr) {
// No ifaddresses available.
gpr_log(GPR_INFO,
"Skipping ListenerAddAllLocalAddressesTest because the machine "
"does not have interfaces configured for listening.");
LOG(INFO) << "Skipping ListenerAddAllLocalAddressesTest because the "
"machine does not have interfaces configured for listening.";
return;
}
int num_ifaddrs = 0;
Expand All @@ -123,9 +122,9 @@ TEST(PosixEngineListenerUtils, ListenerContainerAddAllLocalAddressesTest) {
if (num_ifaddrs == 0 || !result.ok()) {
// Its possible that the machine may not have any Ipv4/Ipv6 interfaces
// configured for listening. In that case, dont fail test.
gpr_log(GPR_INFO,
"Skipping ListenerAddAllLocalAddressesTest because the machine "
"does not have Ipv6/Ipv6 interfaces configured for listening.");
LOG(INFO) << "Skipping ListenerAddAllLocalAddressesTest because the "
"machine does not have Ipv6/Ipv6 interfaces configured for "
"listening.";
return;
}
// Some sockets have been created and bound to interfaces on the machiene.
Expand Down
5 changes: 2 additions & 3 deletions test/core/event_engine/posix/posix_engine_test_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,9 @@
#include <stdlib.h>
#include <sys/socket.h>

#include "absl/log/log.h"
#include "absl/strings/str_format.h"

#include <grpc/support/log.h>

#include "src/core/lib/gprpp/crash.h"

namespace grpc_event_engine {
Expand Down Expand Up @@ -53,7 +52,7 @@ int ConnectToServerOrDie(const ResolvedAddress& server_address) {
pfd.events = POLLOUT;
pfd.revents = 0;
if (poll(&pfd, 1, -1) == -1) {
gpr_log(GPR_ERROR, "poll() failed during connect; errno=%d", errno);
LOG(ERROR) << "poll() failed during connect; errno=" << errno;
abort();
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <vector>

#include "absl/log/check.h"
#include "absl/log/log.h"
#include "absl/memory/memory.h"
#include "absl/status/status.h"
#include "absl/status/statusor.h"
Expand All @@ -37,7 +38,6 @@
#include <grpc/event_engine/event_engine.h>
#include <grpc/grpc.h>
#include <grpc/impl/channel_arg_names.h>
#include <grpc/support/log.h>

#include "src/core/lib/channel/channel_args.h"
#include "src/core/lib/event_engine/channel_args_endpoint_config.h"
Expand Down Expand Up @@ -121,7 +121,7 @@ std::vector<int> CreateConnectedSockets(
pfd.revents = 0;
int ret = poll(&pfd, 1, 1000);
if (ret == -1) {
gpr_log(GPR_ERROR, "poll() failed during connect; errno=%d", errno);
LOG(ERROR) << "poll() failed during connect; errno=" << errno;
abort();
} else if (ret == 0) {
// current connection attempt timed out. It indicates that the
Expand Down
6 changes: 3 additions & 3 deletions test/core/event_engine/posix/timer_manager_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@
#include <random>

#include "absl/functional/any_invocable.h"
#include "absl/log/log.h"
#include "absl/time/clock.h"
#include "absl/time/time.h"
#include "gtest/gtest.h"

#include <grpc/grpc.h>
#include <grpc/support/log.h>

#include "src/core/lib/event_engine/common_closures.h"
#include "src/core/lib/event_engine/posix_engine/timer.h"
Expand Down Expand Up @@ -66,8 +66,8 @@ TEST(TimerManagerTest, StressTest) {
<< called.load(std::memory_order_relaxed) << "/" << kTimerCount
<< " callbacks executed";
}
gpr_log(GPR_DEBUG, "Processed %d/%d callbacks",
called.load(std::memory_order_relaxed), kTimerCount);
VLOG(2) << "Processed " << called.load(std::memory_order_relaxed) << "/"
<< kTimerCount << " callbacks";
absl::SleepFor(absl::Milliseconds(333));
}
}
Expand Down
6 changes: 3 additions & 3 deletions test/core/event_engine/test_suite/tests/dns_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <tuple>
#include <vector>

#include "absl/log/log.h"
#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "absl/strings/str_format.h"
Expand Down Expand Up @@ -128,9 +129,8 @@ class EventEngineDNSTest : public EventEngineTest {
// an indication whether the test is running on RBE or not. Find a better way of
// doing this.
#ifndef GRPC_PORT_ISOLATED_RUNTIME
gpr_log(GPR_ERROR,
"You are invoking the test locally with Bazel, you may need to "
"invoke Bazel with --enable_runfiles=yes.");
LOG(ERROR) << "You are invoking the test locally with Bazel, you may "
"need to invoke Bazel with --enable_runfiles=yes.";
#endif // GRPC_PORT_ISOLATED_RUNTIME
test_records_path = grpc::testing::NormalizeFilePath(test_records_path);
dns_server_path =
Expand Down
6 changes: 3 additions & 3 deletions test/core/event_engine/test_suite/tests/timer_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@
#include "absl/functional/any_invocable.h"
#include "absl/functional/bind_front.h"
#include "absl/functional/function_ref.h"
#include "absl/log/log.h"
#include "absl/time/clock.h"
#include "absl/time/time.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"

#include <grpc/event_engine/event_engine.h>
#include <grpc/support/log.h>

#include "src/core/lib/event_engine/time_util.h"
#include "src/core/lib/gprpp/sync.h"
Expand Down Expand Up @@ -197,8 +197,8 @@ TEST_F(EventEngineTimerTest, StressTestTimersNotCalledBeforeScheduled) {
cv_.Wait(&mu_);
}
if (failed_call_count.load() != 0) {
gpr_log(GPR_DEBUG, "failed timer count: %d of %d", failed_call_count.load(),
thread_count * call_count);
VLOG(2) << "failed timer count: " << failed_call_count.load() << " of "
<< (thread_count * call_count);
}
ASSERT_EQ(0, failed_call_count.load());
}
Expand Down
21 changes: 9 additions & 12 deletions test/core/event_engine/test_suite/tools/echo_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@
#include "absl/flags/flag.h"
#include "absl/flags/parse.h"
#include "absl/functional/any_invocable.h"
#include "absl/log/log.h"
#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "absl/strings/str_format.h"

#include <grpc/event_engine/event_engine.h>
#include <grpc/event_engine/slice_buffer.h>
#include <grpc/grpc.h>
#include <grpc/support/log.h>

#include "src/core/lib/channel/channel_args_preconditioning.h"
#include "src/core/lib/config/core_configuration.h"
Expand Down Expand Up @@ -93,14 +93,12 @@ void ReceiveAndEchoMessage(EventEngine::Endpoint* endpoint, int message_id) {
endpoint->Read(
[&](absl::Status status) {
if (!status.ok()) {
gpr_log(GPR_ERROR, "Error reading from endpoint: %s",
status.ToString().c_str());
LOG(ERROR) << "Error reading from endpoint: " << status;
exit(1);
}
Slice received = buf.TakeFirst();
gpr_log(GPR_ERROR, "Received message %d: %.*s", message_id,
static_cast<int>(received.as_string_view().length()),
received.as_string_view().data());
LOG(ERROR) << "Received message " << message_id << ": "
<< received.as_string_view();
read_done.Notify();
},
&buf, nullptr);
Expand All @@ -124,8 +122,7 @@ void RunUntilInterrupted() {
engine->Connect(
[&](absl::StatusOr<std::unique_ptr<EventEngine::Endpoint>> ep) {
if (!ep.ok()) {
gpr_log(GPR_ERROR, "Error connecting: %s",
ep.status().ToString().c_str());
LOG(ERROR) << "Error connecting: " << ep.status().ToString();
exit(1);
}
endpoint = std::move(*ep);
Expand All @@ -134,10 +131,10 @@ void RunUntilInterrupted() {
*addr, config, memory_quota->CreateMemoryAllocator("client"), 2h);
connected.WaitForNotification();
CHECK_NE(endpoint.get(), nullptr);
gpr_log(GPR_DEBUG, "peer addr: %s",
ResolvedAddressToString(endpoint->GetPeerAddress())->c_str());
gpr_log(GPR_DEBUG, "local addr: %s",
ResolvedAddressToString(endpoint->GetLocalAddress())->c_str());
VLOG(2) << "peer addr: "
<< ResolvedAddressToString(endpoint->GetPeerAddress());
VLOG(2) << "local addr: "
<< ResolvedAddressToString(endpoint->GetLocalAddress());
int message_id = 0;
while (true) {
SendMessage(endpoint.get(), message_id++);
Expand Down
Loading

0 comments on commit 8881bde

Please sign in to comment.