Let metrics libs handle the atomicity #5738
Conversation
ldez
added
kind/enhancement
| m.openConnsGauge.With(labelValues...).Set(float64(openConns)) | ||
| }(labels) | ||
| m.openConnsGauge.With(labels...).Add(1) | ||
| defer m.openConnsGauge.With(labels...).Add(-1) |
There was a problem hiding this comment.
I restored the defer call instead of moving m.openConnsGauge.With(labels...).Add(-1) after m.next.ServeHTTP(recorder, req) because I've a scenario where, when doing a load testing of an SSE endpoint with rate limiting activated, we never go past m.next.ServeHTTP(recorder, req) and the Gauge is never decremented.
@mpl if you have an idea why I would be really interested.
There was a problem hiding this comment.
well, that suggests m.next.ServeHTTP never returns, either because of a deadlock or a blocked channel, does it not?
| m.openConnsGauge.With(labelValues...).Set(float64(openConns)) | ||
| }(labels) | ||
| m.openConnsGauge.With(labels...).Add(1) | ||
| defer m.openConnsGauge.With(labels...).Add(-1) |
There was a problem hiding this comment.
well, that suggests m.next.ServeHTTP never returns, either because of a deadlock or a blocked channel, does it not?
- Let metrics libs handle the atomicity. - Pre-allocate labels slice so that append() calls do not need to extend it. Signed-off-by: Sylvain Rabot <s.rabot@lectra.com>
Signed-off-by: Sylvain Rabot <s.rabot@lectra.com>
Co-Authored-By: mpl <mathieu.lonjaret@gmail.com>
Signed-off-by: Sylvain Rabot <s.rabot@lectra.com>
|
@ldez why did you remove the attempt to create the label slice with the correct size ? |
What does this PR do?
Let metrics libs handle the atomicity.
Motivation
Removes an atomic lock that the libs already implement.
Removes a defer call, the less the better until go 1.14.
More
Additional Notes