From befeeba0f57c6ed3608935d8317fd26289e7e080 Mon Sep 17 00:00:00 2001 From: Tanvi Jagtap <139093547+tanvi-jagtap@users.noreply.github.com> Date: Tue, 14 May 2024 19:14:20 -0700 Subject: [PATCH] [grpc][Gpr_To_Absl_Logging] Migrating from gpr to absl logging - gpr_log (#36594) [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 #36594 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36594 from tanvi-jagtap:regex_src_cpp d9c21d6456a79bf489262aeebec090988ab7b743 PiperOrigin-RevId: 633777020 --- src/cpp/client/secure_credentials.cc | 5 +- src/cpp/common/alts_util.cc | 10 ++-- src/cpp/common/completion_queue_cc.cc | 5 +- src/cpp/ext/gcp/observability_logging_sink.cc | 32 ++++++------- .../get_cpu_stats_unsupported.cc | 6 +-- src/cpp/server/load_reporter/util.cc | 5 +- src/cpp/server/server_builder.cc | 47 +++++++++---------- src/cpp/server/server_cc.cc | 9 ++-- src/cpp/thread_manager/thread_manager.cc | 5 +- 9 files changed, 58 insertions(+), 66 deletions(-) diff --git a/src/cpp/client/secure_credentials.cc b/src/cpp/client/secure_credentials.cc index 3b48e20dcf6..dabe0ce79ad 100644 --- a/src/cpp/client/secure_credentials.cc +++ b/src/cpp/client/secure_credentials.cc @@ -25,6 +25,7 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/str_join.h" @@ -35,7 +36,6 @@ #include #include #include -#include #include #include #include @@ -273,8 +273,7 @@ std::shared_ptr ServiceAccountJWTAccessCredentials( const std::string& json_key, long token_lifetime_seconds) { grpc::internal::GrpcLibrary init; // To call grpc_init(). if (token_lifetime_seconds <= 0) { - gpr_log(GPR_ERROR, - "Trying to create JWTCredentials with non-positive lifetime"); + LOG(ERROR) << "Trying to create JWTCredentials with non-positive lifetime"; return WrapCallCredentials(nullptr); } gpr_timespec lifetime = diff --git a/src/cpp/common/alts_util.cc b/src/cpp/common/alts_util.cc index 784b0157c42..df1a8a8cf5b 100644 --- a/src/cpp/common/alts_util.cc +++ b/src/cpp/common/alts_util.cc @@ -21,10 +21,10 @@ #include #include +#include "absl/log/log.h" #include "upb/mem/arena.hpp" #include -#include #include #include #include @@ -40,25 +40,25 @@ namespace experimental { std::unique_ptr GetAltsContextFromAuthContext( const std::shared_ptr& auth_context) { if (auth_context == nullptr) { - gpr_log(GPR_ERROR, "auth_context is nullptr."); + LOG(ERROR) << "auth_context is nullptr."; return nullptr; } std::vector ctx_vector = auth_context->FindPropertyValues(TSI_ALTS_CONTEXT); if (ctx_vector.size() != 1) { - gpr_log(GPR_ERROR, "contains zero or more than one ALTS context."); + LOG(ERROR) << "contains zero or more than one ALTS context."; return nullptr; } upb::Arena context_arena; grpc_gcp_AltsContext* ctx = grpc_gcp_AltsContext_parse( ctx_vector[0].data(), ctx_vector[0].size(), context_arena.ptr()); if (ctx == nullptr) { - gpr_log(GPR_ERROR, "fails to parse ALTS context."); + LOG(ERROR) << "fails to parse ALTS context."; return nullptr; } if (grpc_gcp_AltsContext_security_level(ctx) < GRPC_SECURITY_MIN || grpc_gcp_AltsContext_security_level(ctx) > GRPC_SECURITY_MAX) { - gpr_log(GPR_ERROR, "security_level is invalid."); + LOG(ERROR) << "security_level is invalid."; return nullptr; } return std::make_unique(AltsContext(ctx)); diff --git a/src/cpp/common/completion_queue_cc.cc b/src/cpp/common/completion_queue_cc.cc index 9c0b7d31bc0..231fb4469e3 100644 --- a/src/cpp/common/completion_queue_cc.cc +++ b/src/cpp/common/completion_queue_cc.cc @@ -19,10 +19,10 @@ #include "absl/base/thread_annotations.h" #include "absl/log/check.h" +#include "absl/log/log.h" #include #include -#include #include #include #include @@ -134,8 +134,7 @@ CompletionQueue::CompletionQueue(grpc_completion_queue* take) void CompletionQueue::Shutdown() { #ifndef NDEBUG if (!ServerListEmpty()) { - gpr_log(GPR_ERROR, - "CompletionQueue shutdown being shutdown before its server."); + LOG(ERROR) << "CompletionQueue shutdown being shutdown before its server."; } #endif CompleteAvalanching(); diff --git a/src/cpp/ext/gcp/observability_logging_sink.cc b/src/cpp/ext/gcp/observability_logging_sink.cc index 6d62c1e4079..e7f71d4f0e8 100644 --- a/src/cpp/ext/gcp/observability_logging_sink.cc +++ b/src/cpp/ext/gcp/observability_logging_sink.cc @@ -22,6 +22,7 @@ #include #include +#include "absl/log/log.h" #include "absl/numeric/int128.h" #include "absl/strings/escaping.h" #include "absl/strings/match.h" @@ -34,7 +35,6 @@ #include "google/protobuf/text_format.h" #include -#include #include #include #include @@ -360,22 +360,20 @@ void ObservabilityLoggingSink::FlushEntriesHelper( &(call->context), &(call->request), &(call->response), [this, call](Status status) { if (!status.ok()) { - gpr_log( - GPR_ERROR, - "GCP Observability Logging Error %d: %s. Dumping log entries.", - status.error_code(), status.error_message().c_str()); + LOG(ERROR) << "GCP Observability Logging Error " + << status.error_code() << ": " << status.error_message() + << ". Dumping log entries."; for (auto& entry : call->request.entries()) { std::string output; ::google::protobuf::TextFormat::PrintToString(entry.json_payload(), &output); - gpr_log( - GPR_INFO, "Log Entry recorded at time: %s : %s", - grpc_core::Timestamp::FromTimespecRoundUp( - gpr_timespec{entry.timestamp().seconds(), - entry.timestamp().nanos(), GPR_CLOCK_REALTIME}) - .ToString() - .c_str(), - output.c_str()); + LOG(INFO) << "Log Entry recorded at time: " + << grpc_core::Timestamp::FromTimespecRoundUp( + gpr_timespec{entry.timestamp().seconds(), + entry.timestamp().nanos(), + GPR_CLOCK_REALTIME}) + .ToString() + << " : " << output; } } delete call; @@ -414,16 +412,16 @@ void ObservabilityLoggingSink::MaybeTriggerFlushLocked() { if (entries_.empty()) return; if (entries_.size() > kMaxEntriesBeforeDump || entries_memory_footprint_ > kMaxMemoryFootprintBeforeDump) { - // Buffer limits have been reached. Dump entries with gpr_log - gpr_log(GPR_INFO, "Buffer limit reached. Dumping log entries."); + // Buffer limits have been reached. Dump entries with LOG + LOG(INFO) << "Buffer limit reached. Dumping log entries."; for (auto& entry : entries_) { google::protobuf::Struct proto; std::string timestamp = entry.timestamp.ToString(); EntryToJsonStructProto(std::move(entry), &proto); std::string output; ::google::protobuf::TextFormat::PrintToString(proto, &output); - gpr_log(GPR_INFO, "Log Entry recorded at time: %s : %s", - timestamp.c_str(), output.c_str()); + LOG(INFO) << "Log Entry recorded at time: " << timestamp << " : " + << output; } entries_.clear(); entries_memory_footprint_ = 0; diff --git a/src/cpp/server/load_reporter/get_cpu_stats_unsupported.cc b/src/cpp/server/load_reporter/get_cpu_stats_unsupported.cc index 83734b9c9d7..2744c66b25e 100644 --- a/src/cpp/server/load_reporter/get_cpu_stats_unsupported.cc +++ b/src/cpp/server/load_reporter/get_cpu_stats_unsupported.cc @@ -20,7 +20,7 @@ #if !defined(GPR_LINUX) && !defined(GPR_WINDOWS) && !defined(GPR_APPLE) -#include +#include "absl/log/log.h" #include "src/core/lib/gprpp/crash.h" #include "src/cpp/server/load_reporter/get_cpu_stats.h" @@ -30,8 +30,8 @@ namespace load_reporter { std::pair GetCpuStatsImpl() { uint64_t busy = 0, total = 0; - gpr_log(GPR_ERROR, - "Platforms other than Linux, Windows, and MacOS are not supported."); + LOG(ERROR) + << "Platforms other than Linux, Windows, and MacOS are not supported."; return std::make_pair(busy, total); } diff --git a/src/cpp/server/load_reporter/util.cc b/src/cpp/server/load_reporter/util.cc index 8daac576d73..043d4ebb480 100644 --- a/src/cpp/server/load_reporter/util.cc +++ b/src/cpp/server/load_reporter/util.cc @@ -21,8 +21,9 @@ #include #include +#include "absl/log/log.h" + #include -#include #include #include #include @@ -41,7 +42,7 @@ void AddLoadReportingCost(grpc::ServerContext* ctx, cost_name.size()); ctx->AddTrailingMetadata(GRPC_LB_COST_MD_KEY, buf); } else { - gpr_log(GPR_ERROR, "Call metric value is not normal."); + LOG(ERROR) << "Call metric value is not normal."; } } diff --git a/src/cpp/server/server_builder.cc b/src/cpp/server/server_builder.cc index b9f71915981..7f96f6c96cf 100644 --- a/src/cpp/server/server_builder.cc +++ b/src/cpp/server/server_builder.cc @@ -26,11 +26,11 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" #include #include #include -#include #include #include #include @@ -139,10 +139,9 @@ ServerBuilder& ServerBuilder::RegisterService(const std::string& host, ServerBuilder& ServerBuilder::RegisterAsyncGenericService( AsyncGenericService* service) { if (generic_service_ || callback_generic_service_) { - gpr_log(GPR_ERROR, - "Adding multiple generic services is unsupported for now. " - "Dropping the service %p", - service); + LOG(ERROR) << "Adding multiple generic services is unsupported for now. " + "Dropping the service " + << service; } else { generic_service_ = service; } @@ -152,10 +151,9 @@ ServerBuilder& ServerBuilder::RegisterAsyncGenericService( ServerBuilder& ServerBuilder::RegisterCallbackGenericService( CallbackGenericService* service) { if (generic_service_ || callback_generic_service_) { - gpr_log(GPR_ERROR, - "Adding multiple generic services is unsupported for now. " - "Dropping the service %p", - service); + LOG(ERROR) << "Adding multiple generic services is unsupported for now. " + "Dropping the service " + << service; } else { callback_generic_service_ = service; } @@ -391,16 +389,16 @@ std::unique_ptr ServerBuilder::BuildAndStart() { if (has_sync_methods) { // This is a Sync server - gpr_log(GPR_INFO, - "Synchronous server. Num CQs: %d, Min pollers: %d, Max Pollers: " - "%d, CQ timeout (msec): %d", - sync_server_settings_.num_cqs, sync_server_settings_.min_pollers, - sync_server_settings_.max_pollers, - sync_server_settings_.cq_timeout_msec); + LOG(INFO) << "Synchronous server. Num CQs: " + << sync_server_settings_.num_cqs + << ", Min pollers: " << sync_server_settings_.min_pollers + << ", Max Pollers: " << sync_server_settings_.max_pollers + << ", CQ timeout (msec): " + << sync_server_settings_.cq_timeout_msec; } if (has_callback_methods) { - gpr_log(GPR_INFO, "Callback server."); + LOG(INFO) << "Callback server."; } std::unique_ptr server(new grpc::Server( @@ -445,22 +443,22 @@ std::unique_ptr ServerBuilder::BuildAndStart() { if (passive_listener != nullptr) { auto* creds = unstarted_listener.credentials->c_creds(); if (creds == nullptr) { - gpr_log(GPR_ERROR, "Credentials missing for PassiveListener"); + LOG(ERROR) << "Credentials missing for PassiveListener"; return nullptr; } auto success = grpc_server_add_passive_listener( core_server, creds, std::move(passive_listener)); if (!success.ok()) { - gpr_log(GPR_ERROR, "Failed to create a passive listener: %s", - success.ToString().c_str()); + LOG(ERROR) << "Failed to create a passive listener: " + << success.ToString(); return nullptr; } } } if (!has_frequently_polled_cqs) { - gpr_log(GPR_ERROR, - "At least one of the completion queues must be frequently polled"); + LOG(ERROR) + << "At least one of the completion queues must be frequently polled"; return nullptr; } @@ -483,9 +481,8 @@ std::unique_ptr ServerBuilder::BuildAndStart() { } else { for (const auto& value : services_) { if (value->service->has_generic_methods()) { - gpr_log(GPR_ERROR, - "Some methods were marked generic but there is no " - "generic service registered."); + LOG(ERROR) << "Some methods were marked generic but there is no " + "generic service registered."; return nullptr; } } @@ -525,7 +522,7 @@ ServerBuilder& ServerBuilder::EnableWorkaround(grpc_workaround_list id) { case GRPC_WORKAROUND_ID_CRONET_COMPRESSION: return AddChannelArgument(GRPC_ARG_WORKAROUND_CRONET_COMPRESSION, 1); default: - gpr_log(GPR_ERROR, "Workaround %u does not exist or is obsolete.", id); + LOG(ERROR) << "Workaround " << id << " does not exist or is obsolete."; return *this; } } diff --git a/src/cpp/server/server_cc.cc b/src/cpp/server/server_cc.cc index ff3c3c7bdb3..099b8ca2141 100644 --- a/src/cpp/server/server_cc.cc +++ b/src/cpp/server/server_cc.cc @@ -30,13 +30,13 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" #include "absl/status/status.h" #include #include #include #include -#include #include #include #include @@ -446,7 +446,7 @@ class Server::SyncRequest final : public grpc::internal::CompletionQueueTag { deserialized_request_ = handler->Deserialize(call_, request_payload_, &request_status_, nullptr); if (!request_status_.ok()) { - gpr_log(GPR_DEBUG, "Failed to deserialize message."); + VLOG(2) << "Failed to deserialize message."; } request_payload_ = nullptr; interceptor_methods_.AddInterceptionHookPoint( @@ -689,7 +689,7 @@ class Server::CallbackRequest final req_->call_, req_->request_payload_, &req_->request_status_, &req_->handler_data_); if (!(req_->request_status_.ok())) { - gpr_log(GPR_DEBUG, "Failed to deserialize message."); + VLOG(2) << "Failed to deserialize message."; } req_->request_payload_ = nullptr; req_->interceptor_methods_.AddInterceptionHookPoint( @@ -1058,8 +1058,7 @@ bool Server::RegisterService(const std::string* addr, grpc::Service* service) { server_, method->name(), addr ? addr->c_str() : nullptr, PayloadHandlingForMethod(method.get()), 0); if (method_registration_tag == nullptr) { - gpr_log(GPR_DEBUG, "Attempt to register %s multiple times", - method->name()); + VLOG(2) << "Attempt to register " << method->name() << " multiple times"; return false; } diff --git a/src/cpp/thread_manager/thread_manager.cc b/src/cpp/thread_manager/thread_manager.cc index 1264bb3102e..1fd3ee20bc8 100644 --- a/src/cpp/thread_manager/thread_manager.cc +++ b/src/cpp/thread_manager/thread_manager.cc @@ -21,10 +21,9 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" #include "absl/strings/str_format.h" -#include - #include "src/core/lib/gprpp/crash.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/thd.h" @@ -41,7 +40,7 @@ ThreadManager::WorkerThread::WorkerThread(ThreadManager* thd_mgr) [](void* th) { static_cast(th)->Run(); }, this, &created_); if (!created_) { - gpr_log(GPR_ERROR, "Could not create grpc_sync_server worker-thread"); + LOG(ERROR) << "Could not create grpc_sync_server worker-thread"; } }