Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add probability to decision when appropriate #1116

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -230,7 +230,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 @@ -305,7 +305,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