Skip to content

Handle metric tag configuration destination conflicts - #669

Merged
riyazsh merged 3 commits into
mainfrom
riyaz/HAMR-395-metric-tag-config-patch
Aug 25, 2026
Merged

Handle metric tag configuration destination conflicts#669
riyazsh merged 3 commits into
mainfrom
riyaz/HAMR-395-metric-tag-config-patch

Conversation

@riyazsh

@riyazsh riyazsh commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • handle metric tag configuration POST conflicts by fetching the existing destination tag config and PATCHing it
  • skip metric tag configs when the destination metric does not exist, matching the terminal gap behavior used by metrics metadata
  • keep metric_percentiles missing-destination-metric handling on the PATCH response path without adding a separate metric metadata preflight read
  • add focused unit coverage for happy path, 409 fallback, missing destination metric skips, and propagated errors

Context

Paxos DDR sync is seeing metric_tag_configurations failures where the destination API returns 409 asking callers to use PATCH.

The surprising part is that this can happen even though the US5 DDR org has no customer traffic and managed-sync is expected to be the writer. The reason is that sync-cli's create-vs-update decision is based on its persisted destination state, not a live existence check against the Metrics API. If destination state has no metric tag configuration entry, sync-cli chooses the create path and sends POST.

That missing destination-state entry does not guarantee the destination config is absent. The destination config may already exist because an earlier partial run created it but did not persist the destination mapping, because GCS destination state is stale/incomplete/pruned, or because metric tag configuration identity is attached to the metric name rather than to a sync-owned UUID. In that state, the destination API is correct to reject POST with 409 and ask the caller to PATCH the existing config.

Refreshing destination state in managed-sync does not fix this resource path by itself: the resource handler still needs to treat the 409 as evidence that the config exists, fetch the existing destination config, seed local destination state, and retry through the normal PATCH update path.

Testing

  • tox -e py312 -- tests/unit/test_metric_percentiles.py tests/unit/test_metric_tag_configurations.py
  • env DD_HTTP_CLIENT_RETRY_TIMEOUT=300 RECORD=false DD_DESTINATION_API_URL=https://api.us5.datadoghq.com DD_DESTINATION_API_KEY=fake DD_DESTINATION_APP_KEY=fake DD_SOURCE_API_URL=https://api.datadoghq.eu DD_SOURCE_API_KEY=fake DD_SOURCE_APP_KEY=fake MAX_WORKERS=10 .tox/py312/bin/pytest -v -m integration tests/integration/resources/test_metric_percentiles.py::TestMetricPercentilesResources
  • .tox/py312/bin/black --line-length 120 --check datadog_sync/model/metric_percentiles.py tests/unit/test_metric_percentiles.py datadog_sync/model/metric_tag_configurations.py tests/unit/test_metric_tag_configurations.py
  • GitHub Actions: all checks passing on b96a791b

Signed-off-by: riyaz.shiraguppi <riyaz.shiraguppi@datadoghq.com>
Signed-off-by: riyaz.shiraguppi <riyaz.shiraguppi@datadoghq.com>

riyazsh commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Update: I added the metric_percentiles fix to this same draft PR.

Additional scope:

  • precheck destination metric existence with GET /api/v1/metrics/{metric} before toggling percentiles
  • skip missing destination metrics instead of burning retries
  • also skip the known legacy percentile endpoint response 500 {"detail":"metric not found"}
  • keep unrelated 5xx responses propagating so true transient/API instability still fails and retries normally

Notebook investigation:

  • current notebooks.py already skips source-side 403 and 404 during per-id import
  • live Paxos US1 app-key checks for notebooks 10851048 and 10705543 returned 404 Notebook not found, which current main already handles as SkipResource
  • no notebook code change included

Additional validation:

  • pytest -q tests/unit/test_metric_percentiles.py tests/unit/test_metric_tag_configurations.py tests/unit/test_notebooks.py -> 34 passed
  • python3 -m compileall datadog_sync/model/metric_percentiles.py tests/unit/test_metric_percentiles.py datadog_sync/model/metric_tag_configurations.py tests/unit/test_metric_tag_configurations.py
  • black --line-length 120 --check datadog_sync/model/metric_percentiles.py tests/unit/test_metric_percentiles.py datadog_sync/model/metric_tag_configurations.py tests/unit/test_metric_tag_configurations.py
  • ruff --line-length 120 datadog_sync/model/metric_percentiles.py tests/unit/test_metric_percentiles.py datadog_sync/model/metric_tag_configurations.py tests/unit/test_metric_tag_configurations.py
  • git diff --check

Signed-off-by: riyaz.shiraguppi <riyaz.shiraguppi@datadoghq.com>
@riyazsh
riyazsh marked this pull request as ready for review August 25, 2026 14:33
@riyazsh
riyazsh requested a review from a team as a code owner August 25, 2026 14:33
@riyazsh
riyazsh force-pushed the riyaz/HAMR-395-metric-tag-config-patch branch from b96a791 to d76eaf0 Compare August 25, 2026 15:05
@riyazsh
riyazsh merged commit 9c36709 into main Aug 25, 2026
11 checks passed
@riyazsh
riyazsh deleted the riyaz/HAMR-395-metric-tag-config-patch branch August 25, 2026 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants