metrics: Monitor rate limit wait duration #17567
Conversation
Codecov Report
@@ Coverage Diff @@
## main #17567 +/- ##
=======================================
Coverage 51.53% 51.54%
=======================================
Files 1717 1717
Lines 85381 85396 +15
Branches 7761 7765 +4
=======================================
+ Hits 44005 44018 +13
Misses 37501 37501
- Partials 3875 3877 +2
|
ryanslade
force-pushed
the
ratelimit-metrics
branch
from
2897fa9
to
b50b9db
Compare
So that we can add more than one collector function
Implemented for GitHub and GitLab
They're already tracked by NewRequestMeter
|
|
||
| This panel indicates time spent waiting for the GitHub graphql API rate limiter. | ||
|
|
||
| Indicates how long we`re waiting on the rate limit once it has been exceeded` |
There was a problem hiding this comment.
Did you quote this on purpose? Or did you mean this:
| Indicates how long we`re waiting on the rate limit once it has been exceeded` | |
| Indicates how long we're waiting on the rate limit once it has been exceeded. |
There was a problem hiding this comment.
Weird, our doc generator seems to be doing this
There was a problem hiding this comment.
The generated docs look fine in the browser though
| @@ -50,6 +50,13 @@ func (r *MonitorRegistry) Count() int { | |||
| return len(r.monitors) | |||
| } | |||
|
|
|||
| // MetricsCollector is used so that we can inject metric collection functions for | |||
| // difference monitor configurations. | |||
| type MetricsCollector struct { | |||
There was a problem hiding this comment.
Why not use an interface instead?
There was a problem hiding this comment.
Because then each caller would have to define their own type to implement it which doesn't seem necessary.
ryanslade
force-pushed
the
ratelimit-metrics
branch
from
a8523a5
to
8435fcd
Compare
|
Notifying subscribers in CODENOTIFY files for diff c293f39...d1b7619.
|
ryanslade
changed the title
| githubRatelimitWaitCounter.WithLabelValues("rest", "TESTNAME").Add((time.Millisecond * 100).Seconds()) | ||
| time.Sleep(1 * time.Second) |
There was a problem hiding this comment.
Hmm... looks like debugging lines?
There was a problem hiding this comment.
Oops, thanks for spotting that!
| githubRemainingGauge.WithLabelValues(resource, svc.DisplayName).Set(n) | ||
| }, | ||
| WaitDuration: func(n time.Duration) { | ||
| githubRatelimitWaitCounter.WithLabelValues(resource, svc.DisplayName).Add(n.Seconds()) |
There was a problem hiding this comment.
Is a ever-growing number on a Grafana panel making sense 🤔 (ignore if this is the purpose)
There was a problem hiding this comment.
I wasn't sure, but agree that maybe we have too many. I'll leave it in for now since the metrics were specifically requested by @slimsag
This change tracks the amount of time we spend waiting on code host rate
limits once they've been exceeded.
It uses header data tracked by our rate limit monitor and there only supports
GitLab and GitHub as these are the only hosts supported by our monitor.
Closes: #17200