[processor/tailsampling] Improved "not sampled" decision cache usage #37189
Conversation
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
…l map if their trace id is in a decision cache Signed-off-by: Sean Porter <portertech@gmail.com>
portertech
commented
Jan 14, 2025
| // IDs. If the trace ID is cached, it deletes the spans from the internal map. | ||
| func (tsp *tailSamplingSpanProcessor) releaseNotSampledTrace(id pcommon.TraceID) { | ||
| tsp.nonSampledIDCache.Put(id, true) | ||
| _, ok := tsp.nonSampledIDCache.Get(id) |
There was a problem hiding this comment.
This Get() would be unnecessary if the cache interface Put() returned bool, error. Saving for a potential future enhancement.
Signed-off-by: Sean Porter <portertech@gmail.com>
jpkrohling
reviewed
Jan 15, 2025
Signed-off-by: Sean Porter <portertech@gmail.com>
portertech
commented
Jan 16, 2025
| @@ -386,8 +385,7 @@ func TestLateArrivingSpanUsesDecisionCache(t *testing.T) { | |||
| // The final decision SHOULD be Sampled. | |||
| require.EqualValues(t, 1, nextConsumer.SpanCount()) | |||
|
|
|||
| // Drop the trace to force cache to make decision | |||
| tsp.dropTrace(traceID, time.Now()) | |||
There was a problem hiding this comment.
@jpkrohling I was able to reuse the existing decision tests for the new functionality 👍 It was meant to be 😄
jpkrohling
approved these changes
Jan 17, 2025
chengchuanpeng
pushed a commit
to chengchuanpeng/opentelemetry-collector-contrib
that referenced
this pull request
Jan 26, 2025
…pen-telemetry#37189) Currently, a "not sampled" decision for a trace is only cached when late spans arrive for it. Whereas a "sampled" decision for a trace is immediately cached. This pull request immediately caches "not sampled" decisions, late spans (or reprocessed spans) for "not sampled" traces now take fewer resources to process. Currently, traces are stored in memory until `num_traces` is reached, then the oldest is deleted to make room. This is a clever mechanism to limit the size* of the internal map, however, trace span counts and their contents likely vary considerably. This pull request doesn't touch this mechanism, but it does delete traces from the internal map after they are released and are added to a decision cache. --------- Signed-off-by: Sean Porter <portertech@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Currently, a "not sampled" decision for a trace is only cached when late spans arrive for it. Whereas a "sampled" decision for a trace is immediately cached. This pull request immediately caches "not sampled" decisions, late spans (or reprocessed spans) for "not sampled" traces now take fewer resources to process.
Currently, traces are stored in memory until
num_tracesis reached, then the oldest is deleted to make room. This is a clever mechanism to limit the size* of the internal map, however, trace span counts and their contents likely vary considerably. This pull request doesn't touch this mechanism, but it does delete traces from the internal map after they are released and are added to a decision cache.