From 188cc5b893343220fc1eca32da3ffbceb0008fe9 Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Mon, 29 Sep 2025 09:14:18 +0800 Subject: [PATCH 1/2] refactor(sentry): optimize event data setting in EventHandleListener Move data setting from after-event handlers to before-event handlers to: - Improve tracing performance by setting data earlier - Ensure consistent data availability across all event types - Reduce code duplication and improve maintainability Affects command, crontab, AMQP, Kafka, and async queue tracing --- .../Tracing/Listener/EventHandleListener.php | 105 ++++++++---------- 1 file changed, 48 insertions(+), 57 deletions(-) diff --git a/src/sentry/src/Tracing/Listener/EventHandleListener.php b/src/sentry/src/Tracing/Listener/EventHandleListener.php index 08045eeae..dd9b3fa68 100644 --- a/src/sentry/src/Tracing/Listener/EventHandleListener.php +++ b/src/sentry/src/Tracing/Listener/EventHandleListener.php @@ -392,6 +392,10 @@ protected function handleCommandStarting(CommandEvent\BeforeHandle $event): void ->setOp('console.command') ->setDescription($command->getDescription()) ->setOrigin('auto.command') + ->setData([ + 'command.arguments' => (fn () => $this->input->getArguments())->call($command), + 'command.options' => (fn () => $this->input->getOptions())->call($command), + ]) ); Coroutine::inCoroutine() && defer(function () use ($transaction) { @@ -415,10 +419,7 @@ protected function handleCommandFinished(CommandEvent\AfterExecute $event): void $command = $event->getCommand(); $exitCode = (fn () => $this->exitCode ?? SymfonyCommand::SUCCESS)->call($command); - $transaction->setData([ - 'command.arguments' => (fn () => $this->input->getArguments())->call($command), - 'command.options' => (fn () => $this->input->getOptions())->call($command), - ])->setTags([ + $transaction->setTags([ 'command.exit_code' => (string) $exitCode, ]); @@ -521,6 +522,14 @@ protected function handleCrontabTaskStarting(CrontabEvent\BeforeExecute $event): ->setDescription($crontab->getMemo()) ->setOrigin('auto.crontab') ->setSource(TransactionSource::task()) + ->setTags([ + 'crontab.rule' => $crontab->getRule(), + 'crontab.type' => $crontab->getType(), + ]) + ->setData([ + 'crontab.options.is_single' => $crontab->isSingleton(), + 'crontab.options.is_on_one_server' => $crontab->isOnOneServer(), + ]) ); defer(function () use ($transaction) { @@ -540,14 +549,6 @@ protected function handleCrontabTaskFinished(CrontabEvent\FailToExecute|CrontabE return; } - $crontab = $event->crontab; - $transaction->setTags([ - 'crontab.rule' => $crontab->getRule(), - 'crontab.type' => $crontab->getType(), - 'crontab.options.is_single' => $crontab->isSingleton(), - 'crontab.options.is_on_one_server' => $crontab->isOnOneServer(), - ]); - if (method_exists($event, 'getThrowable') && $exception = $event->getThrowable()) { $transaction->setStatus(SpanStatus::internalError()) ->setTags([ @@ -570,6 +571,7 @@ protected function handleAmqpMessageProcessing(AmqpEvent\BeforeConsume $event): return; } + /** @var ConsumerMessage $message */ $message = $event->getMessage(); $carrier = null; @@ -590,6 +592,20 @@ protected function handleAmqpMessageProcessing(AmqpEvent\BeforeConsume $event): ->setOp('queue.process') ->setDescription($message::class) ->setOrigin('auto.amqp') + ->setData([ + '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.retry.count' => 0, + 'messaging.destination.name' => $carrier?->get('destination_name') ?: implode(', ', (array) $message->getRoutingKey()), + 'messaging.amqp.message.type' => $message->getTypeString(), + 'messaging.amqp.message.routing_key' => $message->getRoutingKey(), + 'messaging.amqp.message.exchange' => $message->getExchange(), + 'messaging.amqp.message.queue' => $message->getQueue(), + 'messaging.amqp.message.pool_name' => $message->getPoolName(), + ]) ); defer(function () use ($transaction) { @@ -609,24 +625,7 @@ protected function handleAmqpMessageProcessed(AmqpEvent\AfterConsume|AmqpEvent\F return; } - /** @var null|Carrier $carrier */ - $carrier = Context::get(Constants::TRACE_CARRIER); - - /** @var ConsumerMessage $message */ - $message = $event->getMessage(); $transaction->setData([ - '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.retry.count' => 0, - 'messaging.destination.name' => $carrier?->get('destination_name') ?: implode(', ', (array) $message->getRoutingKey()), - 'messaging.amqp.message.type' => $message->getTypeString(), - 'messaging.amqp.message.routing_key' => $message->getRoutingKey(), - 'messaging.amqp.message.exchange' => $message->getExchange(), - 'messaging.amqp.message.queue' => $message->getQueue(), - 'messaging.amqp.message.pool_name' => $message->getPoolName(), 'messaging.amqp.message.result' => $event instanceof AmqpEvent\AfterConsume ? $event->getResult()->value : 'fail', ]); @@ -672,6 +671,17 @@ protected function handleKafkaMessageProcessing(KafkaEvent\BeforeConsume $event) ->setOp('queue.process') ->setDescription($consumer::class) ->setOrigin('auto.kafka') + ->setData([ + '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.retry.count' => 0, + 'messaging.destination.name' => $carrier?->get('destination_name') ?: (is_array($consumer->getTopic()) ? implode(',', $consumer->getTopic()) : $consumer->getTopic()), + 'messaging.kafka.consumer.group' => $consumer->getGroupId(), + 'messaging.kafka.consumer.pool' => $consumer->getPool(), + ]) ); defer(function () use ($transaction) { @@ -691,21 +701,6 @@ protected function handleKafkaMessageProcessed(KafkaEvent\AfterConsume|KafkaEven return; } - /** @var null|Carrier $carrier */ - $carrier = Context::get(Constants::TRACE_CARRIER); - $consumer = $event->getConsumer(); - $transaction->setData([ - '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.retry.count' => 0, - 'messaging.destination.name' => $carrier?->get('destination_name') ?: (is_array($consumer->getTopic()) ? implode(',', $consumer->getTopic()) : $consumer->getTopic()), - 'messaging.kafka.consumer.group' => $consumer->getGroupId(), - 'messaging.kafka.consumer.pool' => $consumer->getPool(), - ]); - if (method_exists($event, 'getThrowable') && $exception = $event->getThrowable()) { $transaction->setStatus(SpanStatus::internalError()) ->setTags([ @@ -737,7 +732,15 @@ protected function handleAsyncQueueJobProcessing(AsyncQueueEvent\BeforeHandle $e ->setName($job::class) ->setOp('queue.process') ->setDescription('async_queue: ' . $job::class) - ->setOrigin('auto.async_queue') + ->setOrigin('auto.async_queue')->setData([ + 'messaging.system' => 'async_queue', + '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.retry.count' => $event->getMessage()->getAttempts(), + 'messaging.destination.name' => $carrier?->get('destination_name') ?: 'unknown queue', + ]) ); defer(function () use ($transaction) { @@ -757,18 +760,6 @@ protected function handleAsyncQueueJobProcessed(AsyncQueueEvent\AfterHandle|Asyn return; } - /** @var null|Carrier $carrier */ - $carrier = Context::get(Constants::TRACE_CARRIER, null, Coroutine::parentId()); - $transaction->setData([ - 'messaging.system' => 'async_queue', - '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.retry.count' => $event->getMessage()->getAttempts(), - 'messaging.destination.name' => $carrier?->get('destination_name') ?: 'unknown queue', - ]); - if (method_exists($event, 'getThrowable') && $exception = $event->getThrowable()) { $transaction->setStatus(SpanStatus::internalError()) ->setTags([ From 1ba9136400d33ce6d1d44710062e10a7594aafc7 Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Mon, 29 Sep 2025 09:16:28 +0800 Subject: [PATCH 2/2] =?UTF-8?q?=E4=BF=AE=E5=A4=8D=EF=BC=9A=E6=B3=A8?= =?UTF-8?q?=E9=87=8A=E6=8E=89=20SentrySdk=20=E7=B1=BB=E7=9A=84=E5=AF=BC?= =?UTF-8?q?=E5=85=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/sentry/src/Aspect/CoroutineAspect.php | 2 +- src/sentry/src/Tracing/Aspect/CoroutineAspect.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/src/Aspect/CoroutineAspect.php b/src/sentry/src/Aspect/CoroutineAspect.php index da7c7cc18..ff1a0da76 100644 --- a/src/sentry/src/Aspect/CoroutineAspect.php +++ b/src/sentry/src/Aspect/CoroutineAspect.php @@ -25,7 +25,7 @@ class CoroutineAspect extends AbstractAspect ]; protected array $keys = [ - SentrySdk::class, + // SentrySdk::class, \Psr\Http\Message\ServerRequestInterface::class, ]; diff --git a/src/sentry/src/Tracing/Aspect/CoroutineAspect.php b/src/sentry/src/Tracing/Aspect/CoroutineAspect.php index 2a0b4b49e..8af31a9a5 100644 --- a/src/sentry/src/Tracing/Aspect/CoroutineAspect.php +++ b/src/sentry/src/Tracing/Aspect/CoroutineAspect.php @@ -32,7 +32,7 @@ class CoroutineAspect extends AbstractAspect ]; protected array $keys = [ - SentrySdk::class, + // SentrySdk::class, \Psr\Http\Message\ServerRequestInterface::class, ];