[processor/tailsampling] add sampling_strategy config with trace-complete and span-ingest modes

[carsonip]

Description

Add sampling_strategy config to processor/tailsampling with two explicit modes:

• trace-complete (default): classic timer-based tail sampling with full-trace evaluation context.
• span-ingest: evaluate each incoming batch on ingest, finalize terminal outcomes (sampled/dropped) immediately, and cleanup-finalize unresolved traces as not sampled without policy re-evaluation.

This implementation also:

• validates strategy values and defaults to trace-complete.
• enforces stateless policies in span-ingest mode (IsStateful() == false).
• records ingest-time decision telemetry for span-ingest.
• aligns tests/docs/schema/comments with implemented behavior.
• supports earlier batch timing after root-span arrival via decision_wait_after_root_received in both modes (earlier decision in trace-complete, earlier pending cleanup finalization in span-ingest).

Link to tracking issue

Fixes #46600.

Testing

• go test ./... (in processor/tailsamplingprocessor)
• Updated/added tests for:
  • ingest-time terminal decisions in span-ingest
  • pending cleanup finalization behavior
  • ingest ordering behavior
  • telemetry emission paths

Documentation

• Updated processor/tailsamplingprocessor/README.md for strategy semantics and flow.
• Updated processor/tailsamplingprocessor/config.schema.yaml descriptions for strategy/timer behavior.
• Updated strategy/timer comments in processor/tailsamplingprocessor/config.go.
• Added changelog entry: .chloggen/tailsampling-sampling-strategy-config.yaml.

[axw]

@carsonip I haven't looked at much of the code, mostly the config & description. I'm just wondering about restricting span-ingest to stateless policies.

This would mean you can't have rate limiting or reservoir sampling on root spans with the same name, for example. Is that intentional?

[carsonip]

I'm just wondering about restricting span-ingest to stateless policies.

It is intentional in this PR to keep the PR simple by banning all stateful ones. In longer term we may loosen the restraction to make it work with more stateful ones. This is because stateful policy evaluation in its current state will create some obviously incorrect interaction.

For example, with byte limiting, in its current state, as it adds the counter at evaluation time. Let's say there are 10 traces, each with 1 root span with 100 bytes and child span with 1 bytes. At a 10-byte limit, if it sees all child spans before root spans, all of them will reach a terminal decision of sample=true and end up ingesting 1010 bytes instead of 10 bytes.

At a high level, stateful policies that concern individual spans, e.g. rate limiting, should be okay, but ones that concern trace completeness may be theoretically infeasible without certain delay like "decision wait"

This would mean you can't have rate limiting or reservoir sampling on root spans with the same name, for example. Is that intentional?

TLDR it will be in follow up PRs

[csmarchbanks]

Sadly I didn't see this until after it was merged, it is similar to some work I tried before implementing the after root span change: #44456. What I found in practice was that most of our instances only positively sample something like 1-10% of traces, so this didn't help with memory as much as I would have liked. I am curious if you found something different when working on this?

In addition, I worry that this solution could cause excessive CPU usage for large traces. Wouldn't each new batch that arrives re-run expensive evaluators such as ottl on each span, not just the new ones? I used a different interface to avoid this in my work, but I also like how simple the IsStateful interface is.

[carsonip]

What I found in practice was that most of our instances only positively sample something like 1-10% of traces, so this didn't help with memory as much as I would have liked. I am curious if you found something different when working on this?

This is a limitation by design I'm afraid. For buffered spans to be released, we'll either have to

• get an early terminal decision (sampled or dropped); or
• wait for decision_wait or decision_wait_after_root_received

and if the policies don't really explicitly "drop", then you can only effectively save memory by early releasing the sampled ones, which would be 1-10% like you said.

Therefore, I tend to agree that unless the policies are tuned very well to sample / drop, the memory savings in its current state would not be significant. However, the true motivation behind this span-ingest mode is to unblock a long running feature request around disk offloading in #42326

In addition, I worry that this solution could cause excessive CPU usage for large traces. Wouldn't each new batch that arrives re-run expensive evaluators such as ottl on each span, not just the new ones?

This is not wrong, but evaluators are still effectively run per span, right? This would only introduce extra overhead in calling the evaluators more frequently on smaller collections of spans.

I used a different interface to avoid this in my work, but I also like how simple the IsStateful interface is.

IsStateful is more for correctness purposes, because I'd like to avoid some obviously incorrect interactions where a trace is sampled because byte_limiting likes the first span of this trace, without knowledge of the size of subsequent spans of the same trace.

[csmarchbanks]

Unblocking the disk offloading work makes sense! Thanks!

In addition, I worry that this solution could cause excessive CPU usage for large traces. Wouldn't each new batch that arrives re-run expensive evaluators such as ottl on each span, not just the new ones?

This is not wrong, but evaluators are still effectively run per span, right? This would only introduce extra overhead in calling the evaluators more frequently on smaller collections of spans.

My concern is that the TSP now effectively calls the evaluators n_batches * n_spans times instead of just n_spans. Probably fine for small traces, but a large trace with lots of spans and batches would be doing a lot of repeat work.

Related to correctness I did open #47476 for some issues I saw and had ran into when doing my work as well :).

[carsonip]

My concern is that the TSP now effectively calls the evaluators n_batches * n_spans times instead of just n_spans. Probably fine for small traces, but a large trace with lots of spans and batches would be doing a lot of repeat work.

On top of it, I'm concerned about the background goroutine being busier and causing backpressure to senders. But I haven't seen a bottleneck in practice so I didn't jump to the conclusion of adding sharded processing immediately. We may need to revisit this later, especially when span buffering involves more expensive IO.

Related to correctness I did open #47476 for some issues I saw and had ran into when doing my work as well :).

Thanks for creating the PR and pointing me to it! I'll take a look later.