Skip to content

Commit 24a3f5c

Browse files
huangdijiaclaude
andcommitted
refactor(sentry): optimize SingletonAspect with match expression (#1049)
* refactor(sentry): optimize SingletonAspect with match expression for class handling Replace the complex closure-based singleton instance reset logic with a more efficient match expression that explicitly handles different Sentry class types: - Direct instantiation for singleton classes (HubAdapter, IntegrationRegistry) - Simplified TraceMetrics creation - Bypass context handling for enum types - Default to original behavior for unknown classes This improves performance, readability, and reduces the overhead of reflection-based property manipulation while maintaining coroutine safety through Context. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * refactor(sentry): replace Co::defer with defer function for coroutine handling * fix(sentry): correct condition check for argument existence in SingletonAspect * refactor(sentry): clean up SingletonAspect by commenting out unused singleton classes and enums --------- Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 30d6e81 commit 24a3f5c

7 files changed

Lines changed: 52 additions & 41 deletions

File tree

src/sentry/src/Aspect/CoroutineAspect.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
use Sentry\SentrySdk;
2020
use Throwable;
2121

22+
use function Hyperf\Coroutine\defer;
23+
2224
class CoroutineAspect extends AbstractAspect
2325
{
2426
public const CONTEXT_KEYS = [
@@ -58,7 +60,7 @@ protected function handleCreate(ProceedingJoinPoint $proceedingJoinPoint): void
5860
}
5961

6062
// Defer the flushing of events until the coroutine completes.
61-
Co::defer(fn () => Integration::flushEvents());
63+
defer(fn () => Integration::flushEvents());
6264

6365
// Continue the callable in the new Coroutine.
6466
$callable();
@@ -73,6 +75,6 @@ protected function handlePrintLog(ProceedingJoinPoint $proceedingJoinPoint): voi
7375
return;
7476
}
7577

76-
Co::defer(fn () => SentrySdk::getCurrentHub()->captureException($throwable));
78+
defer(fn () => SentrySdk::getCurrentHub()->captureException($throwable));
7779
}
7880
}

src/sentry/src/Aspect/SingletonAspect.php

Lines changed: 33 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -19,44 +19,50 @@
1919
class SingletonAspect extends AbstractAspect
2020
{
2121
public array $classes = [
22-
\Sentry\CheckInStatus::class . '::getInstance',
23-
\Sentry\EventType::class . '::getInstance',
24-
\Sentry\MonitorScheduleUnit::class . '::getInstance',
22+
// Singleton Classes
23+
\Sentry\State\HubAdapter::class . '::getInstance',
2524
\Sentry\Integration\IntegrationRegistry::class . '::getInstance',
26-
\Sentry\Logs\LogLevel::class . '::getInstance',
2725
\Sentry\Metrics\TraceMetrics::class . '::getInstance',
28-
\Sentry\State\HubAdapter::class . '::getInstance',
29-
\Sentry\Tracing\SpanStatus::class . '::getInstance',
30-
\Sentry\Tracing\TransactionSource::class . '::getInstance',
31-
\Sentry\Transport\ResultStatus::class . '::getInstance',
32-
\Sentry\Unit::class . '::getInstance',
26+
// Enums
27+
// \Sentry\CheckInStatus::class . '::getInstance',
28+
// \Sentry\EventType::class . '::getInstance',
29+
// \Sentry\MonitorScheduleUnit::class . '::getInstance',
30+
// \Sentry\Logs\LogLevel::class . '::getInstance',
31+
// \Sentry\Tracing\SpanStatus::class . '::getInstance',
32+
// \Sentry\Tracing\TransactionSource::class . '::getInstance',
33+
// \Sentry\Transport\ResultStatus::class . '::getInstance',
34+
// \Sentry\Unit::class . '::getInstance',
3335
];
3436

3537
public function process(ProceedingJoinPoint $proceedingJoinPoint)
3638
{
3739
$key = $className = $proceedingJoinPoint->className;
3840
$arguments = $proceedingJoinPoint->getArguments();
3941

40-
if (isset($arguments[0])) {
42+
if (! array_key_exists(0, $arguments)) {
4143
$key .= '#' . $arguments[0];
4244
}
4345

44-
return Context::getOrSet($key, function () use ($proceedingJoinPoint, $className, $arguments) {
45-
// Reset singleton instance before proceeding
46-
Closure::bind(function () use ($className, $arguments) {
47-
if (property_exists($className, 'instance')) {
48-
$className::$instance = null;
49-
} elseif (
50-
property_exists($className, 'instances')
51-
&& isset($arguments[0])
52-
&& array_key_exists($arguments[0], $className::$instances)
53-
) {
54-
$className::$instances[$arguments[0]] = null;
55-
}
56-
}, null, $className)();
57-
58-
// Proceed to get the singleton instance
59-
return $proceedingJoinPoint->process();
60-
});
46+
return match ($className) {
47+
// Singleton Classes
48+
\Sentry\State\HubAdapter::class,
49+
\Sentry\Integration\IntegrationRegistry::class => Context::getOrSet($key, function () use ($className) {
50+
return Closure::bind(fn () => new $className(), null, $className)();
51+
}),
52+
\Sentry\Metrics\TraceMetrics::class => Context::getOrSet($key, function () use ($className) {
53+
return new $className();
54+
}),
55+
56+
// Enums
57+
// \Sentry\CheckInStatus::class,
58+
// \Sentry\EventType::class,
59+
// \Sentry\MonitorScheduleUnit::class,
60+
// \Sentry\Logs\LogLevel::class,
61+
// \Sentry\Tracing\SpanStatus::class,
62+
// \Sentry\Tracing\TransactionSource::class,
63+
// \Sentry\Transport\ResultStatus::class,
64+
// \Sentry\Unit::class => $proceedingJoinPoint->process(),
65+
default => $proceedingJoinPoint->process(),
66+
};
6167
}
6268
}

src/sentry/src/Metrics/Aspect/HistogramAspect.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616
use FriendsOfHyperf\Sentry\Metrics\Timer;
1717
use Hyperf\Di\Aop\AbstractAspect;
1818
use Hyperf\Di\Aop\ProceedingJoinPoint;
19-
use Hyperf\Engine\Coroutine as Co;
2019

20+
use function Hyperf\Coroutine\defer;
2121
use function Hyperf\Tappable\tap;
2222

2323
class HistogramAspect extends AbstractAspect
@@ -54,7 +54,7 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint): mixed
5454
]);
5555

5656
return tap($proceedingJoinPoint->process(), function () use ($timer) {
57-
Co::defer(fn () => $timer->end(true));
57+
defer(fn () => $timer->end(true));
5858
});
5959
}
6060

src/sentry/src/Metrics/Listener/RequestWatcher.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,14 @@
1414
use FriendsOfHyperf\Sentry\Feature;
1515
use FriendsOfHyperf\Sentry\Metrics\CoroutineServerStats;
1616
use FriendsOfHyperf\Sentry\Metrics\Timer;
17-
use Hyperf\Engine\Coroutine as Co;
1817
use Hyperf\Event\Contract\ListenerInterface;
1918
use Hyperf\HttpServer\Event as HttpEvent;
2019
use Hyperf\HttpServer\Router\Dispatched;
2120
use Hyperf\RpcServer\Event as RpcEvent;
2221
use Psr\Http\Message\ServerRequestInterface;
2322

23+
use function Hyperf\Coroutine\defer;
24+
2425
class RequestWatcher implements ListenerInterface
2526
{
2627
public function __construct(
@@ -56,7 +57,7 @@ public function process(object $event): void
5657
'request_method' => $request->getMethod(),
5758
]);
5859

59-
Co::defer(function () use ($timer) {
60+
defer(function () use ($timer) {
6061
++$this->stats->close_count;
6162
++$this->stats->response_count;
6263
--$this->stats->connection_num;

src/sentry/src/Tracing/Aspect/CoroutineAspect.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
use function FriendsOfHyperf\Sentry\startTransaction;
2727
use function FriendsOfHyperf\Sentry\trace;
28+
use function Hyperf\Coroutine\defer;
2829
use function Sentry\continueTrace;
2930

3031
class CoroutineAspect extends AbstractAspect
@@ -82,7 +83,7 @@ function (Scope $scope) use ($proceedingJoinPoint, $callingOnFunction) {
8283
);
8384

8485
// Defer the finishing of the transaction and flushing of events until the coroutine completes.
85-
Co::defer(function () use ($transaction) {
86+
defer(function () use ($transaction) {
8687
$transaction->finish();
8788
Integration::flushEvents();
8889
});

src/sentry/src/Tracing/Listener/EventHandleListener.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
use Hyperf\Crontab\Event as CrontabEvent;
2929
use Hyperf\Database\Events as DbEvent;
3030
use Hyperf\DbConnection\Pool\PoolFactory;
31-
use Hyperf\Engine\Coroutine as Co;
3231
use Hyperf\Event\Contract\ListenerInterface;
3332
use Hyperf\HttpServer\Event as HttpEvent;
3433
use Hyperf\HttpServer\Router\Dispatched;
@@ -54,6 +53,7 @@
5453

5554
use function FriendsOfHyperf\Sentry\startTransaction;
5655
use function FriendsOfHyperf\Sentry\trace;
56+
use function Hyperf\Coroutine\defer;
5757
use function Sentry\continueTrace;
5858

5959
/**
@@ -327,7 +327,7 @@ protected function handleRequestReceived(HttpEvent\RequestReceived|RpcEvent\Requ
327327

328328
SentrySdk::getCurrentHub()->setSpan($span);
329329

330-
Co::defer(function () use ($transaction, $span) {
330+
defer(function () use ($transaction, $span) {
331331
// Make sure the span is finished after the request is handled
332332
$span->finish();
333333

@@ -530,7 +530,7 @@ protected function handleCrontabTaskStarting(CrontabEvent\BeforeExecute $event):
530530
])
531531
);
532532

533-
Co::defer(function () use ($transaction) {
533+
defer(function () use ($transaction) {
534534
// Make sure the transaction is finished after the task is executed
535535
SentrySdk::getCurrentHub()->setSpan($transaction);
536536

@@ -599,7 +599,7 @@ protected function handleAmqpMessageProcessing(AmqpEvent\BeforeConsume $event):
599599
])
600600
);
601601

602-
Co::defer(function () use ($transaction) {
602+
defer(function () use ($transaction) {
603603
// Make sure the transaction is finished after the message is processed
604604
SentrySdk::getCurrentHub()->setSpan($transaction);
605605

@@ -667,7 +667,7 @@ protected function handleKafkaMessageProcessing(KafkaEvent\BeforeConsume $event)
667667
])
668668
);
669669

670-
Co::defer(function () use ($transaction) {
670+
defer(function () use ($transaction) {
671671
// Make sure the transaction is finished after the message is processed
672672
SentrySdk::getCurrentHub()->setSpan($transaction);
673673

@@ -717,7 +717,7 @@ protected function handleAsyncQueueJobProcessing(AsyncQueueEvent\BeforeHandle $e
717717
])
718718
);
719719

720-
Co::defer(function () use ($transaction) {
720+
defer(function () use ($transaction) {
721721
// Make sure the transaction is finished after the job is processed
722722
SentrySdk::getCurrentHub()->setSpan($transaction);
723723

src/sentry/src/Tracing/Tracer.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use Sentry\Tracing\TransactionSource;
2222
use Throwable;
2323

24+
use function Hyperf\Coroutine\defer;
2425
use function Sentry\trace;
2526

2627
class Tracer
@@ -34,7 +35,7 @@ public function startTransaction(TransactionContext $transactionContext, array $
3435
$hub->pushScope();
3536
$hub->configureScope(static fn (Scope $scope) => $scope->clearBreadcrumbs());
3637

37-
Co::defer(static fn () => $hub->popScope());
38+
defer(static fn () => $hub->popScope());
3839

3940
$transactionContext->setData(['coroutine.id' => Co::id()] + $transactionContext->getData());
4041

0 commit comments

Comments
 (0)