-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Added a graph of command time statistics to grafana #1751
Changes from 4 commits
fa43668
c47e4b5
5600c4c
cc02a6c
9078514
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -742,6 +742,8 @@ void InfoCmd::Do(std::shared_ptr<Slot> slot) { | |
info.append("\r\n"); | ||
InfoExecCount(info); | ||
info.append("\r\n"); | ||
InfoCommandStats(info); | ||
info.append("\r\n"); | ||
InfoCPU(info); | ||
info.append("\r\n"); | ||
InfoReplication(info); | ||
|
@@ -1254,15 +1256,19 @@ void InfoCmd::InfoCommandStats(std::string& info) { | |
tmp_stream.precision(2); | ||
tmp_stream.setf(std::ios::fixed); | ||
tmp_stream << "# Commandstats" << "\r\n"; | ||
for (auto& iter : *g_pika_server->GetCommandStatMap()) { | ||
if (iter.second.cmd_count != 0) { | ||
tmp_stream << "cmdstat_" << iter.first << ":" | ||
<< "calls=" << iter.second.cmd_count << ",usec=" | ||
<< iter.second.cmd_time_consuming | ||
<< ",usec_per_call=" | ||
<< static_cast<double>(iter.second.cmd_time_consuming) / static_cast<double>(iter.second.cmd_count) | ||
<< "\r\n"; | ||
for (auto iter : *g_pika_server->GetCommandStatMap()) { | ||
if (iter.second.cmd_count != 0) { | ||
tmp_stream << iter.first << ":" | ||
<< "calls=" << iter.second.cmd_count << ", usec=" | ||
<< static_cast<double>(iter.second.cmd_time_consuming) / 1000.0 | ||
<< ", usec_per_call="; | ||
if (static_cast<double>(iter.second.cmd_time_consuming) == 0) { | ||
tmp_stream << 0 << "\r\n"; | ||
} else { | ||
tmp_stream << (static_cast<double>(iter.second.cmd_time_consuming) / 1000.0) / static_cast<double>(iter.second.cmd_count) | ||
<< "\r\n"; | ||
} | ||
} | ||
} | ||
info.append(tmp_stream.str()); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Overall, the code patch you provided seems to address the addition of a new function called Here are a few points to consider for your code review:
Remember to thoroughly test your code changes before deploying them to ensure they work as intended and do not introduce any unexpected bugs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Overall, the code patch you provided seems fine and does not contain any obvious bugs. However, there are a couple of improvement suggestions:
Here's an updated version of the code that incorporates these suggestions: void InfoCmd::InfoCommandStats(std::string& info) {
std::ostringstream tmp_stream;
tmp_stream.precision(2);
tmp_stream.setf(std::ios::fixed);
tmp_stream << "# Commandstats" << "\r\n";
for (const auto& iter : *g_pika_server->GetCommandStatMap()) {
if (iter.second.cmd_count != 0) {
tmp_stream << iter.first << ":"
<< "calls=" << iter.second.cmd_count
<< ", usec=" << iter.second.cmd_time_consuming / 1000.0
<< ", usec_per_call="
<< (iter.second.cmd_time_consuming / 1000.0) / iter.second.cmd_count
<< "\r\n";
}
}
info.append(tmp_stream.str());
} Remember to apply consistent formatting and ensure appropriate error handling and boundary checks for your specific use case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code review of the given patch is as follows:
Remember that the provided review is based on a brief analysis, and a deeper understanding may require more context about the codebase and its specific requirements. |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
package metrics | ||
|
||
import "regexp" | ||
|
||
func init() { | ||
Register(collectCommandstatsMetrics) | ||
} | ||
|
||
var collectCommandstatsMetrics = map[string]MetricConfig{ | ||
"commandstats_info": { | ||
Parser: ®exParser{ | ||
name: "commandstats_info", | ||
reg: regexp.MustCompile(`(?P<cmd>[\w]+)[:\s]*calls=(?P<calls>[\d]+)[,\s]*usec=(?P<usec>[\d+\.\d+]+)[,\s]*usec_per_call=(?P<usec_per_call>[\d+\.\d+]+)`), | ||
Parser: &normalParser{}, | ||
}, | ||
MetricMeta: MetaDatas{ | ||
{ | ||
Name: "calls", | ||
Help: "Pika Number of times each command is invoked", | ||
Type: metricTypeGauge, | ||
Labels: []string{LabelNameAddr, LabelNameAlias, "cmd"}, | ||
ValueName: "calls", | ||
}, | ||
{ | ||
Name: "usec", | ||
Help: "Total time taken by each Pika command", | ||
Type: metricTypeGauge, | ||
Labels: []string{LabelNameAddr, LabelNameAlias, "cmd"}, | ||
ValueName: "usec", | ||
}, | ||
{ | ||
Name: "usec_per_call", | ||
Help: "Average time of each Pika command", | ||
Type: metricTypeGauge, | ||
Labels: []string{LabelNameAddr, LabelNameAlias, "cmd"}, | ||
ValueName: "usec_per_call", | ||
}, | ||
}, | ||
}, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
可以封装一个函数