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

Update sampling decision names #1192

Merged
Merged
Show file tree
Hide file tree
Changes from 2 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 @@ -46,6 +46,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Rename `go.opentelemetry.io/otel/api/metric.ConfigureInstrument` to `NewInstrumentConfig` and
`go.opentelemetry.io/otel/api/metric.ConfigureMeter` to `NewMeterConfig`.
- Move the `go.opentelemetry.io/otel/api/unit` package to `go.opentelemetry.io/otel/unit`. (#1185)
- Renamed `SamplingDecision` values to comply with OpenTelemetry specification change. (#1192)

### Fixed

Expand Down
23 changes: 15 additions & 8 deletions sdk/trace/sampling.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,21 @@ type SamplingParameters struct {
Links []api.Link
}

// SamplingDecision indicates whether a span is recorded and sampled.
// SamplingDecision indicates whether a span is dropped, recorded and/or sampled.
type SamplingDecision uint8

// Valid sampling decisions
const (
NotRecord SamplingDecision = iota
Record
RecordAndSampled
// Drop will not record the span and all attributes/events will be dropped
Drop SamplingDecision = iota

// Record indicates the span's `IsRecording() == true`, but `Sampled` flag
// *must not* be set
RecordOnly

// RecordAndSample has span's `IsRecording() == true` and `Sampled` flag
// *must* be set
RecordAndSample
)

// SamplingResult conveys a SamplingDecision and a set of Attributes.
Expand All @@ -63,9 +70,9 @@ type traceIDRatioSampler struct {
func (ts traceIDRatioSampler) ShouldSample(p SamplingParameters) SamplingResult {
x := binary.BigEndian.Uint64(p.TraceID[0:8]) >> 1
if x < ts.traceIDUpperBound {
return SamplingResult{Decision: RecordAndSampled}
return SamplingResult{Decision: RecordAndSample}
}
return SamplingResult{Decision: NotRecord}
return SamplingResult{Decision: Drop}
}

func (ts traceIDRatioSampler) Description() string {
Expand Down Expand Up @@ -95,7 +102,7 @@ func TraceIDRatioBased(fraction float64) Sampler {
type alwaysOnSampler struct{}

func (as alwaysOnSampler) ShouldSample(p SamplingParameters) SamplingResult {
return SamplingResult{Decision: RecordAndSampled}
return SamplingResult{Decision: RecordAndSample}
}

func (as alwaysOnSampler) Description() string {
Expand All @@ -113,7 +120,7 @@ func AlwaysSample() Sampler {
type alwaysOffSampler struct{}

func (as alwaysOffSampler) ShouldSample(p SamplingParameters) SamplingResult {
return SamplingResult{Decision: NotRecord}
return SamplingResult{Decision: Drop}
}

func (as alwaysOffSampler) Description() string {
Expand Down
36 changes: 18 additions & 18 deletions sdk/trace/sampling_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ func TestParentBasedDefaultLocalParentSampled(t *testing.T) {
SpanID: spanID,
TraceFlags: api.FlagsSampled,
}
if sampler.ShouldSample(SamplingParameters{ParentContext: parentCtx}).Decision != RecordAndSampled {
t.Error("Sampling decision should be RecordAndSampled")
if sampler.ShouldSample(SamplingParameters{ParentContext: parentCtx}).Decision != RecordAndSample {
t.Error("Sampling decision should be RecordAndSample")
}
}

Expand All @@ -46,22 +46,22 @@ func TestParentBasedDefaultLocalParentNotSampled(t *testing.T) {
TraceID: traceID,
SpanID: spanID,
}
if sampler.ShouldSample(SamplingParameters{ParentContext: parentCtx}).Decision != NotRecord {
t.Error("Sampling decision should be NotRecord")
if sampler.ShouldSample(SamplingParameters{ParentContext: parentCtx}).Decision != Drop {
t.Error("Sampling decision should be Drop")
}
}

func TestParentBasedWithNoParent(t *testing.T) {
params := SamplingParameters{}

sampler := ParentBased(AlwaysSample())
if sampler.ShouldSample(params).Decision != RecordAndSampled {
t.Error("Sampling decision should be RecordAndSampled")
if sampler.ShouldSample(params).Decision != RecordAndSample {
t.Error("Sampling decision should be RecordAndSample")
}

sampler = ParentBased(NeverSample())
if sampler.ShouldSample(params).Decision != NotRecord {
t.Error("Sampling decision should be NotRecord")
if sampler.ShouldSample(params).Decision != Drop {
t.Error("Sampling decision should be Drop")
}
}

Expand All @@ -77,28 +77,28 @@ func TestParentBasedWithSamplerOptions(t *testing.T) {
WithLocalParentSampled(NeverSample()),
false,
true,
NotRecord,
Drop,
},
{
"localParentNotSampled",
WithLocalParentNotSampled(AlwaysSample()),
false,
false,
RecordAndSampled,
RecordAndSample,
},
{
"remoteParentSampled",
WithRemoteParentSampled(NeverSample()),
true,
true,
NotRecord,
Drop,
},
{
"remoteParentNotSampled",
WithRemoteParentNotSampled(AlwaysSample()),
true,
false,
RecordAndSampled,
RecordAndSample,
},
}

Expand Down Expand Up @@ -126,13 +126,13 @@ func TestParentBasedWithSamplerOptions(t *testing.T) {
)

switch tc.expectedDecision {
case RecordAndSampled:
case RecordAndSample:
if sampler.ShouldSample(params).Decision != tc.expectedDecision {
t.Error("Sampling decision should be RecordAndSampled")
t.Error("Sampling decision should be RecordAndSample")
}
case NotRecord:
case Drop:
if sampler.ShouldSample(params).Decision != tc.expectedDecision {
t.Error("Sampling decision should be NotRecord")
t.Error("Sampling decision should be Drop")
}
}
})
Expand Down Expand Up @@ -181,8 +181,8 @@ func TestTraceIdRatioSamplesInclusively(t *testing.T) {
traceID := idg.NewTraceID()

params := SamplingParameters{TraceID: traceID}
if samplerLo.ShouldSample(params).Decision == RecordAndSampled {
require.Equal(t, RecordAndSampled, samplerHi.ShouldSample(params).Decision,
if samplerLo.ShouldSample(params).Decision == RecordAndSample {
require.Equal(t, RecordAndSample, samplerHi.ShouldSample(params).Decision,
"%s sampled but %s did not", samplerLo.Description(), samplerHi.Description())
}
}
Expand Down
6 changes: 3 additions & 3 deletions sdk/trace/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,15 +426,15 @@ func makeSamplingDecision(data samplingData) SamplingResult {
Attributes: data.attributes,
Links: data.links,
})
if sampled.Decision == RecordAndSampled {
if sampled.Decision == RecordAndSample {
spanContext.TraceFlags |= apitrace.FlagsSampled
} else {
spanContext.TraceFlags &^= apitrace.FlagsSampled
}
return sampled
}
if data.parent.TraceFlags&apitrace.FlagsSampled != 0 {
return SamplingResult{Decision: RecordAndSampled}
return SamplingResult{Decision: RecordAndSample}
}
return SamplingResult{Decision: NotRecord}
return SamplingResult{Decision: Drop}
}
4 changes: 2 additions & 2 deletions sdk/trace/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,9 @@ type testSampler struct {
func (ts *testSampler) ShouldSample(p SamplingParameters) SamplingResult {
ts.callCount++
ts.t.Logf("called sampler for name %q", p.Name)
decision := NotRecord
decision := Drop
if strings.HasPrefix(p.Name, ts.prefix) {
decision = RecordAndSampled
decision = RecordAndSample
}
return SamplingResult{Decision: decision, Attributes: []label.KeyValue{label.Int("callCount", ts.callCount)}}
}
Expand Down