From a5d803c9922b255f714382b97720645eb81c898d Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Wed, 3 Sep 2025 22:44:00 +0800 Subject: [PATCH 01/10] refactor(sentry): replace CarrierPacker with new Carrier utility class - Replace CarrierPacker dependency with new Carrier utility class in AmqpProducerAspect and TracingAmqpListener - Add new Carrier utility class with fluent API for span carrier operations - Deprecate CarrierPacker class (marked for removal in v3.2) - Simplify constructor dependencies by removing CarrierPacker injection - Update carrier packing/unpacking to use new Carrier::fromSpan() and Carrier::fromJson() methods --- .../src/Tracing/Aspect/AmqpProducerAspect.php | 13 +-- .../Tracing/Listener/TracingAmqpListener.php | 13 +-- src/sentry/src/Util/Carrier.php | 97 +++++++++++++++++++ src/sentry/src/Util/CarrierPacker.php | 3 + 4 files changed, 110 insertions(+), 16 deletions(-) create mode 100644 src/sentry/src/Util/Carrier.php diff --git a/src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php b/src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php index 004505700..ae44b25ac 100644 --- a/src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php +++ b/src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php @@ -14,7 +14,7 @@ use FriendsOfHyperf\Sentry\Constants; use FriendsOfHyperf\Sentry\Switcher; use FriendsOfHyperf\Sentry\Tracing\SpanStarter; -use FriendsOfHyperf\Sentry\Util\CarrierPacker; +use FriendsOfHyperf\Sentry\Util\Carrier; use Hyperf\Amqp\Message\ProducerMessage; use Hyperf\Di\Aop\AbstractAspect; use Hyperf\Di\Aop\ProceedingJoinPoint; @@ -33,10 +33,8 @@ class AmqpProducerAspect extends AbstractAspect 'Hyperf\Amqp\Producer::produceMessage', ]; - public function __construct( - protected Switcher $switcher, - protected CarrierPacker $packer - ) { + public function __construct(protected Switcher $switcher) + { } public function process(ProceedingJoinPoint $proceedingJoinPoint) @@ -84,8 +82,7 @@ protected function handleProduceMessage(ProceedingJoinPoint $proceedingJoinPoint 'messaging.amqp.message.exchange' => $producerMessage->getExchange(), 'messaging.amqp.message.pool_name' => $producerMessage->getPoolName(), ]); - - $carrier = $this->packer->pack($span, [ + $carrier = Carrier::fromSpan($span)->with([ 'publish_time' => microtime(true), 'message_id' => $messageId, 'destination_name' => $destinationName, @@ -94,7 +91,7 @@ protected function handleProduceMessage(ProceedingJoinPoint $proceedingJoinPoint (function () use ($carrier) { $this->properties['application_headers'] ??= new AMQPTable(); - $this->properties['application_headers']->set(Constants::TRACE_CARRIER, $carrier); + $this->properties['application_headers']->set(Constants::TRACE_CARRIER, (string) $carrier); })->call($producerMessage); return tap($proceedingJoinPoint->process(), fn () => $span->setOrigin('auto.amqp')->finish()); diff --git a/src/sentry/src/Tracing/Listener/TracingAmqpListener.php b/src/sentry/src/Tracing/Listener/TracingAmqpListener.php index 4472646af..9fce84e23 100644 --- a/src/sentry/src/Tracing/Listener/TracingAmqpListener.php +++ b/src/sentry/src/Tracing/Listener/TracingAmqpListener.php @@ -14,7 +14,7 @@ use FriendsOfHyperf\Sentry\Constants; use FriendsOfHyperf\Sentry\Switcher; use FriendsOfHyperf\Sentry\Tracing\SpanStarter; -use FriendsOfHyperf\Sentry\Util\CarrierPacker; +use FriendsOfHyperf\Sentry\Util\Carrier; use Hyperf\Amqp\Event\AfterConsume; use Hyperf\Amqp\Event\BeforeConsume; use Hyperf\Amqp\Event\FailToConsume; @@ -32,8 +32,7 @@ class TracingAmqpListener implements ListenerInterface use SpanStarter; public function __construct( - protected Switcher $switcher, - protected CarrierPacker $packer + protected Switcher $switcher ) { } @@ -73,7 +72,7 @@ protected function startTransaction(BeforeConsume $event): void /** @var AMQPTable|null $applicationHeaders */ $applicationHeaders = $amqpMessage->has('application_headers') ? $amqpMessage->get('application_headers') : null; if ($applicationHeaders && isset($applicationHeaders[Constants::TRACE_CARRIER])) { - [$sentryTrace, $baggage] = $this->packer->unpack($applicationHeaders[Constants::TRACE_CARRIER]); + [$sentryTrace, $baggage] = Carrier::fromJson($applicationHeaders[Constants::TRACE_CARRIER])->extract(); Context::set(Constants::TRACE_CARRIER, $applicationHeaders[Constants::TRACE_CARRIER]); } } @@ -97,10 +96,8 @@ protected function finishTransaction(AfterConsume|FailToConsume $event): void return; } - $payload = []; - if ($carrier = Context::get(Constants::TRACE_CARRIER)) { - $payload = json_decode((string) $carrier, true); - } + $carrier = Carrier::fromJson((string) Context::get(Constants::TRACE_CARRIER)); + $payload = $carrier->toArray(); /** @var ConsumerMessage $message */ $message = $event->getMessage(); diff --git a/src/sentry/src/Util/Carrier.php b/src/sentry/src/Util/Carrier.php new file mode 100644 index 000000000..1ae86e373 --- /dev/null +++ b/src/sentry/src/Util/Carrier.php @@ -0,0 +1,97 @@ +data, JSON_UNESCAPED_UNICODE); + } + + public static function fromArray(array $data): static + { + return new static($data); + } + + public static function fromJson(string $json): static + { + $data = (array) json_decode($json, true); + + return new static($data); + } + + public static function fromSpan(Span $span): static + { + return new static([ + 'sentry-trace' => $span->toTraceparent(), + 'baggage' => $span->toBaggage(), + 'traceparent' => $span->toW3CTraceparent(), + ]); + } + + public function with(array $data): static + { + $new = clone $this; + $new->data = array_merge($this->data, $data); + return $new; + } + + public function withSpan(Span $span): static + { + return $this->with([ + 'sentry-trace' => $span->toTraceparent(), + 'baggage' => $span->toBaggage(), + 'traceparent' => $span->toW3CTraceparent(), + ]); + } + + public function get(string $key, mixed $default = null): mixed + { + return $this->data[$key] ?? $default; + } + + public function extract(): array + { + return [ + $this->data['sentry-trace'] ?? $this->data['traceparent'] ?? '', + $this->data['baggage'] ?? '', + $this->data['traceparent'] ?? '', + ]; + } + + public function only(array $keys): array + { + return Arr::only($this->data, $keys); + } + + public function jsonSerialize(): mixed + { + return $this->data; + } + + public function toArray(): array + { + return $this->jsonSerialize(); + } +} diff --git a/src/sentry/src/Util/CarrierPacker.php b/src/sentry/src/Util/CarrierPacker.php index 5eb8fbe98..99e0a3af7 100644 --- a/src/sentry/src/Util/CarrierPacker.php +++ b/src/sentry/src/Util/CarrierPacker.php @@ -14,6 +14,9 @@ use Sentry\Tracing\Span; use Throwable; +/** + * @deprecated since v3.1, use FriendsOfHyperf\Sentry\Util\Carrier instead, will be removed in v3.2 + */ class CarrierPacker { /** From 4ee2098dc7f931fc0e87e6842c8610bc0b07b68d Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Thu, 4 Sep 2025 08:17:57 +0800 Subject: [PATCH 02/10] refactor(tracing): replace CarrierPacker with Carrier utility class and update related methods --- .../src/Tracing/Aspect/AmqpProducerAspect.php | 2 +- .../Aspect/AsyncQueueJobMessageAspect.php | 14 +++---- .../Tracing/Aspect/KafkaProducerAspect.php | 39 +++++++++---------- src/sentry/src/Tracing/Aspect/RpcAspect.php | 7 ++-- .../Tracing/Listener/TracingAmqpListener.php | 21 +++++----- .../Listener/TracingAsyncQueueListener.php | 28 +++++-------- .../Tracing/Listener/TracingKafkaListener.php | 29 ++++++-------- src/sentry/src/Tracing/SpanStarter.php | 11 +++--- src/sentry/src/Util/Carrier.php | 35 ++++++++++++++++- 9 files changed, 101 insertions(+), 85 deletions(-) diff --git a/src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php b/src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php index ae44b25ac..6a890607b 100644 --- a/src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php +++ b/src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php @@ -91,7 +91,7 @@ protected function handleProduceMessage(ProceedingJoinPoint $proceedingJoinPoint (function () use ($carrier) { $this->properties['application_headers'] ??= new AMQPTable(); - $this->properties['application_headers']->set(Constants::TRACE_CARRIER, (string) $carrier); + $this->properties['application_headers']->set(Constants::TRACE_CARRIER, $carrier->toJson()); })->call($producerMessage); return tap($proceedingJoinPoint->process(), fn () => $span->setOrigin('auto.amqp')->finish()); diff --git a/src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php b/src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php index 69d3836b9..0df2d2143 100644 --- a/src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php +++ b/src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php @@ -14,7 +14,7 @@ use FriendsOfHyperf\Sentry\Constants; use FriendsOfHyperf\Sentry\Switcher; use FriendsOfHyperf\Sentry\Tracing\SpanStarter; -use FriendsOfHyperf\Sentry\Util\CarrierPacker; +use FriendsOfHyperf\Sentry\Util\Carrier; use Hyperf\AsyncQueue\Driver\RedisDriver; use Hyperf\Context\Context; use Hyperf\Di\Aop\AbstractAspect; @@ -25,6 +25,7 @@ /** * @property \Hyperf\AsyncQueue\Driver\ChannelConfig $channel * @property \Hyperf\Redis\RedisProxy $redis + * @property \Hyperf\Contract\PackerInterface $packer * @property string $poolName */ class AsyncQueueJobMessageAspect extends AbstractAspect @@ -39,8 +40,7 @@ class AsyncQueueJobMessageAspect extends AbstractAspect ]; public function __construct( - protected Switcher $switcher, - protected CarrierPacker $packer + protected Switcher $switcher ) { } @@ -98,7 +98,7 @@ public function handlePush(ProceedingJoinPoint $proceedingJoinPoint) }; $span->setData($data); - $carrier = $this->packer->pack($span, [ + $carrier = Carrier::fromSpan($span)->with([ 'publish_time' => microtime(true), 'message_id' => $messageId, 'destination_name' => $destinationName, @@ -137,9 +137,9 @@ protected function handleSerialize(ProceedingJoinPoint $proceedingJoinPoint) return with($proceedingJoinPoint->process(), function ($result) { if (is_array($result) && $carrier = Context::get(Constants::TRACE_CARRIER)) { if (array_is_list($result)) { - $result[] = $carrier; + $result[] = $carrier->toJson(); } elseif (isset($result['job'])) { - $result[Constants::TRACE_CARRIER] = $carrier; + $result[Constants::TRACE_CARRIER] = $carrier->toJson(); } } @@ -163,7 +163,7 @@ protected function handleUnserialize(ProceedingJoinPoint $proceedingJoinPoint) /** @var string|null $carrier */ if ($carrier) { - Context::set(Constants::TRACE_CARRIER, $carrier); + Context::set(Constants::TRACE_CARRIER, Carrier::fromJson($carrier)); } return $proceedingJoinPoint->process(); diff --git a/src/sentry/src/Tracing/Aspect/KafkaProducerAspect.php b/src/sentry/src/Tracing/Aspect/KafkaProducerAspect.php index 45e71507f..d57de7fb1 100644 --- a/src/sentry/src/Tracing/Aspect/KafkaProducerAspect.php +++ b/src/sentry/src/Tracing/Aspect/KafkaProducerAspect.php @@ -14,7 +14,7 @@ use FriendsOfHyperf\Sentry\Constants; use FriendsOfHyperf\Sentry\Switcher; use FriendsOfHyperf\Sentry\Tracing\SpanStarter; -use FriendsOfHyperf\Sentry\Util\CarrierPacker; +use FriendsOfHyperf\Sentry\Util\Carrier; use Hyperf\Di\Aop\AbstractAspect; use Hyperf\Di\Aop\ProceedingJoinPoint; use longlang\phpkafka\Producer\ProduceMessage; @@ -37,8 +37,7 @@ class KafkaProducerAspect extends AbstractAspect ]; public function __construct( - protected Switcher $switcher, - protected CarrierPacker $packer + protected Switcher $switcher ) { } @@ -78,16 +77,17 @@ protected function sendAsync(ProceedingJoinPoint $proceedingJoinPoint) 'messaging.destination.name' => $destinationName, ]); - $carrier = $this->packer->pack($span, [ - 'publish_time' => microtime(true), - 'message_id' => $messageId, - 'destination_name' => $destinationName, - 'body_size' => $bodySize, - ]); + $carrier = Carrier::fromSpan($span) + ->with([ + 'publish_time' => microtime(true), + 'message_id' => $messageId, + 'destination_name' => $destinationName, + 'body_size' => $bodySize, + ]); $headers = $proceedingJoinPoint->arguments['keys']['headers'] ?? []; $headers[] = (new RecordHeader()) ->setHeaderKey(Constants::TRACE_CARRIER) - ->setValue($carrier); + ->setValue($carrier->toJson()); $proceedingJoinPoint->arguments['keys']['headers'] = $headers; return tap($proceedingJoinPoint->process(), fn () => $span->setOrigin('auto.kafka')->finish()); @@ -106,17 +106,16 @@ protected function sendBatchAsync(ProceedingJoinPoint $proceedingJoinPoint) return $proceedingJoinPoint->process(); } - $packer = $this->packer; - foreach ($messages as $message) { - (function () use ($span, $packer) { - $carrier = $packer->pack($span, [ - 'publish_time' => microtime(true), - 'message_id' => uniqid('kafka_', true), - 'destination_name' => $this->getTopic(), - 'body_size' => strlen((string) $this->getValue()), - ]); - $this->headers[] = (new RecordHeader())->setHeaderKey(Constants::TRACE_CARRIER)->setValue($carrier); + (function () use ($span) { + $carrier = Carrier::fromSpan($span) + ->with([ + 'publish_time' => microtime(true), + 'message_id' => uniqid('kafka_', true), + 'destination_name' => $this->getTopic(), + 'body_size' => strlen((string) $this->getValue()), + ]); + $this->headers[] = (new RecordHeader())->setHeaderKey(Constants::TRACE_CARRIER)->setValue($carrier->toJson()); })->call($message); } diff --git a/src/sentry/src/Tracing/Aspect/RpcAspect.php b/src/sentry/src/Tracing/Aspect/RpcAspect.php index 7e8523a73..6e0cc17ba 100644 --- a/src/sentry/src/Tracing/Aspect/RpcAspect.php +++ b/src/sentry/src/Tracing/Aspect/RpcAspect.php @@ -14,7 +14,7 @@ use FriendsOfHyperf\Sentry\Constants; use FriendsOfHyperf\Sentry\Switcher; use FriendsOfHyperf\Sentry\Tracing\SpanStarter; -use FriendsOfHyperf\Sentry\Util\CarrierPacker; +use FriendsOfHyperf\Sentry\Util\Carrier; use Hyperf\Context\Context; use Hyperf\Contract\ConfigInterface; use Hyperf\Coroutine\Coroutine; @@ -46,8 +46,7 @@ class RpcAspect extends AbstractAspect public function __construct( protected ContainerInterface $container, - protected Switcher $switcher, - protected CarrierPacker $packer + protected Switcher $switcher ) { } @@ -99,7 +98,7 @@ private function handleGenerateRpcPath(ProceedingJoinPoint $proceedingJoinPoint) Context::set(static::SPAN, $span->setData($data)); if ($this->container->has(Rpc\Context::class)) { - $this->container->get(Rpc\Context::class)->set(Constants::TRACE_CARRIER, $this->packer->pack($span)); + $this->container->get(Rpc\Context::class)->set(Constants::TRACE_CARRIER, Carrier::fromSpan($span)->toJson()); } return $path; diff --git a/src/sentry/src/Tracing/Listener/TracingAmqpListener.php b/src/sentry/src/Tracing/Listener/TracingAmqpListener.php index 9fce84e23..ced584837 100644 --- a/src/sentry/src/Tracing/Listener/TracingAmqpListener.php +++ b/src/sentry/src/Tracing/Listener/TracingAmqpListener.php @@ -64,7 +64,6 @@ public function process(object $event): void protected function startTransaction(BeforeConsume $event): void { $message = $event->getMessage(); - $sentryTrace = $baggage = ''; if (method_exists($event, 'getAMQPMessage')) { /** @var AMQPMessage $amqpMessage */ @@ -72,14 +71,14 @@ protected function startTransaction(BeforeConsume $event): void /** @var AMQPTable|null $applicationHeaders */ $applicationHeaders = $amqpMessage->has('application_headers') ? $amqpMessage->get('application_headers') : null; if ($applicationHeaders && isset($applicationHeaders[Constants::TRACE_CARRIER])) { - [$sentryTrace, $baggage] = Carrier::fromJson($applicationHeaders[Constants::TRACE_CARRIER])->extract(); - Context::set(Constants::TRACE_CARRIER, $applicationHeaders[Constants::TRACE_CARRIER]); + $carrier = Carrier::fromJson($applicationHeaders[Constants::TRACE_CARRIER]); + Context::set(Constants::TRACE_CARRIER, $carrier); } } $this->continueTrace( - sentryTrace: $sentryTrace, - baggage: $baggage, + sentryTrace: $carrier?->getSentryTrace(), + baggage: $carrier?->getBaggage(), name: $message::class, op: 'queue.process', description: $message::class, @@ -96,19 +95,19 @@ protected function finishTransaction(AfterConsume|FailToConsume $event): void return; } - $carrier = Carrier::fromJson((string) Context::get(Constants::TRACE_CARRIER)); - $payload = $carrier->toArray(); + /** @var Carrier|null $carrier */ + $carrier = Context::get(Constants::TRACE_CARRIER); /** @var ConsumerMessage $message */ $message = $event->getMessage(); $data = [ 'messaging.system' => 'amqp', 'messaging.operation' => 'process', - 'messaging.message.id' => $payload['message_id'] ?? null, - 'messaging.message.body.size' => $payload['body_size'] ?? null, - 'messaging.message.receive.latency' => isset($payload['publish_time']) ? (microtime(true) - $payload['publish_time']) : null, + 'messaging.message.id' => $carrier->get('message_id'), + 'messaging.message.body.size' => $carrier->get('body_size'), + 'messaging.message.receive.latency' => $carrier->has('publish_time') ? (microtime(true) - $carrier->get('publish_time')) : null, 'messaging.message.retry.count' => 0, - 'messaging.destination.name' => $payload['destination_name'] ?? $message->getExchange(), + 'messaging.destination.name' => $carrier->get('destination_name') ?? $message->getExchange(), // for amqp 'messaging.amqp.message.type' => $message->getTypeString(), 'messaging.amqp.message.routing_key' => $message->getRoutingKey(), diff --git a/src/sentry/src/Tracing/Listener/TracingAsyncQueueListener.php b/src/sentry/src/Tracing/Listener/TracingAsyncQueueListener.php index 2f0af2bff..261987600 100644 --- a/src/sentry/src/Tracing/Listener/TracingAsyncQueueListener.php +++ b/src/sentry/src/Tracing/Listener/TracingAsyncQueueListener.php @@ -14,7 +14,7 @@ use FriendsOfHyperf\Sentry\Constants; use FriendsOfHyperf\Sentry\Switcher; use FriendsOfHyperf\Sentry\Tracing\SpanStarter; -use FriendsOfHyperf\Sentry\Util\CarrierPacker; +use FriendsOfHyperf\Sentry\Util\Carrier; use Hyperf\AsyncQueue\Event\AfterHandle; use Hyperf\AsyncQueue\Event\BeforeHandle; use Hyperf\AsyncQueue\Event\FailedHandle; @@ -31,8 +31,7 @@ class TracingAsyncQueueListener implements ListenerInterface use SpanStarter; public function __construct( - protected Switcher $switcher, - protected CarrierPacker $packer + protected Switcher $switcher ) { } @@ -64,20 +63,14 @@ public function process(object $event): void protected function startTransaction(BeforeHandle $event): void { - $sentryTrace = $baggage = ''; - - /** @var string|null $carrier */ + /** @var Carrier|null $carrier */ $carrier = Context::get(Constants::TRACE_CARRIER, null, Coroutine::parentId()); - if ($carrier) { - [$sentryTrace, $baggage] = $this->packer->unpack($carrier); - } - $job = $event->getMessage()->job(); $this->continueTrace( - sentryTrace: $sentryTrace, - baggage: $baggage, + sentryTrace: $carrier?->getSentryTrace(), + baggage: $carrier?->getBaggage(), name: $job::class, op: 'queue.process', description: 'async_queue: ' . $job::class, @@ -94,17 +87,16 @@ protected function finishTransaction(AfterHandle|RetryHandle|FailedHandle $event return; } - /** @var string|null $carrier */ + /** @var Carrier|null $carrier */ $carrier = Context::get(Constants::TRACE_CARRIER, null, Coroutine::parentId()); - $payload = json_decode((string) $carrier, true); $data = [ 'messaging.system' => 'async_queue', 'messaging.operation' => 'process', - 'messaging.message.id' => $payload['message_id'] ?? null, - 'messaging.message.body.size' => $payload['body_size'] ?? null, - 'messaging.message.receive.latency' => isset($payload['publish_time']) ? (microtime(true) - $payload['publish_time']) : null, + 'messaging.message.id' => $carrier?->get('message_id'), + 'messaging.message.body.size' => $carrier?->get('body_size'), + 'messaging.message.receive.latency' => $carrier?->has('publish_time') ? (microtime(true) - $carrier->get('publish_time')) : null, 'messaging.message.retry.count' => $event->getMessage()->getAttempts(), - 'messaging.destination.name' => $payload['destination_name'] ?? null, + 'messaging.destination.name' => $carrier?->get('destination_name'), ]; $tags = []; diff --git a/src/sentry/src/Tracing/Listener/TracingKafkaListener.php b/src/sentry/src/Tracing/Listener/TracingKafkaListener.php index 316d16500..52841e426 100644 --- a/src/sentry/src/Tracing/Listener/TracingKafkaListener.php +++ b/src/sentry/src/Tracing/Listener/TracingKafkaListener.php @@ -14,7 +14,7 @@ use FriendsOfHyperf\Sentry\Constants; use FriendsOfHyperf\Sentry\Switcher; use FriendsOfHyperf\Sentry\Tracing\SpanStarter; -use FriendsOfHyperf\Sentry\Util\CarrierPacker; +use FriendsOfHyperf\Sentry\Util\Carrier; use Hyperf\Context\Context; use Hyperf\Event\Contract\ListenerInterface; use Hyperf\Kafka\Event\AfterConsume; @@ -30,8 +30,7 @@ class TracingKafkaListener implements ListenerInterface use SpanStarter; public function __construct( - protected Switcher $switcher, - protected CarrierPacker $packer + protected Switcher $switcher ) { } @@ -64,21 +63,20 @@ protected function startTransaction(BeforeConsume $event): void { $consumer = $event->getConsumer(); $message = $event->getData(); - $sentryTrace = $baggage = ''; if ($message instanceof ConsumeMessage) { foreach ($message->getHeaders() as $header) { if ($header->getHeaderKey() === Constants::TRACE_CARRIER) { - [$sentryTrace, $baggage] = $this->packer->unpack($header->getValue()); - Context::set(Constants::TRACE_CARRIER, $header->getValue()); + $carrier = Carrier::fromJson($header->getValue()); + Context::set(Constants::TRACE_CARRIER, $carrier); break; } } } $this->continueTrace( - sentryTrace: $sentryTrace, - baggage: $baggage, + sentryTrace: $carrier->getSentryTrace(), + baggage: $carrier->getBaggage(), name: $consumer->getTopic() . ' process', op: 'queue.process', description: $consumer::class, @@ -95,21 +93,18 @@ protected function finishTransaction(AfterConsume|FailToConsume $event): void return; } - $payload = []; - if ($carrier = (string) Context::get(Constants::TRACE_CARRIER)) { - $payload = json_decode($carrier, true); - } - + /** @var Carrier|null $carrier */ + $carrier = Context::get(Constants::TRACE_CARRIER); $consumer = $event->getConsumer(); $tags = []; $data = [ 'messaging.system' => 'kafka', 'messaging.operation' => 'process', - 'messaging.message.id' => $payload['message_id'] ?? null, - 'messaging.message.body.size' => $payload['body_size'] ?? null, - 'messaging.message.receive.latency' => isset($payload['publish_time']) ? (microtime(true) - $payload['publish_time']) : null, + 'messaging.message.id' => $carrier->get('message_id'), + 'messaging.message.body.size' => $carrier->get('body_size'), + 'messaging.message.receive.latency' => $carrier->has('publish_time') ? (microtime(true) - $carrier->get('publish_time')) : null, 'messaging.message.retry.count' => 0, - 'messaging.destination.name' => $payload['destination_name'] ?? 'unknown', + 'messaging.destination.name' => $carrier->get('destination_name'), // for kafka 'messaging.kafka.consumer.group' => $consumer->getGroupId(), 'messaging.kafka.consumer.pool' => $consumer->getPool(), diff --git a/src/sentry/src/Tracing/SpanStarter.php b/src/sentry/src/Tracing/SpanStarter.php index 460617537..33ad2eb20 100644 --- a/src/sentry/src/Tracing/SpanStarter.php +++ b/src/sentry/src/Tracing/SpanStarter.php @@ -12,7 +12,7 @@ namespace FriendsOfHyperf\Sentry\Tracing; use FriendsOfHyperf\Sentry\Constants; -use FriendsOfHyperf\Sentry\Util\CarrierPacker; +use FriendsOfHyperf\Sentry\Util\Carrier; use Hyperf\Context\ApplicationContext; use Hyperf\Rpc\Context as RpcContext; use Psr\Http\Message\ServerRequestInterface; @@ -59,12 +59,13 @@ protected function startRequestTransaction(ServerRequestInterface $request, ...$ $baggage = $request->getHeaderLine('baggage'); $container = $this->container ?? ApplicationContext::getContainer(); + // Rpc Context if ($container->has(RpcContext::class)) { $rpcContext = $container->get(RpcContext::class); - $carrier = $rpcContext->get(Constants::TRACE_CARRIER); - if ($carrier) { - $packer = $container->get(CarrierPacker::class); - [$sentryTrace, $baggage] = $packer->unpack($carrier); + $payload = $rpcContext->get(Constants::TRACE_CARRIER); + if ($payload) { + $carrier = Carrier::fromJson($payload); + [$sentryTrace, $baggage] = [$carrier->getSentryTrace(), $carrier->getBaggage()]; } } diff --git a/src/sentry/src/Util/Carrier.php b/src/sentry/src/Util/Carrier.php index 1ae86e373..814d7ec13 100644 --- a/src/sentry/src/Util/Carrier.php +++ b/src/sentry/src/Util/Carrier.php @@ -13,11 +13,13 @@ use Hyperf\Collection\Arr; use Hyperf\Contract\Arrayable; +use Hyperf\Contract\Jsonable; +use JsonException; use JsonSerializable; use Sentry\Tracing\Span; use Stringable; -class Carrier implements JsonSerializable, Arrayable, Stringable +class Carrier implements JsonSerializable, Arrayable, Stringable, Jsonable { public function __construct( protected array $data = [] @@ -26,7 +28,7 @@ public function __construct( public function __toString(): string { - return json_encode($this->data, JSON_UNESCAPED_UNICODE); + return $this->toJson(); } public static function fromArray(array $data): static @@ -66,6 +68,26 @@ public function withSpan(Span $span): static ]); } + public function getSentryTrace(): string + { + return $this->data['sentry-trace'] ?? ''; + } + + public function getBaggage(): string + { + return $this->data['baggage'] ?? ''; + } + + public function getTraceparent(): string + { + return $this->data['traceparent'] ?? ''; + } + + public function has(string $key): bool + { + return isset($this->data[$key]); + } + public function get(string $key, mixed $default = null): mixed { return $this->data[$key] ?? $default; @@ -94,4 +116,13 @@ public function toArray(): array { return $this->jsonSerialize(); } + + public function toJson(int $options = JSON_UNESCAPED_UNICODE): string + { + try { + return json_encode($this->toArray(), $options | JSON_THROW_ON_ERROR); + } catch (JsonException $e) { + return '{}'; + } + } } From ef6ca0af305d1fdb13bf118acba04eab2f041c42 Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Thu, 4 Sep 2025 08:22:52 +0800 Subject: [PATCH 03/10] fix(tracing): initialize carrier variable to null in startTransaction method for AMQP and Kafka listeners --- .../src/Tracing/Listener/TracingAmqpListener.php | 1 + .../src/Tracing/Listener/TracingKafkaListener.php | 13 +++++++------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/sentry/src/Tracing/Listener/TracingAmqpListener.php b/src/sentry/src/Tracing/Listener/TracingAmqpListener.php index ced584837..a88aabb36 100644 --- a/src/sentry/src/Tracing/Listener/TracingAmqpListener.php +++ b/src/sentry/src/Tracing/Listener/TracingAmqpListener.php @@ -64,6 +64,7 @@ public function process(object $event): void protected function startTransaction(BeforeConsume $event): void { $message = $event->getMessage(); + $carrier = null; if (method_exists($event, 'getAMQPMessage')) { /** @var AMQPMessage $amqpMessage */ diff --git a/src/sentry/src/Tracing/Listener/TracingKafkaListener.php b/src/sentry/src/Tracing/Listener/TracingKafkaListener.php index 52841e426..ac66d8b29 100644 --- a/src/sentry/src/Tracing/Listener/TracingKafkaListener.php +++ b/src/sentry/src/Tracing/Listener/TracingKafkaListener.php @@ -63,6 +63,7 @@ protected function startTransaction(BeforeConsume $event): void { $consumer = $event->getConsumer(); $message = $event->getData(); + $carrier = null; if ($message instanceof ConsumeMessage) { foreach ($message->getHeaders() as $header) { @@ -75,8 +76,8 @@ protected function startTransaction(BeforeConsume $event): void } $this->continueTrace( - sentryTrace: $carrier->getSentryTrace(), - baggage: $carrier->getBaggage(), + sentryTrace: (string) $carrier?->getSentryTrace(), + baggage: (string) $carrier?->getBaggage(), name: $consumer->getTopic() . ' process', op: 'queue.process', description: $consumer::class, @@ -100,11 +101,11 @@ protected function finishTransaction(AfterConsume|FailToConsume $event): void $data = [ 'messaging.system' => 'kafka', 'messaging.operation' => 'process', - 'messaging.message.id' => $carrier->get('message_id'), - 'messaging.message.body.size' => $carrier->get('body_size'), - 'messaging.message.receive.latency' => $carrier->has('publish_time') ? (microtime(true) - $carrier->get('publish_time')) : null, + 'messaging.message.id' => $carrier?->get('message_id'), + 'messaging.message.body.size' => $carrier?->get('body_size'), + 'messaging.message.receive.latency' => $carrier?->has('publish_time') ? (microtime(true) - $carrier->get('publish_time')) : null, 'messaging.message.retry.count' => 0, - 'messaging.destination.name' => $carrier->get('destination_name'), + 'messaging.destination.name' => $carrier?->get('destination_name'), // for kafka 'messaging.kafka.consumer.group' => $consumer->getGroupId(), 'messaging.kafka.consumer.pool' => $consumer->getPool(), From b5d9c1e2b400c14a8343eadef7bbf8e5b086823e Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Thu, 4 Sep 2025 08:30:17 +0800 Subject: [PATCH 04/10] fix(carrier): handle JSON decoding errors in fromJson method fix(tracing): use null coalescing for optional carrier properties in TracingAmqpListener and TracingKafkaListener --- - | 0 src/sentry/src/Tracing/Listener/TracingAmqpListener.php | 8 ++++---- src/sentry/src/Tracing/Listener/TracingKafkaListener.php | 4 ++-- src/sentry/src/Util/Carrier.php | 9 ++++++++- 4 files changed, 14 insertions(+), 7 deletions(-) create mode 100644 - diff --git a/- b/- new file mode 100644 index 000000000..e69de29bb diff --git a/src/sentry/src/Tracing/Listener/TracingAmqpListener.php b/src/sentry/src/Tracing/Listener/TracingAmqpListener.php index a88aabb36..a5da0767e 100644 --- a/src/sentry/src/Tracing/Listener/TracingAmqpListener.php +++ b/src/sentry/src/Tracing/Listener/TracingAmqpListener.php @@ -104,11 +104,11 @@ protected function finishTransaction(AfterConsume|FailToConsume $event): void $data = [ 'messaging.system' => 'amqp', 'messaging.operation' => 'process', - 'messaging.message.id' => $carrier->get('message_id'), - 'messaging.message.body.size' => $carrier->get('body_size'), - 'messaging.message.receive.latency' => $carrier->has('publish_time') ? (microtime(true) - $carrier->get('publish_time')) : null, + 'messaging.message.id' => $carrier?->get('message_id'), + 'messaging.message.body.size' => $carrier?->get('body_size'), + 'messaging.message.receive.latency' => $carrier?->has('publish_time') ? (microtime(true) - $carrier->get('publish_time')) : null, 'messaging.message.retry.count' => 0, - 'messaging.destination.name' => $carrier->get('destination_name') ?? $message->getExchange(), + 'messaging.destination.name' => $carrier?->get('destination_name') ?? $message->getExchange(), // for amqp 'messaging.amqp.message.type' => $message->getTypeString(), 'messaging.amqp.message.routing_key' => $message->getRoutingKey(), diff --git a/src/sentry/src/Tracing/Listener/TracingKafkaListener.php b/src/sentry/src/Tracing/Listener/TracingKafkaListener.php index ac66d8b29..13db955fd 100644 --- a/src/sentry/src/Tracing/Listener/TracingKafkaListener.php +++ b/src/sentry/src/Tracing/Listener/TracingKafkaListener.php @@ -76,8 +76,8 @@ protected function startTransaction(BeforeConsume $event): void } $this->continueTrace( - sentryTrace: (string) $carrier?->getSentryTrace(), - baggage: (string) $carrier?->getBaggage(), + sentryTrace: $carrier?->getSentryTrace() ?? '', + baggage: $carrier?->getBaggage() ?? '', name: $consumer->getTopic() . ' process', op: 'queue.process', description: $consumer::class, diff --git a/src/sentry/src/Util/Carrier.php b/src/sentry/src/Util/Carrier.php index 814d7ec13..4ae76c7d1 100644 --- a/src/sentry/src/Util/Carrier.php +++ b/src/sentry/src/Util/Carrier.php @@ -38,7 +38,14 @@ public static function fromArray(array $data): static public static function fromJson(string $json): static { - $data = (array) json_decode($json, true); + try { + $data = json_decode($json, true, 512, JSON_THROW_ON_ERROR); + if (! is_array($data)) { + $data = []; + } + } catch (JsonException $e) { + $data = []; + } return new static($data); } From 13e7496d0de2506035071821d060001632b80da9 Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Thu, 4 Sep 2025 08:36:50 +0800 Subject: [PATCH 05/10] test(carrier): add comprehensive tests for Carrier class functionality and error handling --- tests/Sentry/CarrierTest.php | 360 +++++++++++++++++++++++++++++++++++ 1 file changed, 360 insertions(+) create mode 100644 tests/Sentry/CarrierTest.php diff --git a/tests/Sentry/CarrierTest.php b/tests/Sentry/CarrierTest.php new file mode 100644 index 000000000..fa15721f1 --- /dev/null +++ b/tests/Sentry/CarrierTest.php @@ -0,0 +1,360 @@ +testData = [ + 'sentry-trace' => '12345678901234567890123456789012-1234567890123456-1', + 'baggage' => 'sentry-trace_id=12345678901234567890123456789012,sentry-public_key=public_key', + 'traceparent' => '00-12345678901234567890123456789012-1234567890123456-01', + 'custom_field' => 'custom_value', + 'publish_time' => 1234567890.123, + ]; +}); + +describe('Carrier Construction', function () { + test('can be constructed with empty data', function () { + $carrier = new Carrier(); + expect($carrier->toArray())->toBe([]); + }); + + test('can be constructed with initial data', function () { + $carrier = new Carrier($this->testData); + expect($carrier->toArray())->toBe($this->testData); + }); + + test('can be created from array', function () { + $carrier = Carrier::fromArray($this->testData); + expect($carrier->toArray())->toBe($this->testData); + }); + + test('can be created from valid JSON', function () { + $json = json_encode($this->testData); + $carrier = Carrier::fromJson($json); + expect($carrier->toArray())->toBe($this->testData); + }); + + test('handles invalid JSON gracefully', function () { + $carrier = Carrier::fromJson('invalid json'); + expect($carrier->toArray())->toBe([]); + }); + + test('handles non-array JSON gracefully', function () { + $carrier = Carrier::fromJson('"string value"'); + expect($carrier->toArray())->toBe([]); + }); + + test('handles empty JSON', function () { + $carrier = Carrier::fromJson(''); + expect($carrier->toArray())->toBe([]); + }); + + test('can be created from Span', function () { + $spanContext = new SpanContext(); + $span = new Span($spanContext); + + $carrier = Carrier::fromSpan($span); + $data = $carrier->toArray(); + + expect($data)->toHaveKeys(['sentry-trace', 'baggage', 'traceparent']); + expect($data['sentry-trace'])->toBeString(); + expect($data['baggage'])->toBeString(); + expect($data['traceparent'])->toBeString(); + }); +}); + +describe('Data Manipulation', function () { + test('can add data using with method', function () { + $carrier = new Carrier(['existing' => 'value']); + $newCarrier = $carrier->with(['new' => 'data']); + + expect($carrier->toArray())->toBe(['existing' => 'value']); + expect($newCarrier->toArray())->toBe(['existing' => 'value', 'new' => 'data']); + }); + + test('with method merges data correctly', function () { + $carrier = new Carrier(['key1' => 'value1', 'key2' => 'value2']); + $newCarrier = $carrier->with(['key2' => 'updated', 'key3' => 'new']); + + expect($newCarrier->toArray())->toBe([ + 'key1' => 'value1', + 'key2' => 'updated', + 'key3' => 'new', + ]); + }); + + test('can add span data using withSpan method', function () { + $spanContext = new SpanContext(); + $span = new Span($spanContext); + + $carrier = new Carrier(['existing' => 'data']); + $newCarrier = $carrier->withSpan($span); + + $data = $newCarrier->toArray(); + expect($data)->toHaveKey('existing'); + expect($data)->toHaveKeys(['sentry-trace', 'baggage', 'traceparent']); + expect($data['existing'])->toBe('data'); + }); +}); + +describe('Data Access', function () { + test('can check if key exists', function () { + $carrier = new Carrier($this->testData); + + expect($carrier->has('sentry-trace'))->toBeTrue(); + expect($carrier->has('nonexistent'))->toBeFalse(); + }); + + test('can get value by key', function () { + $carrier = new Carrier($this->testData); + + expect($carrier->get('custom_field'))->toBe('custom_value'); + expect($carrier->get('nonexistent'))->toBeNull(); + expect($carrier->get('nonexistent', 'default'))->toBe('default'); + }); + + test('getSentryTrace returns correct value', function () { + $carrier = new Carrier($this->testData); + expect($carrier->getSentryTrace())->toBe('12345678901234567890123456789012-1234567890123456-1'); + }); + + test('getSentryTrace returns empty string when not set', function () { + $carrier = new Carrier(); + expect($carrier->getSentryTrace())->toBe(''); + }); + + test('getBaggage returns correct value', function () { + $carrier = new Carrier($this->testData); + expect($carrier->getBaggage())->toBe('sentry-trace_id=12345678901234567890123456789012,sentry-public_key=public_key'); + }); + + test('getBaggage returns empty string when not set', function () { + $carrier = new Carrier(); + expect($carrier->getBaggage())->toBe(''); + }); + + test('getTraceparent returns correct value', function () { + $carrier = new Carrier($this->testData); + expect($carrier->getTraceparent())->toBe('00-12345678901234567890123456789012-1234567890123456-01'); + }); + + test('getTraceparent returns empty string when not set', function () { + $carrier = new Carrier(); + expect($carrier->getTraceparent())->toBe(''); + }); +}); + +describe('Data Extraction', function () { + test('extract returns array with tracing data', function () { + $carrier = new Carrier($this->testData); + $extracted = $carrier->extract(); + + expect($extracted)->toBe([ + '12345678901234567890123456789012-1234567890123456-1', + 'sentry-trace_id=12345678901234567890123456789012,sentry-public_key=public_key', + '00-12345678901234567890123456789012-1234567890123456-01', + ]); + }); + + test('extract handles missing sentry-trace gracefully', function () { + $data = $this->testData; + unset($data['sentry-trace']); + $carrier = new Carrier($data); + $extracted = $carrier->extract(); + + expect($extracted[0])->toBe('00-12345678901234567890123456789012-1234567890123456-01'); + }); + + test('extract handles missing traceparent and sentry-trace gracefully', function () { + $data = $this->testData; + unset($data['sentry-trace'], $data['traceparent']); + $carrier = new Carrier($data); + $extracted = $carrier->extract(); + + expect($extracted[0])->toBe(''); + }); + + test('only returns specified keys', function () { + $carrier = new Carrier($this->testData); + $result = $carrier->only(['custom_field', 'publish_time']); + + expect($result)->toBe([ + 'custom_field' => 'custom_value', + 'publish_time' => 1234567890.123, + ]); + }); + + test('only handles nonexistent keys gracefully', function () { + $carrier = new Carrier($this->testData); + $result = $carrier->only(['nonexistent', 'custom_field']); + + expect($result)->toBe(['custom_field' => 'custom_value']); + }); +}); + +describe('Serialization', function () { + test('implements JsonSerializable', function () { + $carrier = new Carrier($this->testData); + expect($carrier->jsonSerialize())->toBe($this->testData); + }); + + test('toArray returns correct data', function () { + $carrier = new Carrier($this->testData); + expect($carrier->toArray())->toBe($this->testData); + }); + + test('toJson returns valid JSON string', function () { + $carrier = new Carrier($this->testData); + $json = $carrier->toJson(); + + expect($json)->toBeString(); + expect(json_decode($json, true))->toBe($this->testData); + }); + + test('toJson with custom options', function () { + $data = ['key' => 'value with unicode: 中文']; + $carrier = new Carrier($data); + $json = $carrier->toJson(JSON_UNESCAPED_UNICODE | JSON_PRETTY_PRINT); + + expect($json)->toContain('中文'); + expect($json)->toContain("\n"); // Pretty print adds newlines + }); + + test('toJson handles encoding errors gracefully', function () { + // Create data that cannot be JSON encoded + $resource = fopen('php://memory', 'r'); + $carrier = new Carrier(['resource' => $resource]); + fclose($resource); + + $json = $carrier->toJson(); + expect($json)->toBe('{}'); + }); + + test('__toString returns JSON representation', function () { + $carrier = new Carrier($this->testData); + $string = (string) $carrier; + + expect($string)->toBe($carrier->toJson()); + expect(json_decode($string, true))->toBe($this->testData); + }); + + test('can be JSON encoded directly', function () { + $carrier = new Carrier($this->testData); + $json = json_encode($carrier); + + expect(json_decode($json, true))->toBe($this->testData); + }); +}); + +describe('Edge Cases and Error Handling', function () { + test('handles null values in data', function () { + $data = ['key' => null, 'other' => 'value']; + $carrier = new Carrier($data); + + expect($carrier->get('key'))->toBeNull(); + expect($carrier->has('key'))->toBeFalse(); // isset() returns false for null values + expect($carrier->toArray())->toBe($data); + }); + + test('handles boolean and numeric values', function () { + $data = [ + 'bool_true' => true, + 'bool_false' => false, + 'int' => 42, + 'float' => 3.14, + ]; + $carrier = new Carrier($data); + + expect($carrier->get('bool_true'))->toBeTrue(); + expect($carrier->get('bool_false'))->toBeFalse(); + expect($carrier->get('int'))->toBe(42); + expect($carrier->get('float'))->toBe(3.14); + }); + + test('handles array and object values', function () { + $data = [ + 'array' => ['nested', 'array'], + 'object' => (object) ['property' => 'value'], + ]; + $carrier = new Carrier($data); + + expect($carrier->get('array'))->toBe(['nested', 'array']); + expect($carrier->get('object'))->toBeInstanceOf('stdClass'); + }); + + test('immutability of original carrier in with method', function () { + $original = new Carrier(['original' => 'data']); + $modified = $original->with(['new' => 'data']); + + expect($original === $modified)->toBeFalse(); + expect($original->has('new'))->toBeFalse(); + expect($modified->has('new'))->toBeTrue(); + }); + + test('deep cloning behavior', function () { + $data = ['nested' => ['array' => 'value']]; + $original = new Carrier($data); + $modified = $original->with(['other' => 'data']); + + // Modify the nested array in the modified carrier + $modifiedData = $modified->toArray(); + $modifiedData['nested']['array'] = 'changed'; + + // Original should remain unchanged + expect($original->get('nested'))->toBe(['array' => 'value']); + }); +}); + +describe('Integration Tests', function () { + test('round trip JSON serialization', function () { + $original = new Carrier($this->testData); + $json = $original->toJson(); + $restored = Carrier::fromJson($json); + + expect($restored->toArray())->toBe($original->toArray()); + }); + + test('chaining operations', function () { + $carrier = (new Carrier()) + ->with(['step1' => 'data1']) + ->with(['step2' => 'data2']) + ->with(['step1' => 'updated']); // Override step1 + + expect($carrier->toArray())->toBe([ + 'step1' => 'updated', + 'step2' => 'data2', + ]); + }); + + test('works with Span integration', function () { + $spanContext = new SpanContext(); + $span = new Span($spanContext); + + $carrier = Carrier::fromSpan($span) + ->with(['custom' => 'data']); + + $data = $carrier->toArray(); + expect($data)->toHaveKey('custom'); + expect($data)->toHaveKeys(['sentry-trace', 'baggage', 'traceparent']); + + // Test round trip + $json = $carrier->toJson(); + $restored = Carrier::fromJson($json); + expect($restored->getSentryTrace())->toBe($carrier->getSentryTrace()); + expect($restored->get('custom'))->toBe('data'); + }); +}); \ No newline at end of file From 8f8752de9f12863d6af5812e83a058042f2c04ed Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Thu, 4 Sep 2025 08:41:11 +0800 Subject: [PATCH 06/10] fix(tracing): add type hint for payload variable in startRequestTransaction method --- src/sentry/src/Tracing/SpanStarter.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sentry/src/Tracing/SpanStarter.php b/src/sentry/src/Tracing/SpanStarter.php index 33ad2eb20..05d48d603 100644 --- a/src/sentry/src/Tracing/SpanStarter.php +++ b/src/sentry/src/Tracing/SpanStarter.php @@ -62,6 +62,7 @@ protected function startRequestTransaction(ServerRequestInterface $request, ...$ // Rpc Context if ($container->has(RpcContext::class)) { $rpcContext = $container->get(RpcContext::class); + /** @var string|null $payload */ $payload = $rpcContext->get(Constants::TRACE_CARRIER); if ($payload) { $carrier = Carrier::fromJson($payload); From 90af71ad1d596dc733bdf19e337e7e660ee6a518 Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Thu, 4 Sep 2025 08:44:08 +0800 Subject: [PATCH 07/10] refactor(carrier): remove unused methods and clean up imports in Carrier class --- src/sentry/src/Util/Carrier.php | 15 ------ tests/Sentry/CarrierTest.php | 85 +++++++-------------------------- 2 files changed, 18 insertions(+), 82 deletions(-) diff --git a/src/sentry/src/Util/Carrier.php b/src/sentry/src/Util/Carrier.php index 4ae76c7d1..af5f5c01c 100644 --- a/src/sentry/src/Util/Carrier.php +++ b/src/sentry/src/Util/Carrier.php @@ -11,7 +11,6 @@ namespace FriendsOfHyperf\Sentry\Util; -use Hyperf\Collection\Arr; use Hyperf\Contract\Arrayable; use Hyperf\Contract\Jsonable; use JsonException; @@ -100,20 +99,6 @@ public function get(string $key, mixed $default = null): mixed return $this->data[$key] ?? $default; } - public function extract(): array - { - return [ - $this->data['sentry-trace'] ?? $this->data['traceparent'] ?? '', - $this->data['baggage'] ?? '', - $this->data['traceparent'] ?? '', - ]; - } - - public function only(array $keys): array - { - return Arr::only($this->data, $keys); - } - public function jsonSerialize(): mixed { return $this->data; diff --git a/tests/Sentry/CarrierTest.php b/tests/Sentry/CarrierTest.php index fa15721f1..a05c2f139 100644 --- a/tests/Sentry/CarrierTest.php +++ b/tests/Sentry/CarrierTest.php @@ -12,7 +12,6 @@ namespace FriendsOfHyperf\Tests\Sentry; use FriendsOfHyperf\Sentry\Util\Carrier; -use JsonException; use Sentry\Tracing\Span; use Sentry\Tracing\SpanContext; @@ -100,7 +99,7 @@ test('can add span data using withSpan method', function () { $spanContext = new SpanContext(); $span = new Span($spanContext); - + $carrier = new Carrier(['existing' => 'data']); $newCarrier = $carrier->withSpan($span); @@ -158,54 +157,6 @@ }); }); -describe('Data Extraction', function () { - test('extract returns array with tracing data', function () { - $carrier = new Carrier($this->testData); - $extracted = $carrier->extract(); - - expect($extracted)->toBe([ - '12345678901234567890123456789012-1234567890123456-1', - 'sentry-trace_id=12345678901234567890123456789012,sentry-public_key=public_key', - '00-12345678901234567890123456789012-1234567890123456-01', - ]); - }); - - test('extract handles missing sentry-trace gracefully', function () { - $data = $this->testData; - unset($data['sentry-trace']); - $carrier = new Carrier($data); - $extracted = $carrier->extract(); - - expect($extracted[0])->toBe('00-12345678901234567890123456789012-1234567890123456-01'); - }); - - test('extract handles missing traceparent and sentry-trace gracefully', function () { - $data = $this->testData; - unset($data['sentry-trace'], $data['traceparent']); - $carrier = new Carrier($data); - $extracted = $carrier->extract(); - - expect($extracted[0])->toBe(''); - }); - - test('only returns specified keys', function () { - $carrier = new Carrier($this->testData); - $result = $carrier->only(['custom_field', 'publish_time']); - - expect($result)->toBe([ - 'custom_field' => 'custom_value', - 'publish_time' => 1234567890.123, - ]); - }); - - test('only handles nonexistent keys gracefully', function () { - $carrier = new Carrier($this->testData); - $result = $carrier->only(['nonexistent', 'custom_field']); - - expect($result)->toBe(['custom_field' => 'custom_value']); - }); -}); - describe('Serialization', function () { test('implements JsonSerializable', function () { $carrier = new Carrier($this->testData); @@ -220,7 +171,7 @@ test('toJson returns valid JSON string', function () { $carrier = new Carrier($this->testData); $json = $carrier->toJson(); - + expect($json)->toBeString(); expect(json_decode($json, true))->toBe($this->testData); }); @@ -229,7 +180,7 @@ $data = ['key' => 'value with unicode: 中文']; $carrier = new Carrier($data); $json = $carrier->toJson(JSON_UNESCAPED_UNICODE | JSON_PRETTY_PRINT); - + expect($json)->toContain('中文'); expect($json)->toContain("\n"); // Pretty print adds newlines }); @@ -239,7 +190,7 @@ $resource = fopen('php://memory', 'r'); $carrier = new Carrier(['resource' => $resource]); fclose($resource); - + $json = $carrier->toJson(); expect($json)->toBe('{}'); }); @@ -247,7 +198,7 @@ test('__toString returns JSON representation', function () { $carrier = new Carrier($this->testData); $string = (string) $carrier; - + expect($string)->toBe($carrier->toJson()); expect(json_decode($string, true))->toBe($this->testData); }); @@ -255,7 +206,7 @@ test('can be JSON encoded directly', function () { $carrier = new Carrier($this->testData); $json = json_encode($carrier); - + expect(json_decode($json, true))->toBe($this->testData); }); }); @@ -264,7 +215,7 @@ test('handles null values in data', function () { $data = ['key' => null, 'other' => 'value']; $carrier = new Carrier($data); - + expect($carrier->get('key'))->toBeNull(); expect($carrier->has('key'))->toBeFalse(); // isset() returns false for null values expect($carrier->toArray())->toBe($data); @@ -278,7 +229,7 @@ 'float' => 3.14, ]; $carrier = new Carrier($data); - + expect($carrier->get('bool_true'))->toBeTrue(); expect($carrier->get('bool_false'))->toBeFalse(); expect($carrier->get('int'))->toBe(42); @@ -291,7 +242,7 @@ 'object' => (object) ['property' => 'value'], ]; $carrier = new Carrier($data); - + expect($carrier->get('array'))->toBe(['nested', 'array']); expect($carrier->get('object'))->toBeInstanceOf('stdClass'); }); @@ -299,7 +250,7 @@ test('immutability of original carrier in with method', function () { $original = new Carrier(['original' => 'data']); $modified = $original->with(['new' => 'data']); - + expect($original === $modified)->toBeFalse(); expect($original->has('new'))->toBeFalse(); expect($modified->has('new'))->toBeTrue(); @@ -309,11 +260,11 @@ $data = ['nested' => ['array' => 'value']]; $original = new Carrier($data); $modified = $original->with(['other' => 'data']); - + // Modify the nested array in the modified carrier $modifiedData = $modified->toArray(); $modifiedData['nested']['array'] = 'changed'; - + // Original should remain unchanged expect($original->get('nested'))->toBe(['array' => 'value']); }); @@ -324,7 +275,7 @@ $original = new Carrier($this->testData); $json = $original->toJson(); $restored = Carrier::fromJson($json); - + expect($restored->toArray())->toBe($original->toArray()); }); @@ -333,7 +284,7 @@ ->with(['step1' => 'data1']) ->with(['step2' => 'data2']) ->with(['step1' => 'updated']); // Override step1 - + expect($carrier->toArray())->toBe([ 'step1' => 'updated', 'step2' => 'data2', @@ -343,18 +294,18 @@ test('works with Span integration', function () { $spanContext = new SpanContext(); $span = new Span($spanContext); - + $carrier = Carrier::fromSpan($span) ->with(['custom' => 'data']); - + $data = $carrier->toArray(); expect($data)->toHaveKey('custom'); expect($data)->toHaveKeys(['sentry-trace', 'baggage', 'traceparent']); - + // Test round trip $json = $carrier->toJson(); $restored = Carrier::fromJson($json); expect($restored->getSentryTrace())->toBe($carrier->getSentryTrace()); expect($restored->get('custom'))->toBe('data'); }); -}); \ No newline at end of file +}); From d643ac184c9b38f3cdb06bf2b779ffb72a54bead Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Thu, 4 Sep 2025 08:46:18 +0800 Subject: [PATCH 08/10] refactor(carrier): remove withSpan method and related tests for cleaner code --- src/sentry/src/Util/Carrier.php | 9 --------- tests/Sentry/CarrierTest.php | 13 ------------- 2 files changed, 22 deletions(-) diff --git a/src/sentry/src/Util/Carrier.php b/src/sentry/src/Util/Carrier.php index af5f5c01c..3b39c7167 100644 --- a/src/sentry/src/Util/Carrier.php +++ b/src/sentry/src/Util/Carrier.php @@ -65,15 +65,6 @@ public function with(array $data): static return $new; } - public function withSpan(Span $span): static - { - return $this->with([ - 'sentry-trace' => $span->toTraceparent(), - 'baggage' => $span->toBaggage(), - 'traceparent' => $span->toW3CTraceparent(), - ]); - } - public function getSentryTrace(): string { return $this->data['sentry-trace'] ?? ''; diff --git a/tests/Sentry/CarrierTest.php b/tests/Sentry/CarrierTest.php index a05c2f139..5fe0f9bfe 100644 --- a/tests/Sentry/CarrierTest.php +++ b/tests/Sentry/CarrierTest.php @@ -95,19 +95,6 @@ 'key3' => 'new', ]); }); - - test('can add span data using withSpan method', function () { - $spanContext = new SpanContext(); - $span = new Span($spanContext); - - $carrier = new Carrier(['existing' => 'data']); - $newCarrier = $carrier->withSpan($span); - - $data = $newCarrier->toArray(); - expect($data)->toHaveKey('existing'); - expect($data)->toHaveKeys(['sentry-trace', 'baggage', 'traceparent']); - expect($data['existing'])->toBe('data'); - }); }); describe('Data Access', function () { From f884dd4ae194cca8f754072d6a527e6dcbec753d Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Thu, 4 Sep 2025 08:51:26 +0800 Subject: [PATCH 09/10] refactor(async-queue): simplify carrier assignment logic in handleUnserialize method --- .../Tracing/Aspect/AsyncQueueJobMessageAspect.php | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php b/src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php index 0df2d2143..1fc0fe588 100644 --- a/src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php +++ b/src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php @@ -151,15 +151,11 @@ protected function handleUnserialize(ProceedingJoinPoint $proceedingJoinPoint) { /** @var array $data */ $data = $proceedingJoinPoint->arguments['keys']['data'] ?? []; - $carrier = null; - - if (is_array($data)) { - if (array_is_list($data)) { - $carrier = array_last($data); - } elseif (isset($data['job'])) { - $carrier = $data[Constants::TRACE_CARRIER] ?? ''; - } - } + $carrier = match (true) { + is_array($data) && array_is_list($data) => array_last($data), + isset($data['job']) => $data[Constants::TRACE_CARRIER] ?? '', + default => null, + }; /** @var string|null $carrier */ if ($carrier) { From 805b68b164ab66c7ee85942297a9613d796df882 Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Thu, 4 Sep 2025 08:52:23 +0800 Subject: [PATCH 10/10] chore: remove unused file to clean up the repository --- - | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 - diff --git a/- b/- deleted file mode 100644 index e69de29bb..000000000