Repository navigation
Conversation
Lets a sender attach event-level groups to a bridged event. Receivers that do not read the field ignore it.
Groups passed to fetch (used for group-unit bucketing) never reached the automatic $exposure event, so group-bucketed experiments counted no group exposures. The exposure now carries the groups of the user the variant was evaluated for, as an optional `groups` field on `Exposure`. The field is omitted when the user has no groups. - Custom ExposureTrackingProvider: receives `exposure.groups`. - Integration plugins: IntegrationManager moves `groups` out of the event properties into `ExperimentEvent.groups`. AmplitudeIntegrationPlugin forwards it as `AnalyticsEvent.groups` on the analytics connector, so an analytics receiver can send them as event-level groups.
Exposure dedupe keyed only on user ID and device ID. A user who moved to a new group and got the same variant sent no new exposure, so the new group was never counted. UserSessionExposureTracker and SessionDedupeCache now treat a change in the user's groups as an identity change and reset the cache. Groups compare independent of group type and group name order.
size-limit report 📦
|
kyeh-amp
requested review from
tyiuhc and
zhukaihan
and removed request for
zhukaihan
October 6, 2026 21:52
…ap at 10 values Analytics turns an exposure without a variant into an identify, which writes its groups to the user profile, so default exposures no longer carry groups. Analytics also rejects events with more than 10 group values, so groups are omitted above that limit and a warning is logged. Dedupe uses the groups that exposures carry.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Experiments can be bucketed by a group (for example a company or team) instead of a user. To do that, apps pass groups to the Experiment fetch:
Bucketing already uses those groups. But the automatic
$exposureevent the SDK sends afterward does not carry them. So unless the app also sets the same groups in Analytics withsetGroup, exposure analysis has no groups to count, and group-unit experiments report 0 groups exposed.This PR carries the groups used for evaluation onto the exposure. It also makes a group change send a new exposure.
Why it happens today
sequenceDiagram participant App participant Client as ExperimentClient participant Tracker as Exposure trackers participant Conn as analytics-connector participant Analytics as Analytics SDK App->>Client: fetch(user with groups) Client-->>App: variant (bucketed on groups) App->>Client: variant() / exposure() Client->>Tracker: track(exposure, user) Note over Tracker: user is only used for dedupe,<br/>then dropped Tracker->>Conn: { eventType, eventProperties } Note over Conn: AnalyticsEvent has no groups field Conn->>Analytics: $exposure without groupsThere are two ways an exposure leaves the SDK, and both drop the groups:
IntegrationManager→AmplitudeIntegrationPlugin→analytics-connectorevent bridge. The connectorAnalyticsEventtype has no place for groups.ExposureTrackingProviderpath:UserSessionExposureTrackercallsprovider.track(exposure)without the user, so a custom provider can't see the groups either.A second gap: exposure dedupe only keys on user ID, device ID, flag and variant. If a user switches groups and gets the same variant, no new exposure is sent, so the new group is never counted.
What changes
// analytics-connector: AnalyticsEvent { eventType: string; eventProperties?: Record<string, unknown>; userProperties?: Record<string, unknown>; + groups?: Record<string, string | string[]>; // optional, event-level groups } // experiment-browser: exposure flow exposureInternal(key) user = addContext(getUser()) // already has user.groups + groups = user.groups, or none if > 10 values // logs a warning over the limit - tracker.track(exposure, user) // dedupe: user_id, device_id, flag -> variant + tracker.track(exposure, { ...user, groups }) // dedupe: ... + groups (order-independent) - integrationManager.track(exposure, user) // groups dropped + integrationManager.track(exposure, { ...user, groups }) + // event.groups = groups, only if exposure.variant // custom provider API - track(exposure: Exposure): void + track(exposure: Exposure, groups?: Record<string, string[]>): voidIntegrationManagercopies the evaluation user's groups onto the exposure event's newgroupsfield, andAmplitudeIntegrationPluginpasses it to the event bridge. Groups go in their own field, never insideeventProperties, so they arrive as real event-level groups.ExposureTrackingProvider.trackgets an optional secondgroupsargument. TheExposureobject itself is unchanged, so existing providers that forwardexposureas event properties keep sending exactly what they sent before. The doc comment shows how to send the groups as event-level groups, for exampleamplitude.track('$exposure', exposure, { groups }).setGroup. Sending groups there would silently change the user's group memberships. Both paths apply this rule.LogLevel.Warnand above). It does not keep only the first 10, because that would send an arbitrary partial set of groups.UserSessionExposureTrackerandSessionDedupeCache, through a new order-independentgroupsKeyhelper. The identity uses the groups after the 10-value limit, so groups that are never sent never reset dedupe.When an exposure carries groups:
Compatibility
groupsyet ignores it, so any mix of versions behaves the same as today or better.groupsfield as event options. That is a separate, paired change in Amplitude-TypeScript (feat(analytics-browser): forward connector event groups to track Amplitude-TypeScript#2039).setUser()with new groups and does not fetch again, the exposure carries the new groups next to a variant from the earlier fetch.🤖 Generated with Claude Code
Risk Assessment
Resolved: dropping all groups above 10 values is the intended rule. Analytics rejects such an event, and keeping only the first 10 would send an arbitrary partial set.
Testing
I ran the client and integration test suites in experiment-browser and they all pass, including the 8 new exposure-groups tests. I captured a transcript of what a real client sends to a custom provider and to the analytics connector. With groups set, both paths get the groups: in the connector event's
groupsfield, and as the provider's second argument. Repeating the call with the same groups sends nothing. Changing groups sends the exposure again with the new groups. Removing groups sends the exposure with no groups key. I also ran the new tests against the base commit's source: the 4 positive tests (groups attached, group change re-sends) fail there, so the tests catch the original bug. This is a library change with no UI, so there are no screenshots. Afterwards I deleted node_modules, the built dist folders and the temporary script, so the worktree is clean.Evidence: Exposure payloads from a real client (provider and connector) across group set, dedupe, group change and groups removed
# setUser({user_id:"u1", groups:{org:["acme"]}}); variant("group-flag") provider -> track({"flag_key":"group-flag","variant":"treatment"}, {"org":["acme"]}) connector -> {"eventType":"$exposure","eventProperties":{"flag_key":"group-flag","variant":"treatment"},"groups":{"org":["acme"]}} # variant("group-flag") again (same groups -> deduped, nothing sent) # setUser with groups:{org:["globex"]}; variant("group-flag") (group change -> re-sent) provider -> track({"flag_key":"group-flag","variant":"treatment"}, {"org":["globex"]}) connector -> {"eventType":"$exposure","eventProperties":{"flag_key":"group-flag","variant":"treatment"},"groups":{"org":["globex"]}} # setUser with no groups; variant("group-flag") (identity change, no groups key) provider -> track({"flag_key":"group-flag","variant":"treatment"}) connector -> {"eventType":"$exposure","eventProperties":{"flag_key":"group-flag","variant":"treatment"}}Evidence: New exposure-groups tests run against base (pre-change) source
✕ user groups sent with exposure to provider and connector ✓ user with no groups, exposure has no groups ✓ user with empty groups, exposure has no groups ✓ default exposure has no groups ✕ 10 group values are sent with exposure ✕ more than 10 group values are omitted from exposure with a warning ✕ group change sends exposure again ✓ same groups in a different order are deduplicatedPipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
packages/experiment-browser/src/experimentClient.ts:997- The intent says to "send all of that user's groups rather than guessing the bucketing group type". The diff addsif (count > maxExposureGroupValues) { ... return undefined; }. A user with more than 10 group values now gets no groups on any exposure, so group-unit experiments still show 0 groups exposed for that user. Each exposure is also deduped as if the user had no groups. This was a deliberate later commit, "cap at 10 values". Please confirm that dropping all groups is the intended behavior, rather than some other rule such as keeping the first 10.packages/experiment-browser/src/experimentClient.ts:998- exposureGroups() runs on every exposureInternal call, before dedupe. A user over the cap logs the warning on every variant()/exposure() call, including calls that dedupe drops. With automatic exposure on, this can spam logs. Consider warning once per groups identity.✅ **Test** - passed
✅ No issues found.
yarn install --frozen-lockfile --ignore-scripts+lerna run build --scope @amplitude/analytics-connector --scope @amplitude/experiment-core(setup: the workspace packages had to be built first)npx jest test/client.test.ts test/integration --verbosein packages/experiment-browser: all pass, including the 8 newexposure groupstestsTemporary jest script (since deleted) that drives ExperimentClient with initialVariants, a custom ExposureTrackingProvider and AmplitudeIntegrationPlugin on a real AnalyticsConnector. It prints the exact payloads for four steps: groups set, same groups again, group change, groups removedRegression check: restored base 860a51c source for experiment-browser/src and analytics-connector/src, then ran the newexposure groupstests with transpile-only ts-jest (isolatedModules: true). 4 positive tests fail on base, the 4 no-groups tests pass. Then restored HEAD source✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.