getsentry / getsentry/sentry-java
Replace misleading IScopeObserver.setBreadcrumbs with clearBreadcrumbs
- 主要語言
- Kotlin
- 星號
- 1.4k
- 分支
- 478
- 平均合併
- 3 天 4 小時
- 30 天內合併 PR
- 72
描述
`IScopeObserver.setBreadcrumbs(Collection)` reads like a bulk setter — "replace the observed breadcrumbs with this collection" — but that isn't its contract. Its real meaning is *"the breadcrumbs were cleared"*, encoded as *"the collection I passed you happens to be empty."*
`PersistingScopeObserver` is the only implementation that does anything, and it never looks at the elements:
```java
public void setBreadcrumbs(@NotNull Collection breadcrumbs) {
if (breadcrumbs.isEmpty()) {
// ...enqueue a clear...
}
// non-empty: silently do nothing
}
```
## Why this is a problem
1. **The parameter is a boolean in disguise.** The only thing read off the collection is `isEmpty()`. A non-empty argument is silently ignored, so anyone who takes the name at face value and writes a bulk-replace implements something the SDK never drives that way.
2. **It hides a real race.** `Scope.clearBreadcrumbs()` clears the live queue and then hands *that same live queue* to the observers. If another thread adds a breadcrumb between the `clear()` and the observer loop, the collection is no longer empty, `PersistingScopeObserver` does nothing, and the pre-clear breadcrumbs survive on disk — cleared in memory, not cleared in the scope cache. The signal is carried by mutable state observed after the fact instead of by the call itself. Narrow in practice (`clearBreadcrumbs()` isn't on a hot path), and the symptom is stale breadcrumbs on the next ANR/crash report rather than anything immediately user-visible.
3. **It's called constantly for nothing.** `Scope.addBreadcrumb` invokes both `addBreadcrumb` and `setBreadcrumbs` on every observer for every breadcrumb. The second call can only ever be a no-op (the collection just had an element added), so we pay an extra virtual call per observer per breadcrumb on a hot path — and it's why the batching work in getsentry/sentry-java#5714 needed extra care around clear ordering.
4. **It's inconsistent with the rest of the interface.** Attachments already do this correctly: `addAttachment` / `clearAttachments`. Breadcrumbs are the odd one out.
5. **The name collides with a genuine setter.** `SentryBaseEvent.setBreadcrumbs(List)` really does replace the list, so the same name means two different things depending on the receiver.
## Proposed change
* Add `clearBreadcrumbs()` to `IScopeObserver` and `ScopeObserverAdapter`.
* `Scope.clearBreadcrumbs()` calls `observer.clearBreadcrumbs()`; `Scope.addBreadcrumb` drops its `observer.setBreadcrumbs(...)` call entirely.
* `PersistingScopeObserver.setBreadcrumbs` becomes `clearBreadcrumbs()`, unconditionally enqueueing the clear marker — which also removes the race in (2).
* Remove `setBreadcrumbs` from `IScopeObserver`. It's public API and not `@ApiStatus.Internal`, hence filing this against the 9.0 major.
* Fix `IScopeObserver`'s javadoc, which claims all methods are `default` — none of them are.
* Update `ScopeTest`, `PersistingScopeObserverTest`, and `PersistingScopeObserverBatchingTest`, which currently express "clear" as `setBreadcrumbs(emptyList())`.
## Notes
`NdkScopeObserver` implements `addBreadcrumb` but not `setBreadcrumbs`, so native breadcrumbs are never cleared today. A `clearBreadcrumbs()` on the interface makes that gap visible; there's no corresponding entry point on `INativeScope`, so closing it would be follow-up work.
**Affected files:** `sentry/src/main/java/io/sentry/IScopeObserver.java`, `ScopeObserverAdapter.java`, `Scope.java`, `cache/PersistingScopeObserver.java`, `sentry/api/sentry.api`
貢獻指南
研究方向
先從 IScopeObserver.java、ScopeObserverAdapter.java 和 Scope.java 開始,追蹤 observer 呼叫,然後檢查 PersistingScopeObserver.java 和受影響的測試。更新 issue 中指定的 API surface 和測試,讓清除使用 clearBreadcrumbs(),並確認 batching 和 scope 測試涵蓋新的行為,不再透過 setBreadcrumbs(emptyList()) 表達清除。
由索引模型根據 Issue 內容生成。
評估
- 技術堆疊
- java
- 領域
- backend-api-design
- Issue 類型
- 重構
- 難度
- 4/5
- 預估耗時
- 3-5 天
- 活躍度
- 冷清
- 描述清晰度
- 描述清楚
- 新手友好度
- 56/100