Allocator maps accessed outside of their mutex

[kristinapathak]

opentelemetry-operator/cmd/otel-allocator/allocation/allocator.go

Lines 38 to 48 in 07b9f22

	type Allocator struct {
	m sync.Mutex
	targetsWaiting map[string]TargetItem // temp buffer to keep targets that are waiting to be processed
	collectors map[string]*collector // all current collectors
	TargetItems map[string]*TargetItem
	log logr.Logger
	}

TargetItems and collectors are normally protected by a mutex, but are being accessed in a few places without the mutex being locked first.

TargetItems:

opentelemetry-operator/cmd/otel-allocator/main.go

Line 174 in 07b9f22

for _, v := range s.allocator.TargetItems {

opentelemetry-operator/cmd/otel-allocator/main.go

Line 184 in 07b9f22

for _, v := range s.allocator.TargetItems {

opentelemetry-operator/cmd/otel-allocator/allocation/http.go

Line 26 in 07b9f22

for _, j := range allocator.TargetItems {

collectors:

opentelemetry-operator/cmd/otel-allocator/allocation/http.go

Line 51 in 07b9f22

for _, col := range allocator.collectors {

This is a data race and can cause issues with the endpoints providing the correct information.

I would like to fix this, by:

• modifying the mutex to be a RWMutex
• no longer exporting TargetItems
• adding two functions for the Allocator that each provide a copy of their respective maps - names could be something like Collectors() and TargetItems() or GetCollectors() and GetTargetItems()

A follow up question: If we shallow copy the map values, they will still be pointing to the same objects. How deep do we need to copy everything? Is it enough to dereference the pointers?