fix(collector): fix the write overwrite caused by multiple collectors sharing a global variable DataSource #1857
Conversation
… sharing a global variable DataSource
| @@ -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. | |||
There was a problem hiding this 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.
| @@ -124,10 +123,10 @@ func getReplicaAddrs() ([]string, error) { | |||
| } | |||
|
|
|||
| // Register all metrics. | |||
| func initMetrics() { | |||
| func initMetrics(dataSource int) { | |||
There was a problem hiding this 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
There was a problem hiding this comment.
You can do the refactor in next patches, this PR looks good to me.
…) and dataSource as members of Collector
|
|
||
| return &Collector{detectInterval: detectInterval, detectTimeout: detectTimeout} | ||
| var role string | ||
| if dataSource == 0 { |
There was a problem hiding this comment.
We could just change dataSource to role, and certainly change the type of dataSource to string instead of int which means, changing the type of following constants to string:
const (
MetaServer int = 0
ReplicaServer int = 1
)…e to string instead of int
| func updateMetric(metric Metric, endpoint string, level string, title string) { | ||
| role := RoleByDataSource[DataSource] | ||
| func (collector *Collector) updateMetric(metric Metric, endpoint string, level string, title string) { | ||
| role := collector.role |
There was a problem hiding this comment.
Could the variable role be removed ?
#1820