FFM-11873 Lock for entirety of safeMetricsRequestMap.get() (#356)

* FFM-11873 Lock for entirety of safeMetricsRequestMap.get()

**What**

- Performs a Read Lock for the entirety of `safeMetricsRequestMap.get()`

**Why**

- I saw an error log in QA saying `"fatal error: concurrent map
  iteration and map write"` and it pointed at this code. It turns out
the copy that we were doing to reduce the amount of time the lock was
held was a shallow copy of the maps reference and not a deep copy of its
contents.

**Testing**

- Benchmarked performing a deep copy vs locking for the entirety of
  `get()`

Author: jcox250

clients/metrics_service/safe_metrics_request_map.go

@@ -95,12 +95,11 @@ func (s *safeMetricsRequestMap) get() map[string]domain.MetricsRequest {
 	// Take a copy an unlock so we don't block other thread
 	// marshaling to the response type
 	s.RLock()
-	cpy := s.detailed
-	s.RUnlock()
+	defer s.RUnlock()
 
 	result := map[string]domain.MetricsRequest{}
 
-	for envID, detailedMap := range cpy {
+	for envID, detailedMap := range s.detailed {
 
 		slice := []clientgen.MetricsData{}
 		for _, v := range detailedMap {

clients/metrics_service/safe_metrics_request_map_test.go

@@ -565,3 +565,113 @@ func Test_SafeMetricsRequestMap(t *testing.T) {
 		})
 	}
 }
+
+func BenchmarkSafeMetricsRequestMapGetMethods(b *testing.B) {
+	// Initialize your safeMetricsRequestMap with some data
+	smr := newSafeMetricsRequestMap()
+
+	// Populate smr with test data
+	populateTestData(smr) // Assuming you have a function to populate test data
+
+	// Table-driven benchmarks
+	benchmarks := []struct {
+		name string
+		fn   func() map[string]domain.MetricsRequest
+	}{
+		{name: "get", fn: smr.get},
+		{name: "getNew", fn: smr.getDeepCopyBenchmarkOnly},
+	}
+
+	for _, bm := range benchmarks {
+		// Run the benchmark
+		b.Run(bm.name, func(b *testing.B) {
+			b.ReportAllocs()
+			for i := 0; i < b.N; i++ {
+				_ = bm.fn() // Call the function and discard the result
+			}
+		})
+	}
+}
+
+// populateTestData is a helper function to add test data to the map
+func populateTestData(smr *safeMetricsRequestMap) {
+	// Populate the map with sample data for benchmarking
+	// This function should create a reasonably sized map to reflect a typical workload
+	// Example:
+	smr.add(domain.MetricsRequest{
+		Size:          2,
+		EnvironmentID: "env1",
+		Metrics: clientgen.Metrics{
+			MetricsData: domain.ToPtr([]clientgen.MetricsData{
+				{Count: 3, Attributes: []clientgen.KeyValue{
+					{
+						Key:   "attr1",
+						Value: "attr1",
+					},
+				}},
+				{Count: 4, Attributes: []clientgen.KeyValue{
+					{
+						Key:   "attr2",
+						Value: "attr2",
+					},
+				}},
+			}),
+		},
+	})
+
+	smr.add(domain.MetricsRequest{
+		Size:          2,
+		EnvironmentID: "env2",
+		Metrics: clientgen.Metrics{
+			MetricsData: domain.ToPtr([]clientgen.MetricsData{
+				{Count: 3, Attributes: []clientgen.KeyValue{
+					{
+						Key:   "attr3",
+						Value: "attr3",
+					},
+				}},
+				{Count: 4, Attributes: []clientgen.KeyValue{
+					{
+						Key:   "attr4",
+						Value: "attr4",
+					},
+				}},
+			}),
+		},
+	})
+	// Add more test data as needed
+}
+
+func (s *safeMetricsRequestMap) getDeepCopyBenchmarkOnly() map[string]domain.MetricsRequest {
+	s.RLock()
+	// Deep copy of the outer map
+	cpy := make(map[string]map[string]clientgen.MetricsData, len(s.detailed))
+
+	for envID, detailedMap := range s.detailed {
+		// Deep copy of the nested map
+		nestedCopy := make(map[string]clientgen.MetricsData, len(detailedMap))
+		for key, value := range detailedMap {
+			nestedCopy[key] = value
+		}
+		cpy[envID] = nestedCopy
+	}
+	s.RUnlock()
+
+	result := make(map[string]domain.MetricsRequest, len(cpy))
+
+	for envID, detailedMap := range cpy {
+		slice := make([]clientgen.MetricsData, 0, len(detailedMap))
+		for _, v := range detailedMap {
+			slice = append(slice, v)
+		}
+
+		result[envID] = domain.MetricsRequest{
+			EnvironmentID: envID,
+			Metrics: clientgen.Metrics{
+				MetricsData: domain.ToPtr(slice),
+			},
+		}
+	}
+
+	return result
+}