From 8acddbb01ea4d6bf2674d7646b17b7b3f5a39654 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 11 Apr 2024 13:51:34 -0700 Subject: [PATCH] [call-v3] Improve ChannelFilter::Args structure (#36339) Deprecate things that mention channel stacks directly... a future change will mutate the internal data structure. Closes #36339 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36339 from ctiller:creation-story 79993c8b036cb7f23ace8b26f2d16e936f238e67 PiperOrigin-RevId: 623929372 --- .../channel_idle/legacy_channel_idle_filter.cc | 6 ++++++ .../fault_injection/fault_injection_filter.cc | 4 +--- src/core/ext/filters/rbac/rbac_filter.cc | 4 +--- .../stateful_session/stateful_session_filter.cc | 4 +--- src/core/lib/channel/promise_based_filter.h | 14 ++++++++++++-- 5 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/core/ext/filters/channel_idle/legacy_channel_idle_filter.cc b/src/core/ext/filters/channel_idle/legacy_channel_idle_filter.cc index a1e6fd07160..7d7757ac081 100644 --- a/src/core/ext/filters/channel_idle/legacy_channel_idle_filter.cc +++ b/src/core/ext/filters/channel_idle/legacy_channel_idle_filter.cc @@ -128,6 +128,11 @@ struct LegacyMaxAgeFilter::Config { } }; +// We need access to the channel stack here to send a goaway - but that access +// is deprecated and will be removed when call-v3 is fully enabled. This filter +// will be removed at that time also, so just disable the deprecation warning +// for now. +ABSL_INTERNAL_DISABLE_DEPRECATED_DECLARATION_WARNING absl::StatusOr LegacyClientIdleFilter::Create( const ChannelArgs& args, ChannelFilter::Args filter_args) { LegacyClientIdleFilter filter(filter_args.channel_stack(), @@ -141,6 +146,7 @@ absl::StatusOr LegacyMaxAgeFilter::Create( Config::FromChannelArgs(args)); return absl::StatusOr(std::move(filter)); } +ABSL_INTERNAL_RESTORE_DEPRECATED_DECLARATION_WARNING void LegacyMaxAgeFilter::Shutdown() { max_age_activity_.Reset(); diff --git a/src/core/ext/filters/fault_injection/fault_injection_filter.cc b/src/core/ext/filters/fault_injection/fault_injection_filter.cc index 1ab908fb74d..2464d01d7db 100644 --- a/src/core/ext/filters/fault_injection/fault_injection_filter.cc +++ b/src/core/ext/filters/fault_injection/fault_injection_filter.cc @@ -141,9 +141,7 @@ absl::StatusOr FaultInjectionFilter::Create( } FaultInjectionFilter::FaultInjectionFilter(ChannelFilter::Args filter_args) - : index_(grpc_channel_stack_filter_instance_number( - filter_args.channel_stack(), - filter_args.uninitialized_channel_element())), + : index_(filter_args.instance_id()), service_config_parser_index_( FaultInjectionServiceConfigParser::ParserIndex()), mu_(new Mutex) {} diff --git a/src/core/ext/filters/rbac/rbac_filter.cc b/src/core/ext/filters/rbac/rbac_filter.cc index c9b74aab5c7..68c5e3d7699 100644 --- a/src/core/ext/filters/rbac/rbac_filter.cc +++ b/src/core/ext/filters/rbac/rbac_filter.cc @@ -88,9 +88,7 @@ absl::StatusOr RbacFilter::Create(const ChannelArgs& args, if (auth_context == nullptr) { return GRPC_ERROR_CREATE("No auth context found"); } - return RbacFilter(grpc_channel_stack_filter_instance_number( - filter_args.channel_stack(), - filter_args.uninitialized_channel_element()), + return RbacFilter(filter_args.instance_id(), EvaluateArgs::PerChannelArgs(auth_context, args)); } diff --git a/src/core/ext/filters/stateful_session/stateful_session_filter.cc b/src/core/ext/filters/stateful_session/stateful_session_filter.cc index f9dab68ab5a..9345edeed7d 100644 --- a/src/core/ext/filters/stateful_session/stateful_session_filter.cc +++ b/src/core/ext/filters/stateful_session/stateful_session_filter.cc @@ -78,9 +78,7 @@ absl::StatusOr StatefulSessionFilter::Create( } StatefulSessionFilter::StatefulSessionFilter(ChannelFilter::Args filter_args) - : index_(grpc_channel_stack_filter_instance_number( - filter_args.channel_stack(), - filter_args.uninitialized_channel_element())), + : index_(filter_args.instance_id()), service_config_parser_index_( StatefulSessionServiceConfigParser::ParserIndex()) {} diff --git a/src/core/lib/channel/promise_based_filter.h b/src/core/lib/channel/promise_based_filter.h index 9b5c5ae4f69..25229d16376 100644 --- a/src/core/lib/channel/promise_based_filter.h +++ b/src/core/lib/channel/promise_based_filter.h @@ -88,9 +88,19 @@ class ChannelFilter { grpc_channel_element* channel_element) : channel_stack_(channel_stack), channel_element_(channel_element) {} + ABSL_DEPRECATED("Direct access to channel stack is deprecated") grpc_channel_stack* channel_stack() const { return channel_stack_; } - grpc_channel_element* uninitialized_channel_element() { - return channel_element_; + + // Get the instance id of this filter. + // This id is unique amongst all filters /of the same type/ and densely + // packed (starting at 0) for a given channel stack instantiation. + // eg. for a stack with filter types A B C A B D A the instance ids would be + // 0 0 0 1 1 0 2. + // This is useful for filters that need to store per-instance data in a + // parallel data structure. + size_t instance_id() const { + return grpc_channel_stack_filter_instance_number(channel_stack_, + channel_element_); } private: