go/opentelemetry-go
go/opentelemetry-go
1
0
Fork 0
mirror of https://github.com/open-telemetry/opentelemetry-go.git synced 2025-02-05 13:15:41 +02:00
Code Issues Releases Activity

Rename ParentOrElse sampler to ParentBased and enhance it according to the spec (#1153)

Browse Source 
* Rename ParentOrElse to ParentBased and enhance it according to the spec

- Renaming of type and sampler function
- Enhancing ParentBased with sampler options
- Set default samplers for each applicable parent-based case
- Adjust ShouldSample(...) func accordingly

* Adjust existing tests for ParentBased and add new ones

- add tests for ParentBased sampler options and description
- renaming in trace_test.go

* Update CHANGELOG.md

* PR feedback

- More clearer naming of structs; add comments where missing
- Adhere to the configuration style guide

* PR feedback - punctuation
This commit is contained in:
Matej Gera 2020-09-10 21:39:04 +02:00 committed by GitHub
parent 34c02d1d24
commit 0041e2d26e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 239 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -32,6 +32,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Move `tools` package under `internal`. (#1141)
- Move `go.opentelemetry.io/otel/api/correlation` package to `go.opentelemetry.io/otel/api/baggage`. (#1142)
The `correlation.CorrelationContext` propagator has been renamed `baggage.Baggage`. Other exported functions and types are unchanged.
- Rename `ParentOrElse` sampler to `ParentBased` and allow setting samplers depending on parent span. (#1153)
- In the `go.opentelemetry.io/otel/api/trace` package, `SpanConfigure` was renamed to `NewSpanConfig`. (#1155)
- Change `dependabot.yml` to add a `Skip Changelog` label to dependabot-sourced PRs. (#1161)

2
sdk/trace/provider.go
View File

@ -64,7 +64,7 @@ func NewProvider(opts ...ProviderOption) *Provider {
namedTracer: make(map[instrumentation.Library]*tracer),
}
tp.config.Store(&Config{
DefaultSampler: ParentSample(AlwaysSample()),
DefaultSampler: ParentBased(AlwaysSample()),
IDGenerator: defIDGenerator(),
MaxAttributesPerSpan: DefaultMaxAttributesPerSpan,
MaxEventsPerSpan: DefaultMaxEventsPerSpan,

146
sdk/trace/sampling.go
View File

@ -125,29 +125,131 @@ func NeverSample() Sampler {
return alwaysOffSampler{}
}
// ParentSample returns a Sampler that samples a trace only
// if the the span has a parent span and it is sampled. If the span has
// parent span but it is not sampled, neither will this span. If the span
// does not have a parent the fallback Sampler is used to determine if the
// span should be sampled.
func ParentSample(fallback Sampler) Sampler {
return parentSampler{fallback}
}
type parentSampler struct {
fallback Sampler
}
func (ps parentSampler) ShouldSample(p SamplingParameters) SamplingResult {
if p.ParentContext.IsValid() {
if p.ParentContext.IsSampled() {
return SamplingResult{Decision: RecordAndSampled}
}
return SamplingResult{Decision: NotRecord}
// ParentBased returns a composite sampler which behaves differently,
// based on the parent of the span. If the span has no parent,
// the root(Sampler) is used to make sampling decision. If the span has
// a parent, depending on whether the parent is remote and whether it
// is sampled, one of the following samplers will apply:
// - remoteParentSampled(Sampler) (default: AlwaysOn)
// - remoteParentNotSampled(Sampler) (default: AlwaysOff)
// - localParentSampled(Sampler) (default: AlwaysOn)
// - localParentNotSampled(Sampler) (default: AlwaysOff)
func ParentBased(root Sampler, samplers ...ParentBasedSamplerOption) Sampler {
return parentBased{
root: root,
config: configureSamplersForParentBased(samplers),
}
return ps.fallback.ShouldSample(p)
}
func (ps parentSampler) Description() string {
return fmt.Sprintf("ParentOrElse{%s}", ps.fallback.Description())
type parentBased struct {
root Sampler
config config
}
func configureSamplersForParentBased(samplers []ParentBasedSamplerOption) config {
c := config{
remoteParentSampled: AlwaysSample(),
remoteParentNotSampled: NeverSample(),
localParentSampled: AlwaysSample(),
localParentNotSampled: NeverSample(),
}
for _, so := range samplers {
so.Apply(&c)
}
return c
}
// config is a group of options for parentBased sampler.
type config struct {
remoteParentSampled, remoteParentNotSampled Sampler
localParentSampled, localParentNotSampled Sampler
}
// ParentBasedSamplerOption configures the sampler for a particular sampling case.
type ParentBasedSamplerOption interface {
Apply(*config)
}
// WithRemoteParentSampled sets the sampler for the case of sampled remote parent.
func WithRemoteParentSampled(s Sampler) ParentBasedSamplerOption {
return remoteParentSampledOption{s}
}
type remoteParentSampledOption struct {
s Sampler
}
func (o remoteParentSampledOption) Apply(config *config) {
config.remoteParentSampled = o.s
}
// WithRemoteParentNotSampled sets the sampler for the case of remote parent
// which is not sampled.
func WithRemoteParentNotSampled(s Sampler) ParentBasedSamplerOption {
return remoteParentNotSampledOption{s}
}
type remoteParentNotSampledOption struct {
s Sampler
}
func (o remoteParentNotSampledOption) Apply(config *config) {
config.remoteParentNotSampled = o.s
}
// WithLocalParentSampled sets the sampler for the case of sampled local parent.
func WithLocalParentSampled(s Sampler) ParentBasedSamplerOption {
return localParentSampledOption{s}
}
type localParentSampledOption struct {
s Sampler
}
func (o localParentSampledOption) Apply(config *config) {
config.localParentSampled = o.s
}
// WithLocalParentNotSampled sets the sampler for the case of local parent
// which is not sampled.
func WithLocalParentNotSampled(s Sampler) ParentBasedSamplerOption {
return localParentNotSampledOption{s}
}
type localParentNotSampledOption struct {
s Sampler
}
func (o localParentNotSampledOption) Apply(config *config) {
config.localParentNotSampled = o.s
}
func (pb parentBased) ShouldSample(p SamplingParameters) SamplingResult {
if p.ParentContext.IsValid() {
if p.HasRemoteParent {
if p.ParentContext.IsSampled() {
return pb.config.remoteParentSampled.ShouldSample(p)
}
return pb.config.remoteParentNotSampled.ShouldSample(p)
}
if p.ParentContext.IsSampled() {
return pb.config.localParentSampled.ShouldSample(p)
}
return pb.config.localParentNotSampled.ShouldSample(p)
}
return pb.root.ShouldSample(p)
}
func (pb parentBased) Description() string {
return fmt.Sprintf("ParentBased{root:%s,remoteParentSampled:%s,"+
"remoteParentNotSampled:%s,localParentSampled:%s,localParentNotSampled:%s}",
pb.root.Description(),
pb.config.remoteParentSampled.Description(),
pb.config.remoteParentNotSampled.Description(),
pb.config.localParentSampled.Description(),
pb.config.localParentNotSampled.Description(),
)
}

123
sdk/trace/sampling_test.go
View File

@ -15,6 +15,7 @@
package trace
import (
"fmt"
"math/rand"
"testing"
@ -23,8 +24,8 @@ import (
api "go.opentelemetry.io/otel/api/trace"
)
func TestAlwaysParentSampleWithParentSampled(t *testing.T) {
sampler := ParentSample(AlwaysSample())
func TestParentBasedDefaultLocalParentSampled(t *testing.T) {
sampler := ParentBased(AlwaysSample())
traceID, _ := api.IDFromHex("4bf92f3577b34da6a3ce929d0e0e4736")
spanID, _ := api.SpanIDFromHex("00f067aa0ba902b7")
parentCtx := api.SpanContext{
@ -37,22 +38,8 @@ func TestAlwaysParentSampleWithParentSampled(t *testing.T) {
}
}
func TestNeverParentSampleWithParentSampled(t *testing.T) {
sampler := ParentSample(NeverSample())
traceID, _ := api.IDFromHex("4bf92f3577b34da6a3ce929d0e0e4736")
spanID, _ := api.SpanIDFromHex("00f067aa0ba902b7")
parentCtx := api.SpanContext{
TraceID: traceID,
SpanID: spanID,
TraceFlags: api.FlagsSampled,
}
if sampler.ShouldSample(SamplingParameters{ParentContext: parentCtx}).Decision != RecordAndSampled {
t.Error("Sampling decision should be RecordAndSampled")
}
}
func TestAlwaysParentSampleWithParentNotSampled(t *testing.T) {
sampler := ParentSample(AlwaysSample())
func TestParentBasedDefaultLocalParentNotSampled(t *testing.T) {
sampler := ParentBased(AlwaysSample())
traceID, _ := api.IDFromHex("4bf92f3577b34da6a3ce929d0e0e4736")
spanID, _ := api.SpanIDFromHex("00f067aa0ba902b7")
parentCtx := api.SpanContext{
@ -64,20 +51,114 @@ func TestAlwaysParentSampleWithParentNotSampled(t *testing.T) {
}
}
func TestParentSampleWithNoParent(t *testing.T) {
func TestParentBasedWithNoParent(t *testing.T) {
params := SamplingParameters{}
sampler := ParentSample(AlwaysSample())
sampler := ParentBased(AlwaysSample())
if sampler.ShouldSample(params).Decision != RecordAndSampled {
t.Error("Sampling decision should be RecordAndSampled")
}
sampler = ParentSample(NeverSample())
sampler = ParentBased(NeverSample())
if sampler.ShouldSample(params).Decision != NotRecord {
t.Error("Sampling decision should be NotRecord")
}
}
func TestParentBasedWithSamplerOptions(t *testing.T) {
testCases := []struct {
name string
samplerOption ParentBasedSamplerOption
isParentRemote, isParentSampled bool
expectedDecision SamplingDecision
}{
{
"localParentSampled",
WithLocalParentSampled(NeverSample()),
false,
true,
NotRecord,
},
{
"localParentNotSampled",
WithLocalParentNotSampled(AlwaysSample()),
false,
false,
RecordAndSampled,
},
{
"remoteParentSampled",
WithRemoteParentSampled(NeverSample()),
true,
true,
NotRecord,
},
{
"remoteParentNotSampled",
WithRemoteParentNotSampled(AlwaysSample()),
true,
false,
RecordAndSampled,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
traceID, _ := api.IDFromHex("4bf92f3577b34da6a3ce929d0e0e4736")
spanID, _ := api.SpanIDFromHex("00f067aa0ba902b7")
parentCtx := api.SpanContext{
TraceID: traceID,
SpanID: spanID,
}
if tc.isParentSampled {
parentCtx.TraceFlags = api.FlagsSampled
}
params := SamplingParameters{ParentContext: parentCtx}
if tc.isParentRemote {
params.HasRemoteParent = true
}
sampler := ParentBased(
nil,
tc.samplerOption,
)
switch tc.expectedDecision {
case RecordAndSampled:
if sampler.ShouldSample(params).Decision != tc.expectedDecision {
t.Error("Sampling decision should be RecordAndSampled")
}
case NotRecord:
if sampler.ShouldSample(params).Decision != tc.expectedDecision {
t.Error("Sampling decision should be NotRecord")
}
}
})
}
}
func TestParentBasedDefaultDescription(t *testing.T) {
sampler := ParentBased(AlwaysSample())
expectedDescription := fmt.Sprintf("ParentBased{root:%s,remoteParentSampled:%s,"+
"remoteParentNotSampled:%s,localParentSampled:%s,localParentNotSampled:%s}",
AlwaysSample().Description(),
AlwaysSample().Description(),
NeverSample().Description(),
AlwaysSample().Description(),
NeverSample().Description())
if sampler.Description() != expectedDescription {
t.Error(fmt.Sprintf("Sampler description should be %s, got '%s' instead",
expectedDescription,
sampler.Description(),
))
}
}
// TraceIDRatioBased sampler requirements state
// "A TraceIDRatioBased sampler with a given sampling rate MUST also sample
// all traces that any TraceIDRatioBased sampler with a lower sampling rate

22
sdk/trace/trace_test.go
View File

@ -232,10 +232,10 @@ func TestSampling(t *testing.T) {
"TraceIdRatioBased_.75": {sampler: TraceIDRatioBased(0.75), expect: .75},
"TraceIdRatioBased_2.0": {sampler: TraceIDRatioBased(2.0), expect: 1},
// Spans w/o a parent and using ParentSample(DelegateSampler()) Sampler, receive DelegateSampler's sampling decision
"ParentNeverSample": {sampler: ParentSample(NeverSample()), expect: 0},
"ParentAlwaysSample": {sampler: ParentSample(AlwaysSample()), expect: 1},
"ParentTraceIdRatioBased_.50": {sampler: ParentSample(TraceIDRatioBased(0.50)), expect: .5},
// Spans w/o a parent and using ParentBased(DelegateSampler()) Sampler, receive DelegateSampler's sampling decision
"ParentNeverSample": {sampler: ParentBased(NeverSample()), expect: 0},
"ParentAlwaysSample": {sampler: ParentBased(AlwaysSample()), expect: 1},
"ParentTraceIdRatioBased_.50": {sampler: ParentBased(TraceIDRatioBased(0.50)), expect: .5},
// An unadorned TraceIDRatioBased sampler ignores parent spans
"UnsampledParentSpanWithTraceIdRatioBased_.25": {sampler: TraceIDRatioBased(0.25), expect: .25, parent: true},
@ -248,13 +248,13 @@ func TestSampling(t *testing.T) {
// Spans with a sampled parent but using NeverSample Sampler, are not sampled
"SampledParentSpanWithNeverSample": {sampler: NeverSample(), expect: 0, parent: true, sampledParent: true},
// Spans with a sampled parent and using ParentSample(DelegateSampler()) Sampler, inherit the parent span's sampling status
"SampledParentSpanWithParentNeverSample": {sampler: ParentSample(NeverSample()), expect: 1, parent: true, sampledParent: true},
"UnsampledParentSpanWithParentNeverSampler": {sampler: ParentSample(NeverSample()), expect: 0, parent: true, sampledParent: false},
"SampledParentSpanWithParentAlwaysSampler": {sampler: ParentSample(AlwaysSample()), expect: 1, parent: true, sampledParent: true},
"UnsampledParentSpanWithParentAlwaysSampler": {sampler: ParentSample(AlwaysSample()), expect: 0, parent: true, sampledParent: false},
"SampledParentSpanWithParentTraceIdRatioBased_.50": {sampler: ParentSample(TraceIDRatioBased(0.50)), expect: 1, parent: true, sampledParent: true},
"UnsampledParentSpanWithParentTraceIdRatioBased_.50": {sampler: ParentSample(TraceIDRatioBased(0.50)), expect: 0, parent: true, sampledParent: false},
// Spans with a sampled parent and using ParentBased(DelegateSampler()) Sampler, inherit the parent span's sampling status
"SampledParentSpanWithParentNeverSample": {sampler: ParentBased(NeverSample()), expect: 1, parent: true, sampledParent: true},
"UnsampledParentSpanWithParentNeverSampler": {sampler: ParentBased(NeverSample()), expect: 0, parent: true, sampledParent: false},
"SampledParentSpanWithParentAlwaysSampler": {sampler: ParentBased(AlwaysSample()), expect: 1, parent: true, sampledParent: true},
"UnsampledParentSpanWithParentAlwaysSampler": {sampler: ParentBased(AlwaysSample()), expect: 0, parent: true, sampledParent: false},
"SampledParentSpanWithParentTraceIdRatioBased_.50": {sampler: ParentBased(TraceIDRatioBased(0.50)), expect: 1, parent: true, sampledParent: true},
"UnsampledParentSpanWithParentTraceIdRatioBased_.50": {sampler: ParentBased(TraceIDRatioBased(0.50)), expect: 0, parent: true, sampledParent: false},
} {
tc := tc
t.Run(name, func(t *testing.T) {