Skip to content

Reinstate integration tests skipped in #611 to surface current failure state#613

Draft
nathantournant wants to merge 1 commit into
mainfrom
nathan.tournant/reinstate-skipped-integration-tests
Draft

Reinstate integration tests skipped in #611 to surface current failure state#613
nathantournant wants to merge 1 commit into
mainfrom
nathan.tournant/reinstate-skipped-integration-tests

Conversation

@nathantournant

Copy link
Copy Markdown
Member

Summary

PR #611 added @pytest.mark.skip override stubs for 12 integration tests, citing two broken test-org fixtures:

  • The S3 destination bucket for the logs-archives fixture was deleted.
  • The metric-tag-configurations fixture set on the test org is empty.

This PR removes those skip-override stubs (deletes the override methods so the tests fall back to the inherited BaseResourcesTestClass implementations of the same name), reinstating:

  • tests/integration/resources/test_logs_archives.py: test_resource_sync, test_resource_sync_per_file, test_resource_update_sync, test_resource_update_sync_per_file, test_resource_cleanup, test_no_resource_diffs
  • tests/integration/resources/test_logs_archives_order.py: test_resource_sync, test_resource_sync_per_file
  • tests/integration/resources/test_metric_tag_configurations.py: test_resource_import, test_resource_import_per_file, test_resource_cleanup, test_no_resource_diffs

This PR is intentionally expected to fail CI. It is not meant to be merged as-is. Its purpose is diagnostic: to get a fresh, concrete, dated read on the actual current state of:

  1. the logs-archives S3 destination bucket fixture, and
  2. the metric-tag-configurations fixture on the test org,

and to convert the vague "fixture drift" explanation from #611 into an actionable, checkable state (either the tests still fail with the documented error, confirming the fixtures need re-provisioning, or they unexpectedly pass, meaning #611's skips can be reverted for real).

Related to #611.

Test plan

…e state

PR #611 added @pytest.mark.skip override stubs for 12 integration tests
across test_logs_archives.py, test_logs_archives_order.py, and
test_metric_tag_configurations.py, citing a deleted S3 destination
bucket for the logs-archives fixture and an empty
metric-tag-configurations fixture on the test org. This removes those
skip-override stubs so the tests fall back to the inherited
BaseResourcesTestClass implementations and actually run again in CI,
producing a fresh, concrete, dated signal on whether those two
fixtures are still broken instead of letting the "temporary" skip go
stale unnoticed.

This change is intentionally diagnostic: CI is expected to fail here
until the underlying fixtures are (re)provisioned.

Ref #611.

Environment: Datadog workspace

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@nathantournant

Copy link
Copy Markdown
Member Author

Findings on the two broken fixtures

I dug through the test source and the recorded VCR cassettes (tests/integration/resources/cassettes/) to pin down concrete identifiers. CI hasn't produced a fresh failure log yet for this PR — the Run Integrations Tests workflow is currently queued behind another in-flight run (this repo's integration-test workflow uses a single global concurrency group, cancel-in-progress: false, so runs queue rather than run in parallel) — so the below is derived from cassette/config data, not a live failure trace. I'll follow up once a run actually executes.

1. logs-archives S3 destination bucket

From the recorded cassettes for test_logs_archives / test_logs_archives_order (e.g. TestLogsArchivesResources.test_resource_sync.yaml), the archive's destination config is:

"destination": {
  "bucket": "hamr-logs-archive-for-internal-testing",
  "path": "/logs-archive-testing",
  "type": "s3",
  "encryption": {"type": "NO_OVERRIDE"},
  "storage_class": "STANDARD",
  "integration": {
    "role_name": "datadog-aws-integration-role",
    "account_id": "267727855951"
  }
}

The sync flow reads the archive from the source org (api.datadoghq.eu, i.e. DD_SOURCE_API_URL) and re-creates it against the destination org (api.us5.datadoghq.com, i.e. DD_DESTINATION_API_URL). The recorded source archive's state was already "FAILING" at capture time, consistent with the bucket having since been deleted.

What needs to be (re)provisioned, as concretely as this repo shows it:

  • An S3 bucket literally named hamr-logs-archive-for-internal-testing, in the AWS account 267727855951, with the archive path /logs-archive-testing.
  • The AWS integration role datadog-aws-integration-role needs to exist in that account and be attached as the Datadog AWS integration (the archive references it by role_name + account_id, not raw credentials), with whatever bucket policy Datadog's log-archive S3 destination requires (s3:PutObject/s3:GetBucketLocation/etc. for the Datadog log-archiving principal).
  • Once the bucket + role exist again, the destination org's logs_archives config should be re-pointed at it (or simply let datadog-sync sync recreate it, since the destination side is created fresh by the sync itself).

I could not find any indication in the repo of who owns/owned that AWS account or bucket, or a Terraform/CloudFormation definition for it — this looks like infra that lived outside this repo. If a live run does fail, the next diagnostic step is to pull the actual POST .../logs/config/archives response body from the CI log (gh run view <run-id> --log-failed) to get the exact API error (e.g. a 400 with a message naming the bucket/role) — I did not yet have a fresh failing run to pull that from.

2. metric-tag-configurations fixture

BaseResourcesTestClass.test_resource_import (in tests/integration/helpers.py) asserts len(source_resources) > 0 after importing — i.e. it requires the source org (DD_SOURCE_API_URL, the EU org) to have at least one metric with a tag configuration (manage_tags) already set. No filter is set on TestMetricConfigurationResources, so any configured metric satisfies the test — this isn't about one specific metric name being required by the test logic itself.

That said, the recorded cassette (TestMetricConfigurationResources.test_resource_import.yaml) shows what was previously configured on the source org, for concreteness:

  • msr.distribution — tags ["environment", "host"], type distribution, include_percentiles: true
  • msr.histogram.95percentile, msr.histogram.avg, msr.histogram.count, msr.histogram.max, msr.histogram.median — each tagged ["action_type"], aggregation {"time": "sum", "space": "sum"}

What needs to be provisioned: on the source (EU) test org, create/restore a metric tag configuration for at least one metric (ideally re-create the ones above, e.g. msr.distribution with tags environment/host, or any of the msr.histogram.* metrics with tag action_type) via POST /api/v2/metrics/{metric_name}/tags (or the Datadog UI's "Manage Tags" on a metric). Any single configured metric will unblock test_resource_import/test_resource_import_per_file; test_resource_cleanup and test_no_resource_diffs depend transitively on a prior successful sync, so they should follow once the import/sync tests pass.

I did not find a script or fixture-seeding tool in this repo that provisions test-org metrics/tags automatically — this looks like it needs to be done manually against the test org (or the metric needs to naturally exist with tag configs from real traffic).


This PR is expected to fail CI as-is; the intent is to get a dated, concrete signal (rather than the vague "fixture drift" note in #611) on whether these two fixtures are still broken, so they can be actually fixed instead of the skip going stale.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant