Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,6 @@ protected function produceMessage(ProceedingJoinPoint $proceedingJoinPoint)
$this->properties['application_headers']->set(Constants::TRACE_CARRIER, $carrier);
})->call($producerMessage);

return tap($proceedingJoinPoint->process(), fn () => $span->finish());
return tap($proceedingJoinPoint->process(), fn () => $span->setOrigin('auto.amqp')->finish());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public function handlePush(ProceedingJoinPoint $proceedingJoinPoint)
return $proceedingJoinPoint->process();
} catch (Throwable) {
} finally {
$span->finish();
$span->setOrigin('auto.queue')->finish();
}
}

Expand Down
116 changes: 33 additions & 83 deletions src/sentry/src/Tracing/Aspect/CacheAspect.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
use Hyperf\Di\Aop\AbstractAspect;
use Hyperf\Di\Aop\ProceedingJoinPoint;
use Sentry\SentrySdk;
use Sentry\Tracing\Span;

use function Hyperf\Tappable\tap;

Expand Down Expand Up @@ -53,99 +52,50 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint)
try {
$method = $proceedingJoinPoint->methodName;
$op = match ($method) {
'set' => 'cache.put',
'get', 'fetch' => 'cache.get',
'delete' => 'cache.remove',
'setMultiple' => 'cache.put',
'getMultiple' => 'cache.get',
'deleteMultiple' => 'cache.remove',
'set', 'setMultiple' => 'cache.put',
'get', 'fetch', 'getMultiple' => 'cache.get',
'delete', 'deleteMultiple' => 'cache.remove',
'clear' => 'cache.flush',
default => 'cache',
};

$arguments = $proceedingJoinPoint->arguments['keys'] ?? [];

/** @var string|string[] $key */
$key = match ($method) {
'set', 'get', 'delete', 'setMultiple', 'getMultiple', 'deleteMultiple' => $proceedingJoinPoint->arguments['order'][0] ?? 'unknown',
default => '',
[$key, $ttl] = match ($method) {
'set', 'get', 'delete' => [
$arguments['key'] ?? 'unknown',
$arguments['ttl'] ?? null,
],
'setMultiple' => [
array_keys($arguments['values'] ?? []),
$arguments['ttl'] ?? null,
],
'getMultiple', 'deleteMultiple' => [
$arguments['keys'] ?? [],
$arguments['ttl'] ?? null,
],
default => ['', null],
};

$span = $this->startSpan(op: $op, description: $key);

return tap($proceedingJoinPoint->process(), function ($value) use ($span, $method, $key) {
match ($method) {
'set', => $this->handleSet($span, $key, $value),
'get', 'fetch' => $this->handleGet($span, $key, $value),
'delete' => $this->handleDelete($span, $key, $value),
'setMultiple' => $this->handleSetMultiple($span, $key, $value),
'getMultiple' => $this->handleGetMultiple($span, $key, $value),
'deleteMultiple' => $this->handleDeleteMultiple($span, $key, $value),
'clear' => $this->handleClear($span),
default => null,
$span = $this->startSpan(
op: $op,
description: implode(', ', (array) $key),
asParent: true
);

return tap($proceedingJoinPoint->process(), function ($result) use ($span, $method, $key, $ttl) {
$data = match ($method) {
'set', 'setMultiple' => ['cache.key' => $key, 'cache.ttl' => $ttl],
'delete', 'deleteMultiple' => ['cache.key' => $key],
'get', 'fetch' => ['cache.key' => $key, 'cache.hit' => ! is_null($result)],
'getMultiple' => ['cache.key' => $key, 'cache.hit' => ! empty($result)],
default => [],
};
$span->setOrigin('auto.cache')->setData($data)->finish();
});
} finally {
SentrySdk::getCurrentHub()->setSpan($parent);
}
}

private function handleSet(Span $span, string $key, mixed $value)
{
$span
->setData([
'cache.key' => $key,
])
->finish();
}

private function handleGet(Span $span, string $key, mixed $value)
{
$span
->setData([
'cache.key' => $key,
'cache.hit' => ! is_null($value),
])
->finish();
}

private function handleDelete(Span $span, string $key, mixed $value)
{
$span
->setData([
'cache.key' => $key,
])
->finish();
}

private function handleSetMultiple(Span $span, array $keys, array $values)
{
$span
->setData([
'cache.key' => $keys,
])
->finish();
}

private function handleGetMultiple(Span $span, array $keys, array $values)
{
$span
->setData([
'cache.key' => $keys,
'cache.hit' => ! empty($values),
])
->finish();
}

private function handleDeleteMultiple(Span $span, array $keys, array $values)
{
$span
->setData([
'cache.key' => $keys,
])
->finish();
}

private function handleClear(Span $span)
{
$span->finish();
}
}
3 changes: 1 addition & 2 deletions src/sentry/src/Tracing/Aspect/CoordinatorAspect.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint)
}
throw $exception;
} finally {
$span?->setData($data);
$span?->finish(microtime(true));
$span?->setOrigin('auto.coordinator')->setData($data)->finish(microtime(true));
}
}
}
4 changes: 2 additions & 2 deletions src/sentry/src/Tracing/Aspect/CoroutineAspect.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,11 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint)

throw $exception;
} finally {
$transaction->setData($data);
$transaction->setOrigin('auto.coroutine')->setData($data);
}
};

$parent->finish();
$parent->setOrigin('auto.coroutine')->finish();

return $proceedingJoinPoint->process();
}
Expand Down
3 changes: 1 addition & 2 deletions src/sentry/src/Tracing/Aspect/DbAspect.php
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,7 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint)

throw $exception;
} finally {
$span->setData($data);
$span->finish();
$span->setOrigin('auto.db')->setData($data)->finish();
}

return $result;
Expand Down
3 changes: 1 addition & 2 deletions src/sentry/src/Tracing/Aspect/ElasticsearchAspect.php
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,7 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint)

throw $exception;
} finally {
$span->setData($data);
$span->finish();
$span->setOrigin('auto.elasticsearch')->setData($data)->finish();
}

return $result;
Expand Down
17 changes: 8 additions & 9 deletions src/sentry/src/Tracing/Aspect/GrpcAspect.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,21 +76,20 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint)
}
}
} catch (Throwable $exception) {
$span->setStatus(SpanStatus::internalError());
$span->setTags([
'error' => true,
'exception.class' => $exception::class,
'exception.message' => $exception->getMessage(),
'exception.code' => $exception->getCode(),
]);
$span?->setStatus(SpanStatus::internalError())
->setTags([
'error' => true,
'exception.class' => $exception::class,
'exception.message' => $exception->getMessage(),
'exception.code' => $exception->getCode(),
]);
if ($this->switcher->isTracingExtraTagEnable('exception.stack_trace')) {
$data['exception.stack_trace'] = (string) $exception;
}

throw $exception;
} finally {
$span->setData($data);
$span->finish();
$span?->setOrigin('auto.grpc')->setData($data)->finish();
}
return $result;
}
Expand Down
3 changes: 1 addition & 2 deletions src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,7 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint)
}
}

$span->setData($data);
$span->finish();
$span->setOrigin('auto.http.client')->setData($data)->finish();

if (is_callable($onStats)) {
($onStats)($stats);
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/src/Tracing/Aspect/KafkaProducerAspect.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ protected function sendAsync(ProceedingJoinPoint $proceedingJoinPoint)
->setValue($carrier);
$proceedingJoinPoint->arguments['keys']['headers'] = $headers;

return tap($proceedingJoinPoint->process(), fn () => $span->finish());
return tap($proceedingJoinPoint->process(), fn () => $span->setOrigin('auto.kafka')->finish());
}

protected function sendBatchAsync(ProceedingJoinPoint $proceedingJoinPoint)
Expand All @@ -96,6 +96,6 @@ protected function sendBatchAsync(ProceedingJoinPoint $proceedingJoinPoint)
)->call($message);
}

return tap($proceedingJoinPoint->process(), fn () => $span->finish());
return tap($proceedingJoinPoint->process(), fn () => $span->setOrigin('auto.kafka')->finish());
}
}
3 changes: 1 addition & 2 deletions src/sentry/src/Tracing/Aspect/RedisAspect.php
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,7 @@ class_exists(CommandExecuted::class)

throw $exception;
} finally {
$span->setData($data);
$span->finish();
$span->setOrigin('auto.cache.redis')->setData($data)->finish();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

严重:finally 对 $span 的未判空调用可能 fatal

startSpan() 可能返回 null;即使在 try 内 if (! $span) return $result;,finally 仍会执行,现有代码会在 null 上调用方法。应使用 nullsafe。

建议:

-            $span->setOrigin('auto.cache.redis')->setData($data)->finish();
+            $span?->setOrigin('auto.cache.redis')->setData($data)->finish();
🤖 Prompt for AI Agents
In src/sentry/src/Tracing/Aspect/RedisAspect.php around line 113, the finally
block calls methods on $span without null-checking, which can cause a fatal if
startSpan() returned null; update the finally to use the nullsafe operator (e.g.
$span?->setOrigin(...)?->setData(...)?->finish()) or guard with an if ($span)
check so methods are only invoked when $span is not null.

}

return $result;
Expand Down
21 changes: 8 additions & 13 deletions src/sentry/src/Tracing/Aspect/RpcAspect.php
Original file line number Diff line number Diff line change
Expand Up @@ -124,29 +124,24 @@ private function handleSend(ProceedingJoinPoint $proceedingJoinPoint)
try {
$result = $proceedingJoinPoint->process();

if (! $span) {
return $result;
}

if ($this->switcher->isTracingExtraTagEnable('rpc.result')) {
$data['rpc.result'] = $result;
}
} catch (Throwable $exception) {
$span->setStatus(SpanStatus::internalError());
$span->setTags([
'error' => true,
'exception.class' => $exception::class,
'exception.message' => $exception->getMessage(),
'exception.code' => $exception->getCode(),
]);
$span?->setStatus(SpanStatus::internalError())
->setTags([
'error' => true,
'exception.class' => $exception::class,
'exception.message' => $exception->getMessage(),
'exception.code' => $exception->getCode(),
]);
if ($this->switcher->isTracingExtraTagEnable('exception.stack_trace')) {
$data['exception.stack_trace'] = (string) $exception;
}

throw $exception;
} finally {
$span->setData($data);
$span->finish();
$span?->setOrigin('auto.rpc')->setData($data)->finish();

Context::destroy(static::SPAN);
Context::destroy(static::DATA);
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/src/Tracing/Listener/TracingAmqpListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,10 @@ protected function finishTransaction(AfterConsume|FailToConsume $event): void
}
}

$transaction->setData($data);
$transaction->setTags($tags);
$transaction->setOrigin('auto.amqp')->setData($data)->setTags($tags);

SentrySdk::getCurrentHub()->setSpan($transaction);

$transaction->finish(microtime(true));
}
}
4 changes: 2 additions & 2 deletions src/sentry/src/Tracing/Listener/TracingAsyncQueueListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,10 @@ protected function finishTransaction(AfterHandle|RetryHandle|FailedHandle $event
}
}

$transaction->setData($data);
$transaction->setTags($tags);
$transaction->setOrigin('auto.queue')->setData($data)->setTags($tags);

SentrySdk::getCurrentHub()->setSpan($transaction);

$transaction->finish(microtime(true));
}
}
9 changes: 6 additions & 3 deletions src/sentry/src/Tracing/Listener/TracingCommandListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,14 @@ protected function finishTransaction(AfterExecute $event): void
}
}

$transaction->setStatus($exitCode == SymfonyCommand::SUCCESS ? SpanStatus::ok() : SpanStatus::internalError());
$transaction->setData($data);
$transaction->setTags($tags);
$transaction
->setOrigin('auto.command')
->setStatus($exitCode == SymfonyCommand::SUCCESS ? SpanStatus::ok() : SpanStatus::internalError())
->setData($data)
->setTags($tags);
Comment on lines +117 to +121
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

最终 setStatus 可能覆盖异常状态

若前面已捕获异常并 setStatus(internalError),但 exitCode 为 SUCCESS,这里的链式 setStatus 会把状态改回 ok,导致误报。建议综合异常与退出码计算最终状态:

-        $transaction
-            ->setOrigin('auto.command')
-            ->setStatus($exitCode == SymfonyCommand::SUCCESS ? SpanStatus::ok() : SpanStatus::internalError())
-            ->setData($data)
-            ->setTags($tags);
+        $finalStatus = (isset($exception) && $exception) || $exitCode !== SymfonyCommand::SUCCESS
+            ? SpanStatus::internalError()
+            : SpanStatus::ok();
+        $transaction
+            ->setOrigin('auto.command')
+            ->setStatus($finalStatus)
+            ->setData($data)
+            ->setTags($tags);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
$transaction
->setOrigin('auto.command')
->setStatus($exitCode == SymfonyCommand::SUCCESS ? SpanStatus::ok() : SpanStatus::internalError())
->setData($data)
->setTags($tags);
$finalStatus = (isset($exception) && $exception) || $exitCode !== SymfonyCommand::SUCCESS
? SpanStatus::internalError()
: SpanStatus::ok();
$transaction
->setOrigin('auto.command')
->setStatus($finalStatus)
->setData($data)
->setTags($tags);
🤖 Prompt for AI Agents
In src/sentry/src/Tracing/Listener/TracingCommandListener.php around lines 117
to 121, the chained setStatus call unconditionally uses the exit code and can
overwrite a previously set internalError from a caught exception; determine a
final status first (e.g., if an exception was captured OR $exitCode !==
SymfonyCommand::SUCCESS then SpanStatus::internalError() else SpanStatus::ok())
and call setStatus exactly once with that computed status so exception-derived
errors are not lost.


SentrySdk::getCurrentHub()->setSpan($transaction);

$transaction->finish(microtime(true));
}
}
6 changes: 4 additions & 2 deletions src/sentry/src/Tracing/Listener/TracingCrontabListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,12 @@ protected function finishTransaction(AfterExecute|FailToExecute $event): void
}
}

$transaction->setData($data);
$transaction->setTags($tags);
$transaction->setOrigin('auto.crontab')
->setData($data)
->setTags($tags);

SentrySdk::getCurrentHub()->setSpan($transaction);

$transaction->finish(microtime(true));
}
}
10 changes: 5 additions & 5 deletions src/sentry/src/Tracing/Listener/TracingDbQueryListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,10 @@ protected function queryExecutedHandler(object $event): void
$description = $event->sql;

// Already check in the previous context
/** @var \Sentry\Tracing\Span $span */
$span = $this->startSpan($op, $description);
$span->setData($data);
$span->setStartTimestamp($startTimestamp);
$span->finish($startTimestamp + $event->time / 1000);
$this->startSpan($op, $description)
->setOrigin('auto.db')
->setData($data)
->setStartTimestamp($startTimestamp)
->finish($startTimestamp + $event->time / 1000);
}
}
6 changes: 4 additions & 2 deletions src/sentry/src/Tracing/Listener/TracingKafkaListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,12 @@ protected function finishTransaction(AfterConsume|FailToConsume $event): void
}
}

$transaction->setData($data);
$transaction->setTags($tags);
$transaction->setOrigin('auto.kafka')
->setData($data)
->setTags($tags);

SentrySdk::getCurrentHub()->setSpan($transaction);

$transaction->finish(microtime(true));
}
}
Loading