From 451436e154d4f23f1860d9bfba93493dbed50e13 Mon Sep 17 00:00:00 2001 From: Karthik Ravi Shankar Date: Wed, 10 Jun 2020 16:57:53 -0700 Subject: [PATCH 01/11] Revert https://github.com/grpc/grpc/pull/18345 Move ResourceQuota to grpc. --- include/grpcpp/resource_quota.h | 55 ++++++++++++++++++++++++---- include/grpcpp/server_builder_impl.h | 5 +-- src/cpp/common/channel_arguments.cc | 2 +- src/cpp/common/resource_quota_cc.cc | 4 +- src/cpp/server/server_builder.cc | 2 +- 5 files changed, 53 insertions(+), 15 deletions(-) diff --git a/include/grpcpp/resource_quota.h b/include/grpcpp/resource_quota.h index 333767b95c5..16c0e35385b 100644 --- a/include/grpcpp/resource_quota.h +++ b/include/grpcpp/resource_quota.h @@ -1,6 +1,6 @@ /* * - * Copyright 2019 gRPC authors. + * Copyright 2016 gRPC authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,14 +16,53 @@ * */ -#ifndef GRPCPP_RESOURCE_QUOTA_H -#define GRPCPP_RESOURCE_QUOTA_H +#ifndef GRPCPP_RESOURCE_QUOTA_IMPL_H +#define GRPCPP_RESOURCE_QUOTA_IMPL_H -#include +struct grpc_resource_quota; -namespace grpc { +#include +#include -typedef ::grpc_impl::ResourceQuota ResourceQuota; -} // namespace grpc +namespace grpc_impl { -#endif // GRPCPP_RESOURCE_QUOTA_H +/// ResourceQuota represents a bound on memory and thread usage by the gRPC +/// library. A ResourceQuota can be attached to a server (via \a ServerBuilder), +/// or a client channel (via \a ChannelArguments). +/// gRPC will attempt to keep memory and threads used by all attached entities +/// below the ResourceQuota bound. +class ResourceQuota final : private ::grpc::GrpcLibraryCodegen { + public: + /// \param name - a unique name for this ResourceQuota. + explicit ResourceQuota(const grpc::string& name); + ResourceQuota(); + ~ResourceQuota(); + + /// Resize this \a ResourceQuota to a new size. If \a new_size is smaller + /// than the current size of the pool, memory usage will be monotonically + /// decreased until it falls under \a new_size. + /// No time bound is given for this to occur however. + ResourceQuota& Resize(size_t new_size); + + /// Set the max number of threads that can be allocated from this + /// ResourceQuota object. + /// + /// If the new_max_threads value is smaller than the current value, no new + /// threads are allocated until the number of active threads fall below + /// new_max_threads. There is no time bound on when this may happen i.e none + /// of the current threads are forcefully destroyed and all threads run their + /// normal course. + ResourceQuota& SetMaxThreads(int new_max_threads); + + grpc_resource_quota* c_resource_quota() const { return impl_; } + + private: + ResourceQuota(const ResourceQuota& rhs); + ResourceQuota& operator=(const ResourceQuota& rhs); + + grpc_resource_quota* const impl_; +}; + +} // namespace grpc_impl + +#endif // GRPCPP_RESOURCE_QUOTA_IMPL_H diff --git a/include/grpcpp/server_builder_impl.h b/include/grpcpp/server_builder_impl.h index 171f1ac6c84..d4d5956d8b8 100644 --- a/include/grpcpp/server_builder_impl.h +++ b/include/grpcpp/server_builder_impl.h @@ -41,7 +41,6 @@ struct grpc_resource_quota; namespace grpc_impl { class CompletionQueue; -class ResourceQuota; class Server; class ServerCompletionQueue; class ServerCredentials; @@ -50,6 +49,7 @@ class ServerCredentials; namespace grpc { class AsyncGenericService; +class ResourceQuota; class Service; namespace testing { class ServerBuilderPluginTest; @@ -228,8 +228,7 @@ class ServerBuilder { grpc_compression_algorithm algorithm); /// Set the attached buffer pool for this server - ServerBuilder& SetResourceQuota( - const grpc_impl::ResourceQuota& resource_quota); + ServerBuilder& SetResourceQuota(const ResourceQuota& resource_quota); ServerBuilder& SetOption(std::unique_ptr option); diff --git a/src/cpp/common/channel_arguments.cc b/src/cpp/common/channel_arguments.cc index 4ef45970cdd..ba0220ab876 100644 --- a/src/cpp/common/channel_arguments.cc +++ b/src/cpp/common/channel_arguments.cc @@ -142,7 +142,7 @@ void ChannelArguments::SetUserAgentPrefix( } void ChannelArguments::SetResourceQuota( - const grpc_impl::ResourceQuota& resource_quota) { + const grpc::ResourceQuota& resource_quota) { SetPointerWithVtable(GRPC_ARG_RESOURCE_QUOTA, resource_quota.c_resource_quota(), grpc_resource_quota_arg_vtable()); diff --git a/src/cpp/common/resource_quota_cc.cc b/src/cpp/common/resource_quota_cc.cc index 4fab2975d89..276e5f79548 100644 --- a/src/cpp/common/resource_quota_cc.cc +++ b/src/cpp/common/resource_quota_cc.cc @@ -19,7 +19,7 @@ #include #include -namespace grpc_impl { +namespace grpc { ResourceQuota::ResourceQuota() : impl_(grpc_resource_quota_create(nullptr)) {} @@ -37,4 +37,4 @@ ResourceQuota& ResourceQuota::SetMaxThreads(int new_max_threads) { grpc_resource_quota_set_max_threads(impl_, new_max_threads); return *this; } -} // namespace grpc_impl +} // namespace grpc diff --git a/src/cpp/server/server_builder.cc b/src/cpp/server/server_builder.cc index d5e93476532..0acb486149d 100644 --- a/src/cpp/server/server_builder.cc +++ b/src/cpp/server/server_builder.cc @@ -193,7 +193,7 @@ ServerBuilder& ServerBuilder::SetDefaultCompressionAlgorithm( } ServerBuilder& ServerBuilder::SetResourceQuota( - const grpc_impl::ResourceQuota& resource_quota) { + const grpc::ResourceQuota& resource_quota) { if (resource_quota_ != nullptr) { grpc_resource_quota_unref(resource_quota_); } From f795f604c89cfe646b9ed0911228a2b2bae8a5c8 Mon Sep 17 00:00:00 2001 From: Karthik Ravi Shankar Date: Wed, 10 Jun 2020 17:27:22 -0700 Subject: [PATCH 02/11] Add build changes to move ResourceQuota from grpc_impl to grpc --- BUILD | 1 - BUILD.gn | 1 - CMakeLists.txt | 2 -- Makefile | 2 -- gRPC-C++.podspec | 1 - include/grpcpp/resource_quota.h | 10 +++++----- include/grpcpp/server_builder_impl.h | 2 +- tools/doxygen/Doxyfile.c++ | 1 - tools/doxygen/Doxyfile.c++.internal | 1 - 9 files changed, 6 insertions(+), 15 deletions(-) diff --git a/BUILD b/BUILD index 0d2a7ca058c..655a1b017b9 100644 --- a/BUILD +++ b/BUILD @@ -250,7 +250,6 @@ GRPCXX_PUBLIC_HDRS = [ "include/grpcpp/impl/server_initializer_impl.h", "include/grpcpp/impl/service_type.h", "include/grpcpp/resource_quota.h", - "include/grpcpp/resource_quota_impl.h", "include/grpcpp/security/auth_context.h", "include/grpcpp/security/auth_metadata_processor.h", "include/grpcpp/security/auth_metadata_processor_impl.h", diff --git a/BUILD.gn b/BUILD.gn index a16d298f1be..e444c251c08 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -1175,7 +1175,6 @@ config("grpc_config") { "include/grpcpp/impl/server_initializer_impl.h", "include/grpcpp/impl/service_type.h", "include/grpcpp/resource_quota.h", - "include/grpcpp/resource_quota_impl.h", "include/grpcpp/security/auth_context.h", "include/grpcpp/security/auth_metadata_processor.h", "include/grpcpp/security/auth_metadata_processor_impl.h", diff --git a/CMakeLists.txt b/CMakeLists.txt index 323f7800627..aabacf1cc0b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2737,7 +2737,6 @@ foreach(_hdr include/grpcpp/impl/server_initializer_impl.h include/grpcpp/impl/service_type.h include/grpcpp/resource_quota.h - include/grpcpp/resource_quota_impl.h include/grpcpp/security/auth_context.h include/grpcpp/security/auth_metadata_processor.h include/grpcpp/security/auth_metadata_processor_impl.h @@ -3431,7 +3430,6 @@ foreach(_hdr include/grpcpp/impl/server_initializer_impl.h include/grpcpp/impl/service_type.h include/grpcpp/resource_quota.h - include/grpcpp/resource_quota_impl.h include/grpcpp/security/auth_context.h include/grpcpp/security/auth_metadata_processor.h include/grpcpp/security/auth_metadata_processor_impl.h diff --git a/Makefile b/Makefile index bbaccc44513..75fab892c73 100644 --- a/Makefile +++ b/Makefile @@ -4933,7 +4933,6 @@ PUBLIC_HEADERS_CXX += \ include/grpcpp/impl/server_initializer_impl.h \ include/grpcpp/impl/service_type.h \ include/grpcpp/resource_quota.h \ - include/grpcpp/resource_quota_impl.h \ include/grpcpp/security/auth_context.h \ include/grpcpp/security/auth_metadata_processor.h \ include/grpcpp/security/auth_metadata_processor_impl.h \ @@ -5632,7 +5631,6 @@ PUBLIC_HEADERS_CXX += \ include/grpcpp/impl/server_initializer_impl.h \ include/grpcpp/impl/service_type.h \ include/grpcpp/resource_quota.h \ - include/grpcpp/resource_quota_impl.h \ include/grpcpp/security/auth_context.h \ include/grpcpp/security/auth_metadata_processor.h \ include/grpcpp/security/auth_metadata_processor_impl.h \ diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index d8f404abc1f..c786ae57a24 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -164,7 +164,6 @@ Pod::Spec.new do |s| 'include/grpcpp/impl/server_initializer_impl.h', 'include/grpcpp/impl/service_type.h', 'include/grpcpp/resource_quota.h', - 'include/grpcpp/resource_quota_impl.h', 'include/grpcpp/security/auth_context.h', 'include/grpcpp/security/auth_metadata_processor.h', 'include/grpcpp/security/auth_metadata_processor_impl.h', diff --git a/include/grpcpp/resource_quota.h b/include/grpcpp/resource_quota.h index 16c0e35385b..3c75b271e6d 100644 --- a/include/grpcpp/resource_quota.h +++ b/include/grpcpp/resource_quota.h @@ -16,15 +16,15 @@ * */ -#ifndef GRPCPP_RESOURCE_QUOTA_IMPL_H -#define GRPCPP_RESOURCE_QUOTA_IMPL_H +#ifndef GRPCPP_RESOURCE_QUOTA_H +#define GRPCPP_RESOURCE_QUOTA_H struct grpc_resource_quota; #include #include -namespace grpc_impl { +namespace grpc { /// ResourceQuota represents a bound on memory and thread usage by the gRPC /// library. A ResourceQuota can be attached to a server (via \a ServerBuilder), @@ -63,6 +63,6 @@ class ResourceQuota final : private ::grpc::GrpcLibraryCodegen { grpc_resource_quota* const impl_; }; -} // namespace grpc_impl +} // namespace grpc -#endif // GRPCPP_RESOURCE_QUOTA_IMPL_H +#endif // GRPCPP_RESOURCE_QUOTA_H diff --git a/include/grpcpp/server_builder_impl.h b/include/grpcpp/server_builder_impl.h index d4d5956d8b8..59aa54f4eca 100644 --- a/include/grpcpp/server_builder_impl.h +++ b/include/grpcpp/server_builder_impl.h @@ -228,7 +228,7 @@ class ServerBuilder { grpc_compression_algorithm algorithm); /// Set the attached buffer pool for this server - ServerBuilder& SetResourceQuota(const ResourceQuota& resource_quota); + ServerBuilder& SetResourceQuota(const grpc::ResourceQuota& resource_quota); ServerBuilder& SetOption(std::unique_ptr option); diff --git a/tools/doxygen/Doxyfile.c++ b/tools/doxygen/Doxyfile.c++ index f9fcb6142ee..f1f50fbebe3 100644 --- a/tools/doxygen/Doxyfile.c++ +++ b/tools/doxygen/Doxyfile.c++ @@ -1023,7 +1023,6 @@ include/grpcpp/impl/server_initializer.h \ include/grpcpp/impl/server_initializer_impl.h \ include/grpcpp/impl/service_type.h \ include/grpcpp/resource_quota.h \ -include/grpcpp/resource_quota_impl.h \ include/grpcpp/security/auth_context.h \ include/grpcpp/security/auth_metadata_processor.h \ include/grpcpp/security/auth_metadata_processor_impl.h \ diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index 613e824f4d7..1856fc54f89 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -1023,7 +1023,6 @@ include/grpcpp/impl/server_initializer.h \ include/grpcpp/impl/server_initializer_impl.h \ include/grpcpp/impl/service_type.h \ include/grpcpp/resource_quota.h \ -include/grpcpp/resource_quota_impl.h \ include/grpcpp/security/auth_context.h \ include/grpcpp/security/auth_metadata_processor.h \ include/grpcpp/security/auth_metadata_processor_impl.h \ From 89763a96fd41e8cfd11469db6e4092253c1971f5 Mon Sep 17 00:00:00 2001 From: Karthik Ravi Shankar Date: Wed, 10 Jun 2020 17:36:58 -0700 Subject: [PATCH 03/11] Move OpenCensus back to ::grpc from ::grpc_impl Revert - Revert "Revert "Fold opencensus into grpc_impl namespace"" #18396 --- BUILD | 1 - include/grpcpp/opencensus.h | 42 ++++++----- include/grpcpp/opencensus_impl.h | 47 ------------- src/cpp/ext/filters/census/grpc_plugin.cc | 69 +++++++++---------- src/cpp/ext/filters/census/views.cc | 15 ++++ .../microbenchmarks/bm_opencensus_plugin.cc | 8 +-- 6 files changed, 76 insertions(+), 106 deletions(-) delete mode 100644 include/grpcpp/opencensus_impl.h diff --git a/BUILD b/BUILD index 8fe2d6d2e15..a62ca7e8ec2 100644 --- a/BUILD +++ b/BUILD @@ -2322,7 +2322,6 @@ grpc_cc_library( ], hdrs = [ "include/grpcpp/opencensus.h", - "include/grpcpp/opencensus_impl.h", "src/cpp/ext/filters/census/channel_filter.h", "src/cpp/ext/filters/census/client_filter.h", "src/cpp/ext/filters/census/context.h", diff --git a/include/grpcpp/opencensus.h b/include/grpcpp/opencensus.h index 39bac6bc28c..dbcb7c9caeb 100644 --- a/include/grpcpp/opencensus.h +++ b/include/grpcpp/opencensus.h @@ -1,6 +1,6 @@ /* * - * Copyright 2018 gRPC authors. + * Copyright 2019 gRPC authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,24 +16,32 @@ * */ -#ifndef GRPCPP_OPENCENSUS_H -#define GRPCPP_OPENCENSUS_H +#ifndef GRPCPP_OPENCENSUS_IMPL_H +#define GRPCPP_OPENCENSUS_IMPL_H -#include "grpcpp/opencensus_impl.h" +#include "opencensus/trace/span.h" -namespace grpc { +namespace grpc_impl { +class ServerContext; +// These symbols in this file will not be included in the binary unless +// grpc_opencensus_plugin build target was added as a dependency. At the moment +// it is only setup to be built with Bazel. -static inline void RegisterOpenCensusPlugin() { - ::grpc_impl::RegisterOpenCensusPlugin(); -} -static inline void RegisterOpenCensusViewsForExport() { - ::grpc_impl::RegisterOpenCensusViewsForExport(); -} -static inline ::opencensus::trace::Span GetSpanFromServerContext( - ::grpc_impl::ServerContext* context) { - return ::grpc_impl::GetSpanFromServerContext(context); -} +// Registers the OpenCensus plugin with gRPC, so that it will be used for future +// RPCs. This must be called before any views are created. +void RegisterOpenCensusPlugin(); -} // namespace grpc +// RPC stats definitions, defined by +// https://github.com/census-instrumentation/opencensus-specs/blob/master/stats/gRPC.md -#endif // GRPCPP_OPENCENSUS_H +// Registers the cumulative gRPC views so that they will be exported by any +// registered stats exporter. For on-task stats, construct a View using the +// ViewDescriptors below. +void RegisterOpenCensusViewsForExport(); + +// Returns the tracing Span for the current RPC. +::opencensus::trace::Span GetSpanFromServerContext(ServerContext* context); + +} // namespace grpc_impl + +#endif // GRPCPP_OPENCENSUS_IMPL_H diff --git a/include/grpcpp/opencensus_impl.h b/include/grpcpp/opencensus_impl.h deleted file mode 100644 index dbcb7c9caeb..00000000000 --- a/include/grpcpp/opencensus_impl.h +++ /dev/null @@ -1,47 +0,0 @@ -/* - * - * Copyright 2019 gRPC authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ - -#ifndef GRPCPP_OPENCENSUS_IMPL_H -#define GRPCPP_OPENCENSUS_IMPL_H - -#include "opencensus/trace/span.h" - -namespace grpc_impl { -class ServerContext; -// These symbols in this file will not be included in the binary unless -// grpc_opencensus_plugin build target was added as a dependency. At the moment -// it is only setup to be built with Bazel. - -// Registers the OpenCensus plugin with gRPC, so that it will be used for future -// RPCs. This must be called before any views are created. -void RegisterOpenCensusPlugin(); - -// RPC stats definitions, defined by -// https://github.com/census-instrumentation/opencensus-specs/blob/master/stats/gRPC.md - -// Registers the cumulative gRPC views so that they will be exported by any -// registered stats exporter. For on-task stats, construct a View using the -// ViewDescriptors below. -void RegisterOpenCensusViewsForExport(); - -// Returns the tracing Span for the current RPC. -::opencensus::trace::Span GetSpanFromServerContext(ServerContext* context); - -} // namespace grpc_impl - -#endif // GRPCPP_OPENCENSUS_IMPL_H diff --git a/src/cpp/ext/filters/census/grpc_plugin.cc b/src/cpp/ext/filters/census/grpc_plugin.cc index 63d6f1bde48..3e07f3fd6d4 100644 --- a/src/cpp/ext/filters/census/grpc_plugin.cc +++ b/src/cpp/ext/filters/census/grpc_plugin.cc @@ -31,6 +31,40 @@ namespace grpc { +void RegisterOpenCensusPlugin() { + grpc::RegisterChannelFilter( + "opencensus_client", GRPC_CLIENT_CHANNEL, INT_MAX /* priority */, + nullptr /* condition function */); + grpc::RegisterChannelFilter( + "opencensus_server", GRPC_SERVER_CHANNEL, INT_MAX /* priority */, + nullptr /* condition function */); + + // Access measures to ensure they are initialized. Otherwise, creating a view + // before the first RPC would cause an error. + grpc::RpcClientSentBytesPerRpc(); + grpc::RpcClientReceivedBytesPerRpc(); + grpc::RpcClientRoundtripLatency(); + grpc::RpcClientServerLatency(); + grpc::RpcClientSentMessagesPerRpc(); + grpc::RpcClientReceivedMessagesPerRpc(); + + grpc::RpcServerSentBytesPerRpc(); + grpc::RpcServerReceivedBytesPerRpc(); + grpc::RpcServerServerLatency(); + grpc::RpcServerSentMessagesPerRpc(); + grpc::RpcServerReceivedMessagesPerRpc(); +} + +::opencensus::trace::Span GetSpanFromServerContext( + grpc::ServerContext* context) { + return reinterpret_cast(context->census_context()) + ->Span(); +} + + + // These measure definitions should be kept in sync across opencensus // implementations--see // https://github.com/census-instrumentation/opencensus-java/blob/master/contrib/grpc_metrics/src/main/java/io/opencensus/contrib/grpc/metrics/RpcMeasureConstants.java. @@ -99,38 +133,3 @@ ABSL_CONST_INIT const absl::string_view ABSL_CONST_INIT const absl::string_view kRpcServerServerLatencyMeasureName = "grpc.io/server/server_latency"; } // namespace grpc -namespace grpc_impl { - -void RegisterOpenCensusPlugin() { - grpc::RegisterChannelFilter( - "opencensus_client", GRPC_CLIENT_CHANNEL, INT_MAX /* priority */, - nullptr /* condition function */); - grpc::RegisterChannelFilter( - "opencensus_server", GRPC_SERVER_CHANNEL, INT_MAX /* priority */, - nullptr /* condition function */); - - // Access measures to ensure they are initialized. Otherwise, creating a view - // before the first RPC would cause an error. - grpc::RpcClientSentBytesPerRpc(); - grpc::RpcClientReceivedBytesPerRpc(); - grpc::RpcClientRoundtripLatency(); - grpc::RpcClientServerLatency(); - grpc::RpcClientSentMessagesPerRpc(); - grpc::RpcClientReceivedMessagesPerRpc(); - - grpc::RpcServerSentBytesPerRpc(); - grpc::RpcServerReceivedBytesPerRpc(); - grpc::RpcServerServerLatency(); - grpc::RpcServerSentMessagesPerRpc(); - grpc::RpcServerReceivedMessagesPerRpc(); -} - -::opencensus::trace::Span GetSpanFromServerContext( - grpc::ServerContext* context) { - return reinterpret_cast(context->census_context()) - ->Span(); -} - -} // namespace grpc_impl diff --git a/src/cpp/ext/filters/census/views.cc b/src/cpp/ext/filters/census/views.cc index d7e3c81a955..df7874b39af 100644 --- a/src/cpp/ext/filters/census/views.cc +++ b/src/cpp/ext/filters/census/views.cc @@ -88,6 +88,21 @@ ViewDescriptor HourDescriptor() { } // namespace +void RegisterOpenCensusViewsForExport() { + ClientSentMessagesPerRpcCumulative().RegisterForExport(); + ClientSentBytesPerRpcCumulative().RegisterForExport(); + ClientReceivedMessagesPerRpcCumulative().RegisterForExport(); + ClientReceivedBytesPerRpcCumulative().RegisterForExport(); + ClientRoundtripLatencyCumulative().RegisterForExport(); + ClientServerLatencyCumulative().RegisterForExport(); + + ServerSentMessagesPerRpcCumulative().RegisterForExport(); + ServerSentBytesPerRpcCumulative().RegisterForExport(); + ServerReceivedMessagesPerRpcCumulative().RegisterForExport(); + ServerReceivedBytesPerRpcCumulative().RegisterForExport(); + ServerServerLatencyCumulative().RegisterForExport(); +} + // client cumulative const ViewDescriptor& ClientSentBytesPerRpcCumulative() { const static ViewDescriptor descriptor = diff --git a/test/cpp/microbenchmarks/bm_opencensus_plugin.cc b/test/cpp/microbenchmarks/bm_opencensus_plugin.cc index 908d34aa19f..8b50d249c8c 100644 --- a/test/cpp/microbenchmarks/bm_opencensus_plugin.cc +++ b/test/cpp/microbenchmarks/bm_opencensus_plugin.cc @@ -24,17 +24,13 @@ #include "absl/strings/str_cat.h" #include "include/grpc/grpc.h" #include "include/grpcpp/grpcpp.h" -#include "include/grpcpp/opencensus.h" #include "opencensus/stats/stats.h" #include "src/cpp/ext/filters/census/grpc_plugin.h" #include "src/proto/grpc/testing/echo.grpc.pb.h" #include "test/cpp/microbenchmarks/helpers.h" -using ::grpc::RegisterOpenCensusPlugin; -using ::grpc::RegisterOpenCensusViewsForExport; - absl::once_flag once; -void RegisterOnce() { absl::call_once(once, RegisterOpenCensusPlugin); } +void RegisterOnce() { absl::call_once(once, grpc::RegisterOpenCensusPlugin); } class EchoServer final : public grpc::testing::EchoTestService::Service { grpc::Status Echo(grpc::ServerContext* context, @@ -111,7 +107,7 @@ static void BM_E2eLatencyCensusEnabled(benchmark::State& state) { RegisterOnce(); // This we can safely repeat, and doing so clears accumulated data to avoid // initialization costs varying between runs. - RegisterOpenCensusViewsForExport(); + grpc::RegisterOpenCensusViewsForExport(); EchoServerThread server; std::unique_ptr stub = From e55267a6fdf0d779e1f2ce6005239599c8556cda Mon Sep 17 00:00:00 2001 From: Karthik Ravi Shankar Date: Wed, 10 Jun 2020 17:41:45 -0700 Subject: [PATCH 04/11] Resolve conflicts --- include/grpcpp/opencensus.h | 10 ++++---- src/cpp/ext/filters/census/grpc_plugin.cc | 28 +++++++++++------------ 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/include/grpcpp/opencensus.h b/include/grpcpp/opencensus.h index dbcb7c9caeb..0da949ab73c 100644 --- a/include/grpcpp/opencensus.h +++ b/include/grpcpp/opencensus.h @@ -16,12 +16,12 @@ * */ -#ifndef GRPCPP_OPENCENSUS_IMPL_H -#define GRPCPP_OPENCENSUS_IMPL_H +#ifndef GRPCPP_OPENCENSUS_H +#define GRPCPP_OPENCENSUS_H #include "opencensus/trace/span.h" -namespace grpc_impl { +namespace grpc { class ServerContext; // These symbols in this file will not be included in the binary unless // grpc_opencensus_plugin build target was added as a dependency. At the moment @@ -42,6 +42,6 @@ void RegisterOpenCensusViewsForExport(); // Returns the tracing Span for the current RPC. ::opencensus::trace::Span GetSpanFromServerContext(ServerContext* context); -} // namespace grpc_impl +} // namespace grpc -#endif // GRPCPP_OPENCENSUS_IMPL_H +#endif // GRPCPP_OPENCENSUS_H diff --git a/src/cpp/ext/filters/census/grpc_plugin.cc b/src/cpp/ext/filters/census/grpc_plugin.cc index 3e07f3fd6d4..70034067d2a 100644 --- a/src/cpp/ext/filters/census/grpc_plugin.cc +++ b/src/cpp/ext/filters/census/grpc_plugin.cc @@ -32,29 +32,29 @@ namespace grpc { void RegisterOpenCensusPlugin() { - grpc::RegisterChannelFilter( "opencensus_client", GRPC_CLIENT_CHANNEL, INT_MAX /* priority */, nullptr /* condition function */); - grpc::RegisterChannelFilter( "opencensus_server", GRPC_SERVER_CHANNEL, INT_MAX /* priority */, nullptr /* condition function */); // Access measures to ensure they are initialized. Otherwise, creating a view // before the first RPC would cause an error. - grpc::RpcClientSentBytesPerRpc(); - grpc::RpcClientReceivedBytesPerRpc(); - grpc::RpcClientRoundtripLatency(); - grpc::RpcClientServerLatency(); - grpc::RpcClientSentMessagesPerRpc(); - grpc::RpcClientReceivedMessagesPerRpc(); - - grpc::RpcServerSentBytesPerRpc(); - grpc::RpcServerReceivedBytesPerRpc(); - grpc::RpcServerServerLatency(); - grpc::RpcServerSentMessagesPerRpc(); - grpc::RpcServerReceivedMessagesPerRpc(); + RpcClientSentBytesPerRpc(); + RpcClientReceivedBytesPerRpc(); + RpcClientRoundtripLatency(); + RpcClientServerLatency(); + RpcClientSentMessagesPerRpc(); + RpcClientReceivedMessagesPerRpc(); + + RpcServerSentBytesPerRpc(); + RpcServerReceivedBytesPerRpc(); + RpcServerServerLatency(); + RpcServerSentMessagesPerRpc(); + RpcServerReceivedMessagesPerRpc(); } ::opencensus::trace::Span GetSpanFromServerContext( From 52931a4e1ac9fdd6de40be7101f85a7cd4d71a8b Mon Sep 17 00:00:00 2001 From: Hannah Shi Date: Fri, 19 Jun 2020 18:30:37 +0000 Subject: [PATCH 05/11] update comments of ext files to fit doxygen use --- src/php/ext/grpc/call.c | 7 ++++++- src/php/ext/grpc/call_credentials.c | 5 +++++ src/php/ext/grpc/channel.c | 5 +++++ src/php/ext/grpc/channel_credentials.c | 11 ++++++++--- src/php/ext/grpc/server.c | 5 +++++ src/php/ext/grpc/server_credentials.c | 11 ++++++++--- src/php/ext/grpc/timeval.c | 5 +++++ 7 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/php/ext/grpc/call.c b/src/php/ext/grpc/call.c index 66b26716b13..03f4d56901f 100644 --- a/src/php/ext/grpc/call.c +++ b/src/php/ext/grpc/call.c @@ -16,6 +16,11 @@ * */ +/** + * class Call + * @see https://github.com/grpc/grpc/tree/master/src/php/ext/grpc/call.c + */ + #include "call.h" #include @@ -193,7 +198,7 @@ zval *grpc_php_wrap_call(grpc_call *wrapped, bool owned TSRMLS_DC) { * Must not be closed. * @param string $method The method to call * @param Timeval $deadline_obj The deadline for completing the call - * @param string $host_override The host is set by user (optional) + * @param string $host_override = "" The host is set by user (optional) */ PHP_METHOD(Call, __construct) { zval *channel_obj; diff --git a/src/php/ext/grpc/call_credentials.c b/src/php/ext/grpc/call_credentials.c index 484b9b54bf4..a57a8410366 100644 --- a/src/php/ext/grpc/call_credentials.c +++ b/src/php/ext/grpc/call_credentials.c @@ -16,6 +16,11 @@ * */ +/** + * class CallCredentials + * @see https://github.com/grpc/grpc/tree/master/src/php/ext/grpc/call_credentials.c + */ + #include "call_credentials.h" #include diff --git a/src/php/ext/grpc/channel.c b/src/php/ext/grpc/channel.c index 059e8333db1..47483812b25 100644 --- a/src/php/ext/grpc/channel.c +++ b/src/php/ext/grpc/channel.c @@ -16,6 +16,11 @@ * */ +/** + * class Channel + * @see https://github.com/grpc/grpc/tree/master/src/php/ext/grpc/channel.c + */ + #include "channel.h" #include diff --git a/src/php/ext/grpc/channel_credentials.c b/src/php/ext/grpc/channel_credentials.c index 4fa99b1012d..a1611d13518 100644 --- a/src/php/ext/grpc/channel_credentials.c +++ b/src/php/ext/grpc/channel_credentials.c @@ -16,6 +16,11 @@ * */ +/** + * class ChannelCredentials + * @see https://github.com/grpc/grpc/tree/master/src/php/ext/grpc/channel_credentials.c + */ + #include "channel_credentials.h" #include @@ -134,10 +139,10 @@ PHP_METHOD(ChannelCredentials, createDefault) { /** * Create SSL credentials. - * @param string $pem_root_certs PEM encoding of the server root certificates - * @param string $pem_key_cert_pair.private_key PEM encoding of the client's + * @param string $pem_root_certs = "" PEM encoding of the server root certificates (optional) + * @param string $private_key = "" PEM encoding of the client's * private key (optional) - * @param string $pem_key_cert_pair.cert_chain PEM encoding of the client's + * @param string $cert_chain = "" PEM encoding of the client's * certificate chain (optional) * @return ChannelCredentials The new SSL credentials object */ diff --git a/src/php/ext/grpc/server.c b/src/php/ext/grpc/server.c index 8c7eaee203f..7f3c3b8e3bb 100644 --- a/src/php/ext/grpc/server.c +++ b/src/php/ext/grpc/server.c @@ -16,6 +16,11 @@ * */ +/** + * class Server + * @see https://github.com/grpc/grpc/tree/master/src/php/ext/grpc/server.c + */ + #include "server.h" #include diff --git a/src/php/ext/grpc/server_credentials.c b/src/php/ext/grpc/server_credentials.c index 72ce50fff72..2c2f9c185f3 100644 --- a/src/php/ext/grpc/server_credentials.c +++ b/src/php/ext/grpc/server_credentials.c @@ -16,6 +16,11 @@ * */ +/** + * class ServerCredentials + * @see https://github.com/grpc/grpc/tree/master/src/php/ext/grpc/server_credentials.c + */ + #include "server_credentials.h" #include @@ -56,9 +61,9 @@ zval *grpc_php_wrap_server_credentials(grpc_server_credentials /** * Create SSL credentials. - * @param string pem_root_certs PEM encoding of the server root certificates - * @param string pem_private_key PEM encoding of the client's private key - * @param string pem_cert_chain PEM encoding of the client's certificate chain + * @param string $pem_root_certs PEM encoding of the server root certificates + * @param string $pem_private_key PEM encoding of the client's private key + * @param string $pem_cert_chain PEM encoding of the client's certificate chain * @return Credentials The new SSL credentials object */ PHP_METHOD(ServerCredentials, createSsl) { diff --git a/src/php/ext/grpc/timeval.c b/src/php/ext/grpc/timeval.c index 8f0048def6e..3cfab86849c 100644 --- a/src/php/ext/grpc/timeval.c +++ b/src/php/ext/grpc/timeval.c @@ -16,6 +16,11 @@ * */ +/** + * class Timeval + * @see https://github.com/grpc/grpc/tree/master/src/php/ext/grpc/timeval.c + */ + #include "timeval.h" #include From 11a29eb95a2b1e7e1cb583b782fb1883cd40f775 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Mon, 22 Jun 2020 10:22:19 -0700 Subject: [PATCH 06/11] Implement methods to access auth context and peer info --- .../grpc/_cython/_cygrpc/aio/server.pyx.pxi | 49 +++++ .../grpc/experimental/aio/_base_server.py | 43 +++- src/python/grpcio_tests/tests_aio/tests.json | 2 + .../tests_aio/unit/auth_context_test.py | 194 ++++++++++++++++++ .../tests_aio/unit/context_peer_test.py | 65 ++++++ 5 files changed, 352 insertions(+), 1 deletion(-) create mode 100644 src/python/grpcio_tests/tests_aio/unit/auth_context_test.py create mode 100644 src/python/grpcio_tests/tests_aio/unit/context_peer_test.py diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/aio/server.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/aio/server.pyx.pxi index b842ec6f2ba..63dbfdd75c0 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/aio/server.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/aio/server.pyx.pxi @@ -213,6 +213,43 @@ cdef class _ServicerContext: def disable_next_message_compression(self): self._rpc_state.disable_next_compression = True + def peer(self): + cdef char *c_peer = NULL + c_peer = grpc_call_get_peer(self._rpc_state.call) + peer = (c_peer).decode('utf8') + gpr_free(c_peer) + return peer + + def peer_identities(self): + cdef Call query_call = Call() + query_call.c_call = self._rpc_state.call + identities = peer_identities(query_call) + query_call.c_call = NULL + return identities + + def peer_identity_key(self): + cdef Call query_call = Call() + query_call.c_call = self._rpc_state.call + identity_key = peer_identity_key(query_call) + query_call.c_call = NULL + if identity_key: + return identity_key.decode('utf8') + else: + return None + + def auth_context(self): + cdef Call query_call = Call() + query_call.c_call = self._rpc_state.call + bytes_ctx = auth_context(query_call) + query_call.c_call = NULL + if bytes_ctx: + ctx = {} + for key in bytes_ctx: + ctx[key.decode('utf8')] = bytes_ctx[key] + return ctx + else: + return {} + cdef class _SyncServicerContext: """Sync servicer context for sync handler compatibility.""" @@ -260,6 +297,18 @@ cdef class _SyncServicerContext: def add_callback(self, object callback): self._callbacks.append(callback) + def peer(self): + return self._context.peer() + + def peer_identities(self): + return self._context.peer_identities() + + def peer_identity_key(self): + return self._context.peer_identity_key() + + def auth_context(self): + return self._context.auth_context() + async def _run_interceptor(object interceptors, object query_handler, object handler_call_details): diff --git a/src/python/grpcio/grpc/experimental/aio/_base_server.py b/src/python/grpcio/grpc/experimental/aio/_base_server.py index 86c15fc86b0..926c8651714 100644 --- a/src/python/grpcio/grpc/experimental/aio/_base_server.py +++ b/src/python/grpcio/grpc/experimental/aio/_base_server.py @@ -14,7 +14,7 @@ """Abstract base classes for server-side classes.""" import abc -from typing import Generic, Optional, Sequence +from typing import Generic, Mapping, Optional, Iterable, Sequence import grpc @@ -251,3 +251,44 @@ class ServicerContext(Generic[RequestType, ResponseType], abc.ABC): This method will override any compression configuration set during server creation or set on the call. """ + + @abc.abstractmethod + def peer(self) -> str: + """Identifies the peer that invoked the RPC being serviced. + + Returns: + A string identifying the peer that invoked the RPC being serviced. + The string format is determined by gRPC runtime. + """ + + @abc.abstractmethod + def peer_identities(self) -> Optional[Iterable[bytes]]: + """Gets one or more peer identity(s). + + Equivalent to + servicer_context.auth_context().get(servicer_context.peer_identity_key()) + + Returns: + An iterable of the identities, or None if the call is not + authenticated. Each identity is returned as a raw bytes type. + """ + + @abc.abstractmethod + def peer_identity_key(self) -> Optional[str]: + """The auth property used to identify the peer. + + For example, "x509_common_name" or "x509_subject_alternative_name" are + used to identify an SSL peer. + + Returns: + The auth property (string) that indicates the + peer identity, or None if the call is not authenticated. + """ + + @abc.abstractmethod + def auth_context(self) -> Mapping[str, Iterable[bytes]]: + """Gets the auth context for the call. + + Returns: + A map of strings to an iterable of bytes for each auth property. + """ diff --git a/src/python/grpcio_tests/tests_aio/tests.json b/src/python/grpcio_tests/tests_aio/tests.json index f01d7d0570d..68ff13e5ccc 100644 --- a/src/python/grpcio_tests/tests_aio/tests.json +++ b/src/python/grpcio_tests/tests_aio/tests.json @@ -9,6 +9,7 @@ "unit._metadata_test.TestTypeMetadata", "unit.abort_test.TestAbort", "unit.aio_rpc_error_test.TestAioRpcError", + "unit.auth_context_test.TestAuthContext", "unit.call_test.TestStreamStreamCall", "unit.call_test.TestStreamUnaryCall", "unit.call_test.TestUnaryStreamCall", @@ -16,6 +17,7 @@ "unit.channel_argument_test.TestChannelArgument", "unit.channel_ready_test.TestChannelReady", "unit.channel_test.TestChannel", + "unit.context_peer.TestContextPeer", "unit.client_stream_stream_interceptor_test.TestStreamStreamClientInterceptor", "unit.client_stream_unary_interceptor_test.TestStreamUnaryClientInterceptor", "unit.client_unary_stream_interceptor_test.TestUnaryStreamClientInterceptor", diff --git a/src/python/grpcio_tests/tests_aio/unit/auth_context_test.py b/src/python/grpcio_tests/tests_aio/unit/auth_context_test.py new file mode 100644 index 00000000000..fb303714682 --- /dev/null +++ b/src/python/grpcio_tests/tests_aio/unit/auth_context_test.py @@ -0,0 +1,194 @@ +# Copyright 2020 The gRPC Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Porting auth context tests from sync stack.""" + +import pickle +import unittest +import logging + +import grpc +from grpc.experimental import aio +from grpc.experimental import session_cache +import six + +from tests.unit import resources +from tests_aio.unit._test_base import AioTestBase + +_REQUEST = b'\x00\x00\x00' +_RESPONSE = b'\x00\x00\x00' + +_UNARY_UNARY = '/test/UnaryUnary' + +_SERVER_HOST_OVERRIDE = 'foo.test.google.fr' +_CLIENT_IDS = ( + b'*.test.google.fr', + b'waterzooi.test.google.be', + b'*.test.youtube.com', + b'192.168.1.3', +) +_ID = 'id' +_ID_KEY = 'id_key' +_AUTH_CTX = 'auth_ctx' + +_PRIVATE_KEY = resources.private_key() +_CERTIFICATE_CHAIN = resources.certificate_chain() +_TEST_ROOT_CERTIFICATES = resources.test_root_certificates() +_SERVER_CERTS = ((_PRIVATE_KEY, _CERTIFICATE_CHAIN),) +_PROPERTY_OPTIONS = (( + 'grpc.ssl_target_name_override', + _SERVER_HOST_OVERRIDE, +),) + + +async def handle_unary_unary(unused_request: bytes, + servicer_context: aio.ServicerContext): + return pickle.dumps({ + _ID: servicer_context.peer_identities(), + _ID_KEY: servicer_context.peer_identity_key(), + _AUTH_CTX: servicer_context.auth_context() + }) + + +class TestAuthContext(AioTestBase): + + async def test_insecure(self): + handler = grpc.method_handlers_generic_handler('test', { + 'UnaryUnary': + grpc.unary_unary_rpc_method_handler(handle_unary_unary) + }) + server = aio.server() + server.add_generic_rpc_handlers((handler,)) + port = server.add_insecure_port('[::]:0') + await server.start() + + async with aio.insecure_channel('localhost:%d' % port) as channel: + response = await channel.unary_unary(_UNARY_UNARY)(_REQUEST) + await server.stop(None) + + auth_data = pickle.loads(response) + self.assertIsNone(auth_data[_ID]) + self.assertIsNone(auth_data[_ID_KEY]) + self.assertDictEqual({}, auth_data[_AUTH_CTX]) + + async def test_secure_no_cert(self): + handler = grpc.method_handlers_generic_handler('test', { + 'UnaryUnary': + grpc.unary_unary_rpc_method_handler(handle_unary_unary) + }) + server = aio.server() + server.add_generic_rpc_handlers((handler,)) + server_cred = grpc.ssl_server_credentials(_SERVER_CERTS) + port = server.add_secure_port('[::]:0', server_cred) + await server.start() + + channel_creds = grpc.ssl_channel_credentials( + root_certificates=_TEST_ROOT_CERTIFICATES) + channel = aio.secure_channel('localhost:{}'.format(port), + channel_creds, + options=_PROPERTY_OPTIONS) + response = await channel.unary_unary(_UNARY_UNARY)(_REQUEST) + await channel.close() + await server.stop(None) + + auth_data = pickle.loads(response) + self.assertIsNone(auth_data[_ID]) + self.assertIsNone(auth_data[_ID_KEY]) + self.assertDictEqual( + { + 'security_level': [b'TSI_PRIVACY_AND_INTEGRITY'], + 'transport_security_type': [b'ssl'], + 'ssl_session_reused': [b'false'], + }, auth_data[_AUTH_CTX]) + + async def test_secure_client_cert(self): + handler = grpc.method_handlers_generic_handler('test', { + 'UnaryUnary': + grpc.unary_unary_rpc_method_handler(handle_unary_unary) + }) + server = aio.server() + server.add_generic_rpc_handlers((handler,)) + server_cred = grpc.ssl_server_credentials( + _SERVER_CERTS, + root_certificates=_TEST_ROOT_CERTIFICATES, + require_client_auth=True) + port = server.add_secure_port('[::]:0', server_cred) + await server.start() + + channel_creds = grpc.ssl_channel_credentials( + root_certificates=_TEST_ROOT_CERTIFICATES, + private_key=_PRIVATE_KEY, + certificate_chain=_CERTIFICATE_CHAIN) + channel = aio.secure_channel('localhost:{}'.format(port), + channel_creds, + options=_PROPERTY_OPTIONS) + + response = await channel.unary_unary(_UNARY_UNARY)(_REQUEST) + await channel.close() + await server.stop(None) + + auth_data = pickle.loads(response) + auth_ctx = auth_data[_AUTH_CTX] + self.assertCountEqual(_CLIENT_IDS, auth_data[_ID]) + self.assertEqual('x509_subject_alternative_name', auth_data[_ID_KEY]) + self.assertSequenceEqual([b'ssl'], auth_ctx['transport_security_type']) + self.assertSequenceEqual([b'*.test.google.com'], + auth_ctx['x509_common_name']) + + async def _do_one_shot_client_rpc(self, channel_creds, channel_options, + port, expect_ssl_session_reused): + channel = aio.secure_channel('localhost:{}'.format(port), + channel_creds, + options=channel_options) + response = await channel.unary_unary(_UNARY_UNARY)(_REQUEST) + auth_data = pickle.loads(response) + self.assertEqual(expect_ssl_session_reused, + auth_data[_AUTH_CTX]['ssl_session_reused']) + await channel.close() + + async def test_session_resumption(self): + # Set up a secure server + handler = grpc.method_handlers_generic_handler('test', { + 'UnaryUnary': + grpc.unary_unary_rpc_method_handler(handle_unary_unary) + }) + server = aio.server() + server.add_generic_rpc_handlers((handler,)) + server_cred = grpc.ssl_server_credentials(_SERVER_CERTS) + port = server.add_secure_port('[::]:0', server_cred) + await server.start() + + # Create a cache for TLS session tickets + cache = session_cache.ssl_session_cache_lru(1) + channel_creds = grpc.ssl_channel_credentials( + root_certificates=_TEST_ROOT_CERTIFICATES) + channel_options = _PROPERTY_OPTIONS + ( + ('grpc.ssl_session_cache', cache),) + + # Initial connection has no session to resume + await self._do_one_shot_client_rpc(channel_creds, + channel_options, + port, + expect_ssl_session_reused=[b'false']) + + # Subsequent connections resume sessions + await self._do_one_shot_client_rpc(channel_creds, + channel_options, + port, + expect_ssl_session_reused=[b'true']) + await server.stop(None) + + +if __name__ == '__main__': + logging.basicConfig(level=logging.DEBUG) + unittest.main() diff --git a/src/python/grpcio_tests/tests_aio/unit/context_peer_test.py b/src/python/grpcio_tests/tests_aio/unit/context_peer_test.py new file mode 100644 index 00000000000..ea5f4621afb --- /dev/null +++ b/src/python/grpcio_tests/tests_aio/unit/context_peer_test.py @@ -0,0 +1,65 @@ +# Copyright 2020 The gRPC Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Testing the server context ability to access peer info.""" + +import asyncio +import logging +import os +import unittest +from typing import Callable, Iterable, Sequence, Tuple + +import grpc +from grpc.experimental import aio + +from src.proto.grpc.testing import messages_pb2, test_pb2_grpc +from tests.unit.framework.common import test_constants +from tests_aio.unit import _common +from tests_aio.unit._test_base import AioTestBase +from tests_aio.unit._test_server import TestServiceServicer, start_test_server + +_REQUEST = b'\x03\x07' +_TEST_METHOD = '/test/UnaryUnary' + + +class TestContextPeer(AioTestBase): + + async def test_peer(self): + + @grpc.unary_unary_rpc_method_handler + async def check_peer_unary_unary(request: bytes, + context: aio.ServicerContext): + self.assertEqual(_REQUEST, request) + # The peer address could be ipv4 or ipv6 + self.assertIn('ip', context.peer()) + return request + + # Creates a server + server = aio.server() + handlers = grpc.method_handlers_generic_handler( + 'test', {'UnaryUnary': check_peer_unary_unary}) + server.add_generic_rpc_handlers((handlers,)) + port = server.add_insecure_port('[::]:0') + await server.start() + + # Creates a channel + async with aio.insecure_channel('localhost:%d' % port) as channel: + response = await channel.unary_unary(_TEST_METHOD)(_REQUEST) + self.assertEqual(_REQUEST, response) + + await server.stop(None) + + +if __name__ == '__main__': + logging.basicConfig(level=logging.DEBUG) + unittest.main(verbosity=2) From b96381694cdfe854af70dc37c535064b21619a5c Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Mon, 22 Jun 2020 12:57:09 -0700 Subject: [PATCH 07/11] Sort tests.json alphabetically --- src/python/grpcio_tests/tests_aio/tests.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/grpcio_tests/tests_aio/tests.json b/src/python/grpcio_tests/tests_aio/tests.json index 68ff13e5ccc..d10aadeaddd 100644 --- a/src/python/grpcio_tests/tests_aio/tests.json +++ b/src/python/grpcio_tests/tests_aio/tests.json @@ -17,7 +17,6 @@ "unit.channel_argument_test.TestChannelArgument", "unit.channel_ready_test.TestChannelReady", "unit.channel_test.TestChannel", - "unit.context_peer.TestContextPeer", "unit.client_stream_stream_interceptor_test.TestStreamStreamClientInterceptor", "unit.client_stream_unary_interceptor_test.TestStreamUnaryClientInterceptor", "unit.client_unary_stream_interceptor_test.TestUnaryStreamClientInterceptor", @@ -27,6 +26,7 @@ "unit.compatibility_test.TestCompatibility", "unit.compression_test.TestCompression", "unit.connectivity_test.TestConnectivityState", + "unit.context_peer.TestContextPeer", "unit.done_callback_test.TestDoneCallback", "unit.init_test.TestChannel", "unit.metadata_test.TestMetadata", From c9c01d38faab2d5f7e2ac12f2e8876c3abe343c8 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Mon, 22 Jun 2020 13:24:43 -0700 Subject: [PATCH 08/11] add missing virtual dtor --- src/core/ext/filters/client_channel/resolving_lb_policy.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core/ext/filters/client_channel/resolving_lb_policy.h b/src/core/ext/filters/client_channel/resolving_lb_policy.h index 98ea37fbb64..b44dc1a94e3 100644 --- a/src/core/ext/filters/client_channel/resolving_lb_policy.h +++ b/src/core/ext/filters/client_channel/resolving_lb_policy.h @@ -68,6 +68,8 @@ class ResolvingLoadBalancingPolicy : public LoadBalancingPolicy { RefCountedPtr lb_policy_config; }; + virtual ~ChannelConfigHelper() = default; + // Applies the service config to the channel. virtual ApplyServiceConfigResult ApplyServiceConfig( const Resolver::Result& result) = 0; From ae49a8503889b0644039a92aeeaa71c9b4acf363 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Mon, 22 Jun 2020 13:47:30 -0700 Subject: [PATCH 09/11] Fix the typo --- src/python/grpcio_tests/tests_aio/tests.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/grpcio_tests/tests_aio/tests.json b/src/python/grpcio_tests/tests_aio/tests.json index d10aadeaddd..37aa427dacf 100644 --- a/src/python/grpcio_tests/tests_aio/tests.json +++ b/src/python/grpcio_tests/tests_aio/tests.json @@ -26,7 +26,7 @@ "unit.compatibility_test.TestCompatibility", "unit.compression_test.TestCompression", "unit.connectivity_test.TestConnectivityState", - "unit.context_peer.TestContextPeer", + "unit.context_peer_test.TestContextPeer", "unit.done_callback_test.TestDoneCallback", "unit.init_test.TestChannel", "unit.metadata_test.TestMetadata", From c7f2956b67f0999bd6e1cef7eb2e89c628ec0a7a Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Mon, 22 Jun 2020 16:18:36 -0700 Subject: [PATCH 10/11] Revert "Add message-size check before message decompression" --- BUILD | 2 - BUILD.gn | 1 - CMakeLists.txt | 2 - Makefile | 2 - build_autogenerated.yaml | 2 - config.m4 | 1 - config.w32 | 1 - gRPC-Core.podspec | 1 - grpc.gemspec | 1 - grpc.gyp | 2 - package.xml | 1 - .../service_config_channel_arg_filter.cc | 142 -------- .../ext/filters/http/http_filters_plugin.cc | 3 +- .../message_decompress_filter.cc | 92 ++--- .../message_decompress_filter.h | 4 +- .../message_size/message_size_filter.cc | 106 +++--- .../message_size/message_size_filter.h | 6 - .../plugin_registry/grpc_plugin_registry.cc | 4 - .../grpc_unsecure_plugin_registry.cc | 4 - src/python/grpcio/grpc_core_dependencies.py | 1 - test/core/end2end/tests/max_message_length.cc | 332 ------------------ test/cpp/microbenchmarks/bm_call_create.cc | 3 +- tools/doxygen/Doxyfile.c++.internal | 1 - tools/doxygen/Doxyfile.core.internal | 1 - 24 files changed, 93 insertions(+), 622 deletions(-) delete mode 100644 src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc diff --git a/BUILD b/BUILD index 56a55a30509..48c1c1cffb7 100644 --- a/BUILD +++ b/BUILD @@ -1046,7 +1046,6 @@ grpc_cc_library( "src/core/ext/filters/client_channel/retry_throttle.cc", "src/core/ext/filters/client_channel/server_address.cc", "src/core/ext/filters/client_channel/service_config.cc", - "src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc", "src/core/ext/filters/client_channel/service_config_parser.cc", "src/core/ext/filters/client_channel/subchannel.cc", "src/core/ext/filters/client_channel/subchannel_pool_interface.cc", @@ -1188,7 +1187,6 @@ grpc_cc_library( language = "c++", deps = [ "grpc_base", - "grpc_message_size_filter", ], ) diff --git a/BUILD.gn b/BUILD.gn index 3834e5a0ab7..2d9c2593a5a 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -295,7 +295,6 @@ config("grpc_config") { "src/core/ext/filters/client_channel/service_config.cc", "src/core/ext/filters/client_channel/service_config.h", "src/core/ext/filters/client_channel/service_config_call_data.h", - "src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc", "src/core/ext/filters/client_channel/service_config_parser.cc", "src/core/ext/filters/client_channel/service_config_parser.h", "src/core/ext/filters/client_channel/subchannel.cc", diff --git a/CMakeLists.txt b/CMakeLists.txt index 2f8bac94432..7515cafaf5b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1376,7 +1376,6 @@ add_library(grpc src/core/ext/filters/client_channel/retry_throttle.cc src/core/ext/filters/client_channel/server_address.cc src/core/ext/filters/client_channel/service_config.cc - src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc src/core/ext/filters/client_channel/service_config_parser.cc src/core/ext/filters/client_channel/subchannel.cc src/core/ext/filters/client_channel/subchannel_pool_interface.cc @@ -2049,7 +2048,6 @@ add_library(grpc_unsecure src/core/ext/filters/client_channel/retry_throttle.cc src/core/ext/filters/client_channel/server_address.cc src/core/ext/filters/client_channel/service_config.cc - src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc src/core/ext/filters/client_channel/service_config_parser.cc src/core/ext/filters/client_channel/subchannel.cc src/core/ext/filters/client_channel/subchannel_pool_interface.cc diff --git a/Makefile b/Makefile index de3331162a6..39a4d43a316 100644 --- a/Makefile +++ b/Makefile @@ -3679,7 +3679,6 @@ LIBGRPC_SRC = \ src/core/ext/filters/client_channel/retry_throttle.cc \ src/core/ext/filters/client_channel/server_address.cc \ src/core/ext/filters/client_channel/service_config.cc \ - src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc \ src/core/ext/filters/client_channel/service_config_parser.cc \ src/core/ext/filters/client_channel/subchannel.cc \ src/core/ext/filters/client_channel/subchannel_pool_interface.cc \ @@ -4326,7 +4325,6 @@ LIBGRPC_UNSECURE_SRC = \ src/core/ext/filters/client_channel/retry_throttle.cc \ src/core/ext/filters/client_channel/server_address.cc \ src/core/ext/filters/client_channel/service_config.cc \ - src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc \ src/core/ext/filters/client_channel/service_config_parser.cc \ src/core/ext/filters/client_channel/subchannel.cc \ src/core/ext/filters/client_channel/subchannel_pool_interface.cc \ diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index fac33c197a2..3ac9f3c74b9 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -795,7 +795,6 @@ libs: - src/core/ext/filters/client_channel/retry_throttle.cc - src/core/ext/filters/client_channel/server_address.cc - src/core/ext/filters/client_channel/service_config.cc - - src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc - src/core/ext/filters/client_channel/service_config_parser.cc - src/core/ext/filters/client_channel/subchannel.cc - src/core/ext/filters/client_channel/subchannel_pool_interface.cc @@ -1657,7 +1656,6 @@ libs: - src/core/ext/filters/client_channel/retry_throttle.cc - src/core/ext/filters/client_channel/server_address.cc - src/core/ext/filters/client_channel/service_config.cc - - src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc - src/core/ext/filters/client_channel/service_config_parser.cc - src/core/ext/filters/client_channel/subchannel.cc - src/core/ext/filters/client_channel/subchannel_pool_interface.cc diff --git a/config.m4 b/config.m4 index a6bf9a21b6c..5ee9f1c8d58 100644 --- a/config.m4 +++ b/config.m4 @@ -93,7 +93,6 @@ if test "$PHP_GRPC" != "no"; then src/core/ext/filters/client_channel/retry_throttle.cc \ src/core/ext/filters/client_channel/server_address.cc \ src/core/ext/filters/client_channel/service_config.cc \ - src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc \ src/core/ext/filters/client_channel/service_config_parser.cc \ src/core/ext/filters/client_channel/subchannel.cc \ src/core/ext/filters/client_channel/subchannel_pool_interface.cc \ diff --git a/config.w32 b/config.w32 index 03cf0684684..1c533a33302 100644 --- a/config.w32 +++ b/config.w32 @@ -62,7 +62,6 @@ if (PHP_GRPC != "no") { "src\\core\\ext\\filters\\client_channel\\retry_throttle.cc " + "src\\core\\ext\\filters\\client_channel\\server_address.cc " + "src\\core\\ext\\filters\\client_channel\\service_config.cc " + - "src\\core\\ext\\filters\\client_channel\\service_config_channel_arg_filter.cc " + "src\\core\\ext\\filters\\client_channel\\service_config_parser.cc " + "src\\core\\ext\\filters\\client_channel\\subchannel.cc " + "src\\core\\ext\\filters\\client_channel\\subchannel_pool_interface.cc " + diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 43a47a09fa7..aaf5609a9c3 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -279,7 +279,6 @@ Pod::Spec.new do |s| 'src/core/ext/filters/client_channel/service_config.cc', 'src/core/ext/filters/client_channel/service_config.h', 'src/core/ext/filters/client_channel/service_config_call_data.h', - 'src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc', 'src/core/ext/filters/client_channel/service_config_parser.cc', 'src/core/ext/filters/client_channel/service_config_parser.h', 'src/core/ext/filters/client_channel/subchannel.cc', diff --git a/grpc.gemspec b/grpc.gemspec index 72ec55f0c86..881f55a81c2 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -201,7 +201,6 @@ Gem::Specification.new do |s| s.files += %w( src/core/ext/filters/client_channel/service_config.cc ) s.files += %w( src/core/ext/filters/client_channel/service_config.h ) s.files += %w( src/core/ext/filters/client_channel/service_config_call_data.h ) - s.files += %w( src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc ) s.files += %w( src/core/ext/filters/client_channel/service_config_parser.cc ) s.files += %w( src/core/ext/filters/client_channel/service_config_parser.h ) s.files += %w( src/core/ext/filters/client_channel/subchannel.cc ) diff --git a/grpc.gyp b/grpc.gyp index 587e6ba8d94..e8bed76e891 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -488,7 +488,6 @@ 'src/core/ext/filters/client_channel/retry_throttle.cc', 'src/core/ext/filters/client_channel/server_address.cc', 'src/core/ext/filters/client_channel/service_config.cc', - 'src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc', 'src/core/ext/filters/client_channel/service_config_parser.cc', 'src/core/ext/filters/client_channel/subchannel.cc', 'src/core/ext/filters/client_channel/subchannel_pool_interface.cc', @@ -997,7 +996,6 @@ 'src/core/ext/filters/client_channel/retry_throttle.cc', 'src/core/ext/filters/client_channel/server_address.cc', 'src/core/ext/filters/client_channel/service_config.cc', - 'src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc', 'src/core/ext/filters/client_channel/service_config_parser.cc', 'src/core/ext/filters/client_channel/subchannel.cc', 'src/core/ext/filters/client_channel/subchannel_pool_interface.cc', diff --git a/package.xml b/package.xml index 7553fe27739..262c66a6ac3 100644 --- a/package.xml +++ b/package.xml @@ -181,7 +181,6 @@ - diff --git a/src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc b/src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc deleted file mode 100644 index 6b8e6373af7..00000000000 --- a/src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc +++ /dev/null @@ -1,142 +0,0 @@ -// -// Copyright 2020 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -// This filter reads GRPC_ARG_SERVICE_CONFIG and populates ServiceConfigCallData -// in the call context per call for direct channels. - -#include - -#include "src/core/ext/filters/client_channel/service_config_call_data.h" -#include "src/core/lib/channel/channel_args.h" -#include "src/core/lib/channel/channel_stack.h" -#include "src/core/lib/channel/channel_stack_builder.h" -#include "src/core/lib/surface/channel_init.h" - -namespace grpc_core { - -namespace { - -class ServiceConfigChannelArgChannelData { - public: - explicit ServiceConfigChannelArgChannelData( - const grpc_channel_element_args* args) { - const char* service_config_str = grpc_channel_args_find_string( - args->channel_args, GRPC_ARG_SERVICE_CONFIG); - if (service_config_str != nullptr) { - grpc_error* service_config_error = GRPC_ERROR_NONE; - auto service_config = - ServiceConfig::Create(service_config_str, &service_config_error); - if (service_config_error == GRPC_ERROR_NONE) { - service_config_ = std::move(service_config); - } else { - gpr_log(GPR_ERROR, "%s", grpc_error_string(service_config_error)); - } - GRPC_ERROR_UNREF(service_config_error); - } - } - - RefCountedPtr service_config() const { - return service_config_; - } - - private: - RefCountedPtr service_config_; -}; - -class ServiceConfigChannelArgCallData { - public: - ServiceConfigChannelArgCallData(grpc_call_element* elem, - const grpc_call_element_args* args) { - ServiceConfigChannelArgChannelData* chand = - static_cast(elem->channel_data); - RefCountedPtr service_config = chand->service_config(); - if (service_config != nullptr) { - GPR_DEBUG_ASSERT(args->context != nullptr); - const auto* method_params_vector = - service_config->GetMethodParsedConfigVector(args->path); - args->arena->New( - std::move(service_config), method_params_vector, args->context); - } - } -}; - -grpc_error* ServiceConfigChannelArgInitCallElem( - grpc_call_element* elem, const grpc_call_element_args* args) { - ServiceConfigChannelArgCallData* calld = - static_cast(elem->call_data); - new (calld) ServiceConfigChannelArgCallData(elem, args); - return GRPC_ERROR_NONE; -} - -void ServiceConfigChannelArgDestroyCallElem( - grpc_call_element* elem, const grpc_call_final_info* /* final_info */, - grpc_closure* /* then_schedule_closure */) { - ServiceConfigChannelArgCallData* calld = - static_cast(elem->call_data); - calld->~ServiceConfigChannelArgCallData(); -} - -grpc_error* ServiceConfigChannelArgInitChannelElem( - grpc_channel_element* elem, grpc_channel_element_args* args) { - ServiceConfigChannelArgChannelData* chand = - static_cast(elem->channel_data); - new (chand) ServiceConfigChannelArgChannelData(args); - return GRPC_ERROR_NONE; -} - -void ServiceConfigChannelArgDestroyChannelElem(grpc_channel_element* elem) { - ServiceConfigChannelArgChannelData* chand = - static_cast(elem->channel_data); - chand->~ServiceConfigChannelArgChannelData(); -} - -const grpc_channel_filter ServiceConfigChannelArgFilter = { - grpc_call_next_op, - grpc_channel_next_op, - sizeof(ServiceConfigChannelArgCallData), - ServiceConfigChannelArgInitCallElem, - grpc_call_stack_ignore_set_pollset_or_pollset_set, - ServiceConfigChannelArgDestroyCallElem, - sizeof(ServiceConfigChannelArgChannelData), - ServiceConfigChannelArgInitChannelElem, - ServiceConfigChannelArgDestroyChannelElem, - grpc_channel_next_get_info, - "service_config_channel_arg"}; - -bool maybe_add_service_config_channel_arg_filter( - grpc_channel_stack_builder* builder, void* /* arg */) { - const grpc_channel_args* channel_args = - grpc_channel_stack_builder_get_channel_arguments(builder); - if (grpc_channel_args_want_minimal_stack(channel_args) || - grpc_channel_args_find_string(channel_args, GRPC_ARG_SERVICE_CONFIG) == - nullptr) { - return true; - } - return grpc_channel_stack_builder_prepend_filter( - builder, &ServiceConfigChannelArgFilter, nullptr, nullptr); -} - -} // namespace - -} // namespace grpc_core - -void grpc_service_config_channel_arg_filter_init(void) { - grpc_channel_init_register_stage( - GRPC_CLIENT_DIRECT_CHANNEL, GRPC_CHANNEL_INIT_BUILTIN_PRIORITY, - grpc_core::maybe_add_service_config_channel_arg_filter, nullptr); -} - -void grpc_service_config_channel_arg_filter_shutdown(void) {} diff --git a/src/core/ext/filters/http/http_filters_plugin.cc b/src/core/ext/filters/http/http_filters_plugin.cc index 637dc3030f2..2fedc7fe3d7 100644 --- a/src/core/ext/filters/http/http_filters_plugin.cc +++ b/src/core/ext/filters/http/http_filters_plugin.cc @@ -38,8 +38,7 @@ static optional_filter compress_filter = { &grpc_message_compress_filter, GRPC_ARG_ENABLE_PER_MESSAGE_COMPRESSION}; static optional_filter decompress_filter = { - &grpc_core::MessageDecompressFilter, - GRPC_ARG_ENABLE_PER_MESSAGE_DECOMPRESSION}; + &grpc_message_decompress_filter, GRPC_ARG_ENABLE_PER_MESSAGE_DECOMPRESSION}; static bool is_building_http_like_transport( grpc_channel_stack_builder* builder) { diff --git a/src/core/ext/filters/http/message_compress/message_decompress_filter.cc b/src/core/ext/filters/http/message_compress/message_decompress_filter.cc index c16ea66e7a3..d12f4013bb2 100644 --- a/src/core/ext/filters/http/message_compress/message_decompress_filter.cc +++ b/src/core/ext/filters/http/message_compress/message_decompress_filter.cc @@ -18,8 +18,6 @@ #include -#include "src/core/ext/filters/http/message_compress/message_decompress_filter.h" - #include #include @@ -29,8 +27,7 @@ #include #include -#include "absl/strings/str_format.h" -#include "src/core/ext/filters/message_size/message_size_filter.h" +#include "src/core/ext/filters/http/message_compress/message_decompress_filter.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/compression/algorithm_metadata.h" #include "src/core/lib/compression/compression_args.h" @@ -40,25 +37,14 @@ #include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/slice/slice_string_helpers.h" -namespace grpc_core { namespace { -class ChannelData { - public: - explicit ChannelData(const grpc_channel_element_args* args) - : max_recv_size_(GetMaxRecvSizeFromChannelArgs(args->channel_args)) {} - - int max_recv_size() const { return max_recv_size_; } - - private: - int max_recv_size_; -}; +class ChannelData {}; class CallData { public: - CallData(const grpc_call_element_args& args, const ChannelData* chand) - : call_combiner_(args.call_combiner), - max_recv_message_length_(chand->max_recv_size()) { + explicit CallData(const grpc_call_element_args& args) + : call_combiner_(args.call_combiner) { // Initialize state for recv_initial_metadata_ready callback GRPC_CLOSURE_INIT(&on_recv_initial_metadata_ready_, OnRecvInitialMetadataReady, this, @@ -73,13 +59,6 @@ class CallData { GRPC_CLOSURE_INIT(&on_recv_trailing_metadata_ready_, OnRecvTrailingMetadataReady, this, grpc_schedule_on_exec_ctx); - const MessageSizeParsedConfig* limits = - MessageSizeParsedConfig::GetFromCallContext(args.context); - if (limits != nullptr && limits->limits().max_recv_size >= 0 && - (limits->limits().max_recv_size < max_recv_message_length_ || - max_recv_message_length_ < 0)) { - max_recv_message_length_ = limits->limits().max_recv_size; - } } ~CallData() { grpc_slice_buffer_destroy_internal(&recv_slices_); } @@ -103,7 +82,7 @@ class CallData { void MaybeResumeOnRecvTrailingMetadataReady(); static void OnRecvTrailingMetadataReady(void* arg, grpc_error* error); - CallCombiner* call_combiner_; + grpc_core::CallCombiner* call_combiner_; // Overall error for the call grpc_error* error_ = GRPC_ERROR_NONE; // Fields for handling recv_initial_metadata_ready callback @@ -112,18 +91,17 @@ class CallData { grpc_metadata_batch* recv_initial_metadata_ = nullptr; // Fields for handling recv_message_ready callback bool seen_recv_message_ready_ = false; - int max_recv_message_length_; grpc_message_compression_algorithm algorithm_ = GRPC_MESSAGE_COMPRESS_NONE; grpc_closure on_recv_message_ready_; grpc_closure* original_recv_message_ready_ = nullptr; grpc_closure on_recv_message_next_done_; - OrphanablePtr* recv_message_ = nullptr; + grpc_core::OrphanablePtr* recv_message_ = nullptr; // recv_slices_ holds the slices read from the original recv_message stream. // It is initialized during construction and reset when a new stream is // created using it. grpc_slice_buffer recv_slices_; - std::aligned_storage::type + std::aligned_storage::type recv_replacement_stream_; // Fields for handling recv_trailing_metadata_ready callback bool seen_recv_trailing_metadata_ready_ = false; @@ -161,7 +139,7 @@ void CallData::OnRecvInitialMetadataReady(void* arg, grpc_error* error) { calld->MaybeResumeOnRecvTrailingMetadataReady(); grpc_closure* closure = calld->original_recv_initial_metadata_ready_; calld->original_recv_initial_metadata_ready_ = nullptr; - Closure::Run(DEBUG_LOCATION, closure, GRPC_ERROR_REF(error)); + grpc_core::Closure::Run(DEBUG_LOCATION, closure, GRPC_ERROR_REF(error)); } void CallData::MaybeResumeOnRecvMessageReady() { @@ -192,19 +170,6 @@ void CallData::OnRecvMessageReady(void* arg, grpc_error* error) { 0) { return calld->ContinueRecvMessageReadyCallback(GRPC_ERROR_NONE); } - if (calld->max_recv_message_length_ >= 0 && - (*calld->recv_message_)->length() > - static_cast(calld->max_recv_message_length_)) { - std::string message_string = absl::StrFormat( - "Received message larger than max (%u vs. %d)", - (*calld->recv_message_)->length(), calld->max_recv_message_length_); - GPR_DEBUG_ASSERT(calld->error_ == GRPC_ERROR_NONE); - calld->error_ = grpc_error_set_int( - GRPC_ERROR_CREATE_FROM_COPIED_STRING(message_string.c_str()), - GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_RESOURCE_EXHAUSTED); - return calld->ContinueRecvMessageReadyCallback( - GRPC_ERROR_REF(calld->error_)); - } grpc_slice_buffer_destroy_internal(&calld->recv_slices_); grpc_slice_buffer_init(&calld->recv_slices_); return calld->ContinueReadingRecvMessage(); @@ -276,9 +241,9 @@ void CallData::FinishRecvMessage() { // Initializing recv_replacement_stream_ with decompressed_slices removes // all the slices from decompressed_slices leaving it empty. new (&recv_replacement_stream_) - SliceBufferByteStream(&decompressed_slices, recv_flags); - recv_message_->reset( - reinterpret_cast(&recv_replacement_stream_)); + grpc_core::SliceBufferByteStream(&decompressed_slices, recv_flags); + recv_message_->reset(reinterpret_cast( + &recv_replacement_stream_)); recv_message_ = nullptr; } ContinueRecvMessageReadyCallback(GRPC_ERROR_REF(error_)); @@ -289,7 +254,7 @@ void CallData::ContinueRecvMessageReadyCallback(grpc_error* error) { // The surface will clean up the receiving stream if there is an error. grpc_closure* closure = original_recv_message_ready_; original_recv_message_ready_ = nullptr; - Closure::Run(DEBUG_LOCATION, closure, error); + grpc_core::Closure::Run(DEBUG_LOCATION, closure, error); } void CallData::MaybeResumeOnRecvTrailingMetadataReady() { @@ -318,7 +283,7 @@ void CallData::OnRecvTrailingMetadataReady(void* arg, grpc_error* error) { calld->error_ = GRPC_ERROR_NONE; grpc_closure* closure = calld->original_recv_trailing_metadata_ready_; calld->original_recv_trailing_metadata_ready_ = nullptr; - Closure::Run(DEBUG_LOCATION, closure, error); + grpc_core::Closure::Run(DEBUG_LOCATION, closure, error); } void CallData::DecompressStartTransportStreamOpBatch( @@ -357,44 +322,37 @@ void DecompressStartTransportStreamOpBatch( calld->DecompressStartTransportStreamOpBatch(elem, batch); } -grpc_error* DecompressInitCallElem(grpc_call_element* elem, - const grpc_call_element_args* args) { - ChannelData* chand = static_cast(elem->channel_data); - new (elem->call_data) CallData(*args, chand); +static grpc_error* DecompressInitCallElem(grpc_call_element* elem, + const grpc_call_element_args* args) { + new (elem->call_data) CallData(*args); return GRPC_ERROR_NONE; } -void DecompressDestroyCallElem(grpc_call_element* elem, - const grpc_call_final_info* /*final_info*/, - grpc_closure* /*ignored*/) { +static void DecompressDestroyCallElem( + grpc_call_element* elem, const grpc_call_final_info* /*final_info*/, + grpc_closure* /*ignored*/) { CallData* calld = static_cast(elem->call_data); calld->~CallData(); } -grpc_error* DecompressInitChannelElem(grpc_channel_element* elem, - grpc_channel_element_args* args) { - ChannelData* chand = static_cast(elem->channel_data); - new (chand) ChannelData(args); +static grpc_error* DecompressInitChannelElem( + grpc_channel_element* /*elem*/, grpc_channel_element_args* /*args*/) { return GRPC_ERROR_NONE; } -void DecompressDestroyChannelElem(grpc_channel_element* elem) { - ChannelData* chand = static_cast(elem->channel_data); - chand->~ChannelData(); -} +void DecompressDestroyChannelElem(grpc_channel_element* /*elem*/) {} } // namespace -const grpc_channel_filter MessageDecompressFilter = { +const grpc_channel_filter grpc_message_decompress_filter = { DecompressStartTransportStreamOpBatch, grpc_channel_next_op, sizeof(CallData), DecompressInitCallElem, grpc_call_stack_ignore_set_pollset_or_pollset_set, DecompressDestroyCallElem, - sizeof(ChannelData), + 0, // sizeof(ChannelData) DecompressInitChannelElem, DecompressDestroyChannelElem, grpc_channel_next_get_info, "message_decompress"}; -} // namespace grpc_core diff --git a/src/core/ext/filters/http/message_compress/message_decompress_filter.h b/src/core/ext/filters/http/message_compress/message_decompress_filter.h index f19a4ca0cbd..7d567bf08a2 100644 --- a/src/core/ext/filters/http/message_compress/message_decompress_filter.h +++ b/src/core/ext/filters/http/message_compress/message_decompress_filter.h @@ -23,9 +23,7 @@ #include "src/core/lib/channel/channel_stack.h" -namespace grpc_core { -extern const grpc_channel_filter MessageDecompressFilter; -} // namespace grpc_core +extern const grpc_channel_filter grpc_message_decompress_filter; #endif /* GRPC_CORE_EXT_FILTERS_HTTP_MESSAGE_COMPRESS_MESSAGE_DECOMPRESS_FILTER_H \ */ diff --git a/src/core/ext/filters/message_size/message_size_filter.cc b/src/core/ext/filters/message_size/message_size_filter.cc index 89fdab6fae8..d2ef5477636 100644 --- a/src/core/ext/filters/message_size/message_size_filter.cc +++ b/src/core/ext/filters/message_size/message_size_filter.cc @@ -45,25 +45,6 @@ namespace { size_t g_message_size_parser_index; } // namespace -// -// MessageSizeParsedConfig -// - -const MessageSizeParsedConfig* MessageSizeParsedConfig::GetFromCallContext( - const grpc_call_context_element* context) { - if (context == nullptr) return nullptr; - auto* svc_cfg_call_data = static_cast( - context[GRPC_CONTEXT_SERVICE_CONFIG_CALL_DATA].value); - if (svc_cfg_call_data == nullptr) return nullptr; - return static_cast( - svc_cfg_call_data->GetMethodParsedConfig( - MessageSizeParser::ParserIndex())); -} - -// -// MessageSizeParser -// - std::unique_ptr MessageSizeParser::ParsePerMethodParams(const Json& json, grpc_error** error) { GPR_DEBUG_ASSERT(error != nullptr && *error == GRPC_ERROR_NONE); @@ -116,26 +97,12 @@ void MessageSizeParser::Register() { } size_t MessageSizeParser::ParserIndex() { return g_message_size_parser_index; } - -int GetMaxRecvSizeFromChannelArgs(const grpc_channel_args* args) { - if (grpc_channel_args_want_minimal_stack(args)) return -1; - return grpc_channel_args_find_integer( - args, GRPC_ARG_MAX_RECEIVE_MESSAGE_LENGTH, - {GRPC_DEFAULT_MAX_RECV_MESSAGE_LENGTH, -1, INT_MAX}); -} - -int GetMaxSendSizeFromChannelArgs(const grpc_channel_args* args) { - if (grpc_channel_args_want_minimal_stack(args)) return -1; - return grpc_channel_args_find_integer( - args, GRPC_ARG_MAX_SEND_MESSAGE_LENGTH, - {GRPC_DEFAULT_MAX_SEND_MESSAGE_LENGTH, -1, INT_MAX}); -} - } // namespace grpc_core namespace { struct channel_data { grpc_core::MessageSizeParsedConfig::message_size_limits limits; + grpc_core::RefCountedPtr svc_cfg; }; struct call_data { @@ -151,8 +118,24 @@ struct call_data { // Note: Per-method config is only available on the client, so we // apply the max request size to the send limit and the max response // size to the receive limit. - const grpc_core::MessageSizeParsedConfig* limits = - grpc_core::MessageSizeParsedConfig::GetFromCallContext(args.context); + const grpc_core::MessageSizeParsedConfig* limits = nullptr; + grpc_core::ServiceConfigCallData* svc_cfg_call_data = nullptr; + if (args.context != nullptr) { + svc_cfg_call_data = static_cast( + args.context[GRPC_CONTEXT_SERVICE_CONFIG_CALL_DATA].value); + } + if (svc_cfg_call_data != nullptr) { + limits = static_cast( + svc_cfg_call_data->GetMethodParsedConfig( + grpc_core::MessageSizeParser::ParserIndex())); + } else if (chand.svc_cfg != nullptr) { + const auto* objs_vector = + chand.svc_cfg->GetMethodParsedConfigVector(args.path); + if (objs_vector != nullptr) { + limits = static_cast( + (*objs_vector)[grpc_core::MessageSizeParser::ParserIndex()].get()); + } + } if (limits != nullptr) { if (limits->limits().max_send_size >= 0 && (limits->limits().max_send_size < this->limits.max_send_size || @@ -305,11 +288,35 @@ static void message_size_destroy_call_elem( calld->~call_data(); } +static int default_size(const grpc_channel_args* args, + int without_minimal_stack) { + if (grpc_channel_args_want_minimal_stack(args)) { + return -1; + } + return without_minimal_stack; +} + grpc_core::MessageSizeParsedConfig::message_size_limits get_message_size_limits( const grpc_channel_args* channel_args) { grpc_core::MessageSizeParsedConfig::message_size_limits lim; - lim.max_send_size = grpc_core::GetMaxSendSizeFromChannelArgs(channel_args); - lim.max_recv_size = grpc_core::GetMaxRecvSizeFromChannelArgs(channel_args); + lim.max_send_size = + default_size(channel_args, GRPC_DEFAULT_MAX_SEND_MESSAGE_LENGTH); + lim.max_recv_size = + default_size(channel_args, GRPC_DEFAULT_MAX_RECV_MESSAGE_LENGTH); + for (size_t i = 0; i < channel_args->num_args; ++i) { + if (strcmp(channel_args->args[i].key, GRPC_ARG_MAX_SEND_MESSAGE_LENGTH) == + 0) { + const grpc_integer_options options = {lim.max_send_size, -1, INT_MAX}; + lim.max_send_size = + grpc_channel_arg_get_integer(&channel_args->args[i], options); + } + if (strcmp(channel_args->args[i].key, + GRPC_ARG_MAX_RECEIVE_MESSAGE_LENGTH) == 0) { + const grpc_integer_options options = {lim.max_recv_size, -1, INT_MAX}; + lim.max_recv_size = + grpc_channel_arg_get_integer(&channel_args->args[i], options); + } + } return lim; } @@ -320,6 +327,26 @@ static grpc_error* message_size_init_channel_elem( channel_data* chand = static_cast(elem->channel_data); new (chand) channel_data(); chand->limits = get_message_size_limits(args->channel_args); + // TODO(yashykt): We only need to read GRPC_ARG_SERVICE_CONFIG in the case of + // direct channels. (Service config is otherwise stored in the call_context by + // client_channel filter.) If we ever need a second filter that also needs to + // parse GRPC_ARG_SERVICE_CONFIG, we should refactor this code and add a + // separate filter that reads GRPC_ARG_SERVICE_CONFIG and saves the parsed + // config in the call_context. + const grpc_arg* channel_arg = + grpc_channel_args_find(args->channel_args, GRPC_ARG_SERVICE_CONFIG); + const char* service_config_str = grpc_channel_arg_get_string(channel_arg); + if (service_config_str != nullptr) { + grpc_error* service_config_error = GRPC_ERROR_NONE; + auto svc_cfg = grpc_core::ServiceConfig::Create(service_config_str, + &service_config_error); + if (service_config_error == GRPC_ERROR_NONE) { + chand->svc_cfg = std::move(svc_cfg); + } else { + gpr_log(GPR_ERROR, "%s", grpc_error_string(service_config_error)); + } + GRPC_ERROR_UNREF(service_config_error); + } return GRPC_ERROR_NONE; } @@ -360,9 +387,6 @@ static bool maybe_add_message_size_filter(grpc_channel_stack_builder* builder, void* /*arg*/) { const grpc_channel_args* channel_args = grpc_channel_stack_builder_get_channel_arguments(builder); - if (grpc_channel_args_want_minimal_stack(channel_args)) { - return true; - } bool enable = false; grpc_core::MessageSizeParsedConfig::message_size_limits lim = get_message_size_limits(channel_args); diff --git a/src/core/ext/filters/message_size/message_size_filter.h b/src/core/ext/filters/message_size/message_size_filter.h index ea0dd0266d0..132d7b2af0f 100644 --- a/src/core/ext/filters/message_size/message_size_filter.h +++ b/src/core/ext/filters/message_size/message_size_filter.h @@ -40,9 +40,6 @@ class MessageSizeParsedConfig : public ServiceConfigParser::ParsedConfig { const message_size_limits& limits() const { return limits_; } - static const MessageSizeParsedConfig* GetFromCallContext( - const grpc_call_context_element* context); - private: message_size_limits limits_; }; @@ -57,9 +54,6 @@ class MessageSizeParser : public ServiceConfigParser::Parser { static size_t ParserIndex(); }; -int GetMaxRecvSizeFromChannelArgs(const grpc_channel_args* args); -int GetMaxSendSizeFromChannelArgs(const grpc_channel_args* args); - } // namespace grpc_core #endif /* GRPC_CORE_EXT_FILTERS_MESSAGE_SIZE_MESSAGE_SIZE_FILTER_H */ diff --git a/src/core/plugin_registry/grpc_plugin_registry.cc b/src/core/plugin_registry/grpc_plugin_registry.cc index ef8b10df647..f1c3cbf4036 100644 --- a/src/core/plugin_registry/grpc_plugin_registry.cc +++ b/src/core/plugin_registry/grpc_plugin_registry.cc @@ -64,8 +64,6 @@ void grpc_max_age_filter_init(void); void grpc_max_age_filter_shutdown(void); void grpc_message_size_filter_init(void); void grpc_message_size_filter_shutdown(void); -void grpc_service_config_channel_arg_filter_init(void); -void grpc_service_config_channel_arg_filter_shutdown(void); void grpc_client_authority_filter_init(void); void grpc_client_authority_filter_shutdown(void); void grpc_workaround_cronet_compression_filter_init(void); @@ -116,8 +114,6 @@ void grpc_register_built_in_plugins(void) { grpc_max_age_filter_shutdown); grpc_register_plugin(grpc_message_size_filter_init, grpc_message_size_filter_shutdown); - grpc_register_plugin(grpc_service_config_channel_arg_filter_init, - grpc_service_config_channel_arg_filter_shutdown); grpc_register_plugin(grpc_client_authority_filter_init, grpc_client_authority_filter_shutdown); grpc_register_plugin(grpc_workaround_cronet_compression_filter_init, diff --git a/src/core/plugin_registry/grpc_unsecure_plugin_registry.cc b/src/core/plugin_registry/grpc_unsecure_plugin_registry.cc index 525fa108d81..de53f173294 100644 --- a/src/core/plugin_registry/grpc_unsecure_plugin_registry.cc +++ b/src/core/plugin_registry/grpc_unsecure_plugin_registry.cc @@ -64,8 +64,6 @@ void grpc_max_age_filter_init(void); void grpc_max_age_filter_shutdown(void); void grpc_message_size_filter_init(void); void grpc_message_size_filter_shutdown(void); -void grpc_service_config_channel_arg_filter_init(void); -void grpc_service_config_channel_arg_filter_shutdown(void); void grpc_client_authority_filter_init(void); void grpc_client_authority_filter_shutdown(void); void grpc_workaround_cronet_compression_filter_init(void); @@ -116,8 +114,6 @@ void grpc_register_built_in_plugins(void) { grpc_max_age_filter_shutdown); grpc_register_plugin(grpc_message_size_filter_init, grpc_message_size_filter_shutdown); - grpc_register_plugin(grpc_service_config_channel_arg_filter_init, - grpc_service_config_channel_arg_filter_shutdown); grpc_register_plugin(grpc_client_authority_filter_init, grpc_client_authority_filter_shutdown); grpc_register_plugin(grpc_workaround_cronet_compression_filter_init, diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index 7689ddedeba..a25f10239f6 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -71,7 +71,6 @@ CORE_SOURCE_FILES = [ 'src/core/ext/filters/client_channel/retry_throttle.cc', 'src/core/ext/filters/client_channel/server_address.cc', 'src/core/ext/filters/client_channel/service_config.cc', - 'src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc', 'src/core/ext/filters/client_channel/service_config_parser.cc', 'src/core/ext/filters/client_channel/subchannel.cc', 'src/core/ext/filters/client_channel/subchannel_pool_interface.cc', diff --git a/test/core/end2end/tests/max_message_length.cc b/test/core/end2end/tests/max_message_length.cc index 256cc982940..40e752a3d63 100644 --- a/test/core/end2end/tests/max_message_length.cc +++ b/test/core/end2end/tests/max_message_length.cc @@ -29,7 +29,6 @@ #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/iomgr/exec_ctx.h" #include "src/core/lib/slice/slice_internal.h" -#include "src/core/lib/slice/slice_string_helpers.h" #include "src/core/lib/transport/metadata.h" #include "test/core/end2end/cq_verifier.h" @@ -467,328 +466,6 @@ static void test_max_message_length_on_response(grpc_end2end_test_config config, grpc_byte_buffer_destroy(response_payload); grpc_byte_buffer_destroy(recv_payload); - grpc_call_unref(c); - if (s != nullptr) grpc_call_unref(s); - cq_verifier_destroy(cqv); - end_test(&f); - config.tear_down_data(&f); -} - -static grpc_metadata gzip_compression_override() { - grpc_metadata gzip_compression_override; - gzip_compression_override.key = GRPC_MDSTR_GRPC_INTERNAL_ENCODING_REQUEST; - gzip_compression_override.value = grpc_slice_from_static_string("gzip"); - memset(&gzip_compression_override.internal_data, 0, - sizeof(gzip_compression_override.internal_data)); - return gzip_compression_override; -} - -// Test receive message limit with compressed request larger than the limit -static void test_max_receive_message_length_on_compressed_request( - grpc_end2end_test_config config, bool minimal_stack) { - gpr_log(GPR_INFO, - "test max receive message length on compressed request with " - "minimal_stack=%d", - minimal_stack); - grpc_end2end_test_fixture f; - grpc_call* c = nullptr; - grpc_call* s = nullptr; - cq_verifier* cqv; - grpc_op ops[6]; - grpc_op* op; - grpc_slice request_payload_slice = grpc_slice_malloc(1024); - memset(GRPC_SLICE_START_PTR(request_payload_slice), 'a', 1024); - grpc_byte_buffer* request_payload = - grpc_raw_byte_buffer_create(&request_payload_slice, 1); - grpc_byte_buffer* recv_payload = nullptr; - grpc_metadata_array initial_metadata_recv; - grpc_metadata_array trailing_metadata_recv; - grpc_metadata_array request_metadata_recv; - grpc_call_details call_details; - grpc_status_code status; - grpc_call_error error; - grpc_slice details, status_details; - int was_cancelled = 2; - - // Set limit via channel args. - grpc_arg arg[2]; - arg[0] = grpc_channel_arg_integer_create( - const_cast(GRPC_ARG_MAX_RECEIVE_MESSAGE_LENGTH), 5); - arg[1] = grpc_channel_arg_integer_create( - const_cast(GRPC_ARG_MINIMAL_STACK), minimal_stack); - grpc_channel_args* server_args = - grpc_channel_args_copy_and_add(nullptr, arg, 2); - - f = begin_test(config, "test_max_request_message_length", nullptr, - server_args); - { - grpc_core::ExecCtx exec_ctx; - grpc_channel_args_destroy(server_args); - } - cqv = cq_verifier_create(f.cq); - c = grpc_channel_create_call(f.client, nullptr, GRPC_PROPAGATE_DEFAULTS, f.cq, - grpc_slice_from_static_string("/service/method"), - nullptr, gpr_inf_future(GPR_CLOCK_REALTIME), - nullptr); - GPR_ASSERT(c); - - grpc_metadata_array_init(&initial_metadata_recv); - grpc_metadata_array_init(&trailing_metadata_recv); - grpc_metadata_array_init(&request_metadata_recv); - grpc_call_details_init(&call_details); - - grpc_metadata compression_md = gzip_compression_override(); - memset(ops, 0, sizeof(ops)); - op = ops; - op->op = GRPC_OP_SEND_INITIAL_METADATA; - op->data.send_initial_metadata.count = 1; - op->data.send_initial_metadata.metadata = &compression_md; - op->flags = 0; - op->reserved = nullptr; - op++; - op->op = GRPC_OP_SEND_MESSAGE; - op->data.send_message.send_message = request_payload; - op->flags = 0; - op->reserved = nullptr; - op++; - op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT; - op->flags = 0; - op->reserved = nullptr; - op++; - op->op = GRPC_OP_RECV_INITIAL_METADATA; - op->data.recv_initial_metadata.recv_initial_metadata = &initial_metadata_recv; - op->flags = 0; - op->reserved = nullptr; - op++; - op->op = GRPC_OP_RECV_STATUS_ON_CLIENT; - op->data.recv_status_on_client.trailing_metadata = &trailing_metadata_recv; - op->data.recv_status_on_client.status = &status; - op->data.recv_status_on_client.status_details = &details; - op->flags = 0; - op->reserved = nullptr; - op++; - error = grpc_call_start_batch(c, ops, static_cast(op - ops), tag(1), - nullptr); - GPR_ASSERT(GRPC_CALL_OK == error); - - error = - grpc_server_request_call(f.server, &s, &call_details, - &request_metadata_recv, f.cq, f.cq, tag(101)); - GPR_ASSERT(GRPC_CALL_OK == error); - CQ_EXPECT_COMPLETION(cqv, tag(101), 1); - cq_verify(cqv); - - memset(ops, 0, sizeof(ops)); - op = ops; - op->op = GRPC_OP_RECV_CLOSE_ON_SERVER; - op->data.recv_close_on_server.cancelled = &was_cancelled; - op->flags = 0; - op->reserved = nullptr; - op++; - op->op = GRPC_OP_RECV_MESSAGE; - op->data.recv_message.recv_message = &recv_payload; - op->flags = 0; - op->reserved = nullptr; - op++; - if (minimal_stack) { - /* Expect the RPC to proceed normally for a minimal stack */ - op->op = GRPC_OP_SEND_INITIAL_METADATA; - op->data.send_initial_metadata.count = 0; - op->flags = 0; - op->reserved = nullptr; - op++; - op->op = GRPC_OP_SEND_STATUS_FROM_SERVER; - op->data.send_status_from_server.trailing_metadata_count = 0; - op->data.send_status_from_server.status = GRPC_STATUS_OK; - status_details = grpc_slice_from_static_string("xyz"); - op->data.send_status_from_server.status_details = &status_details; - op->flags = 0; - op->reserved = nullptr; - op++; - } - error = grpc_call_start_batch(s, ops, static_cast(op - ops), tag(102), - nullptr); - GPR_ASSERT(GRPC_CALL_OK == error); - - CQ_EXPECT_COMPLETION(cqv, tag(102), 1); - CQ_EXPECT_COMPLETION(cqv, tag(1), 1); - cq_verify(cqv); - - GPR_ASSERT(0 == grpc_slice_str_cmp(call_details.method, "/service/method")); - if (minimal_stack) { - /* We do not perform message size checks for minimal stack. */ - GPR_ASSERT(status == GRPC_STATUS_OK); - } else { - GPR_ASSERT(was_cancelled == 1); - GPR_ASSERT(status == GRPC_STATUS_RESOURCE_EXHAUSTED); - GPR_ASSERT(grpc_slice_str_cmp( - details, "Received message larger than max (29 vs. 5)") == - 0); - } - grpc_slice_unref(details); - grpc_slice_unref(request_payload_slice); - grpc_metadata_array_destroy(&initial_metadata_recv); - grpc_metadata_array_destroy(&trailing_metadata_recv); - grpc_metadata_array_destroy(&request_metadata_recv); - grpc_call_details_destroy(&call_details); - grpc_byte_buffer_destroy(request_payload); - grpc_byte_buffer_destroy(recv_payload); - grpc_call_unref(c); - if (s != nullptr) grpc_call_unref(s); - cq_verifier_destroy(cqv); - - end_test(&f); - config.tear_down_data(&f); -} - -// Test receive message limit with compressed response larger than the limit. -static void test_max_receive_message_length_on_compressed_response( - grpc_end2end_test_config config, bool minimal_stack) { - gpr_log(GPR_INFO, - "testing max receive message length on compressed response with " - "minimal_stack=%d", - minimal_stack); - grpc_end2end_test_fixture f; - grpc_call* c = nullptr; - grpc_call* s = nullptr; - cq_verifier* cqv; - grpc_op ops[6]; - grpc_op* op; - grpc_slice response_payload_slice = grpc_slice_malloc(1024); - memset(GRPC_SLICE_START_PTR(response_payload_slice), 'a', 1024); - grpc_byte_buffer* response_payload = - grpc_raw_byte_buffer_create(&response_payload_slice, 1); - grpc_byte_buffer* recv_payload = nullptr; - grpc_metadata_array initial_metadata_recv; - grpc_metadata_array trailing_metadata_recv; - grpc_metadata_array request_metadata_recv; - grpc_call_details call_details; - grpc_status_code status; - grpc_call_error error; - grpc_slice details; - int was_cancelled = 2; - - // Set limit via channel args. - grpc_arg arg[2]; - arg[0] = grpc_channel_arg_integer_create( - const_cast(GRPC_ARG_MAX_RECEIVE_MESSAGE_LENGTH), 5); - arg[1] = grpc_channel_arg_integer_create( - const_cast(GRPC_ARG_MINIMAL_STACK), minimal_stack); - grpc_channel_args* client_args = - grpc_channel_args_copy_and_add(nullptr, arg, 2); - - f = begin_test(config, "test_max_response_message_length", client_args, - nullptr); - { - grpc_core::ExecCtx exec_ctx; - grpc_channel_args_destroy(client_args); - } - cqv = cq_verifier_create(f.cq); - - c = grpc_channel_create_call(f.client, nullptr, GRPC_PROPAGATE_DEFAULTS, f.cq, - grpc_slice_from_static_string("/service/method"), - nullptr, gpr_inf_future(GPR_CLOCK_REALTIME), - nullptr); - GPR_ASSERT(c); - - grpc_metadata_array_init(&initial_metadata_recv); - grpc_metadata_array_init(&trailing_metadata_recv); - grpc_metadata_array_init(&request_metadata_recv); - grpc_call_details_init(&call_details); - - memset(ops, 0, sizeof(ops)); - op = ops; - op->op = GRPC_OP_SEND_INITIAL_METADATA; - op->data.send_initial_metadata.count = 0; - op->flags = 0; - op->reserved = nullptr; - op++; - op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT; - op->flags = 0; - op->reserved = nullptr; - op++; - op->op = GRPC_OP_RECV_INITIAL_METADATA; - op->data.recv_initial_metadata.recv_initial_metadata = &initial_metadata_recv; - op->flags = 0; - op->reserved = nullptr; - op++; - op->op = GRPC_OP_RECV_MESSAGE; - op->data.recv_message.recv_message = &recv_payload; - op->flags = 0; - op->reserved = nullptr; - op++; - op->op = GRPC_OP_RECV_STATUS_ON_CLIENT; - op->data.recv_status_on_client.trailing_metadata = &trailing_metadata_recv; - op->data.recv_status_on_client.status = &status; - op->data.recv_status_on_client.status_details = &details; - op->flags = 0; - op->reserved = nullptr; - op++; - error = grpc_call_start_batch(c, ops, static_cast(op - ops), tag(1), - nullptr); - GPR_ASSERT(GRPC_CALL_OK == error); - - error = - grpc_server_request_call(f.server, &s, &call_details, - &request_metadata_recv, f.cq, f.cq, tag(101)); - GPR_ASSERT(GRPC_CALL_OK == error); - CQ_EXPECT_COMPLETION(cqv, tag(101), 1); - cq_verify(cqv); - - grpc_metadata compression_md = gzip_compression_override(); - memset(ops, 0, sizeof(ops)); - op = ops; - op->op = GRPC_OP_SEND_INITIAL_METADATA; - op->data.send_initial_metadata.count = 1; - op->data.send_initial_metadata.metadata = &compression_md; - op->flags = 0; - op->reserved = nullptr; - op++; - op->op = GRPC_OP_RECV_CLOSE_ON_SERVER; - op->data.recv_close_on_server.cancelled = &was_cancelled; - op->flags = 0; - op->reserved = nullptr; - op++; - op->op = GRPC_OP_SEND_MESSAGE; - op->data.send_message.send_message = response_payload; - op->flags = 0; - op->reserved = nullptr; - op++; - op->op = GRPC_OP_SEND_STATUS_FROM_SERVER; - op->data.send_status_from_server.trailing_metadata_count = 0; - op->data.send_status_from_server.status = GRPC_STATUS_OK; - grpc_slice status_details = grpc_slice_from_static_string("xyz"); - op->data.send_status_from_server.status_details = &status_details; - op->flags = 0; - op->reserved = nullptr; - op++; - error = grpc_call_start_batch(s, ops, static_cast(op - ops), tag(102), - nullptr); - GPR_ASSERT(GRPC_CALL_OK == error); - - CQ_EXPECT_COMPLETION(cqv, tag(102), 1); - CQ_EXPECT_COMPLETION(cqv, tag(1), 1); - cq_verify(cqv); - - GPR_ASSERT(0 == grpc_slice_str_cmp(call_details.method, "/service/method")); - if (minimal_stack) { - /* We do not perform message size checks for minimal stack. */ - GPR_ASSERT(status == GRPC_STATUS_OK); - } else { - GPR_ASSERT(status == GRPC_STATUS_RESOURCE_EXHAUSTED); - GPR_ASSERT(grpc_slice_str_cmp( - details, "Received message larger than max (29 vs. 5)") == - 0); - } - grpc_slice_unref(details); - grpc_slice_unref(response_payload_slice); - grpc_metadata_array_destroy(&initial_metadata_recv); - grpc_metadata_array_destroy(&trailing_metadata_recv); - grpc_metadata_array_destroy(&request_metadata_recv); - grpc_call_details_destroy(&call_details); - grpc_byte_buffer_destroy(response_payload); - grpc_byte_buffer_destroy(recv_payload); - grpc_call_unref(c); if (s != nullptr) grpc_call_unref(s); @@ -823,15 +500,6 @@ void max_message_length(grpc_end2end_test_config config) { test_max_message_length_on_response(config, false /* send_limit */, true /* use_service_config */, true /* use_string_json_value */); - /* The following tests are not useful for inproc transport and do not work - * with our simple proxy. */ - if (strcmp(config.name, "inproc") != 0 && - (config.feature_mask & FEATURE_MASK_SUPPORTS_REQUEST_PROXYING) == 0) { - test_max_receive_message_length_on_compressed_request(config, false); - test_max_receive_message_length_on_compressed_request(config, true); - test_max_receive_message_length_on_compressed_response(config, false); - test_max_receive_message_length_on_compressed_response(config, true); - } } void max_message_length_pre_init(void) {} diff --git a/test/cpp/microbenchmarks/bm_call_create.cc b/test/cpp/microbenchmarks/bm_call_create.cc index b4a70272ea6..803ad38f1c5 100644 --- a/test/cpp/microbenchmarks/bm_call_create.cc +++ b/test/cpp/microbenchmarks/bm_call_create.cc @@ -529,10 +529,9 @@ static void BM_IsolatedFilter(benchmark::State& state) { grpc_call_final_info final_info; TestOp test_op_data; const int kArenaSize = 4096; - grpc_call_context_element context[GRPC_CONTEXT_COUNT] = {}; grpc_call_element_args call_args{call_stack, nullptr, - context, + nullptr, method, start_time, deadline, diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index d0828798b00..1a81817d3a9 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -1165,7 +1165,6 @@ src/core/ext/filters/client_channel/server_address.h \ src/core/ext/filters/client_channel/service_config.cc \ src/core/ext/filters/client_channel/service_config.h \ src/core/ext/filters/client_channel/service_config_call_data.h \ -src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc \ src/core/ext/filters/client_channel/service_config_parser.cc \ src/core/ext/filters/client_channel/service_config_parser.h \ src/core/ext/filters/client_channel/subchannel.cc \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index d33ceb61599..a4443ebd080 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -965,7 +965,6 @@ src/core/ext/filters/client_channel/server_address.h \ src/core/ext/filters/client_channel/service_config.cc \ src/core/ext/filters/client_channel/service_config.h \ src/core/ext/filters/client_channel/service_config_call_data.h \ -src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc \ src/core/ext/filters/client_channel/service_config_parser.cc \ src/core/ext/filters/client_channel/service_config_parser.h \ src/core/ext/filters/client_channel/subchannel.cc \ From 9b66452f7c3638de1bf2a81652eba6d842db0a51 Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Tue, 23 Jun 2020 11:41:38 -0700 Subject: [PATCH 11/11] Update PHP README.md --- src/php/README.md | 139 +++++++++++++++++++++++++--------------------- 1 file changed, 75 insertions(+), 64 deletions(-) diff --git a/src/php/README.md b/src/php/README.md index e3723cd2faf..0862ceac446 100644 --- a/src/php/README.md +++ b/src/php/README.md @@ -21,7 +21,7 @@ in 2020). ## Install the _grpc_ extension There are two ways to install the `grpc` extension. -* `pecl` +* Via `pecl` * Build from source ### Install from PECL @@ -33,11 +33,11 @@ $ [sudo] pecl install grpc or specific version ```sh -$ [sudo] pecl install grpc-1.25.0 +$ [sudo] pecl install grpc-1.30.0 ``` -Note: for users on CentOS/RHEL 6, unfortunately this step won’t work. -Please follow the instructions below to compile the PECL extension from source. +Please make sure your `gcc` version satisfies the minimum requirement as +specified [here](https://grpc.io/docs/languages/#official-support). ### Install on Windows @@ -47,7 +47,7 @@ You can download the pre-compiled `grpc.dll` extension from the PECL ### Build from source -Clone this repository at the [latest stable release tag](https://github.com/grpc/grpc/releases) +Clone this repository at the [latest stable release tag](https://github.com/grpc/grpc/releases). ```sh $ git clone -b RELEASE_TAG_HERE https://github.com/grpc/grpc @@ -94,10 +94,10 @@ extension=grpc.so In addition to the `grpc` extension, you will need to install the `grpc/grpc` composer package as well. Add this to your project's `composer.json` file. -``` - "require": { - "grpc/grpc": "v1.25.0" - } +```json + "require": { + "grpc/grpc": "~v1.30.0" + } ``` To run tests with generated stub code from `.proto` files, you will also @@ -127,32 +127,18 @@ the version of grpc inside package.xml file. The compatibility between the grpc and protobuf version is listed as table below: -grpc | protobuf ---- | --- -v1.0.0 | 3.0.0(GA) -v1.0.1 | 3.0.2 -v1.1.0 | 3.1.0 -v1.2.0 | 3.2.0 -v1.2.0 | 3.2.0 -v1.3.4 | 3.3.0 -v1.3.5 | 3.2.0 -v1.4.0 | 3.3.0 -v1.6.0 | 3.4.0 -v1.8.0 | 3.5.0 -v1.12.0 | 3.5.2 -v1.13.1 | 3.5.2 -v1.14.2 | 3.5.2 -v1.15.1 | 3.6.1 -v1.16.1 | 3.6.1 -v1.17.2 | 3.6.1 -v1.18.0 | 3.6.1 -v1.19.1 | 3.6.1 -v1.20.1 | 3.7.0 -v1.21.3 | 3.7.0 -v1.22.0 | 3.8.0 -v1.23.1 | 3.8.0 -v1.24.0 | 3.8.0 -v1.25.0 | 3.8.0 +grpc | protobuf | grpc | protobuf | grpc | protobuf +--- | --- | --- | --- | --- | --- +v1.0.0 | 3.0.0(GA) | v1.12.0 | 3.5.2 | v1.22.0 | 3.8.0 +v1.0.1 | 3.0.2 | v1.13.1 | 3.5.2 | v1.23.1 | 3.8.0 +v1.1.0 | 3.1.0 | v1.14.2 | 3.5.2 | v1.24.0 | 3.8.0 +v1.2.0 | 3.2.0 | v1.15.1 | 3.6.1 | v1.25.0 | 3.8.0 +v1.2.0 | 3.2.0 | v1.16.1 | 3.6.1 | v1.26.0 | 3.8.0 +v1.3.4 | 3.3.0 | v1.17.2 | 3.6.1 | v1.27.3 | 3.11.2 +v1.3.5 | 3.2.0 | v1.18.0 | 3.6.1 | v1.28.1 | 3.11.2 +v1.4.0 | 3.3.0 | v1.19.1 | 3.6.1 | v1.29.0 | 3.11.2 +v1.6.0 | 3.4.0 | v1.20.1 | 3.7.0 | v1.30.0 | 3.12.2 +v1.8.0 | 3.5.0 | v1.21.3 | 3.7.0 If `protoc` hasn't been installed, you can download the `protoc` binary from the protocol buffers @@ -190,6 +176,18 @@ $ git submodule update --init $ make grpc_php_plugin ``` +Alternatively, you can also build the `grpc_php_plugin` with `bazel` now: + +```sh +$ bazel build @com_google_protobuf//:protoc +$ bazel build src/compiler:grpc_php_plugin +``` + +The `protoc` binary will be found in +`bazel-bin/external/com_google_protobuf/protoc`. +The `grpc_php_plugin` binary will be found in +`bazel-bin/src/compiler/grpc_php_plugin`. + Plugin may use the new feature of the new protobuf version, thus please also make sure that the protobuf version installed is compatible with the grpc version you build this plugin. @@ -210,7 +208,7 @@ $ [sudo] pecl install protobuf or specific version ``` sh -$ [sudo] pecl install protobuf-3.8.0 +$ [sudo] pecl install protobuf-3.12.2 ``` And add this to your `php.ini` file: @@ -224,10 +222,10 @@ extension=protobuf.so Or require the `google/protobuf` composer package. Add this to your `composer.json` file: -``` - "require": { - "google/protobuf": "^v3.8.0" - } +```json + "require": { + "google/protobuf": "~v3.12.2" + } ``` ### Generate PHP classes from your service definition @@ -280,8 +278,8 @@ by the `./bin/generate_proto_php.sh` script. ### Run test server -Run a local server serving the math services. Please see [Node][] for how to -run an example server. +Run a local server serving the `Math` +[service](https://github.com/grpc/grpc/blob/master/src/proto/math/math.proto#L42). ```sh $ cd grpc/src/php/tests/generated_code @@ -312,7 +310,7 @@ end-to-end example. Here's how you can specify SSL credentials when creating your PHP client: -``` +```php $client = new Helloworld\GreeterClient('localhost:50051', [ 'credentials' => Grpc\ChannelCredentials::createSsl( file_get_contents('')) @@ -337,16 +335,27 @@ all possible values of the `grpc.grpc.trace` option, please check ``` grpc.grpc_verbosity=debug -grpc.grpc_trace=all,-timer_check +grpc.grpc_trace=all,-polling,-polling_api,-pollable_refcount,-timer,-timer_check grpc.log_filename=/var/log/grpc.log ``` +> Make sure the log file above is writable, by doing the following: +> ``` +> $ sudo touch /var/log/grpc.log +> $ sudo chmod 666 /var/log/grpc.log +> ``` +> Note: The log file does grow pretty quickly depending on how much logs are +> being printed out. Make sure you have other mechanisms (perhaps another +> cronjob) to zero out the log file from time to time, +> e.g. `cp /dev/null /var/log/grpc.log`, or turn these off when logs or tracing +> are not necessary for debugging purposes. + ### User agent string You can customize the user agent string for your gRPC PHP client by specifying this `grpc.primary_user_agent` option when constructing your PHP client: -``` +```php $client = new Helloworld\GreeterClient('localhost:50051', [ 'credentials' => Grpc\ChannelCredentials::createInsecure(), 'grpc.primary_user_agent' => 'my-user-agent-identifier', @@ -358,7 +367,7 @@ $client = new Helloworld\GreeterClient('localhost:50051', [ To change the default maximum message size, specify this `grpc.max_receive_message_length` option when constructing your PHP client: -``` +```php $client = new Helloworld\GreeterClient('localhost:50051', [ 'credentials' => Grpc\ChannelCredentials::createInsecure(), 'grpc.max_receive_message_length' => 8*1024*1024, @@ -369,26 +378,28 @@ $client = new Helloworld\GreeterClient('localhost:50051', [ You can customize the compression behavior on the client side, by specifying the following options when constructing your PHP client. +```php +$client = new Helloworld\GreeterClient('localhost:50051', [ + 'credentials' => Grpc\ChannelCredentials::createInsecure(), + 'grpc.default_compression_algorithm' => 2, + 'grpc.default_compression_level' => 2, +]); ``` -Possible values for grpc.default_compression_algorithm: -0 - No compression -1 - Compress with DEFLATE algorithm -2 - Compress with GZIP algorithm -3 - Stream compression with GZIP algorithm + +Possible values for `grpc.default_compression_algorithm`: + ``` +0: No compression +1: Compress with DEFLATE algorithm +2: Compress with GZIP algorithm +3: Stream compression with GZIP algorithm ``` -Possible values for grpc.default_compression_level: -0 - None -1 - Low level -2 - Medium level -3 - High level + +Possible values for `grpc.default_compression_level`: + ``` -Here's an example on how you can put them all together: +0: None +1: Low level +2: Medium level +3: High level ``` -$client = new Helloworld\GreeterClient('localhost:50051', [ - 'credentials' => Grpc\ChannelCredentials::createInsecure(), - 'grpc.default_compression_algorithm' => 2, - 'grpc.default_compression_level' => 2, -]); - -[Node]:https://github.com/grpc/grpc/tree/master/src/node/examples