From f37b5833f7636a73ab87b59a5838f26efc437107 Mon Sep 17 00:00:00 2001 From: riyazsh <126122908+riyazsh@users.noreply.github.com> Date: Mon, 24 Aug 2026 14:04:52 -0500 Subject: [PATCH 1/3] Handle metric tag config destination conflicts Signed-off-by: riyaz.shiraguppi --- .../model/metric_tag_configurations.py | 51 ++++++- tests/unit/test_metric_tag_configurations.py | 138 ++++++++++++++++++ 2 files changed, 181 insertions(+), 8 deletions(-) create mode 100644 tests/unit/test_metric_tag_configurations.py diff --git a/datadog_sync/model/metric_tag_configurations.py b/datadog_sync/model/metric_tag_configurations.py index 65f0c50d..2869115b 100644 --- a/datadog_sync/model/metric_tag_configurations.py +++ b/datadog_sync/model/metric_tag_configurations.py @@ -7,11 +7,24 @@ from typing import TYPE_CHECKING, Optional, List, Dict, Tuple, cast from datadog_sync.utils.base_resource import BaseResource, ResourceConfig +from datadog_sync.utils.resource_utils import CustomClientHTTPError, SkipResource if TYPE_CHECKING: from datadog_sync.utils.custom_client import CustomClient +def _error_body(error: CustomClientHTTPError) -> str: + return (error.response_body or "").lower() + + +def _is_missing_metric_error(error: CustomClientHTTPError) -> bool: + return error.status_code == 400 and "metric that does not exist" in _error_body(error) + + +def _is_existing_tag_config_conflict(error: CustomClientHTTPError) -> bool: + return error.status_code == 409 and "patch" in _error_body(error) + + class MetricTagConfigurations(BaseResource): resource_type = "metric_tag_configurations" resource_config = ResourceConfig( @@ -47,10 +60,22 @@ async def create_resource(self, _id: str, resource: Dict) -> Tuple[str, Dict]: destination_client = self.config.destination_client payload = {"data": resource} - resp = await destination_client.post( - self.resource_config.base_path + f"/{self.config.state.source[self.resource_type][_id]['id']}/tags", - payload, - ) + path = self.resource_config.base_path + f"/{self.config.state.source[self.resource_type][_id]['id']}/tags" + try: + resp = await destination_client.post(path, payload) + except CustomClientHTTPError as e: + if _is_missing_metric_error(e): + raise SkipResource( + _id, + self.resource_type, + "Metric not present on destination; tag configuration cannot attach.", + ) + if not _is_existing_tag_config_conflict(e): + raise + + existing = await destination_client.get(path) + self.config.state.destination[self.resource_type][_id] = existing["data"] + return await self.update_resource(_id, resource) return _id, resp["data"] @@ -59,10 +84,20 @@ async def update_resource(self, _id: str, resource: Dict) -> Tuple[str, Dict]: if "attributes" in resource: resource["attributes"].pop("metric_type", None) payload = {"data": resource} - resp = await destination_client.patch( - self.resource_config.base_path + f"/{self.config.state.destination[self.resource_type][_id]['id']}/tags", - payload, - ) + try: + resp = await destination_client.patch( + self.resource_config.base_path + + f"/{self.config.state.destination[self.resource_type][_id]['id']}/tags", + payload, + ) + except CustomClientHTTPError as e: + if _is_missing_metric_error(e): + raise SkipResource( + _id, + self.resource_type, + "Metric not present on destination; tag configuration cannot attach.", + ) + raise return _id, resp["data"] diff --git a/tests/unit/test_metric_tag_configurations.py b/tests/unit/test_metric_tag_configurations.py new file mode 100644 index 00000000..93ef628f --- /dev/null +++ b/tests/unit/test_metric_tag_configurations.py @@ -0,0 +1,138 @@ +# 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 asyncio +from types import SimpleNamespace +from unittest.mock import AsyncMock + +import pytest + +from datadog_sync.model.metric_tag_configurations import MetricTagConfigurations +from datadog_sync.utils.resource_utils import CustomClientHTTPError, SkipResource + + +def _run(coro): + loop = asyncio.new_event_loop() + try: + return loop.run_until_complete(coro) + finally: + loop.close() + + +def _http_error(status: int, message: str = "err") -> CustomClientHTTPError: + return CustomClientHTTPError(SimpleNamespace(status=status, message="err"), message=message) + + +def _resource(metric_name: str = "custom.metric") -> dict: + return { + "id": metric_name, + "type": "manage_tags", + "attributes": {"metric_type": "count", "tags": ["env", "service"]}, + } + + +@pytest.fixture +def metric_tag_configurations(mock_config): + mock_config.destination_client = AsyncMock() + return MetricTagConfigurations(mock_config) + + +def test_create_resource_happy_path(metric_tag_configurations): + client = metric_tag_configurations.config.destination_client + client.post = AsyncMock(return_value={"data": _resource()}) + client.get = AsyncMock() + client.patch = AsyncMock() + metric_tag_configurations.config.state.source["metric_tag_configurations"]["custom.metric"] = _resource() + + _id, data = _run(metric_tag_configurations.create_resource("custom.metric", _resource())) + + assert _id == "custom.metric" + assert data == _resource() + client.post.assert_awaited_once_with("/api/v2/metrics/custom.metric/tags", {"data": _resource()}) + client.get.assert_not_awaited() + client.patch.assert_not_awaited() + + +def test_create_resource_missing_destination_metric_raises_skip(metric_tag_configurations): + client = metric_tag_configurations.config.destination_client + client.post = AsyncMock(side_effect=_http_error(400, "Cannot configure tags on a metric that does not exist")) + client.get = AsyncMock() + client.patch = AsyncMock() + metric_tag_configurations.config.state.source["metric_tag_configurations"]["missing.metric"] = _resource( + "missing.metric" + ) + + with pytest.raises(SkipResource) as exc_info: + _run(metric_tag_configurations.create_resource("missing.metric", _resource("missing.metric"))) + + assert "missing.metric" in str(exc_info.value) + assert "not present on destination" in str(exc_info.value) + client.post.assert_awaited_once() + client.get.assert_not_awaited() + client.patch.assert_not_awaited() + + +def test_create_resource_existing_config_conflict_gets_existing_then_patches(metric_tag_configurations): + client = metric_tag_configurations.config.destination_client + client.post = AsyncMock(side_effect=_http_error(409, "Conflicts with existing configuration; use PATCH to update")) + client.get = AsyncMock(return_value={"data": {"id": "custom.metric", "attributes": {"tags": ["env"]}}}) + client.patch = AsyncMock(return_value={"data": {"id": "custom.metric", "attributes": {"tags": ["env", "service"]}}}) + metric_tag_configurations.config.state.source["metric_tag_configurations"]["custom.metric"] = _resource() + + _id, data = _run(metric_tag_configurations.create_resource("custom.metric", _resource())) + + client.post.assert_awaited_once() + client.get.assert_awaited_once_with("/api/v2/metrics/custom.metric/tags") + assert metric_tag_configurations.config.state.destination["metric_tag_configurations"]["custom.metric"] == { + "id": "custom.metric", + "attributes": {"tags": ["env"]}, + } + client.patch.assert_awaited_once() + patch_url = client.patch.await_args.args[0] + assert patch_url == "/api/v2/metrics/custom.metric/tags" + assert client.patch.await_args.args[1]["data"]["attributes"] == {"tags": ["env", "service"]} + assert _id == "custom.metric" + assert data == {"id": "custom.metric", "attributes": {"tags": ["env", "service"]}} + + +def test_create_resource_non_matching_409_propagates(metric_tag_configurations): + client = metric_tag_configurations.config.destination_client + client.post = AsyncMock(side_effect=_http_error(409, "conflict")) + client.get = AsyncMock() + client.patch = AsyncMock() + metric_tag_configurations.config.state.source["metric_tag_configurations"]["custom.metric"] = _resource() + + with pytest.raises(CustomClientHTTPError) as exc_info: + _run(metric_tag_configurations.create_resource("custom.metric", _resource())) + + assert exc_info.value.status_code == 409 + client.get.assert_not_awaited() + client.patch.assert_not_awaited() + + +def test_update_resource_missing_destination_metric_raises_skip(metric_tag_configurations): + client = metric_tag_configurations.config.destination_client + client.patch = AsyncMock(side_effect=_http_error(400, "Cannot configure tags on a metric that does not exist")) + metric_tag_configurations.config.state.destination["metric_tag_configurations"]["missing.metric"] = _resource( + "missing.metric" + ) + + with pytest.raises(SkipResource) as exc_info: + _run(metric_tag_configurations.update_resource("missing.metric", _resource("missing.metric"))) + + assert "missing.metric" in str(exc_info.value) + assert "not present on destination" in str(exc_info.value) + client.patch.assert_awaited_once() + + +def test_update_resource_non_missing_metric_error_propagates(metric_tag_configurations): + client = metric_tag_configurations.config.destination_client + client.patch = AsyncMock(side_effect=_http_error(500, "Internal Server Error")) + metric_tag_configurations.config.state.destination["metric_tag_configurations"]["custom.metric"] = _resource() + + with pytest.raises(CustomClientHTTPError) as exc_info: + _run(metric_tag_configurations.update_resource("custom.metric", _resource())) + + assert exc_info.value.status_code == 500 From 04c2d27f395a24d8ac1b6852f20d7bc8c4118800 Mon Sep 17 00:00:00 2001 From: riyazsh <126122908+riyazsh@users.noreply.github.com> Date: Mon, 24 Aug 2026 14:15:57 -0500 Subject: [PATCH 2/3] Skip missing metric percentiles Signed-off-by: riyaz.shiraguppi --- datadog_sync/model/metric_percentiles.py | 32 +++++- tests/unit/test_metric_percentiles.py | 131 +++++++++++++++++++++++ 2 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 tests/unit/test_metric_percentiles.py diff --git a/datadog_sync/model/metric_percentiles.py b/datadog_sync/model/metric_percentiles.py index b3383a34..d042ed7e 100644 --- a/datadog_sync/model/metric_percentiles.py +++ b/datadog_sync/model/metric_percentiles.py @@ -6,6 +6,15 @@ from datadog_sync.utils.base_resource import BaseResource, ResourceConfig from datadog_sync.utils.custom_client import CustomClient +from datadog_sync.utils.resource_utils import CustomClientHTTPError, SkipResource + + +def _error_body(error: CustomClientHTTPError) -> str: + return (error.response_body or "").lower() + + +def _is_metric_not_found_error(error: CustomClientHTTPError) -> bool: + return error.status_code in (400, 404, 500) and "metric not found" in _error_body(error) class MetricPercentiles(BaseResource): @@ -17,6 +26,7 @@ class MetricPercentiles(BaseResource): ) # Additional MetricPercentiles specific attributes metrics_summaries_get_path = "/metric/distribution/list_summaries" + metrics_metadata_get_path = "/api/v1/metrics" enable_percentiles_path = "/metric/distribution/summary_aggr/percentiles/enable" disable_percentiles_path = "/metric/distribution/summary_aggr/percentiles/disable" @@ -55,8 +65,28 @@ async def update_resource(self, _id: str, resource: Dict) -> Tuple[str, Dict]: # /metric/distribution/summary_aggr, which is not a registered route and # returns 403 empty-body at the OBO auth layer. destination_client = self.config.destination_client + try: + await destination_client.get(self.metrics_metadata_get_path + f"/{_id}") + except CustomClientHTTPError as e: + if e.status_code == 404: + raise SkipResource( + _id, + self.resource_type, + "Metric not present on destination; percentiles cannot attach.", + ) + raise + path = self.enable_percentiles_path if resource.get("include_percentiles") else self.disable_percentiles_path - await destination_client.patch(path, {"metric_names": [_id]}) + try: + await destination_client.patch(path, {"metric_names": [_id]}) + except CustomClientHTTPError as e: + if _is_metric_not_found_error(e): + raise SkipResource( + _id, + self.resource_type, + "Metric not present on destination; percentiles cannot attach.", + ) + raise return _id, resource diff --git a/tests/unit/test_metric_percentiles.py b/tests/unit/test_metric_percentiles.py new file mode 100644 index 00000000..ba8c5c37 --- /dev/null +++ b/tests/unit/test_metric_percentiles.py @@ -0,0 +1,131 @@ +# 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 asyncio +from types import SimpleNamespace +from unittest.mock import AsyncMock + +import pytest + +from datadog_sync.model.metric_percentiles import MetricPercentiles +from datadog_sync.utils.resource_utils import CustomClientHTTPError, SkipResource + + +def _run(coro): + loop = asyncio.new_event_loop() + try: + return loop.run_until_complete(coro) + finally: + loop.close() + + +def _http_error(status: int, message: str = "err") -> CustomClientHTTPError: + return CustomClientHTTPError(SimpleNamespace(status=status, message="err"), message=message) + + +@pytest.fixture +def metric_percentiles(mock_config): + mock_config.destination_client = AsyncMock() + return MetricPercentiles(mock_config) + + +def test_update_resource_existing_metric_enables_percentiles(metric_percentiles): + client = metric_percentiles.config.destination_client + client.get = AsyncMock(return_value={"metric": "custom.metric"}) + client.patch = AsyncMock(return_value={}) + + _id, resource = _run( + metric_percentiles.update_resource("custom.metric", {"metric": "custom.metric", "include_percentiles": True}) + ) + + assert _id == "custom.metric" + assert resource == {"metric": "custom.metric", "include_percentiles": True} + client.get.assert_awaited_once_with("/api/v1/metrics/custom.metric") + client.patch.assert_awaited_once_with( + "/metric/distribution/summary_aggr/percentiles/enable", + {"metric_names": ["custom.metric"]}, + ) + + +def test_update_resource_existing_metric_disables_percentiles(metric_percentiles): + client = metric_percentiles.config.destination_client + client.get = AsyncMock(return_value={"metric": "custom.metric"}) + client.patch = AsyncMock(return_value={}) + + _run(metric_percentiles.update_resource("custom.metric", {"metric": "custom.metric", "include_percentiles": False})) + + client.patch.assert_awaited_once_with( + "/metric/distribution/summary_aggr/percentiles/disable", + {"metric_names": ["custom.metric"]}, + ) + + +def test_update_resource_missing_destination_metric_get_raises_skip(metric_percentiles): + client = metric_percentiles.config.destination_client + client.get = AsyncMock(side_effect=_http_error(404, '{"errors":["custom.metric not found"]}')) + client.patch = AsyncMock() + + with pytest.raises(SkipResource) as exc_info: + _run( + metric_percentiles.update_resource( + "custom.metric", + {"metric": "custom.metric", "include_percentiles": True}, + ) + ) + + assert "custom.metric" in str(exc_info.value) + assert "not present on destination" in str(exc_info.value) + client.patch.assert_not_awaited() + + +def test_update_resource_metric_not_found_patch_raises_skip(metric_percentiles): + client = metric_percentiles.config.destination_client + client.get = AsyncMock(return_value={"metric": "custom.metric"}) + client.patch = AsyncMock(side_effect=_http_error(500, '{"detail":"metric not found"}')) + + with pytest.raises(SkipResource) as exc_info: + _run( + metric_percentiles.update_resource( + "custom.metric", + {"metric": "custom.metric", "include_percentiles": True}, + ) + ) + + assert "custom.metric" in str(exc_info.value) + assert "not present on destination" in str(exc_info.value) + client.patch.assert_awaited_once() + + +def test_update_resource_destination_get_500_propagates(metric_percentiles): + client = metric_percentiles.config.destination_client + client.get = AsyncMock(side_effect=_http_error(500, "Internal Server Error")) + client.patch = AsyncMock() + + with pytest.raises(CustomClientHTTPError) as exc_info: + _run( + metric_percentiles.update_resource( + "custom.metric", + {"metric": "custom.metric", "include_percentiles": True}, + ) + ) + + assert exc_info.value.status_code == 500 + client.patch.assert_not_awaited() + + +def test_update_resource_non_metric_not_found_patch_error_propagates(metric_percentiles): + client = metric_percentiles.config.destination_client + client.get = AsyncMock(return_value={"metric": "custom.metric"}) + client.patch = AsyncMock(side_effect=_http_error(500, "Internal Server Error")) + + with pytest.raises(CustomClientHTTPError) as exc_info: + _run( + metric_percentiles.update_resource( + "custom.metric", + {"metric": "custom.metric", "include_percentiles": True}, + ) + ) + + assert exc_info.value.status_code == 500 From d76eaf03b4f921095fcfd354e7f93ab5817e8b3b Mon Sep 17 00:00:00 2001 From: "riyaz.shiraguppi" Date: Tue, 25 Aug 2026 08:59:58 -0500 Subject: [PATCH 3/3] Avoid metric percentile preflight reads Signed-off-by: riyaz.shiraguppi --- datadog_sync/model/metric_percentiles.py | 12 --------- tests/unit/test_metric_percentiles.py | 32 +++++++++++++----------- 2 files changed, 18 insertions(+), 26 deletions(-) diff --git a/datadog_sync/model/metric_percentiles.py b/datadog_sync/model/metric_percentiles.py index d042ed7e..e89787a6 100644 --- a/datadog_sync/model/metric_percentiles.py +++ b/datadog_sync/model/metric_percentiles.py @@ -26,7 +26,6 @@ class MetricPercentiles(BaseResource): ) # Additional MetricPercentiles specific attributes metrics_summaries_get_path = "/metric/distribution/list_summaries" - metrics_metadata_get_path = "/api/v1/metrics" enable_percentiles_path = "/metric/distribution/summary_aggr/percentiles/enable" disable_percentiles_path = "/metric/distribution/summary_aggr/percentiles/disable" @@ -65,17 +64,6 @@ async def update_resource(self, _id: str, resource: Dict) -> Tuple[str, Dict]: # /metric/distribution/summary_aggr, which is not a registered route and # returns 403 empty-body at the OBO auth layer. destination_client = self.config.destination_client - try: - await destination_client.get(self.metrics_metadata_get_path + f"/{_id}") - except CustomClientHTTPError as e: - if e.status_code == 404: - raise SkipResource( - _id, - self.resource_type, - "Metric not present on destination; percentiles cannot attach.", - ) - raise - path = self.enable_percentiles_path if resource.get("include_percentiles") else self.disable_percentiles_path try: await destination_client.patch(path, {"metric_names": [_id]}) diff --git a/tests/unit/test_metric_percentiles.py b/tests/unit/test_metric_percentiles.py index ba8c5c37..4eb1ae0f 100644 --- a/tests/unit/test_metric_percentiles.py +++ b/tests/unit/test_metric_percentiles.py @@ -33,7 +33,7 @@ def metric_percentiles(mock_config): def test_update_resource_existing_metric_enables_percentiles(metric_percentiles): client = metric_percentiles.config.destination_client - client.get = AsyncMock(return_value={"metric": "custom.metric"}) + client.get = AsyncMock() client.patch = AsyncMock(return_value={}) _id, resource = _run( @@ -42,7 +42,7 @@ def test_update_resource_existing_metric_enables_percentiles(metric_percentiles) assert _id == "custom.metric" assert resource == {"metric": "custom.metric", "include_percentiles": True} - client.get.assert_awaited_once_with("/api/v1/metrics/custom.metric") + client.get.assert_not_awaited() client.patch.assert_awaited_once_with( "/metric/distribution/summary_aggr/percentiles/enable", {"metric_names": ["custom.metric"]}, @@ -51,21 +51,22 @@ def test_update_resource_existing_metric_enables_percentiles(metric_percentiles) def test_update_resource_existing_metric_disables_percentiles(metric_percentiles): client = metric_percentiles.config.destination_client - client.get = AsyncMock(return_value={"metric": "custom.metric"}) + client.get = AsyncMock() client.patch = AsyncMock(return_value={}) _run(metric_percentiles.update_resource("custom.metric", {"metric": "custom.metric", "include_percentiles": False})) + client.get.assert_not_awaited() client.patch.assert_awaited_once_with( "/metric/distribution/summary_aggr/percentiles/disable", {"metric_names": ["custom.metric"]}, ) -def test_update_resource_missing_destination_metric_get_raises_skip(metric_percentiles): +def test_update_resource_missing_destination_metric_patch_raises_skip(metric_percentiles): client = metric_percentiles.config.destination_client - client.get = AsyncMock(side_effect=_http_error(404, '{"errors":["custom.metric not found"]}')) - client.patch = AsyncMock() + client.get = AsyncMock() + client.patch = AsyncMock(side_effect=_http_error(404, '{"errors":["custom.metric not found"]}')) with pytest.raises(SkipResource) as exc_info: _run( @@ -77,12 +78,13 @@ def test_update_resource_missing_destination_metric_get_raises_skip(metric_perce assert "custom.metric" in str(exc_info.value) assert "not present on destination" in str(exc_info.value) - client.patch.assert_not_awaited() + client.get.assert_not_awaited() + client.patch.assert_awaited_once() def test_update_resource_metric_not_found_patch_raises_skip(metric_percentiles): client = metric_percentiles.config.destination_client - client.get = AsyncMock(return_value={"metric": "custom.metric"}) + client.get = AsyncMock() client.patch = AsyncMock(side_effect=_http_error(500, '{"detail":"metric not found"}')) with pytest.raises(SkipResource) as exc_info: @@ -95,13 +97,14 @@ def test_update_resource_metric_not_found_patch_raises_skip(metric_percentiles): assert "custom.metric" in str(exc_info.value) assert "not present on destination" in str(exc_info.value) + client.get.assert_not_awaited() client.patch.assert_awaited_once() -def test_update_resource_destination_get_500_propagates(metric_percentiles): +def test_update_resource_non_metric_not_found_400_patch_error_propagates(metric_percentiles): client = metric_percentiles.config.destination_client - client.get = AsyncMock(side_effect=_http_error(500, "Internal Server Error")) - client.patch = AsyncMock() + client.get = AsyncMock() + client.patch = AsyncMock(side_effect=_http_error(400, "Bad Request")) with pytest.raises(CustomClientHTTPError) as exc_info: _run( @@ -111,13 +114,13 @@ def test_update_resource_destination_get_500_propagates(metric_percentiles): ) ) - assert exc_info.value.status_code == 500 - client.patch.assert_not_awaited() + assert exc_info.value.status_code == 400 + client.get.assert_not_awaited() def test_update_resource_non_metric_not_found_patch_error_propagates(metric_percentiles): client = metric_percentiles.config.destination_client - client.get = AsyncMock(return_value={"metric": "custom.metric"}) + client.get = AsyncMock() client.patch = AsyncMock(side_effect=_http_error(500, "Internal Server Error")) with pytest.raises(CustomClientHTTPError) as exc_info: @@ -129,3 +132,4 @@ def test_update_resource_non_metric_not_found_patch_error_propagates(metric_perc ) assert exc_info.value.status_code == 500 + client.get.assert_not_awaited()