fix(laboratory): emit missing canary metrics for Verify + Receive Session Proposal alerts#5709
Open
svenvoskamp wants to merge 3 commits into
Open
fix(laboratory): emit missing canary metrics for Verify + Receive Session Proposal alerts#5709svenvoskamp wants to merge 3 commits into
svenvoskamp wants to merge 3 commits into
Conversation
The verify canary tests in verify.spec.ts are tagged @canary with metric annotations (HappyPath.verify, UnhappyPath.verify-scam) but never registered an afterEach hook to upload their results, so HappyPath.verify.* and UnhappyPath.verify-scam.* were never published to CloudWatch. Their Grafana alerts therefore fired continuously on No-Data. Add afterEach hooks calling afterEachCanary for the two @canary-tagged verify fixtures, mirroring canary.spec.ts. Non-canary tests sharing these fixtures are skipped by afterEachCanary's tag guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
9 Skipped Deployments
|
Contributor
Visual Regression Test Results ✅ PassedChromatic Build: https://www.chromatic.com/build?appId=6493191bf4b10fed8ca7353f&number=956 👉 Please review the visual changes in Chromatic and accept or reject them. |
Contributor
📦 Bundle Size Check✅ All bundles are within size limits 📊 View detailed bundle sizes> @reown/appkit-monorepo@1.7.1 size /home/runner/work/appkit/appkit > size-limit |
Contributor
Coverage Report
File CoverageNo changed files found. |
…iably emit The "AppKit Receive Session Proposal Timing > 7000ms" Grafana alerts (all 4 regions) were firing on No-Data, not on real latency. Their query targets HappyPath.sign.timing.pairingReceiveSessionProposal, which the canary only recorded when pairingCreatedTime was set — and that was captured by scraping a WalletConnect "set core/pairing/pairing" console log that no longer fires. So the metric was never published (0 datapoints in CloudWatch), while the unconditional receiveSessionProposal metric stayed healthy (~1.8s). Capture pairingCreatedTime deterministically when the wallet begins processing the connection URI instead of depending on SDK console output, and push the metric unconditionally. The metric name is unchanged, so the existing Grafana alert starts receiving data with no dashboard change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
svenvoskamp
changed the title
Contributor
Author
Why these alerts fired, and what this PR changesBoth alert groups were firing on No Data — not on a real Verify failure or a real latency regression. Verified directly against the Summary
Health check for contrast — metrics that are emitted and healthy:
1. AppKit Verify (Happy + Unhappy Scam)
2. AppKit Receive Session Proposal Timing (> 7000ms)
// before — only recorded if a console log happened to fire
walletPage.page.on('console', msg => {
if (msg.text().includes('set') && msg.text().includes('core/pairing/pairing')) {
pairingCreatedTime = new Date()
}
})
...
if (pairingCreatedTime) { // never true anymore
timingRecords.push({ item: 'pairingReceiveSessionProposal', ... })
}
// after — deterministic, always recorded
const pairingCreatedTime = new Date()
...
timingRecords.push({
item: 'pairingReceiveSessionProposal',
timeMs: proposalReceived.getTime() - pairingCreatedTime.getTime()
})Common threadBoth are No-Data firings from opposite directions: Verify = metric never emitted + new alert; Receive Session Proposal = metric used to emit but broke + old alert. Neither is a real product/latency issue. After this deploys and the canary runs, all 8 alerts get real data and evaluate genuine pass/fail / latency. Caveat: the Verify side is fully confirmed from CloudWatch (0 datapoints vs. healthy comparators). The "broke ~15 days ago" timing for |
Convert the consecutive line comments to a capitalized starred block comment (multiline-comment-style / capitalized-comments). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Two groups of Grafana canary alerts have been firing continuously — both on No Data, not real regressions. Verified against the
monitoringAWS account (namespaceprod_Canary_Web3Modal, eu-central-1).1. AppKit Verify (Happy + Unhappy Scam) — 4 alerts
HappyPath.verify.*/UnhappyPath.verify-scam.*have 0 datapoints over 90 days. The verify tests inverify.spec.tsare@canary-tagged with metric annotations, but the file never registered anafterEachcallingafterEachCanary(), so results were never uploaded (onlycanary.spec.ts/siwe-email.spec.tshad it).2. AppKit Receive Session Proposal Timing (> 7000ms) — 4 alerts
The alert queries
HappyPath.sign.timing.pairingReceiveSessionProposal, which also has 0 datapoints — whilereceiveSessionProposalis healthy (~1.8s, never >7s).pairingReceiveSessionProposalwas only recorded whenpairingCreatedTimewas set, captured by scraping a WalletConnectset … core/pairing/pairingconsole log that no longer fires. So it was never published and the alert fired on No-Data.Fixes
verify.spec.ts— addafterEach→afterEachCanaryfor the two@canaryverify fixtures, mirroringcanary.spec.ts. Non-canary tests sharing the fixtures are skipped by the tag guard.w3m-wallet-fixture.ts— capturepairingCreatedTimedeterministically when the wallet starts processing the URI, and pushpairingReceiveSessionProposalunconditionally. Metric name unchanged, so the existing Grafana alert gets data with no dashboard change.Result
Once deployed and the canary runs, all 8 alerts get real data and evaluate genuine pass/fail / latency instead of No-Data. No Grafana provisioning change required.
🤖 Generated with Claude Code