Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call sampler on local child spans. #1233

Merged
merged 4 commits into from
Oct 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
47 changes: 16 additions & 31 deletions sdk/trace/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
80 changes: 80 additions & 0 deletions sdk/trace/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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
Expand Down