refactor(ffe): address review — typed args, dirty-guarded tag writes, simpler UTF-8 truncation, drop dead clear()

Author: pavlokhrebto

src/api/FeatureFlags/SpanEnrichmentAccumulator.php

@@ -37,31 +37,33 @@ final class SpanEnrichmentAccumulator
     /** @var array<string, string> flagKey => stringified default value. */
     private $defaults = array();
 
+    /** @var bool Whether the state changed since the last toSpanTags() encode. */
+    private $dirty = false;
+
     /**
      * Record a serial id seen during evaluation. Deduped via a set; dropped
      * (with no error) once the frozen cap is reached.
      */
-    public function addSerialId($id)
+    public function addSerialId(int $id)
     {
-        $id = (int) $id;
         if (isset($this->serialIds[$id])) {
             return;
         }
         if (count($this->serialIds) >= self::MAX_SERIAL_IDS) {
             return;
         }
         $this->serialIds[$id] = true;
+        $this->dirty = true;
     }
 
     /**
      * Associate a serial id with a (hashed) subject. The targeting key is
      * SHA256-hashed before storage (privacy: raw targeting keys are never
      * emitted) and is only recorded when `do_log` authorizes it.
      */
-    public function addSubject($targetingKey, $id)
+    public function addSubject(string $targetingKey, int $id)
     {
-        $id = (int) $id;
-        $hashed = $this->hashTargetingKey((string) $targetingKey);
+        $hashed = $this->hashTargetingKey($targetingKey);
 
         if (isset($this->subjects[$hashed])) {
             if (isset($this->subjects[$hashed][$id])) {
@@ -71,13 +73,15 @@ public function addSubject($targetingKey, $id)
                 return;
             }
             $this->subjects[$hashed][$id] = true;
+            $this->dirty = true;
             return;
         }
 
         if (count($this->subjects) >= self::MAX_SUBJECTS) {
             return;
         }
         $this->subjects[$hashed] = array($id => true);
+        $this->dirty = true;
     }
 
     /**
@@ -88,9 +92,8 @@ public function addSubject($targetingKey, $id)
      *
      * @param mixed $value
      */
-    public function addDefault($flagKey, $value)
+    public function addDefault(string $flagKey, $value)
     {
-        $flagKey = (string) $flagKey;
         if (array_key_exists($flagKey, $this->defaults)) {
             return;
         }
@@ -99,6 +102,19 @@ public function addDefault($flagKey, $value)
         }
 
         $this->defaults[$flagKey] = $this->stringifyDefault($value);
+        $this->dirty = true;
+    }
+
+    /**
+     * Whether the accumulated state changed since the last call, consuming the
+     * flag. Lets the caller skip re-encoding and re-writing the span tags when
+     * an evaluation added nothing new.
+     */
+    public function consumeDirty(): bool
+    {
+        $wasDirty = $this->dirty;
+        $this->dirty = false;
+        return $wasDirty;
     }
 
     /**
@@ -148,17 +164,6 @@ public function toSpanTags()
         return $tags;
     }
 
-    /**
-     * Reset all accumulated state. Called after the tags are flushed onto the
-     * root span so a reused accumulator never leaks across spans/requests.
-     */
-    public function clear()
-    {
-        $this->serialIds = array();
-        $this->subjects = array();
-        $this->defaults = array();
-    }
-
     /**
      * ULEB128 delta-varint + base64 encoder (frozen).
      *
@@ -300,16 +305,15 @@ private function stringifyFloat($value)
 
     /**
      * Truncate to at most $maxLength characters without splitting a multi-byte
-     * UTF-8 sequence. Falls back to a byte-safe trim if the multibyte helpers
-     * are unavailable.
+     * UTF-8 sequence. Matches up to $maxLength code points from the start: /u
+     * treats the subject as UTF-8 and /s lets '.' span newlines. If the subject
+     * is not valid UTF-8 (preg_match returns false), falls back to a
+     * codepoint-safe byte walk.
      */
     private function truncateUtf8($value, $maxLength)
     {
-        if (function_exists('mb_substr') && function_exists('mb_strlen')) {
-            if (mb_strlen($value, 'UTF-8') <= $maxLength) {
-                return $value;
-            }
-            return mb_substr($value, 0, $maxLength, 'UTF-8');
+        if (preg_match('/\A.{0,' . (int) $maxLength . '}/su', $value, $m)) {
+            return $m[0];
         }
 
         return $this->truncateUtf8ByteFallback($value, $maxLength);

src/api/FeatureFlags/SpanEnrichmentRegistry.php

@@ -39,8 +39,7 @@ final class SpanEnrichmentRegistry
     const CONFIG_KEY = 'DD_EXPERIMENTAL_FLAGGING_PROVIDER_SPAN_ENRICHMENT_ENABLED';
 
     /**
-     * Whether the experimental span-enrichment gate is on. Process-level env
-     * config; cannot toggle mid-request.
+     * Whether the experimental span-enrichment gate is on.
      *
      * @return bool
      */
@@ -80,11 +79,12 @@ public static function record($flagKey, $details, $targetingKey)
                 ObjectKVStore::put($root, self::ACCUMULATOR_KEY, $accumulator);
             }
 
+            // getExposureData() always returns an array (EvaluationDetails types it).
             $exposure = $details->getExposureData();
-            $serialId = is_array($exposure) && array_key_exists(self::SERIAL_ID_METADATA_KEY, $exposure)
+            $serialId = array_key_exists(self::SERIAL_ID_METADATA_KEY, $exposure)
                 ? $exposure[self::SERIAL_ID_METADATA_KEY]
                 : null;
-            $doLog = is_array($exposure) && !empty($exposure[self::DO_LOG_METADATA_KEY]);
+            $doLog = !empty($exposure[self::DO_LOG_METADATA_KEY]);
 
             if ($serialId !== null) {
                 $accumulator->addSerialId((int) $serialId);
@@ -98,11 +98,13 @@ public static function record($flagKey, $details, $targetingKey)
                 }
             }
 
-            // Write (overwrite) the encoded union onto the root span's meta. Safe
-            // to do on every evaluation: toSpanTags() re-encodes the accumulator's
-            // full state, so the latest write is always the complete union.
-            foreach ($accumulator->toSpanTags() as $key => $value) {
-                $root->meta[$key] = $value;
+            // Only re-encode + rewrite the tags when this evaluation actually
+            // changed the accumulated state; a repeated/duplicate evaluation adds
+            // nothing new and the meta already holds the current union.
+            if ($accumulator->consumeDirty()) {
+                foreach ($accumulator->toSpanTags() as $key => $value) {
+                    $root->meta[$key] = $value;
+                }
             }
         } catch (\Throwable $e) {
             // Enrichment must never break flag evaluation.

tests/OpenFeature/SpanEnrichmentAccumulatorTest.php

@@ -308,17 +308,4 @@ public function testFlagsTagIsBareBase64NotJson(): void
         self::assertSame($flags, base64_encode(base64_decode($flags, true)));
     }
 
-    public function testClearResetsState(): void
-    {
-        $acc = new SpanEnrichmentAccumulator();
-        $acc->addSerialId(1);
-        $acc->addDefault('flag', 'value');
-        $acc->addSubject('user', 1);
-
-        $acc->clear();
-
-        self::assertFalse($acc->hasData());
-        self::assertSame([], $acc->toSpanTags());
-    }
-
 }