Repository navigation
spec: Feature-flag gates for analytics and experiment endpoints (#37659) - #37690
freddyDOTCMS wants to merge 24 commits into
Conversation
ref: #29555 Changing link from `/c/dotAI` to `/c/dotai`
|
Claude finished @freddyDOTCMS's task in 2m 37s —— View job Code Review — Feature-flag gates for analytics/experiment endpointsI reviewed the gate logic, error paths, and the new New Issues
Existing
Notes (non-blocking)
No critical or high-severity defects found. The two Medium items on the gate are worth tightening before merge but are non-blocking. |
…ts (#37659) Defines gate behavior for FEATURE_FLAG_CONTENT_ANALYTICS and FEATURE_FLAG_EXPERIMENTS across EventAnalyticsProxyResource and ExperimentsResource, including event filtering, context stripping, siteauth access rules, and restart-required flag enforcement. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ges, auth ordering - Expand Independent Test in all 4 user stories: add unit tests for gate logic and payload manipulation; add Postman/integration split with explicit rationale for why non-403 scenarios cannot use Postman - Add Contract Changes section declaring new 403 response paths on all affected endpoints and the context.experiments payload mutation - Add auth-ordering rule to FR-001 (auth before gate; 401 not 403) with explicit exception for POST event endpoint (site_auth returns 400) - FR-003a: clarify /health is an existing endpoint, not a new one - FR-010: add Postman test declaration for error envelope shape - FR-011: replace startup-cache language with outcome statement; add rollback-safety note; add integration test declaration - FR-011: change implementation-prescriptive scope to observable outcome - FR-001: add auth-ordering exception for POST /analytics/content/event - Add UI health-check note to FR-002a and FR-003a Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
07760e6 to
d81e076
Compare
…, flag/gate contradiction
- FR-007: clarify context.experiments stripping is top-level only (shared
across all events in the batch, not per-event)
- FR-009: define whole-batch-reject semantics — if any event has a
non-pageview event_type the entire request is rejected with 403;
rationale: gate is atomic, silent partial drops obscure client bugs
- Key Entities: fix internal contradiction — describe general live-toggle
behavior and explicitly carve out the gate as startup-read-only exception
- FR-003: drop "and health" exception — FR-003a is the single source for
the /health path; FR-003 now covers all paths except siteauth
- FR-003a: clarify that /analytics/health changes from pass-through proxy
to a dotCMS-produced three-state response (OK/NOT_CONFIGURED/
CONFIGURATION_ERROR); call out as intentional scope beyond "add a gate"
- Event Payload entity: correct shape to {"context":{...},"events":[...]}
and note which gate logic operates per-event vs. top-level
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…cs, UI messages - Remove in-flight edge case (moot with startup-only gate reads) - FR-002a: clarify three-state health check already exists; only the 403 gate is new scope for this feature - FR-003a: document Analytics portlet frontend changes required — update health check client to consume OK/NOT_CONFIGURED/CONFIGURATION_ERROR; specify NOT_CONFIGURED and CONFIGURATION_ERROR analytics-specific messages - Legacy Considerations: add accepted-exception note for GET /analytics/health response shape change when flag=ON; defer to FR-003a for rationale - FR-011: state current live-toggle behavior explicitly, then declare gate MUST NOT use it; name divergent state as intentional; call wiring gate into live-refresh path an implementation error; fix MUST→MUST NOT - FR-003: drop stale "and health" exception (FR-003a is single source) - Key Entities: fix internal contradiction — gate reads startup-only, live-toggle preserved for all other consumers Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…cs-experiment-flag-gates
…h model - Gate ordering: pin feature-flag gate before persistenceMode=readonly check on POST /api/v1/analytics/content/event; 403 short-circuits the chain before persistenceMode is evaluated - FR-010: require FEATURE_DISABLED error code on all flag-disabled 403s to distinguish from SITE_ACCESS_DENIED on the same endpoints; add concrete response example and update Postman test assertion - FR-003a: preserve catch-all auth model — backend user required (401), site READ permission required (403 SITE_ACCESS_DENIED); both run before the feature-flag gate and before the health evaluation - FR-002a: clarify existing health check logic is unchanged; only the 403 gate is new scope for this feature Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Clarify that the true→false default change applies to every consumer of both flags across the system, not only the new gate — ensures a new deployment without explicit config is fully disabled with no internal consumers active. Explain why gate-only would create an inconsistent state. Remove implementation-specific class names. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Closes backward-compat scope question from review: no consumers other than the Analytics portlet are known to read GET /api/v1/analytics/health, so the FR-003a response-shape change affects only the portlet, which is updated as part of this feature. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…o and Arcadio feedback Replace all-or-nothing 403 batch rejection with a partial-success response model that mirrors CAEM's own contract: - POST /content/event never returns 403 for gate reasons; instead returns 202 (success > 0) or 400 (success == 0) with per-event outcomes - New response fields: discarded count, discardedEvents array (gate-filtered events with code: "FEATURE_DISABLED"), re-mapped CAEM error indexes - FR-006: both flags OFF → 400 ERROR, all events in discardedEvents - FR-007: analytics=ON experiments=OFF → strip context.experiments, forward all events; no discardedEvents (stripping is a context mutation) - FR-008: both ON → pass CAEM response through as-is - FR-009: analytics=OFF experiments=ON → per-event filtering; pageview forwarded, non-pageview discarded; 202/400 based on success count - FR-010: scoped to admin endpoints only; ingest uses code: FEATURE_DISABLED in discardedEvents, not the 403 error envelope - FR-011: live-toggle removal extended system-wide to all consumers of both flags, not only the gate; Key Entities updated accordingly - US2 scenario 2 updated to 202 PARTIAL_SUCCESS; scenario 2b added for all-non-pageview batch → 400 ERROR - US3 Independent Test updated: stripping produces no discardedEvents - US4 Independent Test and scenario 1 updated: 400 not 403 for ingest - Added SDK graceful degradation note (out of scope, follow-up task) - Contract Changes section rewritten to document new ingest response shape Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nly gates start/schedule Analytics cannot be enabled without experiments, eliminating the analytics=ON/experiments=OFF state entirely: - Remove User Story 3 (Only Analytics Enabled — impossible state) - Remove FR-007 (context.experiments stripping — only needed for the now-impossible analytics=ON/experiments=OFF case) - Remove Payload mutation section from Contract Changes - Remove SC-003 (analytics-only forwarding criterion) - Remove context.experiments edge case Experiment flag now only gates start/schedule operations: - Rewrite FR-001: only _start and scheduling operations return 403 FEATURE_DISABLED when FEATURE_FLAG_EXPERIMENTS=false; CRUD, results, _end, _cancel, isUserIncluded, and health remain accessible - Update FR-002: all endpoints function normally when flag=true, including start/scheduling - Update FR-002a: health endpoint is always accessible, no longer gated - Update US3 (Both OFF): scenario 2 now reflects only start/schedule blocked; other experiment operations accessible - Update SC-001: experiment start/scheduling blocked, not all endpoints - Renumber FR-008→FR-007, FR-009→FR-008, FR-010→FR-009, FR-011→FR-010 - Renumber SC-004→SC-003, SC-005→SC-004, SC-006→SC-005 - Simplify Event Payload entity: context.experiments forwarded as-is - Update Contract Changes experiments 403 scope to start/schedule only Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…add limited experiment tier
Analytics is now gated by Analytics App configuration (per-site), not a
flag. FEATURE_FLAG_EXPERIMENTS remains as the only flag and now controls
a limited experiment tier rather than a binary on/off gate.
Key changes:
- Remove FEATURE_FLAG_CONTENT_ANALYTICS system-wide; analytics enabled
when Analytics App is configured for the site
- Event ingest gated solely by App configuration (503 when not configured,
forward as-is when configured — regardless of experiments flag)
- Analytics reads return 503 (not 403) when App not configured
- Experiment flag now enables a limited tier: 1 free experiment, max 10
days, _start/_schedule/_abort/archive gated; CRUD/results always accessible
- Add US2: Limited Experiment Mode (flag=false, App configured, free slot)
with real event collection, 10-day UI date picker restriction, and
informational message
- Merge old US2/US3 into single "Limited/Disabled Mode" user story (US3)
- Add Analytics App as a Key Entity; remove FEATURE_FLAG_CONTENT_ANALYTICS
- Analytics health endpoint (GET /analytics/health) always returns
structured OK/NOT_CONFIGURED/CONFIGURATION_ERROR — never proxies raw CAEM
- Experiments health endpoint extended with tier, freeExperimentUsed,
and warning (ANALYTICS_DISABLED) fields
- Free slot count uses {RUNNING, SCHEDULED, ENDED} — ARCHIVED excluded
since archiving is blocked in limited mode
- 400 returned when _start duration exceeds 10 days (not silent cap)
- Remove Independent Test sections from user stories (redundant)
- Update Legacy Considerations, Assumptions, and SC-001–SC-004
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ing test declarations
- FR-009: remove siteauth from 403 scope; add unit test for error body contract
- FR-010: add test-suite audit requirement, scope constraint for live-toggle removal,
FEATURE_FLAG_CAEM_EXPERIMENT_RESULTS and ENABLE_EXPERIMENTS_AUTO_JS_INJECTION left intact
- FR-010: add unit test for default flip (false when flag absent)
- FR-003a: declare health response shape ({ "health": "OK/NOT_CONFIGURED/CONFIGURATION_ERROR" })
- FR-001 / Contract Changes: _end moved to blocked list in all limited-mode states
- FR-006: fix gate ordering (configuration gate → site_auth → persistenceMode → forwarding)
- Key Entities: fix 403 → 503 for analytics read endpoints; fix site_auth ordering note
- Legacy Considerations: acknowledge FR-010 consumer sweep may touch com.dotmarketing.*
- FR-002a: add FEATURE_FLAG_CAEM_EXPERIMENT_RESULTS scope note (new fields apply when true only)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… scope notes - Replace _abort with _cancel throughout (FR-002); implementation note mapping retained - Add _end to US2 blocked list and SC-001 (was missing despite FR-001/Contract Changes) - Clarify _end is gated by FEATURE_FLAG_EXPERIMENTS only — App config has no effect - Reword FEATURE_FLAG_CONTENT_ANALYTICS in Key Entities to "removed from new gate logic" - Add Legacy Considerations note: flag deleted when AnalyticsTrackWebInterceptor is removed Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds data-model.md documenting ExperimentsHealthView (tier/freeExperimentUsed/warning fields), updated TypeScript types, and gate state definitions. Adds contracts/ (experiment-gate.md and analytics-gate.md) with decision matrices, response shapes, and gate ordering. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rning/health tier="full" when FEATURE_FLAG_EXPERIMENTS=true regardless of App configuration. tier="limited" when flag=false. App configuration state is already communicated through the warning and health fields — tying tier to App config conflated two independent concerns (license level vs operational readiness). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ytics endpoints
Backend implementation of the experiment activation gates and Analytics App
configuration gates described in specs/37659-analytics-experiment-flag-gates.
Key changes:
- FEATURE_FLAG_EXPERIMENTS default flips true → false; live-toggle removed from
ConfigExperimentUtil.notify() — a server restart is required for flag changes
- ExperimentsHealthView replaces Map<String,Health> on GET /experiments/health,
carrying tier (FULL|LIMITED), freeExperimentUsed, and warning fields
- ExperimentLimitedModeGate encapsulates limited-mode start checks (slot, schedule,
10-day duration cap); ExperimentsAPIImpl.startLimitedScheduling() enforces the cap
- ExperimentsAPI.isFreeSlotUsed() queries {RUNNING,SCHEDULED,ENDED} experiments
- ExperimentsResource: start/end/cancel/archive gated; healthcheck() returns the new view
- EventAnalyticsProxyResource: new analyticsHealth() endpoint (always 200, never 503);
App-config gate on proxyGetRequest and proxyEventRequest (503 before site_auth)
- ContentAnalyticsUtil: isAppConfigured(Host) and resolveAnalyticsHealth(Host) added
- Language.properties: limited-mode and analytics-warning i18n keys added
- openapi.yaml regenerated
- Unit tests: 42 new/updated tests across 5 classes, all green
- Integration tests: EventAnalyticsProxyResourceIntegrationTest (MainSuite2a),
live-toggle removal test added to ExperimentsResourceIntegrationTest
- Postman: Experiment_Gate collection (9 tests, all passing against live server)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Pull Request Unsafe to Rollback!!!
|
…sponse docs Resolves three documentation gaps found by speckit-converge (Phase 8): - ExperimentsHealthView: remove `allowableValues` from @Schema on `health`, `tier`, and `warning` fields — the declared enum type already provides the values; keeping both caused each value to appear twice in openapi.yaml (e.g. "FULL", "LIMITED", "FULL", "LIMITED"), producing a malformed schema. - ExperimentsHealthView.Tier Javadoc: corrects "full"/"limited" → "FULL"/"LIMITED" to match the actual Jackson-serialized wire format. - ExperimentsResource: add @apiresponse annotations to start(), end(), cancel(), and archive() documenting 200/403/503/400 gate responses — the return type change from ResponseEntitySingleExperimentView to Response left these operations with empty response schemas in openapi.yaml. - Regenerate openapi.yaml after all annotation changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…R-003/US3/AC4)
GET /api/v1/experiments/{id}/results now returns 503 ANALYTICS_NOT_CONFIGURED
when the Analytics App is not configured for the site. Results depend on the
analytics backend; without it they are meaningless and must not be served.
Also adds @apiresponse documentation for the new 503 response case.
openapi.yaml regenerated.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Pull Request Unsafe to Rollback!!!
Note: no database migrations, Elasticsearch mapping changes, or structural storage changes were found in this PR ( |
…ponse Adds @jsonvalue overrides to ExperimentsHealthView.Tier and Warning so they serialize as "full"/"limited" and "analytics_disabled" instead of the default uppercase enum names. This aligns the wire format with the spec examples in FR-002a and data-model.md, while keeping Java enum names in UPPERCASE per convention. The FE TypeScript types and guards are updated separately (FE PR). openapi.yaml regenerated: tier enum now shows "full"/"limited". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The Warning enum now serializes as "analytics_disabled" (lowercase via @jsonvalue). Update the Postman test assertion from 'ANALYTICS_DISABLED' to 'analytics_disabled' to match the actual wire format and keep the test consistent with the Tier enum casing ("full"/"limited") agreed in this feature. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📐 Spec Viewer
FEATURE_FLAG_EXPERIMENTSdefault flipstrue→false. Any deployment that does not explicitly set this flag will have experiments blocked after this ships. Every active customer must have the flag explicitly set totruebefore release.FEATURE_FLAG_CONTENT_ANALYTICSis not changed — it remains in place forAnalyticsTrackWebInterceptor(legacy page-view tracking). Analytics availability in the new gate logic is driven by Analytics App configuration, not by this flag.Summary
FEATURE_FLAG_EXPERIMENTS(experiment tier) + per-site Analytics App configuration (analytics availability)503; health endpoints always return a structured responseGET /api/v1/analytics/healthchanged from raw CAEM proxy to dotCMS-evaluated{ "health": "OK" | "NOT_CONFIGURED" | "CONFIGURATION_ERROR" }FEATURE_FLAG_EXPERIMENTSlive-toggle removed system-wide; restart required for flag changesCloses
Closes #37659
Test plan
/speckit-planruns🤖 Generated with Claude Code