feat(Segment membership inspection PoC): Surface identity counts in segments UI#7467
feat(Segment membership inspection PoC): Surface identity counts in segments UI#7467khvn26 wants to merge 6 commits into
Conversation
Backfills identities from Dynamo to Snowflake daily, then refreshes
per-(segment, environment) match counts in the new `SegmentMembership`
cache. The translator from `flagsmith-sql-flag-engine` turns each
canonical segment into a SQL `WHERE` predicate; counts are
materialised as `COUNT(*) ... GROUP BY environment_id` per segment.
The serializer surfaces them as a list of `{environment, count,
last_synced_at}`, ready to back per-env count badges in the
Identities-tab environment dropdown.
Pipeline shape:
- `backfill_identities_to_snowflake` is the daily recurring task
(`timeout=4h` to fit large environments). After backfilling each
project's environments it dispatches one
`refresh_project_segment_counts(project_id)` per project so the
count refresh always sees the freshly backfilled snapshot rather
than racing a separate schedule.
- `refresh_project_segment_counts` opens its own Snowpark session,
re-checks the FoF flag at execution time so a stale fan-out skips
orgs that have since been disabled, and bulk-upserts via Postgres
`ON CONFLICT` (single statement per project).
- `compute_segment_counts_for_project` returns a list of unsaved
`SegmentMembership` instances; the task stamps `last_synced_at`
consistently across the batch. Untranslatable segments emit a
structlog `compute.segment.skipped` error event so we hear about
predicate gaps rather than silently dropping rows.
Both tasks short-circuit when SNOWFLAKE_* env vars are unset and
skip per-organisation when the `segment_membership_inspection`
Flagsmith-on-Flagsmith flag is False, so SaaS rolls out gradually
and self-hosted is unaffected.
DELETE-then-INSERT runs without an explicit transaction. Snowflake
holds micropartition locks for the lifetime of an open transaction,
and at 10M+ identities a BEGIN/COMMIT around the whole env partition
would keep that lock open for minutes. Per-statement implicit
commits leave a brief mid-refresh window where readers see an empty
partition; acceptable under the FoF flag's gradual rollout.
Backfill writes via Snowpark DataFrames against the canonical
IDENTITIES schema, with `DynamoIdentity` documents projected through
`segment_membership.mappers.map_identity_document_to_snowflake_row`.
Refresh issues a single batched UNION ALL using parameterised SQL —
env keys are bound, predicates from the engine are already escape-
safe. Schema setup is a `RunPython` migration gated on
`is_snowflake_configured()`, so it no-ops on self-hosted and in the
test suite.
The segment serializer surfaces cached counts via a new `memberships`
list field; absence of an entry is the read-side signal, no flag
check on the read path. `SegmentMembershipSerializer` gives
drf-spectacular a typed schema. Adds a generic `batched` helper to
`api/util/util.py` for the per-INSERT batching.
beep boop
…ps prefetch
The new `prefetch_related("memberships")` adds one IN-clause query per
list response, even when no rows exist. Update the regression
expectations so the existing test suite reflects the new baseline.
beep boop
… pre-release Switches the api dep from a private-repo git URL — which the Docker build can't clone in CI — to a versioned pin against Flagsmith's staging CodeArtifact PyPI (`flagsmith-pypi-staging`, account 302456015006, eu-west-2). Initial published release: 0.1.0a1. The reusable docker-build workflow now unconditionally assumes the OIDC role `arn:aws:iam::302456015006:role/codeartifact-github-actions-staging` (trust policy allows any `repo:Flagsmith/*`), fetches an authorisation token, and exposes it to every build as the `codeartifact_token` BuildKit secret. Builds that don't mount the secret simply ignore it; the OIDC + token cost is a couple of seconds per build. `Dockerfile`'s four `make install*` lines mount the `codeartifact_token` secret and export `POETRY_HTTP_BASIC_FLAGSMITH_PYPI_STAGING_*` so poetry resolves the dep from CodeArtifact. The header documents the `--secret="id=codeartifact_token,env=..."` incantation for local builds. beep boop
…fact The unit-test, MCP-schema-push, makefile-target, and update-flagsmith workflows all run `make install-packages`, which now needs CodeArtifact credentials to resolve the `flagsmith-sql-flag-engine` pre-release. Encapsulate the OIDC role assumption + token fetch in a composite action, reuse it from the Docker build workflow, and wire it into every workflow that runs poetry install. beep boop
CodeQL flagged the MD5 truncation as a sensitive-data hashing risk. UUIDv4 already gives us the random bits we need for a dedup key, so take the high 64 bits directly via int.from_bytes and drop the hash. beep boop
Adds two reusable badges sourced from the Segment.memberships field: - SegmentMembershipTotalBadge: aggregates counts across environments and shows the most recent sync time as a relative interval. Rendered on each segment row in the project segments list (with the "identities" noun) and next to the Identities tab label on the segment edit page (compact, count + sync only). - SegmentMembershipEnvBadge: per-env count, rendered as the option label inside the Identities tab's environment select. Selecting an environment in the Identities tab displays the full last_synced_at timestamp underneath the select; before any selection the row stays in place as a placeholder. The shared Option component in project-components now honours selectProps.formatOptionLabel when callers provide one, falling back to the existing label/description layout. Drops the !important on the chip's align-self so badges can opt into vertical centering inline. beep boop
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Docker builds report
|
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Failed testsfirefox › tests/segment-membership-test.pw.ts › Segment membership badges render in list, tab, and env select @oss Details
Failed testsfirefox › tests/segment-membership-test.pw.ts › Segment membership badges render in list, tab, and env select @oss |
Visual Regression16 screenshots compared. See report for details. |
|
I feel like we could neaten up the badges by putting the word identities and ~x days ago into a descriptive tooltip like we do for override badges. Other than that it looks great! |
|
@kyle-ssg The tooltips worked really badly with the tab and the environment select; that's how I came to dumping everything in the chip label. Besides that, I feel like last sync time is important info to surface before we switch to CDC / pure Snowflake lookups for counts. |
Zaimwa9
left a comment
There was a problem hiding this comment.
2 comments on my side:
- A subjective one related to the verbosity of the chip
- As environments are central to those components, it looks to me that we have some room for typing improvements, if you agree on it we could have another pass there sooner than later
| <UsersIcon className='chip-svg-icon' /> | ||
| <span> | ||
| {count} | ||
| {compact ? '' : ` ${noun}`} |
There was a problem hiding this comment.
I would either just get rid of mentioning "identities" as this badge has been already used or use compact everywhere (but it becomes dead code).
Regarding the last sync, this could be a tooltip on hover to not overload the UI
There was a problem hiding this comment.
I'm also thinking that we'll need to display the breakdown per environment for the identities. Could be through the tooltip but we can discuss the obstacles you had implementing it
| return map | ||
| }, [memberships]) | ||
|
|
||
| const renderEnvOption = (data: unknown) => { |
There was a problem hiding this comment.
We should be able to use data: EnvOption directly no ?
|
|
||
| type EnvProps = { | ||
| membership: SegmentMembership | ||
| environment?: Environment |
There was a problem hiding this comment.
NIT typing: I feel like the environment should always be present in those components ? So it should be required here and cascaded in all these new components (e.g here)
89e88f9 to
6e1584b
Compare
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Contributes to #7464 (stacked on top — review after backend PR).
Surfaces the new
Segment.membershipsfield on three places in the existing UI:last_synced_attimestamp underneath.Two small enabling changes outside the segments folder:
Optioninproject-components.jsnow honoursselectProps.formatOptionLabelwhen callers pass one (falls back to the existing label/description layout).!importanton the global.chipalign-selfso the badge can opt into vertical centering inline.Screenshots
Identity count chips in the Segments list:
Identity count chip on the Identities tab in the Segment page:
Per-environment chips:
Last sync date for selected environment:
How did you test this code?
Manually drove the segments list, segment edit page, and the Identities tab in a local dev server pointed at staging, with a fetch interceptor injecting synthetic
membershipsarrays into the segments responses. Verified the total badge on each row, the compact badge next to the Identities tab label, per-env chips inside the select options, and the full timestamp display on selection — including theLast synced: —placeholder before any selection.Added one Playwright spec (
e2e/tests/segment-membership-test.pw.ts) usingpage.routeto stubmembershipsend-to-end against a real seeded segment, asserting the total badge in the list, the badge on the tab, per-env badges in the select, and the full-timestamp line on selection.