Repository: open-telemetry/opentelemetry-go
Pull request #455 (2020-01-30 22:02:24)

Add AlwaysParentSample sampler
Per the `ALWAYS_PARENT` sampler from the spec   https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/sdk-tracing.md#built-in-samplers

amrmahdi: > What should be the behavior if this is a root span? It is more of an issue in the spec since it doesn't define this. If the desired behavior for root is
> 
> * always sample then it is same as probabilitySampler with probability of 1.
> * if do not sample then it is same as probabilitySampler with probability of 0.
> * if sample as certain probability x then it is same as probabilitySampler with x.
> 
> Essentially it is same as probabilitySampler.
> 
> The other thing is that AlwaysParentSampler is same as ProbabilitySampler with probability of 1. If there is a parent and then simply follow the decision on the parent. That is same as AlwaysParentSample (if we clarify what should be the behavior for root span for AlwaysParentSampler).

This is a good point. I believe it should be equivalent to ProbabilitySampler with 0. And then we can just wrap the ProbabilitySampler with 0 inside the AlwaysParentSampler and clarify it in the docs. 
rghetia: > This is a good point. I believe it should be equivalent to ProbabilitySampler with 0. And then we can just wrap the ProbabilitySampler with 0 inside the AlwaysParentSampler and clarify it in the docs.

I agree on ProbabilitySampler with 0.
Actually, spec implicitly implies that on root span do not sample. From spec `Returns RECORD_AND_SAMPLED if SampledFlag is set to true on parent SpanContext and NOT_RECORD otherwise.`

Patch:
diff --git a/sdk/trace/sampling.go b/sdk/trace/sampling.go
index 73675236..4f91d4d9 100644
--- a/sdk/trace/sampling.go
+++ b/sdk/trace/sampling.go
@@ -77,3 +77,11 @@ func NeverSample() Sampler {
 		return SamplingDecision{Sample: false}
 	}
 }
+
+// AlwaysParentSample returns a Sampler that samples a trace only
+// if the parent span is sampled.
+// This Sampler is a passthrough to the ProbabilitySampler with
+// a fraction of value 0.
+func AlwaysParentSample() Sampler {
+	return ProbabilitySampler(0)
+}


Test patch:
diff --git a/sdk/trace/sampling_test.go b/sdk/trace/sampling_test.go
new file mode 100644
index 00000000..488bfe94
--- /dev/null
+++ b/sdk/trace/sampling_test.go
@@ -0,0 +1,49 @@
+// Copyright 2020, OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package trace_test
+
+import (
+	"testing"
+
+	"go.opentelemetry.io/otel/api/core"
+	sdktrace "go.opentelemetry.io/otel/sdk/trace"
+)
+
+func TestAlwaysParentSampleWithParentSampled(t *testing.T) {
+	sampler := sdktrace.AlwaysParentSample()
+	traceID, _ := core.TraceIDFromHex("4bf92f3577b34da6a3ce929d0e0e4736")
+	spanID, _ := core.SpanIDFromHex("00f067aa0ba902b7")
+	parentCtx := core.SpanContext{
+		TraceID:    traceID,
+		SpanID:     spanID,
+		TraceFlags: core.TraceFlagsSampled,
+	}
+	if !sampler(sdktrace.SamplingParameters{ParentContext: parentCtx}).Sample {
+		t.Error("Sampling decision should be true")
+	}
+}
+
+func TestAlwaysParentSampleWithParentNotSampled(t *testing.T) {
+	sampler := sdktrace.AlwaysParentSample()
+	traceID, _ := core.TraceIDFromHex("4bf92f3577b34da6a3ce929d0e0e4736")
+	spanID, _ := core.SpanIDFromHex("00f067aa0ba902b7")
+	parentCtx := core.SpanContext{
+		TraceID: traceID,
+		SpanID:  spanID,
+	}
+	if sampler(sdktrace.SamplingParameters{ParentContext: parentCtx}).Sample {
+		t.Error("Sampling decision should be false")
+	}
+}