Skip to content

Commit

Permalink
Update sampling decision names (#1192)
Browse files Browse the repository at this point in the history
* Rename SamplingDecision enum values

As prescribed in
open-telemetry/opentelemetry-specification#938
and open-telemetry/opentelemetry-specification#956.

* Include in Changelog

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
  • Loading branch information
evantorrie and MrAlias authored Sep 22, 2020
1 parent b9357d7 commit e7e1dce
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 31 deletions.
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

0 comments on commit e7e1dce

Please sign in to comment.