Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

server: introduce ServerMetricRecorder API and move per-call reporting from a C++ interceptor to a C-core filter #32106

Merged
merged 44 commits into from
Feb 1, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
e0b7519
backend metric sampling
yousukseung Jan 13, 2023
4147529
Comments addressed.
yousukseung Jan 19, 2023
8d0e1bf
Merge branch 'master' into test
yousukseung Jan 20, 2023
29b4da6
More comments addressed.
yousukseung Jan 20, 2023
46913b8
Pushing changes left behind locally.
yousukseung Jan 20, 2023
122e975
Removed empty lines
yousukseung Jan 20, 2023
a3099bd
Update OrcaService to use ServerMetricRecorder (no named metrics yet)
yousukseung Jan 21, 2023
d75e814
Comments addressed.
yousukseung Jan 21, 2023
816a8af
More comments addressed
yousukseung Jan 21, 2023
7855ea4
More comments addressed.
yousukseung Jan 21, 2023
1448113
Comments fixed
yousukseung Jan 24, 2023
0ca1a8b
Merge branch 'backend_metric_sampling' into merged
yousukseung Jan 24, 2023
8ac753f
Merge remote-tracking branch 'upstream/master' into backend_metric_sa…
yousukseung Jan 25, 2023
4221d88
Comments addressed.
yousukseung Jan 25, 2023
eca48d4
Test fixed
yousukseung Jan 26, 2023
604df1b
make seq returned always up-to-date
yousukseung Jan 26, 2023
0da8bfe
skip atomic load when not cached
yousukseung Jan 26, 2023
8f586e7
Fixed ABSL_GUARDED_BY
yousukseung Jan 26, 2023
d0ce562
Comments addressed except client_lb_end2end_test
yousukseung Jan 27, 2023
bfc0dc0
test updated
yousukseung Jan 27, 2023
2367bf6
Comments addressed
yousukseung Jan 29, 2023
b471024
BUILD fix.
yousukseung Jan 30, 2023
5669e86
BackendMetricDataState moved to a separate header
yousukseung Jan 30, 2023
33aa915
comments addressed
yousukseung Jan 30, 2023
dddc849
Fixed clang and buildifier errors
yousukseung Jan 30, 2023
d58771f
More sanity check errors fixed.
yousukseung Jan 30, 2023
4c2c860
Merge remote-tracking branch 'upstream/master' into backend_metric_sa…
yousukseung Jan 30, 2023
90f6ec6
Fixed xds tests
yousukseung Jan 30, 2023
c21d62a
Ran generate_projects.sh
yousukseung Jan 30, 2023
a286a88
Comments addressed
yousukseung Jan 31, 2023
42be5de
comments addressed.
yousukseung Jan 31, 2023
c327577
Merge remote-tracking branch 'upstream/master' into backend_metric_sa…
yousukseung Jan 31, 2023
63afafb
generate project
yousukseung Jan 31, 2023
cc228d1
Build fixed
yousukseung Jan 31, 2023
886494d
generate project
yousukseung Jan 31, 2023
2b8e52e
sanity check errors fixed
yousukseung Jan 31, 2023
e22b336
test fixed
yousukseung Jan 31, 2023
21d8bf4
Backup poller period override moved to main()
yousukseung Jan 31, 2023
198eeba
Also move cfstream override
yousukseung Jan 31, 2023
7616eb2
Clang fixes, sanitize
yousukseung Feb 1, 2023
d96d88b
Merge remote-tracking branch 'upstream/master' into backend_metric_sa…
yousukseung Feb 1, 2023
a1b77c5
generate_projects.sh
yousukseung Feb 1, 2023
4632a63
portable print format fix
yousukseung Feb 1, 2023
35c9dc0
Removed outdated comment
yousukseung Feb 1, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -1972,6 +1972,7 @@ grpc_cc_library(
],
visibility = ["@grpc:public"],
deps = [
"gpr",
"grpc_trace",
"//src/core:grpc_backend_metric_data",
],
Expand Down Expand Up @@ -1999,11 +2000,13 @@ grpc_cc_library(
"gpr",
"grpc++",
"grpc_base",
"grpcpp_server_metric_recorder",
"protobuf_duration_upb",
"ref_counted_ptr",
"xds_orca_service_upb",
"xds_orca_upb",
"//src/core:default_event_engine",
"//src/core:grpc_backend_metric_data",
"//src/core:ref_counted",
"//src/core:time",
],
Expand Down
62 changes: 24 additions & 38 deletions include/grpcpp/ext/call_metric_recorder.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,63 +28,49 @@
#include <grpcpp/impl/sync.h>
#include <grpcpp/support/slice.h>

namespace grpc_core {
class Arena;
struct BackendMetricData;
} // namespace grpc_core

namespace grpc {
class ServerBuilder;

namespace experimental {

// Registers the per-rpc orca load reporter into the \a ServerBuilder.
// Once this is done, the server will automatically send the load metrics
// after each RPC as they were reported. In order to report load metrics,
// call the \a ServerContext::ExperimentalGetCallMetricRecorder() method to
// retrieve the recorder for the current call.
void EnableCallMetricRecording(ServerBuilder*);

/// Records call metrics for the purpose of load balancing.
/// During an RPC, call \a ServerContext::ExperimentalGetCallMetricRecorder()
/// method to retrive the recorder for the current call.
class CallMetricRecorder {
public:
virtual ~CallMetricRecorder() = default;

// Records a call metric measurement for CPU utilization.
// Multiple calls to this method will override the stored value.
// Values outside of the valid range [0, 1] are ignored.
/// Records a call metric measurement for CPU utilization.
/// Multiple calls to this method will override the stored value.
/// Values outside of the valid range [0, 1] are ignored.
virtual CallMetricRecorder& RecordCpuUtilizationMetric(double value) = 0;

// Records a call metric measurement for memory utilization.
// Multiple calls to this method will override the stored value.
// Values outside of the valid range [0, 1] are ignored.
/// Records a call metric measurement for memory utilization.
/// Multiple calls to this method will override the stored value.
/// Values outside of the valid range [0, 1] are ignored.
virtual CallMetricRecorder& RecordMemoryUtilizationMetric(double value) = 0;

// Records a call metric measurement for queries per second.
// Multiple calls to this method will override the stored value.
// Values outside of the valid range [0, infy) are ignored.
/// Records a call metric measurement for queries per second.
/// Multiple calls to this method will override the stored value.
/// Values outside of the valid range [0, infy) are ignored.
virtual CallMetricRecorder& RecordQpsMetric(double value) = 0;

// Records a call metric measurement for utilization.
// Multiple calls to this method with the same name will
// override the corresponding stored value. The lifetime of the
// name string needs to be longer than the lifetime of the RPC
// itself, since it's going to be sent as trailers after the RPC
// finishes. It is assumed the strings are common names that
// are global constants.
// Values outside of the valid range [0, 1] are ignored.
/// Records a call metric measurement for utilization.
/// Multiple calls to this method with the same name will
/// override the corresponding stored value. The lifetime of the
/// name string needs to be longer than the lifetime of the RPC
/// itself, since it's going to be sent as trailers after the RPC
/// finishes. It is assumed the strings are common names that
/// are global constants.
/// Values outside of the valid range [0, 1] are ignored.
virtual CallMetricRecorder& RecordUtilizationMetric(string_ref name,
double value) = 0;

// Records a call metric measurement for request cost.
// Multiple calls to this method with the same name will
// override the corresponding stored value. The lifetime of the
// name string needs to be longer than the lifetime of the RPC
// itself, since it's going to be sent as trailers after the RPC
// finishes. It is assumed the strings are common names that
// are global constants.
/// Records a call metric measurement for request cost.
/// Multiple calls to this method with the same name will
/// override the corresponding stored value. The lifetime of the
/// name string needs to be longer than the lifetime of the RPC
/// itself, since it's going to be sent as trailers after the RPC
/// finishes. It is assumed the strings are common names that
/// are global constants.
virtual CallMetricRecorder& RecordRequestCostMetric(string_ref name,
double value) = 0;
};
Expand Down
31 changes: 5 additions & 26 deletions include/grpcpp/ext/orca_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "absl/time/time.h"
#include "absl/types/optional.h"

#include <grpcpp/ext/server_metric_recorder.h>
#include <grpcpp/impl/service_type.h>
#include <grpcpp/impl/sync.h>
#include <grpcpp/server_builder.h>
Expand All @@ -48,38 +49,16 @@ class OrcaService : public Service {
}
};

explicit OrcaService(Options options);

// Sets or removes the CPU utilization value to be reported to clients.
void SetCpuUtilization(double cpu_utilization);
void DeleteCpuUtilization();

// Sets of removes the memory utilization value to be reported to clients.
void SetMemoryUtilization(double memory_utilization);
void DeleteMemoryUtilization();

// Sets of removes the QPS value to be reported to clients.
void SetQps(double qps);
void DeleteQps();

// Sets or removed named utilization values to be reported to clients.
void SetNamedUtilization(std::string name, double utilization);
markdroth marked this conversation as resolved.
Show resolved Hide resolved
void DeleteNamedUtilization(const std::string& name);
void SetAllNamedUtilization(std::map<std::string, double> named_utilization);
OrcaService(const ServerMetricRecorder& server_metric_recorder,
markdroth marked this conversation as resolved.
Show resolved Hide resolved
Options options);

private:
class Reactor;

Slice GetOrCreateSerializedResponse();
Slice CreateSerializedResponse();

const ServerMetricRecorder& server_metric_recorder_;
const absl::Duration min_report_duration_;

grpc::internal::Mutex mu_;
double cpu_utilization_ ABSL_GUARDED_BY(&mu_) = -1;
double memory_utilization_ ABSL_GUARDED_BY(&mu_) = -1;
double qps_ ABSL_GUARDED_BY(&mu_) = -1;
std::map<std::string, double> named_utilization_ ABSL_GUARDED_BY(&mu_);
absl::optional<Slice> response_slice_ ABSL_GUARDED_BY(&mu_);
markdroth marked this conversation as resolved.
Show resolved Hide resolved
};

} // namespace experimental
Expand Down
41 changes: 20 additions & 21 deletions include/grpcpp/ext/server_metric_recorder.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,54 +27,53 @@
#include "absl/types/optional.h"

namespace grpc_core {
class Arena;
struct BackendMetricData;
} // namespace grpc_core

namespace grpc {
class BackendMetricState;
class ServerBuilder;

namespace experimental {

/// Records server wide metrics for the purpose of load balancing.
/// Records server wide metrics to be reported to the client.
/// Server implementation creates an instance and reports server metrics to it,
/// and then passes it to
/// ServerBuilder::experimental_type::EnableCallMetricRecording or
/// experimental::OrcaService that read metrics to include in the report.
class ServerMetricRecorder {
public:
// Records the server CPU utilization in the range [0, 1].
// Values outside of the valid range are rejected.
// Overrides the stored value when called again with a valid value.
/// Records the server CPU utilization in the range [0, 1].
/// Values outside of the valid range are rejected.
/// Overrides the stored value when called again with a valid value.
void SetCpuUtilization(double value);
// Records the server memory utilization in the range [0, 1].
// Values outside of the valid range are rejected.
// Overrides the stored value when called again with a valid value.
void SetMemUtilization(double value);
// Records number of queries per second to the server in the range [0, infy).
// Values outside of the valid range are rejected.
// Overrides the stored value when called again with a valid value.
/// Records the server memory utilization in the range [0, 1].
/// Values outside of the valid range are rejected.
/// Overrides the stored value when called again with a valid value.
void SetMemoryUtilization(double value);
/// Records number of queries per second to the server in the range [0, infy).
/// Values outside of the valid range are rejected.
/// Overrides the stored value when called again with a valid value.
void SetQps(double value);

// Clears the server CPU utilization if recorded.
/// Clears the server CPU utilization if recorded.
void ClearCpuUtilization();
// Clears the server memory utilization if recorded.
void ClearMemUtilization();
// Clears number of queries per second to the server if recorded.
/// Clears the server memory utilization if recorded.
void ClearMemoryUtilization();
/// Clears number of queries per second to the server if recorded.
void ClearQps();

private:
// To access GetMetrics().
friend class grpc::BackendMetricState;
friend class OrcaService;

// Only populates fields in `data` that this has recorded metrics.
void GetMetrics(grpc_core::BackendMetricData* data);
grpc_core::BackendMetricData GetMetrics() const;

// Defaults to -1.0 (unset).
std::atomic<double> cpu_utilization_{-1.0};
std::atomic<double> mem_utilization_{-1.0};
std::atomic<double> qps_{-1.0};

// To access GetMetrics().
friend class grpc::BackendMetricState;
};

} // namespace experimental
Expand Down
16 changes: 7 additions & 9 deletions include/grpcpp/server_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ class ExternalConnectionAcceptorImpl;
class CallbackGenericService;

namespace experimental {
class OrcaServerInterceptorFactory;
// EXPERIMENTAL API:
// Interface for a grpc server to build transports with connections created out
// of band.
Expand Down Expand Up @@ -283,13 +282,13 @@ class ServerBuilder {
std::shared_ptr<experimental::AuthorizationPolicyProviderInterface>
provider);

// Enables per-call load reporting. The server will automatically send the
// load metrics after each RPC. The caller can report load metrics for the
// current call to what ServerContext::ExperimentalGetCallMetricRecorder()
// returns. The server merges metrics from the optional
// server_metric_recorder when provided where the call metric recorder take
// a higher precedence. The caller owns and must ensure the server metric
// recorder outlives the server.
/// Enables per-call load reporting. The server will automatically send the
/// load metrics after each RPC. The caller can report load metrics for the
/// current call to what ServerContext::ExperimentalGetCallMetricRecorder()
/// returns. The server merges metrics from the optional
/// server_metric_recorder when provided where the call metric recorder take
/// a higher precedence. The caller owns and must ensure the server metric
/// recorder outlives the server.
void EnableCallMetricRecording(
experimental::ServerMetricRecorder* server_metric_recorder = nullptr);

Expand Down Expand Up @@ -365,7 +364,6 @@ class ServerBuilder {

private:
friend class grpc::testing::ServerBuilderPluginTest;
friend class grpc::experimental::OrcaServerInterceptorFactory;

struct SyncServerSettings {
SyncServerSettings()
Expand Down
4 changes: 0 additions & 4 deletions include/grpcpp/server_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,6 @@ struct grpc_metadata;
struct grpc_call;
struct census_context;

namespace grpc_core {
struct BackendMetricData;
} // namespace grpc_core

namespace grpc {
template <class W, class R>
class ServerAsyncReader;
Expand Down
8 changes: 2 additions & 6 deletions src/core/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -3605,15 +3605,11 @@ grpc_cc_library(
grpc_cc_library(
name = "grpc_backend_metric_data",
hdrs = [
"ext/filters/backend_metrics/backend_metric_data.h",
"ext/filters/client_channel/lb_policy/backend_metric_data.h",
],
external_deps = ["absl/strings"],
language = "c++",
deps = [
"//:gpr",
"//:gpr_platform",
"//:grpc_trace",
],
deps = [ "//:gpr_platform", ],
)

grpc_cc_library(
Expand Down
60 changes: 0 additions & 60 deletions src/core/ext/filters/backend_metrics/backend_metric_data.h

This file was deleted.

15 changes: 14 additions & 1 deletion src/core/ext/filters/backend_metrics/backend_metric_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#include "upb/upb.hpp"
#include "xds/data/orca/v3/orca_load_report.upb.h"

#include "src/core/ext/filters/backend_metrics/backend_metric_data.h"
#include "src/core/ext/filters/client_channel/lb_policy/backend_metric_data.h"
#include "src/core/lib/debug/trace.h"
#include "src/core/lib/promise/map.h"
#include "src/core/lib/resource_quota/arena.h"
Expand Down Expand Up @@ -92,4 +92,17 @@ ArenaPromise<ServerMetadataHandle> BackendMetricFilter::MakeCallPromise(
}));
}

static bool maybe_prepend_grpc_backend_metric_filter(
markdroth marked this conversation as resolved.
Show resolved Hide resolved
grpc_core::ChannelStackBuilder* builder) {
if (builder->channel_args().Contains(GRPC_ARG_SERVER_CALL_METRIC_RECORDING)) {
builder->PrependFilter(&grpc_core::BackendMetricFilter::kFilter);
}
return true;
}

void RegisterBackendMetricFilter(CoreConfiguration::Builder* builder) {
builder->channel_init()->RegisterStage(
GRPC_SERVER_CHANNEL, INT_MAX, maybe_prepend_grpc_backend_metric_filter);
}

} // namespace grpc_core
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
#include "absl/status/statusor.h"
#include "absl/types/optional.h"

#include "src/core/ext/filters/backend_metrics/backend_metric_data.h"
#include "src/core/ext/filters/backend_metrics/backend_metric_provider.h"
#include "src/core/ext/filters/client_channel/lb_policy/backend_metric_data.h"
#include "src/core/lib/channel/channel_args.h"
#include "src/core/lib/channel/promise_based_filter.h"

Expand Down