Parse 0-probability sampling strategies correctly (fixes: open-telemetry/opentelemetry-java-instrumentation#5899) (#4421)

* Parse 0-probability sampling strategies correctly (fixes: #5899)

* Run spotless

* Move default probabilistic strategy to `parseProbabilistic`
This commit is contained in:
Chris Gray 2022-05-06 06:29:05 -07:00 committed by GitHub
parent 606feb4ef5
commit 360da3cf9b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 34 additions and 6 deletions

View File

@ -43,8 +43,8 @@ class SamplingStrategyResponseUnMarshaler extends UnMarshaler {
parseSamplingStrategyType(responseBuilder, input);
break;
case 18:
input.readRawVarint32(); // skip length
responseBuilder.setProbabilisticSamplingStrategy(parseProbabilistic(input));
int length = input.readRawVarint32();
responseBuilder.setProbabilisticSamplingStrategy(parseProbabilistic(input, length));
break;
case 26:
input.readRawVarint32(); // skip length
@ -80,9 +80,13 @@ class SamplingStrategyResponseUnMarshaler extends UnMarshaler {
}
private static SamplingStrategyResponse.ProbabilisticSamplingStrategy parseProbabilistic(
CodedInputStream input) throws IOException {
CodedInputStream input, int length) throws IOException {
SamplingStrategyResponse.ProbabilisticSamplingStrategy.Builder builder =
new SamplingStrategyResponse.ProbabilisticSamplingStrategy.Builder();
if (length == 0) {
// Default probabilistic strategy.
return builder.setSamplingRate(0.0).build();
}
boolean done = false;
while (!done) {
int tag = input.readTag();
@ -184,8 +188,8 @@ class SamplingStrategyResponseUnMarshaler extends UnMarshaler {
break;
case 18:
probabilisticSamplingParsed = true;
input.readRawVarint32(); // skip length
builder.setProbabilisticSamplingStrategy(parseProbabilistic(input));
int length = input.readRawVarint32();
builder.setProbabilisticSamplingStrategy(parseProbabilistic(input, length));
break;
default:
input.skipField(tag);

View File

@ -9,6 +9,12 @@ import static io.opentelemetry.sdk.extension.trace.jaeger.sampler.JaegerRemoteSa
import static org.assertj.core.api.Assertions.assertThat;
import static org.awaitility.Awaitility.await;
import com.google.common.collect.ImmutableList;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.api.trace.TraceId;
import io.opentelemetry.context.Context;
import io.opentelemetry.sdk.trace.samplers.SamplingResult;
import java.time.Duration;
import org.junit.jupiter.api.Test;
import org.testcontainers.containers.BindMode;
@ -45,7 +51,20 @@ class JaegerRemoteSamplerIntegrationTest {
await()
.atMost(Duration.ofSeconds(10))
.untilAsserted(samplerIsType(remoteSampler, PerOperationSampler.class));
assertThat(remoteSampler.getDescription()).contains("0.33").doesNotContain("150");
assertThat(remoteSampler.getDescription())
.contains("op0")
.contains("op1")
.contains("op2")
.doesNotContain("150");
assertThat(
remoteSampler.shouldSample(
Context.current(),
TraceId.getInvalid(),
"op0",
SpanKind.CLIENT,
Attributes.empty(),
ImmutableList.of()))
.isEqualTo(SamplingResult.drop());
}
}

View File

@ -5,6 +5,11 @@
"type": "probabilistic",
"param": 0.33,
"operation_strategies": [
{
"operation": "op0",
"type": "probabilistic",
"param": 0
},
{
"operation": "op1",
"type": "probabilistic",