Merge branch 'main' into kitschysynq/use-server-span-for-status

CHANGELOG.md

@@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 - Fix the `otelmux` middleware by using `SpanKindServer` when deciding the `SpanStatus`.
   This makes `4xx` response codes to not be an error anymore. (#1973)
+- Fixed jaegerremote sampler not behaving properly with per operation strategy set. (#2137)
 
 ## [1.6.0/0.31.0] - 2022-03-28
 
@@ -24,6 +25,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Upgraded all dependencies on stable modules from `go.opentelemetry.io/otel` from v1.5.0 to v1.6.1. (#2134)
 - Upgraded all dependencies on metric modules from `go.opentelemetry.io/otel` from v0.27.0 to v0.28.0. (#1977)
 
+### Fixed
+
+- otelhttp: Avoid panic by adding nil check to `wrappedBody.Close` (#2164)
+
 ## [1.5.0/0.30.0/0.1.0] - 2022-03-16
 
 ### Added

instrumentation/net/http/otelhttp/transport.go

@@ -186,5 +186,8 @@ func (wb *wrappedBody) Read(b []byte) (int, error) {
 
 func (wb *wrappedBody) Close() error {
 	wb.span.End()
-	return wb.body.Close()
+	if wb.body != nil {
+		return wb.body.Close()
+	}
+	return nil
 }

instrumentation/net/http/otelhttp/transport_test.go

@@ -281,6 +281,13 @@ func TestWrappedBodyClose(t *testing.T) {
 	s.assert(t, true, nil, codes.Unset, "")
 }
 
+func TestWrappedBodyClosePanic(t *testing.T) {
+	s := new(span)
+	var body io.ReadCloser
+	wb := newWrappedBody(s, body)
+	assert.NotPanics(t, func() { wb.Close() }, "nil body should not panic on close")
+}
+
 func TestWrappedBodyCloseError(t *testing.T) {
 	s := new(span)
 	expectedErr := errors.New("test")

samplers/jaegerremote/sampler_remote_options.go

@@ -52,12 +52,10 @@ func newConfig(options ...Option) config {
 	for _, option := range options {
 		option.apply(&c)
 	}
-	c.updaters = append(c.updaters,
-		&perOperationSamplerUpdater{
-			MaxOperations:            c.posParams.MaxOperations,
-			OperationNameLateBinding: c.posParams.OperationNameLateBinding,
-		})
-
+	c.updaters = append([]samplerUpdater{&perOperationSamplerUpdater{
+		MaxOperations:            c.posParams.MaxOperations,
+		OperationNameLateBinding: c.posParams.OperationNameLateBinding,
+	}}, c.updaters...)
 	return c
 }
 

samplers/jaegerremote/sampler_remote_test.go

@@ -125,7 +125,7 @@ func TestRemoteSamplerOptions(t *testing.T) {
 	assert.Equal(t, 42*time.Second, sampler.samplingRefreshInterval)
 	assert.Same(t, fetcher, sampler.samplingFetcher)
 	assert.Same(t, parser, sampler.samplingParser)
-	assert.Same(t, updaters[0], sampler.updaters[0])
+	assert.EqualValues(t, sampler.updaters[0], &perOperationSamplerUpdater{MaxOperations: 42, OperationNameLateBinding: true})
 }
 
 func TestRemoteSamplerOptionsDefaults(t *testing.T) {
@@ -281,6 +281,45 @@ func TestRemotelyControlledSampler_updateSampler(t *testing.T) {
 	}
 }
 
+func TestRemotelyControlledSampler_multiStrategyResponse(t *testing.T) {
+	agent, sampler := initAgent(t)
+	defer agent.Close()
+	initSampler, ok := sampler.sampler.(*probabilisticSampler)
+	assert.True(t, ok)
+
+	defaultSampingRate := 1.0
+	testUnusedOpName := "unused_op"
+	testUnusedOpSamplingRate := 0.0
+
+	res := &jaeger_api_v2.SamplingStrategyResponse{
+		StrategyType:          jaeger_api_v2.SamplingStrategyType_PROBABILISTIC,
+		ProbabilisticSampling: &jaeger_api_v2.ProbabilisticSamplingStrategy{SamplingRate: defaultSampingRate},
+		OperationSampling: &jaeger_api_v2.PerOperationSamplingStrategies{
+			DefaultSamplingProbability:       defaultSampingRate,
+			DefaultLowerBoundTracesPerSecond: 0.001,
+			PerOperationStrategies: []*jaeger_api_v2.OperationSamplingStrategy{
+				{
+					Operation: testUnusedOpName,
+					ProbabilisticSampling: &jaeger_api_v2.ProbabilisticSamplingStrategy{
+						SamplingRate: testUnusedOpSamplingRate,
+					}},
+			},
+		},
+	}
+
+	agent.AddSamplingStrategy("client app", res)
+	sampler.UpdateSampler()
+	s, ok := sampler.sampler.(*perOperationSampler)
+	assert.True(t, ok)
+	assert.NotEqual(t, initSampler, sampler.sampler, "Sampler should have been updated")
+	assert.Equal(t, defaultSampingRate, s.defaultSampler.SamplingRate())
+
+	result := sampler.ShouldSample(makeSamplingParameters(testMaxID-10, testUnusedOpName))
+	assert.Equal(t, trace.RecordAndSample, result.Decision) // first call always pass
+	result = sampler.ShouldSample(makeSamplingParameters(testMaxID, testUnusedOpName))
+	assert.Equal(t, trace.Drop, result.Decision)
+}
+
 func TestSamplerQueryError(t *testing.T) {
 	agent, sampler := initAgent(t)
 	defer agent.Close()