Respect default sampling strategy at service level (#2230)

* Set default sampling for service which has no per-operation strategies Signed-off-by: defool <[email protected]> * Consider sampling type of ratelimiting Signed-off-by: defool <[email protected]>
jaegertracing · May 10, 2020 · 2ea2be9 · 2ea2be9
1 parent 0378e5b
commit 2ea2be9
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 3 deletions.
diff --git a/plugin/sampling/strategystore/static/fixtures/service_no_per_operation.json b/plugin/sampling/strategystore/static/fixtures/service_no_per_operation.json
@@ -0,0 +1,25 @@
+  {
+       "service_strategies": [
+         {
+           "service": "ServiceA",
+           "type": "probabilistic",
+           "param": 1.0
+         },
+         {
+          "service": "ServiceB",
+          "type": "ratelimiting",
+          "param": 3
+        }
+       ],
+       "default_strategy": {
+         "type": "probabilistic",
+         "param": 0.2,
+         "operation_strategies": [
+           {
+             "operation": "/health",
+             "type": "probabilistic",
+             "param": 0.0
+           }
+         ]
+       }
+     }
diff --git a/plugin/sampling/strategystore/static/strategy_store.go b/plugin/sampling/strategystore/static/strategy_store.go
@@ -164,11 +164,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 {
-			// Service has no per-operation strategies, so just reference the default settings.
-			newStore.serviceStrategies[s.Service].OperationSampling = newStore.defaultStrategy.OperationSampling
+			if newStore.defaultStrategy.OperationSampling == nil ||
+				newStore.serviceStrategies[s.Service].ProbabilisticSampling == nil {
+				continue
+			}
+			// Service has no per-operation strategies, so just reference the default settings and change default samplingRate.
+			newOpS := *newStore.defaultStrategy.OperationSampling
+			newOpS.DefaultSamplingProbability = newStore.serviceStrategies[s.Service].ProbabilisticSampling.SamplingRate
+			newStore.serviceStrategies[s.Service].OperationSampling = &newOpS
 			continue
 		}
-
 		if merge {
 			opS.PerOperationStrategies = mergePerOperationSamplingStrategies(
 				opS.PerOperationStrategies,

diff --git a/plugin/sampling/strategystore/static/strategy_store_test.go b/plugin/sampling/strategystore/static/strategy_store_test.go
@@ -321,3 +321,18 @@ func TestAutoUpdateStrategyErrors(t *testing.T) {
 	assert.Equal(t, "blah", store.reloadSamplingStrategyFile(tempFile.Name(), "blah"))
 	assert.Len(t, logs.FilterMessage("failed to update sampling strategies from file").All(), 1)
 }
+
+func TestServiceNoPerOperationStrategies(t *testing.T) {
+	store, err := NewStrategyStore(Options{StrategiesFile: "fixtures/service_no_per_operation.json"}, zap.NewNop())
+	require.NoError(t, err)
+
+	s, err := store.GetSamplingStrategy("ServiceA")
+	require.NoError(t, err)
+	assert.Equal(t, 1.0, s.OperationSampling.DefaultSamplingProbability)
+
+	s, err = store.GetSamplingStrategy("ServiceB")
+	require.NoError(t, err)
+
+	expected := makeResponse(sampling.SamplingStrategyType_RATE_LIMITING, 3)
+	assert.Equal(t, *expected.RateLimitingSampling, *s.RateLimitingSampling)
+}