Skip to content

Commit

Permalink
[grpc][Gpr_To_Absl_Logging] Migrating from gpr to absl logging GPR_AS…
Browse files Browse the repository at this point in the history
…SERT (#36458)

[grpc][Gpr_To_Absl_Logging] Migrating from gpr to absl logging GPR_ASSERT
Replacing GPR_ASSERT with absl CHECK.
These changes have been made using string replacement and regex.
Will not be replacing all instances of CHECK with CHECK_EQ , CHECK_NE etc because there are too many callsites. Only ones which are doable using very simple regex with least chance of failure will be replaced.
Given that we have 5000+ instances of GPR_ASSERT to edit, Doing it manually is too much work for both the author and reviewer.

<!--

If you know who should review your pull request, please assign it to that
person, otherwise the pull request would get assigned randomly.

If your pull request is for a specific language, please add the appropriate
lang label.

-->

Closes #36458

COPYBARA_INTEGRATE_REVIEW=#36458 from tanvi-jagtap:tjagtap_src_core_channelz_client_channel 9f5aca4
PiperOrigin-RevId: 629275206
  • Loading branch information
tanvi-jagtap authored and Copybara-Service committed Apr 30, 2024
1 parent 0944410 commit b73fdce
Show file tree
Hide file tree
Showing 15 changed files with 83 additions and 61 deletions.
3 changes: 3 additions & 0 deletions BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -1364,6 +1364,7 @@ grpc_cc_library(
],
external_deps = [
"absl/base:core_headers",
"absl/log:check",
"absl/status:statusor",
"absl/strings",
"absl/types:optional",
Expand Down Expand Up @@ -3302,6 +3303,7 @@ grpc_cc_library(
"//src/core:lib/uri/uri_parser.h",
],
external_deps = [
"absl/log:check",
"absl/status",
"absl/status:statusor",
"absl/strings",
Expand Down Expand Up @@ -3615,6 +3617,7 @@ grpc_cc_library(
"absl/container:flat_hash_set",
"absl/container:inlined_vector",
"absl/functional:any_invocable",
"absl/log:check",
"absl/status",
"absl/status:statusor",
"absl/strings",
Expand Down
3 changes: 3 additions & 0 deletions src/core/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -3137,6 +3137,7 @@ grpc_cc_library(
],
external_deps = [
"absl/functional:any_invocable",
"absl/log:check",
],
language = "c++",
deps = [
Expand Down Expand Up @@ -3208,6 +3209,7 @@ grpc_cc_library(
"client_channel/config_selector.h",
],
external_deps = [
"absl/log:check",
"absl/status",
"absl/strings",
],
Expand Down Expand Up @@ -3535,6 +3537,7 @@ grpc_cc_library(
"client_channel/http_proxy_mapper.h",
],
external_deps = [
"absl/log:check",
"absl/status",
"absl/status:statusor",
"absl/strings",
Expand Down
5 changes: 3 additions & 2 deletions src/core/channelz/channelz.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <atomic>
#include <cstdint>

#include "absl/log/check.h"
#include "absl/status/statusor.h"
#include "absl/strings/escaping.h"
#include "absl/strings/str_cat.h"
Expand Down Expand Up @@ -355,8 +356,8 @@ void ServerNode::RemoveChildListenSocket(intptr_t child_uuid) {

std::string ServerNode::RenderServerSockets(intptr_t start_socket_id,
intptr_t max_results) {
GPR_ASSERT(start_socket_id >= 0);
GPR_ASSERT(max_results >= 0);
CHECK_GE(start_socket_id, 0);
CHECK_GE(max_results, 0);
// If user does not set max_results, we choose 500.
size_t pagination_limit = max_results == 0 ? 500 : max_results;
Json::Object object;
Expand Down
6 changes: 4 additions & 2 deletions src/core/channelz/channelz_registry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
#include <utility>
#include <vector>

#include "absl/log/check.h"

#include <grpc/grpc.h>
#include <grpc/support/json.h>
#include <grpc/support/log.h>
Expand Down Expand Up @@ -56,9 +58,9 @@ void ChannelzRegistry::InternalRegister(BaseNode* node) {
}

void ChannelzRegistry::InternalUnregister(intptr_t uuid) {
GPR_ASSERT(uuid >= 1);
CHECK_GE(uuid, 1);
MutexLock lock(&mu_);
GPR_ASSERT(uuid <= uuid_generator_);
CHECK(uuid <= uuid_generator_);
node_map_.erase(uuid);
}

Expand Down
57 changes: 29 additions & 28 deletions src/core/client_channel/client_channel_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <vector>

#include "absl/cleanup/cleanup.h"
#include "absl/log/check.h"
#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "absl/strings/cord.h"
Expand Down Expand Up @@ -458,8 +459,8 @@ class DynamicTerminationFilter final {

static grpc_error_handle Init(grpc_channel_element* elem,
grpc_channel_element_args* args) {
GPR_ASSERT(args->is_last);
GPR_ASSERT(elem->filter == &kFilterVtable);
CHECK(args->is_last);
CHECK(elem->filter == &kFilterVtable);
new (elem->channel_data) DynamicTerminationFilter(args->channel_args);
return absl::OkStatus();
}
Expand Down Expand Up @@ -644,7 +645,7 @@ class ClientChannelFilter::SubchannelWrapper final
}
GRPC_CHANNEL_STACK_REF(chand_->owning_stack_, "SubchannelWrapper");
#ifndef NDEBUG
GPR_DEBUG_ASSERT(chand_->work_serializer_->RunningInWorkSerializer());
DCHECK(chand_->work_serializer_->RunningInWorkSerializer());
#endif
if (chand_->channelz_node_ != nullptr) {
auto* subchannel_node = subchannel_->channelz_node();
Expand Down Expand Up @@ -673,7 +674,7 @@ class ClientChannelFilter::SubchannelWrapper final
auto* subchannel_node = subchannel_->channelz_node();
if (subchannel_node != nullptr) {
auto it = chand_->subchannel_refcount_map_.find(subchannel_.get());
GPR_ASSERT(it != chand_->subchannel_refcount_map_.end());
CHECK(it != chand_->subchannel_refcount_map_.end());
--it->second;
if (it->second == 0) {
chand_->channelz_node_->RemoveChildSubchannel(
Expand All @@ -700,7 +701,7 @@ class ClientChannelFilter::SubchannelWrapper final
if (subchannel_node != nullptr) {
auto it =
chand_->subchannel_refcount_map_.find(subchannel_.get());
GPR_ASSERT(it != chand_->subchannel_refcount_map_.end());
CHECK(it != chand_->subchannel_refcount_map_.end());
--it->second;
if (it->second == 0) {
chand_->channelz_node_->RemoveChildSubchannel(
Expand All @@ -718,7 +719,7 @@ class ClientChannelFilter::SubchannelWrapper final
std::unique_ptr<ConnectivityStateWatcherInterface> watcher) override
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand_->work_serializer_) {
auto& watcher_wrapper = watcher_map_[watcher.get()];
GPR_ASSERT(watcher_wrapper == nullptr);
CHECK_EQ(watcher_wrapper, nullptr);
watcher_wrapper = new WatcherWrapper(
std::move(watcher),
RefAsSubclass<SubchannelWrapper>(DEBUG_LOCATION, "WatcherWrapper"));
Expand All @@ -730,7 +731,7 @@ class ClientChannelFilter::SubchannelWrapper final
void CancelConnectivityStateWatch(ConnectivityStateWatcherInterface* watcher)
override ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand_->work_serializer_) {
auto it = watcher_map_.find(watcher);
GPR_ASSERT(it != watcher_map_.end());
CHECK(it != watcher_map_.end());
subchannel_->CancelConnectivityStateWatch(it->second);
watcher_map_.erase(it);
}
Expand All @@ -747,7 +748,7 @@ class ClientChannelFilter::SubchannelWrapper final
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand_->work_serializer_) {
static_cast<InternalSubchannelDataWatcherInterface*>(watcher.get())
->SetSubchannel(subchannel_.get());
GPR_ASSERT(data_watchers_.insert(std::move(watcher)).second);
CHECK(data_watchers_.insert(std::move(watcher)).second);
}

void CancelDataWatcher(DataWatcherInterface* watcher) override
Expand Down Expand Up @@ -923,7 +924,7 @@ ClientChannelFilter::ExternalConnectivityWatcher::ExternalConnectivityWatcher(
{
MutexLock lock(&chand_->external_watchers_mu_);
// Will be deleted when the watch is complete.
GPR_ASSERT(chand->external_watchers_[on_complete] == nullptr);
CHECK(chand->external_watchers_[on_complete] == nullptr);
// Store a ref to the watcher in the external_watchers_ map.
chand->external_watchers_[on_complete] =
RefAsSubclass<ExternalConnectivityWatcher>(
Expand Down Expand Up @@ -1201,9 +1202,9 @@ class ClientChannelFilter::ClientChannelControlHelper final

grpc_error_handle ClientChannelFilter::Init(grpc_channel_element* elem,
grpc_channel_element_args* args) {
GPR_ASSERT(args->is_last);
GPR_ASSERT(elem->filter == &kFilterVtableWithPromises ||
elem->filter == &kFilterVtableWithoutPromises);
CHECK(args->is_last);
CHECK(elem->filter == &kFilterVtableWithPromises ||
elem->filter == &kFilterVtableWithoutPromises);
grpc_error_handle error;
new (elem->channel_data) ClientChannelFilter(args, &error);
return error;
Expand Down Expand Up @@ -1449,7 +1450,7 @@ RefCountedPtr<LoadBalancingPolicy::Config> ChooseLbPolicy(
// - A channel arg, in which case we check that the specified policy exists
// and accepts an empty config. If not, we revert to using pick_first
// lb_policy
GPR_ASSERT(lb_policy_config.ok());
CHECK(lb_policy_config.ok());
return std::move(*lb_policy_config);
}

Expand Down Expand Up @@ -1729,7 +1730,7 @@ void ClientChannelFilter::UpdateServiceConfigInDataPlaneLocked() {
}
RefCountedPtr<DynamicFilters> dynamic_filters =
DynamicFilters::Create(new_args, std::move(filters));
GPR_ASSERT(dynamic_filters != nullptr);
CHECK(dynamic_filters != nullptr);
// Grab data plane lock to update service config.
//
// We defer unreffing the old values (and deallocating memory) until
Expand Down Expand Up @@ -1760,7 +1761,7 @@ void ClientChannelFilter::CreateResolverLocked() {
std::make_unique<ResolverResultHandler>(this));
// Since the validity of the args was checked when the channel was created,
// CreateResolver() must return a non-null result.
GPR_ASSERT(resolver_ != nullptr);
CHECK(resolver_ != nullptr);
UpdateStateLocked(GRPC_CHANNEL_CONNECTING, absl::Status(),
"started resolving");
resolver_->StartLocked();
Expand Down Expand Up @@ -1869,7 +1870,7 @@ T HandlePickResult(
}
auto* drop_pick =
absl::get_if<LoadBalancingPolicy::PickResult::Drop>(&result->result);
GPR_ASSERT(drop_pick != nullptr);
CHECK_NE(drop_pick, nullptr);
return drop_func(drop_pick);
}

Expand Down Expand Up @@ -1963,7 +1964,7 @@ void ClientChannelFilter::StartTransportOpLocked(grpc_transport_op* op) {
}
} else {
// Disconnect.
GPR_ASSERT(disconnect_error_.ok());
CHECK(disconnect_error_.ok());
disconnect_error_ = op->disconnect_with_error;
UpdateStateAndPickerLocked(
GRPC_CHANNEL_SHUTDOWN, absl::Status(), "shutdown from API",
Expand All @@ -1981,7 +1982,7 @@ void ClientChannelFilter::StartTransportOpLocked(grpc_transport_op* op) {
void ClientChannelFilter::StartTransportOp(grpc_channel_element* elem,
grpc_transport_op* op) {
auto* chand = static_cast<ClientChannelFilter*>(elem->channel_data);
GPR_ASSERT(op->set_accept_stream == false);
CHECK(op->set_accept_stream == false);
// Handle bind_pollset.
if (op->bind_pollset != nullptr) {
grpc_pollset_set_add_pollset(chand->interested_parties_, op->bind_pollset);
Expand Down Expand Up @@ -2227,7 +2228,7 @@ ClientChannelFilter::FilterBasedCallData::~FilterBasedCallData() {
CSliceUnref(path_);
// Make sure there are no remaining pending batches.
for (size_t i = 0; i < GPR_ARRAY_SIZE(pending_batches_); ++i) {
GPR_ASSERT(pending_batches_[i] == nullptr);
CHECK_EQ(pending_batches_[i], nullptr);
}
}

Expand Down Expand Up @@ -2389,7 +2390,7 @@ void ClientChannelFilter::FilterBasedCallData::PendingBatchesAdd(
chand(), this, idx);
}
grpc_transport_stream_op_batch*& pending = pending_batches_[idx];
GPR_ASSERT(pending == nullptr);
CHECK_EQ(pending, nullptr);
pending = batch;
}

Expand All @@ -2409,7 +2410,7 @@ void ClientChannelFilter::FilterBasedCallData::FailPendingBatchInCallCombiner(
void ClientChannelFilter::FilterBasedCallData::PendingBatchesFail(
grpc_error_handle error,
YieldCallCombinerPredicate yield_call_combiner_predicate) {
GPR_ASSERT(!error.ok());
CHECK(!error.ok());
if (GRPC_TRACE_FLAG_ENABLED(grpc_client_channel_call_trace)) {
size_t num_batches = 0;
for (size_t i = 0; i < GPR_ARRAY_SIZE(pending_batches_); ++i) {
Expand Down Expand Up @@ -2958,11 +2959,11 @@ ClientChannelFilter::LoadBalancedCall::PickSubchannel(bool was_queued) {

bool ClientChannelFilter::LoadBalancedCall::PickSubchannelImpl(
LoadBalancingPolicy::SubchannelPicker* picker, grpc_error_handle* error) {
GPR_ASSERT(connected_subchannel_ == nullptr);
CHECK(connected_subchannel_ == nullptr);
// Perform LB pick.
LoadBalancingPolicy::PickArgs pick_args;
Slice* path = send_initial_metadata()->get_pointer(HttpPathMetadata());
GPR_ASSERT(path != nullptr);
CHECK_NE(path, nullptr);
pick_args.path = path->as_string_view();
LbCallState lb_call_state(this);
pick_args.call_state = &lb_call_state;
Expand All @@ -2978,7 +2979,7 @@ bool ClientChannelFilter::LoadBalancedCall::PickSubchannelImpl(
"chand=%p lb_call=%p: LB pick succeeded: subchannel=%p",
chand_, this, complete_pick->subchannel.get());
}
GPR_ASSERT(complete_pick->subchannel != nullptr);
CHECK(complete_pick->subchannel != nullptr);
// Grab a ref to the connected subchannel while we're still
// holding the data plane mutex.
SubchannelWrapper* subchannel =
Expand Down Expand Up @@ -3066,7 +3067,7 @@ ClientChannelFilter::FilterBasedLoadBalancedCall::
~FilterBasedLoadBalancedCall() {
// Make sure there are no remaining pending batches.
for (size_t i = 0; i < GPR_ARRAY_SIZE(pending_batches_); ++i) {
GPR_ASSERT(pending_batches_[i] == nullptr);
CHECK_EQ(pending_batches_[i], nullptr);
}
if (on_call_destruction_complete_ != nullptr) {
ExecCtx::Run(DEBUG_LOCATION, on_call_destruction_complete_,
Expand Down Expand Up @@ -3109,7 +3110,7 @@ void ClientChannelFilter::FilterBasedLoadBalancedCall::PendingBatchesAdd(
"chand=%p lb_call=%p: adding pending batch at index %" PRIuPTR,
chand(), this, idx);
}
GPR_ASSERT(pending_batches_[idx] == nullptr);
CHECK_EQ(pending_batches_[idx], nullptr);
pending_batches_[idx] = batch;
}

Expand All @@ -3129,7 +3130,7 @@ void ClientChannelFilter::FilterBasedLoadBalancedCall::
void ClientChannelFilter::FilterBasedLoadBalancedCall::PendingBatchesFail(
grpc_error_handle error,
YieldCallCombinerPredicate yield_call_combiner_predicate) {
GPR_ASSERT(!error.ok());
CHECK(!error.ok());
failure_error_ = error;
if (GRPC_TRACE_FLAG_ENABLED(grpc_client_channel_lb_call_trace)) {
size_t num_batches = 0;
Expand Down Expand Up @@ -3491,7 +3492,7 @@ void ClientChannelFilter::FilterBasedLoadBalancedCall::RetryPickLocked() {

void ClientChannelFilter::FilterBasedLoadBalancedCall::CreateSubchannelCall() {
Slice* path = send_initial_metadata()->get_pointer(HttpPathMetadata());
GPR_ASSERT(path != nullptr);
CHECK_NE(path, nullptr);
SubchannelCall::Args call_args = {
connected_subchannel()->Ref(), pollent_, path->Ref(), /*start_time=*/0,
deadline_, arena_,
Expand Down
3 changes: 2 additions & 1 deletion src/core/client_channel/client_channel_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <utility>

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

#include <grpc/support/log.h>

Expand Down Expand Up @@ -61,7 +62,7 @@ class ClientChannelServiceConfigCallData final : public ServiceConfigCallData {
: ServiceConfigCallData(arena, call_context) {}

void SetOnCommit(absl::AnyInvocable<void()> on_commit) {
GPR_ASSERT(on_commit_ == nullptr);
CHECK(on_commit_ == nullptr);
on_commit_ = std::move(on_commit);
}

Expand Down
5 changes: 3 additions & 2 deletions src/core/client_channel/config_selector.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <utility>
#include <vector>

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

Expand Down Expand Up @@ -97,14 +98,14 @@ class DefaultConfigSelector final : public ConfigSelector {
// The client channel code ensures that this will never be null.
// If neither the resolver nor the client application provide a
// config, a default empty config will be used.
GPR_DEBUG_ASSERT(service_config_ != nullptr);
DCHECK(service_config_ != nullptr);
}

const char* name() const override { return "default"; }

absl::Status GetCallConfig(GetCallConfigArgs args) override {
Slice* path = args.initial_metadata->get_pointer(HttpPathMetadata());
GPR_ASSERT(path != nullptr);
CHECK_NE(path, nullptr);
auto* parsed_method_configs =
service_config_->GetMethodParsedConfigVector(path->c_slice());
args.service_config_call_data->SetServiceConfig(service_config_,
Expand Down
5 changes: 3 additions & 2 deletions src/core/client_channel/dynamic_filters.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <new>
#include <utility>

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

#include <grpc/support/log.h>
Expand Down Expand Up @@ -82,8 +83,8 @@ void DynamicFilters::Call::StartTransportStreamOpBatch(
}

void DynamicFilters::Call::SetAfterCallStackDestroy(grpc_closure* closure) {
GPR_ASSERT(after_call_stack_destroy_ == nullptr);
GPR_ASSERT(closure != nullptr);
CHECK_EQ(after_call_stack_destroy_, nullptr);
CHECK_NE(closure, nullptr);
after_call_stack_destroy_ = closure;
}

Expand Down

0 comments on commit b73fdce

Please sign in to comment.