From f66b6547959e1964932872e15498042156ee8c08 Mon Sep 17 00:00:00 2001 From: Karthik Ravi Shankar Date: Fri, 15 Mar 2019 11:30:23 -0700 Subject: [PATCH 1/6] Revert "Revert "Fold opencensus into grpc_impl namespace"" --- BUILD | 1 + include/grpcpp/opencensus.h | 26 +------- include/grpcpp/opencensus_impl.h | 51 +++++++++++++++ src/cpp/ext/filters/census/grpc_plugin.cc | 65 ++++++++++--------- src/cpp/ext/filters/census/grpc_plugin.h | 6 +- src/cpp/ext/filters/census/views.cc | 32 ++++----- .../census/stats_plugin_end2end_test.cc | 2 +- .../microbenchmarks/bm_opencensus_plugin.cc | 6 +- 8 files changed, 114 insertions(+), 75 deletions(-) create mode 100644 include/grpcpp/opencensus_impl.h diff --git a/BUILD b/BUILD index 835edbfb48f..afeacf0239a 100644 --- a/BUILD +++ b/BUILD @@ -2283,6 +2283,7 @@ 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 29b221f7674..3b170336834 100644 --- a/include/grpcpp/opencensus.h +++ b/include/grpcpp/opencensus.h @@ -19,30 +19,6 @@ #ifndef GRPCPP_OPENCENSUS_H #define GRPCPP_OPENCENSUS_H -#include "opencensus/trace/span.h" - -namespace grpc { -// 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(); - -class ServerContext; - -// Returns the tracing Span for the current RPC. -::opencensus::trace::Span GetSpanFromServerContext(ServerContext* context); - -} // namespace grpc +#include "grpcpp/opencensus_impl.h" #endif // GRPCPP_OPENCENSUS_H diff --git a/include/grpcpp/opencensus_impl.h b/include/grpcpp/opencensus_impl.h new file mode 100644 index 00000000000..631d2b861fd --- /dev/null +++ b/include/grpcpp/opencensus_impl.h @@ -0,0 +1,51 @@ +/* + * + * 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 { + +class ServerContext; +} +namespace grpc_impl { +// 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( + grpc::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 f978ed3bf51..c5018f0673a 100644 --- a/src/cpp/ext/filters/census/grpc_plugin.cc +++ b/src/cpp/ext/filters/census/grpc_plugin.cc @@ -30,35 +30,6 @@ namespace grpc { -void RegisterOpenCensusPlugin() { - RegisterChannelFilter( - "opencensus_client", GRPC_CLIENT_CHANNEL, INT_MAX /* priority */, - nullptr /* condition function */); - 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. - RpcClientSentBytesPerRpc(); - RpcClientReceivedBytesPerRpc(); - RpcClientRoundtripLatency(); - RpcClientServerLatency(); - RpcClientSentMessagesPerRpc(); - RpcClientReceivedMessagesPerRpc(); - - RpcServerSentBytesPerRpc(); - RpcServerReceivedBytesPerRpc(); - RpcServerServerLatency(); - RpcServerSentMessagesPerRpc(); - RpcServerReceivedMessagesPerRpc(); -} - -::opencensus::trace::Span GetSpanFromServerContext(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. @@ -126,5 +97,39 @@ 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/grpc_plugin.h b/src/cpp/ext/filters/census/grpc_plugin.h index 9e319cb994e..209fad139ce 100644 --- a/src/cpp/ext/filters/census/grpc_plugin.h +++ b/src/cpp/ext/filters/census/grpc_plugin.h @@ -22,12 +22,14 @@ #include #include "absl/strings/string_view.h" -#include "include/grpcpp/opencensus.h" +#include "include/grpcpp/opencensus_impl.h" #include "opencensus/stats/stats.h" -namespace grpc { +namespace grpc_impl { class ServerContext; +} +namespace grpc { // The tag keys set when recording RPC stats. ::opencensus::stats::TagKey ClientMethodTagKey(); diff --git a/src/cpp/ext/filters/census/views.cc b/src/cpp/ext/filters/census/views.cc index 2c0c5f72950..d7e3c81a955 100644 --- a/src/cpp/ext/filters/census/views.cc +++ b/src/cpp/ext/filters/census/views.cc @@ -25,6 +25,23 @@ #include "opencensus/stats/internal/set_aggregation_window.h" #include "opencensus/stats/stats.h" +namespace grpc_impl { + +void RegisterOpenCensusViewsForExport() { + grpc::ClientSentMessagesPerRpcCumulative().RegisterForExport(); + grpc::ClientSentBytesPerRpcCumulative().RegisterForExport(); + grpc::ClientReceivedMessagesPerRpcCumulative().RegisterForExport(); + grpc::ClientReceivedBytesPerRpcCumulative().RegisterForExport(); + grpc::ClientRoundtripLatencyCumulative().RegisterForExport(); + grpc::ClientServerLatencyCumulative().RegisterForExport(); + + grpc::ServerSentMessagesPerRpcCumulative().RegisterForExport(); + grpc::ServerSentBytesPerRpcCumulative().RegisterForExport(); + grpc::ServerReceivedMessagesPerRpcCumulative().RegisterForExport(); + grpc::ServerReceivedBytesPerRpcCumulative().RegisterForExport(); + grpc::ServerServerLatencyCumulative().RegisterForExport(); +} +} // namespace grpc_impl namespace grpc { using ::opencensus::stats::Aggregation; @@ -71,21 +88,6 @@ 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/ext/filters/census/stats_plugin_end2end_test.cc b/test/cpp/ext/filters/census/stats_plugin_end2end_test.cc index 73394028309..ad788a2dd68 100644 --- a/test/cpp/ext/filters/census/stats_plugin_end2end_test.cc +++ b/test/cpp/ext/filters/census/stats_plugin_end2end_test.cc @@ -58,7 +58,7 @@ class EchoServer final : public EchoTestService::Service { class StatsPluginEnd2EndTest : public ::testing::Test { protected: - static void SetUpTestCase() { RegisterOpenCensusPlugin(); } + static void SetUpTestCase() { grpc_impl::RegisterOpenCensusPlugin(); } void SetUp() { // Set up a synchronous server on a different thread to avoid the asynch diff --git a/test/cpp/microbenchmarks/bm_opencensus_plugin.cc b/test/cpp/microbenchmarks/bm_opencensus_plugin.cc index 9d42eb891df..d23c4f0573f 100644 --- a/test/cpp/microbenchmarks/bm_opencensus_plugin.cc +++ b/test/cpp/microbenchmarks/bm_opencensus_plugin.cc @@ -29,7 +29,9 @@ #include "test/cpp/microbenchmarks/helpers.h" absl::once_flag once; -void RegisterOnce() { absl::call_once(once, grpc::RegisterOpenCensusPlugin); } +void RegisterOnce() { + absl::call_once(once, grpc_impl::RegisterOpenCensusPlugin); +} class EchoServer final : public grpc::testing::EchoTestService::Service { grpc::Status Echo(grpc::ServerContext* context, @@ -99,7 +101,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. - grpc::RegisterOpenCensusViewsForExport(); + grpc_impl::RegisterOpenCensusViewsForExport(); EchoServerThread server; std::unique_ptr stub = From f570c5ce7e8b0e237b5ad047b22ef39f62c1a990 Mon Sep 17 00:00:00 2001 From: Karthik Ravi Shankar Date: Fri, 15 Mar 2019 11:43:28 -0700 Subject: [PATCH 2/6] Make changes for making opencensus API visible from gRPC namespace --- include/grpcpp/opencensus.h | 15 +++++++++++++++ .../filters/census/stats_plugin_end2end_test.cc | 3 ++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/include/grpcpp/opencensus.h b/include/grpcpp/opencensus.h index 3b170336834..14c6add1d9e 100644 --- a/include/grpcpp/opencensus.h +++ b/include/grpcpp/opencensus.h @@ -21,4 +21,19 @@ #include "grpcpp/opencensus_impl.h" +namespace grpc { + +static inline void RegisterOpenCensusPlugin() { + ::grpc_impl::RegisterOpenCensusPlugin(); +} +static inline void RegisterOpenCensusViewsForExport() { + ::grpc_impl::RegisterOpenCensusViewsForExport(); +} +static inline ::opencensus::trace::Span GetSpanFromServerContext( + ServerContext* context) { + return ::grpc_impl::GetSpanFromServerContext(context); +} + +} // namespace grpc + #endif // GRPCPP_OPENCENSUS_H 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 ad788a2dd68..1b40ca42254 100644 --- a/test/cpp/ext/filters/census/stats_plugin_end2end_test.cc +++ b/test/cpp/ext/filters/census/stats_plugin_end2end_test.cc @@ -25,6 +25,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" #include "include/grpc++/grpc++.h" +#include "include/grpcpp/opencensus.h" #include "opencensus/stats/stats.h" #include "opencensus/stats/testing/test_utils.h" #include "src/cpp/ext/filters/census/grpc_plugin.h" @@ -58,7 +59,7 @@ class EchoServer final : public EchoTestService::Service { class StatsPluginEnd2EndTest : public ::testing::Test { protected: - static void SetUpTestCase() { grpc_impl::RegisterOpenCensusPlugin(); } + static void SetUpTestCase() { grpc::RegisterOpenCensusPlugin(); } void SetUp() { // Set up a synchronous server on a different thread to avoid the asynch From 2a8f3f79abcb5628991605b2c9a442e4ced18fea Mon Sep 17 00:00:00 2001 From: Karthik Ravi Shankar Date: Fri, 15 Mar 2019 17:08:58 -0700 Subject: [PATCH 3/6] Fix more namespace stuff --- test/cpp/microbenchmarks/bm_opencensus_plugin.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/cpp/microbenchmarks/bm_opencensus_plugin.cc b/test/cpp/microbenchmarks/bm_opencensus_plugin.cc index d23c4f0573f..c3fa6f3031a 100644 --- a/test/cpp/microbenchmarks/bm_opencensus_plugin.cc +++ b/test/cpp/microbenchmarks/bm_opencensus_plugin.cc @@ -23,6 +23,7 @@ #include "absl/base/call_once.h" #include "absl/strings/str_cat.h" #include "include/grpc++/grpc++.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" @@ -30,7 +31,7 @@ absl::once_flag once; void RegisterOnce() { - absl::call_once(once, grpc_impl::RegisterOpenCensusPlugin); + absl::call_once(once, grpc::RegisterOpenCensusPlugin); } class EchoServer final : public grpc::testing::EchoTestService::Service { @@ -101,7 +102,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. - grpc_impl::RegisterOpenCensusViewsForExport(); + grpc::RegisterOpenCensusViewsForExport(); EchoServerThread server; std::unique_ptr stub = From 9a41671cb10e645dd490fdf3d3b7c7345e105d81 Mon Sep 17 00:00:00 2001 From: Karthik Ravi Shankar Date: Thu, 21 Mar 2019 14:22:06 -0700 Subject: [PATCH 4/6] Remove grpc:: from API to ensure general availability --- test/cpp/ext/filters/census/stats_plugin_end2end_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 1b40ca42254..0638c3343e6 100644 --- a/test/cpp/ext/filters/census/stats_plugin_end2end_test.cc +++ b/test/cpp/ext/filters/census/stats_plugin_end2end_test.cc @@ -59,7 +59,7 @@ class EchoServer final : public EchoTestService::Service { class StatsPluginEnd2EndTest : public ::testing::Test { protected: - static void SetUpTestCase() { grpc::RegisterOpenCensusPlugin(); } + static void SetUpTestCase() { RegisterOpenCensusPlugin(); } void SetUp() { // Set up a synchronous server on a different thread to avoid the asynch From 4260fe1147240cd8ceb6bc4a2c292f6a1e5e9c96 Mon Sep 17 00:00:00 2001 From: Karthik Ravi Shankar Date: Thu, 21 Mar 2019 17:51:28 -0700 Subject: [PATCH 5/6] More fixes --- src/cpp/ext/filters/census/grpc_plugin.h | 7 ++----- test/cpp/microbenchmarks/bm_opencensus_plugin.cc | 7 +++++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/cpp/ext/filters/census/grpc_plugin.h b/src/cpp/ext/filters/census/grpc_plugin.h index 209fad139ce..993faae0a94 100644 --- a/src/cpp/ext/filters/census/grpc_plugin.h +++ b/src/cpp/ext/filters/census/grpc_plugin.h @@ -22,14 +22,11 @@ #include #include "absl/strings/string_view.h" -#include "include/grpcpp/opencensus_impl.h" +#include "include/grpcpp/opencensus.h" #include "opencensus/stats/stats.h" -namespace grpc_impl { - -class ServerContext; -} namespace grpc { +class ServerContext; // The tag keys set when recording RPC stats. ::opencensus::stats::TagKey ClientMethodTagKey(); diff --git a/test/cpp/microbenchmarks/bm_opencensus_plugin.cc b/test/cpp/microbenchmarks/bm_opencensus_plugin.cc index c3fa6f3031a..0b376c0a0c3 100644 --- a/test/cpp/microbenchmarks/bm_opencensus_plugin.cc +++ b/test/cpp/microbenchmarks/bm_opencensus_plugin.cc @@ -29,9 +29,12 @@ #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, grpc::RegisterOpenCensusPlugin); + absl::call_once(once, RegisterOpenCensusPlugin); } class EchoServer final : public grpc::testing::EchoTestService::Service { @@ -102,7 +105,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. - grpc::RegisterOpenCensusViewsForExport(); + RegisterOpenCensusViewsForExport(); EchoServerThread server; std::unique_ptr stub = From 4e0923e802afe412e369937fde35c44e96f9014f Mon Sep 17 00:00:00 2001 From: Karthik Ravi Shankar Date: Mon, 25 Mar 2019 15:49:22 -0700 Subject: [PATCH 6/6] Fix errors from clang_format_code.sh --- test/cpp/microbenchmarks/bm_opencensus_plugin.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/cpp/microbenchmarks/bm_opencensus_plugin.cc b/test/cpp/microbenchmarks/bm_opencensus_plugin.cc index 0b376c0a0c3..4f4884bef9b 100644 --- a/test/cpp/microbenchmarks/bm_opencensus_plugin.cc +++ b/test/cpp/microbenchmarks/bm_opencensus_plugin.cc @@ -33,9 +33,7 @@ using ::grpc::RegisterOpenCensusPlugin; using ::grpc::RegisterOpenCensusViewsForExport; absl::once_flag once; -void RegisterOnce() { - absl::call_once(once, RegisterOpenCensusPlugin); -} +void RegisterOnce() { absl::call_once(once, RegisterOpenCensusPlugin); } class EchoServer final : public grpc::testing::EchoTestService::Service { grpc::Status Echo(grpc::ServerContext* context,