Fix concurrent map write panic in monitoring middleware

[carsonip]

Motivation/summary

Fix panic due to monitoring middleware concurrent map write.

fatal error: concurrent map writes

goroutine 21913 [running]:
github.com/elastic/apm-server/internal/beater/middleware.(*monitoringMiddleware).getCounter(0xc008a53530, {0x32a4b68?, 0xc0073d1c00?})
	/home/carson/projects/apm-server/internal/beater/middleware/monitoring_middleware.go:87 +0xb8
github.com/elastic/apm-server/internal/beater/middleware.MonitoringMiddleware.(*monitoringMiddleware).Middleware.func1.1(0xc00521a000)
	/home/carson/projects/apm-server/internal/beater/middleware/monitoring_middleware.go:49 +0x59
github.com/elastic/apm-server/internal/beater/api.apmMiddleware.RecoverPanicMiddleware.func2.1(0x3617ce0?)
	/home/carson/projects/apm-server/internal/beater/middleware/recover_panic_middleware.go:60 +0x43
github.com/elastic/apm-server/internal/beater/middleware.TimeoutMiddleware.func1.1(0xc00521a000)
	/home/carson/projects/apm-server/internal/beater/middleware/timeout_middleware.go:35 +0x35
github.com/elastic/apm-server/internal/beater/api.apmMiddleware.LogMiddleware.func1.1(0xc00521a000)
	/home/carson/projects/apm-server/internal/beater/middleware/log_middleware.go:47 +0xaa
github.com/elastic/apm-server/internal/beater/api.NewMux.(*ContextPool).HTTPHandler.func11({0x3632730, 0xc0047620e0}, 0xc0003f88c0)
	/home/carson/projects/apm-server/internal/beater/request/context_pool.go:47 +0xec
net/http.HandlerFunc.ServeHTTP(...)
	/home/carson/apm-server-gopath/pkg/mod/golang.org/[email protected]/src/net/http/server.go:2220
go.elastic.co/apm/module/apmhttp/v2.(*handler).ServeHTTP(0xc006b7de40, {0x3632730, 0xc0047620e0}, 0xc0003f88c0)
	/home/carson/apm-server-gopath/pkg/mod/go.elastic.co/apm/module/apmhttp/[email protected]/handler.go:74 +0xc9
github.com/gorilla/mux.(*Router).ServeHTTP(0xc0070bd2c0, {0x3632730, 0xc0047620e0}, 0xc0003f8640)
	/home/carson/apm-server-gopath/pkg/mod/github.com/gorilla/[email protected]/mux.go:212 +0x2d7
github.com/elastic/gmux.ConfigureServer.(*mux).withGRPCInsecure.func3({0x3632730?, 0xc0047620e0?}, 0x2fe25c0?)
	/home/carson/apm-server-gopath/pkg/mod/github.com/elastic/[email protected]/mux.go:119 +0x105
net/http.HandlerFunc.ServeHTTP(...)
	/home/carson/apm-server-gopath/pkg/mod/golang.org/[email protected]/src/net/http/server.go:2220
net/http.serverHandler.ServeHTTP({0x362c8e8?}, {0x3632730?, 0xc0047620e0?}, 0x6?)
	/home/carson/apm-server-gopath/pkg/mod/golang.org/[email protected]/src/net/http/server.go:3210 +0xa2
net/http.(*conn).serve(0xc0078d7440, {0x3639618, 0xc00bcae120})
	/home/carson/apm-server-gopath/pkg/mod/golang.org/[email protected]/src/net/http/server.go:2092 +0x610
created by net/http.(*Server).Serve in goroutine 21909
	/home/carson/apm-server-gopath/pkg/mod/golang.org/[email protected]/src/net/http/server.go:3360 +0x485

Alternative to this, we could have populated the maps with all the possible keys, but we may risk missing some possible map keys.

Checklist

• Update CHANGELOG.asciidoc
• Documentation has been updated

For functional changes, consider:

• Is it observable through the addition of either logging or metrics?
• Is its use being published in telemetry to enable product improvement?
• Have system tests been added to avoid regression?

How to test these changes

Related issues

[mergify]

This pull request does not have a backport label. Could you fix it @carsonip? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

• backport-7.17 is the label to automatically backport to the 7.17 branch.
• backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit.
• backport-8.x is the label to automatically backport to the 8.x branch.

[mergify]

backport-8.x has been added to help with the transition to the new branch 8.x.
If you don't need it please use backport-skip label.

[kruskall]

I think this could be a good candidate for sync Map, wdyt ? (Since the counter should only be written once but read many times)

[carsonip]

I think this could be a good candidate for sync Map, wdyt ? (Since the counter should only be written once but read many times)

I didn't benchmark. But you should be right as stated in sync.Map docs:

The Map type is optimized for two common use cases: (1) when the entry for a given key is only ever written once but read many times, as in caches that only grow, or (2) when multiple goroutines read, write, and overwrite entries for disjoint sets of keys. In these two cases, use of a Map may significantly reduce lock contention compared to a Go map paired with a separate Mutex or RWMutex.

[carsonip]

With this bench:

package syncmap

import (
	"sync"
	"testing"
)

func BenchmarkSyncMap(b *testing.B) {
	m := sync.Map{}
	m.LoadOrStore("foo", "bar")
	b.RunParallel(func(p *testing.PB) {
		for p.Next() {
			m.Load("foo")
		}
	})
}

func BenchmarkRWMutex(b *testing.B) {
	rw := sync.RWMutex{}
	m := map[string]string{"foo": "bar"}
	b.RunParallel(func(p *testing.PB) {
		for p.Next() {
			rw.RLock()
			_ = m["foo"]
			rw.RUnlock()
		}
	})
}

Result:

goos: linux
goarch: amd64
pkg: playground/syncmap
cpu: 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz
BenchmarkSyncMap-16     334905933                3.563 ns/op
BenchmarkRWMutex-16     25959259                45.41 ns/op
PASS
ok      playground/syncmap      2.857s