[sampling] Fix merging of per-operation strategies into service strategies without them

plugin/sampling/strategystore/static/fixtures/probablistic_with_default_operation_strategy.json

@@ -0,0 +1,14 @@
+{
+    "probabilisticSampling": {
+        "samplingRate": 1
+    },
+    "operationSampling": {
+        "defaultSamplingProbability": 1,
+        "perOperationStrategies": [
+            {
+                "operation": "/health",
+                "probabilisticSampling": {}
+            }
+        ]
+    }
+}

plugin/sampling/strategystore/static/fixtures/ratelimiting_with_default_operation_strategy.json

@@ -0,0 +1,15 @@
+{
+    "strategyType": 1,
+    "rateLimitingSampling": {
+        "maxTracesPerSecond": 3
+    },
+    "operationSampling": {
+        "defaultSamplingProbability": 0.2,
+        "perOperationStrategies": [
+            {
+                "operation": "/health",
+                "probabilisticSampling": {}
+            }
+        ]
+    }
+}

plugin/sampling/strategystore/static/strategy_store.go

@@ -229,13 +229,16 @@ func (h *strategyStore) parseStrategies(strategies *strategies) {
 		// is not merged with and only used as a fallback).
 		opS := newStore.serviceStrategies[s.Service].OperationSampling
 		if opS == nil {
-			if newStore.defaultStrategy.OperationSampling == nil ||
-				newStore.serviceStrategies[s.Service].ProbabilisticSampling == nil {
+			// no default per-operation strategies - just move on
+			if newStore.defaultStrategy.OperationSampling == nil {
 				continue
 			}
-			// Service has no per-operation strategies, so just reference the default settings and change default samplingRate.
+			// Service has no per-operation strategies, so just reference the default settings
+			// and change default samplingRate to the value defined in service strategy if value is available.
 			newOpS := *newStore.defaultStrategy.OperationSampling
-			newOpS.DefaultSamplingProbability = newStore.serviceStrategies[s.Service].ProbabilisticSampling.SamplingRate
+			if newStore.serviceStrategies[s.Service].ProbabilisticSampling != nil {
+				newOpS.DefaultSamplingProbability = newStore.serviceStrategies[s.Service].ProbabilisticSampling.SamplingRate
+			}
 			newStore.serviceStrategies[s.Service].OperationSampling = &newOpS
 			continue
 		}

plugin/sampling/strategystore/static/strategy_store_test.go

@@ -16,10 +16,12 @@ package static
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"path/filepath"
 	"strings"
 	"sync/atomic"
 	"testing"
@@ -465,18 +467,44 @@ func TestAutoUpdateStrategyErrors(t *testing.T) {
 }
 
 func TestServiceNoPerOperationStrategies(t *testing.T) {
+	// given setup of strategy store with no specific per operation sampling strategies
 	store, err := NewStrategyStore(Options{StrategiesFile: "fixtures/service_no_per_operation.json"}, zap.NewNop())
 	require.NoError(t, err)
 
-	s, err := store.GetSamplingStrategy(context.Background(), "ServiceA")
+	// expected response for ServiceA (which has probablistic service level sampling strategy)
+	// shall contain /health operation with default service level sampling strategy as well
+	expectedServiceAResponse, errServiceA := makeServiceStrategyResponseFromFile("fixtures/probablistic_with_default_operation_strategy.json")
+	require.NoError(t, errServiceA)
+
+	strategy, err := store.GetSamplingStrategy(context.Background(), "ServiceA")
 	require.NoError(t, err)
-	assert.Equal(t, 1.0, s.OperationSampling.DefaultSamplingProbability)
+	require.NotNil(t, strategy.OperationSampling)
+
+	assert.Equal(t, expectedServiceAResponse, strategy)
 
-	s, err = store.GetSamplingStrategy(context.Background(), "ServiceB")
+	// expected response for ServiceB (which has ratelimiting service level sampling strategy)
+	// shall contain /health operation with sampling probability taken from defaul operation level strategy
+	expectedServiceBResponse, errServiceB := makeServiceStrategyResponseFromFile("fixtures/ratelimiting_with_default_operation_strategy.json")
+	require.NoError(t, errServiceB)
+
+	strategy, err = store.GetSamplingStrategy(context.Background(), "ServiceB")
 	require.NoError(t, err)
+	require.NotNil(t, strategy.OperationSampling)
 
-	expected := makeResponse(api_v2.SamplingStrategyType_RATE_LIMITING, 3)
-	assert.Equal(t, *expected.RateLimitingSampling, *s.RateLimitingSampling)
+	assert.Equal(t, expectedServiceBResponse, strategy)
+}
+
+func makeServiceStrategyResponseFromFile(serviceStrategyFile string) (resp *api_v2.SamplingStrategyResponse, err error) {
+	strategyBytes, err := os.ReadFile(filepath.Clean(serviceStrategyFile))
+	if err != nil {
+		return nil, fmt.Errorf("failed to read SamplingStrategyResponse file %s: %w", serviceStrategyFile, err)
+	}
+	err = json.Unmarshal(strategyBytes, &resp)
+	if err != nil {
+		return nil, fmt.Errorf("failed to unmarshal SamplingStrategyResponse: %w", err)
+	} else {
+		return resp, nil
+	}
 }
 
 func TestSamplingStrategyLoader(t *testing.T) {