APIGOV-29555 fix race conditions

Axway · Feb 27, 2025 · 85f4654 · 85f4654
1 parent 9d424b7
commit 85f4654
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 17 deletions.
diff --git a/pkg/traceability/sampling/globalsampling.go b/pkg/traceability/sampling/globalsampling.go
@@ -97,7 +97,7 @@ func SetupSampling(cfg Sampling, offlineMode bool, apicDeployment string) error
 	agentSamples = &sample{
 		config:             cfg,
 		currentCounts:      make(map[string]int),
-		counterLock:        sync.Mutex{},
+		samplingLock:       sync.Mutex{},
 		samplingCounter:    0,
 		counterResetPeriod: time.Minute,
 		counterResetStopCh: make(chan struct{}),

diff --git a/pkg/traceability/sampling/sample.go b/pkg/traceability/sampling/sample.go
@@ -11,7 +11,7 @@ import (
 type sample struct {
 	config             Sampling
 	currentCounts      map[string]int
-	counterLock        sync.Mutex
+	samplingLock       sync.Mutex
 	samplingCounter    int32
 	counterResetPeriod time.Duration
 	counterResetStopCh chan struct{}
@@ -35,7 +35,9 @@ func (s *sample) disableSampling() {
 	disableTimer := time.NewTimer(time.Until(s.config.endTime))
 	<-disableTimer.C
 
+	s.samplingLock.Lock()
 	s.config.enabled = false
+	s.samplingLock.Unlock()
 
 	// stop limit reset job when sampling is disabled
 	s.counterResetStopCh <- struct{}{}
@@ -60,19 +62,23 @@ func (s *sample) samplingCounterReset() {
 }
 
 func (s *sample) resetSamplingCounter() {
-	s.counterLock.Lock()
-	defer s.counterLock.Unlock()
+	s.samplingLock.Lock()
+	defer s.samplingLock.Unlock()
 	s.samplingCounter = 0
 }
 
 // ShouldSampleTransaction - receives the transaction details and returns true to sample it false to not
 func (s *sample) ShouldSampleTransaction(details TransactionDetails) bool {
+	s.samplingLock.Lock()
+	defer s.samplingLock.Unlock()
+
 	// check if sampling is enabled
 	if !s.config.enabled {
 		return false
 	}
 
 	// sampling limit per minute exceeded
+
 	if s.config.limit <= s.samplingCounter {
 		return false
 	}
@@ -83,9 +89,7 @@ func (s *sample) ShouldSampleTransaction(details TransactionDetails) bool {
 		return false
 	}
 
-	s.counterLock.Lock()
 	s.samplingCounter++
-	s.counterLock.Unlock()
 
 	return true
 }

diff --git a/pkg/traceability/sampling/sampling_test.go b/pkg/traceability/sampling/sampling_test.go
@@ -127,7 +127,6 @@ func TestSamplingConfig(t *testing.T) {
 func TestShouldSample(t *testing.T) {
 	type transactionCount struct {
 		successCount int
-		errorCount   int
 	}
 	testCases := []struct {
 		name               string
@@ -143,7 +142,6 @@ func TestShouldSample(t *testing.T) {
 			name: "Limit sampling to 10 per period",
 			apiTransactions: map[string]transactionCount{
 				"id1": {successCount: 1000},
-				"id2": {successCount: 1000},
 			},
 			expectedSampled:    40,
 			limit:              10,
@@ -154,7 +152,6 @@ func TestShouldSample(t *testing.T) {
 			name: "Limit sampling to 100 per period",
 			apiTransactions: map[string]transactionCount{
 				"id1": {successCount: 1000},
-				"id2": {successCount: 1000},
 			},
 			expectedSampled:    400,
 			limit:              100,
@@ -165,7 +162,6 @@ func TestShouldSample(t *testing.T) {
 			name: "Limit sampling to 1000 per period",
 			apiTransactions: map[string]transactionCount{
 				"id1": {successCount: 1000},
-				"id2": {successCount: 1000},
 			},
 			expectedSampled:    4000,
 			limit:              1000,
@@ -176,7 +172,6 @@ func TestShouldSample(t *testing.T) {
 			name: "Limit sampling to 0",
 			apiTransactions: map[string]transactionCount{
 				"id1": {successCount: 1000},
-				"id2": {successCount: 1000},
 			},
 			expectedSampled:    0,
 			limit:              0,
@@ -234,19 +229,15 @@ func TestShouldSample(t *testing.T) {
 							for i := 0; i < calls.successCount; i++ {
 								sampleFunc(id, subID, "Success")
 							}
-
-							for i := 0; i < calls.errorCount; i++ {
-								sampleFunc(id, subID, "Failure")
-							}
 						}
 					}
 				}(&waitGroup, apiID, subID, numCalls)
 			}
 
 			waitGroup.Wait()
 			assert.Nil(t, err)
-			assert.Equal(t, 1, sampled)
-			assert.False(t, test.expectedSampled <= sampled && test.expectedSampled+int(test.limit) >= sampled)
+			assert.LessOrEqual(t, test.expectedSampled, sampled)
+			assert.GreaterOrEqual(t, test.expectedSampled+int(test.limit), sampled)
 		})
 	}
 }