Skip to content

Commit

Permalink
Add probability to decision when appropriate (#1116)
Browse files Browse the repository at this point in the history
* add the sampling probability to the decision, when appropriate.

* formatting

* normalize a method name and move attribute to semantic convention attributes

* move the sampling attribute to its own class in the SDK

* Move the sampling priority attribute to a non-public spot
  • Loading branch information
jkwatson authored Apr 22, 2020
1 parent 6e7f34d commit 4d62068
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 9 deletions.
2 changes: 1 addition & 1 deletion sdk/src/main/java/io/opentelemetry/sdk/trace/Sampler.java
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,6 @@ interface Decision {
* span or when sampling decision {@link #isSampled()} changes from false to true.
* @since 0.1.0
*/
Map<String, AttributeValue> attributes();
Map<String, AttributeValue> getAttributes();
}
}
57 changes: 53 additions & 4 deletions sdk/src/main/java/io/opentelemetry/sdk/trace/Samplers.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@

package io.opentelemetry.sdk.trace;

import static io.opentelemetry.common.AttributeValue.doubleAttributeValue;
import static java.util.Collections.singletonMap;

import com.google.auto.value.AutoValue;
import com.google.common.base.Preconditions;
import io.opentelemetry.common.AttributeValue;
Expand All @@ -25,6 +28,7 @@
import io.opentelemetry.trace.SpanContext;
import io.opentelemetry.trace.SpanId;
import io.opentelemetry.trace.TraceId;
import io.opentelemetry.trace.attributes.DoubleAttributeSetter;
import java.util.Collections;
import java.util.List;
import java.util.Map;
Expand All @@ -38,6 +42,7 @@
*/
@Immutable
public final class Samplers {

private static final Sampler ALWAYS_ON = new AlwaysOnSampler();
private static final Sampler ALWAYS_OFF = new AlwaysOffSampler();
private static final Decision ALWAYS_ON_DECISION = new SimpleDecision(/* decision= */ true);
Expand Down Expand Up @@ -162,13 +167,21 @@ static Probability create(double probability) {
} else {
idUpperBound = (long) (probability * Long.MAX_VALUE);
}
return new AutoValue_Samplers_Probability(probability, idUpperBound);
return new AutoValue_Samplers_Probability(
probability,
idUpperBound,
ProbabilityDecision.create(/* decision= */ true, probability),
ProbabilityDecision.create(/* decision= */ false, probability));
}

abstract double getProbability();

abstract long getIdUpperBound();

abstract Decision getPositiveDecision();

abstract Decision getNegativeDecision();

@Override
public final Decision shouldSample(
@Nullable SpanContext parentContext,
Expand Down Expand Up @@ -198,8 +211,8 @@ public final Decision shouldSample(
// This is considered a reasonable tradeoff for the simplicity/performance requirements (this
// code is executed in-line for every Span creation).
return Math.abs(traceId.getLowerLong()) < getIdUpperBound()
? ALWAYS_ON_DECISION
: ALWAYS_OFF_DECISION;
? getPositiveDecision()
: getNegativeDecision();
}

@Override
Expand Down Expand Up @@ -229,8 +242,44 @@ public boolean isSampled() {
}

@Override
public Map<String, AttributeValue> attributes() {
public Map<String, AttributeValue> getAttributes() {
return Collections.emptyMap();
}
}

/**
* Probability value used by a probability-based Span sampling strategy.
*
* <p>Note: This will need to be updated if a specification for this value is merged which changes
* this proposed value. Also, once it's in the spec, we should move it somewhere more visible.
*
* <p>See https://github.com/open-telemetry/opentelemetry-specification/pull/570
*/
static final DoubleAttributeSetter SAMPLING_PROBABILITY =
DoubleAttributeSetter.create("sampling.probability");

/** Probability-based sampling decision with a single attribute for the probability. */
@Immutable
@AutoValue
abstract static class ProbabilityDecision implements Decision {

ProbabilityDecision() {}

/**
* Creates sampling decision without attributes.
*
* @param decision sampling decision
* @param probability the probability that was used for the decision.
*/
static ProbabilityDecision create(boolean decision, double probability) {
return new AutoValue_Samplers_ProbabilityDecision(
decision, singletonMap(SAMPLING_PROBABILITY.key(), doubleAttributeValue(probability)));
}

@Override
public abstract boolean isSampled();

@Override
public abstract Map<String, AttributeValue> getAttributes();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ public Span startSpan() {
return DefaultSpan.create(spanContext);
}

attributes.putAllAttributes(samplingDecision.attributes());
attributes.putAllAttributes(samplingDecision.getAttributes());

return RecordEventsReadableSpan.startSpan(
spanContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package io.opentelemetry.sdk.trace;

import static com.google.common.truth.Truth.assertThat;
import static io.opentelemetry.common.AttributeValue.doubleAttributeValue;

import com.google.common.truth.Truth;
import io.opentelemetry.common.AttributeValue;
Expand Down Expand Up @@ -267,7 +268,8 @@ public void probabilitySampler_SampleBasedOnTraceId() {
Collections.<String, AttributeValue>emptyMap(),
Collections.<Link>emptyList());
assertThat(decision1.isSampled()).isFalse();
assertThat(decision1.attributes()).isEmpty();
assertThat(decision1.getAttributes())
.containsExactly(Samplers.SAMPLING_PROBABILITY.key(), doubleAttributeValue(0.0001));
// This traceId will be sampled by the Probability Sampler because the first 8 bytes as long
// is less than probability * Long.MAX_VALUE;
TraceId sampledtraceId =
Expand Down Expand Up @@ -301,6 +303,7 @@ public void probabilitySampler_SampleBasedOnTraceId() {
Collections.<String, AttributeValue>emptyMap(),
Collections.<Link>emptyList());
assertThat(decision2.isSampled()).isTrue();
assertThat(decision2.attributes()).isEmpty();
assertThat(decision1.getAttributes())
.containsExactly(Samplers.SAMPLING_PROBABILITY.key(), doubleAttributeValue(0.0001));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ public boolean isSampled() {
}

@Override
public Map<String, AttributeValue> attributes() {
public Map<String, AttributeValue> getAttributes() {
Map<String, AttributeValue> attributes = new LinkedHashMap<>();
attributes.put(
samplerAttributeName, AttributeValue.stringAttributeValue("bar"));
Expand Down

0 comments on commit 4d62068

Please sign in to comment.