Delay initial prometheus status metric #2874
Conversation
k8s-ci-robot
added
cncf-cla: yes
internal/ingress/metric/main.go
Outdated
| // the default nginx.conf does not contains | ||
| // a server section with the status port | ||
| go func() { | ||
| time.Sleep(10 * time.Second) |
There was a problem hiding this comment.
IMO 10s is a bit too much, potentially we can loose stats for thousands of requests. Maybe 5s? For the first sync of dynamic configuration we sleep only 1s and it has been fine so far.
But ideally we would need a mechanism to deterministically detect that the port is ready.
There was a problem hiding this comment.
Just in case, this delay only applies to the basic nginx connection count.
Codecov Report
@@ Coverage Diff @@
## master #2874 +/- ##
=======================================
Coverage 47.56% 47.56%
=======================================
Files 76 76
Lines 5483 5483
=======================================
Hits 2608 2608
Misses 2540 2540
Partials 335 335Continue to review full report at Codecov.
|
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aledbf, ElvinEfendi 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 |
Which issue this PR fixes:
This confuses users and they think there's an error when the issue here is just timing.