From 3fd9df6a70b19742ad06d1db75efa9bc0069f7f8 Mon Sep 17 00:00:00 2001 From: "michael.richey" Date: Mon, 13 Apr 2026 15:55:42 -0400 Subject: [PATCH 1/4] perf: filter resources before building dependency graph When syncing with filters (e.g., 5 dashboards out of 1000 in state), get_dependency_graph() was iterating ALL resources and deepcopying each via _resource_connections(). Now it checks the resource filter first and skips non-matching resources, avoiding 995 unnecessary deepcopy calls. Also strips phantom dependency references (nodes in dep sets but not graph keys) to prevent TopologicalSorter from yielding implicit nodes that waste worker cycles, and fixes the return type annotation from List to Set. Co-Authored-By: Claude Opus 4.6 --- datadog_sync/utils/resources_handler.py | 21 +- tests/unit/test_dependency_graph.py | 419 ++++++++++++++++++++++++ tests/unit/test_report_e2e.py | 5 +- 3 files changed, 439 insertions(+), 6 deletions(-) create mode 100644 tests/unit/test_dependency_graph.py diff --git a/datadog_sync/utils/resources_handler.py b/datadog_sync/utils/resources_handler.py index 07930cb6..05edde98 100644 --- a/datadog_sync/utils/resources_handler.py +++ b/datadog_sync/utils/resources_handler.py @@ -38,7 +38,7 @@ def __init__(self, config: Configuration) -> None: self.sorter: Optional[TopologicalSorter] = None self.cleanup_sorter: Optional[TopologicalSorter] = None self.worker: Optional[Workers] = None - self._dependency_graph = Optional[Dict[Tuple[str, str], List[Tuple[str, str]]]] + self._dependency_graph: Optional[Dict[Tuple[str, str], Set[Tuple[str, str]]]] = None @staticmethod def _sanitize_reason(err: Exception) -> str: @@ -542,21 +542,34 @@ async def run_cleanup_sorter(self): await asyncio.sleep(0) - def get_dependency_graph(self) -> Tuple[Dict[Tuple[str, str], List[Tuple[str, str]]], Set[Tuple[str, str]]]: + def get_dependency_graph(self) -> Tuple[Dict[Tuple[str, str], Set[Tuple[str, str]]], Set[Tuple[str, str]]]: """Build the dependency graph for all resources. Returns: - Tuple[Dict[Tuple[str, str], List[Tuple[str, str]]], Set[Tuple[str, str]]]: Returns + Tuple[Dict[Tuple[str, str], Set[Tuple[str, str]]], Set[Tuple[str, str]]]: Returns a tuple of the dependency graph and missing resources. """ dependency_graph = {} missing_resources = set() - for resource_type, _id in self.config.state.get_all_resources(self.config.resources_arg).keys(): + for (resource_type, _id), resource in self.config.state.get_all_resources(self.config.resources_arg).items(): + r_class = self.config.resources[resource_type] + if not r_class.filter(resource): + continue + deps, missing = self._resource_connections(resource_type, _id) dependency_graph[(resource_type, _id)] = deps missing_resources.update(missing) + # Remove dependency references to nodes not in the graph. + # This prevents phantom nodes in the TopologicalSorter — deps that + # reference filtered-out or out-of-scope resources would otherwise + # appear as implicit nodes with no predecessors, get yielded by + # the sorter, dispatched to workers, and waste processing time. + graph_keys = set(dependency_graph.keys()) + for key in dependency_graph: + dependency_graph[key] = dependency_graph[key] & graph_keys + return dependency_graph, missing_resources def _resource_connections(self, resource_type: str, _id: str) -> Tuple[Set[Tuple[str, str]], Set[Tuple[str, str]]]: diff --git a/tests/unit/test_dependency_graph.py b/tests/unit/test_dependency_graph.py new file mode 100644 index 00000000..5058bfea --- /dev/null +++ b/tests/unit/test_dependency_graph.py @@ -0,0 +1,419 @@ +# Unless explicitly stated otherwise all files in this repository are licensed +# under the 3-clause BSD style license (see LICENSE). +# This product includes software developed at Datadog (https://www.datadoghq.com/). +# Copyright 2019 Datadog, Inc. + +import pytest +from unittest.mock import patch + +from datadog_sync.utils.resources_handler import ResourcesHandler +from datadog_sync.utils.filter import process_filters + + +@pytest.fixture +def graph_test(config): + """Provides (handler, config) with full state isolation. + + Saves config.filters, config.filter_operator, config.resources_arg, + and all source state before each test; restores after. + """ + saved_filters = config.filters + saved_operator = config.filter_operator + saved_resources_arg = config.resources_arg[:] + saved_source = {} + for rt in config.resources: + saved_source[rt] = dict(config.state.source[rt]) + + handler = ResourcesHandler(config) + + yield handler, config + + config.filters = saved_filters + config.filter_operator = saved_operator + config.resources_arg = saved_resources_arg + for rt in config.resources: + config.state.source[rt].clear() + config.state.source[rt].update(saved_source.get(rt, {})) + + +def setup_state(config, source_resources, resources_arg=None): + """Populate state.source and optionally set resources_arg. + + Clears ALL resource types in source state first. + """ + for rt in config.resources: + config.state.source[rt].clear() + for rt, resources in source_resources.items(): + for _id, resource in resources.items(): + config.state.source[rt][_id] = resource + if resources_arg is not None: + config.resources_arg = resources_arg + else: + config.resources_arg = list(source_resources.keys()) + + +def setup_filters(config, filter_strings, operator="OR"): + """Set filters on config from filter string list.""" + config.filters = process_filters(filter_strings) + config.filter_operator = operator + + +# --------------------------------------------------------------------------- +# GREEN tests — must pass before AND after the change +# --------------------------------------------------------------------------- + + +def test_no_filters_all_resources_in_graph(graph_test): + handler, config = graph_test + config.filters = {} + setup_state( + config, + { + "dashboards": { + "dash-1": {"id": "dash-1"}, + "dash-2": {"id": "dash-2"}, + "dash-3": {"id": "dash-3"}, + }, + "monitors": { + "mon-1": {"id": "mon-1", "name": "Monitor 1"}, + "mon-2": {"id": "mon-2", "name": "Monitor 2"}, + }, + }, + resources_arg=["dashboards", "monitors"], + ) + + graph, _ = handler.get_dependency_graph() + + assert len(graph) == 5 + assert ("dashboards", "dash-1") in graph + assert ("dashboards", "dash-2") in graph + assert ("dashboards", "dash-3") in graph + assert ("monitors", "mon-1") in graph + assert ("monitors", "mon-2") in graph + + +def test_filter_matching_all_resources_same_as_no_filter(graph_test): + handler, config = graph_test + setup_filters(config, ["Type=dashboards;Name=id;Value=.*"]) + setup_state( + config, + { + "dashboards": { + "dash-1": {"id": "dash-1"}, + "dash-2": {"id": "dash-2"}, + "dash-3": {"id": "dash-3"}, + }, + }, + resources_arg=["dashboards"], + ) + + graph, _ = handler.get_dependency_graph() + + assert len(graph) == 3 + + +def test_resources_with_no_connections_have_empty_deps(graph_test): + handler, config = graph_test + config.filters = {} + setup_state( + config, + { + "monitors": { + "mon-1": {"id": "mon-1", "name": "Monitor 1"}, + "mon-2": {"id": "mon-2", "name": "Monitor 2"}, + }, + }, + resources_arg=["monitors"], + ) + + graph, _ = handler.get_dependency_graph() + + assert len(graph) == 2 + assert graph[("monitors", "mon-1")] == set() + assert graph[("monitors", "mon-2")] == set() + + +def test_missing_dependencies_detected(graph_test): + handler, config = graph_test + config.filters = {} + setup_state( + config, + { + "dashboards": { + "dash-1": {"id": "dash-1", "widgets": [{"definition": {"alert_id": "mon-99"}}]}, + }, + }, + resources_arg=["dashboards", "monitors"], + ) + + _, missing = handler.get_dependency_graph() + + assert ("monitors", "mon-99") in missing + + +def test_empty_state_returns_empty_graph(graph_test): + handler, config = graph_test + config.filters = {} + setup_state(config, {}, resources_arg=["dashboards"]) + + graph, missing = handler.get_dependency_graph() + + assert graph == {} + assert missing == set() + + +def test_single_resource_no_deps(graph_test): + handler, config = graph_test + config.filters = {} + setup_state( + config, + {"monitors": {"mon-1": {"id": "mon-1", "name": "Monitor 1"}}}, + resources_arg=["monitors"], + ) + + graph, _ = handler.get_dependency_graph() + + assert len(graph) == 1 + assert graph[("monitors", "mon-1")] == set() + + +def test_cross_type_dep_preserved_when_both_sides_in_graph(graph_test): + handler, config = graph_test + setup_filters(config, ["Type=dashboards;Name=id;Value=^dash-1$"]) + setup_state( + config, + { + "dashboards": { + "dash-1": {"id": "dash-1", "widgets": [{"definition": {"alert_id": "mon-1"}}]}, + }, + "monitors": { + "mon-1": {"id": "mon-1", "name": "Monitor 1"}, + }, + }, + resources_arg=["dashboards", "monitors"], + ) + + graph, _ = handler.get_dependency_graph() + + assert ("dashboards", "dash-1") in graph + assert ("monitors", "mon-1") in graph + assert ("monitors", "mon-1") in graph[("dashboards", "dash-1")] + + +# --------------------------------------------------------------------------- +# RED tests — fail before the change, pass after +# --------------------------------------------------------------------------- + + +def test_filter_excludes_resources_from_graph(graph_test): + handler, config = graph_test + setup_filters(config, ["Type=dashboards;Name=id;Value=^dash-1$"]) + setup_state( + config, + { + "dashboards": { + "dash-1": {"id": "dash-1"}, + "dash-2": {"id": "dash-2"}, + "dash-3": {"id": "dash-3"}, + }, + }, + resources_arg=["dashboards"], + ) + + graph, _ = handler.get_dependency_graph() + + assert ("dashboards", "dash-1") in graph + assert ("dashboards", "dash-2") not in graph + assert ("dashboards", "dash-3") not in graph + + +def test_graph_size_matches_filtered_count(graph_test): + handler, config = graph_test + setup_filters( + config, + [ + "Type=dashboards;Name=id;Value=^dash-1$", + "Type=dashboards;Name=id;Value=^dash-5$", + ], + ) + setup_state( + config, + { + "dashboards": {f"dash-{i}": {"id": f"dash-{i}"} for i in range(1, 11)}, + }, + resources_arg=["dashboards"], + ) + + graph, _ = handler.get_dependency_graph() + + assert len(graph) == 2 + + +def test_phantom_deps_stripped_from_graph_values(graph_test): + handler, config = graph_test + config.filters = {} + setup_state( + config, + { + "dashboards": { + "dash-1": {"id": "dash-1", "widgets": [{"definition": {"alert_id": "mon-1"}}]}, + }, + "monitors": { + "mon-1": {"id": "mon-1", "name": "Monitor 1"}, + }, + }, + resources_arg=["dashboards"], + ) + + graph, _ = handler.get_dependency_graph() + + assert ("dashboards", "dash-1") in graph + assert ("monitors", "mon-1") not in graph[("dashboards", "dash-1")] + + +def test_filtered_resources_deps_not_computed(graph_test): + handler, config = graph_test + setup_filters(config, ["Type=dashboards;Name=id;Value=^dash-1$"]) + setup_state( + config, + { + "dashboards": { + "dash-1": {"id": "dash-1"}, + "dash-2": {"id": "dash-2"}, + }, + }, + resources_arg=["dashboards"], + ) + + with patch.object( + ResourcesHandler, + "_resource_connections", + wraps=handler._resource_connections, + return_value=(set(), set()), + ) as mock_rc: + graph, _ = handler.get_dependency_graph() + + call_args = [c[0] for c in mock_rc.call_args_list] + assert ("dashboards", "dash-1") in call_args + assert ("dashboards", "dash-2") not in call_args + + +def test_filter_on_one_type_doesnt_affect_other_types(graph_test): + handler, config = graph_test + setup_filters(config, ["Type=dashboards;Name=id;Value=^dash-1$"]) + setup_state( + config, + { + "dashboards": { + "dash-1": {"id": "dash-1"}, + "dash-2": {"id": "dash-2"}, + }, + "monitors": { + "mon-1": {"id": "mon-1", "name": "Monitor 1"}, + "mon-2": {"id": "mon-2", "name": "Monitor 2"}, + }, + }, + resources_arg=["dashboards", "monitors"], + ) + + graph, _ = handler.get_dependency_graph() + + assert ("dashboards", "dash-1") in graph + assert ("dashboards", "dash-2") not in graph + assert ("monitors", "mon-1") in graph + assert ("monitors", "mon-2") in graph + + +def test_or_filter_operator_includes_any_match(graph_test): + handler, config = graph_test + setup_filters( + config, + [ + "Type=dashboards;Name=id;Value=^dash-1$", + "Type=dashboards;Name=id;Value=^dash-3$", + ], + operator="OR", + ) + setup_state( + config, + { + "dashboards": { + "dash-1": {"id": "dash-1"}, + "dash-2": {"id": "dash-2"}, + "dash-3": {"id": "dash-3"}, + }, + }, + resources_arg=["dashboards"], + ) + + graph, _ = handler.get_dependency_graph() + + assert ("dashboards", "dash-1") in graph + assert ("dashboards", "dash-2") not in graph + assert ("dashboards", "dash-3") in graph + + +def test_and_filter_operator_requires_all_match(graph_test): + handler, config = graph_test + setup_filters( + config, + [ + "Type=dashboards;Name=id;Value=^dash-1$", + "Type=dashboards;Name=title;Value=^My Dashboard$", + ], + operator="AND", + ) + setup_state( + config, + { + "dashboards": { + "dash-1": {"id": "dash-1", "title": "My Dashboard"}, + "dash-2": {"id": "dash-2", "title": "My Dashboard"}, + }, + }, + resources_arg=["dashboards"], + ) + + graph, _ = handler.get_dependency_graph() + + assert ("dashboards", "dash-1") in graph + assert ("dashboards", "dash-2") not in graph + + +def test_all_resources_filtered_returns_empty_graph(graph_test): + handler, config = graph_test + setup_filters(config, ["Type=dashboards;Name=id;Value=^nonexistent$"]) + setup_state( + config, + { + "dashboards": { + "dash-1": {"id": "dash-1"}, + "dash-2": {"id": "dash-2"}, + }, + }, + resources_arg=["dashboards"], + ) + + graph, _ = handler.get_dependency_graph() + + assert graph == {} + + +def test_missing_deps_only_from_filtered_resources(graph_test): + handler, config = graph_test + setup_filters(config, ["Type=dashboards;Name=id;Value=^dash-1$"]) + setup_state( + config, + { + "dashboards": { + "dash-1": {"id": "dash-1", "widgets": [{"definition": {"alert_id": "mon-99"}}]}, + "dash-2": {"id": "dash-2", "widgets": [{"definition": {"alert_id": "mon-88"}}]}, + }, + }, + resources_arg=["dashboards", "monitors"], + ) + + _, missing = handler.get_dependency_graph() + + assert ("monitors", "mon-99") in missing + assert ("monitors", "mon-88") not in missing diff --git a/tests/unit/test_report_e2e.py b/tests/unit/test_report_e2e.py index 625a5c55..04779b0e 100644 --- a/tests/unit/test_report_e2e.py +++ b/tests/unit/test_report_e2e.py @@ -378,9 +378,10 @@ def test_filtered_resources_emitted(self, runner): outcomes = _parse_outcomes(ret.output) filtered = [o for o in outcomes if o["status"] == "filtered"] non_filtered = [o for o in outcomes if o["status"] != "filtered"] - # def-456 and ghi-789 should be filtered out, only abc-123 passes + # def-456 and ghi-789 are excluded before graph construction, + # so no "filtered" events are emitted. Only abc-123 passes. assert len(non_filtered) == 1, f"Expected exactly 1 non-filtered, got {non_filtered}" - assert len(filtered) == 2, f"Expected exactly 2 filtered outcomes, got {filtered}" + assert len(filtered) == 0, f"Expected 0 filtered outcomes, got {filtered}" class TestDeleteOutcome: From 3206facde163322d39967227d736a92f17e99308 Mon Sep 17 00:00:00 2001 From: "michael.richey" Date: Mon, 13 Apr 2026 16:26:10 -0400 Subject: [PATCH 2/4] fix: only strip filtered-out deps, preserve cross-type phantom deps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The blanket `dependency_graph[key] & graph_keys` stripped ALL deps not explicitly in the graph, including cross-type phantom deps (e.g., users → roles when --resources=users). Those phantom nodes are essential for TopologicalSorter to yield dependencies before dependents. Replace with targeted `dependency_graph[key] - filtered_out` that only removes deps pointing to resources explicitly excluded by --filter. Also removes 2 redundant tests and moves test_cross_type_phantom_deps_preserved to the GREEN section since it passes on main too. Co-Authored-By: Claude Opus 4.6 --- datadog_sync/utils/resources_handler.py | 17 ++++--- tests/unit/test_dependency_graph.py | 68 +++++++++++-------------- 2 files changed, 38 insertions(+), 47 deletions(-) diff --git a/datadog_sync/utils/resources_handler.py b/datadog_sync/utils/resources_handler.py index 05edde98..d4fffa67 100644 --- a/datadog_sync/utils/resources_handler.py +++ b/datadog_sync/utils/resources_handler.py @@ -551,24 +551,25 @@ def get_dependency_graph(self) -> Tuple[Dict[Tuple[str, str], Set[Tuple[str, str """ dependency_graph = {} missing_resources = set() + filtered_out = set() for (resource_type, _id), resource in self.config.state.get_all_resources(self.config.resources_arg).items(): r_class = self.config.resources[resource_type] if not r_class.filter(resource): + filtered_out.add((resource_type, _id)) continue deps, missing = self._resource_connections(resource_type, _id) dependency_graph[(resource_type, _id)] = deps missing_resources.update(missing) - # Remove dependency references to nodes not in the graph. - # This prevents phantom nodes in the TopologicalSorter — deps that - # reference filtered-out or out-of-scope resources would otherwise - # appear as implicit nodes with no predecessors, get yielded by - # the sorter, dispatched to workers, and waste processing time. - graph_keys = set(dependency_graph.keys()) - for key in dependency_graph: - dependency_graph[key] = dependency_graph[key] & graph_keys + # Remove dependency references to filtered-out resources only. + # Cross-type deps on resource types outside resources_arg must be + # preserved as phantom nodes — TopologicalSorter yields them first, + # ensuring dependencies are synced before dependents. + if filtered_out: + for key in dependency_graph: + dependency_graph[key] = dependency_graph[key] - filtered_out return dependency_graph, missing_resources diff --git a/tests/unit/test_dependency_graph.py b/tests/unit/test_dependency_graph.py index 5058bfea..e9cb383e 100644 --- a/tests/unit/test_dependency_graph.py +++ b/tests/unit/test_dependency_graph.py @@ -162,24 +162,34 @@ def test_empty_state_returns_empty_graph(graph_test): assert missing == set() -def test_single_resource_no_deps(graph_test): +def test_cross_type_dep_preserved_when_both_sides_in_graph(graph_test): handler, config = graph_test - config.filters = {} + setup_filters(config, ["Type=dashboards;Name=id;Value=^dash-1$"]) setup_state( config, - {"monitors": {"mon-1": {"id": "mon-1", "name": "Monitor 1"}}}, - resources_arg=["monitors"], + { + "dashboards": { + "dash-1": {"id": "dash-1", "widgets": [{"definition": {"alert_id": "mon-1"}}]}, + }, + "monitors": { + "mon-1": {"id": "mon-1", "name": "Monitor 1"}, + }, + }, + resources_arg=["dashboards", "monitors"], ) graph, _ = handler.get_dependency_graph() - assert len(graph) == 1 - assert graph[("monitors", "mon-1")] == set() + assert ("dashboards", "dash-1") in graph + assert ("monitors", "mon-1") in graph + assert ("monitors", "mon-1") in graph[("dashboards", "dash-1")] -def test_cross_type_dep_preserved_when_both_sides_in_graph(graph_test): +def test_cross_type_phantom_deps_preserved(graph_test): + """Deps on out-of-scope resource types are preserved as phantom nodes + so TopologicalSorter yields them first, ensuring correct ordering.""" handler, config = graph_test - setup_filters(config, ["Type=dashboards;Name=id;Value=^dash-1$"]) + config.filters = {} setup_state( config, { @@ -190,13 +200,12 @@ def test_cross_type_dep_preserved_when_both_sides_in_graph(graph_test): "mon-1": {"id": "mon-1", "name": "Monitor 1"}, }, }, - resources_arg=["dashboards", "monitors"], + resources_arg=["dashboards"], ) graph, _ = handler.get_dependency_graph() assert ("dashboards", "dash-1") in graph - assert ("monitors", "mon-1") in graph assert ("monitors", "mon-1") in graph[("dashboards", "dash-1")] @@ -227,39 +236,17 @@ def test_filter_excludes_resources_from_graph(graph_test): assert ("dashboards", "dash-3") not in graph -def test_graph_size_matches_filtered_count(graph_test): +def test_filtered_out_deps_stripped_from_graph_values(graph_test): + """Deps pointing to resources excluded by --filter are stripped, + preventing phantom nodes for resources we don't want to process.""" handler, config = graph_test - setup_filters( - config, - [ - "Type=dashboards;Name=id;Value=^dash-1$", - "Type=dashboards;Name=id;Value=^dash-5$", - ], - ) - setup_state( - config, - { - "dashboards": {f"dash-{i}": {"id": f"dash-{i}"} for i in range(1, 11)}, - }, - resources_arg=["dashboards"], - ) - - graph, _ = handler.get_dependency_graph() - - assert len(graph) == 2 - - -def test_phantom_deps_stripped_from_graph_values(graph_test): - handler, config = graph_test - config.filters = {} + setup_filters(config, ["Type=dashboards;Name=id;Value=^dash-1$"]) setup_state( config, { "dashboards": { - "dash-1": {"id": "dash-1", "widgets": [{"definition": {"alert_id": "mon-1"}}]}, - }, - "monitors": { - "mon-1": {"id": "mon-1", "name": "Monitor 1"}, + "dash-1": {"id": "dash-1"}, + "dash-2": {"id": "dash-2"}, }, }, resources_arg=["dashboards"], @@ -268,7 +255,10 @@ def test_phantom_deps_stripped_from_graph_values(graph_test): graph, _ = handler.get_dependency_graph() assert ("dashboards", "dash-1") in graph - assert ("monitors", "mon-1") not in graph[("dashboards", "dash-1")] + assert ("dashboards", "dash-2") not in graph + # dash-2 was filtered out, so any dep referencing it should be stripped + for deps in graph.values(): + assert ("dashboards", "dash-2") not in deps def test_filtered_resources_deps_not_computed(graph_test): From 716d2ff9d5e63fd7074dc7777ff259d212ecc08d Mon Sep 17 00:00:00 2001 From: "michael.richey" Date: Mon, 13 Apr 2026 16:34:56 -0400 Subject: [PATCH 3/4] =?UTF-8?q?fix:=20resolve=20review=20issues=20?= =?UTF-8?q?=E2=80=94=20filtered=20counter,=20dead=20code,=20test=20cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Return filtered_count from get_dependency_graph() and set it on worker.counter so the "Filtered: N" log line is accurate for sync - Fix reset_counter() to also zero the filtered field (pre-existing bug) - Remove dead filter checks in _apply_resource_cb and _diffs_worker_cb (filtered resources are now excluded at the graph level) - Rename test_filtered_resources_emitted to test_filtered_resources_excluded_from_graph - Fix mock using both wraps= and return_value= (return_value wins, wraps was silently ignored) Co-Authored-By: Claude Opus 4.6 --- datadog_sync/utils/resources_handler.py | 26 +++++++-------------- datadog_sync/utils/workers.py | 2 +- tests/unit/test_dependency_graph.py | 31 ++++++++++++------------- tests/unit/test_report_e2e.py | 4 ++-- 4 files changed, 26 insertions(+), 37 deletions(-) diff --git a/datadog_sync/utils/resources_handler.py b/datadog_sync/utils/resources_handler.py index d4fffa67..4951e84e 100644 --- a/datadog_sync/utils/resources_handler.py +++ b/datadog_sync/utils/resources_handler.py @@ -109,7 +109,7 @@ async def reset(self) -> None: async def apply_resources(self) -> Tuple[int, int]: # Build dependency graph and missing resources - self._dependency_graph, missing = self.get_dependency_graph() + self._dependency_graph, missing, filtered_count = self.get_dependency_graph() # Import resources that are missing but needed for resource connections if self.config.force_missing_dependencies and missing: @@ -206,6 +206,7 @@ async def apply_resources(self) -> Tuple[int, int]: ) else: await self.worker.schedule_workers(additional_coros=[self.run_sorter()]) + self.worker.counter.filtered = filtered_count self.config.logger.info(f"finished syncing resource items: {self.worker.counter}.") self.config.state.dump_state() @@ -216,14 +217,6 @@ async def _apply_resource_cb(self, q_item: List) -> None: try: r_class = self.config.resources[resource_type] - - # Filter BEFORE deepcopy to avoid unnecessary memory allocation. - # Safe because filter() only reads the resource dict, never mutates it. - if not r_class.filter(self.config.state.source[resource_type][_id]): - self.worker.counter.increment_filtered() - self._emit(resource_type, _id, "sync", "filtered") - return - resource = deepcopy(self.config.state.source[resource_type][_id]) if not r_class.resource_config.concurrent: @@ -283,7 +276,7 @@ async def _apply_resource_cb(self, q_item: List) -> None: r_class.resource_config.async_lock.release() async def diffs(self) -> None: - self._dependency_graph, _ = self.get_dependency_graph() + self._dependency_graph, _, _ = self.get_dependency_graph() # Run pre-apply hooks resource_types = set(i[0] for i in self._dependency_graph.keys()) @@ -318,10 +311,6 @@ async def _diffs_worker_cb(self, q_item: List) -> None: else: resource = self.config.state.source[resource_type][_id] - if not r_class.filter(resource): - self._emit(resource_type, _id, "sync", "filtered") - return - try: await r_class._pre_resource_action_hook(_id, resource) except SkipResource as e: @@ -542,12 +531,13 @@ async def run_cleanup_sorter(self): await asyncio.sleep(0) - def get_dependency_graph(self) -> Tuple[Dict[Tuple[str, str], Set[Tuple[str, str]]], Set[Tuple[str, str]]]: + def get_dependency_graph( + self, + ) -> Tuple[Dict[Tuple[str, str], Set[Tuple[str, str]]], Set[Tuple[str, str]], int]: """Build the dependency graph for all resources. Returns: - Tuple[Dict[Tuple[str, str], Set[Tuple[str, str]]], Set[Tuple[str, str]]]: Returns - a tuple of the dependency graph and missing resources. + Tuple of (dependency_graph, missing_resources, filtered_count). """ dependency_graph = {} missing_resources = set() @@ -571,7 +561,7 @@ def get_dependency_graph(self) -> Tuple[Dict[Tuple[str, str], Set[Tuple[str, str for key in dependency_graph: dependency_graph[key] = dependency_graph[key] - filtered_out - return dependency_graph, missing_resources + return dependency_graph, missing_resources, len(filtered_out) def _resource_connections(self, resource_type: str, _id: str) -> Tuple[Set[Tuple[str, str]], Set[Tuple[str, str]]]: """Returns the failed connections and missing resources for a given resource. diff --git a/datadog_sync/utils/workers.py b/datadog_sync/utils/workers.py index 50e0e224..7cd66cb3 100644 --- a/datadog_sync/utils/workers.py +++ b/datadog_sync/utils/workers.py @@ -117,7 +117,7 @@ def __str__(self): ) def reset_counter(self) -> None: - self.successes = self.failure = self.skipped = 0 + self.successes = self.failure = self.skipped = self.filtered = 0 def increment_success(self) -> None: self.successes += 1 diff --git a/tests/unit/test_dependency_graph.py b/tests/unit/test_dependency_graph.py index e9cb383e..f79e06bb 100644 --- a/tests/unit/test_dependency_graph.py +++ b/tests/unit/test_dependency_graph.py @@ -82,7 +82,7 @@ def test_no_filters_all_resources_in_graph(graph_test): resources_arg=["dashboards", "monitors"], ) - graph, _ = handler.get_dependency_graph() + graph, _, _ = handler.get_dependency_graph() assert len(graph) == 5 assert ("dashboards", "dash-1") in graph @@ -107,7 +107,7 @@ def test_filter_matching_all_resources_same_as_no_filter(graph_test): resources_arg=["dashboards"], ) - graph, _ = handler.get_dependency_graph() + graph, _, _ = handler.get_dependency_graph() assert len(graph) == 3 @@ -126,7 +126,7 @@ def test_resources_with_no_connections_have_empty_deps(graph_test): resources_arg=["monitors"], ) - graph, _ = handler.get_dependency_graph() + graph, _, _ = handler.get_dependency_graph() assert len(graph) == 2 assert graph[("monitors", "mon-1")] == set() @@ -146,7 +146,7 @@ def test_missing_dependencies_detected(graph_test): resources_arg=["dashboards", "monitors"], ) - _, missing = handler.get_dependency_graph() + _, missing, _ = handler.get_dependency_graph() assert ("monitors", "mon-99") in missing @@ -156,7 +156,7 @@ def test_empty_state_returns_empty_graph(graph_test): config.filters = {} setup_state(config, {}, resources_arg=["dashboards"]) - graph, missing = handler.get_dependency_graph() + graph, missing, _ = handler.get_dependency_graph() assert graph == {} assert missing == set() @@ -178,7 +178,7 @@ def test_cross_type_dep_preserved_when_both_sides_in_graph(graph_test): resources_arg=["dashboards", "monitors"], ) - graph, _ = handler.get_dependency_graph() + graph, _, _ = handler.get_dependency_graph() assert ("dashboards", "dash-1") in graph assert ("monitors", "mon-1") in graph @@ -203,7 +203,7 @@ def test_cross_type_phantom_deps_preserved(graph_test): resources_arg=["dashboards"], ) - graph, _ = handler.get_dependency_graph() + graph, _, _ = handler.get_dependency_graph() assert ("dashboards", "dash-1") in graph assert ("monitors", "mon-1") in graph[("dashboards", "dash-1")] @@ -229,7 +229,7 @@ def test_filter_excludes_resources_from_graph(graph_test): resources_arg=["dashboards"], ) - graph, _ = handler.get_dependency_graph() + graph, _, _ = handler.get_dependency_graph() assert ("dashboards", "dash-1") in graph assert ("dashboards", "dash-2") not in graph @@ -252,7 +252,7 @@ def test_filtered_out_deps_stripped_from_graph_values(graph_test): resources_arg=["dashboards"], ) - graph, _ = handler.get_dependency_graph() + graph, _, _ = handler.get_dependency_graph() assert ("dashboards", "dash-1") in graph assert ("dashboards", "dash-2") not in graph @@ -279,9 +279,8 @@ def test_filtered_resources_deps_not_computed(graph_test): ResourcesHandler, "_resource_connections", wraps=handler._resource_connections, - return_value=(set(), set()), ) as mock_rc: - graph, _ = handler.get_dependency_graph() + graph, _, _ = handler.get_dependency_graph() call_args = [c[0] for c in mock_rc.call_args_list] assert ("dashboards", "dash-1") in call_args @@ -306,7 +305,7 @@ def test_filter_on_one_type_doesnt_affect_other_types(graph_test): resources_arg=["dashboards", "monitors"], ) - graph, _ = handler.get_dependency_graph() + graph, _, _ = handler.get_dependency_graph() assert ("dashboards", "dash-1") in graph assert ("dashboards", "dash-2") not in graph @@ -336,7 +335,7 @@ def test_or_filter_operator_includes_any_match(graph_test): resources_arg=["dashboards"], ) - graph, _ = handler.get_dependency_graph() + graph, _, _ = handler.get_dependency_graph() assert ("dashboards", "dash-1") in graph assert ("dashboards", "dash-2") not in graph @@ -364,7 +363,7 @@ def test_and_filter_operator_requires_all_match(graph_test): resources_arg=["dashboards"], ) - graph, _ = handler.get_dependency_graph() + graph, _, _ = handler.get_dependency_graph() assert ("dashboards", "dash-1") in graph assert ("dashboards", "dash-2") not in graph @@ -384,7 +383,7 @@ def test_all_resources_filtered_returns_empty_graph(graph_test): resources_arg=["dashboards"], ) - graph, _ = handler.get_dependency_graph() + graph, _, _ = handler.get_dependency_graph() assert graph == {} @@ -403,7 +402,7 @@ def test_missing_deps_only_from_filtered_resources(graph_test): resources_arg=["dashboards", "monitors"], ) - _, missing = handler.get_dependency_graph() + _, missing, _ = handler.get_dependency_graph() assert ("monitors", "mon-99") in missing assert ("monitors", "mon-88") not in missing diff --git a/tests/unit/test_report_e2e.py b/tests/unit/test_report_e2e.py index 04779b0e..deb16456 100644 --- a/tests/unit/test_report_e2e.py +++ b/tests/unit/test_report_e2e.py @@ -356,9 +356,9 @@ def test_update_coexists_with_create(self, runner): class TestFilteredOutcome: - """Test that --filter excludes resources and emits filtered status.""" + """Test that --filter excludes resources from the dependency graph.""" - def test_filtered_resources_emitted(self, runner): + def test_filtered_resources_excluded_from_graph(self, runner): _setup_source_dashboards() _setup_dest_dashboards() From fab27ea6a4173d9516dc3de9b7b1196d6868849a Mon Sep 17 00:00:00 2001 From: "michael.richey" Date: Tue, 14 Apr 2026 09:40:24 -0400 Subject: [PATCH 4/4] fix: restore filtered resource outcome emissions for --json consumers Co-Authored-By: Claude Opus 4.6 --- datadog_sync/utils/resources_handler.py | 4 ++++ tests/unit/test_report_e2e.py | 10 +++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/datadog_sync/utils/resources_handler.py b/datadog_sync/utils/resources_handler.py index 4951e84e..a875a936 100644 --- a/datadog_sync/utils/resources_handler.py +++ b/datadog_sync/utils/resources_handler.py @@ -553,6 +553,10 @@ def get_dependency_graph( dependency_graph[(resource_type, _id)] = deps missing_resources.update(missing) + # Emit filtered outcomes so --json consumers see which resources were excluded. + for resource_type, _id in filtered_out: + self._emit(resource_type, _id, "sync", "filtered") + # Remove dependency references to filtered-out resources only. # Cross-type deps on resource types outside resources_arg must be # preserved as phantom nodes — TopologicalSorter yields them first, diff --git a/tests/unit/test_report_e2e.py b/tests/unit/test_report_e2e.py index deb16456..6fdeafd6 100644 --- a/tests/unit/test_report_e2e.py +++ b/tests/unit/test_report_e2e.py @@ -356,9 +356,9 @@ def test_update_coexists_with_create(self, runner): class TestFilteredOutcome: - """Test that --filter excludes resources from the dependency graph.""" + """Test that --filter emits filtered outcomes for excluded resources.""" - def test_filtered_resources_excluded_from_graph(self, runner): + def test_filtered_resources_emitted(self, runner): _setup_source_dashboards() _setup_dest_dashboards() @@ -378,10 +378,10 @@ def test_filtered_resources_excluded_from_graph(self, runner): outcomes = _parse_outcomes(ret.output) filtered = [o for o in outcomes if o["status"] == "filtered"] non_filtered = [o for o in outcomes if o["status"] != "filtered"] - # def-456 and ghi-789 are excluded before graph construction, - # so no "filtered" events are emitted. Only abc-123 passes. + # abc-123 passes the filter; def-456 and ghi-789 are filtered out + # and emitted as "filtered" outcomes. assert len(non_filtered) == 1, f"Expected exactly 1 non-filtered, got {non_filtered}" - assert len(filtered) == 0, f"Expected 0 filtered outcomes, got {filtered}" + assert len(filtered) == 2, f"Expected 2 filtered outcomes, got {filtered}" class TestDeleteOutcome: