XRay Remote Sampler - Preserve previous rule reservoir if rule property has not changed

CHANGELOG.md

@@ -41,6 +41,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - AWS SDK rename attributes `aws.operation`, `aws.service` to `rpc.method`,`rpc.service` in `go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws`. (#3582, #3617) 
 - AWS SDK span name to be of the format `Service.Operation` in `go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws`. (#3582, #3521)
 - Prevent sampler configuration reset from erroneously sampling first span in `go.opentelemetry.io/contrib/samplers/jaegerremote`. (#3603, #3604)
+- AWS XRay Remote Sampling to preserve previous rule if updated rule property has not changed in `go.opentelemetry.io/contrib/samplers/aws/xray`. (#3619, #3620)
 
 ## [1.16.0-rc.1/0.41.0-rc.1/0.10.0-rc.1] - 2023-03-02
 

samplers/aws/xray/internal/manifest.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"math"
 	"net/url"
+	"reflect"
 	"sort"
 	"strings"
 	"sync"
@@ -183,7 +184,22 @@ func (m *Manifest) updateRules(rules *getSamplingRulesOutput) {
 	// Re-sort to fix matching priorities.
 	tempManifest.sort()
 
+	var currentRuleMap = make(map[string]Rule)
+
 	m.mu.Lock()
+	for _, rule := range m.Rules {
+		currentRuleMap[rule.ruleProperties.RuleName] = rule
+	}
+
+	// Preserve entire Rule if newRule.ruleProperties == curRule.ruleProperties
+	for i, newRule := range tempManifest.Rules {
+		if curRule, ok := currentRuleMap[newRule.ruleProperties.RuleName]; ok {
+			if reflect.DeepEqual(newRule.ruleProperties, curRule.ruleProperties) {
+				tempManifest.Rules[i] = curRule
+			}
+		}
+	}
+
 	m.Rules = tempManifest.Rules
 	m.refreshedAt = m.clock.now()
 	m.mu.Unlock()

samplers/aws/xray/internal/manifest_test.go

@@ -1644,6 +1644,143 @@ func TestRaceUpdatingRulesAndTargetsWhileMatching(t *testing.T) {
 	}
 }
 
+// Validate Rules are preserved when a rule is updated with the same ruleProperties.
+func TestPreserveRulesWithSameRuleProperties(t *testing.T) {
+	// getSamplingRules response to update existing manifest rule, with matching ruleProperties
+	ruleRecords := samplingRuleRecords{
+		SamplingRule: &ruleProperties{
+			RuleName:      "r1",
+			Priority:      10000,
+			Host:          "localhost",
+			HTTPMethod:    "*",
+			URLPath:       "/test/path",
+			ReservoirSize: 40,
+			FixedRate:     0.9,
+			Version:       1,
+			ServiceName:   "helios",
+			ResourceARN:   "*",
+			ServiceType:   "*",
+		},
+	}
+
+	// existing rule already present in manifest
+	r1 := Rule{
+		ruleProperties: ruleProperties{
+			RuleName:      "r1",
+			Priority:      10000,
+			Host:          "localhost",
+			HTTPMethod:    "*",
+			URLPath:       "/test/path",
+			ReservoirSize: 40,
+			FixedRate:     0.9,
+			Version:       1,
+			ServiceName:   "helios",
+			ResourceARN:   "*",
+			ServiceType:   "*",
+		},
+		reservoir: &reservoir{
+			capacity:     100,
+			quota:        100,
+			quotaBalance: 80,
+			refreshedAt:  time.Unix(13000000, 0),
+		},
+		samplingStatistics: &samplingStatistics{
+			matchedRequests:  500,
+			sampledRequests:  10,
+			borrowedRequests: 0,
+		},
+	}
+	clock := &mockClock{
+		nowTime: 1500000000,
+	}
+
+	rules := []Rule{r1}
+
+	m := &Manifest{
+		Rules: rules,
+		clock: clock,
+	}
+
+	// Update rules
+	m.updateRules(&getSamplingRulesOutput{
+		SamplingRuleRecords: []*samplingRuleRecords{&ruleRecords},
+	})
+
+	require.Equal(t, r1.reservoir, m.Rules[0].reservoir)
+	require.Equal(t, r1.samplingStatistics, m.Rules[0].samplingStatistics)
+}
+
+// Validate Rules are NOT preserved when a rule is updated with a different ruleProperties with the same RuleName.
+func TestDoNotPreserveRulesWithDifferentRuleProperties(t *testing.T) {
+	// getSamplingRules response to update existing manifest rule, with different ruleProperties
+	ruleRecords := samplingRuleRecords{
+		SamplingRule: &ruleProperties{
+			RuleName:      "r1",
+			Priority:      10000,
+			Host:          "localhost",
+			HTTPMethod:    "*",
+			URLPath:       "/test/path",
+			ReservoirSize: 40,
+			FixedRate:     0.9,
+			Version:       1,
+			ServiceName:   "helios",
+			ResourceARN:   "*",
+			ServiceType:   "*",
+		},
+	}
+
+	// existing rule already present in manifest
+	r1 := Rule{
+		ruleProperties: ruleProperties{
+			RuleName:      "r1",
+			Priority:      10001,
+			Host:          "localhost",
+			HTTPMethod:    "*",
+			URLPath:       "/test/path",
+			ReservoirSize: 40,
+			FixedRate:     0.9,
+			Version:       1,
+			ServiceName:   "helios",
+			ResourceARN:   "*",
+			ServiceType:   "*",
+		},
+		reservoir: &reservoir{
+			capacity:     100,
+			quota:        100,
+			quotaBalance: 80,
+			refreshedAt:  time.Unix(13000000, 0),
+		},
+		samplingStatistics: &samplingStatistics{
+			matchedRequests:  500,
+			sampledRequests:  10,
+			borrowedRequests: 0,
+		},
+	}
+	clock := &mockClock{
+		nowTime: 1500000000,
+	}
+
+	rules := []Rule{r1}
+
+	m := &Manifest{
+		Rules: rules,
+		clock: clock,
+	}
+
+	// Update rules
+	m.updateRules(&getSamplingRulesOutput{
+		SamplingRuleRecords: []*samplingRuleRecords{&ruleRecords},
+	})
+
+	require.Equal(t, m.Rules[0].reservoir.quota, 0.0)
+	require.Equal(t, m.Rules[0].reservoir.quotaBalance, 0.0)
+	require.Equal(t, *m.Rules[0].samplingStatistics, samplingStatistics{
+		matchedRequests:  0,
+		sampledRequests:  0,
+		borrowedRequests: 0,
+	})
+}
+
 // validate no data race is when capturing sampling statistics in manifest while sampling.
 func TestRaceUpdatingSamplingStatisticsWhenSampling(t *testing.T) {
 	// existing rule already present in manifest