Skip to content

Commit 91df77e

Browse files
kanderson250kanderson250
authored
Merge pull request #2513 from newrelic/period-based-adaptive-sampler
Period based adaptive sampler
2 parents f4473c2 + 7ba9e2b commit 91df77e

File tree

45 files changed

+1941
-486
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+1941
-486
lines changed

‎agent-interfaces/src/main/java/com/newrelic/agent/interfaces/SamplingPriorityQueue.java‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,11 @@ public interface SamplingPriorityQueue<E extends PriorityAware> {
3434

3535
String getServiceName();
3636

37-
int getSampled();
38-
39-
int getDecided();
40-
41-
int getTarget();
42-
43-
int getDecidedLast();
37+
int getTotalSampledPriorityEvents();
4438

4539
int size();
4640

4741
void clear();
42+
43+
void logReservoirStats();
4844
}

‎agent-model/src/main/java/com/newrelic/agent/model/AnalyticsEvent.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,6 @@ public long getTimestamp() {
6868
return timestamp;
6969
}
7070

71-
@Override
72-
public boolean decider() {
73-
return false;
74-
}
75-
7671
@Override
7772
public float getPriority() {
7873
return priority;

‎agent-model/src/main/java/com/newrelic/agent/model/PriorityAware.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,10 @@
88
package com.newrelic.agent.model;
99

1010
/**
11-
* Simple interface to grab a priority (float) value from an object and to determine if this app was the "decider".
11+
* Simple interface to grab a priority (float) value from an object.
1212
*/
1313
public interface PriorityAware {
1414

15-
boolean decider();
16-
1715
float getPriority();
1816

1917
}

‎agent-model/src/main/java/com/newrelic/agent/model/SpanEvent.java‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,11 @@ public class SpanEvent extends AnalyticsEvent implements JSONStreamAware {
2525
private final String appName;
2626
private final Map<String, Object> intrinsics;
2727
private final Map<String, Object> agentAttributes;
28-
private final boolean decider;
2928

3029
private SpanEvent(Builder builder) {
3130
super(SPAN, builder.timestamp, builder.priority, builder.userAttributes);
3231
this.appName = builder.appName;
3332
this.agentAttributes = builder.agentAttributes;
34-
this.decider = builder.decider;
3533
this.intrinsics = builder.intrinsics;
3634
}
3735

@@ -51,11 +49,6 @@ public Map<String, Object> getAgentAttributes() {
5149
return agentAttributes;
5250
}
5351

54-
@Override
55-
public boolean decider() {
56-
return decider;
57-
}
58-
5952
@Override
6053
public void writeJSONString(Writer out) throws IOException {
6154
JSONArray.writeJSONString(Arrays.asList(intrinsics, getMutableUserAttributes(), getAgentAttributes()), out);
@@ -98,16 +91,15 @@ public boolean equals(Object o) {
9891
return false;
9992
}
10093
SpanEvent spanEvent = (SpanEvent) o;
101-
return decider == spanEvent.decider &&
102-
Objects.equals(appName, spanEvent.appName) &&
94+
return Objects.equals(appName, spanEvent.appName) &&
10395
Objects.equals(intrinsics, spanEvent.intrinsics) &&
10496
Objects.equals(agentAttributes, spanEvent.agentAttributes) &&
10597
super.equals(o);
10698
}
10799

108100
@Override
109101
public int hashCode() {
110-
return Objects.hash(appName, intrinsics, agentAttributes, decider);
102+
return Objects.hash(appName, intrinsics, agentAttributes);
111103
}
112104

113105
public static class Builder {
@@ -116,7 +108,6 @@ public static class Builder {
116108
private final Map<String, Object> userAttributes = new HashMap<>();
117109
private String appName;
118110
private float priority;
119-
private boolean decider;
120111
private long timestamp;
121112
private Object spanKind;
122113

@@ -191,11 +182,6 @@ public Object getSpanKindFromUserAttributes() {
191182
return result == null ? CLIENT_SPAN_KIND : result;
192183
}
193184

194-
public Builder decider(boolean decider) {
195-
this.decider = decider;
196-
return this;
197-
}
198-
199185
public Builder timestamp(long timestamp) {
200186
this.timestamp = timestamp;
201187
return this;

‎agent-model/src/test/java/com/newrelic/agent/model/AnalyticsEventTest.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ public void allGetters_returnProperly() {
4646
Assert.assertEquals(0.9F, event.getPriority(), .1);
4747
Assert.assertEquals("type", event.getType());
4848
Assert.assertTrue(event.isValid());
49-
Assert.assertFalse(event.decider());
5049
}
5150

5251
@Test

‎agent-model/src/test/java/com/newrelic/agent/model/SpanEventTest.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,6 @@ public void testEquals() {
5353
assertNotEquals(span1, span5);
5454
assertNotEquals(span2, span5);
5555

56-
SpanEvent span6 = baseBuilder(now).decider(false).build();
57-
assertNotEquals(span1, span6);
58-
assertNotEquals(span2, span6);
59-
6056
SpanEvent span7 = baseBuilder(now).appName("somethingDifferent").build();
6157
assertNotEquals(span1, span7);
6258
assertNotEquals(span2, span7);
@@ -128,7 +124,6 @@ public void spanEvent_getsAll_Attributes() {
128124
assertEquals("thud", spanEvent.getName());
129125
assertEquals("8675zzz", spanEvent.getTransactionId());
130126
assertEquals("wally", spanEvent.getAppName());
131-
assertEquals(true, spanEvent.decider());
132127
}
133128

134129
private SpanEvent.Builder baseBuilderExtraUser(long now, String extraUserAttr, String value) {
@@ -155,7 +150,6 @@ private SpanEvent.Builder baseBuilder(long now) {
155150
.putAllUserAttributes(userAttributes)
156151
.putAllAgentAttributes(agentAttributes)
157152
.putAllIntrinsics(intrinsics)
158-
.decider(true)
159153
.appName("wally")
160154
.priority(21.7f)
161155
.timestamp(now);

‎instrumentation-test/src/test/java/com/newrelic/agent/introspec/internal/SpanEventImplTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ public class SpanEventImplTest {
1313
@Test
1414
public void getters_returnUnderlyingSpanEventData() throws IOException {
1515
SpanEvent.Builder spanEventBuilder = SpanEvent.builder();
16-
SpanEvent spanEvent = spanEventBuilder.appName("appname").decider(true).priority(.1f).timestamp(100000).putIntrinsic("category", "http")
16+
SpanEvent spanEvent = spanEventBuilder.appName("appname").priority(.1f).timestamp(100000).putIntrinsic("category", "http")
1717
.putIntrinsic("transactionId", "transactionid").putIntrinsic("duration", 0.1f)
1818
.putIntrinsic("name", "name").putIntrinsic("traceId", "traceid").putIntrinsic("guid", "guid")
1919
.putIntrinsic("parentId", "parentid").putAgentAttribute("http.url", "url").putAgentAttribute("http.statusCode", 200)

‎newrelic-agent/src/main/java/com/newrelic/agent/HeadersUtil.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
package com.newrelic.agent;
99

1010
import com.google.common.collect.ImmutableSet;
11+
import com.newrelic.agent.tracing.Sampled;
1112
import com.newrelic.api.agent.Headers;
1213
import com.newrelic.agent.config.DistributedTracingConfig;
1314
import com.newrelic.agent.tracers.DefaultTracer;
@@ -265,7 +266,7 @@ public static void parseAndAcceptDistributedTraceHeaders(Transaction tx, Inbound
265266
}
266267
if (w3CTracePayload.getTraceParent() != null) {
267268
tx.getSpanProxy().setInitiatingW3CTraceParent(w3CTracePayload.getTraceParent());
268-
tx.applyDistributedTracingSamplerConfig(w3CTracePayload.getTraceParent());
269+
tx.assignPriorityFromRemoteParent(w3CTracePayload.getTraceParent().sampled());
269270
}
270271
if (w3CTracePayload.getTraceState() != null) {
271272
tx.getSpanProxy().setInitiatingW3CTraceState(w3CTracePayload.getTraceState());
@@ -276,6 +277,11 @@ public static void parseAndAcceptDistributedTraceHeaders(Transaction tx, Inbound
276277
String tracePayload = HeadersUtil.getNewRelicTraceHeader(inboundHeaders);
277278
if (tracePayload != null) {
278279
tx.acceptDistributedTracePayload(tracePayload);
280+
DistributedTracePayloadImpl newRelicPayload = tx.getSpanProxy().getInboundDistributedTracePayload();
281+
boolean samplingDecisionExists = newRelicPayload.sampled != Sampled.UNKNOWN;
282+
if (samplingDecisionExists) {
283+
tx.assignPriorityFromRemoteParent(newRelicPayload.sampled.booleanValue());
284+
}
279285
}
280286
}
281287
}

‎newrelic-agent/src/main/java/com/newrelic/agent/RPMService.java‎

Lines changed: 1 addition & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import com.newrelic.agent.config.AgentJarHelper;
1515
import com.newrelic.agent.config.BrowserMonitoringConfig;
1616
import com.newrelic.agent.config.BrowserMonitoringConfigImpl;
17+
import com.newrelic.agent.config.DistributedTracingConfig;
1718
import com.newrelic.agent.config.Hostname;
1819
import com.newrelic.agent.config.SystemPropertyFactory;
1920
import com.newrelic.agent.environment.AgentIdentity;
@@ -238,9 +239,6 @@ private Map<String, Object> getSettings(boolean sendEnvironmentInfo) {
238239
}
239240
settings.put("services", ServiceFactory.getServicesConfiguration());
240241

241-
// the sysprops and envvars above an in unmodifiable collections, so just change it here
242-
updateSamplingTargetSettings(settings);
243-
244242
return settings;
245243
}
246244

@@ -973,51 +971,6 @@ private void handle503Error(Exception e) {
973971
}
974972
}
975973

976-
private void updateSamplingTargetSettings (Map<String, Object> settings) {
977-
// the new distributed_tracing.sampler.adaptive_sampling_target is meant to be per minute
978-
// but, the harvest cycle is 12 times per minute right now,
979-
// so we need to divide this number by 12 until that changes
980-
// there are 2 spots we need to do this:
981-
// - settings.distributed_tracing_sampler_adaptive_sampling_target
982-
// - settings.distributed_tracing.sampler.adaptive_sampling_target
983-
if (settings == null) return;
984-
try {
985-
// local settings
986-
Map<String, Object> dtConfig = (Map<String, Object>) settings.get("distributed_tracing");
987-
if (dtConfig != null) {
988-
Map<String, Object> samplerConfig = (Map<String, Object>) dtConfig.get("sampler");
989-
if (samplerConfig != null) {
990-
Integer adaptiveSamplingTarget = toInt(samplerConfig.get("adaptive_sampling_target"));
991-
if (adaptiveSamplingTarget != null) {
992-
Integer newAdaptiveSamplingTarget = (int) Math.ceil(adaptiveSamplingTarget.doubleValue() / 12.0);
993-
NewRelic.getAgent()
994-
.getLogger()
995-
.log(Level.FINE, "Updating local setting adaptive_sampling_target from " + adaptiveSamplingTarget + " to " +
996-
newAdaptiveSamplingTarget);
997-
samplerConfig.put("adaptive_sampling_target", newAdaptiveSamplingTarget);
998-
}
999-
}
1000-
}
1001-
} catch (Exception e) {
1002-
NewRelic.getAgent().getLogger().log(Level.WARNING, "Unable to parse local setting adaptive_sampling_target setting: {0}", e);
1003-
}
1004-
1005-
try {
1006-
// env var
1007-
if (settings.containsKey("distributed_tracing_sampler_adaptive_sampling_target")) {
1008-
Integer adaptiveSamplingTarget = toInt(settings.get("distributed_tracing_sampler_adaptive_sampling_target"));
1009-
Integer newAdaptiveSamplingTarget = (int) Math.ceil(adaptiveSamplingTarget.doubleValue() / 12.0);
1010-
NewRelic.getAgent()
1011-
.getLogger()
1012-
.log(Level.FINE, "Updating environment variable adaptive_sampling_target from " + adaptiveSamplingTarget + " to " +
1013-
newAdaptiveSamplingTarget);
1014-
settings.put("distributed_tracing_sampler_adaptive_sampling_target", newAdaptiveSamplingTarget);
1015-
}
1016-
} catch (Exception e) {
1017-
NewRelic.getAgent().getLogger().log(Level.WARNING, "Unable to parse environment variable adaptive_sampling_target setting: {0}", e);
1018-
}
1019-
}
1020-
1021974
private static Integer toInt(Object o) {
1022975
if (o == null) return null;
1023976
if (o instanceof Number) {

‎newrelic-agent/src/main/java/com/newrelic/agent/Transaction.java‎

Lines changed: 66 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,6 @@
4040
import com.newrelic.agent.normalization.Normalizer;
4141
import com.newrelic.agent.service.ServiceFactory;
4242
import com.newrelic.agent.service.ServiceUtils;
43-
import com.newrelic.agent.service.analytics.DistributedSamplingPriorityQueue;
44-
import com.newrelic.agent.service.analytics.TransactionEvent;
4543
import com.newrelic.agent.sql.SlowQueryListener;
4644
import com.newrelic.agent.stats.AbstractMetricAggregator;
4745
import com.newrelic.agent.stats.StatsWorks;
@@ -59,8 +57,8 @@
5957
import com.newrelic.agent.tracing.DistributedTracePayloadImpl;
6058
import com.newrelic.agent.tracing.DistributedTraceService;
6159
import com.newrelic.agent.tracing.DistributedTraceServiceImpl;
60+
import com.newrelic.agent.tracing.Sampled;
6261
import com.newrelic.agent.tracing.SpanProxy;
63-
import com.newrelic.agent.tracing.W3CTraceParent;
6462
import com.newrelic.agent.transaction.PriorityTransactionName;
6563
import com.newrelic.agent.transaction.TransactionCache;
6664
import com.newrelic.agent.transaction.TransactionCounts;
@@ -283,6 +281,7 @@ public long getTransportDurationInMillis() {
283281

284282
// WARNING: Mutates this instance by mutating the span proxy
285283
public DistributedTracePayloadImpl createDistributedTracePayload(String spanId) {
284+
assignPriorityRootIfNotSet();
286285
SpanProxy spanProxy = this.spanProxy.get();
287286
long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - this.getTransactionTimer().getStartTimeInNanos());
288287
long txnStartTimeSinceEpochInMillis = System.currentTimeMillis() - elapsedMillis;
@@ -333,39 +332,75 @@ public boolean acceptDistributedTracePayload(DistributedTracePayload payload) {
333332
}
334333
}
335334

336-
public void applyDistributedTracingSamplerConfig(W3CTraceParent parent) {
337-
if (parent != null) {
338-
DistributedTracingConfig dtConfig = getAgentConfig().getDistributedTracingConfig();
339-
if (parent.sampled()) { // traceparent exists and sampled is 1
340-
if (DistributedTracingConfig.SAMPLE_ALWAYS_ON.equals(dtConfig.getRemoteParentSampled())) {
341-
this.setPriorityIfNotNull(2.0f);
342-
} else if (DistributedTracingConfig.SAMPLE_ALWAYS_OFF.equals(dtConfig.getRemoteParentSampled())) {
343-
this.setPriorityIfNotNull(0.0f);
344-
} // else leave it as it was
345-
} else { // traceparent exists and sampled is 0
346-
if (DistributedTracingConfig.SAMPLE_ALWAYS_ON.equals(dtConfig.getRemoteParentNotSampled())) {
347-
this.setPriorityIfNotNull(2.0f);
348-
} else if (DistributedTracingConfig.SAMPLE_ALWAYS_OFF.equals(dtConfig.getRemoteParentNotSampled())) {
349-
this.setPriorityIfNotNull(0.0f);
350-
} // else leave it as it was
335+
/**
336+
* Assigns priority to this transaction using sampling and priority data from a remote parent.
337+
*
338+
* There are two kinds of parent data that are used:
339+
* - The remote parent sampled flag, which indicates whether the remote parent is sampled or not. This
340+
* determines which parent sampler (remote_parent_sampled or remote_parent_not_sampled) to use when making the priority assignment.
341+
* - Inbound priority data, which is taken from the span proxy's inbound payload if available. This
342+
* will be used by the adaptive sampler if configured, and ignored by all other sampler types.
343+
*
344+
* @param remoteParentSampled whether the remote parent was sampled or not
345+
*/
346+
public void assignPriorityFromRemoteParent(boolean remoteParentSampled) {
347+
DistributedTraceService dtService = ServiceFactory.getDistributedTraceService();
348+
float priority = dtService.calculatePriorityRemoteParent(remoteParentSampled, getPriorityFromInboundSamplingDecision());
349+
this.priority.set(priority);
350+
}
351+
352+
/**
353+
* Assigns priority to this transaction (unless previously assigned) without any information from a remote parent.
354+
*
355+
* If Distributed Tracing is enabled, and no priority has been set on this transaction, the configured root sampler
356+
* will be used to obtain a priority for this transaction. No inbound priority data is read (because if an inbound
357+
* payload was processed, it should have made a priority assignment earlier).
358+
*
359+
* If a priority assignment has already been made, this call is ignored. This is a required check to avoid
360+
* overwriting any priority decision that was made earlier, either because a remote parent was processed or a previous
361+
* call to this method was made earlier in the txn's lifecycle. It is also required to avoid accidentally running the
362+
* adaptive sampler twice on the same transaction.
363+
*
364+
* If Distributed Tracing is not enabled, or this is a synthetic transaction, a random priority in [0,1) is assigned.
365+
*/
366+
public void assignPriorityRootIfNotSet(){
367+
if (getAgentConfig().getDistributedTracingConfig().isEnabled()){
368+
if (priority.get() == null){
369+
//The "if" check above is required even though we do compareAndSet(null) below.
370+
//Its purpose is to avoid running the sampler more than once for the same txn.
371+
Float samplerPriority = ServiceFactory.getDistributedTraceService().calculatePriorityRoot();
372+
priority.compareAndSet(null, samplerPriority);
351373
}
374+
} else {
375+
priority.compareAndSet(null, DistributedTraceServiceImpl.nextTruncatedFloat());
352376
}
353377
}
354378

355-
private void checkAndSetPriority() {
356-
if (getAgentConfig().getDistributedTracingConfig().isEnabled()) {
357-
DistributedTraceService distributedTraceService = ServiceFactory.getDistributedTraceService();
358-
359-
DistributedTracePayloadImpl inboundPayload = spanProxy.get().getInboundDistributedTracePayload();
360-
Float inboundPriority = inboundPayload != null ? inboundPayload.priority : null;
361-
362-
DistributedSamplingPriorityQueue<TransactionEvent> reservoir = ServiceFactory.getTransactionEventsService()
363-
.getOrCreateDistributedSamplingReservoir(getApplicationName());
379+
/***
380+
* Retrieve priority from the inbound payload (using both sampling and priority-related information).
381+
*
382+
* First, check to see if there is a sampling decision available on the inbound payload.
383+
* - If there is a sampling decision, use the inbound priority if it exists or compute a new priority.
384+
* - If there is no sampling decision, return null.
385+
*
386+
* In the case of W3C headers, this is distinct from the remoteParentSampled decision we get from the trace parent header.
387+
* The sampling and priority values in the payload come from the trace state header (and they may be missing, even if we got a sampled
388+
* flag on the trace parent header).
389+
*
390+
* @return a float in [0, 2) if priority-related information was found, or null if a new decision needs to be made
391+
*/
364392

365-
priority.compareAndSet(null, distributedTraceService.calculatePriority(inboundPriority, reservoir));
366-
} else {
367-
priority.compareAndSet(null, DistributedTraceServiceImpl.nextTruncatedFloat());
393+
@VisibleForTesting
394+
protected Float getPriorityFromInboundSamplingDecision(){
395+
DistributedTracePayloadImpl payload = spanProxy.get().getInboundDistributedTracePayload();
396+
if (payload != null && payload.sampled != Sampled.UNKNOWN) {
397+
if (payload.priority != null) {
398+
return payload.priority;
399+
} else {
400+
return (payload.sampled.booleanValue() ? 1.0f : 0.0f) + DistributedTraceServiceImpl.nextTruncatedFloat();
401+
}
368402
}
403+
return null;
369404
}
370405

371406
public TransportType getTransportType() {
@@ -496,7 +531,6 @@ protected Transaction() {
496531
// registered.
497532
private void postConstruct() {
498533
this.initialActivity = TransactionActivity.create(this, nextActivityId.getAndIncrement());;
499-
checkAndSetPriority();
500534
}
501535

502536
private static long getGCTime() {
@@ -1012,7 +1046,7 @@ private void finishTransaction() {
10121046
synchronized (lock) {
10131047
// this may have the side-effect of ignoring the transaction
10141048
freezeTransactionName();
1015-
1049+
assignPriorityRootIfNotSet();
10161050
if (ignore) {
10171051
Agent.LOG.log(Level.FINER,
10181052
"Transaction {0} was cancelled: ignored. This is not an error condition.", this);

0 commit comments

Comments
 (0)