Skip to content

Commit 7f85956

Browse files
pedrampddPedrambwplotka
authored
promhttp: add regression test for concurrent map writes (#1274) (#2000)
* promhttp: add regression test for concurrent map writes (#1274) Without the fix in #1318, instrumenting a metric vec partitioned only by labels supplied via WithLabelFromCtx (i.e. with no "code" or "method" label) would cause concurrent requests to share and mutate a single 'emptyLabels' map, producing 'fatal error: concurrent map writes' under load. The fix made labels() allocate a fresh map per call but no regression test was added at the time. This adds a test that exercises both InstrumentHandlerCounter and InstrumentHandlerDuration from many goroutines so the bug would be caught either by the race detector or the runtime's concurrent-map- writes detection. Verified: the test fails (DATA RACE / concurrent map writes panic) when the pre-#1318 labels() implementation is reintroduced, and passes on current main with 'go test -race'. Closes #1274 Signed-off-by: Pedram <pedram@arpdigital.io> * Update prometheus/promhttp/instrument_server_test.go Co-authored-by: Bartlomiej Plotka <bwplotka@gmail.com> Signed-off-by: Pedram Pd <eragon.pedy@gmail.com> --------- Signed-off-by: Pedram <pedram@arpdigital.io> Signed-off-by: Pedram Pd <eragon.pedy@gmail.com> Co-authored-by: Pedram <pedram@arpdigital.io> Co-authored-by: Bartlomiej Plotka <bwplotka@gmail.com>
1 parent 6dd4625 commit 7f85956

1 file changed

Lines changed: 55 additions & 0 deletions

File tree

prometheus/promhttp/instrument_server_test.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"log"
2020
"net/http"
2121
"net/http/httptest"
22+
"sync"
2223
"testing"
2324

2425
"github.com/prometheus/client_golang/prometheus"
@@ -523,6 +524,60 @@ func TestInterfaceUpgrade(t *testing.T) {
523524
}
524525
}
525526

527+
// Regression test against https://github.com/prometheus/client_golang/pull/1318
528+
func TestInstrumentHandlerLabelFromCtxConcurrent(t *testing.T) {
529+
const (
530+
workers = 32
531+
requestsPerWorker = 200
532+
)
533+
534+
counter := prometheus.NewCounterVec(
535+
prometheus.CounterOpts{
536+
Name: "test_requests_total",
537+
Help: "A counter for requests to the wrapped handler.",
538+
},
539+
[]string{"dyn"},
540+
)
541+
histogram := prometheus.NewHistogramVec(
542+
prometheus.HistogramOpts{
543+
Name: "test_request_duration_seconds",
544+
Help: "A histogram of latencies for requests to the wrapped handler.",
545+
},
546+
[]string{"dyn"},
547+
)
548+
549+
reg := prometheus.NewRegistry()
550+
reg.MustRegister(counter, histogram)
551+
552+
next := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
553+
w.WriteHeader(http.StatusOK)
554+
})
555+
556+
dyn := WithLabelFromCtx("dyn", func(_ context.Context) string {
557+
return "v"
558+
})
559+
560+
chain := InstrumentHandlerCounter(
561+
counter,
562+
InstrumentHandlerDuration(histogram, next, dyn),
563+
dyn,
564+
)
565+
566+
var wg sync.WaitGroup
567+
wg.Add(workers)
568+
for i := 0; i < workers; i++ {
569+
go func() {
570+
defer wg.Done()
571+
for j := 0; j < requestsPerWorker; j++ {
572+
req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
573+
rec := httptest.NewRecorder()
574+
chain.ServeHTTP(rec, req)
575+
}
576+
}()
577+
}
578+
wg.Wait()
579+
}
580+
526581
func ExampleInstrumentHandlerDuration() {
527582
inFlightGauge := prometheus.NewGauge(prometheus.GaugeOpts{
528583
Name: "in_flight_requests",

0 commit comments

Comments
 (0)