From 1333473a09995ad7ed982462a786267b27e99aa8 Mon Sep 17 00:00:00 2001 From: Sergey Kleyman Date: Mon, 15 Jan 2024 14:07:55 +0200 Subject: [PATCH 1/2] Workaround potential "malformed UTF-8 characters" exception --- agent/php/ElasticApm/Impl/Util/JsonUtil.php | 2 +- tests/ElasticApmTests/UnitTests/PublicApiTest.php | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/agent/php/ElasticApm/Impl/Util/JsonUtil.php b/agent/php/ElasticApm/Impl/Util/JsonUtil.php index 9c5c79ddd..2f42bff10 100644 --- a/agent/php/ElasticApm/Impl/Util/JsonUtil.php +++ b/agent/php/ElasticApm/Impl/Util/JsonUtil.php @@ -42,7 +42,7 @@ final class JsonUtil */ public static function encode($data, bool $prettyPrint = false): string { - $options = 0; + $options = JSON_INVALID_UTF8_SUBSTITUTE; $options |= $prettyPrint ? JSON_PRETTY_PRINT : 0; $encodedData = json_encode($data, $options); if ($encodedData === false) { diff --git a/tests/ElasticApmTests/UnitTests/PublicApiTest.php b/tests/ElasticApmTests/UnitTests/PublicApiTest.php index 69100b21f..e3dc07933 100644 --- a/tests/ElasticApmTests/UnitTests/PublicApiTest.php +++ b/tests/ElasticApmTests/UnitTests/PublicApiTest.php @@ -585,4 +585,19 @@ public function testNumericStringForLabelKey(): void self::assertSame('label_value_for_key_123', self::getLabel($reportedTx, '123')); self::assertSame('custom_value_for_key_456', self::getTransactionContextCustom($reportedTx, '456')); } + + private const INVALID_UTF8_CHARACTER = "\xb0"; + private const UNICODE_REPLACEMENT_CHARACTER_JSON_ENCODED = '\ufffd'; + + public function testMalformedUtf8(): void + { + // Act + $tx = $this->tracer->beginTransaction('test_TX_name_[' . self::INVALID_UTF8_CHARACTER . ']', 'test_TX_type'); + $tx->end(); + + // Assert + $reportedTx = $this->mockEventSink->singleTransaction(); + $this->assertSame('test_TX_name_[' . json_decode('"' . self::UNICODE_REPLACEMENT_CHARACTER_JSON_ENCODED . '"') . ']', $reportedTx->name); + $this->assertSame('test_TX_type', $reportedTx->type); + } } From 58ccc7cde5c8d791284f48f071f4e7fd60f59eaf Mon Sep 17 00:00:00 2001 From: Sergey Kleyman Date: Mon, 15 Jan 2024 14:10:16 +0200 Subject: [PATCH 2/2] Fixed unit tests --- agent/php/ElasticApm/Impl/Error.php | 16 +++---- agent/php/ElasticApm/Impl/Span.php | 12 ++--- agent/php/ElasticApm/Impl/Transaction.php | 10 ++--- .../UnitTests/Util/MockEventSink.php | 5 --- .../ElasticApmTests/Util/AssertValidTrait.php | 44 ------------------- tests/ElasticApmTests/Util/ErrorDto.php | 6 --- tests/ElasticApmTests/Util/SpanDto.php | 6 --- tests/ElasticApmTests/Util/TransactionDto.php | 6 --- 8 files changed, 19 insertions(+), 86 deletions(-) diff --git a/agent/php/ElasticApm/Impl/Error.php b/agent/php/ElasticApm/Impl/Error.php index b959eb4e5..5addbe0dc 100644 --- a/agent/php/ElasticApm/Impl/Error.php +++ b/agent/php/ElasticApm/Impl/Error.php @@ -48,7 +48,7 @@ class Error implements SerializableDataInterface, LoggableInterface * * @link https://github.com/elastic/apm-server/blob/7.0/docs/spec/errors/error.json#L6 */ - public $timestamp; + private $timestamp; /** * @var string @@ -71,7 +71,7 @@ class Error implements SerializableDataInterface, LoggableInterface * * @link https://github.com/elastic/apm-server/blob/7.0/docs/spec/errors/error.json#L14 */ - public $traceId = null; + private $traceId = null; /** * @var ?string @@ -83,7 +83,7 @@ class Error implements SerializableDataInterface, LoggableInterface * * @link https://github.com/elastic/apm-server/blob/7.0/docs/spec/errors/error.json#L19 */ - public $transactionId = null; + private $transactionId = null; /** * @var ?string @@ -95,7 +95,7 @@ class Error implements SerializableDataInterface, LoggableInterface * * @link https://github.com/elastic/apm-server/blob/7.0/docs/spec/errors/error.json#L24 */ - public $parentId = null; + private $parentId = null; /** * @var ?ErrorTransactionData @@ -104,14 +104,14 @@ class Error implements SerializableDataInterface, LoggableInterface * * @link https://github.com/elastic/apm-server/blob/7.0/docs/spec/errors/error.json#L29 */ - public $transaction = null; + private $transaction = null; /** * @var ?TransactionContext * * @link https://github.com/elastic/apm-server/blob/7.0/docs/spec/errors/error.json#L44 */ - public $context = null; + private $context = null; /** * @var ?string @@ -122,7 +122,7 @@ class Error implements SerializableDataInterface, LoggableInterface * * @link https://github.com/elastic/apm-server/blob/v7.0.0/docs/spec/errors/error.json#L47 */ - public $culprit = null; + private $culprit = null; /** * @var ?ErrorExceptionData @@ -131,7 +131,7 @@ class Error implements SerializableDataInterface, LoggableInterface * * @link https://github.com/elastic/apm-server/blob/7.0/docs/spec/errors/error.json#L29 */ - public $exception = null; + private $exception = null; public static function build(Tracer $tracer, ErrorExceptionData $errorExceptionData, ?Transaction $transaction, ?Span $span): Error { diff --git a/agent/php/ElasticApm/Impl/Span.php b/agent/php/ElasticApm/Impl/Span.php index d0ea79d75..67d08ce11 100644 --- a/agent/php/ElasticApm/Impl/Span.php +++ b/agent/php/ElasticApm/Impl/Span.php @@ -41,22 +41,22 @@ final class Span extends ExecutionSegment implements SpanInterface, SpanToSendInterface { /** @var string */ - public $parentId; + private $parentId; /** @var string */ - public $transactionId; + private $transactionId; /** @var ?string */ - public $action = null; + private $action = null; /** @var ?string */ - public $subtype = null; + private $subtype = null; /** @var null|StackTraceFrame[] */ - public $stackTrace = null; + private $stackTrace = null; /** @var ?SpanContext */ - public $context = null; + private $context = null; /** @var Logger */ private $logger; diff --git a/agent/php/ElasticApm/Impl/Transaction.php b/agent/php/ElasticApm/Impl/Transaction.php index 2f30760d1..a1cc57426 100644 --- a/agent/php/ElasticApm/Impl/Transaction.php +++ b/agent/php/ElasticApm/Impl/Transaction.php @@ -49,22 +49,22 @@ final class Transaction extends ExecutionSegment implements TransactionInterface { /** @var ?string */ - public $parentId = null; + private $parentId = null; /** @var int */ public $startedSpansCount = 0; /** @var int */ - public $droppedSpansCount = 0; + private $droppedSpansCount = 0; /** @var ?string */ - public $result = null; + private $result = null; /** @var bool */ - public $isSampled; + private $isSampled; /** @var ?TransactionContext */ - public $context = null; + private $context = null; /** @var Tracer */ private $tracer; diff --git a/tests/ElasticApmTests/UnitTests/Util/MockEventSink.php b/tests/ElasticApmTests/UnitTests/Util/MockEventSink.php index 1d6958425..c04990f09 100644 --- a/tests/ElasticApmTests/UnitTests/Util/MockEventSink.php +++ b/tests/ElasticApmTests/UnitTests/Util/MockEventSink.php @@ -104,7 +104,6 @@ private function consumeMetadata(Metadata $original): void $deserialized = $this->validateAndDeserializeMetadata($serialized); self::assertValidMetadata($deserialized); - TestCaseBase::assertEqualsEx($original, $deserialized); $this->dataFromAgent->metadatas[] = $deserialized; } @@ -116,7 +115,6 @@ private function consumeTransaction(Transaction $original): void $serialized = SerializationUtil::serializeAsJson($original); $deserialized = $this->validateAndDeserializeTransaction($serialized); $deserialized->assertValid(); - $deserialized->assertEquals($original); TestCaseBase::assertNull($this->dataFromAgent->executionSegmentByIdOrNull($deserialized->id)); ArrayUtilForTests::addUnique($deserialized->id, $deserialized, /* ref */ $this->dataFromAgent->idToTransaction); @@ -135,7 +133,6 @@ private function consumeSpan(SpanToSendInterface $original): void $deserialized = $this->validateAndDeserializeSpan($serialized); $dbgCtx->add(['deserialized' => $deserialized]); $deserialized->assertValid(); - $deserialized->assertEquals($original); TestCaseBase::assertNull($this->dataFromAgent->executionSegmentByIdOrNull($deserialized->id)); ArrayUtilForTests::addUnique($deserialized->id, $deserialized, /* ref */ $this->dataFromAgent->idToSpan); @@ -148,7 +145,6 @@ private function consumeError(Error $original): void $serialized = SerializationUtil::serializeAsJson($original); $deserialized = $this->validateAndDeserializeError($serialized); $deserialized->assertValid(); - $deserialized->assertEquals($original); ArrayUtilForTests::addUnique($deserialized->id, $deserialized, /* ref */ $this->dataFromAgent->idToError); } @@ -161,7 +157,6 @@ private function consumeMetricSet(MetricSet $original): void $serialized = SerializationUtil::serializeAsJson($original); $deserialized = $this->validateAndDeserializeMetricSet($serialized); MetricSetValidator::assertValid($deserialized); - TestCaseBase::assertEqualsEx($original, $deserialized); $this->dataFromAgent->metricSets[] = $deserialized; } diff --git a/tests/ElasticApmTests/Util/AssertValidTrait.php b/tests/ElasticApmTests/Util/AssertValidTrait.php index 79ee69ceb..47f8fc552 100644 --- a/tests/ElasticApmTests/Util/AssertValidTrait.php +++ b/tests/ElasticApmTests/Util/AssertValidTrait.php @@ -25,7 +25,6 @@ use Elastic\Apm\Impl\Constants; use Elastic\Apm\Impl\StackTraceFrame; -use Elastic\Apm\Impl\Util\DbgUtil; use Elastic\Apm\Impl\Util\IdValidationUtil; use Elastic\Apm\Impl\Util\TextUtil; @@ -293,47 +292,4 @@ public static function assertValidCount($count, int $minValue = 0): int TestCaseBase::assertGreaterThanOrEqual($minValue, $count); return $count; } - - /** - * @param mixed $original - * @param mixed $dto - */ - public static function assertEqualOriginalAndDto($original, $dto): void - { - AssertMessageStack::newScope(/* out */ $dbgCtx, AssertMessageStack::funcArgs()); - $dbgCtx->add(['original type' => DbgUtil::getType($original), 'dto type' => DbgUtil::getType($dto)]); - - if (is_object($dto)) { - TestCaseBase::assertIsObject($original); - $originalPropNameToVal = get_object_vars($original); - $dbgCtx->add(['originalPropNameToVal' => $originalPropNameToVal]); - $dbgCtx->pushSubScope(); - foreach (get_object_vars($dto) as $dtoPropName => $dtoPropVal) { - $dbgCtx->clearCurrentSubScope(['dtoPropName' => $dtoPropName, 'dtoPropVal' => $dtoPropVal]); - if ($dtoPropVal !== null) { - TestCaseBase::assertArrayHasKey($dtoPropName, $originalPropNameToVal); - } - if (array_key_exists($dtoPropName, $originalPropNameToVal)) { - self::assertEqualOriginalAndDto($originalPropNameToVal[$dtoPropName], $dtoPropVal); - } - } - $dbgCtx->popSubScope(); - return; - } - - if (is_array($dto)) { - TestCaseBase::assertIsArray($original); - TestCaseBase::assertSameCount($original, $dto); - $dbgCtx->pushSubScope(); - foreach ($dto as $dtoArrayKey => $dtoArrayVal) { - $dbgCtx->clearCurrentSubScope(['dtoArrayKey' => $dtoArrayKey, 'dtoArrayVal' => $dtoArrayVal]); - TestCaseBase::assertArrayHasKey($dtoArrayKey, $original); - self::assertEqualOriginalAndDto($original[$dtoArrayKey], $dtoArrayVal); - } - $dbgCtx->popSubScope(); - return; - } - - TestCaseBase::assertSame($original, $dto); - } } diff --git a/tests/ElasticApmTests/Util/ErrorDto.php b/tests/ElasticApmTests/Util/ErrorDto.php index 0ecce5170..7e7fd6f88 100644 --- a/tests/ElasticApmTests/Util/ErrorDto.php +++ b/tests/ElasticApmTests/Util/ErrorDto.php @@ -24,7 +24,6 @@ namespace ElasticApmTests\Util; use Elastic\Apm\Impl\Constants; -use Elastic\Apm\Impl\Error; use ElasticApmTests\Util\Deserialization\DeserializationUtil; class ErrorDto @@ -152,9 +151,4 @@ public static function assertValidId($errorId): string { return self::assertValidIdEx($errorId, Constants::ERROR_ID_SIZE_IN_BYTES); } - - public function assertEquals(Error $original): void - { - self::assertEqualOriginalAndDto($original, $this); - } } diff --git a/tests/ElasticApmTests/Util/SpanDto.php b/tests/ElasticApmTests/Util/SpanDto.php index 6332f2d83..5099feabd 100644 --- a/tests/ElasticApmTests/Util/SpanDto.php +++ b/tests/ElasticApmTests/Util/SpanDto.php @@ -23,7 +23,6 @@ namespace ElasticApmTests\Util; -use Elastic\Apm\Impl\SpanToSendInterface; use Elastic\Apm\Impl\StackTraceFrame; use ElasticApmTests\Util\Deserialization\DeserializationUtil; use ElasticApmTests\Util\Deserialization\StacktraceDeserializer; @@ -132,11 +131,6 @@ public function assertMatches(SpanExpectations $expectations): void SpanContextExpectations::assertNullableMatches($expectations->context, $this->context); } - public function assertEquals(SpanToSendInterface $original): void - { - self::assertEqualOriginalAndDto($original, $this); - } - public function assertService(?string $targetType, ?string $targetName, string $destinationName, string $destinationResource, string $destinationType): void { TestCaseBase::assertNotNull($this->context); diff --git a/tests/ElasticApmTests/Util/TransactionDto.php b/tests/ElasticApmTests/Util/TransactionDto.php index 1fa59ae03..631bae4fc 100644 --- a/tests/ElasticApmTests/Util/TransactionDto.php +++ b/tests/ElasticApmTests/Util/TransactionDto.php @@ -23,7 +23,6 @@ namespace ElasticApmTests\Util; -use Elastic\Apm\Impl\Transaction; use ElasticApmTests\Util\Deserialization\DeserializationUtil; class TransactionDto extends ExecutionSegmentDto @@ -142,9 +141,4 @@ public function assertMatches(TransactionExpectations $expectations): void $this->context->assertValid(); } } - - public function assertEquals(Transaction $original): void - { - self::assertEqualOriginalAndDto($original, $this); - } }