Add available and waitDuration metric to util/gate.go #17024
Add available and waitDuration metric to util/gate.go #17024pipiland2612 wants to merge 2 commits intoprometheus:mainfrom
Conversation
Signed-off-by: pipiland2612 <[email protected]>
|
Hi @bboreham, I'd love to hear your review in this pull request. Thanks! |
Signed-off-by: pipiland2612 <[email protected]>
|
Hello from the bug-scrub! @juliusv you reviewed the previous PR - could you take a look at this one please? |
bwplotka
left a comment
There was a problem hiding this comment.
Adding high level comments.
Also likely @GiedriusS could review this PR as @GiedriusS requested this feature (:
You can ping @GiedriusS on CNCF Slack too @pipiland2612 - I know Giedrius and I think he wouldn't mind that.
| Name: "available_slots", | ||
| Help: "Available slots in gate (positive = available, negative = queued requests)", | ||
| }), | ||
| promauto.With(r).NewCounter(prometheus.CounterOpts{ |
There was a problem hiding this comment.
Shouldn't this be a native histogram instead? 🤔
| config: config, | ||
| remoteReadSampleLimit: remoteReadSampleLimit, | ||
| remoteReadGate: gate.New(remoteReadConcurrencyLimit, | ||
| promauto.With(r).NewGauge(prometheus.GaugeOpts{ |
There was a problem hiding this comment.
I wonder if this metric shouldn't come from gate "package" and allow specifying handler label 🤔
So prometheus_gate_available_slots{handler="remote_read"} etc
|
Hi @bwplotka, I have ping the author @GiedriusS ealier but seems like he's quite busy and I don't receive any reply yet. Do you think we should continue this PR anymore ? |
|
Hello from the bug scrub! @bwplotka could you consider @pipiland2612's question and help them out? |
What issue is this pull request solves ?
Changes
waitDurationcounter tracks time spent waiting for gate accessavailablegauge is the number that represent the total of free slot and the number of requests waiting to get in the gate (this can be negative to show that there are already somethings waiting)gate_test.goto test the new changes