Skip to content

refactor(sentry): improve span null safety and consistency in tracing aspects #918

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
huangdijia merged 2 commits into from
Sep 22, 2025
Merged

refactor(sentry): improve span null safety and consistency in tracing aspects #918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b6de178
refactor(sentry): improve span null safety and consistency in tracing…
huangdijia Sep 22, 2025
91f59db
fix(filesystem): remove redundant span status setting in process method
huangdijia Sep 22, 2025
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
16 changes: 5 additions & 11 deletions src/sentry/src/Tracing/Aspect/ElasticsearchAspect.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,7 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint)
op: 'db.elasticsearch',
description: sprintf('%s::%s()', $proceedingJoinPoint->className, $proceedingJoinPoint->methodName),
origin: 'auto.elasticsearch',
);

if (! $span) {
return $proceedingJoinPoint->process();
}

$span->setData([
)?->setData([
'coroutine.id' => Coroutine::id(),
'db.system' => 'elasticsearch',
'db.operation.name' => $proceedingJoinPoint->methodName,
Expand All @@ -86,27 +80,27 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint)
try {
$result = $proceedingJoinPoint->process();
if ($this->switcher->isTracingExtraTagEnabled('elasticsearch.result')) {
$span->setData([
$span?->setData([
'elasticsearch.result' => json_encode($result, JSON_UNESCAPED_UNICODE),
]);
}
} catch (Throwable $exception) {
$span->setStatus(SpanStatus::internalError())
$span?->setStatus(SpanStatus::internalError())
->setTags([
'error' => 'true',
'exception.class' => $exception::class,
'exception.message' => $exception->getMessage(),
'exception.code' => (string) $exception->getCode(),
]);
if ($this->switcher->isTracingExtraTagEnabled('exception.stack_trace')) {
$span->setData([
$span?->setData([
'exception.stack_trace' => (string) $exception,
]);
}

throw $exception;
} finally {
$span->finish();
$span?->finish();
}

return $result;
Expand Down
41 changes: 30 additions & 11 deletions src/sentry/src/Tracing/Aspect/FilesystemAspect.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,16 @@
namespace FriendsOfHyperf\Sentry\Tracing\Aspect;

use FriendsOfHyperf\Sentry\Aspect\FilesystemAspect as BaseFilesystemAspect;
use FriendsOfHyperf\Sentry\Tracing\SpanStarter;
use Hyperf\Di\Aop\ProceedingJoinPoint;
use Override;
use Sentry\Tracing\SpanContext;

use function Sentry\trace;
use Sentry\Tracing\SpanStatus;
use Throwable;
Comment on lines +15 to +19
Copy link

@coderabbitai coderabbitai bot Sep 22, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

🧩 Analysis chain

引入 SpanStarter/SpanStatus/Throwable 合理；请确认 #[Override] 的 PHP 版本兼容性

改造方向对齐其它切面，OK。但请确认项目 PHP 约束≥8.3，否则使用 #[Override] 会导致运行时错误。

建议在仓库根目录执行以下脚本核对 PHP 版本要求与 Override 使用点：

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "PHP requirement in composer.json:"
if command -v jq >/dev/null 2>&1; then
  jq -r '.require.php // .require."php"' composer.json
else
  rg -nP '"php"\s*:\s*"[^"]+"' composer.json
fi

echo -e "\nAll occurrences of #[Override]:"
rg -nP '#\[\s*Override\s*\]' -C2

Length of output: 11740

根据脚本结果，项目的 PHP 版本要求是 >=8.1，但 #[Override] 属性需要 PHP 8.3 或更高版本才能使用。尽管发现代码库中已经广泛使用了 #[Override] 属性，但这与项目的 PHP 版本约束存在冲突。

让我进一步验证这个版本兼容性问题：

Based on the verification results, I can see there's a version compatibility issue that needs to be addressed.

PHP 版本约束与 #[Override] 属性不兼容

项目的 composer.json 要求 PHP >=8.1，但 #[Override] 属性需要 PHP 8.3+。虽然代码库中已广泛使用此属性，但当前配置会导致在 PHP 8.1-8.2 环境中运行时出错。

建议：

  • 将 composer.json 中的 PHP 版本约束更新为 >=8.3
  • 或者移除所有 #[Override] 属性以保持向下兼容

🌐 Web query:

PHP Override attribute minimum version requirement

💡 Result:

The built-in #[\Override] attribute was added in PHP 8.3 — so PHP >= 8.3.0 is required for the engine to enforce it (in older PHP versions the attribute syntax is ignored). [1][2][3]

Sources:

  • PHP manual: Override attribute (PHP 8 >= 8.3.0). [1]
  • RFC: Marking overridden methods (#[\Override]). [2]
  • PHP 8.3 migration / new features (lists Override attribute). [3]

必须修复：#[Override] 与 composer.json 中的 PHP 版本约束冲突

引入 SpanStarter/SpanStatus/Throwable 合理；但仓库根目录 composer.json 要求 PHP >=8.1，而内置属性 #[\Override]（等同 #[Override]）仅在 PHP >=8.3 可用，仓库中已多处使用该属性（例如 src/sentry/src/Tracing/Aspect/FilesystemAspect.php:25）。

  • 方案 A（推荐）：将 composer.json 的 php 约束提升到 >=8.3，并同步 CI/镜像/文档。
  • 方案 B：保留 php >=8.1，移除或替换仓库中所有 #[Override] 使用以恢复对 8.1/8.2 的兼容性。
🤖 Prompt for AI Agents 
In src/sentry/src/Tracing/Aspect/FilesystemAspect.php around lines 15 to 19 and
usage at ~line 25, the code uses the built‑in attribute #[Override] which
requires PHP >=8.3 while the repository composer.json currently constrains PHP
to >=8.1, causing a compatibility conflict; fix by either (A, recommended)
updating composer.json php constraint to ">=8.3" and then update CI images/docs
to match, or (B) revert to PHP 8.1/8.2 compatibility by removing/replacing all
uses of the #[Override] attribute across the codebase (search & replace
occurrences and run tests) so the project no longer relies on a PHP 8.3‑only
attribute.


class FilesystemAspect extends BaseFilesystemAspect
{
use SpanStarter;

#[Override]
public function process(ProceedingJoinPoint $proceedingJoinPoint)
{
Expand All @@ -29,13 +31,30 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint)

[$op, $description, $data] = $this->getSentryMetadata($proceedingJoinPoint);

return trace(
fn () => $proceedingJoinPoint->process(),
SpanContext::make()
->setOp($op)
->setData($data)
->setOrigin('auto.filesystem')
->setDescription($description)
);
$span = $this->startSpan(
op: $op,
description: $description,
origin: 'auto.filesystem',
)?->setData($data);

try {
return $proceedingJoinPoint->process();
} catch (Throwable $exception) {
$span?->setStatus(SpanStatus::internalError())
->setTags([
'error' => 'true',
'exception.class' => $exception::class,
'exception.message' => $exception->getMessage(),
'exception.code' => (string) $exception->getCode(),
]);
if ($this->switcher->isTracingExtraTagEnabled('exception.stack_trace')) {
$span?->setData([
'exception.stack_trace' => (string) $exception,
]);
}
throw $exception;
} finally {
$span?->finish();
}
}
}