From c8e0ef0e28227943923195d286da2660c2fe4cc8 Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Thu, 25 Sep 2025 22:41:16 +0800 Subject: [PATCH] refactor(sentry): centralize coroutine ID handling in tracing system - Remove duplicate coroutine.id data from individual tracing aspects - Centralize coroutine ID handling in Tracer class for consistency - Add default transaction metadata (status, source, start timestamp) in Tracer - Remove unused Coroutine imports from aspect files - Improve code maintainability by reducing duplication across aspects This change ensures coroutine IDs are consistently added to all transactions and spans through the central Tracer class rather than duplicating the logic in each individual aspect. --- .../src/Tracing/Aspect/AmqpProducerAspect.php | 2 -- .../Aspect/AsyncQueueJobMessageAspect.php | 2 -- src/sentry/src/Tracing/Aspect/CacheAspect.php | 2 -- .../src/Tracing/Aspect/CoordinatorAspect.php | 2 -- .../src/Tracing/Aspect/CoroutineAspect.php | 1 - src/sentry/src/Tracing/Aspect/DbAspect.php | 2 -- .../src/Tracing/Aspect/ElasticsearchAspect.php | 2 -- .../src/Tracing/Aspect/FilesystemAspect.php | 3 +-- src/sentry/src/Tracing/Aspect/GrpcAspect.php | 2 -- .../Tracing/Aspect/GuzzleHttpClientAspect.php | 1 - .../src/Tracing/Aspect/KafkaProducerAspect.php | 4 +--- src/sentry/src/Tracing/Aspect/RedisAspect.php | 2 -- src/sentry/src/Tracing/Aspect/RpcAspect.php | 2 -- .../Tracing/Aspect/TraceAnnotationAspect.php | 3 +-- .../Tracing/Listener/EventHandleListener.php | 12 ------------ src/sentry/src/Tracing/Tracer.php | 18 ++++++++++++++++++ 16 files changed, 21 insertions(+), 39 deletions(-) diff --git a/src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php b/src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php index 73768b2d6..6c6e7f3da 100644 --- a/src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php +++ b/src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php @@ -16,7 +16,6 @@ use FriendsOfHyperf\Sentry\Util\Carrier; use Hyperf\Amqp\Annotation\Producer; use Hyperf\Amqp\Message\ProducerMessage; -use Hyperf\Coroutine\Coroutine; use Hyperf\Di\Annotation\AnnotationCollector; use Hyperf\Di\Aop\AbstractAspect; use Hyperf\Di\Aop\ProceedingJoinPoint; @@ -101,7 +100,6 @@ function (Scope $scope) use ($proceedingJoinPoint, $producerMessage, $messageId, ->setDescription(sprintf('%s::%s()', $proceedingJoinPoint->className, $proceedingJoinPoint->methodName)) ->setOrigin('auto.amqp') ->setData([ - 'coroutine.id' => Coroutine::id(), 'messaging.system' => 'amqp', 'messaging.operation' => 'publish', 'messaging.message.id' => $messageId, diff --git a/src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php b/src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php index 39835f086..84a71d84b 100644 --- a/src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php +++ b/src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php @@ -16,7 +16,6 @@ use FriendsOfHyperf\Sentry\Util\Carrier; use Hyperf\AsyncQueue\Driver\RedisDriver; use Hyperf\Context\Context; -use Hyperf\Coroutine\Coroutine; use Hyperf\Di\Aop\AbstractAspect; use Hyperf\Di\Aop\ProceedingJoinPoint; use Sentry\State\Scope; @@ -81,7 +80,6 @@ public function handlePush(ProceedingJoinPoint $proceedingJoinPoint) $destinationName = Context::get('sentry.messaging.destination.name', 'default'); $bodySize = (fn ($job) => strlen($this->packer->pack($job)))->call($driver, $job); $data = [ - 'coroutine.id' => Coroutine::id(), 'messaging.system' => 'async_queue', 'messaging.operation' => 'publish', 'messaging.message.id' => $messageId, diff --git a/src/sentry/src/Tracing/Aspect/CacheAspect.php b/src/sentry/src/Tracing/Aspect/CacheAspect.php index 252d386e8..20ce2be15 100644 --- a/src/sentry/src/Tracing/Aspect/CacheAspect.php +++ b/src/sentry/src/Tracing/Aspect/CacheAspect.php @@ -12,7 +12,6 @@ namespace FriendsOfHyperf\Sentry\Tracing\Aspect; use FriendsOfHyperf\Sentry\Switcher; -use Hyperf\Coroutine\Coroutine; use Hyperf\Di\Aop\AbstractAspect; use Hyperf\Di\Aop\ProceedingJoinPoint; use Sentry\State\Scope; @@ -92,7 +91,6 @@ function (Scope $scope) use ($proceedingJoinPoint, $method) { ->setDescription(implode(', ', $keys)) ->setOrigin('auto.cache') ->setData([ - 'coroutine.id' => Coroutine::id(), 'cache.key' => $keys, 'cache.ttl' => $arguments['ttl'] ?? null, 'item_size' => match (true) { diff --git a/src/sentry/src/Tracing/Aspect/CoordinatorAspect.php b/src/sentry/src/Tracing/Aspect/CoordinatorAspect.php index 77e01b329..8e9ff3d94 100644 --- a/src/sentry/src/Tracing/Aspect/CoordinatorAspect.php +++ b/src/sentry/src/Tracing/Aspect/CoordinatorAspect.php @@ -13,7 +13,6 @@ use FriendsOfHyperf\Sentry\Switcher; use Hyperf\Coordinator\Coordinator; -use Hyperf\Coroutine\Coroutine; use Hyperf\Di\Aop\AbstractAspect; use Hyperf\Di\Aop\ProceedingJoinPoint; use Sentry\State\Scope; @@ -46,7 +45,6 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint) ->setDescription(sprintf('%s::%s(%s)', $proceedingJoinPoint->className, $proceedingJoinPoint->methodName, $timeout)) ->setOrigin('auto.coordinator') ->setData([ - 'coroutine.id' => Coroutine::id(), 'timeout' => $timeout, ]) ); diff --git a/src/sentry/src/Tracing/Aspect/CoroutineAspect.php b/src/sentry/src/Tracing/Aspect/CoroutineAspect.php index aaab8122e..e4c8494ee 100644 --- a/src/sentry/src/Tracing/Aspect/CoroutineAspect.php +++ b/src/sentry/src/Tracing/Aspect/CoroutineAspect.php @@ -96,7 +96,6 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint) ->setOp('coroutine.execute') ->setDescription($callingOnFunction) ->setOrigin('auto.coroutine') - ->setData(['coroutine.id' => Co::id()]) ); defer(function () use ($coTransaction) { diff --git a/src/sentry/src/Tracing/Aspect/DbAspect.php b/src/sentry/src/Tracing/Aspect/DbAspect.php index 48a1f3ac9..22d166d23 100644 --- a/src/sentry/src/Tracing/Aspect/DbAspect.php +++ b/src/sentry/src/Tracing/Aspect/DbAspect.php @@ -13,7 +13,6 @@ use FriendsOfHyperf\Sentry\Switcher; use FriendsOfHyperf\Sentry\Util\SqlParser; -use Hyperf\Coroutine\Coroutine; use Hyperf\DB\DB; use Hyperf\DB\Pool\PoolFactory; use Hyperf\Di\Aop\AbstractAspect; @@ -64,7 +63,6 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint) $table = $sqlParse['table']; $operation = $sqlParse['operation']; $data = [ - 'coroutine.id' => Coroutine::id(), 'db.system' => $driver, 'db.name' => $database, 'db.collection.name' => $table, diff --git a/src/sentry/src/Tracing/Aspect/ElasticsearchAspect.php b/src/sentry/src/Tracing/Aspect/ElasticsearchAspect.php index 069b52c2a..8aaa016af 100644 --- a/src/sentry/src/Tracing/Aspect/ElasticsearchAspect.php +++ b/src/sentry/src/Tracing/Aspect/ElasticsearchAspect.php @@ -12,7 +12,6 @@ namespace FriendsOfHyperf\Sentry\Tracing\Aspect; use FriendsOfHyperf\Sentry\Switcher; -use Hyperf\Coroutine\Coroutine; use Hyperf\Di\Aop\AbstractAspect; use Hyperf\Di\Aop\ProceedingJoinPoint; use Sentry\State\Scope; @@ -76,7 +75,6 @@ function (Scope $scope) use ($proceedingJoinPoint) { ->setDescription(sprintf('%s::%s()', $proceedingJoinPoint->className, $proceedingJoinPoint->methodName)) ->setOrigin('auto.elasticsearch') ->setData([ - 'coroutine.id' => Coroutine::id(), 'db.system' => 'elasticsearch', 'db.operation.name' => $proceedingJoinPoint->methodName, 'arguments' => (string) json_encode($proceedingJoinPoint->arguments['keys'], JSON_UNESCAPED_UNICODE), diff --git a/src/sentry/src/Tracing/Aspect/FilesystemAspect.php b/src/sentry/src/Tracing/Aspect/FilesystemAspect.php index c1ce542da..beeed0287 100644 --- a/src/sentry/src/Tracing/Aspect/FilesystemAspect.php +++ b/src/sentry/src/Tracing/Aspect/FilesystemAspect.php @@ -12,7 +12,6 @@ namespace FriendsOfHyperf\Sentry\Tracing\Aspect; use FriendsOfHyperf\Sentry\Aspect\FilesystemAspect as BaseFilesystemAspect; -use Hyperf\Coroutine\Coroutine; use Hyperf\Di\Aop\ProceedingJoinPoint; use Override; use Sentry\State\Scope; @@ -37,7 +36,7 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint) ->setOp($op) ->setDescription($description) ->setOrigin('auto.filesystem') - ->setData(['coroutine.id' => Coroutine::id()] + $data) + ->setData($data) ); } } diff --git a/src/sentry/src/Tracing/Aspect/GrpcAspect.php b/src/sentry/src/Tracing/Aspect/GrpcAspect.php index 5d302c59e..821fa0f68 100644 --- a/src/sentry/src/Tracing/Aspect/GrpcAspect.php +++ b/src/sentry/src/Tracing/Aspect/GrpcAspect.php @@ -12,7 +12,6 @@ namespace FriendsOfHyperf\Sentry\Tracing\Aspect; use FriendsOfHyperf\Sentry\Switcher; -use Hyperf\Coroutine\Coroutine; use Hyperf\Di\Aop\AbstractAspect; use Hyperf\Di\Aop\ProceedingJoinPoint; use Sentry\SentrySdk; @@ -41,7 +40,6 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint) $method = $proceedingJoinPoint->arguments['keys']['method']; $options = $proceedingJoinPoint->arguments['keys']['options']; $data = [ - 'coroutine.id' => Coroutine::id(), 'grpc.method' => $method, 'grpc.options' => $options, ]; diff --git a/src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php b/src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php index 8dfe48e67..6564cdbb9 100644 --- a/src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php +++ b/src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php @@ -105,7 +105,6 @@ function (Scope $scope) use ($proceedingJoinPoint, $options, $guzzleConfig) { 'http.request.user_agent' => $request->getHeaderLine('User-Agent'), // updated key for consistency 'http.request.headers' => $request->getHeaders(), // Other - 'coroutine.id' => Coroutine::id(), 'http.system' => 'guzzle', 'http.guzzle.config' => $guzzleConfig, 'http.guzzle.options' => $options ?? [], diff --git a/src/sentry/src/Tracing/Aspect/KafkaProducerAspect.php b/src/sentry/src/Tracing/Aspect/KafkaProducerAspect.php index 39a96f191..0fdee8e46 100644 --- a/src/sentry/src/Tracing/Aspect/KafkaProducerAspect.php +++ b/src/sentry/src/Tracing/Aspect/KafkaProducerAspect.php @@ -14,7 +14,6 @@ use FriendsOfHyperf\Sentry\Constants; use FriendsOfHyperf\Sentry\Switcher; use FriendsOfHyperf\Sentry\Util\Carrier; -use Hyperf\Coroutine\Coroutine; use Hyperf\Di\Aop\AbstractAspect; use Hyperf\Di\Aop\ProceedingJoinPoint; use longlang\phpkafka\Producer\ProduceMessage; @@ -84,7 +83,6 @@ function (Scope $scope) use ($proceedingJoinPoint, $messageId, $destinationName, ->setDescription(sprintf('%s::%s()', $proceedingJoinPoint->className, $proceedingJoinPoint->methodName)) ->setOrigin('auto.kafka') ->setData([ - 'coroutine.id' => Coroutine::id(), 'messaging.system' => 'kafka', 'messaging.operation' => 'publish', 'messaging.message.id' => $messageId, @@ -124,7 +122,7 @@ function (Scope $scope) use ($proceedingJoinPoint, $messages) { ->setOp('queue.publish') ->setDescription(sprintf('%s::%s()', $proceedingJoinPoint->className, $proceedingJoinPoint->methodName)) ->setOrigin('auto.kafka') - ->setData(['coroutine.id' => Coroutine::id()]) + ->setData(['message.count' => count($messages)]) ); } } diff --git a/src/sentry/src/Tracing/Aspect/RedisAspect.php b/src/sentry/src/Tracing/Aspect/RedisAspect.php index 34c454717..d02511d7c 100644 --- a/src/sentry/src/Tracing/Aspect/RedisAspect.php +++ b/src/sentry/src/Tracing/Aspect/RedisAspect.php @@ -13,7 +13,6 @@ use FriendsOfHyperf\Sentry\Switcher; use FriendsOfHyperf\Support\RedisCommand; -use Hyperf\Coroutine\Coroutine; use Hyperf\Di\Aop\AbstractAspect; use Hyperf\Di\Aop\ProceedingJoinPoint; use Hyperf\Redis\Event\CommandExecuted; @@ -59,7 +58,6 @@ class_exists(CommandExecuted::class) $pool = $this->container->get(PoolFactory::class)->getPool($poolName); $config = (fn () => $this->config ?? [])->call($pool); $data = [ - 'coroutine.id' => Coroutine::id(), 'db.system' => 'redis', 'db.statement' => (new RedisCommand($arguments['name'], $arguments['arguments']))->__toString(), 'db.redis.connection' => $poolName, diff --git a/src/sentry/src/Tracing/Aspect/RpcAspect.php b/src/sentry/src/Tracing/Aspect/RpcAspect.php index 868ed604b..850aaf373 100644 --- a/src/sentry/src/Tracing/Aspect/RpcAspect.php +++ b/src/sentry/src/Tracing/Aspect/RpcAspect.php @@ -16,7 +16,6 @@ use FriendsOfHyperf\Sentry\Util\Carrier; use Hyperf\Context\Context; use Hyperf\Contract\ConfigInterface; -use Hyperf\Coroutine\Coroutine; use Hyperf\Di\Aop\AbstractAspect; use Hyperf\Di\Aop\ProceedingJoinPoint; use Hyperf\Rpc; @@ -81,7 +80,6 @@ private function handleGenerateRpcPath(ProceedingJoinPoint $proceedingJoinPoint) ->setDescription($path) ->setOrigin('auto.rpc') ->setData([ - 'coroutine.id' => Coroutine::id(), 'rpc.system' => $system, 'rpc.service' => $service, 'rpc.method' => $proceedingJoinPoint->arguments['keys']['methodName'] ?? '', diff --git a/src/sentry/src/Tracing/Aspect/TraceAnnotationAspect.php b/src/sentry/src/Tracing/Aspect/TraceAnnotationAspect.php index db90b6c7b..c5b7fdcda 100644 --- a/src/sentry/src/Tracing/Aspect/TraceAnnotationAspect.php +++ b/src/sentry/src/Tracing/Aspect/TraceAnnotationAspect.php @@ -13,7 +13,6 @@ use FriendsOfHyperf\Sentry\Switcher; use FriendsOfHyperf\Sentry\Tracing\Annotation\Trace; -use Hyperf\Coroutine\Coroutine; use Hyperf\Di\Aop\AbstractAspect; use Hyperf\Di\Aop\ProceedingJoinPoint; use Sentry\State\Scope; @@ -42,7 +41,7 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint) return $proceedingJoinPoint->process(); } - $data = ['coroutine.id' => Coroutine::id()]; + $data = []; $methodName = $proceedingJoinPoint->methodName; if (in_array($methodName, ['__call', '__callStatic'])) { diff --git a/src/sentry/src/Tracing/Listener/EventHandleListener.php b/src/sentry/src/Tracing/Listener/EventHandleListener.php index c8923807d..d6f2cfeb2 100644 --- a/src/sentry/src/Tracing/Listener/EventHandleListener.php +++ b/src/sentry/src/Tracing/Listener/EventHandleListener.php @@ -171,7 +171,6 @@ protected function handleDbQueryExecuted(DbEvent\QueryExecuted $event): void } $data = [ - 'coroutine.id' => Coroutine::id(), 'db.system' => $event->connection->getDriverName(), 'db.name' => $event->connection->getDatabaseName(), ]; @@ -228,7 +227,6 @@ protected function handleDbTransactionBeginning(DbEvent\TransactionBeginning $ev ->setOrigin('auto.db') ->setStartTimestamp(microtime(true)) ->setData([ - 'coroutine.id' => Coroutine::id(), 'db.system' => $event->connection->getDriverName(), 'db.name' => $event->connection->getDatabaseName(), 'db.pool.name' => $event->connectionName, @@ -394,8 +392,6 @@ protected function handleCommandStarting(CommandEvent\BeforeHandle $event): void ->setOp('console.command') ->setDescription($command->getDescription()) ->setOrigin('auto.command') - ->setSource(TransactionSource::custom()) - ->setData(['coroutine.id' => Coroutine::id()]) ); } @@ -484,7 +480,6 @@ function (Scope $scope) use ($event) { ->setOp('db.redis') ->setDescription($redisStatement) ->setData([ - 'coroutine.id' => Coroutine::id(), 'db.system' => 'redis', 'db.statement' => $redisStatement, 'db.redis.connection' => $event->connectionName, @@ -516,7 +511,6 @@ protected function handleCrontabTaskStarting(CrontabEvent\BeforeExecute $event): ->setDescription($crontab->getMemo()) ->setOrigin('auto.crontab') ->setSource(TransactionSource::task()) - ->setData(['coroutine.id' => Coroutine::id()]) ); } @@ -584,8 +578,6 @@ protected function handleAmqpMessageProcessing(AmqpEvent\BeforeConsume $event): ->setOp('queue.process') ->setDescription($message::class) ->setOrigin('auto.amqp') - ->setSource(TransactionSource::custom()) - ->setData(['coroutine.id' => Coroutine::id()]) ); } @@ -666,8 +658,6 @@ protected function handleKafkaMessageProcessing(KafkaEvent\BeforeConsume $event) ->setOp('queue.process') ->setDescription($consumer::class) ->setOrigin('auto.kafka') - ->setSource(TransactionSource::custom()) - ->setData(['coroutine.id' => Coroutine::id()]) ); } @@ -732,8 +722,6 @@ protected function handleAsyncQueueJobProcessing(AsyncQueueEvent\BeforeHandle $e ->setOp('queue.process') ->setDescription('async_queue: ' . $job::class) ->setOrigin('auto.async_queue') - ->setSource(TransactionSource::custom()) - ->setData(['coroutine.id' => Coroutine::id()]) ); } diff --git a/src/sentry/src/Tracing/Tracer.php b/src/sentry/src/Tracing/Tracer.php index c74b4b204..a72015471 100644 --- a/src/sentry/src/Tracing/Tracer.php +++ b/src/sentry/src/Tracing/Tracer.php @@ -12,6 +12,7 @@ namespace FriendsOfHyperf\Sentry\Tracing; use FriendsOfHyperf\Sentry\Switcher; +use Hyperf\Engine\Coroutine; use Sentry\SentrySdk; use Sentry\State\HubInterface; use Sentry\State\Scope; @@ -19,6 +20,7 @@ use Sentry\Tracing\SpanStatus; use Sentry\Tracing\Transaction; use Sentry\Tracing\TransactionContext; +use Sentry\Tracing\TransactionSource; use Throwable; use function Hyperf\Tappable\tap; @@ -39,6 +41,20 @@ public function startTransaction(TransactionContext $transactionContext, array $ tap(clone SentrySdk::getCurrentHub(), fn (HubInterface $hub) => $hub->pushScope()) ); + $transactionContext->setData(['coroutine.id' => Coroutine::id()] + $transactionContext->getData()); + + if ($transactionContext->getStartTimestamp() === null) { + $transactionContext->setStartTimestamp(microtime(true)); + } + + if ($transactionContext->getStatus() === null) { + $transactionContext->setStatus(SpanStatus::ok()); + } + + if ($transactionContext->getMetadata()->getSource() === null) { + $transactionContext->setSource(TransactionSource::custom()); + } + $transaction = $hub->startTransaction($transactionContext, $customSamplingContext); $hub->setSpan($transaction); @@ -67,6 +83,8 @@ public function trace(callable $trace, SpanContext $context) $context->setStartTimestamp(microtime(true)); } + $context->setData(['coroutine.id' => Coroutine::id()] + $context->getData()); + return trace( function (Scope $scope) use ($trace, $isTracingExtraTagEnabled) { try {