acelyc111 commented on code in PR #1857:
URL:
https://github.com/apache/incubator-pegasus/pull/1857#discussion_r1455370836
##########
collector/metrics/metric_collector.go:
##########
@@ -124,10 +123,10 @@ func getReplicaAddrs() ([]string, error) {
}
// Register all metrics.
-func initMetrics() {
+func initMetrics(dataSource int) {
Review Comment:
You can do the refactor in next patches, this PR looks good to me.
##########
collector/metrics/metric_collector.go:
##########
@@ -124,10 +123,10 @@ func getReplicaAddrs() ([]string, error) {
}
// Register all metrics.
-func initMetrics() {
+func initMetrics(dataSource int) {
Review Comment:
How about making both the initMetrics function (some other functions are the
same) and dataSource as members of MetricCollector, so the dataSource would not
appear everywhere.
The global variables are the same.
var GaugeMetricsMap map[string]prometheus.GaugeVec
var CounterMetricsMap map[string]prometheus.CounterVec
var SummaryMetricsMap map[string]prometheus.Summary
var RoleByDataSource map[int]string
var TableNameByID map[string]string
##########
collector/metrics/metric_collector.go:
##########
@@ -53,8 +53,7 @@ var GaugeMetricsMap map[string]prometheus.GaugeVec
var CounterMetricsMap map[string]prometheus.CounterVec
var SummaryMetricsMap map[string]prometheus.Summary
-// DataSource 0 meta server, 1 replica server.
-var DataSource int
+// RoleByDataSource 0 meta server, 1 replica server.
Review Comment:
It‘s corresponding to the variables in line 36, it would be better to
mention the variable names instead of the magic numbers (0,1) again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
