-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[probabilistic sampling processor] encoded sampling probability (support OTEP 235) #31894
Merged
Merged
Changes from all commits
Commits
Show all changes
99 commits
Select commit
Hold shift + click to select a range
e822a9b
Add t-value sampler draft
jmacd 1bc6017
copy/import tracestate parser package
jmacd d1fd891
test ot tracestate
jmacd 85e4472
tidy
jmacd bb75f8a
renames
jmacd 6a57b77
testing two parsers w/ generic code
jmacd 7fa8130
integrated
jmacd 36230e7
Comments
jmacd 7bae35c
revert two files
jmacd 9010a67
Update with r, s, and t-value. Now using regexps and strings.IndexBy…
jmacd 0e27e40
fix sampler build
jmacd efcdc3d
add support for s-value for non-consistent mode
jmacd 939c758
WIP
jmacd b9a1e56
Merge branch 'main' of github.com:open-telemetry/opentelemetry-collec…
jmacd a31266c
use new proposed syntax see https://github.com/open-telemetry/opentel…
jmacd 690cd64
update tracestate libs for new encoding
jmacd c8baf29
wip working on probabilistic sampler with two new modes: downsampler …
jmacd 7f47e4a
unsigned implement split
jmacd 422e0b2
two implementations
jmacd 787b9fd
wip
jmacd ed36f03
Merge branch 'main' of github.com:open-telemetry/opentelemetry-collec…
jmacd d795210
Updates for OTEP 235
jmacd 09000f7
wip TODO
jmacd a4d467b
versions.yaml
jmacd e373b9b
Add proportional sampler mode; comment on TODOs; create SamplerMode t…
jmacd fe6a085
back from internal
jmacd 396efb1
wip
jmacd 36de5dd
fix existing tests
jmacd f1aa0ad
:wip:
jmacd 700734e
Update for rejection threshold
jmacd ae50bdd
Merge branch 'main' of github.com:open-telemetry/opentelemetry-collec…
jmacd a94b8e7
fix preexisting tests
jmacd 4edcbcb
basic yes/no t-value sampling test
jmacd 53bf119
Merge branch 'main' of github.com:open-telemetry/opentelemetry-collec…
jmacd 3cdb957
add version for sampling pkg
jmacd e506847
more testing
jmacd 2cddfeb
add probability to threshold with precision option
jmacd f69d6ee
ProbabilityToThresholdWithPrecision
jmacd cc02934
test coverage for equalizing and proportional
jmacd 1eecc4a
config test
jmacd 2159107
comments and notes
jmacd e0898a6
update README
jmacd d0991ed
Merge branch 'main' of github.com:open-telemetry/opentelemetry-collec…
jmacd a002774
Remove sampling pkg, it is now upstream
jmacd 3a49922
Merge branch 'main' of github.com:open-telemetry/opentelemetry-collec…
jmacd fca0184
build w/ new sampling pkg
jmacd f11e0a4
more test coverage
jmacd 3f495a6
more config tests
jmacd 581095c
test precision underflow
jmacd 7b8fd31
test warning logs
jmacd 1a6be4f
hash_seed fixes
jmacd 712cf17
wip
jmacd 34c0d3b
aip
jmacd 7742668
wip-refactoring
jmacd 8d60168
refactor wip
jmacd 3779caa
cleanup refactor
jmacd c261ac1
wip
jmacd 34469e4
moving code
jmacd 8dabf47
fix tests; round up small probs to avoid errors
jmacd d44afb5
preserve legacy behavior
jmacd 1cf9991
logs handled sampling priority differently
jmacd 365d35d
still two errors
jmacd 12a3047
builds
jmacd 8655f42
needs testing
jmacd 468e6c6
fixing tests
jmacd 23b4423
cleanup
jmacd 07841e5
remove strict feature
jmacd 6936bc4
tests fixed
jmacd c132f4c
wip
jmacd bd13ac9
typo
jmacd aa33b1c
more logs tests
jmacd 06556dc
Add more comments
jmacd a4940e6
update README
jmacd 4f616e9
Merge branch 'main' of github.com:open-telemetry/opentelemetry-collec…
jmacd b4ca3aa
Merge branch 'main' of github.com:open-telemetry/opentelemetry-collec…
jmacd fdd26ac
Merge branch 'main' of github.com:open-telemetry/opentelemetry-collec…
jmacd 794d1a1
wip update
jmacd a305a7f
undo comment changes
jmacd 98433af
test all modes logs missing randomness
jmacd 3aa4608
more missing rando
jmacd a0bc49e
smaller diff
jmacd d0aea21
comment carrier
jmacd 7b81625
chlog
jmacd fe4dd37
simplify ctcom
jmacd a244866
lint
jmacd 89331bc
combine README updates
jmacd 04d65c4
tidy
jmacd 9cb1586
Merge branch 'main' of github.com:open-telemetry/opentelemetry-collec…
jmacd a98db61
Add sampler mode use-cases
jmacd d33660b
rephrase tracestate; logs do not use tracestate
jmacd c67350d
explain sampling precision
jmacd b0a9516
move misplaced text
jmacd 95ecbae
remove multierr
jmacd cbcc853
Apply suggestions from code review
jmacd ad32651
only debug and info
jmacd 6b71ea8
adjust test for debug-level logs
jmacd 61abf1f
revert change of default mode
jmacd 0664ea1
Merge branch 'main' of github.com:open-telemetry/opentelemetry-collec…
jmacd 1926afb
Merge branch 'main' into jmacd/tvaluesampler
jmacd 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 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,27 @@ | ||
# Use this changelog template to create an entry for release notes. | ||
|
||
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
change_type: enhancement | ||
|
||
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) | ||
component: probabilisticsamplerprocessor | ||
|
||
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | ||
note: Add Proportional and Equalizing sampling modes | ||
|
||
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. | ||
issues: [31918] | ||
|
||
# (Optional) One or more lines of additional information to render under the primary note. | ||
# These lines will be padded with 2 spaces and then inserted directly into the document. | ||
# Use pipe (|) for multiline entries. | ||
subtext: Both the existing hash_seed mode and the two new modes use OTEP 235 semantic conventions to encode sampling probability. | ||
|
||
# If your change doesn't affect end users or the exported elements of any package, | ||
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. | ||
# Optional: The change log or logs in which this entry should be included. | ||
# e.g. '[user]' or '[user, api]' | ||
# Include 'user' if the change is relevant to end users. | ||
# Include 'api' if there is a change to a library API. | ||
# Default: '[user]' | ||
change_logs: [user] |
This file contains 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 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 |
---|---|---|
|
@@ -5,8 +5,11 @@ package probabilisticsamplerprocessor // import "github.com/open-telemetry/opent | |
|
||
import ( | ||
"fmt" | ||
"math" | ||
|
||
"go.opentelemetry.io/collector/component" | ||
|
||
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/sampling" | ||
) | ||
|
||
type AttributeSource string | ||
|
@@ -35,6 +38,33 @@ type Config struct { | |
// different sampling rates, configuring different seeds avoids that. | ||
HashSeed uint32 `mapstructure:"hash_seed"` | ||
|
||
// Mode selects the sampling behavior. Supported values: | ||
// | ||
// - "hash_seed": the legacy behavior of this processor. | ||
// Using an FNV hash combined with the HashSeed value, this | ||
// sampler performs a non-consistent probabilistic | ||
// downsampling. The number of spans output is expected to | ||
// equal SamplingPercentage (as a ratio) times the number of | ||
// spans inpout, assuming good behavior from FNV and good | ||
// entropy in the hashed attributes or TraceID. | ||
// | ||
// - "equalizing": Using an OTel-specified consistent sampling | ||
// mechanism, this sampler selectively reduces the effective | ||
// sampling probability of arriving spans. This can be | ||
// useful to select a small fraction of complete traces from | ||
// a stream with mixed sampling rates. The rate of spans | ||
// passing through depends on how much sampling has already | ||
// been applied. If an arriving span was head sampled at | ||
// the same probability it passes through. If the span | ||
// arrives with lower probability, a warning is logged | ||
// because it means this sampler is configured with too | ||
// large a sampling probability to ensure complete traces. | ||
// | ||
// - "proportional": Using an OTel-specified consistent sampling | ||
// mechanism, this sampler reduces the effective sampling | ||
// probability of each span by `SamplingProbability`. | ||
Mode SamplerMode `mapstructure:"mode"` | ||
|
||
// FailClosed indicates to not sample data (the processor will | ||
// fail "closed") in case of error, such as failure to parse | ||
// the tracestate field or missing the randomness attribute. | ||
|
@@ -45,6 +75,14 @@ type Config struct { | |
// despite errors using priority. | ||
FailClosed bool `mapstructure:"fail_closed"` | ||
|
||
// SamplingPrecision is how many hex digits of sampling | ||
// threshold will be encoded, from 1 up to 14. Default is 4. | ||
// 0 is treated as full precision. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not according to invalid_zero.yaml. |
||
SamplingPrecision int `mapstructure:"sampling_precision"` | ||
jmacd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/////// | ||
// Logs only fields below. | ||
|
||
// AttributeSource (logs only) defines where to look for the attribute in from_attribute. The allowed values are | ||
// `traceID` or `record`. Default is `traceID`. | ||
AttributeSource `mapstructure:"attribute_source"` | ||
|
@@ -61,11 +99,34 @@ var _ component.Config = (*Config)(nil) | |
|
||
// Validate checks if the processor configuration is valid | ||
func (cfg *Config) Validate() error { | ||
if cfg.SamplingPercentage < 0 { | ||
return fmt.Errorf("negative sampling rate: %.2f", cfg.SamplingPercentage) | ||
pct := float64(cfg.SamplingPercentage) | ||
|
||
if math.IsInf(pct, 0) || math.IsNaN(pct) { | ||
return fmt.Errorf("sampling rate is invalid: %f%%", cfg.SamplingPercentage) | ||
} | ||
ratio := pct / 100.0 | ||
|
||
switch { | ||
case ratio < 0: | ||
return fmt.Errorf("sampling rate is negative: %f%%", cfg.SamplingPercentage) | ||
case ratio == 0: | ||
// Special case | ||
case ratio < sampling.MinSamplingProbability: | ||
// Too-small case | ||
return fmt.Errorf("sampling rate is too small: %g%%", cfg.SamplingPercentage) | ||
default: | ||
// Note that ratio > 1 is specifically allowed by the README, taken to mean 100% | ||
} | ||
|
||
if cfg.AttributeSource != "" && !validAttributeSource[cfg.AttributeSource] { | ||
return fmt.Errorf("invalid attribute source: %v. Expected: %v or %v", cfg.AttributeSource, traceIDAttributeSource, recordAttributeSource) | ||
} | ||
|
||
if cfg.SamplingPrecision == 0 { | ||
return fmt.Errorf("invalid sampling precision: 0") | ||
} else if cfg.SamplingPrecision > sampling.NumHexDigits { | ||
return fmt.Errorf("sampling precision is too great, should be <= 14: %d", cfg.SamplingPrecision) | ||
} | ||
|
||
return nil | ||
} |
This file contains 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 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
3 changes: 1 addition & 2 deletions
3
processor/probabilisticsamplerprocessor/internal/metadata/generated_telemetry_test.go
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
Oops, something went wrong.
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.