feat(new_metrics): support server_stat command showing some important server-level metrics (part 1) #2085
Conversation
empiredan
changed the title
src/shell/command_helper.h
Outdated
| // Adapt the result returned by `get_metrics` into the structure that could be processed by | ||
| // `remote_command`. | ||
| template <typename... Args> | ||
| inline std::pair<bool, std::string> process_get_metrics_result(const dsn::http_result &result, |
There was a problem hiding this comment.
It seems the return type can be replaced by error_s or absl::Status.
There was a problem hiding this comment.
OK, use error_s instead.
| @@ -101,7 +101,7 @@ static command_executor commands[] = { | |||
| "get the node status for this cluster", | |||
| "[-d|--detailed] [-j|--json] [-r|--resolve_ip] [-u|--resource_usage] " | |||
| "[-o|--output file_name] [-s|--status all|alive|unalive] [-q|--qps] " | |||
| "[-t|--sample_interval_ms num]", | |||
| "[-i|--sample_interval_ms num]", | |||
There was a problem hiding this comment.
It should be mentioned in "Behavior changes" in the PR description for this PR.
There was a problem hiding this comment.
OK, the options sample_interval_ms was introduced recently and never released before.
| dsn::json::json_forwarder<meta_server_stats>::encode(stats).to_string()); | ||
| } | ||
|
|
||
| struct replica_server_stats |
There was a problem hiding this comment.
It's the same to meta_server_stats, how about using a unify one?
There was a problem hiding this comment.
Actually they are different, since more metrics would be introduced later.
| dsn::metric_filters filters; | ||
| filters.with_metric_fields = {dsn::kMetricNameField, dsn::kMetricSingleValueField}; | ||
| filters.entity_types = {"server"}; | ||
| filters.entity_metrics = {"virtual_mem_usage_mb", "resident_mem_usage_mb"}; |
There was a problem hiding this comment.
What about defining some static const strings for these metric names?
There was a problem hiding this comment.
These names of metrics have been defined, initialized and set as following statements, thus I think we could just keep them in the format of strings.
METRIC_DEFINE_gauge_int64(server,
virtual_mem_usage_mb,
dsn::metric_unit::kMegaBytes,
"The total amount of virtual memory usage in MB");
METRIC_VAR_INIT_server(virtual_mem_usage_mb)
METRIC_VAR_SET(virtual_mem_usage_mb, virt_mb);
empiredan
force-pushed
the
new-metrics-server-stat-mem
branch
from
ba7f360 to
09f2f8b
Compare
empiredan
force-pushed
the
new-metrics-server-stat-mem
branch
from
a70559b to
b12d717
Compare
empiredan
changed the title
empiredan
changed the title
As the 1st part that support server_stat command, both built-in metrics, the usage
of virtual and physical memory would be shown.