Fixed use case when label/custom key is a numeric value string (#1003)

src/ElasticApm/Impl/ExecutionSegmentContext.php

@@ -42,7 +42,7 @@
  */
 abstract class ExecutionSegmentContext extends ContextPartWrapper implements ExecutionSegmentContextInterface
 {
-    /** @var ?array<string, string|bool|int|float|null> */
+    /** @var ?array<string|bool|int|float|null> */
     public $labels = null;
 
     /** @var Logger */
@@ -57,21 +57,16 @@ protected function __construct(ExecutionSegment $owner)
     }
 
     /**
-     * @param string                                     $key
-     * @param string|bool|int|float|null                 $value
-     * @param bool                                       $enforceKeywordString
-     * @param ?array<string, string|bool|int|float|null> $map
-     * @param string                                     $dbgMapName
+     * @param string                              $key
+     * @param string|bool|int|float|null          $value
+     * @param bool                                $enforceKeywordString
+     * @param ?array<string|bool|int|float|null> &$map
+     * @param string                              $dbgMapName
      *
      * @return void
      */
-    protected function setInKeyValueMap(
-        string $key,
-        $value,
-        bool $enforceKeywordString,
-        ?array &$map,
-        string $dbgMapName
-    ): void {
+    protected function setInKeyValueMap(string $key, $value, bool $enforceKeywordString, ?array &$map, string $dbgMapName): void
+    {
         if ($this->beforeMutating()) {
             return;
         }
@@ -89,9 +84,7 @@ protected function setInKeyValueMap(
             $map = [];
         }
 
-        $map[$this->tracer()->limitString($key, $enforceKeywordString)] = is_string($value)
-            ? $this->tracer()->limitString($value, $enforceKeywordString)
-            : $value;
+        $map[$this->tracer()->limitString($key, $enforceKeywordString)] = is_string($value) ? $this->tracer()->limitString($value, $enforceKeywordString) : $value;
     }
 
     /** @inheritDoc */
@@ -125,7 +118,7 @@ public function jsonSerialize()
         // https://github.com/elastic/apm-server/blob/7.0/docs/spec/context.json#L46
         // https://github.com/elastic/apm-server/blob/7.0/docs/spec/spans/span.json#L88
         if ($this->labels !== null) {
-            SerializationUtil::addNameValueIfNotEmpty('tags', $this->labels, /* ref */ $result);
+            SerializationUtil::addNameValueIfNotEmpty('tags', (object)$this->labels, /* ref */ $result);
         }
 
         return SerializationUtil::postProcessResult($result);

src/ElasticApm/Impl/TransactionContext.php

@@ -38,7 +38,7 @@
  */
 final class TransactionContext extends ExecutionSegmentContext implements TransactionContextInterface
 {
-    /** @var ?array<string, string|bool|int|float|null> */
+    /** @var ?array<string|bool|int|float|null> */
     public $custom = null;
 
     /** @var ?TransactionContextRequest */
@@ -98,7 +98,7 @@ public function jsonSerialize()
         $result = SerializationUtil::preProcessResult(parent::jsonSerialize());
 
         if ($this->custom !== null) {
-            SerializationUtil::addNameValueIfNotEmpty('custom', $this->custom, /* ref */ $result);
+            SerializationUtil::addNameValueIfNotEmpty('custom', (object)$this->custom, /* ref */ $result);
         }
         SerializationUtil::addNameValueIfNotNull('request', $this->request, /* ref */ $result);
         SerializationUtil::addNameValueIfNotNull('user', $this->user, /* ref */ $result);

tests/ElasticApmTests/ComponentTests/WordPressAutoInstrumentationTest.php

@@ -627,7 +627,7 @@ function (AppCodeRequestParams $appCodeRequestParams) use ($testArgs): void {
             self::assertIsArray($stackTrace);
             /** @var StackTraceFrame[] $stackTrace */
             self::assertFalse($expectedSpans[$expectedSpanIndex]->stackTrace->isValueSet());
-            $expectedSpans[$expectedSpanIndex]->stackTrace->setValue(StackTraceExpectations::fromFrames($stackTrace, /* allowToBePrefixOfActual */ false));
+            $expectedSpans[$expectedSpanIndex]->stackTrace->setValue(StackTraceExpectations::fromFrames($stackTrace));
             // Jump to the index of the beginning of the batch of call of the next callback
             $expectedSpanIndex += $callsCount;
         };

tests/ElasticApmTests/TestsSharedCode/StackTraceLimitTestSharedCode.php

@@ -198,7 +198,7 @@ public static function implTestVariousConfigValuesAssertPart(?int $expectedMaxNu
             }
 
             TestCaseBase::assertNotNull($span->stackTrace);
-            TestCaseBase::assertCountAtLeast(1, $span->stackTrace);
+            TestCaseBase::assertCountableNotEmpty($span->stackTrace);
 
             $dbgCtx->pushSubScope();
             foreach (RangeUtil::generateUpTo(min($additionalStackDepth + 2, count($span->stackTrace))) as $additionalDepthCallIndex) {

tests/ElasticApmTests/UnitTests/PublicApiTest.php

@@ -27,6 +27,7 @@
 
 use Elastic\Apm\ElasticApm;
 use Elastic\Apm\Impl\Constants;
+use Elastic\Apm\Impl\TransactionContext;
 use Elastic\Apm\Impl\TransactionContextRequest;
 use ElasticApmTests\UnitTests\Util\TracerUnitTestCaseBase;
 use ElasticApmTests\Util\DataProviderForTestBuilder;
@@ -570,4 +571,18 @@ public function testTransactionSetUserContext($id, ?string $email, ?string $user
         $this->assertSame($email, $reportedTx->context->user->email);
         $this->assertSame($username, $reportedTx->context->user->username);
     }
+
+    public function testNumericStringForLabelKey(): void
+    {
+        $tx = $this->tracer->beginTransaction('test_TX_name', 'test_TX_type');
+        $tx->context()->setLabel('123', 'label_value_for_key_123');
+        $txCtx = $tx->context();
+        self::assertInstanceOf(TransactionContext::class, $txCtx);
+        $txCtx->setCustom('456', 'custom_value_for_key_456');
+        $tx->end();
+
+        $reportedTx = $this->mockEventSink->singleTransaction();
+        self::assertSame('label_value_for_key_123', self::getLabel($reportedTx, '123'));
+        self::assertSame('custom_value_for_key_456', self::getTransactionContextCustom($reportedTx, '456'));
+    }
 }

tests/ElasticApmTests/Util/Deserialization/SerializedEventSinkTrait.php

@@ -32,33 +32,86 @@
 use ElasticApmTests\Util\MetadataValidator;
 use ElasticApmTests\Util\MetricSetValidator;
 use ElasticApmTests\Util\SpanDto;
+use ElasticApmTests\Util\TestCaseBase;
 use ElasticApmTests\Util\TransactionDto;
 
 trait SerializedEventSinkTrait
 {
     /** @var bool */
     public $shouldValidateAgainstSchema = true;
 
+    /**
+     * @param mixed $input
+     *
+     * @return mixed
+     */
+    public static function convertObjectToStringKeyArrayRecursively($input)
+    {
+        AssertMessageStack::newScope(/* out */ $dbgCtx, AssertMessageStack::funcArgs());
+
+        if (is_array($input)) {
+            $dbgCtx->pushSubScope();
+            foreach ($input as $arrKey => $arrValue) {
+                $input[$arrKey] = self::convertObjectToStringKeyArrayRecursively($arrValue);
+            }
+            $dbgCtx->popSubScope();
+            return $input;
+        }
+
+        if (!is_object($input)) {
+            return $input;
+        }
+
+        $allKeysAreStrings = true;
+        $dbgCtx->pushSubScope();
+        foreach (get_object_vars($input) as $propKey => $propValue) {
+            $dbgCtx->add(['propKey' => $propKey, 'propValue' => $propValue]);
+            if (is_numeric($propKey)) {
+                $allKeysAreStrings = false;
+            }
+
+            $input->{$propKey} = self::convertObjectToStringKeyArrayRecursively($propValue);
+        }
+        $dbgCtx->popSubScope();
+
+        if (!$allKeysAreStrings) {
+            return $input;
+        }
+
+        $asArray = [];
+        $dbgCtx->pushSubScope();
+        foreach (get_object_vars($input) as $propKey => $propValue) {
+            $dbgCtx->add(['propKey' => $propKey, 'propValue' => $propValue]);
+            $asArray[$propKey] = $propValue;
+        }
+        $dbgCtx->popSubScope();
+        return $asArray;
+    }
+
     /**
      * @template T of object
      *
-     * @param string                          $serializedData
-     * @param Closure(string): void           $validateAgainstSchema
+     * @param string                   $serializedData
+     * @param Closure(string): void    $validateAgainstSchema
      * @param Closure(array<mixed>): T $deserialize
-     * @param Closure(T): void                $assertValid
+     * @param Closure(T): void         $assertValid
      *
      * @return T
      */
     private static function validateAndDeserialize(string $serializedData, Closure $validateAgainstSchema, Closure $deserialize, Closure $assertValid)
     {
         AssertMessageStack::newScope(/* out */ $dbgCtx, ['serializedData' => $serializedData]);
 
-        /** @var array<string, mixed> $decodedJson */
-        $decodedJson = JsonUtil::decode($serializedData, /* asAssocArray */ true);
+        /** @var object $decodedJson */
+        $decodedJson = JsonUtil::decode($serializedData, /* asAssocArray */ false);
         $dbgCtx->add(['decodedJson' => $decodedJson]);
 
+        $postProcessedDecodedJson = self::convertObjectToStringKeyArrayRecursively($decodedJson);
+        $dbgCtx->add(['postProcessedDecodedJson' => $postProcessedDecodedJson]);
+        TestCaseBase::assertIsArray($postProcessedDecodedJson);
+
         $validateAgainstSchema($serializedData);
-        $deserializedData = $deserialize($decodedJson);
+        $deserializedData = $deserialize($postProcessedDecodedJson);
         $dbgCtx->add(['deserializedData' => $deserializedData]);
         $assertValid($deserializedData);
         return $deserializedData;

tests/ElasticApmTests/Util/ExecutionSegmentContextDto.php

@@ -30,20 +30,22 @@ abstract class ExecutionSegmentContextDto
 {
     use AssertValidTrait;
 
-    /** @var ?array<string, string|bool|int|float|null> */
+    /** @var ?array<string|bool|int|float|null> */
     public $labels = null;
 
     /**
      * @param mixed $actual
      *
-     * @phpstan-assert array<string, string|bool|int|float|null> $actual
+     * @phpstan-assert array<string|bool|int|float|null> $actual
      */
     private static function assertValidKeyValueMap($actual, bool $shouldKeyValueStringsBeKeyword): void
     {
         $maxLength = $shouldKeyValueStringsBeKeyword ? Constants::KEYWORD_STRING_MAX_LENGTH : null;
         TestCaseBase::assertIsArray($actual);
         foreach ($actual as $key => $value) {
-            self::assertValidString($key, /* isNullable: */ false, $maxLength);
+            if (!is_int($key)) {
+                self::assertValidString($key, /* isNullable: */ false, $maxLength);
+            }
             TestCaseBase::assertTrue(ExecutionSegmentContext::doesValueHaveSupportedLabelType($value));
             if (is_string($value)) {
                 self::assertValidString($value, /* isNullable: */ false, $maxLength);
@@ -52,11 +54,11 @@ private static function assertValidKeyValueMap($actual, bool $shouldKeyValueStri
     }
 
     /**
-     * @param Optional<?array<string, string|bool|int|float|null>> $expected
-     * @param mixed                                                $actual
-     * @param bool                                                 $shouldKeyValueStringsBeKeyword
+     * @param Optional<?array<string|bool|int|float|null>> $expected
+     * @param mixed                                        $actual
+     * @param bool                                         $shouldKeyValueStringsBeKeyword
      *
-     * @phpstan-assert ?array<string, string|bool|int|float|null> $actual
+     * @phpstan-assert ?array<string|bool|int|float|null> $actual
      */
     protected static function assertKeyValueMapsMatch(Optional $expected, $actual, bool $shouldKeyValueStringsBeKeyword): void
     {
@@ -66,26 +68,26 @@ protected static function assertKeyValueMapsMatch(Optional $expected, $actual, b
         }
 
         self::assertValidKeyValueMap($actual, $shouldKeyValueStringsBeKeyword);
-        /** @var array<string, string|bool|int|float|null> $actual */
+        /** @var array<string|bool|int|float|null> $actual */
 
         if (!$expected->isValueSet()) {
             return;
         }
 
         $expectedArray = $expected->getValue();
         TestCaseBase::assertNotNull($expectedArray);
-        /** @var array<string, string|bool|int|float|null> $expectedArray */
+        /** @var array<string|bool|int|float|null> $expectedArray */
         TestCaseBase::assertSameCount($expectedArray, $actual);
         foreach ($expectedArray as $expectedKey => $expectedValue) {
             TestCaseBase::assertSameValueInArray($expectedKey, $expectedValue, $actual);
         }
     }
 
     /**
-     * @param Optional<?array<string, string|bool|int|float|null>> $expected
-     * @param mixed                                                $actual
+     * @param Optional<?array<string|bool|int|float|null>> $expected
+     * @param mixed                                        $actual
      *
-     * @phpstan-assert ?array<string, string|bool|int|float|null> $actual
+     * @phpstan-assert ?array<string|bool|int|float|null> $actual
      */
     private static function assertLabelsMatch(Optional $expected, $actual): void
     {
@@ -95,12 +97,12 @@ private static function assertLabelsMatch(Optional $expected, $actual): void
     /**
      * @param mixed $actual
      *
-     * @return ?array<string, string|bool|int|float|null>
+     * @return ?array<string|bool|int|float|null>
      */
     private static function assertValidLabels($actual): ?array
     {
         /**
-         * @var Optional<?array<string, string|bool|int|float|null>> $expected
+         * @var Optional<?array<string|bool|int|float|null>> $expected
          * @noinspection PhpRedundantVariableDocTypeInspection
          */
         $expected = new Optional();
@@ -119,13 +121,32 @@ public static function deserializeKeyValue($key, $value, self $result): bool
     {
         switch ($key) {
             case 'tags':
-                $result->labels = self::assertValidLabels($value);
+                $result->labels = self::assertValidLabels(self::deserializeApmMap($value));
                 return true;
             default:
                 return false;
         }
     }
 
+    /**
+     * @param mixed $value
+     *
+     * @return ?array<array-key, mixed>
+     */
+    protected static function deserializeApmMap($value): ?array
+    {
+        if ($value === null) {
+            return null;
+        }
+
+        if (is_array($value)) {
+            return $value;
+        }
+
+        TestCaseBase::assertIsObject($value);
+        return get_object_vars($value);
+    }
+
     protected function assertMatchesExecutionSegment(ExecutionSegmentContextExpectations $expectations): void
     {
         self::assertLabelsMatch($expectations->labels, $this->labels);

tests/ElasticApmTests/Util/ExecutionSegmentContextExpectations.php

@@ -28,7 +28,7 @@
  */
 class ExecutionSegmentContextExpectations extends ExpectationsBase
 {
-    /** @var Optional<?array<string, string|bool|int|float|null>> */
+    /** @var Optional<?array<string|bool|int|float|null>> */
     public $labels;
 
     public function __construct()

tests/ElasticApmTests/Util/MetricSetValidator.php

@@ -24,8 +24,6 @@
 namespace ElasticApmTests\Util;
 
 use Elastic\Apm\Impl\MetricSet;
-use Elastic\Apm\Impl\Util\ArrayUtil;
-use PHPUnit\Framework\TestCase;
 
 final class MetricSetValidator
 {
@@ -55,10 +53,10 @@ private function assertValidImpl(): void
         TestCaseBase::assertSameNullness($this->actual->transactionName, $this->actual->transactionType);
 
         if ($this->actual->spanSubtype !== null) {
-            TestCase::assertNotNull($this->actual->spanType);
+            TestCaseBase::assertNotNull($this->actual->spanType);
         }
         if ($this->actual->spanType !== null) {
-            TestCase::assertNotNull($this->actual->transactionType);
+            TestCaseBase::assertNotNull($this->actual->transactionType);
         }
 
         self::assertValidSamples($this->actual->samples);
@@ -76,18 +74,18 @@ public static function assertValid(MetricSet $actual, ?MetricSetExpectations $ex
      */
     public static function assertValidSamples($samples): array
     {
-        TestCase::assertTrue(is_array($samples));
+        TestCaseBase::assertIsArray($samples);
         /** @var array<mixed> $samples */
-        TestCase::assertTrue(!ArrayUtil::isEmpty($samples));
+        TestCaseBase::assertCountableNotEmpty($samples);
 
         foreach ($samples as $key => $valueArr) {
             self::assertValidKeywordString($key);
-            TestCase::assertTrue(is_array($valueArr));
+            TestCaseBase::assertTrue(is_array($valueArr));
             /** @var array<mixed> $valueArr */
-            TestCase::assertTrue(count($valueArr) === 1);
-            TestCase::assertTrue(array_key_exists('value', $valueArr));
+            TestCaseBase::assertTrue(count($valueArr) === 1);
+            TestCaseBase::assertTrue(array_key_exists('value', $valueArr));
             $value = $valueArr['value'];
-            TestCase::assertTrue(is_int($value) || is_float($value));
+            TestCaseBase::assertTrue(is_int($value) || is_float($value));
             /** @var float|int $value */
         }
         /** @var array<string, array<string, float|int>> $samples */