From 21f3b5cd52dfcc6a6d5cc9a1daefbc084a7c32e1 Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Mon, 29 Sep 2025 15:25:19 +0800 Subject: [PATCH 1/5] refactor(sentry): simplify tracing data collection and remove sensitive exception details - Remove detailed exception information (class, code, message) from tracing spans for security - Simplify HTTP status handling by using setHttpStatus() method instead of manual status/tag setting - Remove coroutine ID tracking from HTTP client spans to reduce data overhead - Standardize exception handling across all tracing components to only set status without exposing details - Clean up unused imports in GuzzleHttpClientAspect This improves security by preventing sensitive exception data from being sent to Sentry while maintaining essential tracing functionality and error status tracking. --- .../src/Tracing/Aspect/CoroutineAspect.php | 7 +- .../Tracing/Aspect/GuzzleHttpClientAspect.php | 29 ++------ .../Tracing/Listener/EventHandleListener.php | 68 +++++-------------- src/sentry/src/Tracing/Tracer.php | 10 +-- 4 files changed, 23 insertions(+), 91 deletions(-) diff --git a/src/sentry/src/Tracing/Aspect/CoroutineAspect.php b/src/sentry/src/Tracing/Aspect/CoroutineAspect.php index 5abee47db..0b638e2f3 100644 --- a/src/sentry/src/Tracing/Aspect/CoroutineAspect.php +++ b/src/sentry/src/Tracing/Aspect/CoroutineAspect.php @@ -112,12 +112,7 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint) try { $callable(); } catch (Throwable $exception) { - $coTransaction->setStatus(SpanStatus::internalError()) - ->setTags([ - 'error' => 'true', - 'exception.class' => $exception::class, - 'exception.code' => (string) $exception->getCode(), - ]); + $coTransaction->setStatus(SpanStatus::internalError()); throw $exception; } diff --git a/src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php b/src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php index 21fa59dbb..cb82f9a47 100644 --- a/src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php +++ b/src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php @@ -11,12 +11,10 @@ namespace FriendsOfHyperf\Sentry\Tracing\Aspect; -use Error; use FriendsOfHyperf\Sentry\Feature; use GuzzleHttp\Client; use GuzzleHttp\TransferStats; use Hyperf\Context\Context; -use Hyperf\Coroutine\Coroutine; use Hyperf\Di\Aop\AbstractAspect; use Hyperf\Di\Aop\ProceedingJoinPoint; use Psr\Container\ContainerInterface; @@ -24,7 +22,6 @@ use Sentry\State\Scope; use Sentry\Tracing\SpanContext; use Sentry\Tracing\SpanStatus; -use Throwable; use function FriendsOfHyperf\Sentry\trace; @@ -136,32 +133,17 @@ function (Scope $scope) use ($proceedingJoinPoint, $options, $guzzleConfig) { ]); $body->seek($pos); } else { - $span->setData(['http.response.body.contents' => '[binary omitted]']); + $span->setData([ + 'http.response.body.contents' => '[binary omitted]', + ]); } } - $span->setStatus( - SpanStatus::createFromHttpStatusCode($response->getStatusCode()) - ); - - if ($response->getStatusCode() >= 400 && $response->getStatusCode() < 600) { - $span->setTags([ - 'error' => 'true', - 'http.response.reason' => $response->getReasonPhrase(), - ]); - } + $span->setHttpStatus($response->getStatusCode()); } if ($stats->getHandlerErrorData()) { - $exception = $stats->getHandlerErrorData() instanceof Throwable - ? $stats->getHandlerErrorData() - : new Error(); - $span->setStatus(SpanStatus::internalError()) - ->setTags([ - 'error' => 'true', - 'exception.class' => $exception::class, - 'exception.code' => (string) $exception->getCode(), - ]); + $span->setStatus(SpanStatus::internalError()); } if (is_callable($onStats)) { @@ -175,7 +157,6 @@ function (Scope $scope) use ($proceedingJoinPoint, $options, $guzzleConfig) { ->setOp('http.client') ->setDescription($request->getMethod() . ' ' . (string) $request->getUri()) ->setOrigin('auto.http.client') - ->setData(['coroutine.id' => Coroutine::id()]) ); } } diff --git a/src/sentry/src/Tracing/Listener/EventHandleListener.php b/src/sentry/src/Tracing/Listener/EventHandleListener.php index 4868be093..c0d3501af 100644 --- a/src/sentry/src/Tracing/Listener/EventHandleListener.php +++ b/src/sentry/src/Tracing/Listener/EventHandleListener.php @@ -351,23 +351,17 @@ protected function handleRequestHandled(HttpEvent\RequestHandled|RpcEvent\Reques } $traceId = (string) $span->getTraceId(); + if ($event instanceof RpcEvent\RequestHandled && $this->container->has(RpcContext::class)) { $this->container->get(RpcContext::class)->set('sentry-trace-id', $traceId); } elseif ($event->response instanceof ResponsePlusInterface) { $event->response->setHeader('sentry-trace-id', $traceId); } - $span->setStatus( - SpanStatus::createFromHttpStatusCode($event->response->getStatusCode()) - ); + $span->setHttpStatus($event->response->getStatusCode()); - if ($exception = $event->getThrowable()) { - $span->setStatus(SpanStatus::internalError()) - ->setTags([ - 'error' => 'true', - 'exception.class' => $exception::class, - 'exception.code' => (string) $exception->getCode(), - ]); + if ($event->getThrowable()) { + $span->setStatus(SpanStatus::internalError()); } } @@ -422,13 +416,8 @@ protected function handleCommandFinished(CommandEvent\AfterExecute $event): void 'command.exit_code' => (string) $exitCode, ]); - if (method_exists($event, 'getThrowable') && $exception = $event->getThrowable()) { - $transaction->setStatus(SpanStatus::internalError()) - ->setTags([ - 'error' => 'true', - 'exception.class' => $exception::class, - 'exception.code' => (string) $exception->getCode(), - ]); + if (method_exists($event, 'getThrowable') && $event->getThrowable()) { + $transaction->setStatus(SpanStatus::internalError()); } $transaction->setStatus( @@ -463,13 +452,8 @@ function (Scope $scope) use ($event) { $span->setData(['db.redis.result' => $event->result]); } - if ($exception = $event->throwable) { - $span->setStatus(SpanStatus::internalError()) - ->setTags([ - 'error' => 'true', - 'exception.class' => $exception::class, - 'exception.code' => (string) $exception->getCode(), - ]); + if ($event->throwable) { + $span->setStatus(SpanStatus::internalError()); } }, SpanContext::make() @@ -537,13 +521,8 @@ protected function handleCrontabTaskFinished(CrontabEvent\FailToExecute|CrontabE return; } - if (method_exists($event, 'getThrowable') && $exception = $event->getThrowable()) { - $transaction->setStatus(SpanStatus::internalError()) - ->setTags([ - 'error' => 'true', - 'exception.class' => $exception::class, - 'exception.code' => (string) $exception->getCode(), - ]); + if (method_exists($event, 'getThrowable') && $event->getThrowable()) { + $transaction->setStatus(SpanStatus::internalError()); } } @@ -614,13 +593,8 @@ protected function handleAmqpMessageProcessed(AmqpEvent\AfterConsume|AmqpEvent\F 'messaging.amqp.message.result' => $event instanceof AmqpEvent\AfterConsume ? $event->getResult()->value : 'fail', ]); - if (method_exists($event, 'getThrowable') && $exception = $event->getThrowable()) { - $transaction->setStatus(SpanStatus::internalError()) - ->setTags([ - 'error' => 'true', - 'exception.class' => $exception::class, - 'exception.code' => (string) $exception->getCode(), - ]); + if (method_exists($event, 'getThrowable') && $event->getThrowable()) { + $transaction->setStatus(SpanStatus::internalError()); } } @@ -683,13 +657,8 @@ protected function handleKafkaMessageProcessed(KafkaEvent\AfterConsume|KafkaEven return; } - if (method_exists($event, 'getThrowable') && $exception = $event->getThrowable()) { - $transaction->setStatus(SpanStatus::internalError()) - ->setTags([ - 'error' => 'true', - 'exception.class' => $exception::class, - 'exception.code' => (string) $exception->getCode(), - ]); + if (method_exists($event, 'getThrowable') && $event->getThrowable()) { + $transaction->setStatus(SpanStatus::internalError()); } } @@ -739,13 +708,8 @@ protected function handleAsyncQueueJobProcessed(AsyncQueueEvent\AfterHandle|Asyn return; } - if (method_exists($event, 'getThrowable') && $exception = $event->getThrowable()) { - $transaction->setStatus(SpanStatus::internalError()) - ->setTags([ - 'error' => 'true', - 'exception.class' => $exception::class, - 'exception.code' => (string) $exception->getCode(), - ]); + if (method_exists($event, 'getThrowable') && $event->getThrowable()) { + $transaction->setStatus(SpanStatus::internalError()); } } diff --git a/src/sentry/src/Tracing/Tracer.php b/src/sentry/src/Tracing/Tracer.php index 8a21bb076..09da7e05f 100644 --- a/src/sentry/src/Tracing/Tracer.php +++ b/src/sentry/src/Tracing/Tracer.php @@ -84,15 +84,7 @@ function (Scope $scope) use ($trace) { try { return $trace($scope); } catch (Throwable $exception) { - $span = $scope->getSpan(); - if ($span !== null) { - $span->setStatus(SpanStatus::internalError()) - ->setTags([ - 'error' => 'true', - 'exception.class' => $exception::class, - 'exception.code' => (string) $exception->getCode(), - ]); - } + $scope->getSpan()?->setStatus(SpanStatus::internalError()); throw $exception; } }, From e2a89525fb11289e9ab3dde313dcb9689d49b9df Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Mon, 29 Sep 2025 15:28:40 +0800 Subject: [PATCH 2/5] refactor(EventHandleListener): simplify error handling by checking event types directly --- .../src/Tracing/Listener/EventHandleListener.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/sentry/src/Tracing/Listener/EventHandleListener.php b/src/sentry/src/Tracing/Listener/EventHandleListener.php index c0d3501af..62001cddd 100644 --- a/src/sentry/src/Tracing/Listener/EventHandleListener.php +++ b/src/sentry/src/Tracing/Listener/EventHandleListener.php @@ -416,7 +416,7 @@ protected function handleCommandFinished(CommandEvent\AfterExecute $event): void 'command.exit_code' => (string) $exitCode, ]); - if (method_exists($event, 'getThrowable') && $event->getThrowable()) { + if ($event->getThrowable()) { $transaction->setStatus(SpanStatus::internalError()); } @@ -521,7 +521,7 @@ protected function handleCrontabTaskFinished(CrontabEvent\FailToExecute|CrontabE return; } - if (method_exists($event, 'getThrowable') && $event->getThrowable()) { + if ($event instanceof CrontabEvent\FailToExecute) { $transaction->setStatus(SpanStatus::internalError()); } } @@ -593,7 +593,7 @@ protected function handleAmqpMessageProcessed(AmqpEvent\AfterConsume|AmqpEvent\F 'messaging.amqp.message.result' => $event instanceof AmqpEvent\AfterConsume ? $event->getResult()->value : 'fail', ]); - if (method_exists($event, 'getThrowable') && $event->getThrowable()) { + if ($event instanceof AmqpEvent\FailToConsume) { $transaction->setStatus(SpanStatus::internalError()); } } @@ -657,7 +657,7 @@ protected function handleKafkaMessageProcessed(KafkaEvent\AfterConsume|KafkaEven return; } - if (method_exists($event, 'getThrowable') && $event->getThrowable()) { + if ($event instanceof AmqpEvent\FailToConsume) { $transaction->setStatus(SpanStatus::internalError()); } } @@ -708,7 +708,7 @@ protected function handleAsyncQueueJobProcessed(AsyncQueueEvent\AfterHandle|Asyn return; } - if (method_exists($event, 'getThrowable') && $event->getThrowable()) { + if ($event instanceof AmqpEvent\FailToConsume) { $transaction->setStatus(SpanStatus::internalError()); } } From 2cf1bba5fe8779abdd682c215a3fc66a5b024eb0 Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Mon, 29 Sep 2025 15:29:42 +0800 Subject: [PATCH 3/5] refactor(EventHandleListener): update event type checks from AmqpEvent to KafkaEvent and AsyncQueueEvent --- src/sentry/src/Tracing/Listener/EventHandleListener.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/src/Tracing/Listener/EventHandleListener.php b/src/sentry/src/Tracing/Listener/EventHandleListener.php index 62001cddd..72bc2131e 100644 --- a/src/sentry/src/Tracing/Listener/EventHandleListener.php +++ b/src/sentry/src/Tracing/Listener/EventHandleListener.php @@ -657,7 +657,7 @@ protected function handleKafkaMessageProcessed(KafkaEvent\AfterConsume|KafkaEven return; } - if ($event instanceof AmqpEvent\FailToConsume) { + if ($event instanceof KafkaEvent\FailToConsume) { $transaction->setStatus(SpanStatus::internalError()); } } @@ -708,7 +708,7 @@ protected function handleAsyncQueueJobProcessed(AsyncQueueEvent\AfterHandle|Asyn return; } - if ($event instanceof AmqpEvent\FailToConsume) { + if ($event instanceof AsyncQueueEvent\FailToConsume) { $transaction->setStatus(SpanStatus::internalError()); } } From 124b605eb4bce5671a62d62ae9a55e4f968566c7 Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Mon, 29 Sep 2025 15:30:06 +0800 Subject: [PATCH 4/5] refactor(EventHandleListener): update event type check from AsyncQueueEvent\FailToConsume to AsyncQueueEvent\FailedHandle --- src/sentry/src/Tracing/Listener/EventHandleListener.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/src/Tracing/Listener/EventHandleListener.php b/src/sentry/src/Tracing/Listener/EventHandleListener.php index 72bc2131e..c77ec641b 100644 --- a/src/sentry/src/Tracing/Listener/EventHandleListener.php +++ b/src/sentry/src/Tracing/Listener/EventHandleListener.php @@ -708,7 +708,7 @@ protected function handleAsyncQueueJobProcessed(AsyncQueueEvent\AfterHandle|Asyn return; } - if ($event instanceof AsyncQueueEvent\FailToConsume) { + if ($event instanceof AsyncQueueEvent\FailedHandle) { $transaction->setStatus(SpanStatus::internalError()); } } From 0c369e5b8a271a430d07e219e0cc1c330c8e0c87 Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Mon, 29 Sep 2025 15:32:55 +0800 Subject: [PATCH 5/5] refactor(EventHandleListener): add method_exists check for getThrowable before usage --- src/sentry/src/Tracing/Listener/EventHandleListener.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/src/Tracing/Listener/EventHandleListener.php b/src/sentry/src/Tracing/Listener/EventHandleListener.php index c77ec641b..4bea5f9f5 100644 --- a/src/sentry/src/Tracing/Listener/EventHandleListener.php +++ b/src/sentry/src/Tracing/Listener/EventHandleListener.php @@ -360,7 +360,7 @@ protected function handleRequestHandled(HttpEvent\RequestHandled|RpcEvent\Reques $span->setHttpStatus($event->response->getStatusCode()); - if ($event->getThrowable()) { + if (method_exists($event, 'getThrowable') && $event->getThrowable()) { $span->setStatus(SpanStatus::internalError()); } }