[dynamic control] complete the jsonkeyvalue and simpler validation migration (#2715)

Author: jackshirazi

dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/AbstractSourcePolicyValidator.java

@@ -23,7 +23,6 @@ public final TelemetryPolicy validate(SourceWrapper source) {
     }
     SourceFormat format = source.getFormat();
     switch (format) {
-      case JSON: // transitional: same payload shape as JSONKEYVALUE until JSON is removed
       case JSONKEYVALUE:
         return validateJsonSource(((JsonSourceWrapper) source).asJsonNode());
       case KEYVALUE:

dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/LinePerPolicyFileProvider.java

@@ -20,13 +20,14 @@
  * A {@link PolicyProvider} that reads policies from a local file, where each line represents a
  * separate policy configuration.
  *
- * <p>The file format supports JSON and key-value lines:
+ * <p>Each non-empty line is parsed using one of two {@link SourceFormat}s:
  *
  * <ul>
- *   <li><b>JSON Objects:</b> Lines starting with <code>{</code> are treated as JSON objects and
- *       validated against the registered {@link PolicyValidator}s.
- *   <li><b>Key-Value:</b> Lines containing <code>=</code> are treated as key-value policy lines and
- *       validated against the registered {@link PolicyValidator}s.
+ *   <li><b>{@link SourceFormat#JSONKEYVALUE JSONKEYVALUE}:</b> Lines starting with <code>{</code>
+ *       use {@link SourceFormat#JSONKEYVALUE}: JSON text for a single top-level object with exactly
+ *       one key (the policy type) and one value (the policy payload).
+ *   <li><b>{@link SourceFormat#KEYVALUE KEYVALUE}:</b> Lines containing <code>=</code> are parsed
+ *       as {@code policyType=value} and validated against the registered {@link PolicyValidator}s.
  * </ul>
  *
  * <p>Empty lines and lines starting with <code>#</code> are ignored.
@@ -60,7 +61,7 @@ public List<TelemetryPolicy> fetchPolicies() throws IOException {
 
             SourceFormat format;
             if (trimmedLine.startsWith("{")) {
-              format = SourceFormat.JSON;
+              format = SourceFormat.JSONKEYVALUE;
             } else if (trimmedLine.indexOf('=') >= 0) {
               format = SourceFormat.KEYVALUE;
             } else {
@@ -74,12 +75,18 @@ public List<TelemetryPolicy> fetchPolicies() throws IOException {
             }
 
             SourceWrapper parsedSource = parsedSources.get(0);
+            String policyType = parsedSource.getPolicyType();
+            if (policyType == null || policyType.isEmpty()) {
+              logger.info("Policy type not found in line: " + trimmedLine);
+              return;
+            }
             TelemetryPolicy policy = null;
             for (PolicyValidator validator : validators) {
-              policy = validator.validate(parsedSource);
-              if (policy != null) {
-                break;
+              if (!policyType.equals(validator.getPolicyType())) {
+                continue;
               }
+              policy = validator.validate(parsedSource);
+              break;
             }
             if (policy == null) {
               logger.info("Validator not found or rejected for line: " + trimmedLine);

dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/PolicyValidator.java

@@ -5,83 +5,24 @@
 
 package io.opentelemetry.contrib.dynamic.policy;
 
-import io.opentelemetry.contrib.dynamic.policy.source.JsonSourceWrapper;
-import io.opentelemetry.contrib.dynamic.policy.source.KeyValueSourceWrapper;
 import io.opentelemetry.contrib.dynamic.policy.source.SourceWrapper;
 import javax.annotation.Nullable;
 
 public interface PolicyValidator {
-  /**
-   * Validates a policy configuration provided as a JSON string.
-   *
-   * @param json The JSON string containing the policy configuration.
-   * @return The validated {@link TelemetryPolicy}, or {@code null} if the JSON does not contain a
-   *     valid policy for this validator.
-   */
-  @Nullable
-  TelemetryPolicy validate(String json);
-
   /**
    * Validates a parsed policy configuration source.
    *
-   * <p>This is a transitional API: by default it delegates to {@link #validate(String)} and/or
-   * {@link #validateAlias(String, String)} where possible.
-   *
    * @param source parsed source wrapper containing the format and payload
    * @return The validated {@link TelemetryPolicy}, or {@code null} if the source does not contain a
    *     valid policy for this validator.
    */
   @Nullable
-  default TelemetryPolicy validate(SourceWrapper source) {
-    if (source == null) {
-      return null;
-    }
-    if (source instanceof JsonSourceWrapper) {
-      return validate(((JsonSourceWrapper) source).asJsonNode().toString());
-    }
-    if (source instanceof KeyValueSourceWrapper) {
-      KeyValueSourceWrapper kv = (KeyValueSourceWrapper) source;
-      String alias = getAlias();
-      if (alias != null && alias.equals(kv.getKey().trim())) {
-        return validateAlias(kv.getKey().trim(), kv.getValue());
-      }
-    }
-    return null;
-  }
+  TelemetryPolicy validate(SourceWrapper source);
 
   /**
    * Returns the type of the policy this validator handles.
    *
    * @return The policy type string (e.g., "trace-sampling").
    */
   String getPolicyType();
-
-  /**
-   * Validates a policy configuration provided as a key-value pair (alias).
-   *
-   * <p>This is intended for simple configuration cases where a full JSON object is not necessary.
-   *
-   * @param key The alias key (e.g., "trace-sampling.probability").
-   * @param value The value associated with the key.
-   * @return The validated {@link TelemetryPolicy}, or {@code null} if the key/value pair is invalid
-   *     or not handled by this validator.
-   */
-  @Nullable
-  default TelemetryPolicy validateAlias(String key, String value) {
-    throw new UnsupportedOperationException(
-        "Alias validation is not supported by validator "
-            + getClass().getName()
-            + " for key "
-            + key);
-  }
-
-  /**
-   * Returns the alias key supported by this validator, if any.
-   *
-   * @return The alias key string, or {@code null} if aliases are not supported.
-   */
-  @Nullable
-  default String getAlias() {
-    return null;
-  }
 }

dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/source/JsonSourceWrapper.java

@@ -14,7 +14,12 @@
 import java.util.Objects;
 import javax.annotation.Nullable;
 
-/** JSON-backed source wrapper for a single-policy object. */
+/**
+ * Source wrapper for policy payloads parsed from JSON text that matches the {@link
+ * SourceFormat#JSONKEYVALUE} shape: each policy is a JSON object with exactly one top-level key
+ * (the policy type) and one value (the payload). The on-the-wire syntax is standard JSON, not a
+ * separate encoding.
+ */
 public final class JsonSourceWrapper implements SourceWrapper {
   private static final ObjectMapper MAPPER = new ObjectMapper();
   private final JsonNode source;
@@ -25,7 +30,7 @@ public JsonSourceWrapper(JsonNode source) {
 
   @Override
   public SourceFormat getFormat() {
-    return SourceFormat.JSON;
+    return SourceFormat.JSONKEYVALUE;
   }
 
   @Override
@@ -43,11 +48,16 @@ public JsonNode asJsonNode() {
   }
 
   /**
-   * Parses JSON source into one wrapper per top-level policy object.
+   * Parses JSON text into one wrapper per policy object.
+   *
+   * <p>Input must be valid JSON whose structure matches {@link SourceFormat#JSONKEYVALUE}: either a
+   * single JSON object with exactly one top-level key/value pair, or a JSON array of such objects.
+   * An empty JSON array {@code []} yields an empty list.
    *
-   * @return an empty list if the source is an empty JSON array; a non-empty list of wrappers if the
-   *     source is a valid single-policy object or array thereof; or {@code null} if the shape is
-   *     unsupported or the source is not valid JSON.
+   * @return an empty list if the source is an empty JSON array {@code []}; a non-empty list of
+   *     wrappers if the source is a valid single-policy object or non-empty array of such objects;
+   *     or {@code null} if the text is not valid JSON or the value shape is not supported for
+   *     {@link SourceFormat#JSONKEYVALUE}.
    * @throws NullPointerException if source is null
    */
   @Nullable

dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/source/SourceFormat.java

@@ -13,7 +13,6 @@
 /** Supported source formats and their parser dispatch. */
 public enum SourceFormat {
   KEYVALUE("keyvalue", KeyValueSourceWrapper::parse),
-  JSON("json", JsonSourceWrapper::parse), // Will remove
   JSONKEYVALUE("jsonkeyvalue", JsonSourceWrapper::parse);
 
   private final String configValue;

dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/tracesampling/TraceSamplingValidator.java

@@ -5,72 +5,63 @@
 
 package io.opentelemetry.contrib.dynamic.policy.tracesampling;
 
-import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.JsonNode;
-import com.fasterxml.jackson.databind.ObjectMapper;
-import io.opentelemetry.contrib.dynamic.policy.PolicyValidator;
+import io.opentelemetry.contrib.dynamic.policy.AbstractSourcePolicyValidator;
 import io.opentelemetry.contrib.dynamic.policy.TelemetryPolicy;
 import java.util.logging.Logger;
 import javax.annotation.Nullable;
 
 /**
  * Validator for trace sampling policies.
  *
- * <p>This validator handles the "trace-sampling" policy type and supports the
- * "trace-sampling.probability" alias.
+ * <p>This validator handles the "trace-sampling" policy type.
  */
-public final class TraceSamplingValidator implements PolicyValidator {
+public final class TraceSamplingValidator extends AbstractSourcePolicyValidator {
   private static final Logger logger = Logger.getLogger(TraceSamplingValidator.class.getName());
-  private static final ObjectMapper MAPPER = new ObjectMapper();
 
   @Override
   public String getPolicyType() {
     return TraceSamplingRatePolicy.TYPE;
   }
 
   @Override
-  public String getAlias() {
-    return "trace-sampling.probability";
+  @Nullable
+  protected TelemetryPolicy validateJsonValue(JsonNode valueNode) {
+    JsonNode probabilityNode = valueNode;
+    if (valueNode.isObject()) {
+      probabilityNode = valueNode.get("probability");
+      if (probabilityNode == null) {
+        return null;
+      }
+    }
+    Double probability = parseDouble(probabilityNode);
+    if (probability == null) {
+      return null;
+    }
+    return createPolicy(probability);
   }
 
   @Override
   @Nullable
-  public TelemetryPolicy validate(String json) {
-    try {
-      JsonNode node = MAPPER.readTree(json);
-      if (node.has(getPolicyType())) {
-        JsonNode spec = node.get(getPolicyType());
-        if (spec.has("probability")) {
-          JsonNode probNode = spec.get("probability");
-          if (probNode.isNumber()) {
-            double d = probNode.asDouble();
-            if (d >= 0.0 && d <= 1.0) {
-              return new TraceSamplingRatePolicy(d);
-            }
-          }
-        }
-      }
-    } catch (JsonProcessingException e) {
-      // Not valid JSON for this validator
+  protected TelemetryPolicy validateKeyValueValue(String value) {
+    Double probability = parseDouble(value);
+    if (probability == null) {
+      return null;
     }
-    logger.info("Invalid trace-sampling JSON: " + json);
-    return null;
+    return createPolicy(probability);
   }
 
-  @Override
   @Nullable
-  public TelemetryPolicy validateAlias(String key, String value) {
-    if (getAlias() != null && getAlias().equals(key)) {
-      try {
-        double d = Double.parseDouble(value);
-        if (d >= 0.0 && d <= 1.0) {
-          return new TraceSamplingRatePolicy(d);
-        }
-      } catch (NumberFormatException e) {
-        // invalid
-      }
-      logger.info("Ignoring invalid trace-sampling.probability value: " + value);
+  private static TelemetryPolicy createPolicy(double probability) {
+    try {
+      return new TraceSamplingRatePolicy(probability);
+    } catch (IllegalArgumentException e) {
+      logger.info(
+          "Invalid trace-sampling probability '"
+              + probability
+              + "' will be ignored: "
+              + e.getMessage());
+      return null;
     }
-    return null;
   }
 }