[tailsamplingprocessor] [chore] Remove test coupling on component internal fields

[Logiraptor]

While working on a fix for #41656, I've noticed many tests are strongly coupled to the tsp internals. This PR refactors all such coupling to exist in a small number of test helpers, instead of being duplicated across all tests. Those helpers are:

• withPolicies - allows directly setting the policy evaluators which is useful for fine grained decision logic tests
• withTickerLatency - allows speeding up the decision ticker which is useful for speeding up tests
• withTestController - makes the decision tick blocking until triggered. This allows deterministic tests of otherwise non-deterministic behavior.

A few tests were mostly rewritten to make them test externally observable behavior. For example:

• TestLateArrivingSpanUsesDecisionCache - used to inspect idToTrace state to ensure traces are cleared from memory. Now the tsp is destroyed and recreated with the same cache to ensure the cache is the only state available.
• TestSequentialTraceMapSize et al - these used to inspect idToTrace state to ensure trace data is properly tracked. Now they trigger sampling decisions and assert that the output is complete.
• TestSetSamplingPolicy et al - these used to inspect policies state to ensure policies are reloaded. Now they observe sampled traces only.

Instead of invoking the TSP tick loop directly

Instead of directly calling the tsp tick loop, I've added code to wrap the tick loop and make it blocking until called. This still involves some coupling, but since it's encapsulated in one place, it will be easier to change when refactoring the tsp in #41656. I took the opportunity to consolidate the sync batcher into this controller as well. In all cases with a manual tick loop, we also want a synchronous batcher for the same reason. Again this makes it simpler to update later without modifying every test at once.

Instead of accessing the idToTrace map

In all cases where we were directly inspecting the idToTrace map, we could instead just let the tsp sample those traces and inspect the output.

Instead of setting tickerFrequency directly

I've moved these tests to use withTickerFrequency, which is still a coupling, but since it's delegated to the option func, there's a central place to change how it works.

other

I included a final commit: 7bfab91. This just removes an unused field.

I've decided to remove two tests:

• TestConcurrentTraceMapSize - this one writes traces concurrently then asserts on the size of the idToTrace map. We have overlapping tests which write concurrently then assert complete sampled output, which IMO is a better test.
• TestDecisionPolicyMetrics - this one calls the internal makeDecision method and then asserts on the returned metrics, which are only used in debug logging. This feels like over testing to me and makes the tests overly coupled.

[axw]

@Logiraptor I like the sound of this, but it needs to wait until the minimum Go version is bumped to 1.25. That will happen when Go 1.26.0 comes out in February 2026.

[Logiraptor]

@axw Yeah, I just realized that 😓 I will mark this as a draft while I figure out if there's an easy way around it

[Logiraptor]

@axw Figured out an alternative approach. It's not quite as magical, but it accomplishes the goal of reducing how much needs to change when the tsp internals are refactored.

[carsonip]

Thanks! Leaving a comment as I'm also interested in improving tail sampling processor.

PR lgtm at a high level. Alternatively, have you considered keeping policyTicker as TTicker, and creating a testTicker implementing TTicker which allows it to tick as fast as possible, instead of 1ms?

[Logiraptor]

@carsonip I hadn't tried that, no. FWIW, I have another PR in draft which will remove the ticker abstraction in order to consolidate state manipulation into a single select statement: Logiraptor#1 (staged in my fork while I test, but will open the real PR here). In that PR we no longer need a 1ms ticker and can directly control the tick via a side channel.

More context on that PR in case you're interested:
In that PR, I'm hoping to simplify how state is managed in general, so we can mostly avoid locks / channels / sync.Map. In doing so, it also becomes easier to run parallel workers in the TSP, so we can finally overcome the single threaded bottleneck. In testing, I'm finding that most of the policy ticker latency comes from synchronization overhead caused by having so much shared, concurrency-aware state. This also means running those parallel workers doesn't gain as much throughput as expected. So I have a few side quests like this one and #43510 to reduce contention on shared state.

[carsonip]

lgtm, thanks! Tests are much cleaner without coupling to implementation

[portertech]

Was this really a noop?

[axw]

Looks like the processor refers to cfg.NumTraces everywhere else