Call sampler on local child spans. (#1233)

* Call sampler on local child spans.

* Update CHANGELOG.md

* Adding a test for calling the sampler on local child spans

* Add clarifying comment to test

CHANGELOG.md

@@ -17,6 +17,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
   - `EmptySpanContext` is removed.
 - Move the `go.opentelemetry.io/otel/api/trace/tracetest` package into `go.opentelemetry.io/otel/oteltest`. (#1229)
 - OTLP Exporter supports OTLP v0.5.0. (#1230)
+- The Sampler is now called on local child spans. (#1233)
 
 ## [0.13.0] - 2020-10-08
 

sdk/trace/span.go

@@ -397,36 +397,21 @@ type samplingData struct {
 }
 
 func makeSamplingDecision(data samplingData) SamplingResult {
-	if data.noParent || data.remoteParent {
-		// If this span is the child of a local span and no
-		// Sampler is set in the options, keep the parent's
-		// TraceFlags.
-		//
-		// Otherwise, consult the Sampler in the options if it
-		// is non-nil, otherwise the default sampler.
-		sampler := data.cfg.DefaultSampler
-		//if o.Sampler != nil {
-		//	sampler = o.Sampler
-		//}
-		spanContext := &data.span.spanContext
-		sampled := sampler.ShouldSample(SamplingParameters{
-			ParentContext:   data.parent,
-			TraceID:         spanContext.TraceID,
-			Name:            data.name,
-			HasRemoteParent: data.remoteParent,
-			Kind:            data.kind,
-			Attributes:      data.attributes,
-			Links:           data.links,
-		})
-		if sampled.Decision == RecordAndSample {
-			spanContext.TraceFlags |= otel.FlagsSampled
-		} else {
-			spanContext.TraceFlags &^= otel.FlagsSampled
-		}
-		return sampled
-	}
-	if data.parent.TraceFlags&otel.FlagsSampled != 0 {
-		return SamplingResult{Decision: RecordAndSample}
+	sampler := data.cfg.DefaultSampler
+	spanContext := &data.span.spanContext
+	sampled := sampler.ShouldSample(SamplingParameters{
+		ParentContext:   data.parent,
+		TraceID:         spanContext.TraceID,
+		Name:            data.name,
+		HasRemoteParent: data.remoteParent,
+		Kind:            data.kind,
+		Attributes:      data.attributes,
+		Links:           data.links,
+	})
+	if sampled.Decision == RecordAndSample {
+		spanContext.TraceFlags |= otel.FlagsSampled
+	} else {
+		spanContext.TraceFlags &^= otel.FlagsSampled
 	}
-	return SamplingResult{Decision: Drop}
+	return sampled
 }

sdk/trace/trace_test.go

@@ -399,6 +399,72 @@ func TestSetSpanAttributes(t *testing.T) {
 	}
 }
 
+// Test that the sampler is called for local child spans. This is verified by checking
+// that the attributes set in the sampler are set on the child span.
+func TestSamplerAttributesLocalChildSpan(t *testing.T) {
+	sampler := &testSampler{prefix: "span", t: t}
+	te := NewTestExporter()
+	tp := NewTracerProvider(WithConfig(Config{DefaultSampler: sampler}), WithSyncer(te))
+
+	ctx := context.Background()
+	ctx, span := startLocalSpan(tp, ctx, "SpanOne", "span0")
+	_, spanTwo := startLocalSpan(tp, ctx, "SpanTwo", "span1")
+
+	spanTwo.End()
+	span.End()
+
+	got := te.Spans()
+
+	// endSpan expects only a single span in the test exporter, so manually clear the
+	// fields that can't be tested for easily (times, span and trace ids).
+	pid := got[0].SpanContext.SpanID
+	got[0].SpanContext.TraceID = tid
+	got[0].ParentSpanID = sid
+
+	checkTime(&got[0].StartTime)
+	checkTime(&got[0].EndTime)
+
+	got[1].SpanContext.SpanID = otel.SpanID{}
+	got[1].SpanContext.TraceID = tid
+	got[1].ParentSpanID = pid
+	got[0].SpanContext.SpanID = otel.SpanID{}
+
+	checkTime(&got[1].StartTime)
+	checkTime(&got[1].EndTime)
+
+	want := []*export.SpanData{
+		{
+			SpanContext: otel.SpanContext{
+				TraceID:    tid,
+				TraceFlags: 0x1,
+			},
+			ParentSpanID:           sid,
+			Name:                   "span1",
+			Attributes:             []label.KeyValue{label.Int("callCount", 2)},
+			SpanKind:               otel.SpanKindInternal,
+			HasRemoteParent:        false,
+			InstrumentationLibrary: instrumentation.Library{Name: "SpanTwo"},
+		},
+		{
+			SpanContext: otel.SpanContext{
+				TraceID:    tid,
+				TraceFlags: 0x1,
+			},
+			ParentSpanID:           pid,
+			Name:                   "span0",
+			Attributes:             []label.KeyValue{label.Int("callCount", 1)},
+			SpanKind:               otel.SpanKindInternal,
+			HasRemoteParent:        false,
+			ChildSpanCount:         1,
+			InstrumentationLibrary: instrumentation.Library{Name: "SpanOne"},
+		},
+	}
+
+	if diff := cmpDiff(got, want); diff != "" {
+		t.Errorf("SetSpanAttributesLocalChildSpan: -got +want %s", diff)
+	}
+}
+
 func TestSetSpanAttributesOverLimit(t *testing.T) {
 	te := NewTestExporter()
 	cfg := Config{MaxAttributesPerSpan: 2}
@@ -730,6 +796,20 @@ func startNamedSpan(tp *TracerProvider, trName, name string, args ...otel.SpanOp
 	return span
 }
 
+// startLocalSpan is a test utility func that starts a span with a
+// passed name and with the passed context. The context is returned
+// along with the span so this parent can be used to create child
+// spans.
+func startLocalSpan(tp *TracerProvider, ctx context.Context, trName, name string, args ...otel.SpanOption) (context.Context, otel.Span) {
+	args = append(args, otel.WithRecord())
+	ctx, span := tp.Tracer(trName).Start(
+		ctx,
+		name,
+		args...,
+	)
+	return ctx, span
+}
+
 // endSpan is a test utility function that ends the span in the context and
 // returns the exported export.SpanData.
 // It requires that span be sampled using one of these methods