mirror of
https://github.com/stablyai/orca.git
synced 2026-09-23 08:02:31 +00:00
* feat(relay): alert on far-cell placement and skewed region hints US desktops were homed on asia-east2 cells for weeks in 2026-08 with every existing relay alert green. Roughly 226 of 332 hosts on those cells were non-APAC, and a phone connect took ~10 s there against ~0.6 s in region, but nothing in Cloud Monitoring could see distance: the connection, queue, heap, and SQL bars all measure a cell's own health, which was fine. Three policies close that gap. Two read distance per cell, from the accept and control-RTT timing added in the parent commit: phone-accept p95 above 2 s, and control ping p50 above 150 ms. The third reads the cause fleet-wide, as the asia-east2 share of the region hints desktops send the director, so a mis-picking client probe is visible before it lands anyone on a far cell. All three are MQL rather than the metric filters the other relay policies use. Every runtime metric is a DELTA DISTRIBUTION, and a filter condition can only align one with a percentile; each alert needs the sum of the extracted values as a volume floor so a sparse window cannot page. None of these metrics exists in the project yet, so what was checked against production is the query shape: the same MQL run over existing metrics of the same kind. The skew denominator needs one log-based metric per hint key, so `requestedRegionsDelta` now has one per relay region plus the unhinted bucket. Those ride the existing snapshot metric family, which adds map entries without touching the live metrics. A ratchet test pins the key list to relay-contract's RELAY_REGIONS: a region added there without a metric would shrink the denominator, so the test fails rather than letting the share quietly inflate. * fix(relay): compare hinted regions against placed ones, not a fixed share Review found the skew alert inverted at both ends. A fixed 40% bar on the asia-east2 share of region hints was silent through the exact broken state it was written for, and would page forever once the desktop probe is fixed and the genuine APAC share rises past it. An absolute share cannot separate those because it has no reference point. The hint share now has one: the share of assignments the director actually placed in that region during the same hour. Measured over twelve hours on 2026-09-07, while the probe was still mis-picking, asia-east2 was 33.8% of 33,800 hinted requests and 7.9% of 45,364 assignments. That is a 4.27x divergence and a 25.9-point gap, so the alert fires above 2x and 15 points, inside the broken state and outside a healthy one. Both bars must hold: the ratio alone blows up on tiny placement counts, the gap alone misses a proportionally large skew at low volume. The reviewer proposed either bar alone; requiring both keeps each one meaningful and still clears today's numbers with room. `unhinted` requests leave the denominator. They were 27% of all requests, so a client that always sends a hint would move the number from 21.9% to 35.0% with no behaviour change at all. The comparison needs per-region placement counters, so `selectedRegionsDelta` gets log-based metrics alongside the requested ones. Rather than extract four hyphenated map keys through quoted field paths, which nothing in the project does and which cannot be checked without applying, the relay now also publishes flat `requestedRegion<Region>Delta` and `selectedRegion<Region>Delta` fields next to the untouched maps. They are emitted as zeros in every interval, so no series can drop out of the alert's inner join in an hour with no asia placements, which is exactly the hour the skew is worst. Additive only: metricVersion is unchanged, the maps still carry anything outside the catalog, and the emitter's leak guard still passes. Two corrections to what the previous commit claimed. None of these metrics exist in the project yet, so the code, the doc and this message now say what was actually checked against production: the query shapes, run over existing metrics of the same kind. And the control-RTT policy records that EU desktops on us-central1 sit at 100-130 ms, so a European-heavy cell can approach the 150 ms bar while correctly homed. The skew alert will stay lit after a client fix until the backlog is rehomed. Sticky assignment never re-consults the hint, so a desktop already on an asia cell keeps landing there whatever it now asks for. The policy description and the doc both say so, so nobody reads a slow clear as a failed fix. * fix(relay): cross-multiply the skew bars so a zero placement share still fires `hint_share / placement_share` is undefined in the hour that matters most. When the director placed nobody in the region, MQL returns no rows for either 0/0 or x/0, so the series disappears before the gap and volume clauses run and the alert stays silent. That hour is not hypothetical: it is every desktop asking for a region while the director puts nobody there, which is what a drained, fenced, or full region looks like, and it is the most extreme skew the alert can see. The condition is now cross-multiplied, `hint_share > 2 * placement_share`, which is well defined at zero. Both forms were run read-only against production surrogates chosen so the placement denominator is exactly zero: the ratio form returned no rows, the cross-multiplied form returned the series with the condition true on every point. A second surrogate pass with a tiny hint share returned the series with the condition false, so the gap clause still suppresses the healthy shape rather than the query silently matching everything. The flat field names are no longer derived on either side. Terraform title cased each dash-separated part and the emitter upper cased each part's first character, so the ratchet had to pin two source expressions by regex, which a reformat would break and which never compared the actual rendered names. Both sides now declare a literal map, relay-contract's RELAY_REGION_METRIC_SEGMENTS and Terraform's relay_region_field_segments, and the test compares the two declarations against each other and against the expected names. `satisfies Record<RelayRegion, string>` makes a region added without a segment a compile error rather than a silent gap in the alert's denominators. Both ratchets were checked by mutation: a wrong Terraform segment, a contract region with no Terraform entry, and a revert to the ratio form each fail the node test, and the new region fails the contract build.
85 lines
3.9 KiB
JavaScript
85 lines
3.9 KiB
JavaScript
import assert from 'node:assert/strict'
|
|
import { readFileSync } from 'node:fs'
|
|
import test from 'node:test'
|
|
import { fileURLToPath } from 'node:url'
|
|
|
|
// Why: the region-skew alert compares asia-east2's share of assignment hints against its share of
|
|
// actual placements. Both shares are sums over one log-based metric per region, and the region
|
|
// list is written out by hand in Terraform. A region added to the contract without matching
|
|
// metrics would silently drop out of both denominators and move the ratio the alert fires on.
|
|
|
|
const read = (relative) => readFileSync(fileURLToPath(new URL(relative, import.meta.url)), 'utf8')
|
|
const collapse = (text) => text.replaceAll(/\s+/g, ' ')
|
|
|
|
const contractRegions = (() => {
|
|
const source = read('../../packages/relay-contract/src/relay-regions.ts')
|
|
const literal = /export const RELAY_REGIONS = \[([^\]]*)\]/.exec(source)
|
|
assert.ok(literal, 'RELAY_REGIONS literal not found in relay-regions.ts')
|
|
return [...literal[1].matchAll(/'([^']+)'/g)].map((match) => match[1])
|
|
})()
|
|
|
|
const terraform = read('../../infra/terraform/relay-observability.tf')
|
|
|
|
const terraformRegions = (() => {
|
|
const literal = /relay_region_keys = \[([^\]]*)\]/.exec(terraform)
|
|
assert.ok(literal, 'relay_region_keys not found in relay-observability.tf')
|
|
return [...literal[1].matchAll(/"([^"]+)"/g)].map((match) => match[1])
|
|
})()
|
|
|
|
// Both sides now spell the field-name segments out, so the test compares the two declared maps
|
|
// rather than two source expressions. Reformatting either file cannot break this, and a literal
|
|
// expected value below still catches an identical wrong edit made to both.
|
|
const declaredSegments = (source, open, close) => {
|
|
const body = source.slice(source.indexOf(open) + open.length, source.indexOf(close, source.indexOf(open)))
|
|
return Object.fromEntries(
|
|
[...body.matchAll(/'?"?([a-z0-9-]+)'?"?\s*[:=]\s*'?"?([A-Za-z0-9]+)'?"?/g)].map((match) => [
|
|
match[1],
|
|
match[2]
|
|
])
|
|
)
|
|
}
|
|
|
|
const terraformSegments = declaredSegments(terraform, 'relay_region_field_segments = {', '}')
|
|
const contractSegments = declaredSegments(
|
|
read('../../packages/relay-contract/src/relay-regions.ts'),
|
|
'RELAY_REGION_METRIC_SEGMENTS = {',
|
|
'}'
|
|
)
|
|
|
|
test('terraform covers exactly the regions the contract can hint or select', () => {
|
|
assert.deepEqual([...terraformRegions].sort(), [...contractRegions].sort())
|
|
})
|
|
|
|
test('terraform and the contract declare the same flat field segments', () => {
|
|
assert.deepEqual(terraformSegments, contractSegments)
|
|
// Pinned literally so the same wrong edit applied to both sides still fails.
|
|
assert.deepEqual(terraformSegments, { 'us-central1': 'UsCentral1', 'asia-east2': 'AsiaEast2' })
|
|
assert.deepEqual(Object.keys(terraformSegments).sort(), [...contractRegions].sort())
|
|
})
|
|
|
|
test('the skew query compares a catalogued region against itself', () => {
|
|
const columns = terraformRegions.map((region) => region.replaceAll('-', '_'))
|
|
const hint = /hint_share: req_([a-z0-9_]+) \//.exec(terraform)
|
|
const placement = /placement_share: sel_([a-z0-9_]+) \//.exec(terraform)
|
|
assert.ok(hint && placement, 'skew query share columns not found')
|
|
assert.equal(hint[1], placement[1], 'the two shares must be about the same region')
|
|
assert.ok(columns.includes(hint[1]), `${hint[1]} is not one of ${columns.join(', ')}`)
|
|
})
|
|
|
|
test('the skew condition never divides by the placement share', () => {
|
|
// A zero-placement hour is the worst skew there is; MQL drops the row on x/0, so the ratio form
|
|
// silences exactly the case the alert exists for.
|
|
assert.ok(
|
|
!/hint_share \/ placement_share/.test(terraform),
|
|
'cross-multiply instead: hint_share > 2 * placement_share'
|
|
)
|
|
assert.match(collapse(terraform), /condition hint_share > 2 \* placement_share/)
|
|
})
|
|
|
|
test('the unhinted bucket stays out of the skew denominators', () => {
|
|
assert.ok(
|
|
!terraformRegions.includes('unhinted'),
|
|
'unhinted requests are a client-side choice, not a region; including them moves the share'
|
|
)
|
|
})
|