Skip to content

Commit

Permalink
[export] [grpc][Gpr_To_Absl_Logging] Migrating from gpr to absl loggi…
Browse files Browse the repository at this point in the history
…ng - 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.

----
DO NOT SUBMIT. This PR is for testing purposes only. [cl/633802449](http://cl/633802449)

PiperOrigin-RevId: 633802449
  • Loading branch information
tanvi-jagtap authored and Copybara-Service committed May 15, 2024
1 parent befeeba commit 80e19b8
Show file tree
Hide file tree
Showing 15 changed files with 90 additions and 93 deletions.
3 changes: 2 additions & 1 deletion src/core/ext/transport/binder/client/channel_create.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#ifdef GPR_SUPPORT_BINDER_TRANSPORT

#include "absl/log/check.h"
#include "absl/log/log.h"
#include "absl/memory/memory.h"
#include "absl/strings/substitute.h"
#include "absl/time/clock.h"
Expand Down Expand Up @@ -111,7 +112,7 @@ std::shared_ptr<grpc::Channel> CreateCustomBinderChannel(
std::string connection_id =
grpc_binder::GetConnectionIdGenerator()->Generate(uri);

gpr_log(GPR_ERROR, "connection id is %s", connection_id.c_str());
LOG(ERROR) << "connection id is " << connection_id;

// After invoking this Java method, Java code will put endpoint binder into
// `EndpointBinderPool` after the connection succeeds
Expand Down
17 changes: 8 additions & 9 deletions src/core/ext/transport/binder/client/endpoint_binder_pool.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "src/core/ext/transport/binder/client/endpoint_binder_pool.h"

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

#include <grpc/support/port_platform.h>

Expand All @@ -36,11 +37,11 @@ Java_io_grpc_binder_cpp_GrpcBinderConnection_notifyConnected__Ljava_lang_String_
JNIEnv* jni_env, jobject, jstring conn_id_jstring, jobject ibinder) {
jboolean isCopy;
const char* conn_id = jni_env->GetStringUTFChars(conn_id_jstring, &isCopy);
gpr_log(GPR_INFO, "%s invoked with conn_id = %s", __func__, conn_id);
LOG(INFO) << __func__ << " invoked with conn_id = " << conn_id;
CHECK_NE(ibinder, nullptr);
grpc_binder::ndk_util::SpAIBinder aibinder =
grpc_binder::FromJavaBinder(jni_env, ibinder);
gpr_log(GPR_INFO, "%s got aibinder = %p", __func__, aibinder.get());
LOG(INFO) << __func__ << " got aibinder = " << aibinder.get();
auto b = std::make_unique<grpc_binder::BinderAndroid>(aibinder);
CHECK(b != nullptr);
grpc_binder::GetEndpointBinderPool()->AddEndpointBinder(conn_id,
Expand All @@ -58,7 +59,7 @@ namespace grpc_binder {
void EndpointBinderPool::GetEndpointBinder(
std::string conn_id,
std::function<void(std::unique_ptr<grpc_binder::Binder>)> cb) {
gpr_log(GPR_INFO, "EndpointBinder requested. conn_id = %s", conn_id.c_str());
LOG(INFO) << "EndpointBinder requested. conn_id = " << conn_id;
std::unique_ptr<grpc_binder::Binder> b;
{
grpc_core::MutexLock l(&m_);
Expand All @@ -68,9 +69,8 @@ void EndpointBinderPool::GetEndpointBinder(
CHECK(b != nullptr);
} else {
if (pending_requests_.count(conn_id) != 0) {
gpr_log(GPR_ERROR,
"Duplicate GetEndpointBinder requested. conn_id = %s",
conn_id.c_str());
LOG(ERROR) << "Duplicate GetEndpointBinder requested. conn_id = "
<< conn_id;
return;
}
pending_requests_[conn_id] = std::move(cb);
Expand All @@ -83,15 +83,14 @@ void EndpointBinderPool::GetEndpointBinder(

void EndpointBinderPool::AddEndpointBinder(
std::string conn_id, std::unique_ptr<grpc_binder::Binder> b) {
gpr_log(GPR_INFO, "EndpointBinder added. conn_id = %s", conn_id.c_str());
LOG(INFO) << "EndpointBinder added. conn_id = " << conn_id;
CHECK(b != nullptr);
// cb will be set in the following block if there is a pending callback
std::function<void(std::unique_ptr<grpc_binder::Binder>)> cb = nullptr;
{
grpc_core::MutexLock l(&m_);
if (binder_map_.count(conn_id) != 0) {
gpr_log(GPR_ERROR, "EndpointBinder already in the pool. conn_id = %s",
conn_id.c_str());
LOG(ERROR) << "EndpointBinder already in the pool. conn_id = " << conn_id;
return;
}
if (pending_requests_.count(conn_id)) {
Expand Down
7 changes: 4 additions & 3 deletions src/core/ext/transport/binder/client/jni_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "src/core/ext/transport/binder/client/jni_utils.h"

#include "absl/log/check.h"
#include "absl/log/log.h" // IWYU pragma: keep

#include <grpc/support/port_platform.h>

Expand Down Expand Up @@ -83,7 +84,7 @@ void TryEstablishConnection(JNIEnv* env, jobject application,

jmethodID mid = env->GetStaticMethodID(cl, method.c_str(), type.c_str());
if (mid == nullptr) {
gpr_log(GPR_ERROR, "No method id %s", method.c_str());
LOG(ERROR) << "No method id " << method;
}

env->CallStaticVoidMethod(cl, mid, application,
Expand All @@ -107,7 +108,7 @@ void TryEstablishConnectionWithUri(JNIEnv* env, jobject application,

jmethodID mid = env->GetStaticMethodID(cl, method.c_str(), type.c_str());
if (mid == nullptr) {
gpr_log(GPR_ERROR, "No method id %s", method.c_str());
LOG(ERROR) << "No method id " << method;
}

env->CallStaticVoidMethod(cl, mid, application,
Expand All @@ -126,7 +127,7 @@ bool IsSignatureMatch(JNIEnv* env, jobject context, int uid1, int uid2) {

jmethodID mid = env->GetStaticMethodID(cl, method.c_str(), type.c_str());
if (mid == nullptr) {
gpr_log(GPR_ERROR, "No method id %s", method.c_str());
LOG(ERROR) << "No method id " << method;
}

jboolean result = env->CallStaticBooleanMethod(cl, mid, context, uid1, uid2);
Expand Down
6 changes: 3 additions & 3 deletions src/core/ext/transport/binder/transport/binder_transport.cc
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ static void recv_trailing_metadata_locked(void* arg,
// Append status to metadata
// TODO(b/192208695): See if we can avoid to manually put status
// code into the header
gpr_log(GPR_INFO, "status = %d", args->status);
LOG(INFO) << "status = " << args->status;
stream->recv_trailing_metadata->Set(
grpc_core::GrpcStatusMetadata(),
static_cast<grpc_status_code>(args->status));
Expand Down Expand Up @@ -350,7 +350,7 @@ class MetadataEncoder {
}

void Encode(grpc_core::GrpcStatusMetadata, grpc_status_code status) {
gpr_log(GPR_INFO, "send trailing metadata status = %d", status);
LOG(INFO) << "send trailing metadata status = " << status;
tx_->SetStatus(status);
}

Expand Down Expand Up @@ -395,7 +395,7 @@ static void perform_stream_op_locked(void* stream_op,
CHECK(!op->send_initial_metadata && !op->send_message &&
!op->send_trailing_metadata && !op->recv_initial_metadata &&
!op->recv_message && !op->recv_trailing_metadata);
gpr_log(GPR_INFO, "cancel_stream is_client = %d", stream->is_client);
LOG(INFO) << "cancel_stream is_client = " << stream->is_client;
if (!stream->is_client) {
// Send trailing metadata to inform the other end about the cancellation,
// regardless if we'd already done that or not.
Expand Down
11 changes: 6 additions & 5 deletions src/core/ext/transport/binder/utils/ndk_binder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <dlfcn.h>

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

#include <grpc/support/log.h>

Expand Down Expand Up @@ -60,10 +61,10 @@ void SetJvm(JNIEnv* env) {
JavaVM* jvm = nullptr;
jint error = env->GetJavaVM(&jvm);
if (error != JNI_OK) {
gpr_log(GPR_ERROR, "Failed to get JVM");
LOG(ERROR) << "Failed to get JVM";
}
g_jvm = jvm;
gpr_log(GPR_INFO, "JVM cached");
LOG(INFO) << "JVM cached";
}

// `SetJvm` need to be called in the process before `AttachJvm`. This is always
Expand All @@ -77,14 +78,14 @@ bool AttachJvm() {
// Note: The following code would be run at most once per thread.
grpc_core::MutexLock lock(&g_jvm_mu);
if (g_jvm == nullptr) {
gpr_log(GPR_ERROR, "JVM not cached yet");
LOG(ERROR) << "JVM not cached yet";
return false;
}
JNIEnv* env_unused;
// Note that attach a thread that is already attached is a no-op, so it is
// fine to call this again if the thread has already been attached by other.
g_jvm->AttachCurrentThread(&env_unused, /* thr_args= */ nullptr);
gpr_log(GPR_INFO, "JVM attached successfully");
LOG(INFO) << "JVM attached successfully";
g_is_jvm_attached = true;
return true;
}
Expand Down Expand Up @@ -151,7 +152,7 @@ binder_status_t AIBinder_transact(AIBinder* binder, transaction_code_t code,
AParcel** in, AParcel** out,
binder_flags_t flags) {
if (!AttachJvm()) {
gpr_log(GPR_ERROR, "failed to attach JVM. AIBinder_transact might fail.");
LOG(ERROR) << "failed to attach JVM. AIBinder_transact might fail.";
}
FORWARD(AIBinder_transact)(binder, code, in, out, flags);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@
#include <utility>

#include "absl/log/check.h"

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

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

Expand All @@ -36,7 +35,7 @@ const absl::string_view

void TransportStreamReceiverImpl::RegisterRecvInitialMetadata(
StreamIdentifier id, InitialMetadataCallbackType cb) {
gpr_log(GPR_INFO, "%s id = %d is_client = %d", __func__, id, is_client_);
LOG(INFO) << __func__ << " id = " << id << " is_client = " << is_client_;
absl::StatusOr<Metadata> initial_metadata{};
{
grpc_core::MutexLock l(&m_);
Expand Down Expand Up @@ -64,7 +63,7 @@ void TransportStreamReceiverImpl::RegisterRecvInitialMetadata(

void TransportStreamReceiverImpl::RegisterRecvMessage(
StreamIdentifier id, MessageDataCallbackType cb) {
gpr_log(GPR_INFO, "%s id = %d is_client = %d", __func__, id, is_client_);
LOG(INFO) << __func__ << " id = " << id << " is_client = " << is_client_;
absl::StatusOr<std::string> message{};
{
grpc_core::MutexLock l(&m_);
Expand Down Expand Up @@ -98,7 +97,7 @@ void TransportStreamReceiverImpl::RegisterRecvMessage(

void TransportStreamReceiverImpl::RegisterRecvTrailingMetadata(
StreamIdentifier id, TrailingMetadataCallbackType cb) {
gpr_log(GPR_INFO, "%s id = %d is_client = %d", __func__, id, is_client_);
LOG(INFO) << __func__ << " id = " << id << " is_client = " << is_client_;
std::pair<absl::StatusOr<Metadata>, int> trailing_metadata{};
{
grpc_core::MutexLock l(&m_);
Expand All @@ -122,7 +121,7 @@ void TransportStreamReceiverImpl::RegisterRecvTrailingMetadata(

void TransportStreamReceiverImpl::NotifyRecvInitialMetadata(
StreamIdentifier id, absl::StatusOr<Metadata> initial_metadata) {
gpr_log(GPR_INFO, "%s id = %d is_client = %d", __func__, id, is_client_);
LOG(INFO) << __func__ << " id = " << id << " is_client = " << is_client_;
if (!is_client_ && accept_stream_callback_ && initial_metadata.ok()) {
accept_stream_callback_();
}
Expand All @@ -143,7 +142,7 @@ void TransportStreamReceiverImpl::NotifyRecvInitialMetadata(

void TransportStreamReceiverImpl::NotifyRecvMessage(
StreamIdentifier id, absl::StatusOr<std::string> message) {
gpr_log(GPR_INFO, "%s id = %d is_client = %d", __func__, id, is_client_);
LOG(INFO) << __func__ << " id = " << id << " is_client = " << is_client_;
MessageDataCallbackType cb;
{
grpc_core::MutexLock l(&m_);
Expand All @@ -166,7 +165,7 @@ void TransportStreamReceiverImpl::NotifyRecvTrailingMetadata(
// assumes in-order commitments of transactions and that trailing metadata is
// parsed after message data, we can safely cancel all upcoming callbacks of
// recv_message.
gpr_log(GPR_INFO, "%s id = %d is_client = %d", __func__, id, is_client_);
LOG(INFO) << __func__ << " id = " << id << " is_client = " << is_client_;
OnRecvTrailingMetadata(id);
TrailingMetadataCallbackType cb;
{
Expand Down Expand Up @@ -233,7 +232,7 @@ void TransportStreamReceiverImpl::CancelTrailingMetadataCallback(
}

void TransportStreamReceiverImpl::OnRecvTrailingMetadata(StreamIdentifier id) {
gpr_log(GPR_INFO, "%s id = %d is_client = %d", __func__, id, is_client_);
LOG(INFO) << __func__ << " id = " << id << " is_client = " << is_client_;
m_.Lock();
trailing_metadata_recvd_.insert(id);
m_.Unlock();
Expand All @@ -245,7 +244,7 @@ void TransportStreamReceiverImpl::OnRecvTrailingMetadata(StreamIdentifier id) {
}

void TransportStreamReceiverImpl::CancelStream(StreamIdentifier id) {
gpr_log(GPR_INFO, "%s id = %d is_client = %d", __func__, id, is_client_);
LOG(INFO) << __func__ << " id = " << id << " is_client = " << is_client_;
CancelInitialMetadataCallback(id, absl::CancelledError("Stream cancelled"));
CancelMessageCallback(id, absl::CancelledError("Stream cancelled"));
CancelTrailingMetadataCallback(id, absl::CancelledError("Stream cancelled"));
Expand Down
5 changes: 3 additions & 2 deletions src/core/ext/transport/binder/wire_format/binder_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <map>

#include "absl/log/check.h"
#include "absl/log/log.h"
#include "absl/memory/memory.h"
#include "absl/strings/str_cat.h"

Expand Down Expand Up @@ -65,7 +66,7 @@ ndk_util::binder_status_t f_onTransact(ndk_util::AIBinder* binder,
const ndk_util::AParcel* in,
ndk_util::AParcel* /*out*/) {
gpr_log(GPR_INFO, __func__);
gpr_log(GPR_INFO, "tx code = %u", code);
LOG(INFO) << "tx code = " << code;

auto* user_data =
static_cast<BinderUserData*>(ndk_util::AIBinder_getUserData(binder));
Expand All @@ -79,7 +80,7 @@ ndk_util::binder_status_t f_onTransact(ndk_util::AIBinder* binder,
if (status.ok()) {
return ndk_util::STATUS_OK;
} else {
gpr_log(GPR_ERROR, "Callback failed: %s", status.ToString().c_str());
LOG(ERROR) << "Callback failed: " << status.ToString();
return ndk_util::STATUS_UNKNOWN_ERROR;
}
}
Expand Down
25 changes: 10 additions & 15 deletions src/core/ext/transport/binder/wire_format/wire_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#include "absl/cleanup/cleanup.h"
#include "absl/log/check.h"
#include "absl/log/log.h"
#include "absl/types/variant.h"

#include <grpc/support/log.h>
Expand Down Expand Up @@ -78,7 +79,7 @@ absl::Status WriteTrailingMetadata(const Transaction& tx,
} else {
// client suffix currently is always empty according to the wireformat
if (!tx.GetSuffixMetadata().empty()) {
gpr_log(GPR_ERROR, "Got non-empty suffix metadata from client.");
LOG(ERROR) << "Got non-empty suffix metadata from client.";
}
}
return absl::OkStatus();
Expand Down Expand Up @@ -218,8 +219,7 @@ void WireWriterImpl::RunScheduledTxInternal(RunScheduledTxArgs* args) {
return absl::OkStatus();
});
if (!result.ok()) {
gpr_log(GPR_ERROR, "Failed to make binder transaction %s",
result.ToString().c_str());
LOG(ERROR) << "Failed to make binder transaction " << result;
}
delete args;
return;
Expand All @@ -242,8 +242,7 @@ void WireWriterImpl::RunScheduledTxInternal(RunScheduledTxArgs* args) {
if (CanBeSentInOneTransaction(*stream_tx->tx.get())) { // NOLINT
absl::Status result = RpcCallFastPath(std::move(stream_tx->tx));
if (!result.ok()) {
gpr_log(GPR_ERROR, "Failed to handle non-chunked RPC call %s",
result.ToString().c_str());
LOG(ERROR) << "Failed to handle non-chunked RPC call " << result;
}
delete args;
return;
Expand All @@ -256,8 +255,7 @@ void WireWriterImpl::RunScheduledTxInternal(RunScheduledTxArgs* args) {
return RunStreamTx(stream_tx, parcel, &is_last_chunk);
});
if (!result.ok()) {
gpr_log(GPR_ERROR, "Failed to make binder transaction %s",
result.ToString().c_str());
LOG(ERROR) << "Failed to make binder transaction " << result;
}
if (!is_last_chunk) {
{
Expand Down Expand Up @@ -290,18 +288,16 @@ absl::Status WireWriterImpl::SendAck(int64_t num_bytes) {
// Ensure combiner will be run if this is not called from top-level gRPC API
// entrypoint.
grpc_core::ExecCtx exec_ctx;
gpr_log(GPR_INFO, "Ack %" PRId64 " bytes received", num_bytes);
LOG(INFO) << "Ack " << num_bytes << " bytes received";
if (is_transacting_) {
// This can happen because NDK might call our registered callback function
// in the same thread while we are telling it to send a transaction
// `is_transacting_` will be true. `Binder::Transact` is now being called on
// the same thread or the other thread. We are currently in the call stack
// of other transaction, Liveness of ACK is still guaranteed even if this is
// a race with another thread.
gpr_log(
GPR_INFO,
"Scheduling ACK transaction instead of directly execute it to avoid "
"deadlock.");
LOG(INFO) << "Scheduling ACK transaction instead of directly execute it to "
"avoid deadlock.";
auto args = new RunScheduledTxArgs();
args->writer = this;
args->tx = RunScheduledTxArgs::AckTx();
Expand All @@ -318,8 +314,7 @@ absl::Status WireWriterImpl::SendAck(int64_t num_bytes) {
return absl::OkStatus();
});
if (!result.ok()) {
gpr_log(GPR_ERROR, "Failed to make binder transaction %s",
result.ToString().c_str());
LOG(ERROR) << "Failed to make binder transaction " << result;
}
return result;
}
Expand All @@ -328,7 +323,7 @@ void WireWriterImpl::OnAckReceived(int64_t num_bytes) {
// Ensure combiner will be run if this is not called from top-level gRPC API
// entrypoint.
grpc_core::ExecCtx exec_ctx;
gpr_log(GPR_INFO, "OnAckReceived %" PRId64, num_bytes);
LOG(INFO) << "OnAckReceived " << num_bytes;
// Do not try to obtain `write_mu_` in this function. NDKBinder might invoke
// the callback to notify us about new incoming binder transaction when we are
// sending transaction. i.e. `write_mu_` might have already been acquired by
Expand Down

0 comments on commit 80e19b8

Please sign in to comment.