From bf1a26cde884d4aadcf240ee809df5d00ab6961a Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Wed, 22 Mar 2023 15:31:47 -0700 Subject: [PATCH] GcpObservability: Add client api latency (#32645) This PR adds the view `grpc.io/client/api_latency` for GCP Observability which aims to collect the end-to-end time taken by a call. Changes made to support this - 1) A global interceptor factory registration is created for stats plugins. 2) OpenCensus plugin now provides a new interceptor that's responsible for collecting the new latency. 3) Gcp Observability registers this plugin. 4) A new OpenCensus measurement and view is created for api latency. Note that this is internal as of now, since it's not clear if it should be exposed as public experimental API. Leaving that decision for the future. --- BUILD | 2 + CMakeLists.txt | 8 +++ build_autogenerated.yaml | 16 +++++ gRPC-C++.podspec | 3 + grpc.gyp | 2 + include/grpcpp/support/client_interceptor.h | 19 +++++- src/cpp/client/client_interceptor.cc | 3 +- src/cpp/client/client_stats_interceptor.cc | 40 +++++++++++++ src/cpp/client/client_stats_interceptor.h | 33 +++++++++++ src/cpp/ext/filters/census/client_filter.cc | 58 ++++++++++++++++++- src/cpp/ext/filters/census/client_filter.h | 10 ++++ src/cpp/ext/filters/census/grpc_plugin.cc | 3 + src/cpp/ext/filters/census/grpc_plugin.h | 12 +++- src/cpp/ext/filters/census/measures.cc | 13 +++++ src/cpp/ext/filters/census/measures.h | 4 ++ .../filters/census/open_census_call_tracer.h | 4 ++ src/cpp/ext/filters/census/views.cc | 15 +++++ src/cpp/ext/gcp/BUILD | 1 + src/cpp/ext/gcp/observability.cc | 12 +++- test/cpp/ext/filters/census/library.h | 4 ++ .../census/stats_plugin_end2end_test.cc | 13 +++++ tools/doxygen/Doxyfile.c++.internal | 2 + 22 files changed, 269 insertions(+), 8 deletions(-) create mode 100644 src/cpp/client/client_stats_interceptor.cc create mode 100644 src/cpp/client/client_stats_interceptor.h diff --git a/BUILD b/BUILD index b7c1f14fb75..044350b4d8c 100644 --- a/BUILD +++ b/BUILD @@ -265,6 +265,7 @@ GRPCXX_SRCS = [ "src/cpp/client/client_callback.cc", "src/cpp/client/client_context.cc", "src/cpp/client/client_interceptor.cc", + "src/cpp/client/client_stats_interceptor.cc", "src/cpp/client/create_channel.cc", "src/cpp/client/create_channel_internal.cc", "src/cpp/client/create_channel_posix.cc", @@ -296,6 +297,7 @@ GRPCXX_SRCS = [ GRPCXX_HDRS = [ "src/cpp/client/create_channel_internal.h", + "src/cpp/client/client_stats_interceptor.h", "src/cpp/common/channel_filter.h", "src/cpp/server/dynamic_thread_pool.h", "src/cpp/server/external_connection_acceptor_impl.h", diff --git a/CMakeLists.txt b/CMakeLists.txt index 8ae6cb92577..83f1a563936 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3350,6 +3350,7 @@ add_library(grpc++ src/cpp/client/client_callback.cc src/cpp/client/client_context.cc src/cpp/client/client_interceptor.cc + src/cpp/client/client_stats_interceptor.cc src/cpp/client/create_channel.cc src/cpp/client/create_channel_internal.cc src/cpp/client/create_channel_posix.cc @@ -4064,6 +4065,7 @@ add_library(grpc++_unsecure src/cpp/client/client_callback.cc src/cpp/client/client_context.cc src/cpp/client/client_interceptor.cc + src/cpp/client/client_stats_interceptor.cc src/cpp/client/create_channel.cc src/cpp/client/create_channel_internal.cc src/cpp/client/create_channel_posix.cc @@ -7110,6 +7112,7 @@ add_executable(binder_transport_test src/cpp/client/client_callback.cc src/cpp/client/client_context.cc src/cpp/client/client_interceptor.cc + src/cpp/client/client_stats_interceptor.cc src/cpp/client/create_channel.cc src/cpp/client/create_channel_internal.cc src/cpp/client/create_channel_posix.cc @@ -10008,6 +10011,7 @@ add_executable(endpoint_binder_pool_test src/cpp/client/client_callback.cc src/cpp/client/client_context.cc src/cpp/client/client_interceptor.cc + src/cpp/client/client_stats_interceptor.cc src/cpp/client/create_channel.cc src/cpp/client/create_channel_internal.cc src/cpp/client/create_channel_posix.cc @@ -10701,6 +10705,7 @@ add_executable(fake_binder_test src/cpp/client/client_callback.cc src/cpp/client/client_context.cc src/cpp/client/client_interceptor.cc + src/cpp/client/client_stats_interceptor.cc src/cpp/client/create_channel.cc src/cpp/client/create_channel_internal.cc src/cpp/client/create_channel_posix.cc @@ -21688,6 +21693,7 @@ add_executable(transport_stream_receiver_test src/cpp/client/client_callback.cc src/cpp/client/client_context.cc src/cpp/client/client_interceptor.cc + src/cpp/client/client_stats_interceptor.cc src/cpp/client/create_channel.cc src/cpp/client/create_channel_internal.cc src/cpp/client/create_channel_posix.cc @@ -22363,6 +22369,7 @@ add_executable(wire_reader_test src/cpp/client/client_callback.cc src/cpp/client/client_context.cc src/cpp/client/client_interceptor.cc + src/cpp/client/client_stats_interceptor.cc src/cpp/client/create_channel.cc src/cpp/client/create_channel_internal.cc src/cpp/client/create_channel_posix.cc @@ -22463,6 +22470,7 @@ add_executable(wire_writer_test src/cpp/client/client_callback.cc src/cpp/client/client_context.cc src/cpp/client/client_interceptor.cc + src/cpp/client/client_stats_interceptor.cc src/cpp/client/create_channel.cc src/cpp/client/create_channel_internal.cc src/cpp/client/create_channel_posix.cc diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 502fb5888ff..f5962b0ed91 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -3050,6 +3050,7 @@ libs: - src/core/ext/transport/binder/wire_format/wire_reader.h - src/core/ext/transport/binder/wire_format/wire_reader_impl.h - src/core/ext/transport/binder/wire_format/wire_writer.h + - src/cpp/client/client_stats_interceptor.h - src/cpp/client/create_channel_internal.h - src/cpp/client/secure_credentials.h - src/cpp/common/channel_filter.h @@ -3084,6 +3085,7 @@ libs: - src/cpp/client/client_callback.cc - src/cpp/client/client_context.cc - src/cpp/client/client_interceptor.cc + - src/cpp/client/client_stats_interceptor.cc - src/cpp/client/create_channel.cc - src/cpp/client/create_channel_internal.cc - src/cpp/client/create_channel_posix.cc @@ -3448,6 +3450,7 @@ libs: - include/grpcpp/support/validate_service_config.h - include/grpcpp/version_info.h headers: + - src/cpp/client/client_stats_interceptor.h - src/cpp/client/create_channel_internal.h - src/cpp/common/channel_filter.h - src/cpp/server/backend_metric_recorder.h @@ -3461,6 +3464,7 @@ libs: - src/cpp/client/client_callback.cc - src/cpp/client/client_context.cc - src/cpp/client/client_interceptor.cc + - src/cpp/client/client_stats_interceptor.cc - src/cpp/client/create_channel.cc - src/cpp/client/create_channel_internal.cc - src/cpp/client/create_channel_posix.cc @@ -5114,6 +5118,7 @@ targets: - src/core/ext/transport/binder/wire_format/wire_reader.h - src/core/ext/transport/binder/wire_format/wire_reader_impl.h - src/core/ext/transport/binder/wire_format/wire_writer.h + - src/cpp/client/client_stats_interceptor.h - src/cpp/client/create_channel_internal.h - src/cpp/client/secure_credentials.h - src/cpp/common/channel_filter.h @@ -5149,6 +5154,7 @@ targets: - src/cpp/client/client_callback.cc - src/cpp/client/client_context.cc - src/cpp/client/client_interceptor.cc + - src/cpp/client/client_stats_interceptor.cc - src/cpp/client/create_channel.cc - src/cpp/client/create_channel_internal.cc - src/cpp/client/create_channel_posix.cc @@ -6473,6 +6479,7 @@ targets: - src/core/ext/transport/binder/wire_format/wire_reader.h - src/core/ext/transport/binder/wire_format/wire_reader_impl.h - src/core/ext/transport/binder/wire_format/wire_writer.h + - src/cpp/client/client_stats_interceptor.h - src/cpp/client/create_channel_internal.h - src/cpp/client/secure_credentials.h - src/cpp/common/channel_filter.h @@ -6508,6 +6515,7 @@ targets: - src/cpp/client/client_callback.cc - src/cpp/client/client_context.cc - src/cpp/client/client_interceptor.cc + - src/cpp/client/client_stats_interceptor.cc - src/cpp/client/create_channel.cc - src/cpp/client/create_channel_internal.cc - src/cpp/client/create_channel_posix.cc @@ -6895,6 +6903,7 @@ targets: - src/core/ext/transport/binder/wire_format/wire_reader.h - src/core/ext/transport/binder/wire_format/wire_reader_impl.h - src/core/ext/transport/binder/wire_format/wire_writer.h + - src/cpp/client/client_stats_interceptor.h - src/cpp/client/create_channel_internal.h - src/cpp/client/secure_credentials.h - src/cpp/common/channel_filter.h @@ -6930,6 +6939,7 @@ targets: - src/cpp/client/client_callback.cc - src/cpp/client/client_context.cc - src/cpp/client/client_interceptor.cc + - src/cpp/client/client_stats_interceptor.cc - src/cpp/client/create_channel.cc - src/cpp/client/create_channel_internal.cc - src/cpp/client/create_channel_posix.cc @@ -12228,6 +12238,7 @@ targets: - src/core/ext/transport/binder/wire_format/wire_reader.h - src/core/ext/transport/binder/wire_format/wire_reader_impl.h - src/core/ext/transport/binder/wire_format/wire_writer.h + - src/cpp/client/client_stats_interceptor.h - src/cpp/client/create_channel_internal.h - src/cpp/client/secure_credentials.h - src/cpp/common/channel_filter.h @@ -12262,6 +12273,7 @@ targets: - src/cpp/client/client_callback.cc - src/cpp/client/client_context.cc - src/cpp/client/client_interceptor.cc + - src/cpp/client/client_stats_interceptor.cc - src/cpp/client/create_channel.cc - src/cpp/client/create_channel_internal.cc - src/cpp/client/create_channel_posix.cc @@ -12527,6 +12539,7 @@ targets: - src/core/ext/transport/binder/wire_format/wire_reader.h - src/core/ext/transport/binder/wire_format/wire_reader_impl.h - src/core/ext/transport/binder/wire_format/wire_writer.h + - src/cpp/client/client_stats_interceptor.h - src/cpp/client/create_channel_internal.h - src/cpp/client/secure_credentials.h - src/cpp/common/channel_filter.h @@ -12562,6 +12575,7 @@ targets: - src/cpp/client/client_callback.cc - src/cpp/client/client_context.cc - src/cpp/client/client_interceptor.cc + - src/cpp/client/client_stats_interceptor.cc - src/cpp/client/create_channel.cc - src/cpp/client/create_channel_internal.cc - src/cpp/client/create_channel_posix.cc @@ -12632,6 +12646,7 @@ targets: - src/core/ext/transport/binder/wire_format/wire_reader.h - src/core/ext/transport/binder/wire_format/wire_reader_impl.h - src/core/ext/transport/binder/wire_format/wire_writer.h + - src/cpp/client/client_stats_interceptor.h - src/cpp/client/create_channel_internal.h - src/cpp/client/secure_credentials.h - src/cpp/common/channel_filter.h @@ -12667,6 +12682,7 @@ targets: - src/cpp/client/client_callback.cc - src/cpp/client/client_context.cc - src/cpp/client/client_interceptor.cc + - src/cpp/client/client_stats_interceptor.cc - src/cpp/client/create_channel.cc - src/cpp/client/create_channel_internal.cc - src/cpp/client/create_channel_posix.cc diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index d09f057781b..d03714be789 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -1081,6 +1081,8 @@ Pod::Spec.new do |s| 'src/cpp/client/client_callback.cc', 'src/cpp/client/client_context.cc', 'src/cpp/client/client_interceptor.cc', + 'src/cpp/client/client_stats_interceptor.cc', + 'src/cpp/client/client_stats_interceptor.h', 'src/cpp/client/create_channel.cc', 'src/cpp/client/create_channel_internal.cc', 'src/cpp/client/create_channel_internal.h', @@ -2013,6 +2015,7 @@ Pod::Spec.new do |s| 'src/core/tsi/transport_security.h', 'src/core/tsi/transport_security_grpc.h', 'src/core/tsi/transport_security_interface.h', + 'src/cpp/client/client_stats_interceptor.h', 'src/cpp/client/create_channel_internal.h', 'src/cpp/client/secure_credentials.h', 'src/cpp/common/channel_filter.h', diff --git a/grpc.gyp b/grpc.gyp index cba8fd1e7aa..9acf4bee5b8 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -1610,6 +1610,7 @@ 'src/cpp/client/client_callback.cc', 'src/cpp/client/client_context.cc', 'src/cpp/client/client_interceptor.cc', + 'src/cpp/client/client_stats_interceptor.cc', 'src/cpp/client/create_channel.cc', 'src/cpp/client/create_channel_internal.cc', 'src/cpp/client/create_channel_posix.cc', @@ -1751,6 +1752,7 @@ 'src/cpp/client/client_callback.cc', 'src/cpp/client/client_context.cc', 'src/cpp/client/client_interceptor.cc', + 'src/cpp/client/client_stats_interceptor.cc', 'src/cpp/client/create_channel.cc', 'src/cpp/client/create_channel_internal.cc', 'src/cpp/client/create_channel_posix.cc', diff --git a/include/grpcpp/support/client_interceptor.h b/include/grpcpp/support/client_interceptor.h index ef84d184393..b2b9dc76917 100644 --- a/include/grpcpp/support/client_interceptor.h +++ b/include/grpcpp/support/client_interceptor.h @@ -57,7 +57,10 @@ class ClientInterceptorFactoryInterface { namespace internal { extern experimental::ClientInterceptorFactoryInterface* g_global_client_interceptor_factory; -} + +extern experimental::ClientInterceptorFactoryInterface* + g_global_client_stats_interceptor_factory; +} // namespace internal /// ClientRpcInfo represents the state of a particular RPC as it /// appears to an interceptor. It is created and owned by the library and @@ -144,10 +147,22 @@ class ClientRpcInfo { const std::vector>& creators, size_t interceptor_pos) { - if (interceptor_pos > creators.size()) { + // TODO(yashykt): This calculation seems broken for the case where an + // interceptor factor returns nullptr. + size_t num_interceptors = + creators.size() + + (internal::g_global_client_stats_interceptor_factory != nullptr) + + (internal::g_global_client_interceptor_factory != nullptr); + if (interceptor_pos > num_interceptors) { // No interceptors to register return; } + if (internal::g_global_client_stats_interceptor_factory != nullptr) { + interceptors_.push_back(std::unique_ptr( + internal::g_global_client_stats_interceptor_factory + ->CreateClientInterceptor(this))); + --interceptor_pos; + } // NOTE: The following is not a range-based for loop because it will only // iterate over a portion of the creators vector. for (auto it = creators.begin() + interceptor_pos; it != creators.end(); diff --git a/src/cpp/client/client_interceptor.cc b/src/cpp/client/client_interceptor.cc index 93181b825cb..dfa30d6fe41 100644 --- a/src/cpp/client/client_interceptor.cc +++ b/src/cpp/client/client_interceptor.cc @@ -25,7 +25,8 @@ namespace grpc { namespace internal { experimental::ClientInterceptorFactoryInterface* g_global_client_interceptor_factory = nullptr; -} + +} // namespace internal namespace experimental { void RegisterGlobalClientInterceptorFactory( diff --git a/src/cpp/client/client_stats_interceptor.cc b/src/cpp/client/client_stats_interceptor.cc new file mode 100644 index 00000000000..84c017ddbcf --- /dev/null +++ b/src/cpp/client/client_stats_interceptor.cc @@ -0,0 +1,40 @@ +// +// +// Copyright 2023 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. +// +// + +#include + +#include "src/core/lib/gprpp/crash.h" + +namespace grpc { +namespace internal { + +experimental::ClientInterceptorFactoryInterface* + g_global_client_stats_interceptor_factory = nullptr; + +void RegisterGlobalClientStatsInterceptorFactory( + grpc::experimental::ClientInterceptorFactoryInterface* factory) { + if (internal::g_global_client_stats_interceptor_factory != nullptr) { + grpc_core::Crash( + "It is illegal to call RegisterGlobalClientStatsInterceptorFactory " + "multiple times."); + } + internal::g_global_client_interceptor_factory = factory; +} + +} // namespace internal +} // namespace grpc diff --git a/src/cpp/client/client_stats_interceptor.h b/src/cpp/client/client_stats_interceptor.h new file mode 100644 index 00000000000..517f4e43ce0 --- /dev/null +++ b/src/cpp/client/client_stats_interceptor.h @@ -0,0 +1,33 @@ +// +// +// Copyright 2023 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 GRPC_SRC_CPP_CLIENT_CLIENT_STATS_INTERCEPTOR_H +#define GRPC_SRC_CPP_CLIENT_CLIENT_STATS_INTERCEPTOR_H + +#include + +namespace grpc { +namespace internal { + +void RegisterGlobalClientStatsInterceptorFactory( + grpc::experimental::ClientInterceptorFactoryInterface* factory); + +} +} // namespace grpc + +#endif // GRPC_SRC_CPP_CLIENT_CLIENT_STATS_INTERCEPTOR_H diff --git a/src/cpp/ext/filters/census/client_filter.cc b/src/cpp/ext/filters/census/client_filter.cc index c8448cbaad1..b6d1cfb9b3a 100644 --- a/src/cpp/ext/filters/census/client_filter.cc +++ b/src/cpp/ext/filters/census/client_filter.cc @@ -48,7 +48,9 @@ #include #include #include +#include #include +#include #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/channel_stack.h" @@ -59,6 +61,7 @@ #include "src/core/lib/resource_quota/arena.h" #include "src/core/lib/slice/slice.h" #include "src/core/lib/slice/slice_buffer.h" +#include "src/core/lib/surface/call.h" #include "src/core/lib/transport/metadata_batch.h" #include "src/core/lib/transport/transport.h" #include "src/cpp/ext/filters/census/context.h" @@ -215,8 +218,8 @@ void OpenCensusCallTracer::OpenCensusCallAttemptTracer:: std::vector> tags = context_.tags().tags(); tags.emplace_back(ClientMethodTagKey(), std::string(parent_->method_)); - std::string final_status = absl::StatusCodeToString(status_code_); - tags.emplace_back(ClientStatusTagKey(), final_status); + tags.emplace_back(ClientStatusTagKey(), + absl::StatusCodeToString(status_code_)); ::opencensus::stats::Record( {{RpcClientSentBytesPerRpc(), static_cast(transport_stream_stats->outgoing.data_bytes)}, @@ -352,6 +355,20 @@ void OpenCensusCallTracer::RecordAnnotation(absl::string_view annotation) { context_.AddSpanAnnotation(annotation, {}); } +void OpenCensusCallTracer::RecordApiLatency(absl::Duration api_latency, + absl::StatusCode status_code) { + if (OpenCensusStatsEnabled()) { + std::vector> tags = + context_.tags().tags(); + tags.emplace_back(ClientMethodTagKey(), std::string(method_)); + tags.emplace_back(ClientStatusTagKey(), + absl::StatusCodeToString(status_code)); + ::opencensus::stats::Record( + {{RpcClientApiLatency(), absl::ToDoubleMilliseconds(api_latency)}}, + tags); + } +} + CensusContext OpenCensusCallTracer::CreateCensusContextForCallAttempt() { if (!tracing_enabled_) return CensusContext(context_.tags()); GPR_DEBUG_ASSERT(context_.Context().IsValid()); @@ -362,5 +379,42 @@ CensusContext OpenCensusCallTracer::CreateCensusContextForCallAttempt() { return context; } +class OpenCensusClientInterceptor : public grpc::experimental::Interceptor { + public: + explicit OpenCensusClientInterceptor(grpc::experimental::ClientRpcInfo* info) + : info_(info), start_time_(absl::Now()) {} + + void Intercept( + grpc::experimental::InterceptorBatchMethods* methods) override { + if (methods->QueryInterceptionHookPoint( + grpc::experimental::InterceptionHookPoints::POST_RECV_STATUS)) { + auto* tracer = static_cast( + grpc_call_context_get(info_->client_context()->c_call(), + GRPC_CONTEXT_CALL_TRACER_ANNOTATION_INTERFACE)); + if (tracer != nullptr) { + tracer->RecordApiLatency(absl::Now() - start_time_, + static_cast( + methods->GetRecvStatus()->error_code())); + } + } + methods->Proceed(); + } + + private: + grpc::experimental::ClientRpcInfo* info_; + // Start time for measuring end-to-end API latency + absl::Time start_time_; +}; + +// +// OpenCensusClientInterceptorFactory +// + +grpc::experimental::Interceptor* +OpenCensusClientInterceptorFactory::CreateClientInterceptor( + grpc::experimental::ClientRpcInfo* info) { + return new OpenCensusClientInterceptor(info); +} + } // namespace internal } // namespace grpc diff --git a/src/cpp/ext/filters/census/client_filter.h b/src/cpp/ext/filters/census/client_filter.h index 85db1efc5cd..15465507c27 100644 --- a/src/cpp/ext/filters/census/client_filter.h +++ b/src/cpp/ext/filters/census/client_filter.h @@ -23,6 +23,9 @@ #include "absl/status/statusor.h" +#include +#include + #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/channel_fwd.h" #include "src/core/lib/channel/promise_based_filter.h" @@ -49,6 +52,13 @@ class OpenCensusClientFilter : public grpc_core::ChannelFilter { bool tracing_enabled_ = true; }; +class OpenCensusClientInterceptorFactory + : public grpc::experimental::ClientInterceptorFactoryInterface { + public: + grpc::experimental::Interceptor* CreateClientInterceptor( + grpc::experimental::ClientRpcInfo* info) override; +}; + } // namespace internal } // namespace grpc diff --git a/src/cpp/ext/filters/census/grpc_plugin.cc b/src/cpp/ext/filters/census/grpc_plugin.cc index eb62c7d5f5e..623ee575554 100644 --- a/src/cpp/ext/filters/census/grpc_plugin.cc +++ b/src/cpp/ext/filters/census/grpc_plugin.cc @@ -69,6 +69,7 @@ void RegisterOpenCensusPlugin() { RpcClientTransparentRetriesPerCall(); RpcClientRetryDelayPerCall(); RpcClientTransportLatency(); + internal::RpcClientApiLatency(); RpcServerSentBytesPerRpc(); RpcServerReceivedBytesPerRpc(); @@ -180,6 +181,8 @@ ABSL_CONST_INIT const absl::string_view kRpcServerStartedRpcsMeasureName = namespace internal { +ABSL_CONST_INIT const absl::string_view kRpcClientApiLatencyMeasureName = + "grpc.io/client/api_latency"; namespace { std::atomic g_open_census_stats_enabled(true); std::atomic g_open_census_tracing_enabled(true); diff --git a/src/cpp/ext/filters/census/grpc_plugin.h b/src/cpp/ext/filters/census/grpc_plugin.h index 9e2b837ddf8..ad0fd49f7bd 100644 --- a/src/cpp/ext/filters/census/grpc_plugin.h +++ b/src/cpp/ext/filters/census/grpc_plugin.h @@ -27,6 +27,8 @@ #include #include +#include "absl/strings/string_view.h" +#include "opencensus/stats/stats.h" #include "opencensus/tags/tag_key.h" #include "opencensus/tags/tag_map.h" @@ -127,8 +129,14 @@ using experimental::ServerStartedRpcsHour; // NOLINT namespace internal { -// Enables/Disables OpenCensus stats/tracing. It's only safe to do at the start -// of a program, before any channels/servers are built. +extern const absl::string_view kRpcClientApiLatencyMeasureName; + +// This view is in an internal namespace since this is meant just for GCP +// Observability purposes. +const ::opencensus::stats::ViewDescriptor& ClientApiLatency(); + +// Enables/Disables OpenCensus stats/tracing. It's only safe to do at the +// start of a program, before any channels/servers are built. void EnableOpenCensusStats(bool enable); void EnableOpenCensusTracing(bool enable); // Gets the current status of OpenCensus stats/tracing diff --git a/src/cpp/ext/filters/census/measures.cc b/src/cpp/ext/filters/census/measures.cc index d76d7e3c41b..8941d0bd906 100644 --- a/src/cpp/ext/filters/census/measures.cc +++ b/src/cpp/ext/filters/census/measures.cc @@ -24,6 +24,8 @@ #include +#include "src/cpp/ext/filters/census/grpc_plugin.h" + namespace grpc { using ::opencensus::stats::MeasureDouble; @@ -180,4 +182,15 @@ MeasureInt64 RpcServerReceivedMessagesPerRpc() { return measure; } +namespace internal { + +MeasureDouble RpcClientApiLatency() { + static const auto measure = MeasureDouble::Register( + kRpcClientApiLatencyMeasureName, + "End-to-end time taken to complete an RPC", kUnitMilliseconds); + return measure; +} + +} // namespace internal + } // namespace grpc diff --git a/src/cpp/ext/filters/census/measures.h b/src/cpp/ext/filters/census/measures.h index b20e0158ed5..5d0f39022c5 100644 --- a/src/cpp/ext/filters/census/measures.h +++ b/src/cpp/ext/filters/census/measures.h @@ -46,6 +46,10 @@ namespace grpc { ::opencensus::stats::MeasureInt64 RpcServerStartedRpcs(); ::opencensus::stats::MeasureInt64 RpcServerCompletedRpcs(); +namespace internal { +::opencensus::stats::MeasureDouble RpcClientApiLatency(); +} + } // namespace grpc #endif // GRPC_SRC_CPP_EXT_FILTERS_CENSUS_MEASURES_H diff --git a/src/cpp/ext/filters/census/open_census_call_tracer.h b/src/cpp/ext/filters/census/open_census_call_tracer.h index 56192e809ed..979be6544ab 100644 --- a/src/cpp/ext/filters/census/open_census_call_tracer.h +++ b/src/cpp/ext/filters/census/open_census_call_tracer.h @@ -112,6 +112,10 @@ class OpenCensusCallTracer : public grpc_core::ClientCallTracer { bool is_transparent_retry) override; void RecordAnnotation(absl::string_view annotation) override; + // APIs to record API call latency + void RecordApiLatency(absl::Duration api_latency, + absl::StatusCode status_code_); + private: experimental::CensusContext CreateCensusContextForCallAttempt(); diff --git a/src/cpp/ext/filters/census/views.cc b/src/cpp/ext/filters/census/views.cc index 522b5aab1d6..3d3234a8863 100644 --- a/src/cpp/ext/filters/census/views.cc +++ b/src/cpp/ext/filters/census/views.cc @@ -836,4 +836,19 @@ const ViewDescriptor& ServerReceivedMessagesPerRpcHour() { } // namespace experimental +namespace internal { + +const ViewDescriptor& ClientApiLatency() { + const static ViewDescriptor descriptor = + DefaultViewDescriptor() + .set_name("grpc.io/client/api_latency") + .set_measure(kRpcClientApiLatencyMeasureName) + .set_aggregation(MillisDistributionAggregation()) + .add_column(ClientMethodTagKey()) + .add_column(ClientStatusTagKey()); + return descriptor; +} + +} // namespace internal + } // namespace grpc diff --git a/src/cpp/ext/gcp/BUILD b/src/cpp/ext/gcp/BUILD index f13188c411d..fc579a0eb4a 100644 --- a/src/cpp/ext/gcp/BUILD +++ b/src/cpp/ext/gcp/BUILD @@ -58,6 +58,7 @@ grpc_cc_library( "//:gpr", "//:grpc", "//:grpc++", + "//:grpc++_base", "//:grpc_opencensus_plugin", "//src/core:logging_filter", "//src/core:notification", diff --git a/src/cpp/ext/gcp/observability.cc b/src/cpp/ext/gcp/observability.cc index 4ef0437ac32..24249fee5c7 100644 --- a/src/cpp/ext/gcp/observability.cc +++ b/src/cpp/ext/gcp/observability.cc @@ -46,6 +46,8 @@ #include "src/core/ext/filters/logging/logging_filter.h" #include "src/core/lib/gprpp/notification.h" +#include "src/cpp/client/client_stats_interceptor.h" +#include "src/cpp/ext/filters/census/client_filter.h" #include "src/cpp/ext/filters/census/grpc_plugin.h" #include "src/cpp/ext/filters/census/open_census_call_tracer.h" #include "src/cpp/ext/gcp/environment_autodetect.h" @@ -75,6 +77,7 @@ void RegisterOpenCensusViewsForGcpObservability() { ClientStartedRpcs().RegisterForExport(); ClientCompletedRpcs().RegisterForExport(); ClientRoundtripLatency().RegisterForExport(); + internal::ClientApiLatency().RegisterForExport(); ClientSentCompressedMessageBytesPerRpc().RegisterForExport(); ClientReceivedCompressedMessageBytesPerRpc().RegisterForExport(); // Register server default views for GCP observability @@ -105,9 +108,16 @@ absl::Status GcpObservabilityInit() { if (!config->cloud_monitoring.has_value()) { // Disable OpenCensus stats grpc::internal::EnableOpenCensusStats(false); + } else { + // Register the OpenCensus client stats interceptor factory if stats are + // enabled. Note that this is currently separate from the OpenCensus Plugin + // to avoid changing the behavior of the currently available OpenCensus + // plugin. + grpc::internal::RegisterGlobalClientStatsInterceptorFactory( + new grpc::internal::OpenCensusClientInterceptorFactory); } // If tracing or monitoring is enabled, we need to register the OpenCensus - // plugin to wait for the environment to be autodetected. + // plugin as well. if (config->cloud_trace.has_value() || config->cloud_monitoring.has_value()) { grpc::RegisterOpenCensusPlugin(); } diff --git a/test/cpp/ext/filters/census/library.h b/test/cpp/ext/filters/census/library.h index c3c6fa94a3f..9ccc04b1222 100644 --- a/test/cpp/ext/filters/census/library.h +++ b/test/cpp/ext/filters/census/library.h @@ -32,6 +32,8 @@ #include #include "src/core/lib/config/core_configuration.h" +#include "src/cpp/client/client_stats_interceptor.h" +#include "src/cpp/ext/filters/census/client_filter.h" #include "src/cpp/ext/filters/census/context.h" #include "src/proto/grpc/testing/echo.grpc.pb.h" #include "test/core/util/test_lb_policies.h" @@ -137,6 +139,8 @@ class StatsPluginEnd2EndTest : public ::testing::Test { [](grpc_core::CoreConfiguration::Builder* builder) { grpc_core::RegisterQueueOnceLoadBalancingPolicy(builder); }); + grpc::internal::RegisterGlobalClientStatsInterceptorFactory( + new grpc::internal::OpenCensusClientInterceptorFactory); RegisterOpenCensusPlugin(); // OpenCensus C++ has no API to unregister a previously-registered handler, // therefore we register this handler once, and enable/disable recording in diff --git a/test/cpp/ext/filters/census/stats_plugin_end2end_test.cc b/test/cpp/ext/filters/census/stats_plugin_end2end_test.cc index a2d777560b9..3428bbea985 100644 --- a/test/cpp/ext/filters/census/stats_plugin_end2end_test.cc +++ b/test/cpp/ext/filters/census/stats_plugin_end2end_test.cc @@ -217,6 +217,7 @@ TEST_F(StatsPluginEnd2EndTest, Latency) { View client_server_latency_view(ClientServerLatencyCumulative()); View server_server_latency_view(ServerServerLatencyCumulative()); View client_transport_latency_view(experimental::ClientTransportLatency()); + View client_api_latency_view(grpc::internal::ClientApiLatency()); const absl::Time start_time = absl::Now(); { @@ -279,6 +280,18 @@ TEST_F(StatsPluginEnd2EndTest, Latency) { ::testing::Lt(client_transport_latency)))))); } + // client api latency should be less than max time but greater than client + // roundtrip (attempt) latency view. + EXPECT_THAT( + client_api_latency_view.GetData().distribution_data(), + ::testing::UnorderedElementsAre(::testing::Pair( + ::testing::ElementsAre(client_method_name_, "OK"), + ::testing::AllOf(::testing::Property(&Distribution::count, 1), + ::testing::Property(&Distribution::mean, + ::testing::Gt(client_latency)), + ::testing::Property(&Distribution::mean, + ::testing::Lt(max_time)))))); + // client server elapsed time should be the same value propagated to the // client. const auto client_elapsed_time = client_server_latency_view.GetData() diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index 5d5d4cef2fa..e3e1cf38446 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -2710,6 +2710,8 @@ src/cpp/client/channel_cc.cc \ src/cpp/client/client_callback.cc \ src/cpp/client/client_context.cc \ src/cpp/client/client_interceptor.cc \ +src/cpp/client/client_stats_interceptor.cc \ +src/cpp/client/client_stats_interceptor.h \ src/cpp/client/create_channel.cc \ src/cpp/client/create_channel_internal.cc \ src/cpp/client/create_channel_internal.h \