improve L4FailedHealthCheckCount metric #1955
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
approved
|
/assign @panslava |
|
/assign @code-elinka |
|
@cezarygerard: GitHub didn't allow me to assign the following users: code-elinka. Note that only kubernetes members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
pkg/l4lb/l4netlbcontroller.go
Outdated
| // Log here, context/http handler do no log the error. | ||
| klog.Error(msg) | ||
| metrics.PublishL4FailedHealthCheckCount(l4NetLBControllerName) | ||
| metrics.PublishL4FailedHealthCheckCount(l4ILBControllerName) |
pkg/l4lb/l4controller.go
Outdated
| @@ -151,10 +151,14 @@ func (l4c *L4Controller) checkHealth() error { | |||
| // This indicates that the controller was stuck handling a previous update, or sync function did not get invoked. | |||
| syncTimeLatest := lastEnqueueTime.Add(enqueueToSyncDelayThreshold) | |||
| if lastSyncTime.After(syncTimeLatest) { | |||
| msg := fmt.Sprintf("L4 ILB Sync happened at time %v - %v after enqueue time, threshold is %v", lastSyncTime, lastSyncTime.Sub(lastEnqueueTime), enqueueToSyncDelayThreshold) | |||
| msg := fmt.Sprintf("L4 ILB Sync happened at time %v, last enqueue time %v- %v after enqueue time, threshold is %v", lastSyncTime, lastEnqueueTime, lastSyncTime.Sub(lastEnqueueTime), enqueueToSyncDelayThreshold) | |||
There was a problem hiding this comment.
sorry for nit, can you bring back the space after %v?
There was a problem hiding this comment.
oh also it now reads a bit strange "last enqueue time %v- %v after enqueue time", because "after enqueue time" is about "sync happened after enqueue time"
maybe in this way would be better
msg := fmt.Sprintf("L4 ILB Sync happened at time %v, %v after enqueue time, last enqueue time %v, threshold is %v", lastSyncTime,, lastSyncTime.Sub(lastEnqueueTime), lastEnqueueTime, enqueueToSyncDelayThreshold)
pkg/l4lb/l4netlbcontroller.go
Outdated
| @@ -355,10 +355,14 @@ func (lc *L4NetLBController) checkHealth() error { | |||
| // This indicates that the controller was stuck handling a previous update, or sync function did not get invoked. | |||
| syncTimeLatest := lastEnqueueTime.Add(enqueueToSyncDelayThreshold) | |||
| if lastSyncTime.After(syncTimeLatest) { | |||
| msg := fmt.Sprintf("L4 External LoadBalancer Sync happened at time %v - %v after enqueue time, threshold is %v", lastSyncTime, lastSyncTime.Sub(lastEnqueueTime), enqueueToSyncDelayThreshold) | |||
| msg := fmt.Sprintf("L4 NetLB Sync happened at time %v, last enqueue time %v- %v after enqueue time, threshold is %v", lastSyncTime, lastEnqueueTime, lastSyncTime.Sub(lastEnqueueTime), enqueueToSyncDelayThreshold) | |||
There was a problem hiding this comment.
sorry for nit, can you bring back the space after %v?
There was a problem hiding this comment.
Same as for ilb
maybe in this way would be better
msg := fmt.Sprintf("L4 NetLB Sync happened at time %v, %v after enqueue time, last enqueue time %v, threshold is %v", lastSyncTime,, lastSyncTime.Sub(lastEnqueueTime), lastEnqueueTime, enqueueToSyncDelayThreshold)
|
/lgtm |
k8s-ci-robot
added
lgtm
k8s-ci-robot
removed
lgtm
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cezarygerard, panslava The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
do no report multiple times the same case of late sync