Parse features on UFC splits

products/feature-flagging/feature-flagging-bootstrap/build.gradle.kts

@@ -18,6 +18,7 @@ extra["excludedClassesCoverage"] = listOf(
   "datadog.trace.api.featureflag.ufc.v1.ConditionConfiguration",
   "datadog.trace.api.featureflag.ufc.v1.ConditionOperator",
   "datadog.trace.api.featureflag.ufc.v1.Environment",
+  "datadog.trace.api.featureflag.ufc.v1.Feature",
   "datadog.trace.api.featureflag.ufc.v1.Flag",
   "datadog.trace.api.featureflag.ufc.v1.Rule",
   "datadog.trace.api.featureflag.ufc.v1.ServerConfiguration",

products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Feature.java

@@ -0,0 +1,19 @@
+package datadog.trace.api.featureflag.ufc.v1;
+
+import java.util.List;
+
+/** Key-value data that the UFC attaches to a split, with where SDKs deliver it. */
+public class Feature {
+  public final String key;
+  // A String, Double or Boolean; the parser drops features with any other value.
+  public final Object value;
+  // Open strings such as HOOK, EXPOSURE or EVALUATION. Never empty: the parser drops a feature
+  // without destinations.
+  public final List<String> destinations;
+
+  public Feature(final String key, final Object value, final List<String> destinations) {
+    this.key = key;
+    this.value = value;
+    this.destinations = destinations;
+  }
+}

products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Split.java

@@ -11,15 +11,28 @@ public class Split {
   // Moshi reflective deserialization from the UFC "serialId" JSON field. Surfaced as
   // __dd_split_serial_id in eval metadata for APM span enrichment.
   public final Integer serialId;
+  // Null when the UFC split has no features. A provider must check that this field exists before
+  // reading it, because an older agent ships a Split without it.
+  public final List<Feature> features;
 
   public Split(
       final List<Shard> shards,
       final String variationKey,
       final Map<String, String> extraLogging,
       final Integer serialId) {
+    this(shards, variationKey, extraLogging, serialId, null);
+  }
+
+  public Split(
+      final List<Shard> shards,
+      final String variationKey,
+      final Map<String, String> extraLogging,
+      final Integer serialId,
+      final List<Feature> features) {
     this.shards = shards;
     this.variationKey = variationKey;
     this.extraLogging = extraLogging;
     this.serialId = serialId;
+    this.features = features;
   }
 }

products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/UniversalFlagConfigParser.java

@@ -9,6 +9,7 @@
 import datadog.remoteconfig.ConfigurationDeserializer;
 import datadog.trace.api.featureflag.ufc.v1.Allocation;
 import datadog.trace.api.featureflag.ufc.v1.ConditionConfiguration;
+import datadog.trace.api.featureflag.ufc.v1.Feature;
 import datadog.trace.api.featureflag.ufc.v1.Flag;
 import datadog.trace.api.featureflag.ufc.v1.ParsedSemver;
 import datadog.trace.api.featureflag.ufc.v1.Rule;
@@ -23,6 +24,7 @@
 import java.time.Instant;
 import java.time.format.DateTimeFormatter;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -59,6 +61,7 @@ final class UniversalFlagConfigParser implements ConfigurationDeserializer<Serve
           .add(AllocationAdapter.FACTORY)
           .add(FlagMapAdapter.FACTORY)
           .add(LenientBooleanAdapter.FACTORY)
+          .add(FeatureListAdapter.FACTORY)
           .build();
   private static final JsonAdapter<ServerConfiguration> V1_ADAPTER =
       MOSHI.adapter(ServerConfiguration.class);
@@ -438,6 +441,121 @@ public void toJson(@Nonnull final JsonWriter writer, @Nullable final Boolean val
     }
   }
 
+  /**
+   * Reads a split's features. Features only feed exposure hooks, so a malformed list or entry is
+   * dropped instead of rejecting the flag: it must never change which variation a subject gets.
+   */
+  static final class FeatureListAdapter extends JsonAdapter<List<Feature>> {
+
+    private static final Type FEATURES_TYPE = Types.newParameterizedType(List.class, Feature.class);
+
+    static final Factory FACTORY =
+        new Factory() {
+          @Nullable
+          @Override
+          public JsonAdapter<?> create(
+              @Nonnull final Type type,
+              @Nonnull final Set<? extends Annotation> annotations,
+              @Nonnull final Moshi moshi) {
+            if (!annotations.isEmpty() || !Types.equals(type, FEATURES_TYPE)) {
+              return null;
+            }
+            return new FeatureListAdapter();
+          }
+        };
+
+    @Nullable
+    @Override
+    public List<Feature> fromJson(@Nonnull final JsonReader reader) throws IOException {
+      if (reader.peek() != JsonReader.Token.BEGIN_ARRAY) {
+        reader.skipValue();
+        return null;
+      }
+      final List<Feature> features = new ArrayList<>();
+      reader.beginArray();
+      while (reader.hasNext()) {
+        final Feature feature = readFeature(reader);
+        if (feature != null) {
+          features.add(feature);
+        }
+      }
+      reader.endArray();
+      return Collections.unmodifiableList(features);
+    }
+
+    @Nullable
+    private static Feature readFeature(final JsonReader reader) throws IOException {
+      if (reader.peek() != JsonReader.Token.BEGIN_OBJECT) {
+        reader.skipValue();
+        return null;
+      }
+      String key = null;
+      Object value = null;
+      List<String> destinations = Collections.emptyList();
+      reader.beginObject();
+      while (reader.hasNext()) {
+        final String name = reader.nextName();
+        if ("key".equals(name) && reader.peek() == JsonReader.Token.STRING) {
+          key = reader.nextString();
+        } else if ("value".equals(name)) {
+          value = readScalar(reader);
+        } else if ("destinations".equals(name)) {
+          destinations = readDestinations(reader);
+        } else {
+          reader.skipValue();
+        }
+      }
+      reader.endObject();
+      if (key == null || key.isEmpty() || value == null || destinations.isEmpty()) {
+        return null;
+      }
+      return new Feature(key, value, destinations);
+    }
+
+    /** Keeps every non-empty string, including destinations this SDK does not know yet. */
+    private static List<String> readDestinations(final JsonReader reader) throws IOException {
+      if (reader.peek() != JsonReader.Token.BEGIN_ARRAY) {
+        reader.skipValue();
+        return Collections.emptyList();
+      }
+      final List<String> destinations = new ArrayList<>();
+      reader.beginArray();
+      while (reader.hasNext()) {
+        if (reader.peek() == JsonReader.Token.STRING) {
+          final String destination = reader.nextString();
+          if (!destination.isEmpty()) {
+            destinations.add(destination);
+          }
+        } else {
+          reader.skipValue();
+        }
+      }
+      reader.endArray();
+      return Collections.unmodifiableList(destinations);
+    }
+
+    @Nullable
+    private static Object readScalar(final JsonReader reader) throws IOException {
+      switch (reader.peek()) {
+        case STRING:
+          return reader.nextString();
+        case NUMBER:
+          return reader.nextDouble();
+        case BOOLEAN:
+          return reader.nextBoolean();
+        default:
+          reader.skipValue();
+          return null;
+      }
+    }
+
+    @Override
+    public void toJson(@Nonnull final JsonWriter writer, @Nullable final List<Feature> value)
+        throws IOException {
+      throw new UnsupportedOperationException("Reading only adapter");
+    }
+  }
+
   static final class InstantAdapter extends JsonAdapter<Instant> {
 
     @Nullable

products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java

@@ -1,6 +1,7 @@
 package com.datadog.featureflag;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.Collections.emptyList;
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.emptySet;
 import static java.util.Collections.singleton;
@@ -30,14 +31,17 @@
 import datadog.trace.api.Config;
 import datadog.trace.api.featureflag.FeatureFlaggingGateway;
 import datadog.trace.api.featureflag.ufc.v1.Allocation;
+import datadog.trace.api.featureflag.ufc.v1.Feature;
 import datadog.trace.api.featureflag.ufc.v1.Flag;
 import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration;
 import java.io.IOException;
 import java.io.InputStream;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Type;
 import java.time.Instant;
+import java.util.Arrays;
 import java.util.Date;
+import java.util.List;
 import java.util.Map;
 import okio.Buffer;
 import org.junit.jupiter.api.AfterEach;
@@ -154,6 +158,110 @@ void coercesSerialIdWithoutRejectingTheFlag(final String wireValue, final int ex
     assertEquals(Integer.valueOf(expected), serialIdOf(config));
   }
 
+  @Test
+  void parsesSplitFeatures() throws Exception {
+    final ServerConfiguration config =
+        deserialize(
+            configWithFeatures(
+                "[{\"key\": \"holdout.key\", \"value\": \"q4-global\", \"destinations\": [\"HOOK\"]},"
+                    + " {\"key\": \"holdout.weight\", \"value\": 0.5,"
+                    + " \"destinations\": [\"EXPOSURE\", \"EVALUATION\"]},"
+                    + " {\"key\": \"holdout.should_include_in_holdout_analysis\", \"value\": true,"
+                    + " \"destinations\": [\"HOOK\", \"SOME_FUTURE_DESTINATION\"]}]"));
+
+    final List<Feature> features = featuresOf(config);
+    assertEquals(3, features.size());
+    assertFeature(features.get(0), "holdout.key", "q4-global", "HOOK");
+    assertFeature(features.get(1), "holdout.weight", 0.5, "EXPOSURE", "EVALUATION");
+    assertFeature(
+        features.get(2),
+        "holdout.should_include_in_holdout_analysis",
+        true,
+        "HOOK",
+        "SOME_FUTURE_DESTINATION");
+    assertEquals(Integer.valueOf(7), serialIdOf(config));
+  }
+
+  @Test
+  void parsesAbsentSplitFeaturesAsNull() throws Exception {
+    final ServerConfiguration config = deserialize(configWithFeatures(null));
+
+    assertNull(featuresOf(config));
+    assertEquals(Integer.valueOf(7), serialIdOf(config));
+  }
+
+  @Test
+  void dropsMalformedSplitFeaturesAndKeepsTheFlag() throws Exception {
+    final String hook = ", \"destinations\": [\"HOOK\"]";
+    final ServerConfiguration config =
+        deserialize(
+            configWithFeatures(
+                "[{\"key\": \"object-value\", \"value\": {\"nested\": true}"
+                    + hook
+                    + "},"
+                    + " {\"key\": \"array-value\", \"value\": [1]"
+                    + hook
+                    + "},"
+                    + " {\"key\": \"null-value\", \"value\": null"
+                    + hook
+                    + "},"
+                    + " {\"key\": \"\", \"value\": \"empty key\""
+                    + hook
+                    + "},"
+                    + " {\"key\": 3, \"value\": \"numeric key\""
+                    + hook
+                    + "},"
+                    + " {\"value\": \"missing key\""
+                    + hook
+                    + "},"
+                    + " {\"key\": \"missing value\""
+                    + hook
+                    + "},"
+                    + " {\"key\": \"no-destinations\", \"value\": \"dropped\"},"
+                    + " {\"key\": \"empty-destinations\", \"value\": \"dropped\", \"destinations\": []},"
+                    + " {\"key\": \"destinations-not-a-list\", \"value\": \"dropped\","
+                    + " \"destinations\": \"HOOK\"},"
+                    + " {\"key\": \"only-invalid-destinations\", \"value\": \"dropped\","
+                    + " \"destinations\": [1, \"\", null, {\"name\": \"HOOK\"}]},"
+                    + " \"not-an-object\","
+                    + " {\"key\": \"holdout.key\", \"value\": \"q4-global\","
+                    + " \"destinations\": [1, \"HOOK\", \"\"], \"destination\": \"HOOK\"}]"));
+
+    assertTrue(config.flags.containsKey("valid-flag"));
+    final List<Feature> features = featuresOf(config);
+    assertEquals(1, features.size());
+    assertFeature(features.get(0), "holdout.key", "q4-global", "HOOK");
+  }
+
+  @Test
+  void ignoresSplitFeaturesThatAreNotAList() throws Exception {
+    final ServerConfiguration config =
+        deserialize(configWithFeatures("{\"holdout.key\": \"q4-global\"}"));
+
+    assertTrue(config.flags.containsKey("valid-flag"));
+    assertNull(featuresOf(config));
+    assertEquals(Integer.valueOf(7), serialIdOf(config));
+  }
+
+  private static void assertFeature(
+      final Feature feature, final String key, final Object value, final String... destinations) {
+    assertEquals(key, feature.key);
+    assertEquals(value, feature.value);
+    assertEquals(Arrays.asList(destinations), feature.destinations);
+  }
+
+  private static List<Feature> featuresOf(final ServerConfiguration config) {
+    return config.flags.get("valid-flag").allocations.get(0).splits.get(0).features;
+  }
+
+  /** Inserts the raw features JSON; null omits the key. */
+  private static String configWithFeatures(final String featuresJson) throws IOException {
+    final String json = resource("split-features.json");
+    return featuresJson == null
+        ? json.replace("\"features\": \"${features}\",", "")
+        : json.replace("\"${features}\"", featuresJson);
+  }
+
   private static Integer serialIdOf(final ServerConfiguration config) {
     return config.flags.get("valid-flag").allocations.get(0).splits.get(0).serialId;
   }
@@ -261,6 +369,35 @@ void lenientBooleanAdapterIsReadOnly() {
         UnsupportedOperationException.class, () -> adapter.toJson(mock(JsonWriter.class), true));
   }
 
+  @Test
+  void featureListAdapterFactoryOnlyCreatesAdapterForUnannotatedFeatureList() {
+    final Moshi moshi = moshi();
+    final Type featuresType = Types.newParameterizedType(List.class, Feature.class);
+
+    final JsonAdapter<?> adapter =
+        UniversalFlagConfigParser.FeatureListAdapter.FACTORY.create(
+            featuresType, emptySet(), moshi);
+
+    assertNotNull(adapter);
+    assertTrue(adapter instanceof UniversalFlagConfigParser.FeatureListAdapter);
+    assertNull(
+        UniversalFlagConfigParser.FeatureListAdapter.FACTORY.create(
+            Types.newParameterizedType(List.class, String.class), emptySet(), moshi));
+    assertNull(
+        UniversalFlagConfigParser.FeatureListAdapter.FACTORY.create(
+            featuresType, singleton(mock(Annotation.class)), moshi));
+  }
+
+  @Test
+  void featureListAdapterIsReadOnly() {
+    final UniversalFlagConfigParser.FeatureListAdapter adapter =
+        new UniversalFlagConfigParser.FeatureListAdapter();
+
+    assertThrows(
+        UnsupportedOperationException.class,
+        () -> adapter.toJson(mock(JsonWriter.class), emptyList()));
+  }
+
   @Test
   void allowsNullFlagMap() throws Exception {
     final ServerConfiguration config = deserialize(resource("null-flags.json"));