Skip to content

test(amp): defensive retry-on-empty for AMP query path - #736

Closed
olowosulu wants to merge 1 commit into
aws:mainfrom
olowosulu:fix/amp-retry-on-empty
Closed

test(amp): defensive retry-on-empty for AMP query path#736
olowosulu wants to merge 1 commit into
aws:mainfrom
olowosulu:fix/amp-retry-on-empty

Conversation

@olowosulu

@olowosulu olowosulu commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Scope (updated after root-cause investigation)

This is a defensive hardening of the AMP query path. It does not fix the current al2:selinux_amp_test failures. See "Why this does not fix the current flake" below.

What this changes

validateMetric issued a single PromQL instant query (avg_over_time(m[3m])) via queryAMPMetrics with no retry, and failed the metric if the result set was empty. This adds queryAMPMetricsWithRetry (3 attempts, 30s apart), retrying on both request errors and empty result sets, and switches validateMetric to use it.

  • Each attempt is a fresh instant query evaluated at the AMP server's current time, so the 3-minute avg_over_time lookback advances on its own. There is no explicit start/end window to recompute.
  • On exhaustion the last response or error is returned unchanged, so a genuine empty result still fails the test.

This guards against remote-write propagation delay: once a workspace exists and the agent is publishing, a metric can briefly be un-queryable and land moments later.

Why this does not fix the current flake

Investigation of PR aws/amazon-cloudwatch-agent#2225's failing al2:selinux_amp_test jobs (run 30667884230 attempts 1 and 9, run 30764785279 attempt 2, run 30995079411 attempt 1 on the current head) shows every failure is a Terraform provisioning error, upstream of the query path:

Error: creating Prometheus Workspace: operation error amp: CreateWorkspace,
https response error StatusCode: 402, ..., ServiceQuotaExceededException:
Limit exceeded. Maximum workspaces per region: 75
  on .terraform/modules/amp/main.tf line 30, in resource "aws_prometheus_workspace" "this"

terraform apply fails at aws_prometheus_workspace creation, so the Go test binary never runs and queryAMPMetrics is never reached. No query-path log lines and no 429/403/throttle appear in any failed log. That is an infrastructure service-quota issue (the region is at the 75 AMP-workspace cap), addressed by raising the quota and/or ensuring workspaces are reliably torn down, not by this test-code change.

Status

Kept open as an independent, low-risk query-path improvement. It is not a fix for the quota-driven flake and should not be treated as one.

Verification

  • gofmt clean
  • go test -c -tags=integration ./test/amp/ compiles on go 1.23.12 (matches CI)
  • Not exercised in CI

al2:selinux_amp_test (EC2SELinuxTests) is a top flaky job in recent CI
run-history (roughly 9 of 16 observations, ~56 percent). It provisions a
real AMP workspace via Terraform then validates each metric with a single
avg_over_time(m[3m]) instant query and no retry, so a datapoint not yet
queryable due to remote-write propagation fails the whole suite
(TestGroupResult is all-or-nothing).

Add queryAMPMetricsWithRetry wrapping queryAMPMetrics with 3 attempts 30s
apart, retrying on request errors and empty result sets. There is no
explicit query window to recompute: each attempt is a fresh instant query
evaluated at AMP server-now, so the 3m lookback advances on its own. On
exhaustion the last response or error is returned unchanged so
validateMetric's existing empty-result check still fails the test
genuinely.

Verified: gofmt clean; go test -c -tags=integration ./test/amp/ compiles
on go 1.23.12 (matches CI ~1.23.0). Not exercised in CI.
@olowosulu
olowosulu requested a review from a team as a code owner August 5, 2026 12:48
@olowosulu olowosulu changed the title test(amp): retry AMP query on empty result test(amp): defensive retry-on-empty for AMP query path Aug 5, 2026
@olowosulu olowosulu closed this Aug 5, 2026
@olowosulu

Copy link
Copy Markdown
Contributor Author

Closing this in favor of aws/amazon-cloudwatch-agent#2233.

This PR hardened the AMP query path (retry-on-empty) on the theory that
al2:selinux_amp_test was flaky due to remote-write propagation delay.
Pulling the failing job logs showed that is not the cause. The job fails
earlier, at terraform apply while creating the workspace:

Error: creating Prometheus Workspace: ... StatusCode: 402,
ServiceQuotaExceededException: Limit exceeded. Maximum workspaces per region: 75

The Go test binary never runs, so the query path this PR touches is never
reached. The region had accumulated orphaned cwagent-integ-test-* workspaces
(left behind by cancelled or hard-killed jobs whose Terraform state is gone) up
to the 75-workspace service quota, so every new run failed at CreateWorkspace.

The real fix is aws/amazon-cloudwatch-agent#2233, which adds a clean_amp
cleaner to the daily resource-cleaner cron (clean-aws-resources.yml), matching
the existing clean_* cleaners for other resource types, so leaked workspaces
are reclaimed automatically.

Closing this since it targets no observed failure. Happy to reopen if the query
path ever needs proactive retry hardening on its own merits.

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