feat: Add scope feature flag API#8147
Conversation
Introduces the internal foundation for feature flag evaluation tracking. The implementation adds a shared feature flag value/evaluation model plus dedicated buffers for scope and span storage. Scope storage keeps the latest unique evaluations up to the configured internal limit and evicts the oldest entry on overflow. Span storage has its own smaller per-span limit and rejects new flag names once full while still allowing updates to existing flags. This also wires the private storage into scope lifecycle behavior: - scope cloning copies feature flags without sharing future mutations - `clear()` clears feature flags - scope serialization includes the stored flag context - span internals can store and serialize feature flag evaluations as span data No public feature flag API is added in this PR; this is the private storage layer that later API and integration work will build on.
- faster insertion/lookup - index shifting paths keep same o(n) complexity
Adds public Swift APIs to record and remove feature flag evaluations on SentrySDK, SentryHub, and Scope. Feature flags are attached to regular error/message events, excluded from transactions/replays/feedback, and synchronized with scope observers, so crash-time scope snapshots also contain the latest flags context data.
|
denrase
requested review from
NinjaLikesCheez,
itaybre,
noahsmartin and
philprime
as code owners
|
@philprime As this is introducing new public API, how would you prefer we do the ObjC surface? Here in this PR or in a separate issue? We do have a separte issue to track this, I'm fine with either way. |
# Conflicts: # Sources/Sentry/SentryScope.m # Sources/Swift/FeatureFlags/SentryFeatureFlagBufferWrapper.swift # Sources/Swift/Scope.swift # Tests/SentryTests/SentryScopeSwiftTests.swift
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cb258e2. Configure here.
📲 Install BuildsiOS
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| - (void)updateFeatureFlagsContext | ||
| { | ||
| // Must be called while synchronized on _contextDictionary. | ||
| NSDictionary<NSString *, id> *_Nullable featureFlags = [_featureFlagBuffer serializeForContext]; | ||
| if (featureFlags.count > 0) { | ||
| _contextDictionary[@"flags"] = featureFlags; |
There was a problem hiding this comment.
Bug: The new feature flag mechanism uses the context key "flags". This can collide with user data if a developer uses the public setContextValue:forKey:@"flags" API, causing data loss.
Severity: MEDIUM
Suggested Fix
To prevent this collision, either document that the "flags" key is reserved and should not be used with setContextValue:forKey:, or implement a protection mechanism. For example, you could add input validation to setContextValue:forKey: to reject the "flags" key, or namespace the internal feature flag key (e.g., sentry_feature_flags) to avoid conflicts with user-defined keys.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: Sources/Sentry/SentryScope.m#L750-L755
Potential issue: The new feature flag implementation stores its data in the scope's
context under the key `"flags"`. However, the public API `setContextValue:forKey:`
allows users to set any context key, including `"flags"`. This creates a collision risk.
If a user calls `scope.setContext(value: myDict, key: "flags")`, it will overwrite the
feature flag data stored by the SDK. Conversely, subsequent feature flag updates will
overwrite the user's custom context. This can lead to data loss for either feature flags
or user-defined context, as the `"flags"` key is not documented as reserved or
protected.
Did we get this right? 👍 / 👎 to inform future reviews.
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 53097d6 | 1218.02 ms | 1251.70 ms | 33.68 ms |
| 1c5ecda | 1219.35 ms | 1253.76 ms | 34.41 ms |
| e0946cf | 1233.33 ms | 1258.57 ms | 25.24 ms |
| 8180609 | 1214.67 ms | 1243.36 ms | 28.69 ms |
| 85ee155 | 1227.40 ms | 1251.52 ms | 24.12 ms |
| 151b3ee | 1238.69 ms | 1270.73 ms | 32.05 ms |
| 379d045 | 1210.00 ms | 1241.24 ms | 31.24 ms |
| 7814719 | 1220.98 ms | 1254.14 ms | 33.16 ms |
| 1aa0a18 | 1225.33 ms | 1255.65 ms | 30.31 ms |
| 4c437ea | 1217.19 ms | 1250.79 ms | 33.60 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 53097d6 | 24.14 KiB | 1.16 MiB | 1.13 MiB |
| 1c5ecda | 24.14 KiB | 1.15 MiB | 1.12 MiB |
| e0946cf | 24.14 KiB | 1.16 MiB | 1.13 MiB |
| 8180609 | 24.14 KiB | 1.16 MiB | 1.13 MiB |
| 85ee155 | 24.14 KiB | 1.16 MiB | 1.13 MiB |
| 151b3ee | 24.14 KiB | 1.15 MiB | 1.13 MiB |
| 379d045 | 24.14 KiB | 1.18 MiB | 1.15 MiB |
| 7814719 | 24.14 KiB | 1.15 MiB | 1.13 MiB |
| 1aa0a18 | 24.14 KiB | 1.18 MiB | 1.15 MiB |
| 4c437ea | 24.14 KiB | 1.18 MiB | 1.15 MiB |
philprime
left a comment
philprime
left a comment
There was a problem hiding this comment.
Good progress, couple of small comments.
Regarding your question in adding the ObjC wrapper API in this PR too: Yes please do that. If this pull requests is merged, and we do a release, the changelog will mention that new API is available, but it won't be for SentryObjC.
We should not see these two targets as different SDKs, but the drift must stay minimal
| } | ||
| @synchronized(_contextDictionary) { | ||
| [_contextDictionary removeAllObjects]; | ||
| [_featureFlagBuffer removeAll]; |
There was a problem hiding this comment.
m: Shouldn't this be in a synchronized block for the _featureFlagBuffer instead?
| BOOL isRegularEvent | ||
| = event.type == nil || [event.type isEqualToString:SentryEnvelopeItemTypes.event]; | ||
| if (!isRegularEvent) { | ||
| [newContext removeObjectForKey:@"flags"]; | ||
| } |
There was a problem hiding this comment.
m: Please add a comment explaining the reasoning behind this. Also add a SDK log warning for users, so they understand why their flags context was removed
| } | ||
| } | ||
|
|
||
| - (void)updateFeatureFlagsContext |
There was a problem hiding this comment.
m: I believe this method must synchronize feature flag buffer too, as there is no contract in place to block devs from calling it directly.
2 participants
📜 Description
Adds public Swift APIs to record and remove feature flag evaluations on SentrySDK, SentryHub, and Scope. Feature flags are attached to regular error/message events, excluded from transactions/replays/feedback, and synchronized with scope observers, so crash-time scope snapshots also contain the latest flags context data.
💡 Motivation and Context
Closes #7989
💚 How did you test it?
Unit tests + Sample app
https://sentry-sdks.sentry.io/issues/6605337140/events/latest/?project=5428557&query=is%3Aunresolved&referrer=latest-event
📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.