Skip to content

feat: alarm and test for order tracking silently stopping - #698

Open
codyborn wants to merge 1 commit into
mainfrom
observability-order-tracking
Open

feat: alarm and test for order tracking silently stopping#698
codyborn wants to merge 1 commit into
mainfrom
observability-order-tracking

Conversation

@codyborn

Copy link
Copy Markdown
Collaborator

Order tracking stopped starting on every chain for a day and nothing detected it. Three independent gaps allowed that; this closes all three.

1. No alarm on the only signal that existed

UniswapXOrderService swallows post-persist failures deliberately — an accepted order must never be reported as rejected. So when tracking breaks, requests still return 201 and no 4xx/5xx alarm moves. PostOrderPostPersistFailure was emitted for every order on every chain and appeared exactly once in the repo: the putMetric call. Nothing alarmed on it, nothing dashboarded it.

Adds a SEV3 on it.

2. No alarm for tracking stopping entirely

The step function alarms are rate alarms over 100*((throttled+failed+timedOut+aborted)/started) with treatMissingData: IGNORE. When zero executions start, the expression has no datapoints and IGNORE holds them at their last state — OK. They detect "executions are failing" and are structurally blind to "executions stopped happening", which is what a broken ARN or bad deploy actually looks like.

Adds a SEV2 heartbeat with TreatMissingData.BREACHING, mirroring the existing LoopCompleted alarm in status-stack.

Two design notes:

  • Alarms on a new OrderTrackerStarted counter, not AWS/States ExecutionsStarted. The AWS metric is per-state-machine, and summing one metric per chain hits CloudWatch's 10-metric cap for math-expression alarms — I tried it and CDK rejects it outright at 21 chains. Our own counter is aggregate by construction and needs no maintenance as chains are added.
  • Prod only. Beta has no steady organic order flow, so an hourly heartbeat there would alarm on idleness rather than breakage. Beta is covered by the synth tests below plus the post-deploy e2e suite.

3. No test of the CDK at all

bin/ had zero test coverage, so a map that was declared but never populated type-checked (an index signature returns string, not string | undefined, without noUncheckedIndexedAccess), synthesized, and deployed.

Adds synth tests covering both sides of the seam:

  • StepFunctionStack publishes a name for every supported chain, and the name map stays key-aligned with the ARN map.
  • LambdaStack's STATE_MACHINE_NAMES contains no "undefined", has an entry per chain, and never regresses to the per-chain STATE_MACHINE_ARN_<chainId> shape that exhausted the 4KB limit in the first place.

Verified they fail for the right reason: removing the name-map assignment fails exactly 3 of them and nothing else. A guard nobody has watched go red isn't a guard.

Tests

558 pass. The 14 remaining failures are pre-existing — clean main reports the identical 14 (field-validator / order-validator, LABS_COSIGNER). tsc clean, eslint no new errors.

On CD gating — worth reading before adding more

I went in expecting to have to add integ tests and gate CD on them. Both already exist:

  • bin/app.ts's addIntegTests adds a post-deploy CodeBuildStep running yarn test:e2e to both the beta and prod stages via addPost. A beta failure fails the pipeline, so prod promotion is already blocked.
  • The e2e suite already asserts outcomes that depend on the step function firingorder.test.ts:543 expects an order to reach expired, and :588/:602 expect filled. waitAndGetOrderStatus's own comment says "We have to wait for the sfn to fire."

So this bug should have failed beta e2e and blocked prod. That leaves an open question I could not answer without pipeline access: did the e2e run and fail (in which case CD worked and only beta was affected), or did something let it through? Worth confirming before adding redundant gating.

What the synth tests above genuinely add is speed and locality: they catch this class in CI in seconds, pre-deploy, instead of after a beta deploy plus a four-minute e2e wait.

🤖 Generated with Claude Code

Order tracking stopped starting for every chain for a day and nothing
detected it. Three independent gaps let that happen; this closes all three.

1. No alarm on the only signal that existed. UniswapXOrderService swallows
   post-persist failures on purpose so an accepted order is never reported
   as rejected, so a broken tracker leaves requests returning 201 and no
   error-rate alarm moving. PostOrderPostPersistFailure was emitted but
   appeared exactly once in the repo — nothing alarmed on it. Adds a SEV3.

2. No alarm for tracking stopping entirely. The step function alarms are
   rate alarms over failed/started with TreatMissingData.IGNORE, so with
   zero executions the expression has no datapoints and they hold their
   last state — OK. They detect "executions are failing" and are blind to
   "executions stopped happening". Adds a SEV2 heartbeat on a new
   OrderTrackerStarted counter with TreatMissingData.BREACHING.

   Alarms on our own counter rather than AWS/States ExecutionsStarted
   because that metric is per-state-machine and summing one per chain
   exceeds CloudWatch's 10-metric cap for math-expression alarms (21 chains
   today). Prod-only: beta has no steady organic order flow, so an hourly
   heartbeat there would alarm on idleness rather than breakage.

3. No test of the CDK. bin/ had no test coverage at all, so a map that was
   declared but never populated type-checked, synthesized, and deployed.
   Adds synth tests covering both sides of the seam: StepFunctionStack
   publishes a name for every supported chain and stays key-aligned with
   the ARN map, and LambdaStack's STATE_MACHINE_NAMES contains no
   "undefined", has an entry per chain, and never regresses to the
   per-chain env var shape that exhausted the 4KB limit.

Verified the tests fail for the right reason: removing the name-map
assignment fails exactly 3 of them and nothing else.

Tests: 558 pass, 14 pre-existing failures unchanged (field-validator /
order-validator LABS_COSIGNER). tsc clean, eslint no new errors.
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