feat(rum): add rum_retention_filters resource

[michael-richey]

Summary

Adds rum_retention_filters, unifying the generic retention_filters and exclusion_filters types (distinguished by data.type), which live on separate sub-paths under /api/v2/rum/applications/{app_id}.

Retention filters are parent-scoped under a RUM application, but the application id is not part of the filter body. The model injects a synthetic _application_id during enumeration and remaps it (source app id → destination app id) via resource_connections before apply.

Stacked on #715 (rum_metrics), which is stacked on #714 (rum_applications prep).

Design notes

• prep_resource runs after connect_resources and removes excluded_attributes. So _application_id is deliberately not in excluded_attributes (it must survive prep so create/update can read it) and is instead kept out of diffs via deep_diff_config.exclude_regex_paths — the same pattern users.py uses for handle/service_account.
• id is excluded (server-assigned on create; update rewrites it to the destination id).
• Type-based routing: retention_filters → /retention_filters/{rf_id}; exclusion_filters → /retention_filters/exclusion/{ef_id}.
• Create data has no id (popped); update sets resource["id"] = destination_id.

Changes

• datadog_sync/model/rum_retention_filters.py (new) — RUMRetentionFilters
• datadog_sync/models/__init__.py — register RUMRetentionFilters
• tests/unit/test_rum_retention_filters.py (new) — 9 unit tests
• README.md — add rum_retention_filters (depends on rum_applications)

Testing

• pytest tests/unit/test_rum_retention_filters.py → 9 passed
• Full unit suite: 1485 passed, 5 pre-existing subprocess-test failures unchanged, 8 skipped.

Follow-ups

• Order syncing is a separate rum_retention_filters_order resource (next PR), matching the codebase convention (logs_archives_order, sensitive_data_scanner_groups_order, logs_indexes_order, logs_pipelines_order). Order must be applied after filters exist at the destination, so a pre_apply_hook (which runs before apply) is incorrect; a dedicated order resource synced after the filters is the only correct design.
• Integration tests + VCR cassettes are deferred (require sandbox-org API access to record). Unit tests fully cover model behavior via AsyncMock clients.

[michael-richey]

Good implementation overall, and the tests are thorough for both generic and exclusion filter paths.

What I liked:

• Parent-scoped handling is explicit and readable.
• _application_id remapping pattern is applied consistently.
• CRUD path switching via type (retention_filters vs exclusion_filters) is well covered by tests.

No blockers in this PR diff.

[Copilot]

Copilot review overview

🟡 Changes recommended

Destination filters cannot be reconciled when persisted source-to-destination state is unavailable, risking duplicates or conflicts.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

• Reconcile existing destination filters before creating · New

What changed in this PR

Adds parent-scoped RUM retention and exclusion filter synchronization.

Changes:

• Implements enumeration, ID remapping, and CRUD routing.
• Registers and documents the resource.
• Adds unit coverage for primary workflows.

File	Description
datadog_sync/​model/​rum_retention_filters.py	Implements the resource model and CRUD behavior.
datadog_sync/​models/​__init__.py	Registers the model.
tests/​unit/​test_rum_retention_filters.py	Tests routing, remapping, and CRUD operations.
README.md	Documents support and dependencies.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[michael-richey]

Fixed in 7cd1ba2d. create_resource now reconciles against the destination before POSTing: it GETs the destination app's filter list and checks for a matching filter (scoped by app + type + name). If found, it hydrates state.destination and delegates to update_resource instead of creating a duplicate. New test verifies the reconciliation path.

[michael-richey]

Integration Test Findings (EU1 → US5 sync)

Ran a full integration test: imported all RUM resources from the EU1 internal testing org, then synced to the US5 internal testing org. Found 3 failures in rum_retention_filters:

1. default_sessions — 400 Bad Request

attribute "source" must be one of "ui terraform"

The sync is sending a source attribute on this system-provisioned default filter that the API rejects. The source field must be "ui" or "terraform", but the imported state either has a different value or is missing it. These default filters already exist in the destination org, so the failure is on the update/PATCH path.

2. default_errors — 400 Bad Request

attribute "source" must be one of "ui terraform"

Same root cause as default_sessions — the source attribute is being sent with an invalid value on a system-provisioned default filter.

3. error_tracking_exclusion_filter — 400 Bad Request

only 'enabled' can be updated on the error tracking exclusion filter

The sync is trying to update fields beyond enabled (e.g. name, query) on this special exclusion filter. The API only allows toggling the enabled boolean. The model needs special handling to only send the enabled field on update for this filter type.

Suggested fixes

1. For default_sessions and default_errors: don't send the source attribute on update (or set it to "ui" / "terraform" as the API requires).
2. For error_tracking_exclusion_filter: only send the enabled field in the PATCH body — strip all other attributes.

[michael-richey]

Fixed in 8c284702. Two issues addressed:

1. default_sessions / default_errors (400 'attribute source must be one of ui terraform'): The source field is a runtime-only response attribute not in the OpenAPI spec. Added attributes.source to excluded_attributes so prep_resource strips it before the PATCH.

2. error_tracking_exclusion_filter (400 'only enabled can be updated on the error tracking exclusion filter'): update_resource now detects when the destination id is error_tracking_exclusion_filter and strips all attributes except enabled from the PATCH body.

New test verifies only enabled is sent for the error_tracking filter.

[michael-richey]

Re-test Results (after fix 8c28470)

Re-ran the full integration test with the updated branch. Partial success:

✅ Fixed: error_tracking_exclusion_filter

No longer fails — the fix to only send enabled on update works correctly.

❌ Still failing: default_sessions and default_errors

400 Bad Request - attribute "source" must be one of "ui terraform"

Root cause found: The source field is in meta, not in attributes. The raw source state shows:

"meta": {"source": "default", "updated_at": 1790715841247, "updated_by_handle": "Datadog", "edition_mode": "complete"}

The excluded_attributes list has "attributes.source" which strips source from attributes — but source is actually under meta, so it's never stripped. The PATCH body includes meta.source: "default", which the API rejects because "default" is not "ui" or "terraform".

Suggested fix: Add "meta.source" (or the entire "meta" key) to excluded_attributes so it's stripped from the PATCH body. The meta object contains runtime-only fields (updated_at, updated_by_handle, edition_mode, source) that shouldn't be sent in create/update requests.

[michael-richey]

Additional Finding: error_tracking_exclusion_filter cannot be deleted

During the lifecycle test (reset/delete phase), rum_retention_filters delete also fails:

ERROR - error deleting resource rum_retention_filters with id error_tracking_exclusion_filter

The error_tracking_exclusion_filter is a special exclusion filter that likely cannot be deleted via the API (it's a system-provisioned filter). The delete_resource method should handle this gracefully — either skip the delete for exclusion filters, or catch the API error and log a warning instead of failing.

Lifecycle test results for rum_retention_filters

Scenario	Result	Details
Create	❌ FAIL	default_sessions/default_errors: source in meta not stripped (previously reported)
Update	❌ FAIL	Same source/meta bug on update path
Skip	✅ PASS	No diff detected when source and destination match
Delete	❌ FAIL	error_tracking_exclusion_filter cannot be deleted (new finding)

Summary of all issues on this PR

1. default_sessions / default_errors: meta.source: "default" sent in PATCH body → 400. Fix attempted in 8c28470 added "attributes.source" to excluded_attributes, but source is under meta, not attributes. Need to add "meta" or "meta.source" to excluded_attributes.
2. error_tracking_exclusion_filter update: Fixed ✅ — only sends enabled on update.
3. error_tracking_exclusion_filter delete: New finding — API rejects deletion. Need to skip or handle gracefully.

[michael-richey]

Fixed in dec60672. Two issues:

1. source under meta, not attributes: Changed excluded_attributes from "attributes.source" to "meta" — the entire meta object (containing source, updated_at, updated_by_handle, edition_mode) is now stripped by prep_resource before the PATCH.

2. error_tracking_exclusion_filter cannot be deleted: delete_resource now detects this filter id and skips the delete with a warning instead of failing. New test verifies the skip.

[michael-richey]

Thanks for building this out — the API coverage is valuable. I did find one correctness issue to address before merge:

Potential key collision across applications

get_resources() currently stores exclusion/retention filters keyed by resource["id"] alone. That looks unsafe for app-scoped filters.

For exclusion filters specifically, Datadog’s API docs indicate the built-in Error Tracking filter uses the fixed id error_tracking_exclusion_filter and is returned for each RUM app. With multiple apps, this key will collide and later apps overwrite earlier ones in the source state.

Impact:

• Import/sync state for multi-app orgs can drop/overwrite filters.
• Downstream resources that rely on these ids (notably ordering in the next PR) can remap ambiguously.

Suggested fix:

• Use a composite key including app id, e.g. <app_id>:<filter_id> (similar to rum_permanent_retention_filters).
• Keep raw id in payload as-is for API calls, but key local state by composite id.
• Add a multi-app test case proving no collisions for built-in/default filter ids.

Once that is in, this should be in great shape.