diff --git a/dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/TelemetryPolicy.java b/dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/TelemetryPolicy.java index 11dfae6b2..220ca2a09 100644 --- a/dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/TelemetryPolicy.java +++ b/dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/TelemetryPolicy.java @@ -5,43 +5,38 @@ package io.opentelemetry.contrib.dynamic.policy; -import com.fasterxml.jackson.databind.JsonNode; import java.util.Objects; -import javax.annotation.Nullable; /** - * Represents a single Telemetry Policy, comprising a type and a specification. + * Represents a single Telemetry Policy identified by type. * *

A {@code TelemetryPolicy} instance encapsulates a specific rule or configuration intent - * identified by its {@link #getType() type}. The behavior of the policy is defined by its {@link - * #getSpec() specification}, which is a JSON object. + * identified by its {@link #getType() type}. * - *

Policies are typically immutable data carriers. The {@code spec} can be {@code null}, which - * conventionally signifies the removal or absence of a policy for the given type in certain - * contexts (e.g., when calculating diffs or updates). + *

Policies are immutable data carriers. * - *

As an example take the JSON structure `{"trace-sampling": {"probability" : 0.5}}` This is of - * type "trace-sampling", with spec `{"probability" : 0.5}`, indicating the intent that the trace - * sampling-probability be set to 50% + *

As an example, policy type {@code trace-sampling} indicates that trace sampling behavior + * should be configured. + * + *

Direct instantiation of this base class is intentionally supported for type-only policy + * signals (for example, to indicate policy removal/reset without policy-specific values). * * @see io.opentelemetry.contrib.dynamic.policy */ public class TelemetryPolicy { private final String type; // e.g. "trace-sampling" - // JSON content after schema validation - // null means removed, which is relevant for merging policies - @Nullable private final JsonNode spec; /** * Constructs a new TelemetryPolicy. * + *

This constructor is used by type-specific subclasses and also directly for type-only policy + * signals such as policy removal/reset. + * * @param type the type of the policy (e.g., "trace-sampling"), must not be null. - * @param spec the JSON specification of the policy, or {@code null} to indicate removal. */ - public TelemetryPolicy(String type, @Nullable JsonNode spec) { + public TelemetryPolicy(String type) { Objects.requireNonNull(type, "type cannot be null"); this.type = type; - this.spec = spec; } /** @@ -55,34 +50,4 @@ public TelemetryPolicy(String type, @Nullable JsonNode spec) { public String getType() { return type; } - - /** - * Returns the specification of this policy. - * - *

The specification is a JSON structure defining the parameters of the policy. If {@code - * null}, it may indicate that the policy is being removed or is empty. - * - * @return the policy specification, or {@code null}. - */ - @Nullable - public JsonNode getSpec() { - return spec; - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (!(o instanceof TelemetryPolicy)) { - return false; - } - TelemetryPolicy that = (TelemetryPolicy) o; - return Objects.equals(type, that.type) && Objects.equals(spec, that.spec); - } - - @Override - public int hashCode() { - return Objects.hash(type, spec); - } } diff --git a/dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/TraceSamplingRatePolicy.java b/dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/TraceSamplingRatePolicy.java new file mode 100644 index 000000000..1fb285121 --- /dev/null +++ b/dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/TraceSamplingRatePolicy.java @@ -0,0 +1,24 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.contrib.dynamic.policy; + +public final class TraceSamplingRatePolicy extends TelemetryPolicy { + public static final String TYPE = "trace-sampling"; + + private final double probability; + + public TraceSamplingRatePolicy(double probability) { + super(TYPE); + if (Double.isNaN(probability) || probability < 0.0 || probability > 1.0) { + throw new IllegalArgumentException("probability must be within [0.0, 1.0]"); + } + this.probability = probability; + } + + public double getProbability() { + return probability; + } +} diff --git a/dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/TraceSamplingRatePolicyImplementer.java b/dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/TraceSamplingRatePolicyImplementer.java index 18b38c90b..95947bf8d 100644 --- a/dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/TraceSamplingRatePolicyImplementer.java +++ b/dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/TraceSamplingRatePolicyImplementer.java @@ -5,7 +5,6 @@ package io.opentelemetry.contrib.dynamic.policy; -import com.fasterxml.jackson.databind.JsonNode; import io.opentelemetry.contrib.dynamic.sampler.DelegatingSampler; import io.opentelemetry.sdk.trace.samplers.Sampler; import java.util.Collections; @@ -16,25 +15,21 @@ * Implements the {@code trace-sampling} policy by updating a {@link DelegatingSampler}. * *

This implementer listens for validated {@link TelemetryPolicy} updates of type {@code - * "trace-sampling"} and applies the {@code probability} field to the delegate sampler using {@link - * Sampler#traceIdRatioBased(double)} wrapped by {@link Sampler#parentBased(Sampler)}. + * "trace-sampling"} and applies {@link TraceSamplingRatePolicy#getProbability()} to the delegate + * sampler using {@link Sampler#traceIdRatioBased(double)} wrapped by {@link + * Sampler#parentBased(Sampler)}. * - *

If the policy spec is {@code null} (policy removal), the delegate falls back to {@link - * Sampler#alwaysOn()}. + *

If a type-only {@link TelemetryPolicy} of type {@code "trace-sampling"} is received, it is + * treated as policy removal and the delegate falls back to {@link Sampler#alwaysOn()}. * *

Validation is performed by {@link TraceSamplingValidator}; this implementer only consumes * policies produced by that validator. * - *

Policies with a non-null spec that omit {@code probability} are ignored, which should not - * occur if validation is functioning correctly. - * *

This class is thread-safe. Calls to {@link #onPoliciesChanged(List)} can occur concurrently * with sampling operations on the associated {@link DelegatingSampler}. */ public final class TraceSamplingRatePolicyImplementer implements PolicyImplementer { - private static final String TRACE_SAMPLING_TYPE = "trace-sampling"; - private static final String PROBABILITY_FIELD = "probability"; private static final List VALIDATORS = Collections.singletonList(new TraceSamplingValidator()); @@ -58,19 +53,17 @@ public List getValidators() { @Override public void onPoliciesChanged(List policies) { for (TelemetryPolicy policy : policies) { - if (!TRACE_SAMPLING_TYPE.equals(policy.getType())) { + if (!TraceSamplingRatePolicy.TYPE.equals(policy.getType())) { continue; } - JsonNode spec = policy.getSpec(); - if (spec == null) { + if (!(policy instanceof TraceSamplingRatePolicy)) { + // Type-only policy represents removing trace-sampling config. delegatingSampler.setDelegate(Sampler.alwaysOn()); continue; } - if (spec.has(PROBABILITY_FIELD)) { - double ratio = spec.get(PROBABILITY_FIELD).asDouble(1.0); - Sampler sampler = Sampler.parentBased(Sampler.traceIdRatioBased(ratio)); - delegatingSampler.setDelegate(sampler); - } + double ratio = ((TraceSamplingRatePolicy) policy).getProbability(); + Sampler sampler = Sampler.parentBased(Sampler.traceIdRatioBased(ratio)); + delegatingSampler.setDelegate(sampler); } } } diff --git a/dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/TraceSamplingValidator.java b/dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/TraceSamplingValidator.java index 1279e4731..38ad2e60d 100644 --- a/dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/TraceSamplingValidator.java +++ b/dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/TraceSamplingValidator.java @@ -23,7 +23,7 @@ public final class TraceSamplingValidator implements PolicyValidator { @Override public String getPolicyType() { - return "trace-sampling"; + return TraceSamplingRatePolicy.TYPE; } @Override @@ -43,7 +43,7 @@ public TelemetryPolicy validate(String json) { if (probNode.isNumber()) { double d = probNode.asDouble(); if (d >= 0.0 && d <= 1.0) { - return new TelemetryPolicy(getPolicyType(), spec); + return new TraceSamplingRatePolicy(d); } } } @@ -62,8 +62,7 @@ public TelemetryPolicy validateAlias(String key, String value) { try { double d = Double.parseDouble(value); if (d >= 0.0 && d <= 1.0) { - JsonNode spec = MAPPER.createObjectNode().put("probability", d); - return new TelemetryPolicy(getPolicyType(), spec); + return new TraceSamplingRatePolicy(d); } } catch (NumberFormatException e) { // invalid diff --git a/dynamic-control/src/test/java/io/opentelemetry/contrib/dynamic/policy/LinePerPolicyFileProviderTest.java b/dynamic-control/src/test/java/io/opentelemetry/contrib/dynamic/policy/LinePerPolicyFileProviderTest.java index 92dad4259..3d8070418 100644 --- a/dynamic-control/src/test/java/io/opentelemetry/contrib/dynamic/policy/LinePerPolicyFileProviderTest.java +++ b/dynamic-control/src/test/java/io/opentelemetry/contrib/dynamic/policy/LinePerPolicyFileProviderTest.java @@ -111,7 +111,7 @@ public TelemetryPolicy validate(String json) { if (!acceptJson) { return null; } - return new TelemetryPolicy(TRACE_SAMPLING_TYPE, null); + return new TelemetryPolicy(TRACE_SAMPLING_TYPE); } @Override @@ -124,7 +124,7 @@ public TelemetryPolicy validateAlias(String key, String value) { if (!acceptAlias) { return null; } - return new TelemetryPolicy(TRACE_SAMPLING_TYPE, null); + return new TelemetryPolicy(TRACE_SAMPLING_TYPE); } @Override diff --git a/dynamic-control/src/test/java/io/opentelemetry/contrib/dynamic/policy/TraceSamplingRatePolicyImplementerTest.java b/dynamic-control/src/test/java/io/opentelemetry/contrib/dynamic/policy/TraceSamplingRatePolicyImplementerTest.java index 729dd88e6..559b783bf 100644 --- a/dynamic-control/src/test/java/io/opentelemetry/contrib/dynamic/policy/TraceSamplingRatePolicyImplementerTest.java +++ b/dynamic-control/src/test/java/io/opentelemetry/contrib/dynamic/policy/TraceSamplingRatePolicyImplementerTest.java @@ -8,8 +8,6 @@ import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.context.Context; @@ -24,15 +22,13 @@ class TraceSamplingRatePolicyImplementerTest { - private static final ObjectMapper MAPPER = new ObjectMapper(); - @Test - void nullSpecFallsBackToAlwaysOn() { + void typeOnlyTraceSamplingPolicyFallsBackToAlwaysOn() { DelegatingSampler delegatingSampler = new DelegatingSampler(Sampler.alwaysOff()); TraceSamplingRatePolicyImplementer implementer = new TraceSamplingRatePolicyImplementer(delegatingSampler); - implementer.onPoliciesChanged(singletonList(new TelemetryPolicy("trace-sampling", null))); + implementer.onPoliciesChanged(singletonList(new TelemetryPolicy("trace-sampling"))); assertThat(decisionFor(delegatingSampler)).isEqualTo(SamplingDecision.RECORD_AND_SAMPLE); } @@ -43,8 +39,7 @@ void appliesProbabilityToDelegate() { TraceSamplingRatePolicyImplementer implementer = new TraceSamplingRatePolicyImplementer(delegatingSampler); - implementer.onPoliciesChanged( - singletonList(new TelemetryPolicy("trace-sampling", spec("probability", 1.0)))); + implementer.onPoliciesChanged(singletonList(new TraceSamplingRatePolicy(1.0))); assertThat(decisionFor(delegatingSampler)).isEqualTo(SamplingDecision.RECORD_AND_SAMPLE); } @@ -55,20 +50,7 @@ void ignoresUnrelatedPolicyTypes() { TraceSamplingRatePolicyImplementer implementer = new TraceSamplingRatePolicyImplementer(delegatingSampler); - implementer.onPoliciesChanged( - singletonList(new TelemetryPolicy("other-policy", spec("value", 1.0)))); - - assertThat(decisionFor(delegatingSampler)).isEqualTo(SamplingDecision.DROP); - } - - @Test - void ignoresTraceSamplingPolicyWithoutProbability() { - DelegatingSampler delegatingSampler = new DelegatingSampler(Sampler.alwaysOff()); - TraceSamplingRatePolicyImplementer implementer = - new TraceSamplingRatePolicyImplementer(delegatingSampler); - - implementer.onPoliciesChanged( - singletonList(new TelemetryPolicy("trace-sampling", spec("other-field", 1.0)))); + implementer.onPoliciesChanged(singletonList(new TelemetryPolicy("other-policy"))); assertThat(decisionFor(delegatingSampler)).isEqualTo(SamplingDecision.DROP); } @@ -80,9 +62,7 @@ void lastTraceSamplingPolicyWins() { new TraceSamplingRatePolicyImplementer(delegatingSampler); List policies = - Arrays.asList( - new TelemetryPolicy("trace-sampling", spec("probability", 0.0)), - new TelemetryPolicy("trace-sampling", spec("probability", 1.0))); + Arrays.asList(new TraceSamplingRatePolicy(0.0), new TraceSamplingRatePolicy(1.0)); implementer.onPoliciesChanged(policies); @@ -100,8 +80,4 @@ private static SamplingDecision decisionFor(DelegatingSampler sampler) { Collections.emptyList()); return result.getDecision(); } - - private static JsonNode spec(String field, double value) { - return MAPPER.createObjectNode().put(field, value); - } } diff --git a/dynamic-control/src/test/java/io/opentelemetry/contrib/dynamic/policy/TraceSamplingValidatorTest.java b/dynamic-control/src/test/java/io/opentelemetry/contrib/dynamic/policy/TraceSamplingValidatorTest.java index bdfee8ec9..d24090850 100644 --- a/dynamic-control/src/test/java/io/opentelemetry/contrib/dynamic/policy/TraceSamplingValidatorTest.java +++ b/dynamic-control/src/test/java/io/opentelemetry/contrib/dynamic/policy/TraceSamplingValidatorTest.java @@ -15,7 +15,6 @@ class TraceSamplingValidatorTest { private static final String TRACE_SAMPLING_POLICY_TYPE = "trace-sampling"; - private static final String PROBABILITY_FIELD = "probability"; private static final String TRACE_SAMPLING_ALIAS = "trace-sampling.probability"; private final TraceSamplingValidator validator = new TraceSamplingValidator(); @@ -36,7 +35,8 @@ void testValidate_ValidJson() { TelemetryPolicy policy = validator.validate(json); assertThat(policy).isNotNull(); assertThat(policy.getType()).isEqualTo(TRACE_SAMPLING_POLICY_TYPE); - assertThat(policy.getSpec().get(PROBABILITY_FIELD).asDouble()).isCloseTo(0.5, within(1e-9)); + assertThat(policy).isInstanceOf(TraceSamplingRatePolicy.class); + assertThat(((TraceSamplingRatePolicy) policy).getProbability()).isCloseTo(0.5, within(1e-9)); } @ParameterizedTest @@ -46,7 +46,8 @@ void testValidate_ValidJson_BoundaryValues(double probability) { TelemetryPolicy policy = validator.validate(json); assertThat(policy).isNotNull(); assertThat(policy.getType()).isEqualTo(TRACE_SAMPLING_POLICY_TYPE); - assertThat(policy.getSpec().get(PROBABILITY_FIELD).asDouble()) + assertThat(policy).isInstanceOf(TraceSamplingRatePolicy.class); + assertThat(((TraceSamplingRatePolicy) policy).getProbability()) .isCloseTo(probability, within(1e-9)); } @@ -70,8 +71,7 @@ void testValidate_InvalidJson_MissingProbability() { @Test void testValidate_InvalidJson_ProbabilityNotNumber() { - String json = - "{\"" + TRACE_SAMPLING_POLICY_TYPE + "\": {\"" + PROBABILITY_FIELD + "\": \"high\"}}"; + String json = "{\"" + TRACE_SAMPLING_POLICY_TYPE + "\": {\"probability\": \"high\"}}"; assertThat(validator.validate(json)).isNull(); } @@ -87,7 +87,8 @@ void testValidateAlias_Valid() { TelemetryPolicy policy = validator.validateAlias(TRACE_SAMPLING_ALIAS, "0.5"); assertThat(policy).isNotNull(); assertThat(policy.getType()).isEqualTo(TRACE_SAMPLING_POLICY_TYPE); - assertThat(policy.getSpec().get(PROBABILITY_FIELD).asDouble()).isCloseTo(0.5, within(1e-9)); + assertThat(policy).isInstanceOf(TraceSamplingRatePolicy.class); + assertThat(((TraceSamplingRatePolicy) policy).getProbability()).isCloseTo(0.5, within(1e-9)); } @ParameterizedTest @@ -96,7 +97,8 @@ void testValidateAlias_Valid_BoundaryValues(String probability) { TelemetryPolicy policy = validator.validateAlias(TRACE_SAMPLING_ALIAS, probability); assertThat(policy).isNotNull(); assertThat(policy.getType()).isEqualTo(TRACE_SAMPLING_POLICY_TYPE); - assertThat(policy.getSpec().get(PROBABILITY_FIELD).asDouble()) + assertThat(policy).isInstanceOf(TraceSamplingRatePolicy.class); + assertThat(((TraceSamplingRatePolicy) policy).getProbability()) .isCloseTo(Double.parseDouble(probability), within(1e-9)); } @@ -117,12 +119,6 @@ void testValidateAlias_InvalidValue_OutOfRange(String probability) { } private static String jsonForProbability(double probability) { - return "{\"" - + TRACE_SAMPLING_POLICY_TYPE - + "\": {\"" - + PROBABILITY_FIELD - + "\": " - + probability - + "}}"; + return "{\"" + TRACE_SAMPLING_POLICY_TYPE + "\": {\"probability\": " + probability + "}}"; } }