-
Notifications
You must be signed in to change notification settings - Fork 807
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 UserReplicaGroupMetrics #6463
base: master
Are you sure you want to change the base?
Changes from 1 commit
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 |
---|---|---|
|
@@ -137,6 +137,7 @@ type HATracker struct { | |
electedReplicaTimestamp *prometheus.GaugeVec | ||
electedReplicaPropagationTime prometheus.Histogram | ||
kvCASCalls *prometheus.CounterVec | ||
userReplicaGroupCount *prometheus.GaugeVec | ||
|
||
cleanupRuns prometheus.Counter | ||
replicasMarkedForDeletion prometheus.Counter | ||
|
@@ -182,6 +183,11 @@ func NewHATracker(cfg HATrackerConfig, limits HATrackerLimits, trackerStatusConf | |
Help: "The total number of CAS calls to the KV store for a user ID/cluster.", | ||
}, []string{"user", "cluster"}), | ||
|
||
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"}), | ||
|
||
cleanupRuns: promauto.With(reg).NewCounter(prometheus.CounterOpts{ | ||
Name: "ha_tracker_replicas_cleanup_started_total", | ||
Help: "Number of elected replicas cleanup loops started.", | ||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's do 30s interval and make the interval a const 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. for sure! :) |
||
t.updateUserReplicaGroupCount() | ||
} | ||
}() | ||
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. Can we move this to the 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. okay :) |
||
|
||
t.Service = services.NewBasicService(nil, t.loop, nil) | ||
return t, nil | ||
|
@@ -521,3 +533,12 @@ func (c *HATracker) SnapshotElectedReplicas() map[string]ReplicaDesc { | |
} | ||
return electedCopy | ||
} | ||
|
||
func (t *HATracker) updateUserReplicaGroupCount() { | ||
t.electedLock.RLock() | ||
defer t.electedLock.RUnlock() | ||
|
||
for user, groups := range t.replicaGroups { | ||
t.userReplicaGroupCount.WithLabelValues(user).Set(float64(len(groups))) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -795,3 +795,36 @@ func checkReplicaDeletionState(t *testing.T, duration time.Duration, c *HATracke | |
require.Equal(t, expectedMarkedForDeletion, markedForDeletion, "KV entry marked for deletion") | ||
} | ||
} | ||
func TestHATracker_UserReplicaGroupMetrics(t *testing.T) { | ||
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. Let's remove this test and reuse the same test of TestHATracker_MetricsCleanup where we check metrics clean up functionality. You can follow what I did yeya24@9a35c7d#diff-59f8014af0fba125c4aa53685006c000bde94ce36bc718ce8613a465263a9ad7R630 |
||
t.Parallel() | ||
reg := prometheus.NewPedanticRegistry() | ||
tracker := &HATracker{ | ||
userReplicaGroupCount: prometheus.NewGaugeVec(prometheus.GaugeOpts{ | ||
Name: "ha_tracker_user_replica_group_count", | ||
Help: "Number of HA replica groups tracked for each user.", | ||
}, []string{"user"}), | ||
replicaGroups: map[string]map[string]struct{}{ | ||
"user1": {"group1": {}, "group2": {}}, | ||
"user2": {"groupA": {}, "groupB": {}, "groupC": {}}, | ||
}, | ||
} | ||
reg.MustRegister(tracker.userReplicaGroupCount) | ||
tracker.updateUserReplicaGroupCount() | ||
expectedMetrics := ` | ||
# HELP ha_tracker_user_replica_group_count Number of HA replica groups tracked for each user. | ||
# TYPE ha_tracker_user_replica_group_count gauge | ||
ha_tracker_user_replica_group_count{user="user1"} 2 | ||
ha_tracker_user_replica_group_count{user="user2"} 3 | ||
` | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no need set this manually. 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. okay! |
||
tracker.updateUserReplicaGroupCount() | ||
expectedMetricsAfterCleanup := ` | ||
# HELP ha_tracker_user_replica_group_count Number of HA replica groups tracked for each user. | ||
# TYPE ha_tracker_user_replica_group_count gauge | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add 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. yeah why not :) |
||
} |
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.
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