[dynamic control] Move away from JSON requirement (#2652)

Author: jackshirazi

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.
  *
  * <p>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}.
  *
- * <p>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).
+ * <p>Policies are immutable data carriers.
  *
- * <p>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%
+ * <p>As an example, policy type {@code trace-sampling} indicates that trace sampling behavior
+ * should be configured.
+ *
+ * <p>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.
    *
+   * <p>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.
-   *
-   * <p>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);
-  }
 }

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;
+  }
+}

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}.
  *
  * <p>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)}.
  *
- * <p>If the policy spec is {@code null} (policy removal), the delegate falls back to {@link
- * Sampler#alwaysOn()}.
+ * <p>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()}.
  *
  * <p>Validation is performed by {@link TraceSamplingValidator}; this implementer only consumes
  * policies produced by that validator.
  *
- * <p>Policies with a non-null spec that omit {@code probability} are ignored, which should not
- * occur if validation is functioning correctly.
- *
  * <p>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<PolicyValidator> VALIDATORS =
       Collections.<PolicyValidator>singletonList(new TraceSamplingValidator());
 
@@ -58,19 +53,17 @@ public List<PolicyValidator> getValidators() {
   @Override
   public void onPoliciesChanged(List<TelemetryPolicy> 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);
     }
   }
 }

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

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

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<TelemetryPolicy> 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);
-  }
 }

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 + "}}";
   }
 }