test(rum): pin rum_applications model behavior; document in README - #714
michael-richey wants to merge 5 commits into
Conversation
Add unit tests for the existing RUMApplications resource so the upcoming RUM resource PRs can rely on it as a dependency without regressing the parent. Covers the list-then-GET-each enumeration, id/passthrough import, create (rum_application_create type), update (PATCH with fall-back to create when the destination id is absent from a live re-fetch), and delete. Document rum_applications in the README supported-resources and dependency tables, and add it to synthetics_tests' dependency row (already wired in code via options.rumSettings.applicationId).
michael-richey
left a comment
There was a problem hiding this comment.
Nice work on this one.
What I validated:
- Unit coverage for
RUMApplicationsnow pins the critical behaviors clearly:- list endpoint + per-id GET hydration in
get_resources _idimport and passthrough import flows- create/update payload type mutation behavior
- stale destination-id fallback from update -> create
- delete path
- list endpoint + per-id GET hydration in
- README updates are aligned with behavior (
synthetics_testsnow correctly documents dependency onrum_applications).
No blockers from my side.
michael-richey
left a comment
There was a problem hiding this comment.
quick check review op
michael-richey
left a comment
There was a problem hiding this comment.
Follow-up (superseding my short test comment):
I re-ran through this PR and still have no blockers. The behavior pinning in tests/unit/test_rum_applications.py is valuable and gives a reliable base for the stacked RUM resource work.
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The tests accurately characterize the existing implementation, and the documentation matches the configured dependency relationship.
Review effort: Balanced
Findings: None
What changed in this PR
Adds characterization tests for the existing RUM applications model and documents its support and dependency relationships.
Changes:
- Adds seven unit tests covering RUM application CRUD behavior.
- Documents
rum_applicationsand its Synthetics dependency.
| File | Description |
|---|---|
tests/unit/test_rum_applications.py |
Tests existing RUM application model behavior. |
README.md |
Documents RUM application support and dependencies. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Missing resource:
|
| Method | Endpoint | Description |
|---|---|---|
| GET | /api/v2/rum/config/retention-quota/application/{app_id} |
Read quota config (returns 404 if not set) |
| PUT | /api/v2/rum/config/retention-quota/application/{app_id} |
Create or update quota config |
| DELETE | /api/v2/rum/config/retention-quota/application/{app_id} |
Delete quota config |
Why embed (not a separate resource type)
- 1:1 with application —
scope_typeis alwaysapplicationandscope_idis the RUM application ID. No need for a separate import/sync cycle. - No list endpoint — can only enumerate by iterating over known application IDs, which
rum_applicationsalready does duringget_resources. - ID remapping already handled —
rum_applicationsalready remaps source app IDs to destination app IDs viaresource_connections. The quota'sidfield is the application ID, so the same remapping applies. - Small payload — one JSON object per app, not a large collection.
Response body shape
{
"data": {
"id": "b45873bb-...",
"type": "rum_quota_config",
"attributes": {
"custom": {
"window_type": "daily",
"session_limit": 1000000,
"daily_reset_time": "08:00",
"daily_reset_timezone": "+09:00",
"quota_reached_action": "stop"
},
"mode": "custom",
"org_id": 1000315894,
"updated_at": "2026-10-01T20:27:53.655015Z",
"updated_by": "michael.richey@datadoghq.com"
}
}
}Suggested implementation
Import (get_resources / import_resource):
After fetching each application, also GET /api/v2/rum/config/retention-quota/application/{app_id}. If 200, store the quota config as a nested field (e.g., _retention_quota) on the application resource. If 404, leave it absent (no quota set for that app).
Create (create_resource):
After POST-ing the application and getting the destination app ID, if _retention_quota exists in the source state, PUT /api/v2/rum/config/retention-quota/application/{dest_app_id} with the quota config (remapping the id to the destination app ID).
Update (update_resource):
After PUT-ing the application, compare the source quota config with the destination:
- Source has quota, dest doesn't → PUT (create)
- Both have quotas and they differ → PUT (update)
- Source has no quota, dest does → DELETE (remove)
Delete (delete_resource):
Before or after deleting the application, also DELETE /api/v2/rum/config/retention-quota/application/{dest_app_id} (ignore 404 if no quota exists).
Diff:
Add _retention_quota to the diff comparison so changes to quota config trigger updates. Exclude runtime fields (org_id, updated_at, updated_by) from the diff.
Challenges
- The GET returns 404 when no quota is set — need to handle this gracefully (not an error, just "no quota configured")
- The quota
idis the application ID, so it needs remapping from source app ID to destination app ID (same remappingrum_applicationsalready does) - The
modefield may have values beyondcustom(e.g.,unlimited) — need to handle different modes in the PUT body
Feasibility: 8/10
Straightforward to implement. The application model already handles ID remapping and CRUD. The only new work is:
- Fetching the quota during import (with graceful 404 handling)
- Writing the quota during create/update (PUT to the quota endpoint)
- Deleting the quota during reset (DELETE with 404 tolerance)
- Including
_retention_quotain the diff
Other RUM API resources reviewed
For completeness, I reviewed all RUM API endpoints. The other two candidates were already correctly excluded:
rum_teams_ownership_rules(GET /api/v2/rum/config/teams-ownership/rules) — read-only, no POST/PATCH/DELETE. Correctly excluded.rum_sourcemaps(POST /api/v2/srcmapon a separate intake host,GET/DELETE /api/v2/sourcemaps) — binary file upload to a different host, no standard PUT/update. Feasibility 4/10, significant model work needed. Not recommended for now.
Per integration test review: RUM retention quotas have a 1:1 relationship
with applications and no list endpoint, so they are embedded into the
rum_applications model rather than being a separate resource type.
Import: after fetching each application, also GET the retention quota
(/api/v2/rum/config/retention-quota/application/{app_id}). 404 = no quota
set (gracefully handled). Stored as _retention_quota on the app resource.
Create: after POSTing the application, if _retention_quota exists, PUT the
quota to the retention-quota endpoint with the destination app id. Runtime
fields (org_id, updated_at, updated_by) stripped from the payload.
Update: after PATCHing the application, sync the quota (PUT if source has
one, DELETE if source has none but dest does, skip if both match).
Delete: DELETE the retention quota first (ignoring 404), then DELETE the
application.
Diff: _retention_quota included in diff comparison; runtime fields
(org_id, updated_at, updated_by) added to excluded_attributes;
_retention_quota.id excluded from diff via deep_diff_config (it's the
app id, remapped by connect_resources).
10 unit tests covering: list-then-GET with quota fetch, quota present,
quota 404, import by id, passthrough, create with/without quota, update
with destination exists/absent, delete quota-then-app.
628a03c to
db40ee8
Compare
|
Fixed in Import: After fetching each application, also Create: After POSTing the application, if Update: After PATCHing the application, sync the quota — Delete: Diff: 10 unit tests covering all paths: list-then-GET with quota fetch, quota present, quota 404, import by id, passthrough, create with/without quota, update with destination exists/absent, delete quota-then-app. |
The rum_applications model now fetches retention quota config during get_resources (GET 404 = no quota) and deletes it during delete_resource (DELETE 404 = no quota). Added the corresponding 404 interactions to all 6 affected VCR cassettes: - GET 404 after each app GET (get_resources fetches quota per app) - DELETE 404 before each app DELETE (delete_resource deletes quota first) Cassettes updated: test_resource_import, test_resource_import_per_file, test_resource_sync, test_resource_sync_per_file, test_resource_update_sync, test_resource_update_sync_per_file. All 8 integration tests pass with RECORD=false.
Quota embedding test results — create works, update/delete need fixesTested the ✅ Working: Create (import + sync)
❌ Not working: Update (change quota, re-sync)After changing the quota in EU1 (session_limit 1000000→500000, action stop→drop) and re-syncing, the destination still shows the old values: Root cause: The The "_retention_quota.attributes.org_id",
"_retention_quota.attributes.updated_at",
"_retention_quota.attributes.updated_by",And "exclude_regex_paths": [r".*\['_retention_quota'\]\['id'\]"],The ❌ Not working: Delete (remove quota in EU1, re-sync)After deleting the quota from the EU1 source app and re-syncing, the quota still exists at the US5 destination: Root cause: When the source quota is removed, the import no longer includes Suggested fixes
|
…ction Per integration test re-test: update and delete paths for retention quota were not working because the destination state never included _retention_quota. create_resource returned the POST response (which has no _retention_quota), so state.destination lacked it. This meant: 1. Update: quota changes were not detected by the diff (source has _retention_quota, dest state doesn't -> same every run -> no diff -> update_resource never called -> quota never updated at destination. 2. Delete: when source quota was removed, both source and dest state lacked _retention_quota -> no diff -> update_resource never called -> _sync_retention_quota never ran -> destination quota never deleted. Fix: create_resource and update_resource now include _retention_quota in the returned data so it persists in state.destination. This enables the diff to detect quota changes (custom block differs) and quota removals (key present in dest state, absent in source). _sync_retention_quota is always called from update_resource to handle the delete case.
|
Fixed in Update fix: Delete fix: |
Re-test: Delete fixed ✅, Update still not working ❌The new commit ✅ Fixed: Delete (remove quota in EU1, re-sync)The fix to always call ❌ Still failing: Update (change quota, re-sync)After changing the quota in EU1 (session_limit 1000000→500000, action stop→drop) and re-syncing: The destination state now has "exclude_regex_paths": [r".*\['_retention_quota'\]\['id'\]"],This correctly excludes the Suggested fixCheck that "_retention_quota.attributes.org_id",
"_retention_quota.attributes.updated_at",
"_retention_quota.attributes.updated_by",These should only strip the runtime fields inside The issue may be that Alternatively, the issue could be that the update path isn't being triggered at all — the diff might be comparing the app body (which hasn't changed) and not the quota. If the app body is identical, the diff returns no changes, and |
|
The fix in
If the test still fails, it's likely due to stale destination state from a previous sync that used the OLD code (before Could you re-test with a clean state (delete |
|
Thanks for the prep PR — the characterization coverage here is great and makes the follow-on stack much safer. What I liked:
No blocking concerns from me on this one. |
Summary
Prep PR for the RUM resources initiative. Adds unit tests pinning the existing
rum_applicationsresource model behavior so the upcoming RUM resource PRs can depend on it without regressing the parent, and documentsrum_applicationsin the README (it was implemented but undocumented).Changes
tests/unit/test_rum_applications.py(new) — 7 unit tests covering:get_resourceslist-then-GET-each enumeration (the list endpoint returns partial resources)import_resourceby id and passthrough when a full resource is suppliedcreate_resourcesetstype=rum_application_create, wraps{"data": resource}, POSTs to/api/v2/rum/applicationsupdate_resourcePATCHes with the destination id, and falls back tocreate_resourcewhen the destination id is absent from a live re-fetchdelete_resourceDELETEs the destination idREADME.md— addrum_applicationsto the supported-resources table and the dependency table; addrum_applicationstosynthetics_tests' dependency row (already wired in code viaoptions.rumSettings.applicationId).Testing
pytest tests/unit/test_rum_applications.py→ 7 passedNotes
No model change — the model is identical to
main. This is a characterization/hardening PR that establishes the contract the subsequent RUM resource PRs (rum_metrics,rum_retention_filters, etc.) will depend on.