Fix data-race in metric without code and method but with `WithLabelFromCtx` (#1318)

This commit fixes a data race that exists when the metric used in any
`promhttp` middleware doesn't collect the `code` and `method` but uses
`WithLabelFromCtx` to collect values from context.

The problem happens because when no `code` and `method` tags are
collected, the `labels` function returns a pre-initialized map
`emptyLabels` for every request.

When one or multipe `WithLabelFromCtx` options are configured, the
returned map from the `labels` function call is used to collect the
metrics from context which creates a multi-write data race.

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
This commit is contained in:
Tiago Silva 2023-08-01 17:11:17 +01:00 committed by GitHub
parent 7f2db5f1ed
commit 59c00e3e9c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 5 additions and 8 deletions

View File

@ -389,16 +389,13 @@ func isLabelCurried(c prometheus.Collector, label string) bool {
return true return true
} }
// emptyLabels is a one-time allocation for non-partitioned metrics to avoid
// unnecessary allocations on each request.
var emptyLabels = prometheus.Labels{}
func labels(code, method bool, reqMethod string, status int, extraMethods ...string) prometheus.Labels { func labels(code, method bool, reqMethod string, status int, extraMethods ...string) prometheus.Labels {
if !(code || method) {
return emptyLabels
}
labels := prometheus.Labels{} labels := prometheus.Labels{}
if !(code || method) {
return labels
}
if code { if code {
labels["code"] = sanitizeCode(status) labels["code"] = sanitizeCode(status)
} }

View File

@ -250,7 +250,7 @@ func TestLabels(t *testing.T) {
}{ }{
"empty": { "empty": {
varLabels: []string{}, varLabels: []string{},
wantLabels: emptyLabels, wantLabels: prometheus.Labels{},
reqMethod: "GET", reqMethod: "GET",
respStatus: 200, respStatus: 200,
ok: true, ok: true,