From c95886f65aca16b89dcafe11cd102928e8ef025c Mon Sep 17 00:00:00 2001 From: Tanvi Jagtap Date: Thu, 16 May 2024 02:15:01 -0700 Subject: [PATCH] [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. PiperOrigin-RevId: 634272059 --- src/core/lib/surface/call.cc | 2 +- src/core/lib/surface/channel_init.cc | 7 ++++--- src/core/lib/surface/completion_queue.cc | 9 +++++---- src/core/lib/surface/init.cc | 9 +++++---- src/core/resolver/binder/binder_resolver.cc | 5 +++-- src/core/resolver/dns/c_ares/dns_resolver_ares.cc | 3 ++- src/core/resolver/dns/c_ares/grpc_ares_wrapper.cc | 3 ++- src/core/resolver/dns/dns_resolver_plugin.cc | 9 +++++---- .../event_engine/event_engine_client_channel_resolver.cc | 3 ++- src/core/resolver/dns/native/dns_resolver.cc | 5 +++-- src/core/resolver/google_c2p/google_c2p_resolver.cc | 3 ++- src/core/server/server.cc | 5 +++-- .../service_config/service_config_channel_arg_filter.cc | 3 ++- 13 files changed, 39 insertions(+), 27 deletions(-) diff --git a/src/core/lib/surface/call.cc b/src/core/lib/surface/call.cc index cd0a1ed7dd0..cd9a15d0bd6 100644 --- a/src/core/lib/surface/call.cc +++ b/src/core/lib/surface/call.cc @@ -325,7 +325,7 @@ void Call::HandleCompressionAlgorithmDisabled( grpc_compression_algorithm_name(compression_algorithm, &algo_name); std::string error_msg = absl::StrFormat("Compression algorithm '%s' is disabled.", algo_name); - gpr_log(GPR_ERROR, "%s", error_msg.c_str()); + LOG(ERROR) << error_msg; CancelWithError(grpc_error_set_int(absl::UnimplementedError(error_msg), StatusIntProperty::kRpcStatus, GRPC_STATUS_UNIMPLEMENTED)); diff --git a/src/core/lib/surface/channel_init.cc b/src/core/lib/surface/channel_init.cc index 698a57b195d..da694aa59be 100644 --- a/src/core/lib/surface/channel_init.cc +++ b/src/core/lib/surface/channel_init.cc @@ -27,6 +27,7 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_join.h" #include "absl/strings/string_view.h" @@ -313,7 +314,7 @@ ChannelInit::StackConfig ChannelInit::BuildStackConfig( const auto filter_str = absl::StrCat(" ", loc_strs[filter.filter], NameFromChannelFilter(filter.filter), after_str); - gpr_log(GPR_INFO, "%s", filter_str.c_str()); + LOG(INFO) << filter_str; } // Finally list out the terminal filters and where they were registered // from. @@ -325,7 +326,7 @@ ChannelInit::StackConfig ChannelInit::BuildStackConfig( strlen(NameFromChannelFilter(terminal.filter)), ' '), "[terminal]"); - gpr_log(GPR_INFO, "%s", filter_str.c_str()); + LOG(INFO) << filter_str; } } // Check if there are no terminal filters: this would be an error. @@ -398,7 +399,7 @@ bool ChannelInit::CreateStack(ChannelStackBuilder* builder) const { "\n"); } } - gpr_log(GPR_ERROR, "%s", error.c_str()); + LOG(ERROR) << error; return false; } for (const auto& post_processor : stack_config.post_processors) { diff --git a/src/core/lib/surface/completion_queue.cc b/src/core/lib/surface/completion_queue.cc index 92b52c4216e..b439191f1bb 100644 --- a/src/core/lib/surface/completion_queue.cc +++ b/src/core/lib/surface/completion_queue.cc @@ -28,6 +28,7 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" #include "absl/status/status.h" #include "absl/strings/str_format.h" #include "absl/strings/str_join.h" @@ -260,7 +261,7 @@ struct cq_next_data { CHECK_EQ(queue.num_items(), 0); #ifndef NDEBUG if (pending_events.load(std::memory_order_acquire) != 0) { - gpr_log(GPR_ERROR, "Destroying CQ without draining it fully."); + LOG(ERROR) << "Destroying CQ without draining it fully."; } #endif } @@ -290,7 +291,7 @@ struct cq_pluck_data { CHECK(completed_head.next == reinterpret_cast(&completed_head)); #ifndef NDEBUG if (pending_events.load(std::memory_order_acquire) != 0) { - gpr_log(GPR_ERROR, "Destroying CQ without draining it fully."); + LOG(ERROR) << "Destroying CQ without draining it fully."; } #endif } @@ -327,7 +328,7 @@ struct cq_callback_data { ~cq_callback_data() { #ifndef NDEBUG if (pending_events.load(std::memory_order_acquire) != 0) { - gpr_log(GPR_ERROR, "Destroying CQ without draining it fully."); + LOG(ERROR) << "Destroying CQ without draining it fully."; } #endif } @@ -942,7 +943,7 @@ static void dump_pending_tags(grpc_completion_queue* cq) { parts.push_back(absl::StrFormat(" %p", cq->outstanding_tags[i])); } gpr_mu_unlock(cq->mu); - gpr_log(GPR_DEBUG, "%s", absl::StrJoin(parts, "").c_str()); + VLOG(2) << absl::StrJoin(parts, ""); } #else static void dump_pending_tags(grpc_completion_queue* /*cq*/) {} diff --git a/src/core/lib/surface/init.cc b/src/core/lib/surface/init.cc index b2a766ac8a0..fc8e75484d2 100644 --- a/src/core/lib/surface/init.cc +++ b/src/core/lib/surface/init.cc @@ -19,6 +19,7 @@ #include "src/core/lib/surface/init.h" #include "absl/base/thread_annotations.h" +#include "absl/log/log.h" #include #include @@ -137,7 +138,7 @@ void grpc_shutdown_from_cleanup_thread(void* /*ignored*/) { return; } grpc_shutdown_internal_locked(); - gpr_log(GPR_DEBUG, "grpc_shutdown from cleanup thread done"); + VLOG(2) << "grpc_shutdown from cleanup thread done"; } void grpc_shutdown(void) { @@ -155,14 +156,14 @@ void grpc_shutdown(void) { 0) && grpc_core::ExecCtx::Get() == nullptr) { // just run clean-up when this is called on non-executor thread. - gpr_log(GPR_DEBUG, "grpc_shutdown starts clean-up now"); + VLOG(2) << "grpc_shutdown starts clean-up now"; g_shutting_down = true; grpc_shutdown_internal_locked(); - gpr_log(GPR_DEBUG, "grpc_shutdown done"); + VLOG(2) << "grpc_shutdown done"; } else { // spawn a detached thread to do the actual clean up in case we are // currently in an executor thread. - gpr_log(GPR_DEBUG, "grpc_shutdown spawns clean-up thread"); + VLOG(2) << "grpc_shutdown spawns clean-up thread"; g_initializations++; g_shutting_down = true; grpc_core::Thread cleanup_thread( diff --git a/src/core/resolver/binder/binder_resolver.cc b/src/core/resolver/binder/binder_resolver.cc index 903a5528470..7cd03f29193 100644 --- a/src/core/resolver/binder/binder_resolver.cc +++ b/src/core/resolver/binder/binder_resolver.cc @@ -37,6 +37,7 @@ #include #include +#include "absl/log/log.h" #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" @@ -126,12 +127,12 @@ class BinderResolverFactory final : public ResolverFactory { grpc_resolved_address addr; { if (!uri.authority().empty()) { - gpr_log(GPR_ERROR, "authority is not supported in binder scheme"); + LOG(ERROR) << "authority is not supported in binder scheme"; return false; } grpc_error_handle error = BinderAddrPopulate(uri.path(), &addr); if (!error.ok()) { - gpr_log(GPR_ERROR, "%s", StatusToString(error).c_str()); + LOG(ERROR) << StatusToString(error); return false; } } diff --git a/src/core/resolver/dns/c_ares/dns_resolver_ares.cc b/src/core/resolver/dns/c_ares/dns_resolver_ares.cc index 4fa22539ba1..06b9985c948 100644 --- a/src/core/resolver/dns/c_ares/dns_resolver_ares.cc +++ b/src/core/resolver/dns/c_ares/dns_resolver_ares.cc @@ -24,6 +24,7 @@ #include #include "absl/base/thread_annotations.h" +#include "absl/log/log.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/match.h" @@ -348,7 +349,7 @@ class AresClientChannelDNSResolverFactory final : public ResolverFactory { bool IsValidUri(const URI& uri) const override { if (absl::StripPrefix(uri.path(), "/").empty()) { - gpr_log(GPR_ERROR, "no server name supplied in dns URI"); + LOG(ERROR) << "no server name supplied in dns URI"; return false; } return true; diff --git a/src/core/resolver/dns/c_ares/grpc_ares_wrapper.cc b/src/core/resolver/dns/c_ares/grpc_ares_wrapper.cc index e1f717cb9d4..7994a1900a5 100644 --- a/src/core/resolver/dns/c_ares/grpc_ares_wrapper.cc +++ b/src/core/resolver/dns/c_ares/grpc_ares_wrapper.cc @@ -19,6 +19,7 @@ #include #include +#include "absl/log/log.h" #include "absl/strings/string_view.h" #include @@ -952,7 +953,7 @@ static bool resolve_as_ip_literal_locked( static bool target_matches_localhost_inner(const char* name, std::string* host, std::string* port) { if (!grpc_core::SplitHostPort(name, host, port)) { - gpr_log(GPR_ERROR, "Unable to split host and port for name: %s", name); + LOG(ERROR) << "Unable to split host and port for name: " << name; return false; } return gpr_stricmp(host->c_str(), "localhost") == 0; diff --git a/src/core/resolver/dns/dns_resolver_plugin.cc b/src/core/resolver/dns/dns_resolver_plugin.cc index a0f92a6c600..122c3fd740e 100644 --- a/src/core/resolver/dns/dns_resolver_plugin.cc +++ b/src/core/resolver/dns/dns_resolver_plugin.cc @@ -15,6 +15,7 @@ #include +#include "absl/log/log.h" #include "absl/strings/match.h" #include @@ -32,14 +33,14 @@ namespace grpc_core { void RegisterDnsResolver(CoreConfiguration::Builder* builder) { #ifdef GRPC_IOS_EVENT_ENGINE_CLIENT - gpr_log(GPR_DEBUG, "Using EventEngine dns resolver"); + VLOG(2) << "Using EventEngine dns resolver"; builder->resolver_registry()->RegisterResolverFactory( std::make_unique()); return; #endif #ifndef GRPC_DO_NOT_INSTANTIATE_POSIX_POLLER if (IsEventEngineDnsEnabled()) { - gpr_log(GPR_DEBUG, "Using EventEngine dns resolver"); + VLOG(2) << "Using EventEngine dns resolver"; builder->resolver_registry()->RegisterResolverFactory( std::make_unique()); return; @@ -48,14 +49,14 @@ void RegisterDnsResolver(CoreConfiguration::Builder* builder) { auto resolver = ConfigVars::Get().DnsResolver(); // ---- Ares resolver ---- if (ShouldUseAresDnsResolver(resolver)) { - gpr_log(GPR_DEBUG, "Using ares dns resolver"); + VLOG(2) << "Using ares dns resolver"; RegisterAresDnsResolver(builder); return; } // ---- Native resolver ---- if (absl::EqualsIgnoreCase(resolver, "native") || !builder->resolver_registry()->HasResolverFactory("dns")) { - gpr_log(GPR_DEBUG, "Using native dns resolver"); + VLOG(2) << "Using native dns resolver"; RegisterNativeDnsResolver(builder); return; } diff --git a/src/core/resolver/dns/event_engine/event_engine_client_channel_resolver.cc b/src/core/resolver/dns/event_engine/event_engine_client_channel_resolver.cc index 8a9d1a49db9..743422ab603 100644 --- a/src/core/resolver/dns/event_engine/event_engine_client_channel_resolver.cc +++ b/src/core/resolver/dns/event_engine/event_engine_client_channel_resolver.cc @@ -26,6 +26,7 @@ #include "absl/base/thread_annotations.h" #include "absl/cleanup/cleanup.h" #include "absl/log/check.h" +#include "absl/log/log.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/match.h" @@ -567,7 +568,7 @@ absl::optional EventEngineClientChannelDNSResolver:: bool EventEngineClientChannelDNSResolverFactory::IsValidUri( const URI& uri) const { if (absl::StripPrefix(uri.path(), "/").empty()) { - gpr_log(GPR_ERROR, "no server name supplied in dns URI"); + LOG(ERROR) << "no server name supplied in dns URI"; return false; } return true; diff --git a/src/core/resolver/dns/native/dns_resolver.cc b/src/core/resolver/dns/native/dns_resolver.cc index 43dddc719a4..8ecb92d4857 100644 --- a/src/core/resolver/dns/native/dns_resolver.cc +++ b/src/core/resolver/dns/native/dns_resolver.cc @@ -20,6 +20,7 @@ #include #include "absl/functional/bind_front.h" +#include "absl/log/log.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" @@ -150,11 +151,11 @@ class NativeClientChannelDNSResolverFactory final : public ResolverFactory { bool IsValidUri(const URI& uri) const override { if (GPR_UNLIKELY(!uri.authority().empty())) { - gpr_log(GPR_ERROR, "authority based dns uri's not supported"); + LOG(ERROR) << "authority based dns uri's not supported"; return false; } if (absl::StripPrefix(uri.path(), "/").empty()) { - gpr_log(GPR_ERROR, "no server name supplied in dns URI"); + LOG(ERROR) << "no server name supplied in dns URI"; return false; } return true; diff --git a/src/core/resolver/google_c2p/google_c2p_resolver.cc b/src/core/resolver/google_c2p/google_c2p_resolver.cc index 0fb3744a3e8..d58e9a81b3e 100644 --- a/src/core/resolver/google_c2p/google_c2p_resolver.cc +++ b/src/core/resolver/google_c2p/google_c2p_resolver.cc @@ -22,6 +22,7 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" @@ -282,7 +283,7 @@ class GoogleCloud2ProdResolverFactory final : public ResolverFactory { bool IsValidUri(const URI& uri) const override { if (GPR_UNLIKELY(!uri.authority().empty())) { - gpr_log(GPR_ERROR, "google-c2p URI scheme does not support authorities"); + LOG(ERROR) << "google-c2p URI scheme does not support authorities"; return false; } return true; diff --git a/src/core/server/server.cc b/src/core/server/server.cc index e66c3c5d232..b891001f3d4 100644 --- a/src/core/server/server.cc +++ b/src/core/server/server.cc @@ -33,6 +33,7 @@ #include "absl/cleanup/cleanup.h" #include "absl/container/flat_hash_map.h" #include "absl/log/check.h" +#include "absl/log/log.h" #include "absl/status/status.h" #include "absl/types/optional.h" @@ -980,7 +981,7 @@ grpc_error_handle Server::SetupTransport( } t->StartConnectivityWatch(MakeOrphanable( t->RefAsSubclass(), Ref())); - gpr_log(GPR_INFO, "Adding connection"); + LOG(INFO) << "Adding connection"; connections_.emplace(std::move(t)); ++connections_open_; } else { @@ -1494,7 +1495,7 @@ void Server::ChannelData::Destroy() { GRPC_CLOSURE_INIT(&finish_destroy_channel_closure_, FinishDestroy, this, grpc_schedule_on_exec_ctx); if (GRPC_TRACE_FLAG_ENABLED(grpc_server_channel_trace)) { - gpr_log(GPR_INFO, "Disconnected client"); + LOG(INFO) << "Disconnected client"; } grpc_transport_op* op = grpc_make_transport_op(&finish_destroy_channel_closure_); diff --git a/src/core/service_config/service_config_channel_arg_filter.cc b/src/core/service_config/service_config_channel_arg_filter.cc index c73e9dd450e..9afa99018fc 100644 --- a/src/core/service_config/service_config_channel_arg_filter.cc +++ b/src/core/service_config/service_config_channel_arg_filter.cc @@ -22,6 +22,7 @@ #include #include +#include "absl/log/log.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/types/optional.h" @@ -69,7 +70,7 @@ class ServiceConfigChannelArgFilter final auto service_config = ServiceConfigImpl::Create(args, *service_config_str); if (!service_config.ok()) { - gpr_log(GPR_ERROR, "%s", service_config.status().ToString().c_str()); + LOG(ERROR) << service_config.status().ToString(); } else { service_config_ = std::move(*service_config); }