Skip to content

Commit

Permalink
Fix data-race in metric without code and method but with `WithLabelFr…
Browse files Browse the repository at this point in the history
…omCtx`

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>
  • Loading branch information
tigrato committed Jul 31, 2023
1 parent 7f2db5f commit 5113fb6
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 7 deletions.
9 changes: 3 additions & 6 deletions prometheus/promhttp/instrument_server.go
Expand Up @@ -389,15 +389,12 @@ func isLabelCurried(c prometheus.Collector, label string) bool {
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 {
labels := prometheus.Labels{}

if !(code || method) {
return emptyLabels
return labels
}
labels := prometheus.Labels{}

if code {
labels["code"] = sanitizeCode(status)
Expand Down
2 changes: 1 addition & 1 deletion prometheus/promhttp/instrument_server_test.go
Expand Up @@ -250,7 +250,7 @@ func TestLabels(t *testing.T) {
}{
"empty": {
varLabels: []string{},
wantLabels: emptyLabels,
wantLabels: prometheus.Labels{},
reqMethod: "GET",
respStatus: 200,
ok: true,
Expand Down

0 comments on commit 5113fb6

Please sign in to comment.