Skip to content

Commit 1eac120

Browse files
author
mvkab
committed
feat grpc: add client QoS configuration validation
Add validation for methods in client QoS config to catch typos and invalid configs early: \- validate RPC path format (\`package.ServiceName/MethodName\`) \- check method existence against service metadata Add \`grpc\_client\_qos\_invalid\_configuration\` alert that fires on invalid methods and clears when config is fixed. commit_hash:ceeecd02ed6fc69552fed8040f61767acc5bcbf9
1 parent 1da1995 commit 1eac120

19 files changed

+523
-6
lines changed

.mapping.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2340,6 +2340,7 @@
23402340
"grpc/include/userver/ugrpc/client/impl/client_data.hpp":"taxi/uservices/userver/grpc/include/userver/ugrpc/client/impl/client_data.hpp",
23412341
"grpc/include/userver/ugrpc/client/impl/client_data_accessor.hpp":"taxi/uservices/userver/grpc/include/userver/ugrpc/client/impl/client_data_accessor.hpp",
23422342
"grpc/include/userver/ugrpc/client/impl/client_internals.hpp":"taxi/uservices/userver/grpc/include/userver/ugrpc/client/impl/client_internals.hpp",
2343+
"grpc/include/userver/ugrpc/client/impl/client_qos_errors_reporter.hpp":"taxi/uservices/userver/grpc/include/userver/ugrpc/client/impl/client_qos_errors_reporter.hpp",
23432344
"grpc/include/userver/ugrpc/client/impl/codegen_declarations.hpp":"taxi/uservices/userver/grpc/include/userver/ugrpc/client/impl/codegen_declarations.hpp",
23442345
"grpc/include/userver/ugrpc/client/impl/codegen_definitions.hpp":"taxi/uservices/userver/grpc/include/userver/ugrpc/client/impl/codegen_definitions.hpp",
23452346
"grpc/include/userver/ugrpc/client/impl/compat/channel_arguments_builder.hpp":"taxi/uservices/userver/grpc/include/userver/ugrpc/client/impl/compat/channel_arguments_builder.hpp",
@@ -2485,6 +2486,9 @@
24852486
"grpc/src/ugrpc/client/impl/client_factory_config.cpp":"taxi/uservices/userver/grpc/src/ugrpc/client/impl/client_factory_config.cpp",
24862487
"grpc/src/ugrpc/client/impl/client_factory_config.hpp":"taxi/uservices/userver/grpc/src/ugrpc/client/impl/client_factory_config.hpp",
24872488
"grpc/src/ugrpc/client/impl/client_qos.cpp":"taxi/uservices/userver/grpc/src/ugrpc/client/impl/client_qos.cpp",
2489+
"grpc/src/ugrpc/client/impl/client_qos_errors_reporter.cpp":"taxi/uservices/userver/grpc/src/ugrpc/client/impl/client_qos_errors_reporter.cpp",
2490+
"grpc/src/ugrpc/client/impl/client_qos_validation.cpp":"taxi/uservices/userver/grpc/src/ugrpc/client/impl/client_qos_validation.cpp",
2491+
"grpc/src/ugrpc/client/impl/client_qos_validation.hpp":"taxi/uservices/userver/grpc/src/ugrpc/client/impl/client_qos_validation.hpp",
24882492
"grpc/src/ugrpc/client/impl/compat/channel_arguments_builder.cpp":"taxi/uservices/userver/grpc/src/ugrpc/client/impl/compat/channel_arguments_builder.cpp",
24892493
"grpc/src/ugrpc/client/impl/compat/retry_policy.cpp":"taxi/uservices/userver/grpc/src/ugrpc/client/impl/compat/retry_policy.cpp",
24902494
"grpc/src/ugrpc/client/impl/compat/retry_policy.hpp":"taxi/uservices/userver/grpc/src/ugrpc/client/impl/compat/retry_policy.hpp",
@@ -2605,6 +2609,7 @@
26052609
"grpc/tests/client_factory_test.cpp":"taxi/uservices/userver/grpc/tests/client_factory_test.cpp",
26062610
"grpc/tests/client_middleware_hooks_test.cpp":"taxi/uservices/userver/grpc/tests/client_middleware_hooks_test.cpp",
26072611
"grpc/tests/client_qos_test.cpp":"taxi/uservices/userver/grpc/tests/client_qos_test.cpp",
2612+
"grpc/tests/client_qos_validation_test.cpp":"taxi/uservices/userver/grpc/tests/client_qos_validation_test.cpp",
26082613
"grpc/tests/compat/channel_arguments_builder_test.cpp":"taxi/uservices/userver/grpc/tests/compat/channel_arguments_builder_test.cpp",
26092614
"grpc/tests/congestion_control_test.cpp":"taxi/uservices/userver/grpc/tests/congestion_control_test.cpp",
26102615
"grpc/tests/datetime_utils_test.cpp":"taxi/uservices/userver/grpc/tests/datetime_utils_test.cpp",

core/include/userver/components/statistics_storage.hpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ class StatisticsStorage final : public RawComponentBase {
4747

4848
utils::statistics::MetricsStoragePtr GetMetricsStorage() { return metrics_storage_; }
4949

50+
utils::statistics::MetricsStorage& GetMetricsStorageRef() {
51+
UASSERT(metrics_storage_ != nullptr);
52+
return *metrics_storage_;
53+
}
54+
5055
static yaml_config::Schema GetStaticConfigSchema();
5156

5257
private:

grpc/functional_tests/metrics/tests/static/metrics_values.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
alerts.grpc_client_qos_invalid_configuration: GAUGE
12
grpc.client.by-destination.abandoned-error: grpc_destination=samples.api.GreeterService/SayHello, grpc_destination_full=greeter/samples.api.GreeterService/SayHello, grpc_method=SayHello, grpc_service=samples.api.GreeterService RATE
23
grpc.client.by-destination.active: grpc_destination=samples.api.GreeterService/SayHello, grpc_destination_full=greeter/samples.api.GreeterService/SayHello, grpc_method=SayHello, grpc_service=samples.api.GreeterService GAUGE
34
grpc.client.by-destination.cancelled-by-deadline-propagation: grpc_destination=samples.api.GreeterService/SayHello, grpc_destination_full=greeter/samples.api.GreeterService/SayHello, grpc_method=SayHello, grpc_service=samples.api.GreeterService RATE

grpc/functional_tests/metrics/tests/test_metrics.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,12 @@ def _normalize_metrics(metrics: str) -> str:
1616
def _drop_non_grpc_metrics(metrics: list[str]) -> list[str]:
1717
result = []
1818
for line in metrics:
19-
if line.startswith(('grpc.server', 'grpc.client')):
20-
result.append(line)
19+
try:
20+
path = line.split(':')[0]
21+
if 'grpc' in path:
22+
result.append(line)
23+
except IndexError:
24+
continue
2125

2226
return result
2327

grpc/include/userver/ugrpc/client/client_factory.hpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include <userver/ugrpc/client/client_settings.hpp>
1515
#include <userver/ugrpc/client/fwd.hpp>
1616
#include <userver/ugrpc/client/impl/client_internals.hpp>
17+
#include <userver/ugrpc/client/impl/client_qos_errors_reporter.hpp>
1718
#include <userver/ugrpc/client/middlewares/pipeline.hpp>
1819
#include <userver/ugrpc/impl/static_service_metadata.hpp>
1920

@@ -58,6 +59,7 @@ class ClientFactory final {
5859
impl::MiddlewarePipelineCreator& middleware_pipeline_creator,
5960
ugrpc::impl::CompletionQueuePoolBase& completion_queues,
6061
ugrpc::impl::StatisticsStorage& statistics_storage,
62+
utils::statistics::MetricsStorage& metrics_storage,
6163
testsuite::GrpcControl& testsuite_grpc,
6264
dynamic_config::Source config_source
6365
);
@@ -74,6 +76,7 @@ class ClientFactory final {
7476
impl::MiddlewarePipelineCreator& middleware_pipeline_creator_;
7577
ugrpc::impl::CompletionQueuePoolBase& completion_queues_;
7678
ugrpc::impl::StatisticsStorage& client_statistics_storage_;
79+
impl::ClientQosErrorsReporter client_qos_errors_reporter_;
7780
const dynamic_config::Source config_source_;
7881
testsuite::GrpcControl& testsuite_grpc_;
7982
};

grpc/include/userver/ugrpc/client/impl/client_data.hpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include <userver/ugrpc/client/client_qos.hpp>
1717
#include <userver/ugrpc/client/impl/channel_argument_utils.hpp>
1818
#include <userver/ugrpc/client/impl/client_internals.hpp>
19+
#include <userver/ugrpc/client/impl/client_qos_errors_reporter.hpp>
1920
#include <userver/ugrpc/client/impl/compat/channel_arguments_builder.hpp>
2021
#include <userver/ugrpc/client/impl/stub_handle.hpp>
2122
#include <userver/ugrpc/client/impl/stub_state.hpp>
@@ -153,7 +154,12 @@ class ClientData final {
153154

154155
template <typename Service>
155156
void OnConfigUpdate(const dynamic_config::Snapshot& config) {
156-
auto client_qos = internals_.qos ? config[*internals_.qos] : ClientQos{};
157+
const auto& client_qos = internals_.qos ? config[*internals_.qos] : ClientQos{};
158+
159+
if (metadata_.has_value() && internals_.qos) {
160+
internals_.client_qos_error_reporter
161+
.ValidateAndReportClientQosErrors(client_qos, internals_.qos->GetName(), *metadata_);
162+
}
157163

158164
std::string target = internals_.endpoint;
159165

grpc/include/userver/ugrpc/client/impl/client_internals.hpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <userver/ugrpc/client/middlewares/fwd.hpp>
1414
#include <userver/ugrpc/client/proxy_settings.hpp>
1515
#include <userver/ugrpc/client/retry_config.hpp>
16+
#include <userver/utils/statistics/fwd.hpp>
1617

1718
USERVER_NAMESPACE_BEGIN
1819

@@ -23,13 +24,16 @@ class CompletionQueuePoolBase;
2324

2425
namespace ugrpc::client::impl {
2526

27+
class ClientQosErrorsReporter;
28+
2629
/// Contains all non-code-generated dependencies for creating a gRPC client
2730
struct ClientInternals final {
2831
std::string client_name;
2932
std::string endpoint;
3033
Middlewares middlewares;
3134
ugrpc::impl::CompletionQueuePoolBase& completion_queues;
3235
ugrpc::impl::StatisticsStorage& statistics_storage;
36+
ClientQosErrorsReporter& client_qos_error_reporter;
3337
dynamic_config::Source config_source;
3438
testsuite::GrpcControl& testsuite_grpc;
3539
const dynamic_config::Key<ClientQos>* qos{nullptr};
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
#pragma once
2+
3+
#include <userver/ugrpc/impl/static_service_metadata.hpp>
4+
#include <userver/utils/statistics/metrics_storage.hpp>
5+
6+
USERVER_NAMESPACE_BEGIN
7+
8+
namespace ugrpc::client {
9+
struct ClientQos;
10+
} // namespace ugrpc::client
11+
12+
namespace ugrpc::client::impl {
13+
14+
class ClientQosErrorsReporter final {
15+
public:
16+
explicit ClientQosErrorsReporter(utils::statistics::MetricsStorage& metrics_storage);
17+
18+
/// @brief Validates client QOS configuration and synchronizes validity alert state.
19+
/// @details Performs validation using @ref ValidateClientQosMethodsExistence and:
20+
/// - Fires `grpc_client_qos_invalid_configuration` alert if errors are found
21+
/// - Stops the alert if configuration is valid
22+
/// Logs warnings for each detected issue.
23+
void ValidateAndReportClientQosErrors(
24+
const ClientQos& client_qos,
25+
std::string_view config_name,
26+
const ugrpc::impl::StaticServiceMetadata& metadata
27+
);
28+
29+
private:
30+
utils::statistics::MetricsStorage& metrics_storage_;
31+
};
32+
33+
} // namespace ugrpc::client::impl
34+
35+
USERVER_NAMESPACE_END

grpc/include/userver/ugrpc/tests/service.hpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <userver/dynamic_config/storage_mock.hpp>
1212
#include <userver/dynamic_config/test_helpers.hpp>
1313
#include <userver/testsuite/grpc_control.hpp>
14+
#include <userver/utils/statistics/metrics_storage.hpp>
1415
#include <userver/utils/statistics/storage.hpp>
1516

1617
#include <userver/ugrpc/client/client_factory.hpp>
@@ -92,6 +93,8 @@ class ServiceBase {
9293
server::ServiceConfig MakeServiceConfig();
9394

9495
utils::statistics::Storage statistics_storage_;
96+
utils::statistics::MetricsStorage metrics_storage_;
97+
std::vector<utils::statistics::Entry> metrics_storage_registration_;
9598
dynamic_config::StorageMock config_storage_;
9699
std::optional<std::string> unix_socket_path_;
97100
server::Server server_;

grpc/include/userver/ugrpc/tests/standalone_client.hpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <userver/dynamic_config/test_helpers.hpp>
99
#include <userver/engine/io/sockaddr.hpp>
1010
#include <userver/testsuite/grpc_control.hpp>
11+
#include <userver/utils/statistics/metrics_storage.hpp>
1112
#include <userver/utils/statistics/storage.hpp>
1213

1314
#include <userver/ugrpc/client/client_factory.hpp>
@@ -44,6 +45,8 @@ class StandaloneClientFactory final {
4445

4546
private:
4647
utils::statistics::Storage statistics_storage_;
48+
utils::statistics::MetricsStorage metrics_storage_;
49+
std::vector<utils::statistics::Entry> metrics_storage_registration_;
4750
ugrpc::impl::StatisticsStorage
4851
client_statistics_storage_{statistics_storage_, ugrpc::impl::StatisticsDomain::kClient};
4952
dynamic_config::StorageMock config_storage_{dynamic_config::MakeDefaultStorage({})};

0 commit comments

Comments
 (0)