Skip to content

Commit

Permalink
[Gpr_To_Absl_Logging] Migrating from gpr to absl logging - gpr_log (g…
Browse files Browse the repository at this point in the history 
…rpc#37092)

[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#37092

COPYBARA_INTEGRATE_REVIEW=grpc#37092 from tanvi-jagtap:src_core_handshaker 297b2bd
PiperOrigin-RevId: 647964631
  • Loading branch information
@tanvi-jagtap @copybara-github
tanvi-jagtap authored and copybara-github committed Jun 29, 2024
1 parent 49a730e commit cfb1c61
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 61 deletions.
36 changes: 15 additions & 21 deletions src/core/handshaker/handshaker.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#include "absl/functional/any_invocable.h"
#include "absl/log/check.h"
#include "absl/log/log.h"
#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "absl/strings/str_format.h"
Expand All @@ -33,7 +34,6 @@
#include <grpc/event_engine/event_engine.h>
#include <grpc/slice_buffer.h>
#include <grpc/support/alloc.h>
#include <grpc/support/log.h>
#include <grpc/support/port_platform.h>

#include "src/core/lib/channel/channel_args.h"
Expand Down Expand Up @@ -142,17 +142,15 @@ void HandshakeManager::Shutdown(absl::Status error) {
MutexLock lock(&mu_);
if (!is_shutdown_) {
if (GRPC_TRACE_FLAG_ENABLED(handshaker)) {
gpr_log(GPR_INFO, "handshake_manager %p: Shutdown() called: %s", this,
error.ToString().c_str());
LOG(INFO) << "handshake_manager " << this
<< ": Shutdown() called: " << error;
}
is_shutdown_ = true;
// Shutdown the handshaker that's currently in progress, if any.
if (index_ > 0) {
if (GRPC_TRACE_FLAG_ENABLED(handshaker)) {
gpr_log(
GPR_INFO,
"handshake_manager %p: shutting down handshaker at index %" PRIuPTR,
this, index_ - 1);
LOG(INFO) << "handshake_manager " << this
<< ": shutting down handshaker at index " << index_ - 1;
}
handshakers_[index_ - 1]->Shutdown(std::move(error));
}
Expand All @@ -161,11 +159,9 @@ void HandshakeManager::Shutdown(absl::Status error) {

void HandshakeManager::CallNextHandshakerLocked(absl::Status error) {
if (GRPC_TRACE_FLAG_ENABLED(handshaker)) {
gpr_log(GPR_INFO,
"handshake_manager %p: error=%s shutdown=%d index=%" PRIuPTR
", args=%s",
this, error.ToString().c_str(), is_shutdown_, index_,
HandshakerArgsString(&args_).c_str());
LOG(INFO) << "handshake_manager " << this << ": error=" << error
<< " shutdown=" << is_shutdown_ << " index=" << index_
<< ", args=" << HandshakerArgsString(&args_);
}
CHECK(index_ <= handshakers_.size());
// If we got an error or we've been shut down or we're exiting early or
Expand All @@ -178,10 +174,10 @@ void HandshakeManager::CallNextHandshakerLocked(absl::Status error) {
args_.endpoint.reset();
}
if (GRPC_TRACE_FLAG_ENABLED(handshaker)) {
gpr_log(GPR_INFO,
"handshake_manager %p: handshaking complete -- scheduling "
"on_handshake_done with error=%s",
this, error.ToString().c_str());
LOG(INFO) << "handshake_manager " << this
<< ": handshaking complete -- scheduling "
"on_handshake_done with error="
<< error;
}
// Cancel deadline timer, since we're invoking the on_handshake_done
// callback now.
Expand All @@ -202,11 +198,9 @@ void HandshakeManager::CallNextHandshakerLocked(absl::Status error) {
// Call the next handshaker.
auto handshaker = handshakers_[index_];
if (GRPC_TRACE_FLAG_ENABLED(handshaker)) {
gpr_log(
GPR_INFO,
"handshake_manager %p: calling handshaker %s [%p] at index %" PRIuPTR,
this, std::string(handshaker->name()).c_str(), handshaker.get(),
index_);
LOG(INFO) << "handshake_manager " << this << ": calling handshaker "
<< handshaker->name() << " [" << handshaker.get() << "] at index "
<< index_;
}
++index_;
handshaker->DoHandshake(&args_, [self = Ref()](absl::Status error) mutable {
Expand Down
10 changes: 5 additions & 5 deletions src/core/handshaker/http_connect/http_connect_handshaker.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <utility>

#include "absl/base/thread_annotations.h"
#include "absl/log/log.h"
#include "absl/status/status.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/string_view.h"
Expand All @@ -34,7 +35,6 @@
#include <grpc/slice.h>
#include <grpc/slice_buffer.h>
#include <grpc/support/alloc.h>
#include <grpc/support/log.h>
#include <grpc/support/port_platform.h>

#include "src/core/handshaker/handshaker.h"
Expand Down Expand Up @@ -279,8 +279,8 @@ void HttpConnectHandshaker::DoHandshake(
for (size_t i = 0; i < num_header_strings; ++i) {
char* sep = strchr(header_strings[i], ':');
if (sep == nullptr) {
gpr_log(GPR_ERROR, "skipping unparseable HTTP CONNECT header: %s",
header_strings[i]);
LOG(ERROR) << "skipping unparseable HTTP CONNECT header: "
<< header_strings[i];
continue;
}
*sep = '\0';
Expand All @@ -296,8 +296,8 @@ void HttpConnectHandshaker::DoHandshake(
// Log connection via proxy.
std::string proxy_name(grpc_endpoint_get_peer(args->endpoint.get()));
std::string server_name_string(*server_name);
gpr_log(GPR_INFO, "Connecting to server %s via HTTP proxy %s",
server_name_string.c_str(), proxy_name.c_str());
LOG(INFO) << "Connecting to server " << server_name_string
<< " via HTTP proxy " << proxy_name;
// Construct HTTP CONNECT request.
grpc_http_request request;
request.method = const_cast<char*>("CONNECT");
Expand Down
40 changes: 18 additions & 22 deletions src/core/handshaker/http_connect/http_proxy_mapper.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,12 @@ absl::optional<std::string> GetHttpProxyServer(
if (uri_str->empty()) return absl::nullopt;
uri = URI::Parse(*uri_str);
if (!uri.ok() || uri->authority().empty()) {
gpr_log(GPR_ERROR, "cannot parse value of 'http_proxy' env var. Error: %s",
uri.status().ToString().c_str());
LOG(ERROR) << "cannot parse value of 'http_proxy' env var. Error: "
<< uri.status();
return absl::nullopt;
}
if (uri->scheme() != "http") {
gpr_log(GPR_ERROR, "'%s' scheme not supported in proxy URI",
uri->scheme().c_str());
LOG(ERROR) << "'" << uri->scheme() << "' scheme not supported in proxy URI";
return absl::nullopt;
}
// Split on '@' to separate user credentials from host
Expand Down Expand Up @@ -213,20 +212,18 @@ absl::optional<std::string> HttpProxyMapper::MapName(
if (!name_to_resolve.has_value()) return name_to_resolve;
absl::StatusOr<URI> uri = URI::Parse(server_uri);
if (!uri.ok() || uri->path().empty()) {
gpr_log(GPR_ERROR,
"'http_proxy' environment variable set, but cannot "
"parse server URI '%s' -- not using proxy. Error: %s",
std::string(server_uri).c_str(), uri.status().ToString().c_str());
LOG(ERROR) << "'http_proxy' environment variable set, but cannot "
"parse server URI '"
<< server_uri << "' -- not using proxy. Error: " << uri.status();
return absl::nullopt;
}
if (uri->scheme() == "unix") {
gpr_log(GPR_INFO, "not using proxy for Unix domain socket '%s'",
std::string(server_uri).c_str());
LOG(INFO) << "not using proxy for Unix domain socket '" << server_uri
<< "'";
return absl::nullopt;
}
if (uri->scheme() == "vsock") {
gpr_log(GPR_INFO, "not using proxy for VSock '%s'",
std::string(server_uri).c_str());
LOG(INFO) << "not using proxy for VSock '" << server_uri << "'";
return absl::nullopt;
}
// Prefer using 'no_grpc_proxy'. Fallback on 'no_proxy' if it is not set.
Expand All @@ -239,18 +236,17 @@ absl::optional<std::string> HttpProxyMapper::MapName(
std::string server_port;
if (!SplitHostPort(absl::StripPrefix(uri->path(), "/"), &server_host,
&server_port)) {
gpr_log(GPR_INFO,
"unable to split host and port, not checking no_proxy list for "
"host '%s'",
std::string(server_uri).c_str());
LOG(INFO) << "unable to split host and port, not checking no_proxy list "
"for host '"
<< server_uri << "'";
} else {
auto address = StringToSockaddr(server_host, 0);
if (AddressIncluded(address.ok()
? absl::optional<grpc_resolved_address>(*address)
: absl::nullopt,
server_host, *no_proxy_str)) {
gpr_log(GPR_INFO, "not using proxy for host in no_proxy list '%s'",
std::string(server_uri).c_str());
LOG(INFO) << "not using proxy for host in no_proxy list '" << server_uri
<< "'";
return absl::nullopt;
}
}
Expand All @@ -275,14 +271,14 @@ absl::optional<grpc_resolved_address> HttpProxyMapper::MapAddress(
}
auto address_string = grpc_sockaddr_to_string(&address, true);
if (!address_string.ok()) {
gpr_log(GPR_ERROR, "Unable to convert address to string: %s",
std::string(address_string.status().message()).c_str());
LOG(ERROR) << "Unable to convert address to string: "
<< address_string.status();
return absl::nullopt;
}
std::string host_name, port;
if (!SplitHostPort(*address_string, &host_name, &port)) {
gpr_log(GPR_ERROR, "Address %s cannot be split in host and port",
address_string->c_str());
LOG(ERROR) << "Address " << *address_string
<< " cannot be split in host and port";
return absl::nullopt;
}
auto enabled_addresses = GetChannelArgOrEnvVarValue(
Expand Down
20 changes: 7 additions & 13 deletions src/core/handshaker/security/secure_endpoint.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
#include <grpc/slice_buffer.h>
#include <grpc/support/alloc.h>
#include <grpc/support/atm.h>
#include <grpc/support/log.h>
#include <grpc/support/port_platform.h>
#include <grpc/support/sync.h>

Expand Down Expand Up @@ -160,9 +159,8 @@ static void secure_endpoint_unref(secure_endpoint* ep, const char* reason,
const char* file, int line) {
if (GRPC_TRACE_FLAG_ENABLED(secure_endpoint)) {
gpr_atm val = gpr_atm_no_barrier_load(&ep->ref.count);
gpr_log(file, line, GPR_LOG_SEVERITY_DEBUG,
"SECENDP unref %p : %s %" PRIdPTR " -> %" PRIdPTR, ep, reason, val,
val - 1);
VLOG(2).AtLocation(file, line) << "SECENDP unref " << ep << " : " << reason
<< " " << val << " -> " << val - 1;
}
if (gpr_unref(&ep->ref)) {
destroy(ep);
Expand All @@ -173,9 +171,8 @@ static void secure_endpoint_ref(secure_endpoint* ep, const char* reason,
const char* file, int line) {
if (GRPC_TRACE_FLAG_ENABLED(secure_endpoint)) {
gpr_atm val = gpr_atm_no_barrier_load(&ep->ref.count);
gpr_log(file, line, GPR_LOG_SEVERITY_DEBUG,
"SECENDP ref %p : %s %" PRIdPTR " -> %" PRIdPTR, ep, reason, val,
val + 1);
VLOG(2).AtLocation(file, line) << "SECENDP ref " << ep << " : " << reason
<< " " << val << " -> " << val + 1;
}
gpr_ref(&ep->ref);
}
Expand All @@ -200,8 +197,7 @@ static void maybe_post_reclaimer(secure_endpoint* ep) {
[ep](absl::optional<grpc_core::ReclamationSweep> sweep) {
if (sweep.has_value()) {
if (GRPC_TRACE_FLAG_ENABLED(resource_quota)) {
gpr_log(GPR_INFO,
"secure endpoint: benign reclamation to free memory");
LOG(INFO) << "secure endpoint: benign reclamation to free memory";
}
grpc_slice temp_read_slice;
grpc_slice temp_write_slice;
Expand Down Expand Up @@ -296,8 +292,7 @@ static void on_read(void* user_data, grpc_error_handle error) {
&unprotected_buffer_size_written);
gpr_mu_unlock(&ep->protector_mu);
if (result != TSI_OK) {
gpr_log(GPR_ERROR, "Decryption error: %s",
tsi_result_to_string(result));
LOG(ERROR) << "Decryption error: " << tsi_result_to_string(result);
break;
}
message_bytes += processed_message_size;
Expand Down Expand Up @@ -448,8 +443,7 @@ static void endpoint_write(grpc_endpoint* secure_ep, grpc_slice_buffer* slices,
&protected_buffer_size_to_send);
gpr_mu_unlock(&ep->protector_mu);
if (result != TSI_OK) {
gpr_log(GPR_ERROR, "Encryption error: %s",
tsi_result_to_string(result));
LOG(ERROR) << "Encryption error: " << tsi_result_to_string(result);
break;
}
message_bytes += processed_message_size;
Expand Down

0 comments on commit cfb1c61

Please sign in to comment.