Skip to content

Commit f37b993

Browse files
SergeyKleymanSergeyKleyman
authored
Fixed use case when label/custom key is a numeric value string (#1003)
1 parent ab84deb commit f37b993

12 files changed

+165
-64
lines changed

‎src/ElasticApm/Impl/ExecutionSegmentContext.php‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
*/
4343
abstract class ExecutionSegmentContext extends ContextPartWrapper implements ExecutionSegmentContextInterface
4444
{
45-
/** @var ?array<string, string|bool|int|float|null> */
45+
/** @var ?array<string|bool|int|float|null> */
4646
public $labels = null;
4747

4848
/** @var Logger */
@@ -57,21 +57,16 @@ protected function __construct(ExecutionSegment $owner)
5757
}
5858

5959
/**
60-
* @param string $key
61-
* @param string|bool|int|float|null $value
62-
* @param bool $enforceKeywordString
63-
* @param ?array<string, string|bool|int|float|null> $map
64-
* @param string $dbgMapName
60+
* @param string $key
61+
* @param string|bool|int|float|null $value
62+
* @param bool $enforceKeywordString
63+
* @param ?array<string|bool|int|float|null> &$map
64+
* @param string $dbgMapName
6565
*
6666
* @return void
6767
*/
68-
protected function setInKeyValueMap(
69-
string $key,
70-
$value,
71-
bool $enforceKeywordString,
72-
?array &$map,
73-
string $dbgMapName
74-
): void {
68+
protected function setInKeyValueMap(string $key, $value, bool $enforceKeywordString, ?array &$map, string $dbgMapName): void
69+
{
7570
if ($this->beforeMutating()) {
7671
return;
7772
}
@@ -89,9 +84,7 @@ protected function setInKeyValueMap(
8984
$map = [];
9085
}
9186

92-
$map[$this->tracer()->limitString($key, $enforceKeywordString)] = is_string($value)
93-
? $this->tracer()->limitString($value, $enforceKeywordString)
94-
: $value;
87+
$map[$this->tracer()->limitString($key, $enforceKeywordString)] = is_string($value) ? $this->tracer()->limitString($value, $enforceKeywordString) : $value;
9588
}
9689

9790
/** @inheritDoc */
@@ -125,7 +118,7 @@ public function jsonSerialize()
125118
// https://github.com/elastic/apm-server/blob/7.0/docs/spec/context.json#L46
126119
// https://github.com/elastic/apm-server/blob/7.0/docs/spec/spans/span.json#L88
127120
if ($this->labels !== null) {
128-
SerializationUtil::addNameValueIfNotEmpty('tags', $this->labels, /* ref */ $result);
121+
SerializationUtil::addNameValueIfNotEmpty('tags', (object)$this->labels, /* ref */ $result);
129122
}
130123

131124
return SerializationUtil::postProcessResult($result);

‎src/ElasticApm/Impl/TransactionContext.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
*/
3939
final class TransactionContext extends ExecutionSegmentContext implements TransactionContextInterface
4040
{
41-
/** @var ?array<string, string|bool|int|float|null> */
41+
/** @var ?array<string|bool|int|float|null> */
4242
public $custom = null;
4343

4444
/** @var ?TransactionContextRequest */
@@ -98,7 +98,7 @@ public function jsonSerialize()
9898
$result = SerializationUtil::preProcessResult(parent::jsonSerialize());
9999

100100
if ($this->custom !== null) {
101-
SerializationUtil::addNameValueIfNotEmpty('custom', $this->custom, /* ref */ $result);
101+
SerializationUtil::addNameValueIfNotEmpty('custom', (object)$this->custom, /* ref */ $result);
102102
}
103103
SerializationUtil::addNameValueIfNotNull('request', $this->request, /* ref */ $result);
104104
SerializationUtil::addNameValueIfNotNull('user', $this->user, /* ref */ $result);

‎tests/ElasticApmTests/ComponentTests/WordPressAutoInstrumentationTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -627,7 +627,7 @@ function (AppCodeRequestParams $appCodeRequestParams) use ($testArgs): void {
627627
self::assertIsArray($stackTrace);
628628
/** @var StackTraceFrame[] $stackTrace */
629629
self::assertFalse($expectedSpans[$expectedSpanIndex]->stackTrace->isValueSet());
630-
$expectedSpans[$expectedSpanIndex]->stackTrace->setValue(StackTraceExpectations::fromFrames($stackTrace, /* allowToBePrefixOfActual */ false));
630+
$expectedSpans[$expectedSpanIndex]->stackTrace->setValue(StackTraceExpectations::fromFrames($stackTrace));
631631
// Jump to the index of the beginning of the batch of call of the next callback
632632
$expectedSpanIndex += $callsCount;
633633
};

‎tests/ElasticApmTests/TestsSharedCode/StackTraceLimitTestSharedCode.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ public static function implTestVariousConfigValuesAssertPart(?int $expectedMaxNu
198198
}
199199

200200
TestCaseBase::assertNotNull($span->stackTrace);
201-
TestCaseBase::assertCountAtLeast(1, $span->stackTrace);
201+
TestCaseBase::assertCountableNotEmpty($span->stackTrace);
202202

203203
$dbgCtx->pushSubScope();
204204
foreach (RangeUtil::generateUpTo(min($additionalStackDepth + 2, count($span->stackTrace))) as $additionalDepthCallIndex) {

‎tests/ElasticApmTests/UnitTests/PublicApiTest.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
use Elastic\Apm\ElasticApm;
2929
use Elastic\Apm\Impl\Constants;
30+
use Elastic\Apm\Impl\TransactionContext;
3031
use Elastic\Apm\Impl\TransactionContextRequest;
3132
use ElasticApmTests\UnitTests\Util\TracerUnitTestCaseBase;
3233
use ElasticApmTests\Util\DataProviderForTestBuilder;
@@ -570,4 +571,18 @@ public function testTransactionSetUserContext($id, ?string $email, ?string $user
570571
$this->assertSame($email, $reportedTx->context->user->email);
571572
$this->assertSame($username, $reportedTx->context->user->username);
572573
}
574+
575+
public function testNumericStringForLabelKey(): void
576+
{
577+
$tx = $this->tracer->beginTransaction('test_TX_name', 'test_TX_type');
578+
$tx->context()->setLabel('123', 'label_value_for_key_123');
579+
$txCtx = $tx->context();
580+
self::assertInstanceOf(TransactionContext::class, $txCtx);
581+
$txCtx->setCustom('456', 'custom_value_for_key_456');
582+
$tx->end();
583+
584+
$reportedTx = $this->mockEventSink->singleTransaction();
585+
self::assertSame('label_value_for_key_123', self::getLabel($reportedTx, '123'));
586+
self::assertSame('custom_value_for_key_456', self::getTransactionContextCustom($reportedTx, '456'));
587+
}
573588
}

‎tests/ElasticApmTests/Util/Deserialization/SerializedEventSinkTrait.php‎

Lines changed: 59 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,33 +32,86 @@
3232
use ElasticApmTests\Util\MetadataValidator;
3333
use ElasticApmTests\Util\MetricSetValidator;
3434
use ElasticApmTests\Util\SpanDto;
35+
use ElasticApmTests\Util\TestCaseBase;
3536
use ElasticApmTests\Util\TransactionDto;
3637

3738
trait SerializedEventSinkTrait
3839
{
3940
/** @var bool */
4041
public $shouldValidateAgainstSchema = true;
4142

43+
/**
44+
* @param mixed $input
45+
*
46+
* @return mixed
47+
*/
48+
public static function convertObjectToStringKeyArrayRecursively($input)
49+
{
50+
AssertMessageStack::newScope(/* out */ $dbgCtx, AssertMessageStack::funcArgs());
51+
52+
if (is_array($input)) {
53+
$dbgCtx->pushSubScope();
54+
foreach ($input as $arrKey => $arrValue) {
55+
$input[$arrKey] = self::convertObjectToStringKeyArrayRecursively($arrValue);
56+
}
57+
$dbgCtx->popSubScope();
58+
return $input;
59+
}
60+
61+
if (!is_object($input)) {
62+
return $input;
63+
}
64+
65+
$allKeysAreStrings = true;
66+
$dbgCtx->pushSubScope();
67+
foreach (get_object_vars($input) as $propKey => $propValue) {
68+
$dbgCtx->add(['propKey' => $propKey, 'propValue' => $propValue]);
69+
if (is_numeric($propKey)) {
70+
$allKeysAreStrings = false;
71+
}
72+
73+
$input->{$propKey} = self::convertObjectToStringKeyArrayRecursively($propValue);
74+
}
75+
$dbgCtx->popSubScope();
76+
77+
if (!$allKeysAreStrings) {
78+
return $input;
79+
}
80+
81+
$asArray = [];
82+
$dbgCtx->pushSubScope();
83+
foreach (get_object_vars($input) as $propKey => $propValue) {
84+
$dbgCtx->add(['propKey' => $propKey, 'propValue' => $propValue]);
85+
$asArray[$propKey] = $propValue;
86+
}
87+
$dbgCtx->popSubScope();
88+
return $asArray;
89+
}
90+
4291
/**
4392
* @template T of object
4493
*
45-
* @param string $serializedData
46-
* @param Closure(string): void $validateAgainstSchema
94+
* @param string $serializedData
95+
* @param Closure(string): void $validateAgainstSchema
4796
* @param Closure(array<mixed>): T $deserialize
48-
* @param Closure(T): void $assertValid
97+
* @param Closure(T): void $assertValid
4998
*
5099
* @return T
51100
*/
52101
private static function validateAndDeserialize(string $serializedData, Closure $validateAgainstSchema, Closure $deserialize, Closure $assertValid)
53102
{
54103
AssertMessageStack::newScope(/* out */ $dbgCtx, ['serializedData' => $serializedData]);
55104

56-
/** @var array<string, mixed> $decodedJson */
57-
$decodedJson = JsonUtil::decode($serializedData, /* asAssocArray */ true);
105+
/** @var object $decodedJson */
106+
$decodedJson = JsonUtil::decode($serializedData, /* asAssocArray */ false);
58107
$dbgCtx->add(['decodedJson' => $decodedJson]);
59108

109+
$postProcessedDecodedJson = self::convertObjectToStringKeyArrayRecursively($decodedJson);
110+
$dbgCtx->add(['postProcessedDecodedJson' => $postProcessedDecodedJson]);
111+
TestCaseBase::assertIsArray($postProcessedDecodedJson);
112+
60113
$validateAgainstSchema($serializedData);
61-
$deserializedData = $deserialize($decodedJson);
114+
$deserializedData = $deserialize($postProcessedDecodedJson);
62115
$dbgCtx->add(['deserializedData' => $deserializedData]);
63116
$assertValid($deserializedData);
64117
return $deserializedData;

‎tests/ElasticApmTests/Util/ExecutionSegmentContextDto.php‎

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,20 +30,22 @@ abstract class ExecutionSegmentContextDto
3030
{
3131
use AssertValidTrait;
3232

33-
/** @var ?array<string, string|bool|int|float|null> */
33+
/** @var ?array<string|bool|int|float|null> */
3434
public $labels = null;
3535

3636
/**
3737
* @param mixed $actual
3838
*
39-
* @phpstan-assert array<string, string|bool|int|float|null> $actual
39+
* @phpstan-assert array<string|bool|int|float|null> $actual
4040
*/
4141
private static function assertValidKeyValueMap($actual, bool $shouldKeyValueStringsBeKeyword): void
4242
{
4343
$maxLength = $shouldKeyValueStringsBeKeyword ? Constants::KEYWORD_STRING_MAX_LENGTH : null;
4444
TestCaseBase::assertIsArray($actual);
4545
foreach ($actual as $key => $value) {
46-
self::assertValidString($key, /* isNullable: */ false, $maxLength);
46+
if (!is_int($key)) {
47+
self::assertValidString($key, /* isNullable: */ false, $maxLength);
48+
}
4749
TestCaseBase::assertTrue(ExecutionSegmentContext::doesValueHaveSupportedLabelType($value));
4850
if (is_string($value)) {
4951
self::assertValidString($value, /* isNullable: */ false, $maxLength);
@@ -52,11 +54,11 @@ private static function assertValidKeyValueMap($actual, bool $shouldKeyValueStri
5254
}
5355

5456
/**
55-
* @param Optional<?array<string, string|bool|int|float|null>> $expected
56-
* @param mixed $actual
57-
* @param bool $shouldKeyValueStringsBeKeyword
57+
* @param Optional<?array<string|bool|int|float|null>> $expected
58+
* @param mixed $actual
59+
* @param bool $shouldKeyValueStringsBeKeyword
5860
*
59-
* @phpstan-assert ?array<string, string|bool|int|float|null> $actual
61+
* @phpstan-assert ?array<string|bool|int|float|null> $actual
6062
*/
6163
protected static function assertKeyValueMapsMatch(Optional $expected, $actual, bool $shouldKeyValueStringsBeKeyword): void
6264
{
@@ -66,26 +68,26 @@ protected static function assertKeyValueMapsMatch(Optional $expected, $actual, b
6668
}
6769

6870
self::assertValidKeyValueMap($actual, $shouldKeyValueStringsBeKeyword);
69-
/** @var array<string, string|bool|int|float|null> $actual */
71+
/** @var array<string|bool|int|float|null> $actual */
7072

7173
if (!$expected->isValueSet()) {
7274
return;
7375
}
7476

7577
$expectedArray = $expected->getValue();
7678
TestCaseBase::assertNotNull($expectedArray);
77-
/** @var array<string, string|bool|int|float|null> $expectedArray */
79+
/** @var array<string|bool|int|float|null> $expectedArray */
7880
TestCaseBase::assertSameCount($expectedArray, $actual);
7981
foreach ($expectedArray as $expectedKey => $expectedValue) {
8082
TestCaseBase::assertSameValueInArray($expectedKey, $expectedValue, $actual);
8183
}
8284
}
8385

8486
/**
85-
* @param Optional<?array<string, string|bool|int|float|null>> $expected
86-
* @param mixed $actual
87+
* @param Optional<?array<string|bool|int|float|null>> $expected
88+
* @param mixed $actual
8789
*
88-
* @phpstan-assert ?array<string, string|bool|int|float|null> $actual
90+
* @phpstan-assert ?array<string|bool|int|float|null> $actual
8991
*/
9092
private static function assertLabelsMatch(Optional $expected, $actual): void
9193
{
@@ -95,12 +97,12 @@ private static function assertLabelsMatch(Optional $expected, $actual): void
9597
/**
9698
* @param mixed $actual
9799
*
98-
* @return ?array<string, string|bool|int|float|null>
100+
* @return ?array<string|bool|int|float|null>
99101
*/
100102
private static function assertValidLabels($actual): ?array
101103
{
102104
/**
103-
* @var Optional<?array<string, string|bool|int|float|null>> $expected
105+
* @var Optional<?array<string|bool|int|float|null>> $expected
104106
* @noinspection PhpRedundantVariableDocTypeInspection
105107
*/
106108
$expected = new Optional();
@@ -119,13 +121,32 @@ public static function deserializeKeyValue($key, $value, self $result): bool
119121
{
120122
switch ($key) {
121123
case 'tags':
122-
$result->labels = self::assertValidLabels($value);
124+
$result->labels = self::assertValidLabels(self::deserializeApmMap($value));
123125
return true;
124126
default:
125127
return false;
126128
}
127129
}
128130

131+
/**
132+
* @param mixed $value
133+
*
134+
* @return ?array<array-key, mixed>
135+
*/
136+
protected static function deserializeApmMap($value): ?array
137+
{
138+
if ($value === null) {
139+
return null;
140+
}
141+
142+
if (is_array($value)) {
143+
return $value;
144+
}
145+
146+
TestCaseBase::assertIsObject($value);
147+
return get_object_vars($value);
148+
}
149+
129150
protected function assertMatchesExecutionSegment(ExecutionSegmentContextExpectations $expectations): void
130151
{
131152
self::assertLabelsMatch($expectations->labels, $this->labels);

‎tests/ElasticApmTests/Util/ExecutionSegmentContextExpectations.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
*/
2929
class ExecutionSegmentContextExpectations extends ExpectationsBase
3030
{
31-
/** @var Optional<?array<string, string|bool|int|float|null>> */
31+
/** @var Optional<?array<string|bool|int|float|null>> */
3232
public $labels;
3333

3434
public function __construct()

‎tests/ElasticApmTests/Util/MetricSetValidator.php‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@
2424
namespace ElasticApmTests\Util;
2525

2626
use Elastic\Apm\Impl\MetricSet;
27-
use Elastic\Apm\Impl\Util\ArrayUtil;
28-
use PHPUnit\Framework\TestCase;
2927

3028
final class MetricSetValidator
3129
{
@@ -55,10 +53,10 @@ private function assertValidImpl(): void
5553
TestCaseBase::assertSameNullness($this->actual->transactionName, $this->actual->transactionType);
5654

5755
if ($this->actual->spanSubtype !== null) {
58-
TestCase::assertNotNull($this->actual->spanType);
56+
TestCaseBase::assertNotNull($this->actual->spanType);
5957
}
6058
if ($this->actual->spanType !== null) {
61-
TestCase::assertNotNull($this->actual->transactionType);
59+
TestCaseBase::assertNotNull($this->actual->transactionType);
6260
}
6361

6462
self::assertValidSamples($this->actual->samples);
@@ -76,18 +74,18 @@ public static function assertValid(MetricSet $actual, ?MetricSetExpectations $ex
7674
*/
7775
public static function assertValidSamples($samples): array
7876
{
79-
TestCase::assertTrue(is_array($samples));
77+
TestCaseBase::assertIsArray($samples);
8078
/** @var array<mixed> $samples */
81-
TestCase::assertTrue(!ArrayUtil::isEmpty($samples));
79+
TestCaseBase::assertCountableNotEmpty($samples);
8280

8381
foreach ($samples as $key => $valueArr) {
8482
self::assertValidKeywordString($key);
85-
TestCase::assertTrue(is_array($valueArr));
83+
TestCaseBase::assertTrue(is_array($valueArr));
8684
/** @var array<mixed> $valueArr */
87-
TestCase::assertTrue(count($valueArr) === 1);
88-
TestCase::assertTrue(array_key_exists('value', $valueArr));
85+
TestCaseBase::assertTrue(count($valueArr) === 1);
86+
TestCaseBase::assertTrue(array_key_exists('value', $valueArr));
8987
$value = $valueArr['value'];
90-
TestCase::assertTrue(is_int($value) || is_float($value));
88+
TestCaseBase::assertTrue(is_int($value) || is_float($value));
9189
/** @var float|int $value */
9290
}
9391
/** @var array<string, array<string, float|int>> $samples */

0 commit comments

Comments
 (0)