From 42648a6fdfbd7250249750b25cfcb568ad9528b6 Mon Sep 17 00:00:00 2001 From: Tanvi Jagtap <139093547+tanvi-jagtap@users.noreply.github.com> Date: Mon, 10 Jun 2024 10:03:49 -0700 Subject: [PATCH 1/9] [Gpr_To_Absl_Logging] Replace gpr_should_log with ABSL_VLOG_IS_ON (#36868) [Gpr_To_Absl_Logging] Removing instances of gpr_should_log and replacing it with ABSL_VLOG_IS_ON. gpr_should_log function will be deleted soon. **VLOG(2)** is equivalent to gpr_log(**GPR_DEBUG**, ... ) We are replacing two instances of **gpr_log(GPR_INFO**, ... ) with **VLOG(2)** because this code appears to be slightly expensive. I can also continue to use LOG(INFO) and check current settings using absl::MinLogLevel() . Would you want me to do that? Closes #36868 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36868 from tanvi-jagtap:fix_tcp_windows_gpr_should_log c2b660d4e221dd6aaa1f3f5316d6bbe619274f23 PiperOrigin-RevId: 641938430 --- src/core/lib/iomgr/tcp_windows.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/core/lib/iomgr/tcp_windows.cc b/src/core/lib/iomgr/tcp_windows.cc index f4f8b9c7c3d..7e4a2e2d60d 100644 --- a/src/core/lib/iomgr/tcp_windows.cc +++ b/src/core/lib/iomgr/tcp_windows.cc @@ -25,6 +25,7 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" #include #include @@ -200,14 +201,13 @@ static void on_read(void* tcpp, grpc_error_handle error) { } CHECK((size_t)info->bytes_transferred == tcp->read_slices->length); - if (GRPC_TRACE_FLAG_ENABLED(tcp) && - gpr_should_log(GPR_LOG_SEVERITY_INFO)) { + if (GRPC_TRACE_FLAG_ENABLED(tcp) && ABSL_VLOG_IS_ON(2)) { size_t i; for (i = 0; i < tcp->read_slices->count; i++) { char* dump = grpc_dump_slice(tcp->read_slices->slices[i], GPR_DUMP_HEX | GPR_DUMP_ASCII); - gpr_log(GPR_INFO, "READ %p (peer=%s): %s", tcp, - tcp->peer_string.c_str(), dump); + VLOG(2) << "READ " << tcp << " (peer=" << tcp->peer_string + << "): " << dump; gpr_free(dump); } } @@ -350,13 +350,13 @@ static void win_write(grpc_endpoint* ep, grpc_slice_buffer* slices, WSABUF* buffers = local_buffers; size_t len, async_buffers_offset = 0; - if (GRPC_TRACE_FLAG_ENABLED(tcp) && gpr_should_log(GPR_LOG_SEVERITY_INFO)) { + if (GRPC_TRACE_FLAG_ENABLED(tcp) && ABSL_VLOG_IS_ON(2)) { size_t i; for (i = 0; i < slices->count; i++) { char* data = grpc_dump_slice(slices->slices[i], GPR_DUMP_HEX | GPR_DUMP_ASCII); - gpr_log(GPR_INFO, "WRITE %p (peer=%s): %s", tcp, tcp->peer_string.c_str(), - data); + VLOG(2) << "WRITE " << tcp << " (peer=" << tcp->peer_string + << "): " << data; gpr_free(data); } } From 203f2b4d16b897a40638c3ce5d9ac3f777103ddd Mon Sep 17 00:00:00 2001 From: Tanvi Jagtap <139093547+tanvi-jagtap@users.noreply.github.com> Date: Mon, 10 Jun 2024 10:06:40 -0700 Subject: [PATCH 2/9] [Gpr_To_Absl_Logging] Replace gpr_should_log with ABSL_VLOG_IS_ON (#36861) [Gpr_To_Absl_Logging] Removing instances of gpr_should_log. Replacing it with ABSL_VLOG_IS_ON. VLOG(2) is equivalent to gpr_log(GPR_DEBUG, ... ) This function will be deprecated and deleted soon. Closes #36861 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36861 from tanvi-jagtap:gpr_should_log_DEBUG_replace 09cd39e05a8d396cb90a8b99ccb9b570e0ff9759 PiperOrigin-RevId: 641939617 --- src/core/lib/iomgr/endpoint_cfstream.cc | 5 ++--- src/core/lib/iomgr/event_engine_shims/endpoint.cc | 4 ++-- src/core/lib/iomgr/tcp_posix.cc | 4 ++-- src/core/xds/grpc/xds_cluster.cc | 3 +-- src/core/xds/grpc/xds_endpoint.cc | 3 +-- src/core/xds/grpc/xds_listener.cc | 6 ++---- src/core/xds/grpc/xds_route_config.cc | 3 +-- src/core/xds/xds_client/xds_api.cc | 12 ++++-------- 8 files changed, 15 insertions(+), 25 deletions(-) diff --git a/src/core/lib/iomgr/endpoint_cfstream.cc b/src/core/lib/iomgr/endpoint_cfstream.cc index 157cca42795..3ce89bf7aa9 100644 --- a/src/core/lib/iomgr/endpoint_cfstream.cc +++ b/src/core/lib/iomgr/endpoint_cfstream.cc @@ -110,7 +110,7 @@ static grpc_error_handle CFStreamAnnotateError(grpc_error_handle src_error) { } static void CallReadCb(CFStreamEndpoint* ep, grpc_error_handle error) { - if (GRPC_TRACE_FLAG_ENABLED(tcp) && gpr_should_log(GPR_LOG_SEVERITY_DEBUG)) { + if (GRPC_TRACE_FLAG_ENABLED(tcp) && ABSL_VLOG_IS_ON(2)) { gpr_log(GPR_DEBUG, "CFStream endpoint:%p call_read_cb %p %p:%p", ep, ep->read_cb, ep->read_cb->cb, ep->read_cb->cb_arg); size_t i; @@ -221,8 +221,7 @@ static void WriteAction(void* arg, grpc_error_handle error) { EP_UNREF(ep, "write"); } - if (GRPC_TRACE_FLAG_ENABLED(tcp) && - gpr_should_log(GPR_LOG_SEVERITY_DEBUG)) { + if (GRPC_TRACE_FLAG_ENABLED(tcp) && ABSL_VLOG_IS_ON(2)) { grpc_slice trace_slice = grpc_slice_sub(slice, 0, write_size); char* dump = grpc_dump_slice(trace_slice, GPR_DUMP_HEX | GPR_DUMP_ASCII); gpr_log(GPR_DEBUG, "WRITE %p (peer=%s): %s", ep, ep->peer_string.c_str(), diff --git a/src/core/lib/iomgr/event_engine_shims/endpoint.cc b/src/core/lib/iomgr/event_engine_shims/endpoint.cc index a33d712f2b9..f83db8ea877 100644 --- a/src/core/lib/iomgr/event_engine_shims/endpoint.cc +++ b/src/core/lib/iomgr/event_engine_shims/endpoint.cc @@ -123,7 +123,7 @@ class EventEngineEndpointWrapper { size_t i; gpr_log(GPR_INFO, "TCP: %p READ error=%s", eeep_->wrapper, status.ToString().c_str()); - if (gpr_should_log(GPR_LOG_SEVERITY_DEBUG)) { + if (ABSL_VLOG_IS_ON(2)) { for (i = 0; i < pending_read_buffer_->count; i++) { char* dump = grpc_dump_slice(pending_read_buffer_->slices[i], GPR_DUMP_HEX | GPR_DUMP_ASCII); @@ -154,7 +154,7 @@ class EventEngineEndpointWrapper { size_t i; gpr_log(GPR_INFO, "TCP: %p WRITE (peer=%s)", this, std::string(PeerAddress()).c_str()); - if (gpr_should_log(GPR_LOG_SEVERITY_DEBUG)) { + if (ABSL_VLOG_IS_ON(2)) { for (i = 0; i < slices->count; i++) { char* dump = grpc_dump_slice(slices->slices[i], GPR_DUMP_HEX | GPR_DUMP_ASCII); diff --git a/src/core/lib/iomgr/tcp_posix.cc b/src/core/lib/iomgr/tcp_posix.cc index 9ceeacfece5..86cfbe3dcc7 100644 --- a/src/core/lib/iomgr/tcp_posix.cc +++ b/src/core/lib/iomgr/tcp_posix.cc @@ -857,7 +857,7 @@ static void tcp_trace_read(grpc_tcp* tcp, grpc_error_handle error) size_t i; gpr_log(GPR_INFO, "READ %p (peer=%s) error=%s", tcp, tcp->peer_string.c_str(), grpc_core::StatusToString(error).c_str()); - if (gpr_should_log(GPR_LOG_SEVERITY_DEBUG)) { + if (ABSL_VLOG_IS_ON(2)) { for (i = 0; i < tcp->incoming_buffer->count; i++) { char* dump = grpc_dump_slice(tcp->incoming_buffer->slices[i], GPR_DUMP_HEX | GPR_DUMP_ASCII); @@ -1849,7 +1849,7 @@ static void tcp_write(grpc_endpoint* ep, grpc_slice_buffer* buf, for (i = 0; i < buf->count; i++) { gpr_log(GPR_INFO, "WRITE %p (peer=%s)", tcp, tcp->peer_string.c_str()); - if (gpr_should_log(GPR_LOG_SEVERITY_DEBUG)) { + if (ABSL_VLOG_IS_ON(2)) { char* data = grpc_dump_slice(buf->slices[i], GPR_DUMP_HEX | GPR_DUMP_ASCII); VLOG(2) << "WRITE DATA: " << data; diff --git a/src/core/xds/grpc/xds_cluster.cc b/src/core/xds/grpc/xds_cluster.cc index d9670594f6b..2df6dd04f79 100644 --- a/src/core/xds/grpc/xds_cluster.cc +++ b/src/core/xds/grpc/xds_cluster.cc @@ -754,8 +754,7 @@ absl::StatusOr> CdsResourceParse( void MaybeLogCluster(const XdsResourceType::DecodeContext& context, const envoy_config_cluster_v3_Cluster* cluster) { - if (GRPC_TRACE_FLAG_ENABLED_OBJ(*context.tracer) && - gpr_should_log(GPR_LOG_SEVERITY_DEBUG)) { + if (GRPC_TRACE_FLAG_ENABLED_OBJ(*context.tracer) && ABSL_VLOG_IS_ON(2)) { const upb_MessageDef* msg_type = envoy_config_cluster_v3_Cluster_getmsgdef(context.symtab); char buf[10240]; diff --git a/src/core/xds/grpc/xds_endpoint.cc b/src/core/xds/grpc/xds_endpoint.cc index 6c44a2dd238..49d8f73ceea 100644 --- a/src/core/xds/grpc/xds_endpoint.cc +++ b/src/core/xds/grpc/xds_endpoint.cc @@ -156,8 +156,7 @@ namespace { void MaybeLogClusterLoadAssignment( const XdsResourceType::DecodeContext& context, const envoy_config_endpoint_v3_ClusterLoadAssignment* cla) { - if (GRPC_TRACE_FLAG_ENABLED_OBJ(*context.tracer) && - gpr_should_log(GPR_LOG_SEVERITY_DEBUG)) { + if (GRPC_TRACE_FLAG_ENABLED_OBJ(*context.tracer) && ABSL_VLOG_IS_ON(2)) { const upb_MessageDef* msg_type = envoy_config_endpoint_v3_ClusterLoadAssignment_getmsgdef( context.symtab); diff --git a/src/core/xds/grpc/xds_listener.cc b/src/core/xds/grpc/xds_listener.cc index b781e18f64c..c4528088fb5 100644 --- a/src/core/xds/grpc/xds_listener.cc +++ b/src/core/xds/grpc/xds_listener.cc @@ -285,8 +285,7 @@ void MaybeLogHttpConnectionManager( const XdsResourceType::DecodeContext& context, const envoy_extensions_filters_network_http_connection_manager_v3_HttpConnectionManager* http_connection_manager_config) { - if (GRPC_TRACE_FLAG_ENABLED_OBJ(*context.tracer) && - gpr_should_log(GPR_LOG_SEVERITY_DEBUG)) { + if (GRPC_TRACE_FLAG_ENABLED_OBJ(*context.tracer) && ABSL_VLOG_IS_ON(2)) { const upb_MessageDef* msg_type = envoy_extensions_filters_network_http_connection_manager_v3_HttpConnectionManager_getmsgdef( context.symtab); @@ -1091,8 +1090,7 @@ absl::StatusOr> LdsResourceParse( void MaybeLogListener(const XdsResourceType::DecodeContext& context, const envoy_config_listener_v3_Listener* listener) { - if (GRPC_TRACE_FLAG_ENABLED_OBJ(*context.tracer) && - gpr_should_log(GPR_LOG_SEVERITY_DEBUG)) { + if (GRPC_TRACE_FLAG_ENABLED_OBJ(*context.tracer) && ABSL_VLOG_IS_ON(2)) { const upb_MessageDef* msg_type = envoy_config_listener_v3_Listener_getmsgdef(context.symtab); char buf[10240]; diff --git a/src/core/xds/grpc/xds_route_config.cc b/src/core/xds/grpc/xds_route_config.cc index 4cb6c001e6f..d0c3af0df0e 100644 --- a/src/core/xds/grpc/xds_route_config.cc +++ b/src/core/xds/grpc/xds_route_config.cc @@ -1143,8 +1143,7 @@ namespace { void MaybeLogRouteConfiguration( const XdsResourceType::DecodeContext& context, const envoy_config_route_v3_RouteConfiguration* route_config) { - if (GRPC_TRACE_FLAG_ENABLED_OBJ(*context.tracer) && - gpr_should_log(GPR_LOG_SEVERITY_DEBUG)) { + if (GRPC_TRACE_FLAG_ENABLED_OBJ(*context.tracer) && ABSL_VLOG_IS_ON(2)) { const upb_MessageDef* msg_type = envoy_config_route_v3_RouteConfiguration_getmsgdef(context.symtab); char buf[10240]; diff --git a/src/core/xds/xds_client/xds_api.cc b/src/core/xds/xds_client/xds_api.cc index 40bd50e411c..1297752b659 100644 --- a/src/core/xds/xds_client/xds_api.cc +++ b/src/core/xds/xds_client/xds_api.cc @@ -130,8 +130,7 @@ void PopulateMetadataValue(google_protobuf_Value* value_pb, const Json& value, void MaybeLogDiscoveryRequest( const XdsApiContext& context, const envoy_service_discovery_v3_DiscoveryRequest* request) { - if (GRPC_TRACE_FLAG_ENABLED_OBJ(*context.tracer) && - gpr_should_log(GPR_LOG_SEVERITY_DEBUG)) { + if (GRPC_TRACE_FLAG_ENABLED_OBJ(*context.tracer) && ABSL_VLOG_IS_ON(2)) { const upb_MessageDef* msg_type = envoy_service_discovery_v3_DiscoveryRequest_getmsgdef(context.def_pool); char buf[10240]; @@ -262,8 +261,7 @@ namespace { void MaybeLogDiscoveryResponse( const XdsApiContext& context, const envoy_service_discovery_v3_DiscoveryResponse* response) { - if (GRPC_TRACE_FLAG_ENABLED_OBJ(*context.tracer) && - gpr_should_log(GPR_LOG_SEVERITY_DEBUG)) { + if (GRPC_TRACE_FLAG_ENABLED_OBJ(*context.tracer) && ABSL_VLOG_IS_ON(2)) { const upb_MessageDef* msg_type = envoy_service_discovery_v3_DiscoveryResponse_getmsgdef( context.def_pool); @@ -351,8 +349,7 @@ namespace { void MaybeLogLrsRequest( const XdsApiContext& context, const envoy_service_load_stats_v3_LoadStatsRequest* request) { - if (GRPC_TRACE_FLAG_ENABLED_OBJ(*context.tracer) && - gpr_should_log(GPR_LOG_SEVERITY_DEBUG)) { + if (GRPC_TRACE_FLAG_ENABLED_OBJ(*context.tracer) && ABSL_VLOG_IS_ON(2)) { const upb_MessageDef* msg_type = envoy_service_load_stats_v3_LoadStatsRequest_getmsgdef( context.def_pool); @@ -513,8 +510,7 @@ namespace { void MaybeLogLrsResponse( const XdsApiContext& context, const envoy_service_load_stats_v3_LoadStatsResponse* response) { - if (GRPC_TRACE_FLAG_ENABLED_OBJ(*context.tracer) && - gpr_should_log(GPR_LOG_SEVERITY_DEBUG)) { + if (GRPC_TRACE_FLAG_ENABLED_OBJ(*context.tracer) && ABSL_VLOG_IS_ON(2)) { const upb_MessageDef* msg_type = envoy_service_load_stats_v3_LoadStatsResponse_getmsgdef( context.def_pool); From fabf135c7f43f22e79dcb47fd0adf42cb3f64bc1 Mon Sep 17 00:00:00 2001 From: Tanvi Jagtap <139093547+tanvi-jagtap@users.noreply.github.com> Date: Mon, 10 Jun 2024 10:06:43 -0700 Subject: [PATCH 3/9] [Gpr_To_Absl_Logging] Replace gpr_should_log with ABSL_VLOG_IS_ON (#36866) [Gpr_To_Absl_Logging] Removing instances of gpr_should_log and replacing it with ABSL_VLOG_IS_ON. gpr_should_log function will be deleted soon. **VLOG(2)** is equivalent to gpr_log(**GPR_DEBUG**, ... ) We are replacing two instances of **gpr_log(GPR_INFO**, ... ) with **VLOG(2)** because this code appears to be slightly expensive. I can also continue to use LOG(INFO) and check current settings using absl::MinLogLevel() . Would you want me to do that? Closes #36866 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36866 from tanvi-jagtap:fix_secure_endpoint_gpr_should 556006c720ffdf6247d174065c9970092be4cf9e PiperOrigin-RevId: 641939641 --- src/core/handshaker/security/secure_endpoint.cc | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/core/handshaker/security/secure_endpoint.cc b/src/core/handshaker/security/secure_endpoint.cc index 2ca7e6773e2..18fde152438 100644 --- a/src/core/handshaker/security/secure_endpoint.cc +++ b/src/core/handshaker/security/secure_endpoint.cc @@ -26,6 +26,7 @@ #include "absl/base/thread_annotations.h" #include "absl/log/check.h" +#include "absl/log/log.h" #include "absl/status/status.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" @@ -232,13 +233,12 @@ static void flush_read_staging_buffer(secure_endpoint* ep, uint8_t** cur, } static void call_read_cb(secure_endpoint* ep, grpc_error_handle error) { - if (GRPC_TRACE_FLAG_ENABLED(secure_endpoint) && - gpr_should_log(GPR_LOG_SEVERITY_INFO)) { + if (GRPC_TRACE_FLAG_ENABLED(secure_endpoint) && ABSL_VLOG_IS_ON(2)) { size_t i; for (i = 0; i < ep->read_buffer->count; i++) { char* data = grpc_dump_slice(ep->read_buffer->slices[i], GPR_DUMP_HEX | GPR_DUMP_ASCII); - gpr_log(GPR_INFO, "READ %p: %s", ep, data); + VLOG(2) << "READ " << ep << ": " << data; gpr_free(data); } } @@ -400,12 +400,11 @@ static void endpoint_write(grpc_endpoint* secure_ep, grpc_slice_buffer* slices, grpc_slice_buffer_reset_and_unref(&ep->output_buffer); - if (GRPC_TRACE_FLAG_ENABLED(secure_endpoint) && - gpr_should_log(GPR_LOG_SEVERITY_INFO)) { + if (GRPC_TRACE_FLAG_ENABLED(secure_endpoint) && ABSL_VLOG_IS_ON(2)) { for (i = 0; i < slices->count; i++) { char* data = grpc_dump_slice(slices->slices[i], GPR_DUMP_HEX | GPR_DUMP_ASCII); - gpr_log(GPR_INFO, "WRITE %p: %s", ep, data); + VLOG(2) << "WRITE " << ep << ": " << data; gpr_free(data); } } From 87321f08b32a2197bcb4b70e8b2ab8c43083e4d5 Mon Sep 17 00:00:00 2001 From: Yijie Ma Date: Mon, 10 Jun 2024 14:50:09 -0700 Subject: [PATCH 4/9] [OTPlugin] Per-channel OpenTelemetry plugin (#36729) Closes #36729 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36729 from yijiem:per-channel-stats-plugin 4786bed42f11b0a164f21e040439c3145f5d1e3d PiperOrigin-RevId: 642030366 --- BUILD | 1 + include/grpc/impl/channel_arg_names.h | 2 + include/grpcpp/ext/otel_plugin.h | 35 ++- src/core/lib/surface/legacy_channel.cc | 24 +- src/core/telemetry/metrics.h | 8 + src/cpp/ext/otel/key_value_iterable.h | 10 +- src/cpp/ext/otel/otel_client_call_tracer.cc | 64 ++--- src/cpp/ext/otel/otel_client_call_tracer.h | 12 +- src/cpp/ext/otel/otel_plugin.cc | 114 +++++++-- src/cpp/ext/otel/otel_plugin.h | 49 +++- src/cpp/ext/otel/otel_server_call_tracer.cc | 8 +- src/cpp/ext/otel/otel_server_call_tracer.h | 12 +- test/core/test_util/fake_stats_plugin.h | 8 + test/cpp/ext/csm/csm_observability_test.cc | 26 +- test/cpp/ext/otel/otel_plugin_test.cc | 255 +++++++++++++++++++- test/cpp/ext/otel/otel_test_library.cc | 52 +++- test/cpp/ext/otel/otel_test_library.h | 25 ++ 17 files changed, 591 insertions(+), 114 deletions(-) diff --git a/BUILD b/BUILD index 39ed4cea3bd..50dba1e5e71 100644 --- a/BUILD +++ b/BUILD @@ -2950,6 +2950,7 @@ grpc_cc_library( ], language = "c++", deps = [ + ":grpc++", "//src/cpp/ext/otel:otel_plugin", ], ) diff --git a/include/grpc/impl/channel_arg_names.h b/include/grpc/impl/channel_arg_names.h index efd380b8681..c47d928d5c1 100644 --- a/include/grpc/impl/channel_arg_names.h +++ b/include/grpc/impl/channel_arg_names.h @@ -398,6 +398,8 @@ * If unspecified, it is unlimited */ #define GRPC_ARG_MAX_ALLOWED_INCOMING_CONNECTIONS \ "grpc.max_allowed_incoming_connections" +/** Configure per-channel or per-server stats plugins. */ +#define GRPC_ARG_EXPERIMENTAL_STATS_PLUGINS "grpc.experimental.stats_plugins" /** \} */ #endif /* GRPC_IMPL_CHANNEL_ARG_NAMES_H */ diff --git a/include/grpcpp/ext/otel_plugin.h b/include/grpcpp/ext/otel_plugin.h index 1b6bb9eb762..9de57fbb496 100644 --- a/include/grpcpp/ext/otel_plugin.h +++ b/include/grpcpp/ext/otel_plugin.h @@ -26,17 +26,18 @@ #include "absl/functional/any_invocable.h" #include "absl/status/status.h" +#include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include "opentelemetry/metrics/meter_provider.h" #include #include +#include +#include namespace grpc { - namespace internal { class OpenTelemetryPluginBuilderImpl; -class OpenTelemetryPlugin; } // namespace internal class OpenTelemetryPluginOption { @@ -44,6 +45,21 @@ class OpenTelemetryPluginOption { virtual ~OpenTelemetryPluginOption() = default; }; +namespace experimental { +/// EXPERIMENTAL API +class OpenTelemetryPlugin { + public: + virtual ~OpenTelemetryPlugin() = default; + /// EXPERIMENTAL API + /// Adds this OpenTelemetryPlugin to the channel args \a args. + virtual void AddToChannelArguments(grpc::ChannelArguments* args) = 0; + /// EXPERIMENTAL API + /// Adds this OpenTelemetryPlugin to the channel arguments that will be used + /// to create the server through \a builder. + virtual void AddToServerBuilder(grpc::ServerBuilder* builder) = 0; +}; +} // namespace experimental + /// The most common way to use this API is - /// /// OpenTelemetryPluginBuilder().SetMeterProvider(provider).BuildAndRegister(); @@ -113,8 +129,8 @@ class OpenTelemetryPluginBuilder { /// If set, \a generic_method_attribute_filter is called per call with a /// generic method type to decide whether to record the method name or to /// replace it with "other". Non-generic or pre-registered methods remain - /// unaffected. If not set, by default, generic method names are replaced with - /// "other" when recording metrics. + /// unaffected. If not set, by default, generic method names are replaced + /// with "other" when recording metrics. OpenTelemetryPluginBuilder& SetGenericMethodAttributeFilter( absl::AnyInvocable generic_method_attribute_filter); @@ -139,9 +155,16 @@ class OpenTelemetryPluginBuilder { OpenTelemetryPluginBuilder& SetChannelScopeFilter( absl::AnyInvocable channel_scope_filter); - /// Registers a global plugin that acts on all channels and servers running on - /// the process. + /// Builds and registers a global plugin that acts on all channels and servers + /// running on the process. Must be called no more than once and must not be + /// called if Build() is called. absl::Status BuildAndRegisterGlobal(); + /// EXPERIMENTAL API + /// Builds an open telemetry plugin, returns the plugin object when succeeded + /// or an error status when failed. Must be called no more than once and must + /// not be called if BuildAndRegisterGlobal() is called. + GRPC_MUST_USE_RESULT + absl::StatusOr> Build(); private: std::unique_ptr impl_; diff --git a/src/core/lib/surface/legacy_channel.cc b/src/core/lib/surface/legacy_channel.cc index 0b12f96274c..57196e97c63 100644 --- a/src/core/lib/surface/legacy_channel.cc +++ b/src/core/lib/surface/legacy_channel.cc @@ -92,14 +92,34 @@ absl::StatusOr> LegacyChannel::Create( if (channel_stack_type == GRPC_SERVER_CHANNEL) { *(*r)->stats_plugin_group = GlobalStatsPluginRegistry::GetStatsPluginsForServer(args); + // Add per-server stats plugins. + auto* stats_plugin_list = args.GetPointer< + std::shared_ptr>>>( + GRPC_ARG_EXPERIMENTAL_STATS_PLUGINS); + if (stats_plugin_list != nullptr) { + for (const auto& plugin : **stats_plugin_list) { + (*r)->stats_plugin_group->AddStatsPlugin( + plugin, plugin->GetServerScopeConfig(args)); + } + } } else { std::string authority = args.GetOwnedString(GRPC_ARG_DEFAULT_AUTHORITY) .value_or(CoreConfiguration::Get() .resolver_registry() .GetDefaultAuthority(target)); + experimental::StatsPluginChannelScope scope(target, authority); *(*r)->stats_plugin_group = - GlobalStatsPluginRegistry::GetStatsPluginsForChannel( - experimental::StatsPluginChannelScope(target, authority)); + GlobalStatsPluginRegistry::GetStatsPluginsForChannel(scope); + // Add per-channel stats plugins. + auto* stats_plugin_list = args.GetPointer< + std::shared_ptr>>>( + GRPC_ARG_EXPERIMENTAL_STATS_PLUGINS); + if (stats_plugin_list != nullptr) { + for (const auto& plugin : **stats_plugin_list) { + (*r)->stats_plugin_group->AddStatsPlugin( + plugin, plugin->GetChannelScopeConfig(scope)); + } + } } return MakeRefCounted( grpc_channel_stack_type_is_client(builder.channel_stack_type()), diff --git a/src/core/telemetry/metrics.h b/src/core/telemetry/metrics.h index 38a51b442f4..eec7d075dce 100644 --- a/src/core/telemetry/metrics.h +++ b/src/core/telemetry/metrics.h @@ -289,6 +289,14 @@ class StatsPlugin { // configure the ServerCallTracer in GetServerCallTracer(). virtual std::pair> IsEnabledForServer( const ChannelArgs& args) const = 0; + // Gets a scope config for the client channel specified by \a scope. Note that + // the stats plugin should have been enabled for the channel. + virtual std::shared_ptr GetChannelScopeConfig( + const experimental::StatsPluginChannelScope& scope) const = 0; + // Gets a scope config for the server specified by \a args. Note that the + // stats plugin should have been enabled for the server. + virtual std::shared_ptr GetServerScopeConfig( + const ChannelArgs& args) const = 0; // Adds \a value to the uint64 counter specified by \a handle. \a label_values // and \a optional_label_values specify attributes that are associated with diff --git a/src/cpp/ext/otel/key_value_iterable.h b/src/cpp/ext/otel/key_value_iterable.h index 48fdb9a62b0..caad77aee36 100644 --- a/src/cpp/ext/otel/key_value_iterable.h +++ b/src/cpp/ext/otel/key_value_iterable.h @@ -47,7 +47,7 @@ inline opentelemetry::nostd::string_view AbslStrViewToOpenTelemetryStrView( // An iterable class based on opentelemetry::common::KeyValueIterable that // allows gRPC to iterate on its various sources of attributes and avoid an // allocation in cases wherever possible. -class OpenTelemetryPlugin::KeyValueIterable +class OpenTelemetryPluginImpl::KeyValueIterable : public opentelemetry::common::KeyValueIterable { public: KeyValueIterable( @@ -55,10 +55,10 @@ class OpenTelemetryPlugin::KeyValueIterable injected_labels_from_plugin_options, absl::Span> additional_labels, - const OpenTelemetryPlugin::ActivePluginOptionsView* + const OpenTelemetryPluginImpl::ActivePluginOptionsView* active_plugin_options_view, absl::Span optional_labels, - bool is_client, const OpenTelemetryPlugin* otel_plugin) + bool is_client, const OpenTelemetryPluginImpl* otel_plugin) : injected_labels_from_plugin_options_( injected_labels_from_plugin_options), additional_labels_(additional_labels), @@ -149,11 +149,11 @@ class OpenTelemetryPlugin::KeyValueIterable injected_labels_from_plugin_options_; absl::Span> additional_labels_; - const OpenTelemetryPlugin::ActivePluginOptionsView* + const OpenTelemetryPluginImpl::ActivePluginOptionsView* active_plugin_options_view_; absl::Span optional_labels_; bool is_client_; - const OpenTelemetryPlugin* otel_plugin_; + const OpenTelemetryPluginImpl* otel_plugin_; }; } // namespace internal diff --git a/src/cpp/ext/otel/otel_client_call_tracer.cc b/src/cpp/ext/otel/otel_client_call_tracer.cc index 1fdfe086d8b..80202def13e 100644 --- a/src/cpp/ext/otel/otel_client_call_tracer.cc +++ b/src/cpp/ext/otel/otel_client_call_tracer.cc @@ -61,11 +61,12 @@ namespace grpc { namespace internal { // -// OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer +// OpenTelemetryPluginImpl::ClientCallTracer::CallAttemptTracer // -OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer::CallAttemptTracer( - const OpenTelemetryPlugin::ClientCallTracer* parent, bool arena_allocated) +OpenTelemetryPluginImpl::ClientCallTracer::CallAttemptTracer::CallAttemptTracer( + const OpenTelemetryPluginImpl::ClientCallTracer* parent, + bool arena_allocated) : parent_(parent), arena_allocated_(arena_allocated), start_time_(absl::Now()) { @@ -86,7 +87,7 @@ OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer::CallAttemptTracer( } } -void OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer:: +void OpenTelemetryPluginImpl::ClientCallTracer::CallAttemptTracer:: RecordReceivedInitialMetadata(grpc_metadata_batch* recv_initial_metadata) { if (recv_initial_metadata != nullptr && recv_initial_metadata->get(grpc_core::GrpcTrailersOnly()) @@ -97,7 +98,7 @@ void OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer:: PopulateLabelInjectors(recv_initial_metadata); } -void OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer:: +void OpenTelemetryPluginImpl::ClientCallTracer::CallAttemptTracer:: RecordSendInitialMetadata(grpc_metadata_batch* send_initial_metadata) { parent_->scope_config_->active_plugin_options_view().ForEach( [&](const InternalOpenTelemetryPluginOption& plugin_option, @@ -111,33 +112,33 @@ void OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer:: parent_->otel_plugin_); } -void OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer:: +void OpenTelemetryPluginImpl::ClientCallTracer::CallAttemptTracer:: RecordSendMessage(const grpc_core::SliceBuffer& send_message) { RecordAnnotation( absl::StrFormat("Send message: %ld bytes", send_message.Length())); } -void OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer:: +void OpenTelemetryPluginImpl::ClientCallTracer::CallAttemptTracer:: RecordSendCompressedMessage( const grpc_core::SliceBuffer& send_compressed_message) { RecordAnnotation(absl::StrFormat("Send compressed message: %ld bytes", send_compressed_message.Length())); } -void OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer:: +void OpenTelemetryPluginImpl::ClientCallTracer::CallAttemptTracer:: RecordReceivedMessage(const grpc_core::SliceBuffer& recv_message) { RecordAnnotation( absl::StrFormat("Received message: %ld bytes", recv_message.Length())); } -void OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer:: +void OpenTelemetryPluginImpl::ClientCallTracer::CallAttemptTracer:: RecordReceivedDecompressedMessage( const grpc_core::SliceBuffer& recv_decompressed_message) { RecordAnnotation(absl::StrFormat("Received decompressed message: %ld bytes", recv_decompressed_message.Length())); } -void OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer:: +void OpenTelemetryPluginImpl::ClientCallTracer::CallAttemptTracer:: RecordReceivedTrailingMetadata( absl::Status status, grpc_metadata_batch* recv_trailing_metadata, const grpc_transport_stream_stats* transport_stream_stats) { @@ -179,10 +180,10 @@ void OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer:: } } -void OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer::RecordCancel( +void OpenTelemetryPluginImpl::ClientCallTracer::CallAttemptTracer::RecordCancel( absl::Status /*cancel_error*/) {} -void OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer::RecordEnd( +void OpenTelemetryPluginImpl::ClientCallTracer::CallAttemptTracer::RecordEnd( const gpr_timespec& /*latency*/) { if (arena_allocated_) { this->~CallAttemptTracer(); @@ -191,29 +192,30 @@ void OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer::RecordEnd( } } -void OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer::RecordAnnotation( - absl::string_view /*annotation*/) { +void OpenTelemetryPluginImpl::ClientCallTracer::CallAttemptTracer:: + RecordAnnotation(absl::string_view /*annotation*/) { // Not implemented } -void OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer::RecordAnnotation( - const Annotation& /*annotation*/) { +void OpenTelemetryPluginImpl::ClientCallTracer::CallAttemptTracer:: + RecordAnnotation(const Annotation& /*annotation*/) { // Not implemented } -std::shared_ptr -OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer::StartNewTcpTrace() { +std::shared_ptr OpenTelemetryPluginImpl:: + ClientCallTracer::CallAttemptTracer::StartNewTcpTrace() { // No TCP trace. return nullptr; } -void OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer::SetOptionalLabel( - OptionalLabelKey key, grpc_core::RefCountedStringValue value) { +void OpenTelemetryPluginImpl::ClientCallTracer::CallAttemptTracer:: + SetOptionalLabel(OptionalLabelKey key, + grpc_core::RefCountedStringValue value) { CHECK(key < OptionalLabelKey::kSize); optional_labels_[static_cast(key)] = std::move(value); } -void OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer:: +void OpenTelemetryPluginImpl::ClientCallTracer::CallAttemptTracer:: PopulateLabelInjectors(grpc_metadata_batch* metadata) { parent_->scope_config_->active_plugin_options_view().ForEach( [&](const InternalOpenTelemetryPluginOption& plugin_option, @@ -229,23 +231,23 @@ void OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer:: } // -// OpenTelemetryPlugin::ClientCallTracer +// OpenTelemetryPluginImpl::ClientCallTracer // -OpenTelemetryPlugin::ClientCallTracer::ClientCallTracer( +OpenTelemetryPluginImpl::ClientCallTracer::ClientCallTracer( const grpc_core::Slice& path, grpc_core::Arena* arena, - bool registered_method, OpenTelemetryPlugin* otel_plugin, - std::shared_ptr scope_config) + bool registered_method, OpenTelemetryPluginImpl* otel_plugin, + std::shared_ptr scope_config) : path_(path.Ref()), arena_(arena), registered_method_(registered_method), otel_plugin_(otel_plugin), scope_config_(std::move(scope_config)) {} -OpenTelemetryPlugin::ClientCallTracer::~ClientCallTracer() {} +OpenTelemetryPluginImpl::ClientCallTracer::~ClientCallTracer() {} -OpenTelemetryPlugin::ClientCallTracer::CallAttemptTracer* -OpenTelemetryPlugin::ClientCallTracer::StartNewAttempt( +OpenTelemetryPluginImpl::ClientCallTracer::CallAttemptTracer* +OpenTelemetryPluginImpl::ClientCallTracer::StartNewAttempt( bool is_transparent_retry) { // We allocate the first attempt on the arena and all subsequent attempts // on the heap, so that in the common case we don't require a heap @@ -268,7 +270,7 @@ OpenTelemetryPlugin::ClientCallTracer::StartNewAttempt( return new CallAttemptTracer(this, /*arena_allocated=*/false); } -absl::string_view OpenTelemetryPlugin::ClientCallTracer::MethodForStats() +absl::string_view OpenTelemetryPluginImpl::ClientCallTracer::MethodForStats() const { absl::string_view method = absl::StripPrefix(path_.as_string_view(), "/"); if (registered_method_ || @@ -279,12 +281,12 @@ absl::string_view OpenTelemetryPlugin::ClientCallTracer::MethodForStats() return "other"; } -void OpenTelemetryPlugin::ClientCallTracer::RecordAnnotation( +void OpenTelemetryPluginImpl::ClientCallTracer::RecordAnnotation( absl::string_view /*annotation*/) { // Not implemented } -void OpenTelemetryPlugin::ClientCallTracer::RecordAnnotation( +void OpenTelemetryPluginImpl::ClientCallTracer::RecordAnnotation( const Annotation& /*annotation*/) { // Not implemented } diff --git a/src/cpp/ext/otel/otel_client_call_tracer.h b/src/cpp/ext/otel/otel_client_call_tracer.h index 366c83064f1..99a5f1c4602 100644 --- a/src/cpp/ext/otel/otel_client_call_tracer.h +++ b/src/cpp/ext/otel/otel_client_call_tracer.h @@ -46,13 +46,13 @@ namespace grpc { namespace internal { -class OpenTelemetryPlugin::ClientCallTracer +class OpenTelemetryPluginImpl::ClientCallTracer : public grpc_core::ClientCallTracer { public: class CallAttemptTracer : public grpc_core::ClientCallTracer::CallAttemptTracer { public: - CallAttemptTracer(const OpenTelemetryPlugin::ClientCallTracer* parent, + CallAttemptTracer(const OpenTelemetryPluginImpl::ClientCallTracer* parent, bool arena_allocated); std::string TraceId() override { @@ -113,8 +113,8 @@ class OpenTelemetryPlugin::ClientCallTracer ClientCallTracer( const grpc_core::Slice& path, grpc_core::Arena* arena, - bool registered_method, OpenTelemetryPlugin* otel_plugin, - std::shared_ptr scope_config); + bool registered_method, OpenTelemetryPluginImpl* otel_plugin, + std::shared_ptr scope_config); ~ClientCallTracer() override; std::string TraceId() override { @@ -143,8 +143,8 @@ class OpenTelemetryPlugin::ClientCallTracer grpc_core::Slice path_; grpc_core::Arena* arena_; const bool registered_method_; - OpenTelemetryPlugin* otel_plugin_; - std::shared_ptr scope_config_; + OpenTelemetryPluginImpl* otel_plugin_; + std::shared_ptr scope_config_; grpc_core::Mutex mu_; // Non-transparent attempts per call uint64_t retries_ ABSL_GUARDED_BY(&mu_) = 0; diff --git a/src/cpp/ext/otel/otel_plugin.cc b/src/cpp/ext/otel/otel_plugin.cc index 55116f5b947..07bae2c06ca 100644 --- a/src/cpp/ext/otel/otel_plugin.cc +++ b/src/cpp/ext/otel/otel_plugin.cc @@ -86,7 +86,7 @@ absl::flat_hash_set BaseMetrics() { } } // namespace -class OpenTelemetryPlugin::NPCMetricsKeyValueIterable +class OpenTelemetryPluginImpl::NPCMetricsKeyValueIterable : public opentelemetry::common::KeyValueIterable { public: NPCMetricsKeyValueIterable( @@ -229,10 +229,11 @@ OpenTelemetryPluginBuilderImpl::SetChannelScopeFilter( absl::Status OpenTelemetryPluginBuilderImpl::BuildAndRegisterGlobal() { if (meter_provider_ == nullptr) { - return absl::OkStatus(); + return absl::InvalidArgumentError( + "Need to configure a valid meter provider."); } grpc_core::GlobalStatsPluginRegistry::RegisterStatsPlugin( - std::make_shared( + std::make_shared( metrics_, meter_provider_, std::move(target_attribute_filter_), std::move(generic_method_attribute_filter_), std::move(server_selector_), std::move(plugin_options_), @@ -240,8 +241,22 @@ absl::Status OpenTelemetryPluginBuilderImpl::BuildAndRegisterGlobal() { return absl::OkStatus(); } -OpenTelemetryPlugin::CallbackMetricReporter::CallbackMetricReporter( - OpenTelemetryPlugin* ot_plugin, grpc_core::RegisteredMetricCallback* key) +absl::StatusOr> +OpenTelemetryPluginBuilderImpl::Build() { + if (meter_provider_ == nullptr) { + return absl::InvalidArgumentError( + "Need to configure a valid meter provider."); + } + return std::make_shared( + metrics_, meter_provider_, std::move(target_attribute_filter_), + std::move(generic_method_attribute_filter_), std::move(server_selector_), + std::move(plugin_options_), std::move(optional_label_keys_), + std::move(channel_scope_filter_)); +} + +OpenTelemetryPluginImpl::CallbackMetricReporter::CallbackMetricReporter( + OpenTelemetryPluginImpl* ot_plugin, + grpc_core::RegisteredMetricCallback* key) : ot_plugin_(ot_plugin), key_(key) { // Since we are updating the timestamp and updating the cache for all // registered instruments in a RegisteredMetricCallback, we will need to @@ -275,7 +290,7 @@ OpenTelemetryPlugin::CallbackMetricReporter::CallbackMetricReporter( } } -void OpenTelemetryPlugin::CallbackMetricReporter::ReportInt64( +void OpenTelemetryPluginImpl::CallbackMetricReporter::ReportInt64( grpc_core::GlobalInstrumentsRegistry::GlobalInstrumentHandle handle, int64_t value, absl::Span label_values, absl::Span optional_values) { @@ -303,7 +318,7 @@ void OpenTelemetryPlugin::CallbackMetricReporter::ReportInt64( cell.insert_or_assign(std::move(key), value); } -void OpenTelemetryPlugin::CallbackMetricReporter::ReportDouble( +void OpenTelemetryPluginImpl::CallbackMetricReporter::ReportDouble( grpc_core::GlobalInstrumentsRegistry::GlobalInstrumentHandle handle, double value, absl::Span label_values, absl::Span optional_values) { @@ -331,7 +346,12 @@ void OpenTelemetryPlugin::CallbackMetricReporter::ReportDouble( cell.insert_or_assign(std::move(key), value); } -OpenTelemetryPlugin::OpenTelemetryPlugin( +void OpenTelemetryPluginImpl::ServerBuilderOption::UpdateArguments( + grpc::ChannelArguments* args) { + plugin_->AddToChannelArguments(args); +} + +OpenTelemetryPluginImpl::OpenTelemetryPluginImpl( const absl::flat_hash_set& metrics, opentelemetry::nostd::shared_ptr meter_provider, @@ -548,7 +568,7 @@ namespace { constexpr absl::string_view kLocality = "grpc.lb.locality"; } -absl::string_view OpenTelemetryPlugin::OptionalLabelKeyToString( +absl::string_view OpenTelemetryPluginImpl::OptionalLabelKeyToString( grpc_core::ClientCallTracer::CallAttemptTracer::OptionalLabelKey key) { switch (key) { case grpc_core::ClientCallTracer::CallAttemptTracer::OptionalLabelKey:: @@ -560,7 +580,7 @@ absl::string_view OpenTelemetryPlugin::OptionalLabelKeyToString( } absl::optional -OpenTelemetryPlugin::OptionalLabelStringToKey(absl::string_view key) { +OpenTelemetryPluginImpl::OptionalLabelStringToKey(absl::string_view key) { if (key == kLocality) { return grpc_core::ClientCallTracer::CallAttemptTracer::OptionalLabelKey:: kLocality; @@ -569,7 +589,7 @@ OpenTelemetryPlugin::OptionalLabelStringToKey(absl::string_view key) { } std::pair> -OpenTelemetryPlugin::IsEnabledForChannel( +OpenTelemetryPluginImpl::IsEnabledForChannel( const OpenTelemetryPluginBuilder::ChannelScope& scope) const { if (channel_scope_filter_ == nullptr || channel_scope_filter_(scope)) { return {true, std::make_shared(this, scope)}; @@ -578,7 +598,7 @@ OpenTelemetryPlugin::IsEnabledForChannel( } std::pair> -OpenTelemetryPlugin::IsEnabledForServer( +OpenTelemetryPluginImpl::IsEnabledForServer( const grpc_core::ChannelArgs& args) const { // Return true only if there is no server selector registered or if the // server selector returns true. @@ -588,7 +608,21 @@ OpenTelemetryPlugin::IsEnabledForServer( return {false, nullptr}; } -void OpenTelemetryPlugin::AddCounter( +std::shared_ptr +OpenTelemetryPluginImpl::GetChannelScopeConfig( + const OpenTelemetryPluginBuilder::ChannelScope& scope) const { + GPR_ASSERT(channel_scope_filter_ == nullptr || channel_scope_filter_(scope)); + return std::make_shared(this, scope); +} + +std::shared_ptr +OpenTelemetryPluginImpl::GetServerScopeConfig( + const grpc_core::ChannelArgs& args) const { + GPR_ASSERT(server_selector_ == nullptr || server_selector_(args)); + return std::make_shared(this, args); +} + +void OpenTelemetryPluginImpl::AddCounter( grpc_core::GlobalInstrumentsRegistry::GlobalInstrumentHandle handle, uint64_t value, absl::Span label_values, absl::Span optional_values) { @@ -612,7 +646,7 @@ void OpenTelemetryPlugin::AddCounter( instrument_data.optional_labels_bits)); } -void OpenTelemetryPlugin::AddCounter( +void OpenTelemetryPluginImpl::AddCounter( grpc_core::GlobalInstrumentsRegistry::GlobalInstrumentHandle handle, double value, absl::Span label_values, absl::Span optional_values) { @@ -636,7 +670,7 @@ void OpenTelemetryPlugin::AddCounter( instrument_data.optional_labels_bits)); } -void OpenTelemetryPlugin::RecordHistogram( +void OpenTelemetryPluginImpl::RecordHistogram( grpc_core::GlobalInstrumentsRegistry::GlobalInstrumentHandle handle, uint64_t value, absl::Span label_values, absl::Span optional_values) { @@ -662,7 +696,7 @@ void OpenTelemetryPlugin::RecordHistogram( opentelemetry::context::Context{}); } -void OpenTelemetryPlugin::RecordHistogram( +void OpenTelemetryPluginImpl::RecordHistogram( grpc_core::GlobalInstrumentsRegistry::GlobalInstrumentHandle handle, double value, absl::Span label_values, absl::Span optional_values) { @@ -688,7 +722,7 @@ void OpenTelemetryPlugin::RecordHistogram( opentelemetry::context::Context{}); } -void OpenTelemetryPlugin::AddCallback( +void OpenTelemetryPluginImpl::AddCallback( grpc_core::RegisteredMetricCallback* callback) { std::vector< absl::variant*, CallbackGaugeState*>> @@ -764,7 +798,7 @@ void OpenTelemetryPlugin::AddCallback( } } -void OpenTelemetryPlugin::RemoveCallback( +void OpenTelemetryPluginImpl::RemoveCallback( grpc_core::RegisteredMetricCallback* callback) { std::vector< absl::variant*, CallbackGaugeState*>> @@ -841,7 +875,7 @@ void OpenTelemetryPlugin::RemoveCallback( } template -void OpenTelemetryPlugin::CallbackGaugeState::Observe( +void OpenTelemetryPluginImpl::CallbackGaugeState::Observe( opentelemetry::metrics::ObserverResult& result, const Cache& cache) { const auto& descriptor = grpc_core::GlobalInstrumentsRegistry::GetInstrumentDescriptor({id}); @@ -868,8 +902,9 @@ void OpenTelemetryPlugin::CallbackGaugeState::Observe( // OpenTelemetry calls our callback with its observable_registry's lock // held. template -void OpenTelemetryPlugin::CallbackGaugeState::CallbackGaugeCallback( - opentelemetry::metrics::ObserverResult result, void* arg) { +void OpenTelemetryPluginImpl::CallbackGaugeState:: + CallbackGaugeCallback(opentelemetry::metrics::ObserverResult result, + void* arg) { auto* callback_gauge_state = static_cast*>(arg); auto now = grpc_core::Timestamp::Now(); grpc_core::MutexLock plugin_lock(&callback_gauge_state->ot_plugin->mu_); @@ -892,32 +927,54 @@ void OpenTelemetryPlugin::CallbackGaugeState::CallbackGaugeCallback( } } -grpc_core::ClientCallTracer* OpenTelemetryPlugin::GetClientCallTracer( +grpc_core::ClientCallTracer* OpenTelemetryPluginImpl::GetClientCallTracer( const grpc_core::Slice& path, bool registered_method, std::shared_ptr scope_config) { return grpc_core::GetContext() ->ManagedNew( path, grpc_core::GetContext(), registered_method, this, - std::static_pointer_cast( + std::static_pointer_cast( scope_config)); } -grpc_core::ServerCallTracer* OpenTelemetryPlugin::GetServerCallTracer( +grpc_core::ServerCallTracer* OpenTelemetryPluginImpl::GetServerCallTracer( std::shared_ptr scope_config) { return grpc_core::GetContext() ->ManagedNew( this, - std::static_pointer_cast( + std::static_pointer_cast( scope_config)); } -bool OpenTelemetryPlugin::IsInstrumentEnabled( +bool OpenTelemetryPluginImpl::IsInstrumentEnabled( grpc_core::GlobalInstrumentsRegistry::GlobalInstrumentHandle handle) const { return !absl::holds_alternative( instruments_data_.at(handle.index).instrument); } +void OpenTelemetryPluginImpl::AddToChannelArguments( + grpc::ChannelArguments* args) { + const grpc_channel_args c_args = args->c_channel_args(); + auto* stats_plugin_list = grpc_channel_args_find_pointer< + std::shared_ptr>>>( + &c_args, GRPC_ARG_EXPERIMENTAL_STATS_PLUGINS); + if (stats_plugin_list != nullptr) { + (*stats_plugin_list)->emplace_back(shared_from_this()); + } else { + auto stats_plugin_list = std::make_shared< + std::vector>>(); + args->SetPointerWithVtable( + GRPC_ARG_EXPERIMENTAL_STATS_PLUGINS, &stats_plugin_list, + grpc_core::ChannelArgTypeTraits::VTable()); + stats_plugin_list->emplace_back(shared_from_this()); + } +} + +void OpenTelemetryPluginImpl::AddToServerBuilder(grpc::ServerBuilder* builder) { + builder->SetOption(std::make_unique(shared_from_this())); +} + } // namespace internal constexpr absl::string_view @@ -1012,4 +1069,9 @@ absl::Status OpenTelemetryPluginBuilder::BuildAndRegisterGlobal() { return impl_->BuildAndRegisterGlobal(); } +absl::StatusOr> +OpenTelemetryPluginBuilder::Build() { + return impl_->Build(); +} + } // namespace grpc diff --git a/src/cpp/ext/otel/otel_plugin.h b/src/cpp/ext/otel/otel_plugin.h index 6a8c84fe43c..70321fb021d 100644 --- a/src/cpp/ext/otel/otel_plugin.h +++ b/src/cpp/ext/otel/otel_plugin.h @@ -40,6 +40,7 @@ #include #include +#include #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/transport/metadata_batch.h" @@ -176,6 +177,8 @@ class OpenTelemetryPluginBuilderImpl { bool(const OpenTelemetryPluginBuilder::ChannelScope& /*scope*/) const> channel_scope_filter); absl::Status BuildAndRegisterGlobal(); + absl::StatusOr> + Build(); const absl::flat_hash_set& TestOnlyEnabledMetrics() { return metrics_; @@ -199,9 +202,12 @@ class OpenTelemetryPluginBuilderImpl { channel_scope_filter_; }; -class OpenTelemetryPlugin : public grpc_core::StatsPlugin { +class OpenTelemetryPluginImpl + : public grpc::experimental::OpenTelemetryPlugin, + public grpc_core::StatsPlugin, + public std::enable_shared_from_this { public: - OpenTelemetryPlugin( + OpenTelemetryPluginImpl( const absl::flat_hash_set& metrics, opentelemetry::nostd::shared_ptr meter_provider, @@ -229,7 +235,7 @@ class OpenTelemetryPlugin : public grpc_core::StatsPlugin { class ActivePluginOptionsView { public: static ActivePluginOptionsView MakeForClient( - absl::string_view target, const OpenTelemetryPlugin* otel_plugin) { + absl::string_view target, const OpenTelemetryPluginImpl* otel_plugin) { return ActivePluginOptionsView( [target](const InternalOpenTelemetryPluginOption& plugin_option) { return plugin_option.IsActiveOnClientChannel(target); @@ -239,7 +245,7 @@ class OpenTelemetryPlugin : public grpc_core::StatsPlugin { static ActivePluginOptionsView MakeForServer( const grpc_core::ChannelArgs& args, - const OpenTelemetryPlugin* otel_plugin) { + const OpenTelemetryPluginImpl* otel_plugin) { return ActivePluginOptionsView( [&args](const InternalOpenTelemetryPluginOption& plugin_option) { return plugin_option.IsActiveOnServer(args); @@ -250,7 +256,7 @@ class OpenTelemetryPlugin : public grpc_core::StatsPlugin { bool ForEach(absl::FunctionRef< bool(const InternalOpenTelemetryPluginOption&, size_t)> func, - const OpenTelemetryPlugin* otel_plugin) const { + const OpenTelemetryPluginImpl* otel_plugin) const { for (size_t i = 0; i < otel_plugin->plugin_options().size(); ++i) { const auto& plugin_option = otel_plugin->plugin_options()[i]; if (active_mask_[i] && !func(*plugin_option, i)) { @@ -263,7 +269,7 @@ class OpenTelemetryPlugin : public grpc_core::StatsPlugin { private: explicit ActivePluginOptionsView( absl::FunctionRef func, - const OpenTelemetryPlugin* otel_plugin) { + const OpenTelemetryPluginImpl* otel_plugin) { for (size_t i = 0; i < otel_plugin->plugin_options().size(); ++i) { const auto& plugin_option = otel_plugin->plugin_options()[i]; if (plugin_option != nullptr && func(*plugin_option)) { @@ -277,7 +283,7 @@ class OpenTelemetryPlugin : public grpc_core::StatsPlugin { class ClientScopeConfig : public grpc_core::StatsPlugin::ScopeConfig { public: - ClientScopeConfig(const OpenTelemetryPlugin* otel_plugin, + ClientScopeConfig(const OpenTelemetryPluginImpl* otel_plugin, const OpenTelemetryPluginBuilder::ChannelScope& scope) : active_plugin_options_view_(ActivePluginOptionsView::MakeForClient( scope.target(), otel_plugin)), @@ -302,7 +308,7 @@ class OpenTelemetryPlugin : public grpc_core::StatsPlugin { }; class ServerScopeConfig : public grpc_core::StatsPlugin::ScopeConfig { public: - ServerScopeConfig(const OpenTelemetryPlugin* otel_plugin, + ServerScopeConfig(const OpenTelemetryPluginImpl* otel_plugin, const grpc_core::ChannelArgs& args) : active_plugin_options_view_( ActivePluginOptionsView::MakeForServer(args, otel_plugin)) {} @@ -339,7 +345,7 @@ class OpenTelemetryPlugin : public grpc_core::StatsPlugin { // This object should be used inline. class CallbackMetricReporter : public grpc_core::CallbackMetricReporter { public: - CallbackMetricReporter(OpenTelemetryPlugin* ot_plugin, + CallbackMetricReporter(OpenTelemetryPluginImpl* ot_plugin, grpc_core::RegisteredMetricCallback* key) ABSL_EXCLUSIVE_LOCKS_REQUIRED(ot_plugin->mu_); @@ -357,10 +363,23 @@ class OpenTelemetryPlugin : public grpc_core::StatsPlugin { ABSL_EXCLUSIVE_LOCKS_REQUIRED( CallbackGaugeState::ot_plugin->mu_) override; - OpenTelemetryPlugin* ot_plugin_; + OpenTelemetryPluginImpl* ot_plugin_; grpc_core::RegisteredMetricCallback* key_; }; + class ServerBuilderOption : public grpc::ServerBuilderOption { + public: + explicit ServerBuilderOption( + std::shared_ptr plugin) + : plugin_(std::move(plugin)) {} + void UpdateArguments(grpc::ChannelArguments* args) override; + void UpdatePlugins(std::vector>* + /*plugins*/) override {} + + private: + std::shared_ptr plugin_; + }; + // Returns the string form of \a key static absl::string_view OptionalLabelKeyToString( grpc_core::ClientCallTracer::CallAttemptTracer::OptionalLabelKey key); @@ -371,12 +390,20 @@ class OpenTelemetryPlugin : public grpc_core::StatsPlugin { grpc_core::ClientCallTracer::CallAttemptTracer::OptionalLabelKey> OptionalLabelStringToKey(absl::string_view key); + // grpc::OpenTelemetryPlugin: + void AddToChannelArguments(grpc::ChannelArguments* args) override; + void AddToServerBuilder(grpc::ServerBuilder* builder) override; + // StatsPlugin: std::pair> IsEnabledForChannel( const OpenTelemetryPluginBuilder::ChannelScope& scope) const override; std::pair> IsEnabledForServer(const grpc_core::ChannelArgs& args) const override; + std::shared_ptr GetChannelScopeConfig( + const OpenTelemetryPluginBuilder::ChannelScope& scope) const override; + std::shared_ptr GetServerScopeConfig( + const grpc_core::ChannelArgs& args) const override; void AddCounter( grpc_core::GlobalInstrumentsRegistry::GlobalInstrumentHandle handle, uint64_t value, absl::Span label_values, @@ -442,7 +469,7 @@ class OpenTelemetryPlugin : public grpc_core::StatsPlugin { // One instrument can be registered by multiple callbacks. absl::flat_hash_map caches ABSL_GUARDED_BY(ot_plugin->mu_); - OpenTelemetryPlugin* ot_plugin; + OpenTelemetryPluginImpl* ot_plugin; static void CallbackGaugeCallback( opentelemetry::metrics::ObserverResult result, void* arg) diff --git a/src/cpp/ext/otel/otel_server_call_tracer.cc b/src/cpp/ext/otel/otel_server_call_tracer.cc index d62af3d9081..28c7c6d7a7b 100644 --- a/src/cpp/ext/otel/otel_server_call_tracer.cc +++ b/src/cpp/ext/otel/otel_server_call_tracer.cc @@ -49,7 +49,7 @@ namespace grpc { namespace internal { -void OpenTelemetryPlugin::ServerCallTracer::RecordReceivedInitialMetadata( +void OpenTelemetryPluginImpl::ServerCallTracer::RecordReceivedInitialMetadata( grpc_metadata_batch* recv_initial_metadata) { path_ = recv_initial_metadata->get_pointer(grpc_core::HttpPathMetadata())->Ref(); @@ -80,7 +80,7 @@ void OpenTelemetryPlugin::ServerCallTracer::RecordReceivedInitialMetadata( } } -void OpenTelemetryPlugin::ServerCallTracer::RecordSendInitialMetadata( +void OpenTelemetryPluginImpl::ServerCallTracer::RecordSendInitialMetadata( grpc_metadata_batch* send_initial_metadata) { scope_config_->active_plugin_options_view().ForEach( [&](const InternalOpenTelemetryPluginOption& plugin_option, @@ -96,14 +96,14 @@ void OpenTelemetryPlugin::ServerCallTracer::RecordSendInitialMetadata( otel_plugin_); } -void OpenTelemetryPlugin::ServerCallTracer::RecordSendTrailingMetadata( +void OpenTelemetryPluginImpl::ServerCallTracer::RecordSendTrailingMetadata( grpc_metadata_batch* /*send_trailing_metadata*/) { // We need to record the time when the trailing metadata was sent to // mark the completeness of the request. elapsed_time_ = absl::Now() - start_time_; } -void OpenTelemetryPlugin::ServerCallTracer::RecordEnd( +void OpenTelemetryPluginImpl::ServerCallTracer::RecordEnd( const grpc_call_final_info* final_info) { std::array, 2> additional_labels = { diff --git a/src/cpp/ext/otel/otel_server_call_tracer.h b/src/cpp/ext/otel/otel_server_call_tracer.h index 66c460d6b65..6d7a7ada915 100644 --- a/src/cpp/ext/otel/otel_server_call_tracer.h +++ b/src/cpp/ext/otel/otel_server_call_tracer.h @@ -30,14 +30,14 @@ namespace grpc { namespace internal { -// OpenTelemetryPlugin::ServerCallTracer implementation +// OpenTelemetryPluginImpl::ServerCallTracer implementation -class OpenTelemetryPlugin::ServerCallTracer +class OpenTelemetryPluginImpl::ServerCallTracer : public grpc_core::ServerCallTracer { public: ServerCallTracer( - OpenTelemetryPlugin* otel_plugin, - std::shared_ptr scope_config) + OpenTelemetryPluginImpl* otel_plugin, + std::shared_ptr scope_config) : start_time_(absl::Now()), injected_labels_from_plugin_options_( otel_plugin->plugin_options().size()), @@ -129,8 +129,8 @@ class OpenTelemetryPlugin::ServerCallTracer bool registered_method_; std::vector> injected_labels_from_plugin_options_; - OpenTelemetryPlugin* otel_plugin_; - std::shared_ptr scope_config_; + OpenTelemetryPluginImpl* otel_plugin_; + std::shared_ptr scope_config_; }; } // namespace internal diff --git a/test/core/test_util/fake_stats_plugin.h b/test/core/test_util/fake_stats_plugin.h index e9dcaa240a8..6380e5c55d7 100644 --- a/test/core/test_util/fake_stats_plugin.h +++ b/test/core/test_util/fake_stats_plugin.h @@ -270,6 +270,14 @@ class FakeStatsPlugin : public StatsPlugin { const ChannelArgs& /*args*/) const override { return {true, nullptr}; } + std::shared_ptr GetChannelScopeConfig( + const experimental::StatsPluginChannelScope& /*scope*/) const override { + return nullptr; + } + std::shared_ptr GetServerScopeConfig( + const ChannelArgs& /*args*/) const override { + return nullptr; + } void AddCounter( GlobalInstrumentsRegistry::GlobalInstrumentHandle handle, uint64_t value, diff --git a/test/cpp/ext/csm/csm_observability_test.cc b/test/cpp/ext/csm/csm_observability_test.cc index c10aa96fa69..380c8940b7a 100644 --- a/test/cpp/ext/csm/csm_observability_test.cc +++ b/test/cpp/ext/csm/csm_observability_test.cc @@ -20,6 +20,7 @@ #include "google/cloud/opentelemetry/resource_detector.h" #include "gtest/gtest.h" +#include "opentelemetry/sdk/metrics/meter_provider.h" #include #include @@ -32,8 +33,13 @@ namespace testing { namespace { TEST(CsmObservabilityBuilderTest, Basic) { - EXPECT_EQ(CsmObservabilityBuilder().BuildAndRegister().status(), - absl::OkStatus()); + EXPECT_EQ( + CsmObservabilityBuilder() + .SetMeterProvider( + std::make_shared()) + .BuildAndRegister() + .status(), + absl::OkStatus()); } TEST(GsmDependencyTest, GoogleCloudOpenTelemetryDependency) { @@ -68,7 +74,13 @@ TEST(CsmChannelTargetSelectorTest, XdsTargetsWithTDAuthority) { } TEST(CsmChannelTargetSelectorTest, CsmObservabilityOutOfScope) { - { auto obs = CsmObservabilityBuilder().BuildAndRegister(); } + { + auto obs = + CsmObservabilityBuilder() + .SetMeterProvider( + std::make_shared()) + .BuildAndRegister(); + } // When CsmObservability goes out of scope, the target selector should return // false as well. EXPECT_FALSE(internal::CsmChannelTargetSelector("foo.bar.google.com")); @@ -83,7 +95,13 @@ TEST(CsmServerSelectorTest, ChannelArgs) { } TEST(CsmServerSelectorTest, CsmObservabilityOutOfScope) { - { auto obs = CsmObservabilityBuilder().BuildAndRegister(); } + { + auto obs = + CsmObservabilityBuilder() + .SetMeterProvider( + std::make_shared()) + .BuildAndRegister(); + } // When CsmObservability goes out of scope, the server selector should return // false as well. EXPECT_FALSE(internal::CsmServerSelector(grpc_core::ChannelArgs())); diff --git a/test/cpp/ext/otel/otel_plugin_test.cc b/test/cpp/ext/otel/otel_plugin_test.cc index 258f44fb315..ad7bc11d565 100644 --- a/test/cpp/ext/otel/otel_plugin_test.cc +++ b/test/cpp/ext/otel/otel_plugin_test.cc @@ -30,6 +30,7 @@ #include "opentelemetry/metrics/provider.h" #include "opentelemetry/nostd/variant.h" #include "opentelemetry/sdk/common/attribute_utils.h" +#include "opentelemetry/sdk/metrics/data/metric_data.h" #include "opentelemetry/sdk/metrics/data/point_data.h" #include "opentelemetry/sdk/metrics/meter_provider.h" #include "opentelemetry/sdk/metrics/metric_reader.h" @@ -1524,7 +1525,6 @@ TEST_F(OpenTelemetryPluginNPCMetricsTest, .Build(); // Build and register a separate OpenTelemetryPlugin and verify its histogram // recording. - grpc::internal::OpenTelemetryPluginBuilderImpl ot_builder; auto reader = BuildAndRegisterOpenTelemetryPlugin(std::move( Options() .set_metric_names({kMetricName}) @@ -1534,7 +1534,6 @@ TEST_F(OpenTelemetryPluginNPCMetricsTest, }) .add_optional_label(kOptionalLabelKeys[0]) .add_optional_label(kOptionalLabelKeys[1]))); - EXPECT_EQ(ot_builder.BuildAndRegisterGlobal(), absl::OkStatus()); grpc_core::ChannelArgs args; args = args.Set(GRPC_ARG_SERVER_SELECTOR_KEY, GRPC_ARG_SERVER_SELECTOR_VALUE); { @@ -2002,6 +2001,258 @@ TEST(OpenTelemetryPluginMetricsEnablingDisablingTest, TestEnableDisableAPIs) { ::testing::UnorderedElementsAre("grpc.test.metric_3")); } +TEST_F(OpenTelemetryPluginEnd2EndTest, RegisterMultipleStatsPluginsPerChannel) { + std::shared_ptr plugin1; + std::shared_ptr reader1; + std::tie(plugin1, reader1) = BuildOpenTelemetryPlugin(std::move( + Options().set_metric_names({grpc::OpenTelemetryPluginBuilder:: + kClientAttemptDurationInstrumentName}))); + std::shared_ptr plugin2; + std::shared_ptr reader2; + std::tie(plugin2, reader2) = BuildOpenTelemetryPlugin(std::move( + Options().set_metric_names({grpc::OpenTelemetryPluginBuilder:: + kClientAttemptDurationInstrumentName}))); + Init(std::move( + Options() + .set_metric_names({grpc::OpenTelemetryPluginBuilder:: + kClientAttemptDurationInstrumentName}) + .add_per_channel_stats_plugin(std::move(plugin1)) + .add_per_channel_stats_plugin(std::move(plugin2)))); + const std::vector kLabelKeys = { + "grpc.method", "grpc.target", "grpc.status"}; + const std::vector kLabelValues = { + kMethodName, canonical_server_address_, "OK"}; + const std::vector kOptionalLabelKeys = {}; + const std::vector kOptionalLabelValues = {}; + SendRPC(); + const char* kMetricName = "grpc.client.attempt.duration"; + auto data = ReadCurrentMetricsData( + [&](const absl::flat_hash_map< + std::string, + std::vector>& + data) { return !data.contains(kMetricName); }); + EXPECT_THAT( + data, + ::testing::ElementsAre(::testing::Pair( + kMetricName, + ::testing::ElementsAre(::testing::AllOf( + AttributesEq(kLabelKeys, kLabelValues, kOptionalLabelKeys, + kOptionalLabelValues), + ::testing::Field( + &opentelemetry::sdk::metrics::PointDataAttributes::point_data, + ::testing::VariantWith< + opentelemetry::sdk::metrics::HistogramPointData>( + ::testing::Field(&opentelemetry::sdk::metrics:: + HistogramPointData::count_, + ::testing::Eq(1))))))))); + data = ReadCurrentMetricsData( + [&](const absl::flat_hash_map< + std::string, + std::vector>& + data) { return !data.contains(kMetricName); }, + reader1.get()); + EXPECT_THAT( + data, + ::testing::ElementsAre(::testing::Pair( + kMetricName, + ::testing::ElementsAre(::testing::AllOf( + AttributesEq(kLabelKeys, kLabelValues, kOptionalLabelKeys, + kOptionalLabelValues), + ::testing::Field( + &opentelemetry::sdk::metrics::PointDataAttributes::point_data, + ::testing::VariantWith< + opentelemetry::sdk::metrics::HistogramPointData>( + ::testing::Field(&opentelemetry::sdk::metrics:: + HistogramPointData::count_, + ::testing::Eq(1))))))))); + data = ReadCurrentMetricsData( + [&](const absl::flat_hash_map< + std::string, + std::vector>& + data) { return !data.contains(kMetricName); }, + reader2.get()); + EXPECT_THAT( + data, + ::testing::ElementsAre(::testing::Pair( + kMetricName, + ::testing::ElementsAre(::testing::AllOf( + AttributesEq(kLabelKeys, kLabelValues, kOptionalLabelKeys, + kOptionalLabelValues), + ::testing::Field( + &opentelemetry::sdk::metrics::PointDataAttributes::point_data, + ::testing::VariantWith< + opentelemetry::sdk::metrics::HistogramPointData>( + ::testing::Field(&opentelemetry::sdk::metrics:: + HistogramPointData::count_, + ::testing::Eq(1))))))))); +} + +TEST_F(OpenTelemetryPluginEnd2EndTest, + RegisterSameStatsPluginForMultipleChannels) { + // channel1 channel2 + // | | + // | (global plugin, plugin1) | (global plugin, plugin1, plugin2) + // | | + std::shared_ptr plugin1; + std::shared_ptr reader1; + std::tie(plugin1, reader1) = BuildOpenTelemetryPlugin(std::move( + Options().set_metric_names({grpc::OpenTelemetryPluginBuilder:: + kClientAttemptDurationInstrumentName}))); + std::shared_ptr plugin2; + std::shared_ptr reader2; + std::tie(plugin2, reader2) = BuildOpenTelemetryPlugin(std::move( + Options().set_metric_names({grpc::OpenTelemetryPluginBuilder:: + kClientAttemptDurationInstrumentName}))); + Init(std::move( + Options() + .set_metric_names({grpc::OpenTelemetryPluginBuilder:: + kClientAttemptDurationInstrumentName}) + .add_per_channel_stats_plugin(plugin1))); + // Adds the same plugin to another channel. + ChannelArguments channel_args; + plugin1->AddToChannelArguments(&channel_args); + plugin2->AddToChannelArguments(&channel_args); + auto channel2 = grpc::CreateCustomChannel( + server_address_, grpc::InsecureChannelCredentials(), channel_args); + std::unique_ptr stub = + EchoTestService::NewStub(std::move(channel2)); + // Sends 2 RPCs, one from each channel. + EchoRequest request; + request.set_message("foo"); + EchoResponse response; + grpc::ClientContext context; + stub->Echo(&context, request, &response); + SendRPC(); + const std::vector kLabelKeys = { + "grpc.method", "grpc.target", "grpc.status"}; + const std::vector kLabelValues = { + kMethodName, canonical_server_address_, "OK"}; + const std::vector kOptionalLabelKeys = {}; + const std::vector kOptionalLabelValues = {}; + const char* kMetricName = "grpc.client.attempt.duration"; + // Verifies that we got 2 histogram points in global plugin. + auto data = ReadCurrentMetricsData( + [&](const absl::flat_hash_map< + std::string, + std::vector>& + data) { return !data.contains(kMetricName); }); + EXPECT_THAT( + data, + ::testing::ElementsAre(::testing::Pair( + kMetricName, + ::testing::ElementsAre(::testing::AllOf( + AttributesEq(kLabelKeys, kLabelValues, kOptionalLabelKeys, + kOptionalLabelValues), + ::testing::Field( + &opentelemetry::sdk::metrics::PointDataAttributes::point_data, + ::testing::VariantWith< + opentelemetry::sdk::metrics::HistogramPointData>( + ::testing::Field(&opentelemetry::sdk::metrics:: + HistogramPointData::count_, + ::testing::Eq(2))))))))); + // Verifies that we got 2 histogram points in plugin1. + data = ReadCurrentMetricsData( + [&](const absl::flat_hash_map< + std::string, + std::vector>& + data) { return !data.contains(kMetricName); }, + reader1.get()); + EXPECT_THAT( + data, + ::testing::ElementsAre(::testing::Pair( + kMetricName, + ::testing::ElementsAre(::testing::AllOf( + AttributesEq(kLabelKeys, kLabelValues, kOptionalLabelKeys, + kOptionalLabelValues), + ::testing::Field( + &opentelemetry::sdk::metrics::PointDataAttributes::point_data, + ::testing::VariantWith< + opentelemetry::sdk::metrics::HistogramPointData>( + ::testing::Field(&opentelemetry::sdk::metrics:: + HistogramPointData::count_, + ::testing::Eq(2))))))))); + // Verifies that we got 1 histogram point in plugin2. + data = ReadCurrentMetricsData( + [&](const absl::flat_hash_map< + std::string, + std::vector>& + data) { return !data.contains(kMetricName); }, + reader2.get()); + EXPECT_THAT( + data, + ::testing::ElementsAre(::testing::Pair( + kMetricName, + ::testing::ElementsAre(::testing::AllOf( + AttributesEq(kLabelKeys, kLabelValues, kOptionalLabelKeys, + kOptionalLabelValues), + ::testing::Field( + &opentelemetry::sdk::metrics::PointDataAttributes::point_data, + ::testing::VariantWith< + opentelemetry::sdk::metrics::HistogramPointData>( + ::testing::Field(&opentelemetry::sdk::metrics:: + HistogramPointData::count_, + ::testing::Eq(1))))))))); +} + +TEST_F(OpenTelemetryPluginEnd2EndTest, RegisterMultipleStatsPluginsPerServer) { + std::shared_ptr plugin; + std::shared_ptr reader; + std::tie(plugin, reader) = BuildOpenTelemetryPlugin(std::move( + Options().set_metric_names({grpc::OpenTelemetryPluginBuilder:: + kServerCallDurationInstrumentName}))); + Init(std::move(Options() + .set_metric_names({grpc::OpenTelemetryPluginBuilder:: + kServerCallDurationInstrumentName}) + .add_per_server_stats_plugin(std::move(plugin)))); + const std::vector kLabelKeys = {"grpc.method", + "grpc.status"}; + const std::vector kLabelValues = {kMethodName, "OK"}; + const std::vector kOptionalLabelKeys = {}; + const std::vector kOptionalLabelValues = {}; + SendRPC(); + const char* kMetricName = "grpc.server.call.duration"; + // Verifies that both plugins have the server-side metrics recorded. + auto data = ReadCurrentMetricsData( + [&](const absl::flat_hash_map< + std::string, + std::vector>& + data) { return !data.contains(kMetricName); }); + EXPECT_THAT( + data, + ::testing::ElementsAre(::testing::Pair( + kMetricName, + ::testing::ElementsAre(::testing::AllOf( + AttributesEq(kLabelKeys, kLabelValues, kOptionalLabelKeys, + kOptionalLabelValues), + ::testing::Field( + &opentelemetry::sdk::metrics::PointDataAttributes::point_data, + ::testing::VariantWith< + opentelemetry::sdk::metrics::HistogramPointData>( + ::testing::Field(&opentelemetry::sdk::metrics:: + HistogramPointData::count_, + ::testing::Eq(1))))))))); + data = ReadCurrentMetricsData( + [&](const absl::flat_hash_map< + std::string, + std::vector>& + data) { return !data.contains(kMetricName); }, + reader.get()); + EXPECT_THAT( + data, + ::testing::ElementsAre(::testing::Pair( + kMetricName, + ::testing::ElementsAre(::testing::AllOf( + AttributesEq(kLabelKeys, kLabelValues, kOptionalLabelKeys, + kOptionalLabelValues), + ::testing::Field( + &opentelemetry::sdk::metrics::PointDataAttributes::point_data, + ::testing::VariantWith< + opentelemetry::sdk::metrics::HistogramPointData>( + ::testing::Field(&opentelemetry::sdk::metrics:: + HistogramPointData::count_, + ::testing::Eq(1))))))))); +} + } // namespace } // namespace testing } // namespace grpc diff --git a/test/cpp/ext/otel/otel_test_library.cc b/test/cpp/ext/otel/otel_test_library.cc index 338527903b4..043d8c2dbc9 100644 --- a/test/cpp/ext/otel/otel_test_library.cc +++ b/test/cpp/ext/otel/otel_test_library.cc @@ -149,7 +149,6 @@ void OpenTelemetryPluginEnd2EndTest::Init(Options config) { if (!config.service_config.empty()) { channel_args.SetString(GRPC_ARG_SERVICE_CONFIG, config.service_config); } - reader_ = BuildAndRegisterOpenTelemetryPlugin(std::move(config)); grpc_init(); grpc::ServerBuilder builder; int port; @@ -157,11 +156,18 @@ void OpenTelemetryPluginEnd2EndTest::Init(Options config) { builder.AddListeningPort("0.0.0.0:0", grpc::InsecureServerCredentials(), &port); builder.RegisterService(&service_); + for (auto& per_server_stats_plugin : config.per_server_stats_plugins) { + per_server_stats_plugin->AddToServerBuilder(&builder); + } server_ = builder.BuildAndStart(); ASSERT_NE(nullptr, server_); ASSERT_NE(0, port); server_address_ = absl::StrCat("localhost:", port); canonical_server_address_ = absl::StrCat("dns:///", server_address_); + for (auto& per_channel_stats_plugin : config.per_channel_stats_plugins) { + per_channel_stats_plugin->AddToChannelArguments(&channel_args); + } + reader_ = BuildAndRegisterOpenTelemetryPlugin(std::move(config)); auto channel = grpc::CreateCustomChannel( server_address_, grpc::InsecureChannelCredentials(), channel_args); @@ -238,10 +244,35 @@ OpenTelemetryPluginEnd2EndTest::ReadCurrentMetricsData( return data; } +std::pair, + std::shared_ptr> +OpenTelemetryPluginEnd2EndTest::BuildOpenTelemetryPlugin( + OpenTelemetryPluginEnd2EndTest::Options options) { + grpc::internal::OpenTelemetryPluginBuilderImpl ot_builder; + auto reader = ConfigureOTBuilder(std::move(options), &ot_builder); + auto plugin = ot_builder.Build(); + EXPECT_TRUE(plugin.ok()); + return {*plugin, reader}; +} + std::shared_ptr OpenTelemetryPluginEnd2EndTest::BuildAndRegisterOpenTelemetryPlugin( OpenTelemetryPluginEnd2EndTest::Options options) { grpc::internal::OpenTelemetryPluginBuilderImpl ot_builder; + absl::Status expected_status; + if (!options.use_meter_provider) { + expected_status = + absl::InvalidArgumentError("Need to configure a valid meter provider."); + } + auto reader = ConfigureOTBuilder(std::move(options), &ot_builder); + EXPECT_EQ(ot_builder.BuildAndRegisterGlobal(), expected_status); + return reader; +} + +std::shared_ptr +OpenTelemetryPluginEnd2EndTest::ConfigureOTBuilder( + OpenTelemetryPluginEnd2EndTest::Options options, + grpc::internal::OpenTelemetryPluginBuilderImpl* ot_builder) { // We are resetting the MeterProvider and OpenTelemetry plugin at the start // of each test to avoid test results from one test carrying over to another // test. (Some measurements can get arbitrarily delayed.) @@ -252,28 +283,27 @@ OpenTelemetryPluginEnd2EndTest::BuildAndRegisterOpenTelemetryPlugin( std::shared_ptr reader = std::make_shared(); meter_provider->AddMetricReader(reader); - ot_builder.DisableAllMetrics(); - ot_builder.EnableMetrics(options.metric_names); + ot_builder->DisableAllMetrics(); + ot_builder->EnableMetrics(options.metric_names); if (options.use_meter_provider) { auto meter_provider = std::make_shared(); reader.reset(new grpc::testing::MockMetricReader); meter_provider->AddMetricReader(reader); - ot_builder.SetMeterProvider(std::move(meter_provider)); + ot_builder->SetMeterProvider(std::move(meter_provider)); } - ot_builder.SetChannelScopeFilter(std::move(options.channel_scope_filter)); - ot_builder.SetServerSelector(std::move(options.server_selector)); - ot_builder.SetTargetAttributeFilter( + ot_builder->SetChannelScopeFilter(std::move(options.channel_scope_filter)); + ot_builder->SetServerSelector(std::move(options.server_selector)); + ot_builder->SetTargetAttributeFilter( std::move(options.target_attribute_filter)); - ot_builder.SetGenericMethodAttributeFilter( + ot_builder->SetGenericMethodAttributeFilter( std::move(options.generic_method_attribute_filter)); for (auto& option : options.plugin_options) { - ot_builder.AddPluginOption(std::move(option)); + ot_builder->AddPluginOption(std::move(option)); } for (auto& optional_label_key : options.optional_label_keys) { - ot_builder.AddOptionalLabel(optional_label_key); + ot_builder->AddOptionalLabel(optional_label_key); } - EXPECT_EQ(ot_builder.BuildAndRegisterGlobal(), absl::OkStatus()); return reader; } diff --git a/test/cpp/ext/otel/otel_test_library.h b/test/cpp/ext/otel/otel_test_library.h index f8573ccaa90..38859f68710 100644 --- a/test/cpp/ext/otel/otel_test_library.h +++ b/test/cpp/ext/otel/otel_test_library.h @@ -132,6 +132,18 @@ class OpenTelemetryPluginEnd2EndTest : public ::testing::Test { return *this; } + Options& add_per_channel_stats_plugin( + std::shared_ptr plugin) { + per_channel_stats_plugins.emplace_back(std::move(plugin)); + return *this; + } + + Options& add_per_server_stats_plugin( + std::shared_ptr plugin) { + per_server_stats_plugins.emplace_back(std::move(plugin)); + return *this; + } + std::vector metric_names; // TODO(yashykt): opentelemetry::sdk::resource::Resource doesn't have a copy // assignment operator so wrapping it in a unique_ptr till it is fixed. @@ -158,6 +170,10 @@ class OpenTelemetryPluginEnd2EndTest : public ::testing::Test { std::unique_ptr> plugin_options; absl::flat_hash_set optional_label_keys; + std::vector> + per_channel_stats_plugins; + std::vector> + per_server_stats_plugins; }; class MetricsCollectorThread { @@ -183,6 +199,11 @@ class OpenTelemetryPluginEnd2EndTest : public ::testing::Test { std::thread thread_; }; + static std::shared_ptr + ConfigureOTBuilder( + OpenTelemetryPluginEnd2EndTest::Options options, + grpc::internal::OpenTelemetryPluginBuilderImpl* ot_builder); + // Note that we can't use SetUp() here since we want to send in parameters. void Init(Options config); @@ -193,6 +214,10 @@ class OpenTelemetryPluginEnd2EndTest : public ::testing::Test { void SendRPC(); void SendGenericRPC(); + std::pair, + std::shared_ptr> + BuildOpenTelemetryPlugin(OpenTelemetryPluginEnd2EndTest::Options options); + std::shared_ptr BuildAndRegisterOpenTelemetryPlugin( OpenTelemetryPluginEnd2EndTest::Options options); From 3d1f460fcce81a2b433483f4ce60c32147b34c83 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 10 Jun 2024 16:03:48 -0700 Subject: [PATCH 5/9] [experiments] Update some expirations - still working on them (#36871) Closes #36871 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36871 from ctiller:xup 6d8e47bdb81cb192a9c529fcc8f232c8281787d6 PiperOrigin-RevId: 642052522 --- src/core/lib/experiments/experiments.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/lib/experiments/experiments.yaml b/src/core/lib/experiments/experiments.yaml index 638e9a9571c..c0ca8edc969 100644 --- a/src/core/lib/experiments/experiments.yaml +++ b/src/core/lib/experiments/experiments.yaml @@ -124,7 +124,7 @@ - name: multiping description: Allow more than one ping to be in flight at a time by default. - expiry: 2024/06/15 + expiry: 2024/09/15 owner: ctiller@google.com test_tags: [flow_control_test] - name: peer_state_based_framing @@ -143,7 +143,7 @@ - name: promise_based_inproc_transport description: Use promises for the in-process transport. - expiry: 2024/06/06 + expiry: 2024/09/15 owner: ctiller@google.com test_tags: [] allow_in_fuzzing_config: false # experiment currently crashes if enabled From c605b9846788faee33671a16e1d9afad77ec3f9d Mon Sep 17 00:00:00 2001 From: Nana Pang Date: Mon, 10 Jun 2024 16:23:04 -0700 Subject: [PATCH 6/9] [Telemetry] Add function `FindInstrumentByName` in telemetry/metrics.h to allow users query instruments list. PiperOrigin-RevId: 642057981 --- src/core/BUILD | 1 + src/core/telemetry/metrics.cc | 14 ++++++++++++++ src/core/telemetry/metrics.h | 2 ++ test/core/telemetry/metrics_test.cc | 15 +++++++++++++++ 4 files changed, 32 insertions(+) diff --git a/src/core/BUILD b/src/core/BUILD index 3f29d96f28d..757aaad95b0 100644 --- a/src/core/BUILD +++ b/src/core/BUILD @@ -7825,6 +7825,7 @@ grpc_cc_library( "absl/functional:function_ref", "absl/log:check", "absl/strings", + "absl/types:optional", "absl/types:span", ], language = "c++", diff --git a/src/core/telemetry/metrics.cc b/src/core/telemetry/metrics.cc index db47f1f8608..adf5393942f 100644 --- a/src/core/telemetry/metrics.cc +++ b/src/core/telemetry/metrics.cc @@ -17,6 +17,7 @@ #include #include "absl/log/check.h" +#include "absl/types/optional.h" #include @@ -161,4 +162,17 @@ GlobalStatsPluginRegistry::GetStatsPluginsForServer(const ChannelArgs& args) { return group; } +absl::optional +GlobalInstrumentsRegistry::FindInstrumentByName(absl::string_view name) { + const auto& instruments = GetInstrumentList(); + for (const auto& descriptor : instruments) { + if (descriptor.name == name) { + GlobalInstrumentsRegistry::GlobalInstrumentHandle handle; + handle.index = descriptor.index; + return handle; + } + } + return absl::nullopt; +} + } // namespace grpc_core diff --git a/src/core/telemetry/metrics.h b/src/core/telemetry/metrics.h index eec7d075dce..40db0350bf2 100644 --- a/src/core/telemetry/metrics.h +++ b/src/core/telemetry/metrics.h @@ -211,6 +211,8 @@ class GlobalInstrumentsRegistry { absl::FunctionRef f); static const GlobalInstrumentDescriptor& GetInstrumentDescriptor( GlobalInstrumentHandle handle); + static absl::optional + FindInstrumentByName(absl::string_view name); private: friend class GlobalInstrumentsRegistryTestPeer; diff --git a/test/core/telemetry/metrics_test.cc b/test/core/telemetry/metrics_test.cc index 51b98831018..a57a0118ae7 100644 --- a/test/core/telemetry/metrics_test.cc +++ b/test/core/telemetry/metrics_test.cc @@ -629,6 +629,21 @@ TEST_F(MetricsTest, DisableByDefaultMetricIsNotRecordedByFakeStatsPlugin) { absl::nullopt); } +TEST_F(MetricsTest, FindInstrumentByName) { + auto uint64_counter_handle = + GlobalInstrumentsRegistry::RegisterUInt64Counter( + "uint64_counter", "A simple uint64 counter.", "unit", true) + .Labels("label_key_1", "label_key_2") + .OptionalLabels("optional_label_key_1", "optional_label_key_2") + .Build(); + auto instrument = + GlobalInstrumentsRegistry::FindInstrumentByName("uint64_counter"); + EXPECT_THAT(instrument, + ::testing::Optional(::testing::Field( + &GlobalInstrumentsRegistry::GlobalInstrumentHandle::index, + ::testing::Eq(uint64_counter_handle.index)))); +} + using MetricsDeathTest = MetricsTest; TEST_F(MetricsDeathTest, RegisterTheSameMetricNameWouldCrash) { From 7ce0ccdb403b7d96b89b9fd786a853cc56501aba Mon Sep 17 00:00:00 2001 From: Yousuk Seung Date: Mon, 10 Jun 2024 16:50:07 -0700 Subject: [PATCH 7/9] child post-fork gRPC shutdown to wait briefly (#36873) gRPC shutdown may happen asynchronously with `work_serializer_dispatch`, so wait briefly until shutdown completes in the child's post-fork. Closes #36873 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36873 from yousukseung:python-fork-shutdown-wait 7908a6912a6eeb91cd423e770f07ffe4b34accd9 PiperOrigin-RevId: 642066286 --- .../grpc/_cython/_cygrpc/fork_posix.pyx.pxi | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi index d901cfddf43..05889384945 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi @@ -79,11 +79,15 @@ cdef void __postfork_child() noexcept nogil: _LOGGER.error('Exiting child due to raised exception') _LOGGER.error(sys.exc_info()[0]) os._exit(os.EX_USAGE) - - if grpc_is_initialized() > 0: - with gil: - _LOGGER.error('Failed to shutdown gRPC Core after fork()') - os._exit(os.EX_USAGE) + # Give ~2s to shutdown asynchronously. + wait_ms = 10 + while wait_ms < 1500: + if grpc_is_initialized() == 0: + return + time.sleep(wait_ms / 1000) + wait_ms = wait_ms * 2 + _LOGGER.error('Failed to shutdown gRPC Core after fork()') + os._exit(os.EX_USAGE) def fork_handlers_and_grpc_init(): @@ -153,7 +157,7 @@ def get_fork_epoch(): def is_fork_support_enabled(): return _GRPC_ENABLE_FORK_SUPPORT - + def fork_register_channel(channel): if _GRPC_ENABLE_FORK_SUPPORT: _fork_state.channels.add(channel) From 03e91b6811e2f426570be6c94cc563523000fd06 Mon Sep 17 00:00:00 2001 From: Tanvi Jagtap <139093547+tanvi-jagtap@users.noreply.github.com> Date: Mon, 10 Jun 2024 17:49:58 -0700 Subject: [PATCH 8/9] [Gpr_To_Absl_Logging] Move function to test header form log.h (#36860) [Gpr_To_Absl_Logging] Move function to test header form log.h This is not really needed in log.h Closes #36860 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36860 from tanvi-jagtap:move_function_to_test_header e6494bd06f2e4a08c91eb41f420607f9568b22ac PiperOrigin-RevId: 642080756 --- grpc.def | 1 - include/grpc/support/log.h | 2 -- src/core/util/log.cc | 5 ----- src/ruby/ext/grpc/rb_grpc_imports.generated.c | 2 -- src/ruby/ext/grpc/rb_grpc_imports.generated.h | 3 --- test/core/call/yodel/fuzzer_main.cc | 3 ++- test/core/end2end/end2end_test_fuzzer.cc | 3 ++- test/core/end2end/fuzzers/api_fuzzer.cc | 3 ++- test/core/end2end/fuzzers/client_fuzzer.cc | 3 ++- test/core/end2end/fuzzers/server_fuzzer.cc | 3 ++- .../event_engine_client_channel_resolver/resolver_fuzzer.cc | 3 ++- test/core/nanopb/fuzzer_response.cc | 4 +++- test/core/nanopb/fuzzer_serverlist.cc | 4 +++- test/core/resource_quota/BUILD | 1 + test/core/resource_quota/memory_quota_fuzzer.cc | 3 ++- test/core/security/alts_credentials_fuzzer.cc | 3 ++- test/core/security/ssl_server_fuzzer.cc | 3 ++- test/core/test_util/BUILD | 1 + test/core/test_util/test_config.cc | 6 ++++++ test/core/test_util/test_config.h | 3 +++ test/core/transport/binder/end2end/fuzzers/client_fuzzer.cc | 3 ++- test/core/transport/binder/end2end/fuzzers/server_fuzzer.cc | 3 ++- test/core/transport/chttp2/hpack_parser_fuzzer_test.cc | 3 ++- test/core/transport/chttp2/hpack_sync_fuzzer.cc | 3 ++- test/cpp/interop/BUILD | 1 + test/cpp/interop/metrics_client.cc | 3 ++- 26 files changed, 46 insertions(+), 29 deletions(-) diff --git a/grpc.def b/grpc.def index acee3331a74..154e8eee94d 100644 --- a/grpc.def +++ b/grpc.def @@ -236,7 +236,6 @@ EXPORTS gpr_log_message gpr_set_log_verbosity gpr_log_verbosity_init - gpr_disable_all_logs gpr_set_log_function gpr_assertion_failed gpr_format_message diff --git a/include/grpc/support/log.h b/include/grpc/support/log.h index 1d51ffd9b85..57560029624 100644 --- a/include/grpc/support/log.h +++ b/include/grpc/support/log.h @@ -69,8 +69,6 @@ GPRAPI void gpr_set_log_verbosity(gpr_log_severity min_severity_to_print); GPRAPI void gpr_log_verbosity_init(void); -GPRAPI void gpr_disable_all_logs(void); - /** Log overrides: applications can use this API to intercept logging calls and use their own implementations */ diff --git a/src/core/util/log.cc b/src/core/util/log.cc index c89609cbd2f..77326f716ae 100644 --- a/src/core/util/log.cc +++ b/src/core/util/log.cc @@ -197,8 +197,3 @@ void gpr_set_log_function(gpr_log_func f) { "functionality. gRFC: https://github.com/grpc/proposal/pull/425 "; gpr_atm_no_barrier_store(&g_log_func, (gpr_atm)(f ? f : gpr_default_log)); } - -void gpr_disable_all_logs() { - absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfinity); - absl::SetVLogLevel("*grpc*/*", -1); -} diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.c b/src/ruby/ext/grpc/rb_grpc_imports.generated.c index 664a8f01482..e4607ce6d00 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.c +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.c @@ -259,7 +259,6 @@ gpr_should_log_type gpr_should_log_import; gpr_log_message_type gpr_log_message_import; gpr_set_log_verbosity_type gpr_set_log_verbosity_import; gpr_log_verbosity_init_type gpr_log_verbosity_init_import; -gpr_disable_all_logs_type gpr_disable_all_logs_import; gpr_set_log_function_type gpr_set_log_function_import; gpr_assertion_failed_type gpr_assertion_failed_import; gpr_format_message_type gpr_format_message_import; @@ -550,7 +549,6 @@ void grpc_rb_load_imports(HMODULE library) { gpr_log_message_import = (gpr_log_message_type) GetProcAddress(library, "gpr_log_message"); gpr_set_log_verbosity_import = (gpr_set_log_verbosity_type) GetProcAddress(library, "gpr_set_log_verbosity"); gpr_log_verbosity_init_import = (gpr_log_verbosity_init_type) GetProcAddress(library, "gpr_log_verbosity_init"); - gpr_disable_all_logs_import = (gpr_disable_all_logs_type) GetProcAddress(library, "gpr_disable_all_logs"); gpr_set_log_function_import = (gpr_set_log_function_type) GetProcAddress(library, "gpr_set_log_function"); gpr_assertion_failed_import = (gpr_assertion_failed_type) GetProcAddress(library, "gpr_assertion_failed"); gpr_format_message_import = (gpr_format_message_type) GetProcAddress(library, "gpr_format_message"); diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.h b/src/ruby/ext/grpc/rb_grpc_imports.generated.h index 3b3b401a257..42fb67dface 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.h +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.h @@ -753,9 +753,6 @@ extern gpr_set_log_verbosity_type gpr_set_log_verbosity_import; typedef void(*gpr_log_verbosity_init_type)(void); extern gpr_log_verbosity_init_type gpr_log_verbosity_init_import; #define gpr_log_verbosity_init gpr_log_verbosity_init_import -typedef void(*gpr_disable_all_logs_type)(void); -extern gpr_disable_all_logs_type gpr_disable_all_logs_import; -#define gpr_disable_all_logs gpr_disable_all_logs_import typedef void(*gpr_set_log_function_type)(gpr_log_func func); extern gpr_set_log_function_type gpr_set_log_function_import; #define gpr_set_log_function gpr_set_log_function_import diff --git a/test/core/call/yodel/fuzzer_main.cc b/test/core/call/yodel/fuzzer_main.cc index 3cd0340c227..f88cda36333 100644 --- a/test/core/call/yodel/fuzzer_main.cc +++ b/test/core/call/yodel/fuzzer_main.cc @@ -31,6 +31,7 @@ #include "test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.h" #include "test/core/test_util/fuzz_config_vars.h" #include "test/core/test_util/proto_bit_gen.h" +#include "test/core/test_util/test_config.h" bool squelch = true; @@ -43,7 +44,7 @@ DEFINE_PROTO_FUZZER(const transport_test_suite::Msg& msg) { const int test_id = msg.test_id() % tests->size(); if (squelch && !grpc_core::GetEnv("GRPC_TRACE_FUZZER").has_value()) { - gpr_disable_all_logs(); + grpc_disable_all_absl_logs(); } grpc_core::ConfigVars::Overrides overrides = diff --git a/test/core/end2end/end2end_test_fuzzer.cc b/test/core/end2end/end2end_test_fuzzer.cc index be79482d7e2..c0f5cb3bbf9 100644 --- a/test/core/end2end/end2end_test_fuzzer.cc +++ b/test/core/end2end/end2end_test_fuzzer.cc @@ -43,6 +43,7 @@ #include "test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.h" #include "test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.pb.h" #include "test/core/test_util/fuzz_config_vars.h" +#include "test/core/test_util/test_config.h" using ::grpc_event_engine::experimental::FuzzingEventEngine; using ::grpc_event_engine::experimental::GetDefaultEventEngine; @@ -91,7 +92,7 @@ void RunEnd2endFuzzer(const core_end2end_test_fuzzer::Msg& msg) { const int test_id = msg.test_id() % tests.size(); if (squelch && !GetEnv("GRPC_TRACE_FUZZER").has_value()) { - gpr_disable_all_logs(); + grpc_disable_all_absl_logs(); } // TODO(ctiller): make this per fixture? diff --git a/test/core/end2end/fuzzers/api_fuzzer.cc b/test/core/end2end/fuzzers/api_fuzzer.cc index 02f296ec19f..e86b0a25d71 100644 --- a/test/core/end2end/fuzzers/api_fuzzer.cc +++ b/test/core/end2end/fuzzers/api_fuzzer.cc @@ -69,6 +69,7 @@ #include "test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.pb.h" #include "test/core/test_util/fuzz_config_vars.h" #include "test/core/test_util/fuzzing_channel_args.h" +#include "test/core/test_util/test_config.h" // IWYU pragma: no_include @@ -504,7 +505,7 @@ using grpc_core::testing::ApiFuzzer; DEFINE_PROTO_FUZZER(const api_fuzzer::Msg& msg) { if (squelch && !grpc_core::GetEnv("GRPC_TRACE_FUZZER").has_value()) { - gpr_disable_all_logs(); + grpc_disable_all_absl_logs(); } grpc_core::ApplyFuzzConfigVars(msg.config_vars()); grpc_core::TestOnlyReloadExperimentsFromConfigVariables(); diff --git a/test/core/end2end/fuzzers/client_fuzzer.cc b/test/core/end2end/fuzzers/client_fuzzer.cc index eb20355a31a..3e7ca5089ba 100644 --- a/test/core/end2end/fuzzers/client_fuzzer.cc +++ b/test/core/end2end/fuzzers/client_fuzzer.cc @@ -43,6 +43,7 @@ #include "test/core/end2end/fuzzers/network_input.h" #include "test/core/test_util/fuzz_config_vars.h" #include "test/core/test_util/mock_endpoint.h" +#include "test/core/test_util/test_config.h" bool squelch = true; bool leak_check = true; @@ -99,7 +100,7 @@ class ClientFuzzer final : public BasicFuzzer { DEFINE_PROTO_FUZZER(const fuzzer_input::Msg& msg) { if (squelch && !grpc_core::GetEnv("GRPC_TRACE_FUZZER").has_value()) { - gpr_disable_all_logs(); + grpc_disable_all_absl_logs(); } if (msg.network_input().size() != 1) return; grpc_core::ApplyFuzzConfigVars(msg.config_vars()); diff --git a/test/core/end2end/fuzzers/server_fuzzer.cc b/test/core/end2end/fuzzers/server_fuzzer.cc index 3e310fa6cee..a161815f51a 100644 --- a/test/core/end2end/fuzzers/server_fuzzer.cc +++ b/test/core/end2end/fuzzers/server_fuzzer.cc @@ -31,6 +31,7 @@ #include "test/core/end2end/fuzzers/fuzzing_common.h" #include "test/core/end2end/fuzzers/network_input.h" #include "test/core/test_util/fuzz_config_vars.h" +#include "test/core/test_util/test_config.h" bool squelch = true; bool leak_check = true; @@ -95,7 +96,7 @@ void RunServerFuzzer( absl::FunctionRef server_setup) { if (squelch && !GetEnv("GRPC_TRACE_FUZZER").has_value()) { - gpr_disable_all_logs(); + grpc_disable_all_absl_logs(); } static const int once = []() { ForceEnableExperiment("event_engine_client", true); diff --git a/test/core/ext/filters/event_engine_client_channel_resolver/resolver_fuzzer.cc b/test/core/ext/filters/event_engine_client_channel_resolver/resolver_fuzzer.cc index ed7d60e3397..b305f41f776 100644 --- a/test/core/ext/filters/event_engine_client_channel_resolver/resolver_fuzzer.cc +++ b/test/core/ext/filters/event_engine_client_channel_resolver/resolver_fuzzer.cc @@ -48,6 +48,7 @@ #include "test/core/ext/filters/event_engine_client_channel_resolver/resolver_fuzzer.pb.h" #include "test/core/test_util/fuzz_config_vars.h" #include "test/core/test_util/fuzzing_channel_args.h" +#include "test/core/test_util/test_config.h" bool squelch = true; @@ -254,7 +255,7 @@ grpc_core::ResolverArgs ConstructResolverArgs( DEFINE_PROTO_FUZZER(const event_engine_client_channel_resolver::Msg& msg) { if (squelch) { - gpr_disable_all_logs(); + grpc_disable_all_absl_logs(); } bool done_resolving = false; grpc_core::ApplyFuzzConfigVars(msg.config_vars()); diff --git a/test/core/nanopb/fuzzer_response.cc b/test/core/nanopb/fuzzer_response.cc index 93537339e3a..9dd8ebbce7a 100644 --- a/test/core/nanopb/fuzzer_response.cc +++ b/test/core/nanopb/fuzzer_response.cc @@ -22,6 +22,8 @@ #include #include +#include "test/core/test_util/test_config.h" + bool squelch = true; bool leak_check = true; @@ -29,7 +31,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* /*data*/, size_t /*size*/) { grpc_init(); if (squelch) { - gpr_disable_all_logs(); + grpc_disable_all_absl_logs(); } // TODO(veblush): Convert this to upb. diff --git a/test/core/nanopb/fuzzer_serverlist.cc b/test/core/nanopb/fuzzer_serverlist.cc index 8f83c0a32a1..4e22041dfe5 100644 --- a/test/core/nanopb/fuzzer_serverlist.cc +++ b/test/core/nanopb/fuzzer_serverlist.cc @@ -22,6 +22,8 @@ #include #include +#include "test/core/test_util/test_config.h" + bool squelch = true; bool leak_check = true; @@ -29,7 +31,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* /*data*/, size_t /*size*/) { grpc_init(); if (squelch) { - gpr_disable_all_logs(); + grpc_disable_all_absl_logs(); } // TODO(veblush): Convert this to upb. diff --git a/test/core/resource_quota/BUILD b/test/core/resource_quota/BUILD index 74f08344668..d1029eb8b22 100644 --- a/test/core/resource_quota/BUILD +++ b/test/core/resource_quota/BUILD @@ -169,5 +169,6 @@ grpc_proto_fuzzer( "//src/core:memory_quota", "//src/core:useful", "//test/core/test_util:fuzz_config_vars", + "//test/core/test_util:grpc_test_util", ], ) diff --git a/test/core/resource_quota/memory_quota_fuzzer.cc b/test/core/resource_quota/memory_quota_fuzzer.cc index 6c3a1f4badb..2fb432cbe28 100644 --- a/test/core/resource_quota/memory_quota_fuzzer.cc +++ b/test/core/resource_quota/memory_quota_fuzzer.cc @@ -41,6 +41,7 @@ #include "test/core/resource_quota/call_checker.h" #include "test/core/resource_quota/memory_quota_fuzzer.pb.h" #include "test/core/test_util/fuzz_config_vars.h" +#include "test/core/test_util/test_config.h" bool squelch = true; bool leak_check = true; @@ -189,7 +190,7 @@ class Fuzzer { DEFINE_PROTO_FUZZER(const memory_quota_fuzzer::Msg& msg) { if (squelch) { - gpr_disable_all_logs(); + grpc_disable_all_absl_logs(); } grpc_core::ApplyFuzzConfigVars(msg.config_vars()); grpc_core::TestOnlyReloadExperimentsFromConfigVariables(); diff --git a/test/core/security/alts_credentials_fuzzer.cc b/test/core/security/alts_credentials_fuzzer.cc index 2d01d66012c..224a5ea9859 100644 --- a/test/core/security/alts_credentials_fuzzer.cc +++ b/test/core/security/alts_credentials_fuzzer.cc @@ -33,6 +33,7 @@ #include "src/core/lib/security/credentials/alts/check_gcp_environment.h" #include "src/core/lib/security/credentials/alts/grpc_alts_credentials_options.h" #include "test/core/test_util/fuzzer_util.h" +#include "test/core/test_util/test_config.h" using grpc_core::testing::grpc_fuzzer_get_next_byte; using grpc_core::testing::grpc_fuzzer_get_next_string; @@ -63,7 +64,7 @@ static void read_target_service_accounts( extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { if (squelch && !grpc_core::GetEnv("GRPC_TRACE_FUZZER").has_value()) { - gpr_disable_all_logs(); + grpc_disable_all_absl_logs(); } input_stream inp = {data, data + size}; grpc_init(); diff --git a/test/core/security/ssl_server_fuzzer.cc b/test/core/security/ssl_server_fuzzer.cc index a7498f6d7cd..62a9de9d69e 100644 --- a/test/core/security/ssl_server_fuzzer.cc +++ b/test/core/security/ssl_server_fuzzer.cc @@ -29,6 +29,7 @@ #include "src/core/lib/security/credentials/credentials.h" #include "src/core/lib/security/security_connector/security_connector.h" #include "test/core/test_util/mock_endpoint.h" +#include "test/core/test_util/test_config.h" #include "test/core/test_util/tls_utils.h" #define CA_CERT_PATH "src/core/tsi/test_creds/ca.pem" @@ -59,7 +60,7 @@ static void on_handshake_done(void* arg, grpc_error_handle error) { extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { if (squelch) { - gpr_disable_all_logs(); + grpc_disable_all_absl_logs(); } grpc_init(); { diff --git a/test/core/test_util/BUILD b/test/core/test_util/BUILD index db6a89cf1b9..d2e935f75f4 100644 --- a/test/core/test_util/BUILD +++ b/test/core/test_util/BUILD @@ -120,6 +120,7 @@ grpc_cc_library( "absl/base:core_headers", "absl/debugging:failure_signal_handler", "absl/log:check", + "absl/log:globals", "absl/log:log", "absl/status", "absl/status:statusor", diff --git a/test/core/test_util/test_config.cc b/test/core/test_util/test_config.cc index b816f5f1b10..2bc8a1861a9 100644 --- a/test/core/test_util/test_config.cc +++ b/test/core/test_util/test_config.cc @@ -22,6 +22,7 @@ #include #include "absl/debugging/failure_signal_handler.h" +#include "absl/log/globals.h" #include "absl/log/log.h" #include "absl/status/status.h" #include "absl/strings/match.h" @@ -150,6 +151,11 @@ bool grpc_wait_until_shutdown(int64_t time_s) { return true; } +void grpc_disable_all_absl_logs() { + absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfinity); + absl::SetVLogLevel("*grpc*/*", -1); +} + namespace grpc { namespace testing { diff --git a/test/core/test_util/test_config.h b/test/core/test_util/test_config.h index 9fe1f84c324..04876b7cf54 100644 --- a/test/core/test_util/test_config.h +++ b/test/core/test_util/test_config.h @@ -42,6 +42,9 @@ gpr_timespec grpc_timeout_milliseconds_to_deadline(int64_t time_ms); // Prefer TestEnvironment below. void grpc_test_init(int* argc, char** argv); +// Disable all absl logs via SetMinLogLevel and SetVLogLevel +void grpc_disable_all_absl_logs(void); + // Wait until gRPC is fully shut down. // Returns if grpc is shutdown bool grpc_wait_until_shutdown(int64_t time_s); diff --git a/test/core/transport/binder/end2end/fuzzers/client_fuzzer.cc b/test/core/transport/binder/end2end/fuzzers/client_fuzzer.cc index a5fa9905d81..e62c056c8b7 100644 --- a/test/core/transport/binder/end2end/fuzzers/client_fuzzer.cc +++ b/test/core/transport/binder/end2end/fuzzers/client_fuzzer.cc @@ -27,6 +27,7 @@ #include "src/core/lib/surface/channel.h" #include "src/core/lib/surface/channel_create.h" #include "src/libfuzzer/libfuzzer_macro.h" +#include "test/core/test_util/test_config.h" #include "test/core/transport/binder/end2end/fuzzers/binder_transport_fuzzer.pb.h" #include "test/core/transport/binder/end2end/fuzzers/fuzzer_utils.h" @@ -37,7 +38,7 @@ static void* tag(intptr_t t) { return reinterpret_cast(t); } DEFINE_PROTO_FUZZER(const binder_transport_fuzzer::Input& input) { if (squelch) { - gpr_disable_all_logs(); + grpc_disable_all_absl_logs(); } grpc_init(); { diff --git a/test/core/transport/binder/end2end/fuzzers/server_fuzzer.cc b/test/core/transport/binder/end2end/fuzzers/server_fuzzer.cc index 38bc3add000..bc8f0400b00 100644 --- a/test/core/transport/binder/end2end/fuzzers/server_fuzzer.cc +++ b/test/core/transport/binder/end2end/fuzzers/server_fuzzer.cc @@ -22,6 +22,7 @@ #include "src/core/lib/slice/slice_internal.h" #include "src/core/server/server.h" #include "src/libfuzzer/libfuzzer_macro.h" +#include "test/core/test_util/test_config.h" #include "test/core/transport/binder/end2end/fuzzers/binder_transport_fuzzer.pb.h" #include "test/core/transport/binder/end2end/fuzzers/fuzzer_utils.h" @@ -32,7 +33,7 @@ static void* tag(intptr_t t) { return reinterpret_cast(t); } DEFINE_PROTO_FUZZER(const binder_transport_fuzzer::Input& input) { if (squelch) { - gpr_disable_all_logs(); + grpc_disable_all_absl_logs(); } grpc_init(); { diff --git a/test/core/transport/chttp2/hpack_parser_fuzzer_test.cc b/test/core/transport/chttp2/hpack_parser_fuzzer_test.cc index 61dfc6a43ad..a6296636b8e 100644 --- a/test/core/transport/chttp2/hpack_parser_fuzzer_test.cc +++ b/test/core/transport/chttp2/hpack_parser_fuzzer_test.cc @@ -43,6 +43,7 @@ #include "src/libfuzzer/libfuzzer_macro.h" #include "test/core/test_util/fuzz_config_vars.h" #include "test/core/test_util/proto_bit_gen.h" +#include "test/core/test_util/test_config.h" #include "test/core/transport/chttp2/hpack_parser_fuzzer.pb.h" // IWYU pragma: no_include @@ -52,7 +53,7 @@ bool leak_check = true; DEFINE_PROTO_FUZZER(const hpack_parser_fuzzer::Msg& msg) { if (squelch) { - gpr_disable_all_logs(); + grpc_disable_all_absl_logs(); } grpc_core::ProtoBitGen proto_bit_src(msg.random_numbers()); grpc_core::ApplyFuzzConfigVars(msg.config_vars()); diff --git a/test/core/transport/chttp2/hpack_sync_fuzzer.cc b/test/core/transport/chttp2/hpack_sync_fuzzer.cc index b79953a7805..13aa0f2bb4a 100644 --- a/test/core/transport/chttp2/hpack_sync_fuzzer.cc +++ b/test/core/transport/chttp2/hpack_sync_fuzzer.cc @@ -47,6 +47,7 @@ #include "src/libfuzzer/libfuzzer_macro.h" #include "test/core/test_util/fuzz_config_vars.h" #include "test/core/test_util/proto_bit_gen.h" +#include "test/core/test_util/test_config.h" #include "test/core/transport/chttp2/hpack_sync_fuzzer.pb.h" bool squelch = true; @@ -169,7 +170,7 @@ void FuzzOneInput(const hpack_sync_fuzzer::Msg& msg) { DEFINE_PROTO_FUZZER(const hpack_sync_fuzzer::Msg& msg) { if (squelch) { - gpr_disable_all_logs(); + grpc_disable_all_absl_logs(); } grpc_core::ApplyFuzzConfigVars(msg.config_vars()); grpc_core::TestOnlyReloadExperimentsFromConfigVariables(); diff --git a/test/cpp/interop/BUILD b/test/cpp/interop/BUILD index 28624783556..768b355f754 100644 --- a/test/cpp/interop/BUILD +++ b/test/cpp/interop/BUILD @@ -158,6 +158,7 @@ grpc_cc_binary( language = "C++", deps = [ "//:grpc++", + "//test/core/test_util:grpc_test_util", "//test/cpp/util:metrics_server_lib", "//test/cpp/util:test_config", ], diff --git a/test/cpp/interop/metrics_client.cc b/test/cpp/interop/metrics_client.cc index 739764db17e..f15466edcc4 100644 --- a/test/cpp/interop/metrics_client.cc +++ b/test/cpp/interop/metrics_client.cc @@ -27,6 +27,7 @@ #include "src/core/lib/gprpp/crash.h" #include "src/proto/grpc/testing/metrics.grpc.pb.h" #include "src/proto/grpc/testing/metrics.pb.h" +#include "test/core/test_util/test_config.h" #include "test/cpp/util/metrics_server.h" #include "test/cpp/util/test_config.h" @@ -90,7 +91,7 @@ int main(int argc, char** argv) { // The output of metrics client is in some cases programmatically parsed (for // example by the stress test framework). So, we do not want any of the log // from the grpc library appearing on stdout. - gpr_disable_all_logs(); + grpc_disable_all_absl_logs(); std::shared_ptr channel( grpc::CreateChannel(absl::GetFlag(FLAGS_metrics_server_address), From e4b7d46f4305da721d4e29a9936fc91634ddcd23 Mon Sep 17 00:00:00 2001 From: Tanvi Jagtap <139093547+tanvi-jagtap@users.noreply.github.com> Date: Mon, 10 Jun 2024 20:06:12 -0700 Subject: [PATCH 9/9] [grpc][Gpr_To_Absl_Logging] Migrating from gpr to absl logging - gpr_log (#36800) [grpc][Gpr_To_Absl_Logging] Migrating from gpr to absl logging - gpr_log In this CL we are migrating from gRPCs own gpr logging mechanism to absl logging mechanism. The intention is to deprecate gpr_log in the future. We have the following mapping 1. gpr_log(GPR_INFO,...) -> LOG(INFO) 2. gpr_log(GPR_ERROR,...) -> LOG(ERROR) 3. gpr_log(GPR_DEBUG,...) -> VLOG(2) Reviewers need to check : 1. If the above mapping is correct. 2. The content of the log is as before. gpr_log format strings did not use string_view or std::string . absl LOG accepts these. So there will be some elimination of string_view and std::string related conversions. This is expected. Closes #36800 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36800 from tanvi-jagtap:src_core_lib cc0411e99dc79ec991db597727adabacdfc53b0f PiperOrigin-RevId: 642109824 --- src/core/lib/channel/channel_args.cc | 28 ++++---- src/core/lib/channel/promise_based_filter.h | 2 +- src/core/lib/compression/message_compress.cc | 3 +- src/core/lib/gprpp/crash.cc | 5 +- src/core/lib/gprpp/dual_ref_counted.h | 58 +++++++++-------- src/core/lib/gprpp/posix/stat.cc | 7 +- src/core/lib/gprpp/posix/thd.cc | 14 ++-- src/core/lib/gprpp/ref_counted.h | 43 ++++++------- src/core/lib/gprpp/time.cc | 7 +- src/core/lib/gprpp/time.h | 1 + src/core/lib/gprpp/validation_errors.cc | 7 +- src/core/lib/gprpp/windows/stat.cc | 7 +- .../authorization/cel_authorization_engine.cc | 19 +++--- .../security/authorization/evaluate_args.cc | 11 ++-- .../lib/security/authorization/matchers.cc | 6 +- .../certificate_provider_registry.cc | 5 +- .../alts/check_gcp_environment_no_op.cc | 5 +- .../grpc_alts_credentials_client_options.cc | 10 +-- .../alts/grpc_alts_credentials_options.cc | 6 +- .../lib/security/credentials/credentials.cc | 10 +-- .../external/external_account_credentials.cc | 12 ++-- .../google_default/credentials_generic.cc | 6 +- .../google_default_credentials.cc | 5 +- .../security/credentials/jwt/json_token.cc | 6 +- .../security/credentials/jwt/jwt_verifier.cc | 36 +++++------ .../credentials/ssl/ssl_credentials.cc | 27 +++----- .../tls/grpc_tls_certificate_provider.cc | 51 ++++++--------- .../tls/grpc_tls_credentials_options.cc | 5 +- .../credentials/tls/grpc_tls_crl_provider.cc | 8 +-- .../credentials/tls/tls_credentials.cc | 19 +++--- .../lib/security/credentials/tls/tls_utils.cc | 8 +-- .../alts/alts_security_connector.cc | 29 ++++----- .../fake/fake_security_connector.cc | 23 ++++--- .../load_system_roots_supported.cc | 4 +- .../local/local_security_connector.cc | 20 +++--- .../ssl/ssl_security_connector.cc | 33 +++++----- .../security/security_connector/ssl_utils.cc | 23 +++---- .../tls/tls_security_connector.cc | 64 ++++++++----------- .../http_client/httpcli_security_connector.cc | 12 ++-- 39 files changed, 286 insertions(+), 359 deletions(-) diff --git a/src/core/lib/channel/channel_args.cc b/src/core/lib/channel/channel_args.cc index 1e63ae38f9e..1c66f414350 100644 --- a/src/core/lib/channel/channel_args.cc +++ b/src/core/lib/channel/channel_args.cc @@ -29,6 +29,7 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" #include "absl/strings/match.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" @@ -36,7 +37,6 @@ #include #include -#include #include #include @@ -264,8 +264,7 @@ absl::optional ChannelArgs::GetBool(absl::string_view name) const { if (v == nullptr) return absl::nullopt; auto i = v->GetIfInt(); if (!i.has_value()) { - gpr_log(GPR_ERROR, "%s ignored: it must be an integer", - std::string(name).c_str()); + LOG(ERROR) << name << " ignored: it must be an integer"; return absl::nullopt; } switch (*i) { @@ -274,8 +273,8 @@ absl::optional ChannelArgs::GetBool(absl::string_view name) const { case 1: return true; default: - gpr_log(GPR_ERROR, "%s treated as bool but set to %d (assuming true)", - std::string(name).c_str(), *i); + LOG(ERROR) << name << " treated as bool but set to " << *i + << " (assuming true)"; return true; } } @@ -557,17 +556,15 @@ int grpc_channel_arg_get_integer(const grpc_arg* arg, const grpc_integer_options options) { if (arg == nullptr) return options.default_value; if (arg->type != GRPC_ARG_INTEGER) { - gpr_log(GPR_ERROR, "%s ignored: it must be an integer", arg->key); + LOG(ERROR) << arg->key << " ignored: it must be an integer"; return options.default_value; } if (arg->value.integer < options.min_value) { - gpr_log(GPR_ERROR, "%s ignored: it must be >= %d", arg->key, - options.min_value); + LOG(ERROR) << arg->key << " ignored: it must be >= " << options.min_value; return options.default_value; } if (arg->value.integer > options.max_value) { - gpr_log(GPR_ERROR, "%s ignored: it must be <= %d", arg->key, - options.max_value); + LOG(ERROR) << arg->key << " ignored: it must be <= " << options.max_value; return options.default_value; } return arg->value.integer; @@ -583,7 +580,7 @@ int grpc_channel_args_find_integer(const grpc_channel_args* args, char* grpc_channel_arg_get_string(const grpc_arg* arg) { if (arg == nullptr) return nullptr; if (arg->type != GRPC_ARG_STRING) { - gpr_log(GPR_ERROR, "%s ignored: it must be an string", arg->key); + LOG(ERROR) << arg->key << " ignored: it must be an string"; return nullptr; } return arg->value.string; @@ -598,7 +595,7 @@ char* grpc_channel_args_find_string(const grpc_channel_args* args, bool grpc_channel_arg_get_bool(const grpc_arg* arg, bool default_value) { if (arg == nullptr) return default_value; if (arg->type != GRPC_ARG_INTEGER) { - gpr_log(GPR_ERROR, "%s ignored: it must be an integer", arg->key); + LOG(ERROR) << arg->key << " ignored: it must be an integer"; return default_value; } switch (arg->value.integer) { @@ -607,8 +604,8 @@ bool grpc_channel_arg_get_bool(const grpc_arg* arg, bool default_value) { case 1: return true; default: - gpr_log(GPR_ERROR, "%s treated as bool but set to %d (assuming true)", - arg->key, arg->value.integer); + LOG(ERROR) << arg->key << " treated as bool but set to " + << arg->value.integer << " (assuming true)"; return true; } } @@ -667,8 +664,7 @@ ChannelArgs ChannelArgsBuiltinPrecondition(const grpc_channel_args* src) { if (key == GRPC_ARG_PRIMARY_USER_AGENT_STRING || key == GRPC_ARG_SECONDARY_USER_AGENT_STRING) { if (src->args[i].type != GRPC_ARG_STRING) { - gpr_log(GPR_ERROR, "Channel argument '%s' should be a string", - std::string(key).c_str()); + LOG(ERROR) << "Channel argument '" << key << "' should be a string"; } else { concatenated_values[key].push_back(src->args[i].value.string); } diff --git a/src/core/lib/channel/promise_based_filter.h b/src/core/lib/channel/promise_based_filter.h index a95b643e9b2..c40b1fb0964 100644 --- a/src/core/lib/channel/promise_based_filter.h +++ b/src/core/lib/channel/promise_based_filter.h @@ -32,6 +32,7 @@ #include "absl/container/inlined_vector.h" #include "absl/functional/function_ref.h" #include "absl/log/check.h" +#include "absl/log/log.h" #include "absl/meta/type_traits.h" #include "absl/status/status.h" #include "absl/strings/string_view.h" @@ -39,7 +40,6 @@ #include #include -#include #include #include "src/core/lib/channel/call_finalization.h" diff --git a/src/core/lib/compression/message_compress.cc b/src/core/lib/compression/message_compress.cc index bb806509286..e26369ad276 100644 --- a/src/core/lib/compression/message_compress.cc +++ b/src/core/lib/compression/message_compress.cc @@ -28,7 +28,6 @@ #include #include -#include #include #include "src/core/lib/slice/slice.h" @@ -63,7 +62,7 @@ static int zlib_body(z_stream* zs, grpc_slice_buffer* input, } r = flate(zs, flush); if (r < 0 && r != Z_BUF_ERROR /* not fatal */) { - gpr_log(GPR_INFO, "zlib error (%d)", r); + LOG(INFO) << "zlib error (" << r << ")"; goto error; } } while (zs->avail_out == 0); diff --git a/src/core/lib/gprpp/crash.cc b/src/core/lib/gprpp/crash.cc index 1db8043438b..35128d73a11 100644 --- a/src/core/lib/gprpp/crash.cc +++ b/src/core/lib/gprpp/crash.cc @@ -19,16 +19,15 @@ #include +#include "absl/log/log.h" #include "absl/strings/str_cat.h" -#include #include namespace grpc_core { void Crash(absl::string_view message, SourceLocation location) { - gpr_log(location.file(), location.line(), GPR_LOG_SEVERITY_ERROR, "%s", - std::string(message).c_str()); + LOG(ERROR).AtLocation(location.file(), location.line()) << message; abort(); } diff --git a/src/core/lib/gprpp/dual_ref_counted.h b/src/core/lib/gprpp/dual_ref_counted.h index a7482577eff..3f714e57657 100644 --- a/src/core/lib/gprpp/dual_ref_counted.h +++ b/src/core/lib/gprpp/dual_ref_counted.h @@ -21,8 +21,8 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" -#include #include #include "src/core/lib/gprpp/debug_location.h" @@ -93,8 +93,9 @@ class DualRefCounted : public Impl { #ifndef NDEBUG const uint32_t weak_refs = GetWeakRefs(prev_ref_pair); if (trace_ != nullptr) { - gpr_log(GPR_INFO, "%s:%p unref %d -> %d, weak_ref %d -> %d", trace_, this, - strong_refs, strong_refs - 1, weak_refs, weak_refs + 1); + LOG(INFO) << trace_ << ":" << this << " unref " << strong_refs << " -> " + << strong_refs - 1 << ", weak_ref " << weak_refs << " -> " + << weak_refs + 1; } CHECK_GT(strong_refs, 0u); #endif @@ -111,9 +112,10 @@ class DualRefCounted : public Impl { #ifndef NDEBUG const uint32_t weak_refs = GetWeakRefs(prev_ref_pair); if (trace_ != nullptr) { - gpr_log(GPR_INFO, "%s:%p %s:%d unref %d -> %d, weak_ref %d -> %d) %s", - trace_, this, location.file(), location.line(), strong_refs, - strong_refs - 1, weak_refs, weak_refs + 1, reason); + LOG(INFO) << trace_ << ":" << this << " " << location.file() << ":" + << location.line() << " unref " << strong_refs << " -> " + << strong_refs - 1 << ", weak_ref " << weak_refs << " -> " + << weak_refs + 1 << ") " << reason; } CHECK_GT(strong_refs, 0u); #else @@ -135,8 +137,9 @@ class DualRefCounted : public Impl { #ifndef NDEBUG const uint32_t weak_refs = GetWeakRefs(prev_ref_pair); if (trace_ != nullptr) { - gpr_log(GPR_INFO, "%s:%p ref_if_non_zero %d -> %d (weak_refs=%d)", - trace_, this, strong_refs, strong_refs + 1, weak_refs); + LOG(INFO) << trace_ << ":" << this << " ref_if_non_zero " << strong_refs + << " -> " << strong_refs + 1 << " (weak_refs=" << weak_refs + << ")"; } #endif if (strong_refs == 0) return nullptr; @@ -153,10 +156,10 @@ class DualRefCounted : public Impl { #ifndef NDEBUG const uint32_t weak_refs = GetWeakRefs(prev_ref_pair); if (trace_ != nullptr) { - gpr_log(GPR_INFO, - "%s:%p %s:%d ref_if_non_zero %d -> %d (weak_refs=%d) %s", - trace_, this, location.file(), location.line(), strong_refs, - strong_refs + 1, weak_refs, reason); + LOG(INFO) << trace_ << ":" << this << " " << location.file() << ":" + << location.line() << " ref_if_non_zero " << strong_refs + << " -> " << strong_refs + 1 << " (weak_refs=" << weak_refs + << ") " << reason; } #else // Avoid unused-parameter warnings for debug-only parameters @@ -211,8 +214,8 @@ class DualRefCounted : public Impl { const uint32_t weak_refs = GetWeakRefs(prev_ref_pair); const uint32_t strong_refs = GetStrongRefs(prev_ref_pair); if (trace != nullptr) { - gpr_log(GPR_INFO, "%s:%p weak_unref %d -> %d (refs=%d)", trace, this, - weak_refs, weak_refs - 1, strong_refs); + LOG(INFO) << trace << ":" << this << " weak_unref " << weak_refs << " -> " + << weak_refs - 1 << " (refs=" << strong_refs << ")"; } CHECK_GT(weak_refs, 0u); #endif @@ -233,9 +236,9 @@ class DualRefCounted : public Impl { const uint32_t weak_refs = GetWeakRefs(prev_ref_pair); const uint32_t strong_refs = GetStrongRefs(prev_ref_pair); if (trace != nullptr) { - gpr_log(GPR_INFO, "%s:%p %s:%d weak_unref %d -> %d (refs=%d) %s", trace, - this, location.file(), location.line(), weak_refs, weak_refs - 1, - strong_refs, reason); + LOG(INFO) << trace << ":" << this << " " << location.file() << ":" + << location.line() << " weak_unref " << weak_refs << " -> " + << weak_refs - 1 << " (refs=" << strong_refs << ") " << reason; } CHECK_GT(weak_refs, 0u); #else @@ -298,8 +301,8 @@ class DualRefCounted : public Impl { const uint32_t weak_refs = GetWeakRefs(prev_ref_pair); CHECK_NE(strong_refs, 0u); if (trace_ != nullptr) { - gpr_log(GPR_INFO, "%s:%p ref %d -> %d; (weak_refs=%d)", trace_, this, - strong_refs, strong_refs + 1, weak_refs); + LOG(INFO) << trace_ << ":" << this << " ref " << strong_refs << " -> " + << strong_refs + 1 << "; (weak_refs=" << weak_refs << ")"; } #else refs_.fetch_add(MakeRefPair(1, 0), std::memory_order_relaxed); @@ -313,9 +316,10 @@ class DualRefCounted : public Impl { const uint32_t weak_refs = GetWeakRefs(prev_ref_pair); CHECK_NE(strong_refs, 0u); if (trace_ != nullptr) { - gpr_log(GPR_INFO, "%s:%p %s:%d ref %d -> %d (weak_refs=%d) %s", trace_, - this, location.file(), location.line(), strong_refs, - strong_refs + 1, weak_refs, reason); + LOG(INFO) << trace_ << ":" << this << " " << location.file() << ":" + << location.line() << " ref " << strong_refs << " -> " + << strong_refs + 1 << " (weak_refs=" << weak_refs << ") " + << reason; } #else // Use conditionally-important parameters @@ -332,8 +336,8 @@ class DualRefCounted : public Impl { const uint32_t strong_refs = GetStrongRefs(prev_ref_pair); const uint32_t weak_refs = GetWeakRefs(prev_ref_pair); if (trace_ != nullptr) { - gpr_log(GPR_INFO, "%s:%p weak_ref %d -> %d; (refs=%d)", trace_, this, - weak_refs, weak_refs + 1, strong_refs); + LOG(INFO) << trace_ << ":" << this << " weak_ref " << weak_refs << " -> " + << weak_refs + 1 << "; (refs=" << strong_refs << ")"; } if (strong_refs == 0) CHECK_NE(weak_refs, 0u); #else @@ -348,9 +352,9 @@ class DualRefCounted : public Impl { const uint32_t strong_refs = GetStrongRefs(prev_ref_pair); const uint32_t weak_refs = GetWeakRefs(prev_ref_pair); if (trace_ != nullptr) { - gpr_log(GPR_INFO, "%s:%p %s:%d weak_ref %d -> %d (refs=%d) %s", trace_, - this, location.file(), location.line(), weak_refs, weak_refs + 1, - strong_refs, reason); + LOG(INFO) << trace_ << ":" << this << " " << location.file() << ":" + << location.line() << " weak_ref " << weak_refs << " -> " + << weak_refs + 1 << " (refs=" << strong_refs << ") " << reason; } if (strong_refs == 0) CHECK_NE(weak_refs, 0u); #else diff --git a/src/core/lib/gprpp/posix/stat.cc b/src/core/lib/gprpp/posix/stat.cc index f46741af5c9..e0d2e617506 100644 --- a/src/core/lib/gprpp/posix/stat.cc +++ b/src/core/lib/gprpp/posix/stat.cc @@ -28,8 +28,7 @@ #include #include "absl/log/check.h" - -#include +#include "absl/log/log.h" #include "src/core/lib/gprpp/stat.h" #include "src/core/lib/gprpp/strerror.h" @@ -42,8 +41,8 @@ absl::Status GetFileModificationTime(const char* filename, time_t* timestamp) { struct stat buf; if (stat(filename, &buf) != 0) { std::string error_msg = StrError(errno); - gpr_log(GPR_ERROR, "stat failed for filename %s with error %s.", filename, - error_msg.c_str()); + LOG(ERROR) << "stat failed for filename " << filename << " with error " + << error_msg; return absl::Status(absl::StatusCode::kInternal, error_msg); } // Last file/directory modification time. diff --git a/src/core/lib/gprpp/posix/thd.cc b/src/core/lib/gprpp/posix/thd.cc index dd3124fad25..b481fa7c565 100644 --- a/src/core/lib/gprpp/posix/thd.cc +++ b/src/core/lib/gprpp/posix/thd.cc @@ -33,9 +33,8 @@ #include #include "absl/log/check.h" -#include "absl/log/log.h" // IWYU pragma: keep +#include "absl/log/log.h" -#include #include #include #include @@ -158,8 +157,7 @@ class ThreadInternalsPosix : public internal::ThreadInternalsInterface { CHECK_EQ(pthread_attr_destroy(&attr), 0); if (!(*success)) { - gpr_log(GPR_ERROR, "pthread_create failed: %s", - StrError(pthread_create_err).c_str()); + LOG(ERROR) << "pthread_create failed: " << StrError(pthread_create_err); // don't use gpr_free, as this was allocated using malloc (see above) free(info); if (options.tracked()) { @@ -199,8 +197,8 @@ class ThreadInternalsPosix : public internal::ThreadInternalsInterface { void Thread::Signal(gpr_thd_id tid, int sig) { auto kill_err = pthread_kill((pthread_t)tid, sig); if (kill_err != 0) { - gpr_log(GPR_ERROR, "pthread_kill for tid %" PRIdPTR " failed: %s", tid, - StrError(kill_err).c_str()); + LOG(ERROR) << "pthread_kill for tid " << tid + << " failed: " << StrError(kill_err); } } @@ -208,8 +206,8 @@ void Thread::Signal(gpr_thd_id tid, int sig) { void Thread::Kill(gpr_thd_id tid) { auto cancel_err = pthread_cancel((pthread_t)tid); if (cancel_err != 0) { - gpr_log(GPR_ERROR, "pthread_cancel for tid %" PRIdPTR " failed: %s", tid, - StrError(cancel_err).c_str()); + LOG(ERROR) << "pthread_cancel for tid " << tid + << " failed: " << StrError(cancel_err); } } #else // GPR_ANDROID diff --git a/src/core/lib/gprpp/ref_counted.h b/src/core/lib/gprpp/ref_counted.h index 8e85d3448d8..e8b6347f4b9 100644 --- a/src/core/lib/gprpp/ref_counted.h +++ b/src/core/lib/gprpp/ref_counted.h @@ -24,8 +24,8 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" -#include #include #include "src/core/lib/gprpp/atomic_utils.h" @@ -73,8 +73,8 @@ class RefCount { #ifndef NDEBUG const Value prior = value_.fetch_add(n, std::memory_order_relaxed); if (trace_ != nullptr) { - gpr_log(GPR_INFO, "%s:%p ref %" PRIdPTR " -> %" PRIdPTR, trace_, this, - prior, prior + n); + LOG(INFO) << trace_ << ":" << this << " ref " << prior << " -> " + << prior + n; } #else value_.fetch_add(n, std::memory_order_relaxed); @@ -84,9 +84,9 @@ class RefCount { #ifndef NDEBUG const Value prior = value_.fetch_add(n, std::memory_order_relaxed); if (trace_ != nullptr) { - gpr_log(GPR_INFO, "%s:%p %s:%d ref %" PRIdPTR " -> %" PRIdPTR " %s", - trace_, this, location.file(), location.line(), prior, prior + n, - reason); + LOG(INFO) << trace_ << ":" << this << " " << location.file() << ":" + << location.line() << " ref " << prior << " -> " << prior + n + << " " << reason; } #else // Use conditionally-important parameters @@ -101,8 +101,8 @@ class RefCount { #ifndef NDEBUG const Value prior = value_.fetch_add(1, std::memory_order_relaxed); if (trace_ != nullptr) { - gpr_log(GPR_INFO, "%s:%p ref %" PRIdPTR " -> %" PRIdPTR, trace_, this, - prior, prior + 1); + LOG(INFO) << trace_ << ":" << this << " ref " << prior << " -> " + << prior + 1; } assert(prior > 0); #else @@ -113,9 +113,9 @@ class RefCount { #ifndef NDEBUG const Value prior = value_.fetch_add(1, std::memory_order_relaxed); if (trace_ != nullptr) { - gpr_log(GPR_INFO, "%s:%p %s:%d ref %" PRIdPTR " -> %" PRIdPTR " %s", - trace_, this, location.file(), location.line(), prior, prior + 1, - reason); + LOG(INFO) << trace_ << ":" << this << " " << location.file() << ":" + << location.line() << " ref " << prior << " -> " << prior + 1 + << " " << reason; } assert(prior > 0); #else @@ -130,8 +130,8 @@ class RefCount { #ifndef NDEBUG if (trace_ != nullptr) { const Value prior = get(); - gpr_log(GPR_INFO, "%s:%p ref_if_non_zero %" PRIdPTR " -> %" PRIdPTR, - trace_, this, prior, prior + 1); + LOG(INFO) << trace_ << ":" << this << " ref_if_non_zero " << prior + << " -> " << prior + 1; } #endif return IncrementIfNonzero(&value_); @@ -140,10 +140,9 @@ class RefCount { #ifndef NDEBUG if (trace_ != nullptr) { const Value prior = get(); - gpr_log(GPR_INFO, - "%s:%p %s:%d ref_if_non_zero %" PRIdPTR " -> %" PRIdPTR " %s", - trace_, this, location.file(), location.line(), prior, prior + 1, - reason); + LOG(INFO) << trace_ << ":" << this << " " << location.file() << ":" + << location.line() << " ref_if_non_zero " << prior << " -> " + << prior + 1 << " " << reason; } #endif // Avoid unused-parameter warnings for debug-only parameters @@ -163,8 +162,8 @@ class RefCount { const Value prior = value_.fetch_sub(1, std::memory_order_acq_rel); #ifndef NDEBUG if (trace != nullptr) { - gpr_log(GPR_INFO, "%s:%p unref %" PRIdPTR " -> %" PRIdPTR, trace, this, - prior, prior - 1); + LOG(INFO) << trace << ":" << this << " unref " << prior << " -> " + << prior - 1; } DCHECK_GT(prior, 0); #endif @@ -180,9 +179,9 @@ class RefCount { const Value prior = value_.fetch_sub(1, std::memory_order_acq_rel); #ifndef NDEBUG if (trace != nullptr) { - gpr_log(GPR_INFO, "%s:%p %s:%d unref %" PRIdPTR " -> %" PRIdPTR " %s", - trace, this, location.file(), location.line(), prior, prior - 1, - reason); + LOG(INFO) << trace << ":" << this << " " << location.file() << ":" + << location.line() << " unref " << prior << " -> " << prior - 1 + << " " << reason; } DCHECK_GT(prior, 0); #else diff --git a/src/core/lib/gprpp/time.cc b/src/core/lib/gprpp/time.cc index 02240710a54..db657a0880b 100644 --- a/src/core/lib/gprpp/time.cc +++ b/src/core/lib/gprpp/time.cc @@ -21,9 +21,9 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" #include "absl/strings/str_format.h" -#include #include #include @@ -60,9 +60,8 @@ GPR_ATTRIBUTE_NOINLINE std::pair InitTime() { if (process_epoch_seconds > 1) { break; } - gpr_log(GPR_INFO, - "gpr_now(GPR_CLOCK_MONOTONIC) returns a very small number: " - "sleeping for 100ms"); + LOG(INFO) << "gpr_now(GPR_CLOCK_MONOTONIC) returns a very small number: " + "sleeping for 100ms"; gpr_sleep_until(gpr_time_add(now, gpr_time_from_millis(100, GPR_TIMESPAN))); } diff --git a/src/core/lib/gprpp/time.h b/src/core/lib/gprpp/time.h index 96963753d38..1f2a4815682 100644 --- a/src/core/lib/gprpp/time.h +++ b/src/core/lib/gprpp/time.h @@ -24,6 +24,7 @@ #include "absl/types/optional.h" #include +#include #include #include diff --git a/src/core/lib/gprpp/validation_errors.cc b/src/core/lib/gprpp/validation_errors.cc index 4cae4f814e4..84d8d75a2fb 100644 --- a/src/core/lib/gprpp/validation_errors.cc +++ b/src/core/lib/gprpp/validation_errors.cc @@ -18,12 +18,12 @@ #include +#include "absl/log/log.h" #include "absl/status/status.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_join.h" #include "absl/strings/strip.h" -#include #include namespace grpc_core { @@ -39,9 +39,8 @@ void ValidationErrors::PopField() { fields_.pop_back(); } void ValidationErrors::AddError(absl::string_view error) { auto key = absl::StrJoin(fields_, ""); if (field_errors_[key].size() >= max_error_count_) { - gpr_log(GPR_DEBUG, - "Ignoring validation error: too many errors found (%" PRIuPTR ")", - max_error_count_); + VLOG(2) << "Ignoring validation error: too many errors found (" + << max_error_count_ << ")"; return; } field_errors_[key].emplace_back(error); diff --git a/src/core/lib/gprpp/windows/stat.cc b/src/core/lib/gprpp/windows/stat.cc index 3d2d42f814b..aa1017a1d4a 100644 --- a/src/core/lib/gprpp/windows/stat.cc +++ b/src/core/lib/gprpp/windows/stat.cc @@ -23,8 +23,7 @@ #include #include "absl/log/check.h" - -#include +#include "absl/log/log.h" #include "src/core/lib/gprpp/crash.h" #include "src/core/lib/gprpp/stat.h" @@ -38,8 +37,8 @@ absl::Status GetFileModificationTime(const char* filename, time_t* timestamp) { struct _stat buf; if (_stat(filename, &buf) != 0) { std::string error_msg = StrError(errno); - gpr_log(GPR_ERROR, "_stat failed for filename %s with error %s.", filename, - error_msg.c_str()); + LOG(ERROR) << "_stat failed for filename " << filename << " with error " + << error_msg; return absl::Status(absl::StatusCode::kInternal, error_msg); } // Last file/directory modification time. diff --git a/src/core/lib/security/authorization/cel_authorization_engine.cc b/src/core/lib/security/authorization/cel_authorization_engine.cc index 526fb79576e..d2d14ebbb8b 100644 --- a/src/core/lib/security/authorization/cel_authorization_engine.cc +++ b/src/core/lib/security/authorization/cel_authorization_engine.cc @@ -19,13 +19,13 @@ #include #include +#include "absl/log/log.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" #include "absl/types/span.h" #include "upb/base/string_view.h" #include "upb/message/map.h" -#include #include namespace grpc_core { @@ -50,16 +50,14 @@ std::unique_ptr CelAuthorizationEngine::CreateCelAuthorizationEngine( const std::vector& rbac_policies) { if (rbac_policies.empty() || rbac_policies.size() > 2) { - gpr_log(GPR_ERROR, - "Invalid rbac policies vector. Must contain either one or two rbac " - "policies."); + LOG(ERROR) << "Invalid rbac policies vector. Must contain either one or " + "two rbac policies."; return nullptr; } else if (rbac_policies.size() == 2 && (envoy_config_rbac_v3_RBAC_action(rbac_policies[0]) != kDeny || envoy_config_rbac_v3_RBAC_action(rbac_policies[1]) != kAllow)) { - gpr_log(GPR_ERROR, - "Invalid rbac policies vector. Must contain one deny \ - policy and one allow policy, in that order."); + LOG(ERROR) << "Invalid rbac policies vector. Must contain one deny policy " + "and one allow policy, in that order."; return nullptr; } else { return std::make_unique(rbac_policies); @@ -175,10 +173,9 @@ std::unique_ptr CelAuthorizationEngine::CreateActivation( mock_cel::CelValue::CreateStringView(cert_server_name)); } } else { - gpr_log(GPR_ERROR, - "Error: Authorization engine does not support evaluating " - "attribute %s.", - elem.c_str()); + LOG(ERROR) << "Error: Authorization engine does not support evaluating " + "attribute " + << elem; } } return activation; diff --git a/src/core/lib/security/authorization/evaluate_args.cc b/src/core/lib/security/authorization/evaluate_args.cc index 8da1bd7eeb3..9a420347c09 100644 --- a/src/core/lib/security/authorization/evaluate_args.cc +++ b/src/core/lib/security/authorization/evaluate_args.cc @@ -23,7 +23,6 @@ #include "absl/strings/numbers.h" #include -#include #include #include "src/core/handshaker/endpoint_info/endpoint_info_handshaker.h" @@ -48,19 +47,17 @@ EvaluateArgs::PerChannelArgs::Address ParseEndpointUri( absl::string_view host_view; absl::string_view port_view; if (!SplitHostPort(uri->path(), &host_view, &port_view)) { - gpr_log(GPR_DEBUG, "Failed to split %s into host and port.", - uri->path().c_str()); + VLOG(2) << "Failed to split " << uri->path() << " into host and port."; return address; } if (!absl::SimpleAtoi(port_view, &address.port)) { - gpr_log(GPR_DEBUG, "Port %s is out of range or null.", - std::string(port_view).c_str()); + VLOG(2) << "Port " << port_view << " is out of range or null."; } address.address_str = std::string(host_view); auto resolved_address = StringToSockaddr(uri->path()); if (!resolved_address.ok()) { - gpr_log(GPR_DEBUG, "Address \"%s\" is not IPv4/IPv6. Error: %s", - uri->path().c_str(), resolved_address.status().ToString().c_str()); + VLOG(2) << "Address \"" << uri->path() + << "\" is not IPv4/IPv6. Error: " << resolved_address.status(); memset(&address.address, 0, sizeof(address.address)); } else { address.address = *resolved_address; diff --git a/src/core/lib/security/authorization/matchers.cc b/src/core/lib/security/authorization/matchers.cc index 8fc2fe4d2d0..887f74e79ed 100644 --- a/src/core/lib/security/authorization/matchers.cc +++ b/src/core/lib/security/authorization/matchers.cc @@ -18,12 +18,12 @@ #include +#include "absl/log/log.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include -#include #include #include "src/core/lib/address_utils/parse_address.h" @@ -157,8 +157,8 @@ IpAuthorizationMatcher::IpAuthorizationMatcher(Type type, Rbac::CidrRange range) auto address = StringToSockaddr(range.address_prefix, 0); // Port does not matter here. if (!address.ok()) { - gpr_log(GPR_DEBUG, "CidrRange address \"%s\" is not IPv4/IPv6. Error: %s", - range.address_prefix.c_str(), address.status().ToString().c_str()); + VLOG(2) << "CidrRange address \"" << range.address_prefix + << "\" is not IPv4/IPv6. Error: " << address.status(); memset(&subnet_address_, 0, sizeof(subnet_address_)); return; } diff --git a/src/core/lib/security/certificate_provider/certificate_provider_registry.cc b/src/core/lib/security/certificate_provider/certificate_provider_registry.cc index a2ca59f95be..7a213fdc884 100644 --- a/src/core/lib/security/certificate_provider/certificate_provider_registry.cc +++ b/src/core/lib/security/certificate_provider/certificate_provider_registry.cc @@ -22,8 +22,8 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" -#include #include namespace grpc_core { @@ -31,8 +31,7 @@ namespace grpc_core { void CertificateProviderRegistry::Builder::RegisterCertificateProviderFactory( std::unique_ptr factory) { absl::string_view name = factory->name(); - gpr_log(GPR_DEBUG, "registering certificate provider factory for \"%s\"", - std::string(name).c_str()); + VLOG(2) << "registering certificate provider factory for \"" << name << "\""; CHECK(factories_.emplace(name, std::move(factory)).second); } diff --git a/src/core/lib/security/credentials/alts/check_gcp_environment_no_op.cc b/src/core/lib/security/credentials/alts/check_gcp_environment_no_op.cc index 7c50a81f609..9174d301799 100644 --- a/src/core/lib/security/credentials/alts/check_gcp_environment_no_op.cc +++ b/src/core/lib/security/credentials/alts/check_gcp_environment_no_op.cc @@ -20,14 +20,13 @@ #if !defined(GPR_LINUX) && !defined(GPR_WINDOWS) -#include +#include "absl/log/log.h" #include "src/core/lib/gprpp/crash.h" #include "src/core/lib/security/credentials/alts/check_gcp_environment.h" bool grpc_alts_is_running_on_gcp() { - gpr_log(GPR_INFO, - "ALTS: Platforms other than Linux and Windows are not supported"); + LOG(INFO) << "ALTS: Platforms other than Linux and Windows are not supported"; return false; } diff --git a/src/core/lib/security/credentials/alts/grpc_alts_credentials_client_options.cc b/src/core/lib/security/credentials/alts/grpc_alts_credentials_client_options.cc index 4b9e91b9833..a6053a5d810 100644 --- a/src/core/lib/security/credentials/alts/grpc_alts_credentials_client_options.cc +++ b/src/core/lib/security/credentials/alts/grpc_alts_credentials_client_options.cc @@ -16,9 +16,10 @@ // // +#include "absl/log/log.h" + #include #include -#include #include #include @@ -44,10 +45,9 @@ static target_service_account* target_service_account_create( void grpc_alts_credentials_client_options_add_target_service_account( grpc_alts_credentials_options* options, const char* service_account) { if (options == nullptr || service_account == nullptr) { - gpr_log( - GPR_ERROR, - "Invalid nullptr arguments to " - "grpc_alts_credentials_client_options_add_target_service_account()"); + LOG(ERROR) + << "Invalid nullptr arguments to " + "grpc_alts_credentials_client_options_add_target_service_account()"; return; } auto client_options = diff --git a/src/core/lib/security/credentials/alts/grpc_alts_credentials_options.cc b/src/core/lib/security/credentials/alts/grpc_alts_credentials_options.cc index ec29ac2dbff..6b11ab3f623 100644 --- a/src/core/lib/security/credentials/alts/grpc_alts_credentials_options.cc +++ b/src/core/lib/security/credentials/alts/grpc_alts_credentials_options.cc @@ -18,8 +18,9 @@ #include "src/core/lib/security/credentials/alts/grpc_alts_credentials_options.h" +#include "absl/log/log.h" + #include -#include #include grpc_alts_credentials_options* grpc_alts_credentials_options_copy( @@ -29,8 +30,7 @@ grpc_alts_credentials_options* grpc_alts_credentials_options_copy( return options->vtable->copy(options); } // An error occurred. - gpr_log(GPR_ERROR, - "Invalid arguments to grpc_alts_credentials_options_copy()"); + LOG(ERROR) << "Invalid arguments to grpc_alts_credentials_options_copy()"; return nullptr; } diff --git a/src/core/lib/security/credentials/credentials.cc b/src/core/lib/security/credentials/credentials.cc index 4220938e31a..1152cfe68df 100644 --- a/src/core/lib/security/credentials/credentials.cc +++ b/src/core/lib/security/credentials/credentials.cc @@ -22,8 +22,8 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" -#include #include #include "src/core/lib/channel/channel_args.h" @@ -75,8 +75,8 @@ grpc_channel_credentials* grpc_channel_credentials_from_arg( const grpc_arg* arg) { if (strcmp(arg->key, GRPC_ARG_CHANNEL_CREDENTIALS) != 0) return nullptr; if (arg->type != GRPC_ARG_POINTER) { - gpr_log(GPR_ERROR, "Invalid type %d for arg %s", arg->type, - GRPC_ARG_CHANNEL_CREDENTIALS); + LOG(ERROR) << "Invalid type " << arg->type << " for arg " + << GRPC_ARG_CHANNEL_CREDENTIALS; return nullptr; } return static_cast(arg->value.pointer.p); @@ -141,8 +141,8 @@ grpc_arg grpc_server_credentials_to_arg(grpc_server_credentials* c) { grpc_server_credentials* grpc_server_credentials_from_arg(const grpc_arg* arg) { if (strcmp(arg->key, GRPC_SERVER_CREDENTIALS_ARG) != 0) return nullptr; if (arg->type != GRPC_ARG_POINTER) { - gpr_log(GPR_ERROR, "Invalid type %d for arg %s", arg->type, - GRPC_SERVER_CREDENTIALS_ARG); + LOG(ERROR) << "Invalid type " << arg->type << " for arg " + << GRPC_SERVER_CREDENTIALS_ARG; return nullptr; } return static_cast(arg->value.pointer.p); diff --git a/src/core/lib/security/credentials/external/external_account_credentials.cc b/src/core/lib/security/credentials/external/external_account_credentials.cc index f7fd5a805e1..74552306131 100644 --- a/src/core/lib/security/credentials/external/external_account_credentials.cc +++ b/src/core/lib/security/credentials/external/external_account_credentials.cc @@ -23,6 +23,7 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/escaping.h" @@ -41,7 +42,6 @@ #include #include #include -#include #include #include @@ -588,9 +588,8 @@ grpc_call_credentials* grpc_external_account_credentials_create( const char* json_string, const char* scopes_string) { auto json = grpc_core::JsonParse(json_string); if (!json.ok()) { - gpr_log(GPR_ERROR, - "External account credentials creation failed. Error: %s.", - json.status().ToString().c_str()); + LOG(ERROR) << "External account credentials creation failed. Error: " + << json.status(); return nullptr; } std::vector scopes = absl::StrSplit(scopes_string, ','); @@ -599,9 +598,8 @@ grpc_call_credentials* grpc_external_account_credentials_create( *json, std::move(scopes), &error) .release(); if (!error.ok()) { - gpr_log(GPR_ERROR, - "External account credentials creation failed. Error: %s.", - grpc_core::StatusToString(error).c_str()); + LOG(ERROR) << "External account credentials creation failed. Error: " + << grpc_core::StatusToString(error); return nullptr; } return creds; diff --git a/src/core/lib/security/credentials/google_default/credentials_generic.cc b/src/core/lib/security/credentials/google_default/credentials_generic.cc index bfcc7c37e08..cf748049b98 100644 --- a/src/core/lib/security/credentials/google_default/credentials_generic.cc +++ b/src/core/lib/security/credentials/google_default/credentials_generic.cc @@ -18,10 +18,10 @@ #include +#include "absl/log/log.h" #include "absl/strings/str_cat.h" #include "absl/types/optional.h" -#include #include #include "src/core/lib/gprpp/env.h" @@ -31,8 +31,8 @@ std::string grpc_get_well_known_google_credentials_file_path_impl(void) { auto base = grpc_core::GetEnv(GRPC_GOOGLE_CREDENTIALS_PATH_ENV_VAR); if (!base.has_value()) { - gpr_log(GPR_ERROR, "Could not get " GRPC_GOOGLE_CREDENTIALS_PATH_ENV_VAR - " environment variable."); + LOG(ERROR) << "Could not get " << GRPC_GOOGLE_CREDENTIALS_PATH_ENV_VAR + << " environment variable."; return ""; } return absl::StrCat(*base, "/", GRPC_GOOGLE_CREDENTIALS_PATH_SUFFIX); diff --git a/src/core/lib/security/credentials/google_default/google_default_credentials.cc b/src/core/lib/security/credentials/google_default/google_default_credentials.cc index bebf35e4710..a7e6070e788 100644 --- a/src/core/lib/security/credentials/google_default/google_default_credentials.cc +++ b/src/core/lib/security/credentials/google_default/google_default_credentials.cc @@ -37,7 +37,6 @@ #include #include #include -#include #include #include @@ -411,8 +410,8 @@ grpc_channel_credentials* grpc_google_default_credentials_create( creds.get(), call_creds.get(), nullptr); CHECK_NE(result, nullptr); } else { - gpr_log(GPR_ERROR, "Could not create google default credentials: %s", - grpc_core::StatusToString(error).c_str()); + LOG(ERROR) << "Could not create google default credentials: " + << grpc_core::StatusToString(error); } return result; } diff --git a/src/core/lib/security/credentials/jwt/json_token.cc b/src/core/lib/security/credentials/jwt/json_token.cc index 456ff99dd4f..0c81efb6690 100644 --- a/src/core/lib/security/credentials/jwt/json_token.cc +++ b/src/core/lib/security/credentials/jwt/json_token.cc @@ -39,7 +39,6 @@ #include #include #include -#include #include #include #include @@ -141,8 +140,7 @@ grpc_auth_json_key grpc_auth_json_key_create_from_string( Json json; auto json_or = grpc_core::JsonParse(json_string); if (!json_or.ok()) { - gpr_log(GPR_ERROR, "JSON key parsing error: %s", - json_or.status().ToString().c_str()); + LOG(ERROR) << "JSON key parsing error: " << json_or.status(); } else { json = std::move(*json_or); } @@ -238,7 +236,7 @@ const EVP_MD* openssl_digest_from_algorithm(const char* algorithm) { if (strcmp(algorithm, GRPC_JWT_RSA_SHA256_ALGORITHM) == 0) { return EVP_sha256(); } else { - gpr_log(GPR_ERROR, "Unknown algorithm %s.", algorithm); + LOG(ERROR) << "Unknown algorithm " << algorithm; return nullptr; } } diff --git a/src/core/lib/security/credentials/jwt/jwt_verifier.cc b/src/core/lib/security/credentials/jwt/jwt_verifier.cc index e88b909696f..14da6885097 100644 --- a/src/core/lib/security/credentials/jwt/jwt_verifier.cc +++ b/src/core/lib/security/credentials/jwt/jwt_verifier.cc @@ -51,7 +51,6 @@ #include #include #include -#include #include #include @@ -120,8 +119,7 @@ static Json parse_json_part_from_jwt(const char* str, size_t len) { } auto json = grpc_core::JsonParse(string); if (!json.ok()) { - gpr_log(GPR_ERROR, "JSON parse error: %s", - json.status().ToString().c_str()); + LOG(ERROR) << "JSON parse error: " << json.status(); return Json(); // JSON null } return std::move(*json); @@ -129,7 +127,7 @@ static Json parse_json_part_from_jwt(const char* str, size_t len) { static const char* validate_string_field(const Json& json, const char* key) { if (json.type() != Json::Type::kString) { - gpr_log(GPR_ERROR, "Invalid %s field", key); + LOG(ERROR) << "Invalid " << key << " field"; return nullptr; } return json.string().c_str(); @@ -138,7 +136,7 @@ static const char* validate_string_field(const Json& json, const char* key) { static gpr_timespec validate_time_field(const Json& json, const char* key) { gpr_timespec result = gpr_time_0(GPR_CLOCK_REALTIME); if (json.type() != Json::Type::kNumber) { - gpr_log(GPR_ERROR, "Invalid %s field", key); + LOG(ERROR) << "Invalid " << key << " field"; return result; } result.tv_sec = strtol(json.string().c_str(), nullptr, 10); @@ -335,9 +333,9 @@ grpc_jwt_verifier_status grpc_jwt_claims_check(const grpc_jwt_claims* claims, // issued. if (grpc_jwt_issuer_email_domain(claims->iss) != nullptr && claims->sub != nullptr && strcmp(claims->iss, claims->sub) != 0) { - gpr_log(GPR_ERROR, - "Email issuer (%s) cannot assert another subject (%s) than itself.", - claims->iss, claims->sub); + LOG(ERROR) << "Email issuer (" << claims->iss + << ") cannot assert another subject (" << claims->sub + << ") than itself."; return GRPC_JWT_VERIFIER_BAD_SUBJECT; } @@ -347,9 +345,9 @@ grpc_jwt_verifier_status grpc_jwt_claims_check(const grpc_jwt_claims* claims, audience_ok = claims->aud != nullptr && strcmp(audience, claims->aud) == 0; } if (!audience_ok) { - gpr_log(GPR_ERROR, "Audience mismatch: expected %s and found %s.", - audience == nullptr ? "NULL" : audience, - claims->aud == nullptr ? "NULL" : claims->aud); + LOG(ERROR) << "Audience mismatch: expected " + << (audience == nullptr ? "NULL" : audience) << " and found " + << (claims->aud == nullptr ? "NULL" : claims->aud); return GRPC_JWT_VERIFIER_BAD_AUDIENCE; } return GRPC_JWT_VERIFIER_OK; @@ -435,8 +433,7 @@ static Json json_from_http(const grpc_http_response* response) { return Json(); // JSON null } if (response->status != 200) { - gpr_log(GPR_ERROR, "Call to http server failed with error %d.", - response->status); + LOG(ERROR) << "Call to http server failed with error " << response->status; return Json(); // JSON null } auto json = grpc_core::JsonParse( @@ -535,7 +532,7 @@ static EVP_PKEY* pkey_from_jwk(const Json& json, const char* kty) { CHECK(json.type() == Json::Type::kObject); CHECK_NE(kty, nullptr); if (strcmp(kty, "RSA") != 0) { - gpr_log(GPR_ERROR, "Unsupported key type %s.", kty); + LOG(ERROR) << "Unsupported key type " << kty; goto end; } #if OPENSSL_VERSION_NUMBER < 0x30000000L @@ -646,9 +643,8 @@ static EVP_PKEY* find_verification_key(const Json& json, const char* header_alg, return pkey_from_jwk(jkey, kty); } } - gpr_log(GPR_ERROR, - "Could not find matching key in key set for kid=%s and alg=%s", - header_kid, header_alg); + LOG(ERROR) << "Could not find matching key in key set for kid=" << header_kid + << " and alg=" << header_alg; return nullptr; } @@ -700,8 +696,8 @@ static void on_keys_retrieved(void* user_data, grpc_error_handle /*error*/) { verification_key = find_verification_key(json, ctx->header->alg, ctx->header->kid); if (verification_key == nullptr) { - gpr_log(GPR_ERROR, "Could not find verification key with kid %s.", - ctx->header->kid); + LOG(ERROR) << "Could not find verification key with kid " + << ctx->header->kid; status = GRPC_JWT_VERIFIER_KEY_RETRIEVAL_ERROR; goto end; } @@ -748,7 +744,7 @@ static void on_openid_config_retrieved(void* user_data, jwks_uri = validate_string_field(*cur, "jwks_uri"); if (jwks_uri == nullptr) goto error; if (strstr(jwks_uri, "https://") != jwks_uri) { - gpr_log(GPR_ERROR, "Invalid non https jwks_uri: %s.", jwks_uri); + LOG(ERROR) << "Invalid non https jwks_uri: " << jwks_uri; goto error; } jwks_uri += 8; diff --git a/src/core/lib/security/credentials/ssl/ssl_credentials.cc b/src/core/lib/security/credentials/ssl/ssl_credentials.cc index 9537ce1bb88..b7ab0592641 100644 --- a/src/core/lib/security/credentials/ssl/ssl_credentials.cc +++ b/src/core/lib/security/credentials/ssl/ssl_credentials.cc @@ -29,7 +29,6 @@ #include #include -#include #include #include @@ -86,9 +85,8 @@ grpc_ssl_credentials::create_security_connector( grpc_core::RefCountedPtr call_creds, const char* target, grpc_core::ChannelArgs* args) { if (config_.pem_root_certs == nullptr) { - gpr_log(GPR_ERROR, - "No root certs in config. Client-side security connector must have " - "root certs."); + LOG(ERROR) << "No root certs in config. Client-side security connector " + "must have root certs."; return nullptr; } absl::optional overridden_target_name = @@ -112,9 +110,7 @@ grpc_ssl_credentials::create_security_connector( &config_, config_.pem_root_certs, root_store_, session_cache, &factory_with_cache); if (status != GRPC_SECURITY_OK) { - gpr_log(GPR_ERROR, - "InitializeClientHandshakerFactory returned bad " - "status."); + LOG(ERROR) << "InitializeClientHandshakerFactory returned bad status."; return nullptr; } security_connector = grpc_ssl_channel_security_connector_create( @@ -197,9 +193,8 @@ grpc_security_status grpc_ssl_credentials::InitializeClientHandshakerFactory( config->pem_key_cert_pair->cert_chain != nullptr; tsi_ssl_client_handshaker_options options; if (pem_root_certs == nullptr) { - gpr_log( - GPR_ERROR, - "Handshaker factory creation failed. pem_root_certs cannot be nullptr"); + LOG(ERROR) << "Handshaker factory creation failed. pem_root_certs cannot " + "be nullptr"; return GRPC_SECURITY_ERROR; } options.pem_root_certs = pem_root_certs; @@ -218,8 +213,8 @@ grpc_security_status grpc_ssl_credentials::InitializeClientHandshakerFactory( handshaker_factory); gpr_free(options.alpn_protocols); if (result != TSI_OK) { - gpr_log(GPR_ERROR, "Handshaker factory creation failed with %s.", - tsi_result_to_string(result)); + LOG(ERROR) << "Handshaker factory creation failed with " + << tsi_result_to_string(result); return GRPC_SECURITY_ERROR; } return GRPC_SECURITY_OK; @@ -454,16 +449,14 @@ grpc_server_credentials* grpc_ssl_server_credentials_create_with_options( grpc_server_credentials* retval = nullptr; if (options == nullptr) { - gpr_log(GPR_ERROR, - "Invalid options trying to create SSL server credentials."); + LOG(ERROR) << "Invalid options trying to create SSL server credentials."; goto done; } if (options->certificate_config == nullptr && options->certificate_config_fetcher == nullptr) { - gpr_log(GPR_ERROR, - "SSL server credentials options must specify either " - "certificate config or fetcher."); + LOG(ERROR) << "SSL server credentials options must specify either " + "certificate config or fetcher."; goto done; } else if (options->certificate_config_fetcher != nullptr && options->certificate_config_fetcher->cb == nullptr) { diff --git a/src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.cc b/src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.cc index aa1dc975a5d..8b3913856fd 100644 --- a/src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.cc +++ b/src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.cc @@ -24,11 +24,11 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" #include "absl/status/status.h" #include #include -#include #include #include @@ -123,9 +123,9 @@ FileWatcherCertificateProvider::FileWatcherCertificateProvider( refresh_interval_sec_(refresh_interval_sec), distributor_(MakeRefCounted()) { if (refresh_interval_sec_ < kMinimumFileWatcherRefreshIntervalSeconds) { - gpr_log(GPR_INFO, - "FileWatcherCertificateProvider refresh_interval_sec_ set to value " - "less than minimum. Overriding configured value to minimum."); + LOG(INFO) << "FileWatcherCertificateProvider refresh_interval_sec_ set to " + "value less than minimum. Overriding configured value to " + "minimum."; refresh_interval_sec_ = kMinimumFileWatcherRefreshIntervalSeconds; } // Private key and identity cert files must be both set or both unset. @@ -284,9 +284,8 @@ FileWatcherCertificateProvider::ReadRootCertificatesFromFile( auto root_slice = LoadFile(root_cert_full_path, /*add_null_terminator=*/false); if (!root_slice.ok()) { - gpr_log(GPR_ERROR, "Reading file %s failed: %s", - root_cert_full_path.c_str(), - root_slice.status().ToString().c_str()); + LOG(ERROR) << "Reading file " << root_cert_full_path + << " failed: " << root_slice.status(); return absl::nullopt; } return std::string(root_slice->as_string_view()); @@ -316,34 +315,29 @@ FileWatcherCertificateProvider::ReadIdentityKeyCertPairFromFiles( time_t identity_key_ts_before = GetModificationTime(private_key_path.c_str()); if (identity_key_ts_before == 0) { - gpr_log( - GPR_ERROR, - "Failed to get the file's modification time of %s. Start retrying...", - private_key_path.c_str()); + LOG(ERROR) << "Failed to get the file's modification time of " + << private_key_path << ". Start retrying..."; continue; } time_t identity_cert_ts_before = GetModificationTime(identity_certificate_path.c_str()); if (identity_cert_ts_before == 0) { - gpr_log( - GPR_ERROR, - "Failed to get the file's modification time of %s. Start retrying...", - identity_certificate_path.c_str()); + LOG(ERROR) << "Failed to get the file's modification time of " + << identity_certificate_path << ". Start retrying..."; continue; } // Read the identity files. auto key_slice = LoadFile(private_key_path, /*add_null_terminator=*/false); if (!key_slice.ok()) { - gpr_log(GPR_ERROR, "Reading file %s failed: %s. Start retrying...", - private_key_path.c_str(), key_slice.status().ToString().c_str()); + LOG(ERROR) << "Reading file " << private_key_path + << " failed: " << key_slice.status() << ". Start retrying..."; continue; } auto cert_slice = LoadFile(identity_certificate_path, /*add_null_terminator=*/false); if (!cert_slice.ok()) { - gpr_log(GPR_ERROR, "Reading file %s failed: %s. Start retrying...", - identity_certificate_path.c_str(), - cert_slice.status().ToString().c_str()); + LOG(ERROR) << "Reading file " << identity_certificate_path + << " failed: " << cert_slice.status() << ". Start retrying..."; continue; } std::string private_key(key_slice->as_string_view()); @@ -354,25 +348,22 @@ FileWatcherCertificateProvider::ReadIdentityKeyCertPairFromFiles( time_t identity_key_ts_after = GetModificationTime(private_key_path.c_str()); if (identity_key_ts_before != identity_key_ts_after) { - gpr_log(GPR_ERROR, - "Last modified time before and after reading %s is not the same. " - "Start retrying...", - private_key_path.c_str()); + LOG(ERROR) << "Last modified time before and after reading " + << private_key_path << " is not the same. Start retrying..."; continue; } time_t identity_cert_ts_after = GetModificationTime(identity_certificate_path.c_str()); if (identity_cert_ts_before != identity_cert_ts_after) { - gpr_log(GPR_ERROR, - "Last modified time before and after reading %s is not the same. " - "Start retrying...", - identity_certificate_path.c_str()); + LOG(ERROR) << "Last modified time before and after reading " + << identity_certificate_path + << " is not the same. Start retrying..."; continue; } return identity_pairs; } - gpr_log(GPR_ERROR, - "All retry attempts failed. Will try again after the next interval."); + LOG(ERROR) << "All retry attempts failed. Will try again after the next " + "interval."; return absl::nullopt; } diff --git a/src/core/lib/security/credentials/tls/grpc_tls_credentials_options.cc b/src/core/lib/security/credentials/tls/grpc_tls_credentials_options.cc index 288b2debc00..bb078cc8079 100644 --- a/src/core/lib/security/credentials/tls/grpc_tls_credentials_options.cc +++ b/src/core/lib/security/credentials/tls/grpc_tls_credentials_options.cc @@ -24,7 +24,6 @@ #include "absl/log/log.h" #include -#include #include #include "src/core/lib/debug/trace.h" @@ -129,8 +128,8 @@ void grpc_tls_credentials_options_set_tls_session_key_log_file_path( // Tls session key logging is assumed to be enabled if the specified log // file is non-empty. if (path != nullptr) { - gpr_log(GPR_INFO, - "Enabling TLS session key logging with keys stored at: %s", path); + LOG(INFO) << "Enabling TLS session key logging with keys stored at: " + << path; } else { LOG(INFO) << "Disabling TLS session key logging"; } diff --git a/src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc b/src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc index 2da191aeca5..4db21c2885c 100644 --- a/src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc +++ b/src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc @@ -35,14 +35,13 @@ #include #include "absl/container/flat_hash_map.h" +#include "absl/log/log.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_join.h" #include "absl/types/span.h" -#include - #include "src/core/lib/event_engine/default_event_engine.h" #include "src/core/lib/gprpp/directory_reader.h" #include "src/core/lib/gprpp/load_file.h" @@ -130,9 +129,8 @@ absl::StatusOr> CreateStaticCrlProvider( } bool inserted = crl_map.emplace((*crl)->Issuer(), std::move(*crl)).second; if (!inserted) { - gpr_log(GPR_ERROR, - "StaticCrlProvider received multiple CRLs with the same issuer. " - "The first one in the span will be used."); + LOG(ERROR) << "StaticCrlProvider received multiple CRLs with the same " + "issuer. The first one in the span will be used."; } } StaticCrlProvider provider = StaticCrlProvider(std::move(crl_map)); diff --git a/src/core/lib/security/credentials/tls/tls_credentials.cc b/src/core/lib/security/credentials/tls/tls_credentials.cc index de88b4dd342..d825b365cb7 100644 --- a/src/core/lib/security/credentials/tls/tls_credentials.cc +++ b/src/core/lib/security/credentials/tls/tls_credentials.cc @@ -28,7 +28,6 @@ #include #include #include -#include #include #include "src/core/lib/channel/channel_args.h" @@ -63,9 +62,8 @@ bool CredentialOptionSanityCheck(grpc_tls_credentials_options* options, return false; } if (!options->crl_directory().empty() && options->crl_provider() != nullptr) { - gpr_log(GPR_ERROR, - "Setting crl_directory and crl_provider not supported. Using the " - "crl_provider."); + LOG(ERROR) << "Setting crl_directory and crl_provider not supported. Using " + "the crl_provider."; // TODO(gtcooke94) - Maybe return false here. Right now object lifetime of // this options struct is leaky if false is returned and represents a more // complex fix to handle in another PR. @@ -74,21 +72,20 @@ bool CredentialOptionSanityCheck(grpc_tls_credentials_options* options, // indicate callers are doing something wrong with the API. if (is_client && options->cert_request_type() != GRPC_SSL_DONT_REQUEST_CLIENT_CERTIFICATE) { - gpr_log(GPR_ERROR, - "Client's credentials options should not set cert_request_type."); + LOG(ERROR) + << "Client's credentials options should not set cert_request_type."; } if (!is_client && !options->verify_server_cert()) { - gpr_log(GPR_ERROR, - "Server's credentials options should not set verify_server_cert."); + LOG(ERROR) + << "Server's credentials options should not set verify_server_cert."; } // In the following conditions, there could be severe security issues. if (is_client && options->certificate_verifier() == nullptr) { // If no verifier is specified on the client side, use the hostname verifier // as default. Users who want to bypass all the verifier check should // implement an external verifier instead. - gpr_log(GPR_INFO, - "No verifier specified on the client side. Using default hostname " - "verifier"); + LOG(INFO) << "No verifier specified on the client side. Using default " + "hostname verifier"; options->set_certificate_verifier( grpc_core::MakeRefCounted()); } diff --git a/src/core/lib/security/credentials/tls/tls_utils.cc b/src/core/lib/security/credentials/tls/tls_utils.cc index df0ffa8308d..3b17ad3f7f2 100644 --- a/src/core/lib/security/credentials/tls/tls_utils.cc +++ b/src/core/lib/security/credentials/tls/tls_utils.cc @@ -22,11 +22,11 @@ #include +#include "absl/log/log.h" #include "absl/strings/ascii.h" #include "absl/strings/match.h" #include "absl/strings/str_cat.h" -#include #include namespace grpc_core { @@ -99,11 +99,11 @@ absl::string_view GetAuthPropertyValue(grpc_auth_context* context, grpc_auth_context_find_properties_by_name(context, property_name); const grpc_auth_property* prop = grpc_auth_property_iterator_next(&it); if (prop == nullptr) { - gpr_log(GPR_DEBUG, "No value found for %s property.", property_name); + VLOG(2) << "No value found for " << property_name << " property."; return ""; } if (grpc_auth_property_iterator_next(&it) != nullptr) { - gpr_log(GPR_DEBUG, "Multiple values found for %s property.", property_name); + VLOG(2) << "Multiple values found for " << property_name << " property."; return ""; } return absl::string_view(prop->value, prop->value_length); @@ -120,7 +120,7 @@ std::vector GetAuthPropertyArray(grpc_auth_context* context, prop = grpc_auth_property_iterator_next(&it); } if (values.empty()) { - gpr_log(GPR_DEBUG, "No value found for %s property.", property_name); + VLOG(2) << "No value found for " << property_name << " property."; } return values; } diff --git a/src/core/lib/security/security_connector/alts/alts_security_connector.cc b/src/core/lib/security/security_connector/alts/alts_security_connector.cc index 07bb8c5ee3f..370f21555d0 100644 --- a/src/core/lib/security/security_connector/alts/alts_security_connector.cc +++ b/src/core/lib/security/security_connector/alts/alts_security_connector.cc @@ -24,6 +24,7 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" #include "absl/status/status.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" @@ -33,7 +34,6 @@ #include #include #include -#include #include #include @@ -187,8 +187,7 @@ namespace internal { RefCountedPtr grpc_alts_auth_context_from_tsi_peer( const tsi_peer* peer) { if (peer == nullptr) { - gpr_log(GPR_ERROR, - "Invalid arguments to grpc_alts_auth_context_from_tsi_peer()"); + LOG(ERROR) << "Invalid arguments to grpc_alts_auth_context_from_tsi_peer()"; return nullptr; } // Validate certificate type. @@ -197,21 +196,21 @@ RefCountedPtr grpc_alts_auth_context_from_tsi_peer( if (cert_type_prop == nullptr || strncmp(cert_type_prop->value.data, TSI_ALTS_CERTIFICATE_TYPE, cert_type_prop->value.length) != 0) { - gpr_log(GPR_ERROR, "Invalid or missing certificate type property."); + LOG(ERROR) << "Invalid or missing certificate type property."; return nullptr; } // Check if security level exists. const tsi_peer_property* security_level_prop = tsi_peer_get_property_by_name(peer, TSI_SECURITY_LEVEL_PEER_PROPERTY); if (security_level_prop == nullptr) { - gpr_log(GPR_ERROR, "Missing security level property."); + LOG(ERROR) << "Missing security level property."; return nullptr; } // Validate RPC protocol versions. const tsi_peer_property* rpc_versions_prop = tsi_peer_get_property_by_name(peer, TSI_ALTS_RPC_VERSIONS); if (rpc_versions_prop == nullptr) { - gpr_log(GPR_ERROR, "Missing rpc protocol versions property."); + LOG(ERROR) << "Missing rpc protocol versions property."; return nullptr; } grpc_gcp_rpc_protocol_versions local_versions, peer_versions; @@ -222,21 +221,21 @@ RefCountedPtr grpc_alts_auth_context_from_tsi_peer( grpc_gcp_rpc_protocol_versions_decode(slice, &peer_versions); CSliceUnref(slice); if (!decode_result) { - gpr_log(GPR_ERROR, "Invalid peer rpc protocol versions."); + LOG(ERROR) << "Invalid peer rpc protocol versions."; return nullptr; } // TODO(unknown): Pass highest common rpc protocol version to grpc caller. bool check_result = grpc_gcp_rpc_protocol_versions_check( &local_versions, &peer_versions, nullptr); if (!check_result) { - gpr_log(GPR_ERROR, "Mismatch of local and peer rpc protocol versions."); + LOG(ERROR) << "Mismatch of local and peer rpc protocol versions."; return nullptr; } // Validate ALTS Context. const tsi_peer_property* alts_context_prop = tsi_peer_get_property_by_name(peer, TSI_ALTS_CONTEXT); if (alts_context_prop == nullptr) { - gpr_log(GPR_ERROR, "Missing alts context property."); + LOG(ERROR) << "Missing alts context property."; return nullptr; } // Create auth context. @@ -269,7 +268,7 @@ RefCountedPtr grpc_alts_auth_context_from_tsi_peer( } } if (!grpc_auth_context_peer_is_authenticated(ctx.get())) { - gpr_log(GPR_ERROR, "Invalid unauthenticated peer."); + LOG(ERROR) << "Invalid unauthenticated peer."; ctx.reset(DEBUG_LOCATION, "test"); return nullptr; } @@ -285,9 +284,8 @@ grpc_alts_channel_security_connector_create( grpc_core::RefCountedPtr request_metadata_creds, const char* target_name) { if (channel_creds == nullptr || target_name == nullptr) { - gpr_log( - GPR_ERROR, - "Invalid arguments to grpc_alts_channel_security_connector_create()"); + LOG(ERROR) + << "Invalid arguments to grpc_alts_channel_security_connector_create()"; return nullptr; } return grpc_core::MakeRefCounted( @@ -298,9 +296,8 @@ grpc_core::RefCountedPtr grpc_alts_server_security_connector_create( grpc_core::RefCountedPtr server_creds) { if (server_creds == nullptr) { - gpr_log( - GPR_ERROR, - "Invalid arguments to grpc_alts_server_security_connector_create()"); + LOG(ERROR) + << "Invalid arguments to grpc_alts_server_security_connector_create()"; return nullptr; } return grpc_core::MakeRefCounted( diff --git a/src/core/lib/security/security_connector/fake/fake_security_connector.cc b/src/core/lib/security/security_connector/fake/fake_security_connector.cc index 21aa4bc62a5..25f7bf736f7 100644 --- a/src/core/lib/security/security_connector/fake/fake_security_connector.cc +++ b/src/core/lib/security/security_connector/fake/fake_security_connector.cc @@ -25,6 +25,7 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" #include "absl/status/status.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" @@ -165,28 +166,30 @@ class grpc_fake_channel_security_connector final gpr_string_split(expected_targets_->c_str(), ";", &lbs_and_backends, &lbs_and_backends_size); if (lbs_and_backends_size > 2 || lbs_and_backends_size == 0) { - gpr_log(GPR_ERROR, "Invalid expected targets arg value: '%s'", - expected_targets_->c_str()); + LOG(ERROR) << "Invalid expected targets arg value: '" + << expected_targets_->c_str() << "'"; goto done; } if (is_lb_channel_) { if (lbs_and_backends_size != 2) { - gpr_log(GPR_ERROR, - "Invalid expected targets arg value: '%s'. Expectations for LB " - "channels must be of the form 'be1,be2,be3,...;lb1,lb2,...", - expected_targets_->c_str()); + LOG(ERROR) << "Invalid expected targets arg value: '" + << expected_targets_->c_str() + << "'. Expectations for LB channels must be of the form " + "'be1,be2,be3,...;lb1,lb2,..."; goto done; } if (!fake_check_target(target_, lbs_and_backends[1])) { - gpr_log(GPR_ERROR, "LB target '%s' not found in expected set '%s'", - target_, lbs_and_backends[1]); + LOG(ERROR) << "LB target '" << target_ + << "' not found in expected set '" << lbs_and_backends[1] + << "'"; goto done; } success = true; } else { if (!fake_check_target(target_, lbs_and_backends[0])) { - gpr_log(GPR_ERROR, "Backend target '%s' not found in expected set '%s'", - target_, lbs_and_backends[0]); + LOG(ERROR) << "Backend target '" << target_ + << "' not found in expected set '" << lbs_and_backends[0] + << "'"; goto done; } success = true; diff --git a/src/core/lib/security/security_connector/load_system_roots_supported.cc b/src/core/lib/security/security_connector/load_system_roots_supported.cc index a7dd8610de5..26d49e67cad 100644 --- a/src/core/lib/security/security_connector/load_system_roots_supported.cc +++ b/src/core/lib/security/security_connector/load_system_roots_supported.cc @@ -35,7 +35,6 @@ #include "absl/log/log.h" #include -#include #include "src/core/lib/config/config_vars.h" #include "src/core/lib/gprpp/load_file.h" @@ -81,8 +80,7 @@ void GetAbsoluteFilePath(const char* valid_file_dir, int path_len = snprintf(path_buffer, MAXPATHLEN, "%s/%s", valid_file_dir, file_entry_name); if (path_len == 0) { - gpr_log(GPR_ERROR, "failed to get absolute path for file: %s", - file_entry_name); + LOG(ERROR) << "failed to get absolute path for file: " << file_entry_name; } } } diff --git a/src/core/lib/security/security_connector/local/local_security_connector.cc b/src/core/lib/security/security_connector/local/local_security_connector.cc index b03d46d219b..b78ddbe4830 100644 --- a/src/core/lib/security/security_connector/local/local_security_connector.cc +++ b/src/core/lib/security/security_connector/local/local_security_connector.cc @@ -24,6 +24,7 @@ #include #include "absl/log/check.h" +#include "absl/log/log.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/match.h" @@ -33,7 +34,6 @@ #include #include #include -#include #include #include @@ -99,8 +99,7 @@ void local_check_peer(tsi_peer peer, grpc_endpoint* ep, absl::string_view local_addr = grpc_endpoint_get_local_address(ep); absl::StatusOr uri = grpc_core::URI::Parse(local_addr); if (!uri.ok() || !grpc_parse_uri(*uri, &resolved_addr)) { - gpr_log(GPR_ERROR, "Could not parse endpoint address: %s", - std::string(local_addr.data(), local_addr.size()).c_str()); + LOG(ERROR) << "Could not parse endpoint address: " << local_addr; } else { grpc_resolved_address addr_normalized; grpc_resolved_address* addr = @@ -269,9 +268,8 @@ grpc_local_channel_security_connector_create( grpc_core::RefCountedPtr request_metadata_creds, const grpc_core::ChannelArgs& args, const char* target_name) { if (channel_creds == nullptr || target_name == nullptr) { - gpr_log( - GPR_ERROR, - "Invalid arguments to grpc_local_channel_security_connector_create()"); + LOG(ERROR) << "Invalid arguments to " + "grpc_local_channel_security_connector_create()"; return nullptr; } // Perform sanity check on UDS address. For TCP local connection, the check @@ -283,9 +281,8 @@ grpc_local_channel_security_connector_create( if (creds->connect_type() == UDS && !absl::StartsWith(server_uri_str, GRPC_UDS_URI_PATTERN) && !absl::StartsWith(server_uri_str, GRPC_ABSTRACT_UDS_URI_PATTERN)) { - gpr_log(GPR_ERROR, - "Invalid UDS target name to " - "grpc_local_channel_security_connector_create()"); + LOG(ERROR) << "Invalid UDS target name to " + "grpc_local_channel_security_connector_create()"; return nullptr; } return grpc_core::MakeRefCounted( @@ -296,9 +293,8 @@ grpc_core::RefCountedPtr grpc_local_server_security_connector_create( grpc_core::RefCountedPtr server_creds) { if (server_creds == nullptr) { - gpr_log( - GPR_ERROR, - "Invalid arguments to grpc_local_server_security_connector_create()"); + LOG(ERROR) + << "Invalid arguments to grpc_local_server_security_connector_create()"; return nullptr; } return grpc_core::MakeRefCounted( diff --git a/src/core/lib/security/security_connector/ssl/ssl_security_connector.cc b/src/core/lib/security/security_connector/ssl/ssl_security_connector.cc index 52ec0f39534..b8fd066dab4 100644 --- a/src/core/lib/security/security_connector/ssl/ssl_security_connector.cc +++ b/src/core/lib/security/security_connector/ssl/ssl_security_connector.cc @@ -32,7 +32,6 @@ #include "absl/strings/string_view.h" #include -#include #include #include "src/core/handshaker/handshaker.h" @@ -113,8 +112,8 @@ class grpc_ssl_channel_security_connector final /*network_bio_buf_size=*/0, /*ssl_bio_buf_size=*/0, &tsi_hs); if (result != TSI_OK) { - gpr_log(GPR_ERROR, "Handshaker creation failed with error %s.", - tsi_result_to_string(result)); + LOG(ERROR) << "Handshaker creation failed with error " + << tsi_result_to_string(result); return; } // Create handshakers. @@ -205,8 +204,7 @@ class grpc_ssl_server_security_connector if (has_cert_config_fetcher()) { // Load initial credentials from certificate_config_fetcher: if (!try_fetch_ssl_server_credentials()) { - gpr_log(GPR_ERROR, - "Failed loading SSL server credentials from fetcher."); + LOG(ERROR) << "Failed loading SSL server credentials from fetcher."; return GRPC_SECURITY_ERROR; } } else { @@ -237,8 +235,8 @@ class grpc_ssl_server_security_connector &options, &server_handshaker_factory_); gpr_free(alpn_protocol_strings); if (result != TSI_OK) { - gpr_log(GPR_ERROR, "Handshaker factory creation failed with %s.", - tsi_result_to_string(result)); + LOG(ERROR) << "Handshaker factory creation failed with " + << tsi_result_to_string(result); return GRPC_SECURITY_ERROR; } } @@ -255,8 +253,8 @@ class grpc_ssl_server_security_connector server_handshaker_factory_, /*network_bio_buf_size=*/0, /*ssl_bio_buf_size=*/0, &tsi_hs); if (result != TSI_OK) { - gpr_log(GPR_ERROR, "Handshaker creation failed with error %s.", - tsi_result_to_string(result)); + LOG(ERROR) << "Handshaker creation failed with error " + << tsi_result_to_string(result); return; } // Create handshakers. @@ -300,9 +298,8 @@ class grpc_ssl_server_security_connector status = try_replace_server_handshaker_factory(certificate_config); } else { // Log error, continue using previously-loaded credentials. - gpr_log(GPR_ERROR, - "Failed fetching new server credentials, continuing to " - "use previously-loaded credentials."); + LOG(ERROR) << "Failed fetching new server credentials, continuing to " + "use previously-loaded credentials."; status = false; } @@ -319,12 +316,12 @@ class grpc_ssl_server_security_connector bool try_replace_server_handshaker_factory( const grpc_ssl_server_certificate_config* config) { if (config == nullptr) { - gpr_log(GPR_ERROR, - "Server certificate config callback returned invalid (NULL) " - "config."); + LOG(ERROR) + << "Server certificate config callback returned invalid (NULL) " + "config."; return false; } - gpr_log(GPR_DEBUG, "Using new server certificate config (%p).", config); + VLOG(2) << "Using new server certificate config (" << config << ")."; size_t num_alpn_protocols = 0; const char** alpn_protocol_strings = @@ -352,8 +349,8 @@ class grpc_ssl_server_security_connector gpr_free(alpn_protocol_strings); if (result != TSI_OK) { - gpr_log(GPR_ERROR, "Handshaker factory creation failed with %s.", - tsi_result_to_string(result)); + LOG(ERROR) << "Handshaker factory creation failed with " + << tsi_result_to_string(result); return false; } set_server_handshaker_factory(new_handshaker_factory); diff --git a/src/core/lib/security/security_connector/ssl_utils.cc b/src/core/lib/security/security_connector/ssl_utils.cc index 1e69711bf33..840602eec4e 100644 --- a/src/core/lib/security/security_connector/ssl_utils.cc +++ b/src/core/lib/security/security_connector/ssl_utils.cc @@ -36,7 +36,6 @@ #include #include #include -#include #include #include #include @@ -420,9 +419,8 @@ grpc_security_status grpc_ssl_tsi_client_handshaker_factory_init( const char* root_certs; const tsi_ssl_root_certs_store* root_store; if (pem_root_certs == nullptr && !skip_server_certificate_verification) { - gpr_log(GPR_INFO, - "No root certificates specified; use ones stored in system default " - "locations instead"); + LOG(INFO) << "No root certificates specified; use ones stored in system " + "default locations instead"; // Use default root certificates. root_certs = grpc_core::DefaultSslRootStore::GetPemRootCerts(); if (root_certs == nullptr) { @@ -459,8 +457,8 @@ grpc_security_status grpc_ssl_tsi_client_handshaker_factory_init( handshaker_factory); gpr_free(options.alpn_protocols); if (result != TSI_OK) { - gpr_log(GPR_ERROR, "Handshaker factory creation failed with %s.", - tsi_result_to_string(result)); + LOG(ERROR) << "Handshaker factory creation failed with " + << tsi_result_to_string(result); return GRPC_SECURITY_ERROR; } return GRPC_SECURITY_OK; @@ -498,8 +496,8 @@ grpc_security_status grpc_ssl_tsi_server_handshaker_factory_init( handshaker_factory); gpr_free(alpn_protocol_strings); if (result != TSI_OK) { - gpr_log(GPR_ERROR, "Handshaker factory creation failed with %s.", - tsi_result_to_string(result)); + LOG(ERROR) << "Handshaker factory creation failed with " + << tsi_result_to_string(result); return GRPC_SECURITY_ERROR; } return GRPC_SECURITY_OK; @@ -576,9 +574,8 @@ grpc_slice DefaultSslRootStore::ComputePemRootCerts() { auto slice = LoadFile(default_root_certs_path, /*add_null_terminator=*/true); if (!slice.ok()) { - gpr_log(GPR_ERROR, "error loading file %s: %s", - default_root_certs_path.c_str(), - slice.status().ToString().c_str()); + LOG(ERROR) << "error loading file " << default_root_certs_path << ": " + << slice.status(); } else { result = std::move(*slice); } @@ -604,8 +601,8 @@ grpc_slice DefaultSslRootStore::ComputePemRootCerts() { if (result.empty() && ovrd_res != GRPC_SSL_ROOTS_OVERRIDE_FAIL_PERMANENTLY) { auto slice = LoadFile(installed_roots_path, /*add_null_terminator=*/true); if (!slice.ok()) { - gpr_log(GPR_ERROR, "error loading file %s: %s", installed_roots_path, - slice.status().ToString().c_str()); + LOG(ERROR) << "error loading file " << installed_roots_path << ": " + << slice.status(); } else { result = std::move(*slice); } diff --git a/src/core/lib/security/security_connector/tls/tls_security_connector.cc b/src/core/lib/security/security_connector/tls/tls_security_connector.cc index e74d9ca2bc4..ca1098fe78d 100644 --- a/src/core/lib/security/security_connector/tls/tls_security_connector.cc +++ b/src/core/lib/security/security_connector/tls/tls_security_connector.cc @@ -33,7 +33,6 @@ #include #include #include -#include #include #include @@ -247,21 +246,18 @@ TlsChannelSecurityConnector::CreateTlsChannelSecurityConnector( const char* target_name, const char* overridden_target_name, tsi_ssl_session_cache* ssl_session_cache) { if (channel_creds == nullptr) { - gpr_log(GPR_ERROR, - "channel_creds is nullptr in " - "TlsChannelSecurityConnectorCreate()"); + LOG(ERROR) << "channel_creds is nullptr in " + "TlsChannelSecurityConnectorCreate()"; return nullptr; } if (options == nullptr) { - gpr_log(GPR_ERROR, - "options is nullptr in " - "TlsChannelSecurityConnectorCreate()"); + LOG(ERROR) << "options is nullptr in " + "TlsChannelSecurityConnectorCreate()"; return nullptr; } if (target_name == nullptr) { - gpr_log(GPR_ERROR, - "target_name is nullptr in " - "TlsChannelSecurityConnectorCreate()"); + LOG(ERROR) << "target_name is nullptr in " + "TlsChannelSecurityConnectorCreate()"; return nullptr; } return MakeRefCounted( @@ -355,8 +351,8 @@ void TlsChannelSecurityConnector::add_handshakers( /*network_bio_buf_size=*/0, /*ssl_bio_buf_size=*/0, &tsi_hs); if (result != TSI_OK) { - gpr_log(GPR_ERROR, "Handshaker creation failed with error %s.", - tsi_result_to_string(result)); + LOG(ERROR) << "Handshaker creation failed with error " + << tsi_result_to_string(result); } } // If tsi_hs is null, this will add a failing handshaker. @@ -401,9 +397,8 @@ void TlsChannelSecurityConnector::cancel_check_peer( if (it != pending_verifier_requests_.end()) { pending_verifier_request = it->second->request(); } else { - gpr_log(GPR_INFO, - "TlsChannelSecurityConnector::cancel_check_peer: no " - "corresponding pending request found"); + LOG(INFO) << "TlsChannelSecurityConnector::cancel_check_peer: no " + "corresponding pending request found"; } } if (pending_verifier_request != nullptr) { @@ -463,14 +458,12 @@ void TlsChannelSecurityConnector::TlsChannelCertificateWatcher:: void TlsChannelSecurityConnector::TlsChannelCertificateWatcher::OnError( grpc_error_handle root_cert_error, grpc_error_handle identity_cert_error) { if (!root_cert_error.ok()) { - gpr_log(GPR_ERROR, - "TlsChannelCertificateWatcher getting root_cert_error: %s", - StatusToString(root_cert_error).c_str()); + LOG(ERROR) << "TlsChannelCertificateWatcher getting root_cert_error: " + << StatusToString(root_cert_error); } if (!identity_cert_error.ok()) { - gpr_log(GPR_ERROR, - "TlsChannelCertificateWatcher getting identity_cert_error: %s", - StatusToString(identity_cert_error).c_str()); + LOG(ERROR) << "TlsChannelCertificateWatcher getting identity_cert_error: " + << StatusToString(identity_cert_error); } } @@ -566,15 +559,13 @@ TlsServerSecurityConnector::CreateTlsServerSecurityConnector( RefCountedPtr server_creds, RefCountedPtr options) { if (server_creds == nullptr) { - gpr_log(GPR_ERROR, - "server_creds is nullptr in " - "TlsServerSecurityConnectorCreate()"); + LOG(ERROR) << "server_creds is nullptr in " + "TlsServerSecurityConnectorCreate()"; return nullptr; } if (options == nullptr) { - gpr_log(GPR_ERROR, - "options is nullptr in " - "TlsServerSecurityConnectorCreate()"); + LOG(ERROR) << "options is nullptr in " + "TlsServerSecurityConnectorCreate()"; return nullptr; } return MakeRefCounted(std::move(server_creds), @@ -634,8 +625,8 @@ void TlsServerSecurityConnector::add_handshakers( server_handshaker_factory_, /*network_bio_buf_size=*/0, /*ssl_bio_buf_size=*/0, &tsi_hs); if (result != TSI_OK) { - gpr_log(GPR_ERROR, "Handshaker creation failed with error %s.", - tsi_result_to_string(result)); + LOG(ERROR) << "Handshaker creation failed with error " + << tsi_result_to_string(result); } } // If tsi_hs is null, this will add a failing handshaker. @@ -680,9 +671,8 @@ void TlsServerSecurityConnector::cancel_check_peer( if (it != pending_verifier_requests_.end()) { pending_verifier_request = it->second->request(); } else { - gpr_log(GPR_INFO, - "TlsServerSecurityConnector::cancel_check_peer: no " - "corresponding pending request found"); + LOG(INFO) << "TlsServerSecurityConnector::cancel_check_peer: no " + "corresponding pending request found"; } } if (pending_verifier_request != nullptr) { @@ -732,14 +722,12 @@ void TlsServerSecurityConnector::TlsServerCertificateWatcher:: void TlsServerSecurityConnector::TlsServerCertificateWatcher::OnError( grpc_error_handle root_cert_error, grpc_error_handle identity_cert_error) { if (!root_cert_error.ok()) { - gpr_log(GPR_ERROR, - "TlsServerCertificateWatcher getting root_cert_error: %s", - StatusToString(root_cert_error).c_str()); + LOG(ERROR) << "TlsServerCertificateWatcher getting root_cert_error: " + << StatusToString(root_cert_error); } if (!identity_cert_error.ok()) { - gpr_log(GPR_ERROR, - "TlsServerCertificateWatcher getting identity_cert_error: %s", - StatusToString(identity_cert_error).c_str()); + LOG(ERROR) << "TlsServerCertificateWatcher getting identity_cert_error: " + << StatusToString(identity_cert_error); } } diff --git a/src/core/util/http_client/httpcli_security_connector.cc b/src/core/util/http_client/httpcli_security_connector.cc index ac0c1adb257..c183f95a07e 100644 --- a/src/core/util/http_client/httpcli_security_connector.cc +++ b/src/core/util/http_client/httpcli_security_connector.cc @@ -33,7 +33,6 @@ #include #include #include -#include #include #include "src/core/handshaker/handshaker.h" @@ -96,8 +95,8 @@ class grpc_httpcli_ssl_channel_security_connector final handshaker_factory_, secure_peer_name_, /*network_bio_buf_size=*/0, /*ssl_bio_buf_size=*/0, &handshaker); if (result != TSI_OK) { - gpr_log(GPR_ERROR, "Handshaker creation failed with error %s.", - tsi_result_to_string(result)); + LOG(ERROR) << "Handshaker creation failed with error " + << tsi_result_to_string(result); } } handshake_mgr->Add(SecurityHandshakerCreate(handshaker, this, args)); @@ -150,8 +149,7 @@ httpcli_ssl_channel_security_connector_create( const char* pem_root_certs, const tsi_ssl_root_certs_store* root_store, const char* secure_peer_name) { if (secure_peer_name != nullptr && pem_root_certs == nullptr) { - gpr_log(GPR_ERROR, - "Cannot assert a secure peer name without a trust root."); + LOG(ERROR) << "Cannot assert a secure peer name without a trust root."; return nullptr; } RefCountedPtr c = @@ -159,8 +157,8 @@ httpcli_ssl_channel_security_connector_create( secure_peer_name == nullptr ? nullptr : gpr_strdup(secure_peer_name)); tsi_result result = c->InitHandshakerFactory(pem_root_certs, root_store); if (result != TSI_OK) { - gpr_log(GPR_ERROR, "Handshaker factory creation failed with %s.", - tsi_result_to_string(result)); + LOG(ERROR) << "Handshaker factory creation failed with " + << tsi_result_to_string(result); return nullptr; } return c;