-
Notifications
You must be signed in to change notification settings - Fork 3k
Add new tail sampling processor policy: probabilistic #3876
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
Merged
Merged
Changes from 5 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
7578e67
Add new tail sampling processor policy: percentage
f829362
Update CHANGELOG.md
d487e06
Fix tail_sampling_config.yaml
b7d9747
Fix typo
7e6eb4f
Reset counters to avoid overflow, improve tests
387d13b
Merge branch 'main' into percentage_sampling
3f620b0
Move into internal as well
dcc1d0e
Use sampling_percentage similar to the probabilistic sampling processor
4e4e90b
Combine tests in a single table-test
5b7729f
Rename percentage -> probabilistic for consistency with probabilistic…
56fdb1a
Add IncludeAlreadySampled option
7a69d0b
Clarify test cases
d9e96e6
Use the same algorithm as the probabilistic sampling processor
91f8d5e
Simplify if-else case
e674a9e
Typo
648bdc3
Order of tail sampling policies does not matter
1d58e79
Switch hashing implementation from murmur3 to fnv-1a
b82595c
Lower amount of traces to sample in test to speed up tests
9106f13
Merge branch 'main' into percentage_sampling
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
// Copyright The OpenTelemetry Authors | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package sampling | ||
|
||
import ( | ||
"errors" | ||
|
||
"go.opentelemetry.io/collector/consumer/pdata" | ||
"go.uber.org/zap" | ||
) | ||
|
||
type percentageFilter struct { | ||
logger *zap.Logger | ||
percentage float32 | ||
tracesSampled int | ||
tracesProcessed int | ||
} | ||
|
||
var _ PolicyEvaluator = (*percentageFilter)(nil) | ||
|
||
// NewPercentageFilter creates a policy evaluator that samples a percentage of | ||
// traces. | ||
func NewPercentageFilter(logger *zap.Logger, percentage float32) (PolicyEvaluator, error) { | ||
if percentage < 0 || percentage > 1 { | ||
return nil, errors.New("expected a percentage between 0 and 1") | ||
} | ||
|
||
return &percentageFilter{ | ||
logger: logger, | ||
percentage: percentage, | ||
}, nil | ||
} | ||
|
||
// OnLateArrivingSpans notifies the evaluator that the given list of spans arrived | ||
// after the sampling decision was already taken for the trace. | ||
// This gives the evaluator a chance to log any message/metrics and/or update any | ||
// related internal state. | ||
func (r *percentageFilter) OnLateArrivingSpans(Decision, []*pdata.Span) error { | ||
r.logger.Debug("Triggering action for late arriving spans in percentage filter") | ||
return nil | ||
} | ||
|
||
// Evaluate looks at the trace data and returns a corresponding SamplingDecision. | ||
func (r *percentageFilter) Evaluate(_ pdata.TraceID, trace *TraceData) (Decision, error) { | ||
r.logger.Debug("Evaluating spans in percentage filter") | ||
|
||
// ignore traces that have already been sampled before | ||
for _, decision := range trace.Decisions { | ||
if decision == Sampled { | ||
return NotSampled, nil | ||
} | ||
} | ||
|
||
decision := NotSampled | ||
|
||
if float32(r.tracesSampled)/float32(r.tracesProcessed) <= r.percentage { | ||
r.tracesSampled++ | ||
yvrhdn marked this conversation as resolved.
Show resolved
Hide resolved
|
||
decision = Sampled | ||
} | ||
r.tracesProcessed++ | ||
|
||
// reset counters to avoid overflow | ||
if r.tracesProcessed == 1000 { | ||
r.tracesSampled = 0 | ||
r.tracesProcessed = 0 | ||
} | ||
|
||
return decision, nil | ||
} |
103 changes: 103 additions & 0 deletions
103
processor/tailsamplingprocessor/sampling/percentage_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
// Copyright The OpenTelemetry Authors | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package sampling | ||
|
||
import ( | ||
"fmt" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/assert" | ||
"go.opentelemetry.io/collector/consumer/pdata" | ||
"go.uber.org/zap" | ||
) | ||
|
||
func TestNewPercentageFilter_errorHandling(t *testing.T) { | ||
_, err := NewPercentageFilter(zap.NewNop(), -1) | ||
assert.EqualError(t, err, "expected a percentage between 0 and 1") | ||
|
||
_, err = NewPercentageFilter(zap.NewNop(), 1.5) | ||
assert.EqualError(t, err, "expected a percentage between 0 and 1") | ||
} | ||
|
||
func TestPercentageSampling(t *testing.T) { | ||
var empty = map[string]pdata.AttributeValue{} | ||
|
||
cases := []float32{0.01, 0.1, 0.125, 0.33, 0.5, 0.66} | ||
|
||
for _, percentage := range cases { | ||
t.Run(fmt.Sprintf("sample %.2f", percentage), func(t *testing.T) { | ||
trace := newTraceStringAttrs(empty, "example", "value") | ||
traceID := pdata.NewTraceID([16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}) | ||
|
||
percentageFilter, err := NewPercentageFilter(zap.NewNop(), percentage) | ||
assert.NoError(t, err) | ||
|
||
traceCount := 2000 | ||
sampled := 0 | ||
|
||
for i := 0; i < traceCount; i++ { | ||
decision, err := percentageFilter.Evaluate(traceID, trace) | ||
assert.NoError(t, err) | ||
|
||
if decision == Sampled { | ||
sampled++ | ||
} | ||
} | ||
|
||
assert.InDelta(t, percentage*float32(traceCount), sampled, 0.001, "Amount of sampled traces") | ||
}) | ||
} | ||
} | ||
|
||
func TestPercentageSampling_ignoreAlreadySampledTraces(t *testing.T) { | ||
var empty = map[string]pdata.AttributeValue{} | ||
|
||
trace := newTraceStringAttrs(empty, "example", "value") | ||
traceID := pdata.NewTraceID([16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}) | ||
|
||
var percentage float32 = 0.33 | ||
|
||
percentageFilter, err := NewPercentageFilter(zap.NewNop(), percentage) | ||
assert.NoError(t, err) | ||
|
||
traceCount := 100 | ||
sampled := 0 | ||
|
||
for i := 0; i < traceCount; i++ { | ||
trace.Decisions = []Decision{NotSampled, NotSampled} | ||
decision, err := percentageFilter.Evaluate(traceID, trace) | ||
assert.NoError(t, err) | ||
|
||
if decision == Sampled { | ||
sampled++ | ||
} | ||
|
||
// trace has been sampled, should be ignored | ||
trace.Decisions = []Decision{NotSampled, Sampled} | ||
decision, err = percentageFilter.Evaluate(traceID, trace) | ||
assert.NoError(t, err) | ||
assert.Equal(t, decision, NotSampled) | ||
} | ||
|
||
assert.EqualValues(t, percentage*float32(traceCount), sampled) | ||
} | ||
|
||
func TestOnLateArrivingSpans_PercentageSampling(t *testing.T) { | ||
percentageFilter, err := NewPercentageFilter(zap.NewNop(), 0.1) | ||
assert.Nil(t, err) | ||
|
||
err = percentageFilter.OnLateArrivingSpans(NotSampled, nil) | ||
assert.Nil(t, err) | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would I use this instead of the probabilistic sampling processor?
https://github.com/open-telemetry/opentelemetry-collector/tree/main/processor/probabilisticsamplerprocessor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, wasn't aware of the probabilistic sampling processor (for some reason) 🤦🏻 I'll take a look at it and make sure our terminology is consistent.
I think the main difference is when the sampling decision happens. If you only want percentage or probabilistic sampling it doesn't make sense to use this tail sampling processor.
But combining this with other tail sampling policies makes it more meaningful. For instance:
This pipeline would sample all traces with status code error and 50% of the remaining traces. Using the probabilistic sampling processor you risk dropping traces with errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Make sure to mention that in the readme then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a difference between this and just putting the probabilistic sampling processor after tail sampling at 50%?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The result will be slightly different: putting the probabilistic sampling processor after the tail sampling processor will filter what the tail sampling processor samples.
The percentage sampler as implemented here will filter what is not sampled by another policy.
So for instance a pipeline:
Result: 50% of traces with error
Why:
While the following:
Result: all traces with error + 50% of traces without