Added UserReplicaGroupMetrics #6463
Conversation
Signed-off-by: 7h3-3mp7y-m4n <[email protected]>
| userReplicaGroupCount: promauto.With(reg).NewGaugeVec(prometheus.GaugeOpts{ | ||
| Name: "ha_tracker_user_replica_group_count", | ||
| Help: "Number of HA replica groups tracked for each user.", | ||
| }, []string{"user"}), |
There was a problem hiding this comment.
We need to clean up the metric in CleanupHATrackerMetricsForUser so that when a user is not active anymore the corresponding user dimension is removed from the metric
pkg/ha/ha_tracker.go
Outdated
| time.Sleep(10 * time.Second) | ||
| t.updateUserReplicaGroupCount() | ||
| } | ||
| }() |
There was a problem hiding this comment.
Can we move this to the loop method?
You can follow cleanupOldReplicasLoop and do it similarly.
pkg/ha/ha_tracker.go
Outdated
| @@ -212,6 +218,12 @@ func NewHATracker(cfg HATrackerConfig, limits HATrackerLimits, trackerStatusConf | |||
| } | |||
| t.client = client | |||
| } | |||
| go func() { | |||
| for { | |||
| time.Sleep(10 * time.Second) | |||
There was a problem hiding this comment.
Let's do 30s interval and make the interval a const
pkg/ha/ha_tracker_test.go
Outdated
| ` | ||
| require.NoError(t, testutil.GatherAndCompare(reg, strings.NewReader(expectedMetrics), "ha_tracker_user_replica_group_count")) | ||
| delete(tracker.replicaGroups, "user1") | ||
| tracker.userReplicaGroupCount.WithLabelValues("user1").Set(0) |
There was a problem hiding this comment.
There is no need set this manually.
pkg/ha/ha_tracker_test.go
Outdated
| ha_tracker_user_replica_group_count{user="user2"} 3 | ||
| ha_tracker_user_replica_group_count{user="user1"} 0 | ||
| ` | ||
| require.NoError(t, testutil.GatherAndCompare(reg, strings.NewReader(expectedMetricsAfterCleanup), "ha_tracker_user_replica_group_count")) |
There was a problem hiding this comment.
Can we add CleanupHATrackerMetricsForUser for one test user and make sure ha_tracker_user_replica_group_count is cleaned up?
There was a problem hiding this comment.
yeah why not :)
Signed-off-by: 7h3-3mp7y-m4n <[email protected]>
|
hey @yeya24 , I’m currently working on the tests and ran into an issue. Whenever I use |
|
Hey @7h3-3mp7y-m4n, can you please share the error you got? And your test code if possible. I checked out your code locally and tried to update the metrics clean up test. yeya24@9a35c7d |
Signed-off-by: 7h3-3mp7y-m4n <[email protected]>
Hi @yeya24 , I'm sorry for the previous version of the test case, I was writing it manually, and it turned out a bit rough. I’ve now implemented the changes you suggested, and it's ready for your review. Thanks for your patience! |
Signed-off-by: 7h3-3mp7y-m4n <[email protected]>
Well, I made this as a draft so that I could review it and update you with my current progress. I think it's ready to review! |
What this PR does:
The main purpose of this pr is to add a goroutine to update tenant HA replica groups to metrics periodically.
Which issue(s) this PR fixes:
Fixes #6397
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]