fix: change default values to true in Sentry Switcher #894
Conversation
Updated default values from false to true for: - isEnable() - isBreadcrumbEnable() - isTracingEnable() - isTracingSpanEnable() - isTracingExtraTagEnable() This ensures Sentry features are enabled by default unless explicitly disabled in configuration.
|
Caution Review failedThe pull request is closed. Walkthrough将 Sentry Switcher 中多处读取配置时的默认值由“禁用”改为“启用”，并更新相关单元测试。缺失或未设置配置键时，以下开关默认返回 true：enable、breadcrumbs、tracing.enable、tracing.spans、tracing.extra_tags。无函数签名或外部 API 变更。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Switcher
participant Config
Caller->>Switcher: isEnable/isBreadcrumbEnable/... (key)
Switcher->>Config: get("sentry...." + key)
alt 配置存在
Config-->>Switcher: 返回具体值
Switcher-->>Caller: 根据值返回（true/false）
else 配置缺失
note right of Switcher #E6F7FF: 新行为：默认 true（启用）\n旧行为：默认 false（禁用）
Switcher-->>Caller: true
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.1.17)Note: Using configuration file /phpstan.neon.dist. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/sentry/src/Switcher.php (4)
27-27: 默认改为 true：请确认破坏性变更与发布策略这会把“显式启用/缺省禁用”切为“缺省启用/显式关闭”，可能导致在未配置场景下开始上报事件（合规/隐私需评估）。请在变更日志/发布说明中明确，并确认是否需要 SemVer 的次/主版本升级。
为避免字符串形式的 "false"/"0" 被 (bool) 误判为 true，建议统一做布尔规范化：
- return (bool) $this->config->get('sentry.enable.' . $key, true); + return $this->toBool($this->config->get('sentry.enable.' . $key, true), true);可在类内新增一个小工具方法（见本评论末尾附加代码）。
需要我起草迁移指南/CHANGELOG 条目吗？
附加代码（放在类中任意私有位置）：
private function toBool(mixed $value, bool $default = true): bool { if (is_bool($value)) { return $value; } $normalized = filter_var($value, FILTER_VALIDATE_BOOL, FILTER_NULL_ON_FAILURE); return $normalized ?? $default; }
32-32: Breadcrumbs 默认开启 OK，但建议布尔值规范化同上，(bool) 对非空字符串会返回 true。为健壮性与可预期行为，建议复用 toBool：
- return (bool) $this->config->get('sentry.breadcrumbs.' . $key, true); + return $this->toBool($this->config->get('sentry.breadcrumbs.' . $key, true), true);请在文档中注明“未配置时 Breadcrumbs 默认开启（opt-out）”。
41-41: Tracing 每键默认开启与全局门控配合良好；同样建议布尔规范化全局开关 sentry.enable_tracing（Line 37，默认 true）为前置门控，逻辑正确；为避免字符串布尔误判，建议：
- return (bool) $this->config->get('sentry.tracing.enable.' . $key, true); + return $this->toBool($this->config->get('sentry.tracing.enable.' . $key, true), true);
50-50: Span 开关默认 true 一致性良好；建议与上文一致做布尔规范化- return (bool) $this->config->get('sentry.tracing.spans.' . $key, true); + return $this->toBool($this->config->get('sentry.tracing.spans.' . $key, true), true);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/sentry/src/Switcher.php(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
| public function isTracingExtraTagEnable(string $key): bool | ||
| { | ||
| return (bool) ($this->config->get('sentry.tracing.extra_tags', [])[$key] ?? false); | ||
| return (bool) ($this->config->get('sentry.tracing.extra_tags', [])[$key] ?? true); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
潜在致命错误：对非数组配置做数组下标访问会触发 TypeError
如果用户将 sentry.tracing.extra_tags 误配为布尔/字符串（这一类误配在实际环境并不少见），$this->config->get(... )[$key] 在 PHP 8+ 会报 “Cannot use array offset on type string/bool”。建议先归一为数组并做布尔规范化：
- return (bool) ($this->config->get('sentry.tracing.extra_tags', [])[$key] ?? true);
+ $extraTags = $this->config->get('sentry.tracing.extra_tags', []);
+ $extraTags = is_array($extraTags) ? $extraTags : [];
+ $val = $extraTags[$key] ?? true;
+ return $this->toBool($val, true);请补一条用例覆盖：extra_tags 配置为 'false'（字符串）与 false（布尔）时的行为。
为验证潜在 TypeError 问题，请打印 src/sentry/src/Switcher.php 文件第 45–75 行内容：
🏁 Script executed:
#!/bin/bash
set -e
# 打印 Switcher.php 文件第 45-75 行以查看上下文
sed -n '45,75p' src/sentry/src/Switcher.phpLength of output: 900
为确认 toBool 方法和 config 属性类型，请打印 Switcher.php 中相关定义：
🏁 Script executed:
#!/bin/bash
set -e
# 查找 toBool 方法定义
rg -n "function toBool" -C3 src/sentry/src/Switcher.php || echo "未找到 toBool 方法"
# 查找 config 属性声明
rg -n "private .*config" -C3 src/sentry/src/Switcher.php || echo "未找到 config 属性"
# 查找构造函数，确认注入类型
rg -n "function __construct" -C3 src/sentry/src/Switcher.php || echo "未找到 __construct"Length of output: 381
确保 isTracingExtraTagEnable 安全处理 extra_tags 并添加测试
- 先将配置值归一为数组：
$extraTags = $this->config->get('sentry.tracing.extra_tags', []); if (! is_array($extraTags)) { $extraTags = []; }
- 根据键取值并做布尔规范化：可直接用 PHP 内置
filter_var如需支持更多字符串（如$val = $extraTags[$key] ?? true; return filter_var($val, FILTER_VALIDATE_BOOLEAN, ['options' => ['default' => true]]);
"off"/"no"），可自行实现toBool(mixed $val, bool $default): bool辅助方法。 - 补充单元测试，覆盖当
sentry.tracing.extra_tags被误配置为布尔或字符串（如false、'false'）时的行为。
🤖 Prompt for AI Agents
In src/sentry/src/Switcher.php around line 55, isTracingExtraTagEnable currently
assumes the config value is an array and casts the lookup result directly to
bool; change it to first normalize the config value to an array (if
get('sentry.tracing.extra_tags', []) returns non-array, set to []), then read
$val = $extraTags[$key] ?? true and normalize to boolean using PHP's filter_var
with FILTER_VALIDATE_BOOLEAN and default true (or call a small toBool helper
that supports values like "off"/"no"); also add unit tests covering cases where
sentry.tracing.extra_tags is a boolean, string ('false', 'off'), null, and a
proper array to assert correct boolean normalization.
* fix: change default values to true in Sentry Switcher Updated default values from false to true for: - isEnable() - isBreadcrumbEnable() - isTracingEnable() - isTracingSpanEnable() - isTracingExtraTagEnable() This ensures Sentry features are enabled by default unless explicitly disabled in configuration. * fix: update tracing extra tag value for consistency in SwitcherTest --------- Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
Summary
falsetotruefor all Sentry Switcher methodsChanges
Updated default values in the following methods:
isEnable(): false → trueisBreadcrumbEnable(): false → trueisTracingEnable(): false → trueisTracingSpanEnable(): false → trueisTracingExtraTagEnable(): false → trueImpact
This change ensures Sentry features are enabled by default unless explicitly disabled in the configuration, following the principle of opt-out rather than opt-in for monitoring capabilities.
Test plan
falsevalues in config still disable featuresSummary by CodeRabbit