Skip to content

Conversation

kanderson250
Copy link
Contributor

Resolves #2485

This PR decouples our adaptive sampler from the harvest cycle, in an effort to support upcoming initiatives for core tracing and otel style sampling algorithms.

What's Changed

Previously, our adaptive sampler was tied to the transaction reservoir (and therefore, the harvest cycle) to perform its math. This PR:

Adds/Updates

  • Refactors the AdaptiveSampler to be fully self-encapsulated. It runs its own timer, and tracks its own stats, regardless of the state of any event reservoir. To support this self-encapsulated math, a Transaction now needs to assign priority as late in its lifecycle as possible. See the below comment on Transaction changes.
  • Refactors all 3 existing samplers (Adaptive, AlwaysOn, AlwaysOff) to implement a common Sampler interface.
  • Refactors the Transaction class to assign priority as late in its lifecycle as possible. This is required to avoid running the adaptive sampler on a transaction that later accepts DT headers (in which case, the adaptive sampler isn't required to make a decision, and the inbound headers alone should be used).
    • "As late in the lifecycle as possible" means when createDistributedTracePayload is called, or the transaction finishes.
    • As before, the Transaction also assigns priority when remote parent sampling decisions are found.
  • Refactors a bit of the remote parent sampling logic to comply with the latest agent specs. Most notably, remote parent sampling decisions may now be taken from the New Relic DT Headers (not just the W3C headers).

Removes

  • All the stuff that was required to support our old implementation of the adaptive sampler. This primarily means removing the PriorityAware "decided" field and all references to it. This field used to be used by the reservoir to track adaptive-sampler related statistics (decidedCountLast) - it was not used when deciding whether to add an event to the reservoir, but rather to power the math of the adaptive sampler. There are many changes related to this update.

Configuration

The AdaptiveSampler can be configured with the adaptive_sampling_target setting.

Relevant Specifications

Adaptive Sampler specification: https://source.datanerd.us/agents/agent-specs/blob/main/distributed_tracing/Distributed-Tracing.md#adaptive-sampler
Remote Parent Sampled Flag specification: https://source.datanerd.us/agents/agent-specs/blob/main/distributed_tracing/Distributed-Tracing.md#determining-whether-the-remote-parent-was-sampled

Testing

Unit tests have been added for the adaptive sampler, Transaction class, and DistributedTraceServiceImpl to support these changes. Cross agent tests have been added. Existing tests have been updated.

@codecov-commenter
Copy link

codecov-commenter commented Oct 7, 2025

Codecov Report

❌ Patch coverage is 87.32394% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.26%. Comparing base (1792d4d) to head (ef72ba8).
⚠️ Report is 36 commits behind head on main.

Files with missing lines Patch % Lines
...lic/agent/tracing/DistributedTraceServiceImpl.java 73.68% 4 Missing and 1 partial ⚠️
.../src/main/java/com/newrelic/agent/HeadersUtil.java 20.00% 4 Missing ⚠️
.../src/main/java/com/newrelic/agent/Transaction.java 82.35% 1 Missing and 2 partials ⚠️
...a/com/newrelic/agent/config/ConfigServiceImpl.java 80.00% 2 Missing ⚠️
...wrelic/agent/tracing/samplers/AdaptiveSampler.java 95.91% 0 Missing and 2 partials ⚠️
.../main/java/com/newrelic/agent/model/SpanEvent.java 50.00% 1 Missing ⚠️
.../analytics/CollectorSpanEventReservoirManager.java 75.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2513      +/-   ##
============================================
- Coverage     71.15%   70.26%   -0.89%     
+ Complexity    10746    10046     -700     
============================================
  Files           849      850       +1     
  Lines         42594    40904    -1690     
  Branches       6634     6191     -443     
============================================
- Hits          30307    28743    -1564     
+ Misses         9454     9359      -95     
+ Partials       2833     2802      -31     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kanderson250 kanderson250 marked this pull request as ready for review October 8, 2025 18:50
@kanderson250 kanderson250 changed the title (WIP) Period based adaptive sampler Period based adaptive sampler Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

tracing enhancements

3 participants