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#37090)

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

COPYBARA_INTEGRATE_REVIEW=grpc#37090 from tanvi-jagtap:src_core_ext_transport_chaotic_good 5e709bc
PiperOrigin-RevId: 647960983
  • Loading branch information
tanvi-jagtap authored and copybara-github committed Jun 29, 2024
1 parent ab2b28d commit 49a730e
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 83 deletions.
4 changes: 3 additions & 1 deletion src/core/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -7167,7 +7167,7 @@ grpc_cc_library(
"ext/transport/inproc/legacy_inproc_transport.h",
],
external_deps = [
"absl/log",
"absl/log:log",
"absl/log:check",
"absl/status",
"absl/status:statusor",
Expand Down Expand Up @@ -7517,6 +7517,7 @@ grpc_cc_library(
"absl/container:flat_hash_map",
"absl/functional:any_invocable",
"absl/log:check",
"absl/log:log",
"absl/random",
"absl/random:bit_gen_ref",
"absl/status",
Expand Down Expand Up @@ -7959,6 +7960,7 @@ grpc_cc_library(
],
external_deps = [
"absl/log:check",
"absl/log:log",
"absl/random",
"absl/random:bit_gen_ref",
"absl/status",
Expand Down
28 changes: 14 additions & 14 deletions src/core/ext/transport/chaotic_good/chaotic_good_transport.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <cstdint>
#include <utility>

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

#include <grpc/support/port_platform.h>
Expand Down Expand Up @@ -56,11 +57,10 @@ class ChaoticGoodTransport : public RefCounted<ChaoticGoodTransport> {
auto buffers = frame.Serialize(&encoder_, saw_encoding_errors);
// ignore encoding errors: they will be logged separately already
if (GRPC_TRACE_FLAG_ENABLED(chaotic_good)) {
gpr_log(GPR_INFO, "CHAOTIC_GOOD: WriteFrame to:%s %s",
ResolvedAddressToString(control_endpoint_.GetPeerAddress())
.value_or("<<unknown peer address>>")
.c_str(),
frame.ToString().c_str());
LOG(INFO) << "CHAOTIC_GOOD: WriteFrame to:"
<< ResolvedAddressToString(control_endpoint_.GetPeerAddress())
.value_or("<<unknown peer address>>")
<< " " << frame.ToString();
}
return TryJoin<absl::StatusOr>(
control_endpoint_.Write(std::move(buffers.control)),
Expand All @@ -77,13 +77,13 @@ class ChaoticGoodTransport : public RefCounted<ChaoticGoodTransport> {
FrameHeader::Parse(reinterpret_cast<const uint8_t*>(
GRPC_SLICE_START_PTR(read_buffer.c_slice())));
if (GRPC_TRACE_FLAG_ENABLED(chaotic_good)) {
gpr_log(GPR_INFO, "CHAOTIC_GOOD: ReadHeader from:%s %s",
ResolvedAddressToString(control_endpoint_.GetPeerAddress())
.value_or("<<unknown peer address>>")
.c_str(),
frame_header.ok()
? frame_header->ToString().c_str()
: frame_header.status().ToString().c_str());
LOG(INFO) << "CHAOTIC_GOOD: ReadHeader from:"
<< ResolvedAddressToString(
control_endpoint_.GetPeerAddress())
.value_or("<<unknown peer address>>")
<< " "
<< (frame_header.ok() ? frame_header->ToString()
: frame_header.status().ToString());
}
// Read header and trailers from control endpoint.
// Read message padding and message from data endpoint.
Expand Down Expand Up @@ -126,8 +126,8 @@ class ChaoticGoodTransport : public RefCounted<ChaoticGoodTransport> {
auto s = frame.Deserialize(&parser_, header, bitgen_, arena,
std::move(buffers), limits);
if (GRPC_TRACE_FLAG_ENABLED(chaotic_good)) {
gpr_log(GPR_INFO, "CHAOTIC_GOOD: DeserializeFrame %s",
s.ok() ? frame.ToString().c_str() : s.ToString().c_str());
LOG(INFO) << "CHAOTIC_GOOD: DeserializeFrame "
<< (s.ok() ? frame.ToString() : s.ToString());
}
return s;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <utility>

#include "absl/log/check.h"
#include "absl/log/log.h"
#include "absl/random/bit_gen_ref.h"
#include "absl/status/status.h"
#include "absl/status/statusor.h"
Expand Down Expand Up @@ -319,8 +320,7 @@ void ChaoticGoodConnector::OnHandshakeDone(
EventEngineWakeupScheduler(event_engine_),
[self = RefAsSubclass<ChaoticGoodConnector>()](absl::Status status) {
if (GRPC_TRACE_FLAG_ENABLED(chaotic_good)) {
gpr_log(GPR_INFO, "ChaoticGoodConnector::OnHandshakeDone: %s",
status.ToString().c_str());
LOG(INFO) << "ChaoticGoodConnector::OnHandshakeDone: " << status;
}
if (status.ok()) {
MutexLock lock(&self->mu_);
Expand Down
6 changes: 3 additions & 3 deletions src/core/ext/transport/chaotic_good/client_transport.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@
#include <utility>

#include "absl/log/check.h"
#include "absl/log/log.h"
#include "absl/random/bit_gen_ref.h"
#include "absl/random/random.h"
#include "absl/status/status.h"
#include "absl/status/statusor.h"

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

#include "src/core/ext/transport/chaotic_good/chaotic_good_transport.h"
Expand Down Expand Up @@ -281,8 +281,8 @@ auto ChaoticGoodClientTransport::CallOutboundLoop(uint32_t stream_id,
call_handler.PullClientInitialMetadata(),
[send_fragment](ClientMetadataHandle md) mutable {
if (GRPC_TRACE_FLAG_ENABLED(chaotic_good)) {
gpr_log(GPR_INFO, "CHAOTIC_GOOD: Sending initial metadata: %s",
md->DebugString().c_str());
LOG(INFO) << "CHAOTIC_GOOD: Sending initial metadata: "
<< md->DebugString();
}
ClientFragmentFrame frame;
frame.headers = std::move(md);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
#include <grpc/event_engine/event_engine.h>
#include <grpc/grpc.h>
#include <grpc/slice.h>
#include <grpc/support/log.h>
#include <grpc/support/port_platform.h>

#include "src/core/ext/transport/chaotic_good/frame.h"
Expand Down Expand Up @@ -114,8 +113,7 @@ absl::StatusOr<int> ChaoticGoodServerListener::Bind(
};
auto shutdown_cb = [](absl::Status status) {
if (!status.ok()) {
gpr_log(GPR_ERROR, "Server accept connection failed: %s",
StatusToString(status).c_str());
LOG(ERROR) << "Server accept connection failed: " << status;
}
};
CHECK_NE(event_engine_, nullptr);
Expand Down Expand Up @@ -303,10 +301,10 @@ auto ChaoticGoodServerListener::ActiveConnection::HandshakingState::
[self](PromiseEndpoint ret) -> absl::Status {
MutexLock lock(&self->connection_->listener_->mu_);
if (GRPC_TRACE_FLAG_ENABLED(chaotic_good)) {
gpr_log(
GPR_INFO, "%p Data endpoint setup done: shutdown=%s",
self->connection_.get(),
self->connection_->listener_->shutdown_ ? "true" : "false");
LOG(INFO) << self->connection_.get()
<< " Data endpoint setup done: shutdown="
<< (self->connection_->listener_->shutdown_ ? "true"
: "false");
}
if (self->connection_->listener_->shutdown_) {
return absl::UnavailableError("Server shutdown");
Expand Down
69 changes: 31 additions & 38 deletions src/core/ext/transport/chaotic_good/server_transport.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <tuple>

#include "absl/log/check.h"
#include "absl/log/log.h"
#include "absl/random/bit_gen_ref.h"
#include "absl/random/random.h"
#include "absl/status/status.h"
Expand All @@ -27,7 +28,6 @@
#include <grpc/event_engine/event_engine.h>
#include <grpc/grpc.h>
#include <grpc/slice.h>
#include <grpc/support/log.h>
#include <grpc/support/port_platform.h>

#include "src/core/ext/transport/chaotic_good/chaotic_good_transport.h"
Expand Down Expand Up @@ -76,8 +76,8 @@ auto ChaoticGoodServerTransport::PushFragmentIntoCall(
uint32_t stream_id) {
DCHECK(frame.headers == nullptr);
if (GRPC_TRACE_FLAG_ENABLED(chaotic_good)) {
gpr_log(GPR_INFO, "CHAOTIC_GOOD: PushFragmentIntoCall: frame=%s",
frame.ToString().c_str());
LOG(INFO) << "CHAOTIC_GOOD: PushFragmentIntoCall: frame="
<< frame.ToString();
}
return Seq(If(
frame.message.has_value(),
Expand All @@ -89,7 +89,7 @@ auto ChaoticGoodServerTransport::PushFragmentIntoCall(
[this, call_initiator, end_of_stream = frame.end_of_stream,
stream_id](StatusFlag status) mutable -> StatusFlag {
if (!status.ok() && GRPC_TRACE_FLAG_ENABLED(chaotic_good)) {
gpr_log(GPR_INFO, "CHAOTIC_GOOD: Failed PushFragmentIntoCall");
LOG(INFO) << "CHAOTIC_GOOD: Failed PushFragmentIntoCall";
}
if (end_of_stream || !status.ok()) {
call_initiator.FinishSends();
Expand Down Expand Up @@ -124,10 +124,8 @@ auto ChaoticGoodServerTransport::MaybePushFragmentIntoCall(
// already been removed from the stream_map and hence the EOF frame
// cannot be pushed into the call. No need to log such frames.
if (!frame.end_of_stream) {
gpr_log(
GPR_INFO,
"CHAOTIC_GOOD: Cannot pass frame to stream. Error:%s Frame:%s",
error.ToString().c_str(), frame.ToString().c_str());
LOG(INFO) << "CHAOTIC_GOOD: Cannot pass frame to stream. Error:"
<< error.ToString() << " Frame:" << frame.ToString();
}
return Immediate(std::move(error));
});
Expand All @@ -137,8 +135,7 @@ auto ChaoticGoodServerTransport::SendFragment(
ServerFragmentFrame frame, MpscSender<ServerFrame> outgoing_frames,
CallInitiator call_initiator) {
if (GRPC_TRACE_FLAG_ENABLED(chaotic_good)) {
gpr_log(GPR_INFO, "CHAOTIC_GOOD: SendFragment: frame=%s",
frame.ToString().c_str());
LOG(INFO) << "CHAOTIC_GOOD: SendFragment: frame=" << frame.ToString();
}
// Capture the call_initiator to ensure the underlying call spine is alive
// until the outgoing_frames.Send promise completes.
Expand Down Expand Up @@ -189,9 +186,8 @@ auto ChaoticGoodServerTransport::SendCallInitialMetadataAndBody(
[stream_id, outgoing_frames, call_initiator,
this](absl::optional<ServerMetadataHandle> md) mutable {
if (GRPC_TRACE_FLAG_ENABLED(chaotic_good)) {
gpr_log(GPR_INFO,
"CHAOTIC_GOOD: SendCallInitialMetadataAndBody: md=%s",
md.has_value() ? (*md)->DebugString().c_str() : "null");
LOG(INFO) << "CHAOTIC_GOOD: SendCallInitialMetadataAndBody: md="
<< (md.has_value() ? (*md)->DebugString() : "null");
}
return If(
md.has_value(),
Expand All @@ -211,28 +207,26 @@ auto ChaoticGoodServerTransport::SendCallInitialMetadataAndBody(
auto ChaoticGoodServerTransport::CallOutboundLoop(
uint32_t stream_id, CallInitiator call_initiator) {
auto outgoing_frames = outgoing_frames_.MakeSender();
return Seq(Map(SendCallInitialMetadataAndBody(stream_id, outgoing_frames,
call_initiator),
[stream_id](absl::Status main_body_result) {
if (GRPC_TRACE_FLAG_ENABLED(chaotic_good)) {
gpr_log(GPR_DEBUG,
"CHAOTIC_GOOD: CallOutboundLoop: stream_id=%d "
"main_body_result=%s",
stream_id, main_body_result.ToString().c_str());
}
return Empty{};
}),
call_initiator.PullServerTrailingMetadata(),
// Capture the call_initator to ensure the underlying call_spine
// is alive until the SendFragment promise completes.
[stream_id, outgoing_frames,
call_initiator](ServerMetadataHandle md) mutable {
ServerFragmentFrame frame;
frame.trailers = std::move(md);
frame.stream_id = stream_id;
return SendFragment(std::move(frame), outgoing_frames,
call_initiator);
});
return Seq(
Map(SendCallInitialMetadataAndBody(stream_id, outgoing_frames,
call_initiator),
[stream_id](absl::Status main_body_result) {
if (GRPC_TRACE_FLAG_ENABLED(chaotic_good)) {
VLOG(2) << "CHAOTIC_GOOD: CallOutboundLoop: stream_id="
<< stream_id << " main_body_result=" << main_body_result;
}
return Empty{};
}),
call_initiator.PullServerTrailingMetadata(),
// Capture the call_initator to ensure the underlying call_spine
// is alive until the SendFragment promise completes.
[stream_id, outgoing_frames,
call_initiator](ServerMetadataHandle md) mutable {
ServerFragmentFrame frame;
frame.trailers = std::move(md);
frame.stream_id = stream_id;
return SendFragment(std::move(frame), outgoing_frames, call_initiator);
});
}

auto ChaoticGoodServerTransport::DeserializeAndPushFragmentToNewCall(
Expand Down Expand Up @@ -347,9 +341,8 @@ auto ChaoticGoodServerTransport::OnTransportActivityDone(
return [self = RefAsSubclass<ChaoticGoodServerTransport>(),
activity](absl::Status status) {
if (GRPC_TRACE_FLAG_ENABLED(chaotic_good)) {
gpr_log(GPR_INFO,
"CHAOTIC_GOOD: OnTransportActivityDone: activity=%s status=%s",
std::string(activity).c_str(), status.ToString().c_str());
LOG(INFO) << "CHAOTIC_GOOD: OnTransportActivityDone: activity="
<< activity << " status=" << status;
}
self->AbortWithError();
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@

#include "src/core/ext/transport/cronet/client/secure/cronet_channel_create.h"

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

#include <grpc/impl/channel_arg_names.h>
#include <grpc/support/log.h>
#include <grpc/support/port_platform.h>

#include "src/core/ext/transport/cronet/transport/cronet_transport.h"
Expand All @@ -38,9 +38,8 @@
GRPCAPI grpc_channel* grpc_cronet_secure_channel_create(
void* engine, const char* target, const grpc_channel_args* args,
void* reserved) {
gpr_log(GPR_DEBUG,
"grpc_create_cronet_transport: stream_engine = %p, target=%s", engine,
target);
VLOG(2) << "grpc_create_cronet_transport: stream_engine = " << engine
<< ", target=" << target;

// Disable client authority filter when using Cronet
auto channel_args = grpc_core::CoreConfiguration::Get()
Expand Down
16 changes: 7 additions & 9 deletions src/core/ext/transport/cronet/transport/cronet_transport.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <utility>

#include "absl/log/check.h"
#include "absl/log/log.h"
#include "absl/status/status.h"
#include "absl/strings/match.h"
#include "absl/strings/str_cat.h"
Expand All @@ -39,7 +40,6 @@
#include <grpc/slice.h>
#include <grpc/status.h>
#include <grpc/support/alloc.h>
#include <grpc/support/log.h>
#include <grpc/support/port_platform.h>
#include <grpc/support/sync.h>

Expand Down Expand Up @@ -446,11 +446,9 @@ static void convert_cronet_array_to_metadata(
}
mds->Append(header_array->headers[i].key, grpc_core::Slice(value),
[&](absl::string_view error, const grpc_core::Slice& value) {
gpr_log(GPR_DEBUG, "Failed to parse metadata: %s",
absl::StrCat("key=", header_array->headers[i].key,
" error=", error,
" value=", value.as_string_view())
.c_str());
VLOG(2) << "Failed to parse metadata: key="
<< header_array->headers[i].key << " error=" << error
<< " value=" << value.as_string_view();
});
}
}
Expand All @@ -459,7 +457,7 @@ static void convert_cronet_array_to_metadata(
// Cronet callback
//
static void on_failed(bidirectional_stream* stream, int net_error) {
gpr_log(GPR_ERROR, "on_failed(%p, %d)", stream, net_error);
LOG(ERROR) << "on_failed(" << stream << ", " << net_error << ")";
grpc_core::ApplicationCallbackExecCtx callback_exec_ctx;
grpc_core::ExecCtx exec_ctx;

Expand Down Expand Up @@ -1486,8 +1484,8 @@ grpc_core::Transport* grpc_create_cronet_transport(
if (0 ==
strcmp(args->args[i].key, GRPC_ARG_USE_CRONET_PACKET_COALESCING)) {
if (GPR_UNLIKELY(args->args[i].type != GRPC_ARG_INTEGER)) {
gpr_log(GPR_ERROR, "%s ignored: it must be an integer",
GRPC_ARG_USE_CRONET_PACKET_COALESCING);
LOG(ERROR) << GRPC_ARG_USE_CRONET_PACKET_COALESCING
<< " ignored: it must be an integer";
} else {
ct->use_packet_coalescing = (args->args[i].value.integer != 0);
}
Expand Down
8 changes: 3 additions & 5 deletions src/core/ext/transport/inproc/inproc_transport.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@
#include <memory>

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

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

#include "src/core/ext/transport/inproc/legacy_inproc_transport.h"
Expand Down Expand Up @@ -77,8 +77,7 @@ class InprocServerTransport final : public ServerTransport {
void SetPollset(grpc_stream*, grpc_pollset*) override {}
void SetPollsetSet(grpc_stream*, grpc_pollset_set*) override {}
void PerformOp(grpc_transport_op* op) override {
gpr_log(GPR_INFO, "inproc server op: %s",
grpc_transport_op_string(op).c_str());
LOG(INFO) << "inproc server op: " << grpc_transport_op_string(op);
if (op->start_connectivity_watch != nullptr) {
connected_state()->AddWatcher(op->start_connectivity_watch_state,
std::move(op->start_connectivity_watch));
Expand Down Expand Up @@ -240,8 +239,7 @@ InprocServerTransport::MakeClientTransport() {

RefCountedPtr<Channel> MakeLameChannel(absl::string_view why,
absl::Status error) {
gpr_log(GPR_ERROR, "%s: %s", std::string(why).c_str(),
std::string(error.message()).c_str());
LOG(ERROR) << why << ": " << error.message();
intptr_t integer;
grpc_status_code status = GRPC_STATUS_INTERNAL;
if (grpc_error_get_int(error, StatusIntProperty::kRpcStatus, &integer)) {
Expand Down

0 comments on commit 49a730e

Please sign in to comment.