Repository navigation
feat(aws-startup-advisor): close heroku-to-aws sizing-table and fast-path gaps from a large Enterprise estate - #608
Conversation
…path gaps from a large Enterprise estate A large Heroku Enterprise run deferred its biggest Postgres databases (private-8/-9/-xl-9), its Redis 'mini' instances, its Private Space Kafka add-ons (private-standard-*), and every scheduler add-on, because the lookup tables had no rows for those plans and the fast-path key for Heroku Scheduler did not match the slug the CLI actually reports. Sizing tables (knowledge/design): - postgres-rds-sizing: add standard-8/9/10, premium/private/shield -8, -9, -10 and the L/XL -6 and -9 variants (3-8 TB plans). Instance classes continue the existing meets-or-exceeds ladder and all exist in the vendored rate card. - redis-elasticache-sizing: add mini, private-9/10/12/14 and shield-3..14. Correct the memory_limit of premium-5..14 and private-3..7 to the plan sizes Heroku publishes today (the skill's own pricing cache already had them); the old values were ~10x too large and over-sized ElastiCache by two to three node classes. - kafka-msk-sizing: add private-standard-0/1/2 and shield-standard/extended rows (same capacity as the Common Runtime plan of the same number). - fast-path-addons: key Heroku Scheduler as 'scheduler' (the CLI slug) with a 'heroku scheduler' alias; add advanced scheduler, rediscloud, jawsdb, jawsdb maria, auth0, herokuconnect and inference (heroku-inference). New targets without rate rows are flagged as unpriced in their notes; the Estimate's existing unpriced handling applies. Discovery: - discover-live: parse Following:/Forked From: from pg:info into config.pg_role / pg_leader so leader+follower pairs on one app are visible (Advanced-tier add-ons already carry this in advanced_pools[]). - design-mapping 2B: collapse followers onto the leader's reader/replica (warning + metadata.postgres_followers_collapsed), warn on forks, and warn when Terraform-only discovery cannot tell them apart. - discover-assemble rule 5a + schema: environment_tier per app from pipeline stage, else a name-token heuristic; counts in metadata.apps_by_environment_tier. Pricing cache: full Postgres Standard/Premium/Private/Shield Classic plan lists with current public prices. Private and Shield are NOT priced equal to Premium as the cache previously claimed. The Advanced-tier section is untouched. tests/test_heroku_postgres_advanced_pricing.py's Classic-row guard now compares cell values instead of column padding and expects the corrected private-0/shield-0 prices. Ported from awslabs/startups#364 (closed in favour of this PR).
leon1418
left a comment
There was a problem hiding this comment.
[🤖 AI review 🤖]
Reviewed b16463067f13af25e680e8f07c91be07f244211f against 00840a095dcc53242dcb19402e7140fa896d2d46 with OCR-assisted repository review and one independent general reviewer. Five corrections are needed:
- Preserve follower read capacity across Design, Estimate and Generate, including unresolved leaders.
- Size the nine affected Redis rows using memory available after the default reserve.
- Give the 13 new large Aurora mappings adequate per-instance capacity or an explicit deferral.
- Load and populate the Redis Cloud sizing/configuration contract.
- Make all documented Heroku Connect aliases survive normalization.
Validation: all ten changed files and relevant unchanged consumers reviewed; 22 original-source tests passed; core rate coverage, vendor plan prices, alias and capacity checks completed. Current-head CI reports success. Static contract probes were used; no live account migration or AWS deployment was run. Details and bounded acceptance criteria are in the inline comments.
…es and follower handling - Followers (P1): 2B defers followers to a new 2B-F pass that runs after every leader is mapped. Leader found via the new pg_attachment_names[] Discover records per Postgres add-on. Mapped leader -> aws_config.read_replicas += 1 (generated as an RDS read replica or an extra Aurora reader, and priced: RDS x(1+read_replicas), Aurora x(1+max(1,read_replicas)) - which also makes the Aurora estimate match the writer+reader Terraform already generates). Unresolved leader -> explicit Specialist Gate entry. Unique-data accounting stays separate from replica capacity. - Redis (P1): the nine -10/-12/-14 rows now pick nodes whose usable memory after the 25% default reserve meets the plan (m6g.4xlarge/8xlarge/12xlarge). - Aurora (P2): the fourteen -9/-l-9/-xl-9/-10 rows map to db.x2g.16xlarge (1,024 GiB); rate row added to the shared pricing file ($12.064/hr, AWS Pricing API, us-east-1) and vendored copies re-synced. - Redis Cloud (P2): the Redis sizing table loads for rediscloud-only inventories and 2E Step 4b emits a full ElastiCache entry by plan memory. - Connect alias (P2): aliases are keyed by the post-strip form (connect -> herokuconnect); the dead heroku-connect key is removed. - Schema: environment_tier, apps_by_environment_tier, pg_role, pg_leader and pg_attachment_names added to the fenced inventory examples so the artifact validator admits them (found by probing). - tests/test_heroku_sizing_capacity.py: usable-after-reserve >= plan for the Redis rows, Aurora RAM >= source RAM for the large rows, every emitted class has a rate row, and the documented normalization resolves every Connect and Scheduler spelling while rejecting partial matches. Review findings: leon1418 on PR aws#608.
…and in Batch 2, plan default, workbook guards Review findings on PR aws#611. - preferences.json contract (clarify-assemble.md): add data.database_sizing_basis, data.rightsizing_candidates_shown[] and commercial.heroku_contract (Q16) to the fenced template so validate-artifacts accepts them; checklist rows for both. - Clarify Q6d: one Right-sizing rule shared by Q6d, Design 2B step 2a and the Estimate - plan storage from the pricing cache Storage column, candidate = utilization <= 25 AND fitting_plan != plan AND data_size_gb > 0, fitting plan = smallest same-family row with Storage >= 2x data and never more storage than the source plan; 20 GB RDS floor applies to allocated storage in Design step 6 only. Q6d joins Batch 2; default is plan unless sources.Q6d == user; fast path keeps plan. - Design 2a applies only to resource_ids in rightsizing_candidates_shown; worked cases. - Estimate: example re-priced on standard-4 -> standard-0, rounding rule stated, missing pg_role treated as leader with role: "unknown". - Report and workbook carry the storage-only caveat; workbook rows keyed by resource_id with a Role column; malformed candidate entries are skipped; the Postgres data total is labelled "all databases (roles unknown)" when discovery wrote no pg_role (role population itself lands in PR aws#608). Verified: mise run ci (lint:md, lint:artifacts, lint:vendored-parity, lint:skill-refs, pytest 1475 passed + 225 subtests, telemetry) passed; validate-artifacts --run-dir on a preferences.json carrying the three new keys now PASSes (was 3x UNKNOWN_KEY).
herosjourney
left a comment
There was a problem hiding this comment.
Reviewed: head b164630 (base 00840a0), 10 files (+609/−81). Only heroku-to-aws is touched, in the single plugin tree plugins/aws-startup-advisor (this repo has no migrate/ twin).
Ran:
pytest plugins/aws-startup-advisor/tests -q: 1475 passed, 225 subtests passed.- Changed JSON parses: 4/4 OK.
tools/sync-plugin-skills.py --check: in sync.sync-vendored.py --check: PASS, 62 files.check-skill-refs.py --check: PASS, 1498 refs, 0 missing.validate-artifacts.py --fixtures: PASS, 104 artifacts.- A script cross-checking every sizing row against
heroku-pricing-cache.mdand against instance memory.
GitHub CI (startup-advisor, CodeQL, author checklist) is green at head and not restated.
Primary sources (read 2026-11-10):
- Heroku Dev Center: heroku-postgres-plans, heroku-postgres-and-private-spaces, heroku-postgres-follower-databases, heroku-inference-api-model-cards.
- Heroku Elements: heroku-postgresql, heroku-redis, heroku-kafka.
- AWS: AuroraUserGuide Concepts.DBInstanceClass.SupportAurora; RDS USER_ReadRepl and Concepts.MultiAZ; AWS Price List API (AmazonRDS, us-east-1).
Did not run: live AWS calls, an end-to-end skill run, terraform plan, or mise run ci locally (CI ran it). ElastiCache node memory figures were not re-fetched; the node-type names were confirmed.
Cross-plugin parity: PASS. There is a single tree, and vendored parity is unchanged.
Merge state: merges cleanly into main. It has a textual conflict with #610 in references/shared/schema-discover-heroku.md, and shares files with #609, #611 and #614 without textual conflict. #609 reads environment_tier / apps_by_environment_tier, which only this PR produces, so landing #608 → #609 is the natural order.
Change narrative: A large Enterprise run deferred its biggest databases, Redis mini, Private/Shield Kafka and Scheduler, because lookup rows were missing or mis-keyed. The PR:
- adds sizing rows;
- re-keys Scheduler to its CLI slug;
- adds 7 fast-path add-ons;
- corrects Private/Shield Postgres prices and Redis memory sizes;
- adds follower/fork and environment-tier discovery.
Body vs diff: it mostly matches. The body says it adds "premium … -8, -9", but premium-8/premium-9 already exist at base.
Verified correct:
- All Standard/Premium/Private/Shield Postgres plan names, RAM, storage and $/month in the cache match Heroku Elements and Dev Center (56 rows). Private and Shield really are priced above Premium.
- Redis plan sizes and prices match Elements (25 plans, including
miniat $3). - Kafka
private-standard-*/shield-*names and prices match Elements. - The plan sets in the sizing JSONs and the cache are identical.
- Every RDS class meets or exceeds the plan RAM.
- MSK storage covers the Heroku capacity on all 19 Kafka rows.
- Rate-card RDS and Aurora rates match the Price List.
- The Scheduler re-key fixes a real miss.
| Axis | Stance | One line |
|---|---|---|
| Parsimony | advisory | Mostly data-only and in idiom. Three new producer fields have no reader, and runtime strings carry revision history. |
| Completeness | finding | Follower collapse is false on the RDS branch and for cross-app followers. The RedisCloud sizing instruction can't be reached. No regression test covers the headline fix. |
| Security | clear | No new command (pg:info was already captured), no IaC, no credentials. |
| Alternatives | advisory | Two below. |
| Contradictions | finding | The RedisCloud row says "ElastiCache Redis" vs the Valkey default. It sizes from memory_limit, which the sizing file declares provenance-only. The Heroku Inference note contradicts Heroku's model cards. |
| Overlap | advisory | Plan RAM is restated in the cache and in the JSONs; a parity test would keep them from drifting. |
| Cross-plugin parity | clear | Single tree; vendored and skill parity PASS. |
| Propagation & data | finding | SSOT: heroku-pricing-cache.md + knowledge/design/*.json. Consumers swept: design-mapping, estimate-cost-engine, generate-terraform, generate-report, clarify-interview. The Aurora 768 GB/1 TB rows are undersized. |
Blocking
- [Completeness / Contradictions] Follower collapse removes databases and read capacity the AWS design never replaces —
references/phases/design/design-mapping.md:261
A Heroku follower is a read replica of a leader on the same app; on AWS that role is served by the leader's Aurora reader or RDS Multi-AZ standby / read replica.
Both premises contradict the primary sources:
- Heroku: "Followers don't need to be tied to the same application … you can provision the leader and follower databases on separate applications" (heroku-postgres-follower-databases). The follower shows its leader by attachment name ("Following: DATABASE"), so
pg_leaderdoes not identify an inventory resource. - AWS: "A Multi-AZ DB instance deployment has one standby DB instance that provides failover support, but doesn't serve read traffic" (Concepts.MultiAZ).
- The RDS branch, the default path, emits a single
aws_db_instance(generate-terraform.md:1747) and no read replica.
Consequence: on the default path, an app that reads from HEROKU_POSTGRESQL_<COLOR>_URL loses that endpoint, while the design tells the user reads are covered. A follower whose leader lives on another app gets no database designed at all.
Fix: collapse only when pg_leader resolves to a heroku-postgresql resource on the same app in this inventory; otherwise design it normally and warn. Make the claim branch-accurate:
"On Aurora, reads go to the cluster reader endpoint. On RDS, a Multi-AZ standby does not serve reads — add a read replica if the app routes reads to this follower."
Also drop "excluded from data-volume totals": no data-volume total exists in Design, Estimate or Generate. Regression: a worked-case table in 2B covering:
- same-app follower × RDS;
- same-app follower × Aurora;
- cross-app follower;
- fork.
Should-fix
- [Propagation & data] The Aurora class is half the plan RAM on every 768 GB / 1 TB row —
knowledge/design/postgres-rds-sizing.json:141(same on all -9/-l-9/-xl-9/-10 rows; pre-existing atpremium-9)
"aurora_instance_class": "db.r6g.16xlarge"
db.r6g.16xlarge is 512 GiB. Aurora PostgreSQL supports db.x2g.16xlarge (1024 GiB). The cross-check flagged 14 undersized Aurora rows and 0 RDS rows. The estimate also understates cost by (12.064 − 8.306) × 730 × 2 = $5,486.68/month per database (Price List, us-east-1).
Fix: set db.x2g.16xlarge on those rows. Add "db.x2g.16xlarge": 12.064 under aurora_postgresql.instances in skills/shared/pricing/aws-infra-pricing.json, then run tools/sync-vendored.py.
- [Contradictions / Completeness] The RedisCloud row has the wrong engine label and a sizing instruction Design can't reach —
knowledge/design/fast-path-addons.json:58
- (a)
redis-elasticache-sizing.jsonloads only when "inventory has a heroku-redis addon" (design.md:13), so a RedisCloud-only app never has it in context. - (b) That file says
memory_limitis "PROVENANCE ONLY … do not recompute node_type from it". - (c)
"aws_services": ["ElastiCache Redis"](:57) is the Redis OSS exception label. The default engine is Valkey (design-mapping.md:351/362).
Fix: use "ElastiCache Valkey". Then either extend the _knowledge _when to include rediscloud and add an explicit 2C rule, or state that the node type is chosen in Design with a warning.
- [Propagation & data] The Heroku Inference note contradicts Heroku's model cards —
knowledge/design/fast-path-addons.json:83
"Map to Bedrock directly; model IDs carry over."
Heroku Inference IDs are Heroku-specific (claude-sonnet-5-5, nova-pro, gpt-oss-120b) and sit on an OpenAI-style /v1/chat/completions endpoint. Bedrock uses different model / inference-profile IDs and a different API, so copied INFERENCE_MODEL_ID values fail. Fix: say the IDs and API must be remapped, and route the code rewrite to the llm-to-bedrock skill.
-
[Completeness] Three new fields have no reader —
discover-assemble.md:53,design-mapping.md:264. A sweep of the skill tree,tests/andfixtures/forapps_by_environment_tier,environment_tierandpostgres_followers_collapsedfinds only the producer and schema lines. (#609 adds readers for the first two; nothing readspostgres_followers_collapsed.) Fix: render them in thegenerate-report.mdestate summary, or drop them. -
[Propagation & data] No regression test for the headline fixes, and the edited test contradicts its own name —
tests/test_heroku_postgres_advanced_pricing.py:216.test_pricing_cache_classic_tier_rows_untouchednow asserts changed values. Fix: rename it to…_match_published_prices, and add a test asserting:
- the
schedulerkey and alias; - every Classic cache plan has a sizing row with matching RAM /
memory_limit; - class GiB ≥
ramfor both class columns. This last check would have caught Should-fix 1.
Nits
design-mapping.md:405andkafka-msk-sizing.json:10say "3 brokers" for standard/extended/private; add Shield, which the PR adds atkafka-msk-sizing.json:8.design-mapping.md:486-487: theheroku-connectalias can't fire, because step 2 strips theheroku-prefix first.jawsdb-mariaalready normalizes without the alias.redis-elasticache-sizing.json:264andheroku-pricing-cache.md:106-107carry revision history in runtime-read text, and the "~10x … two to three instance classes" figure is off. The actual ratios are 1.4–2.5× on premium and 10–33× on private.- PR body:
premium-8andpremium-9existed at base.
Heads-up (out of diff, pre-existing — not blocking)
- Heroku Kafka Extended plans are "8 Kafka Brokers" with 400 / 800 / 2400 GB capacity (Elements).
_broker_count_and_azsmaps extended to 3 brokers, and the cache lists 1.5 / 3 / 6 TB. The newshield-extended-*rows inherit this. - Kafka
basic-1/basic-2are priced in the cache but have no sizing row. - Redis legacy
premium-8(15 GB) maps tocache.m6g.xlarge, which has about 12.9 GiB (not re-fetched this session).
Alternatives (max 2)
- Model a same-app follower as an RDS read replica on the leader instead of collapsing it. This preserves read capacity on the default RDS branch. Recommendation: do the guarded collapse now and the replica as a named follow-up.
- Defer 768 GB+ plans on the Aurora path instead of adding an x2g rate. That would defer the biggest databases again, which undercuts the PR's goal. Recommendation: switch the class to x2g as in Should-fix 1.
Verdict
Comment. One Blocking finding (follower collapse) and five Should-fix items are open. The data additions themselves check out against Heroku and AWS primary sources.
Commenting rather than approving: author == reviewer identity.
…label, report readers for tier/follower metadata Review findings on PR aws#608. - design-mapping 2B/2B-F: pg_leader is matched by attachment name; followers of a leader on another app fall to the Specialist Gate with a warning saying so; branch-accurate served-by text (Aurora reader endpoint vs RDS read replica, Multi-AZ standby does not serve reads); follower data_size_gb leaves the volume total only when the leader resolved. - fast-path rediscloud row: aws_services ElastiCache Valkey; memory_limit lookup in 2E 4b named as the documented exception to PROVENANCE ONLY in both files. - fast-path inference row: IDs and API must be remapped; hand off to llm-to-bedrock. - generate-report Step 1 gathers metadata.apps_by_environment_tier and metadata.postgres_followers_collapsed; decision-summary estate line and exec-costs collapsed-follower line render them. - kafka: Shield in the 3-broker list and _topology_preservation. - redis _plan_memory_note: current state only, no revision history. - tests: rename classic-rows test to _match_published_prices; add cache<->sizing parity (RAM / memory_limit per plan), class GiB >= plan RAM, scheduler row. Verified: mise run ci (lint:md 0 errors, lint:artifacts PASS, 1654 pytest passed, 311 telemetry passed).
…and in Batch 2, plan default, workbook guards Review findings on PR aws#611. - preferences.json contract (clarify-assemble.md): add data.database_sizing_basis, data.rightsizing_candidates_shown[] and commercial.heroku_contract (Q16) to the fenced template so validate-artifacts accepts them; checklist rows for both. - Clarify Q6d: one Right-sizing rule shared by Q6d, Design 2B step 2a and the Estimate - plan storage from the pricing cache Storage column, candidate = utilization <= 25 AND fitting_plan != plan AND data_size_gb > 0, fitting plan = smallest same-family row with Storage >= 2x data and never more storage than the source plan; 20 GB RDS floor applies to allocated storage in Design step 6 only. Q6d joins Batch 2; default is plan unless sources.Q6d == user; fast path keeps plan. - Design 2a applies only to resource_ids in rightsizing_candidates_shown; worked cases. - Estimate: example re-priced on standard-4 -> standard-0, rounding rule stated, missing pg_role treated as leader with role: "unknown". - Report and workbook carry the storage-only caveat; workbook rows keyed by resource_id with a Role column; malformed candidate entries are skipped; the Postgres data total is labelled "all databases (roles unknown)" when discovery wrote no pg_role (role population itself lands in PR aws#608). Verified: mise run ci (lint:md, lint:artifacts, lint:vendored-parity, lint:skill-refs, pytest 1475 passed + 225 subtests, telemetry) passed; validate-artifacts --run-dir on a preferences.json carrying the three new keys now PASSes (was 3x UNKNOWN_KEY).
…table-coverage # Conflicts: # plugins/aws-startup-advisor/skills/heroku-to-aws/references/shared/schema-discover-heroku.md
Stale: findings addressed and all threads resolved; branch has since been updated with main (84393b4). Re-review requested.
|
All review findings are addressed and every thread is resolved. The branch is now merged up to current main (head |
Problem
Running
heroku-to-awsagainst a large Heroku Enterprise account (hundreds of apps, multi-terabyte Postgres, dozens of Redis instances, Kafka in Private Spaces, a long tail of third-party add-ons) left important parts of the estate as "Deferred":private-8,private-9,private-xl-9have no sizing row, so the biggest data stores in the estate were unsizedminiprivate-standard-*scheduler, but the fast-path row was keyedheroku scheduler, so each one fell through to the specialist gate;advanced-schedulerhad no row at all. Together these were the large majority of the unmapped add-ons.rediscloud,auth0,herokuconnect,jawsdb,heroku-inferencehad no fast-path rowThe same run also multiple-counted one app's data volume (a leader plus followers each reported at full size) and had no way to say how much of the estate is production. Separately, the pricing cache said Private and Shield Postgres plans are priced equal to Premium; they are not, and the Redis sizing table carried plan memory values roughly 10× what Heroku publishes today.
Fix
Sizing tables (
knowledge/design/)postgres-rds-sizing.json:standard-8/9/10, andpremium/private/shield-8,-9,-10,-l-6,-xl-6,-l-9,-xl-9. Instance classes follow the table's existing ladder; every class already has a rate row in the vendored pricing file.redis-elasticache-sizing.json:mini,private-9/10/12/14,shield-3..14. Also correctsmemory_limitonpremium-5..14andprivate-3..7to the plan sizes Heroku publishes (which the skill's ownheroku-pricing-cache.mdalready carried).private-7was 100 GB; it is 7 GB. The old values pushed ElastiCache two to three node classes too high.kafka-msk-sizing.json:private-standard-0/1/2,shield-standard-*,shield-extended-*.fast-path-addons.json: row keyedschedulerwith aheroku scheduleralias; new rowsadvanced scheduler,rediscloud,jawsdb,jawsdb maria,auth0,herokuconnect,inference. Targets with no rate row say so in their notes; the Estimate's existing unpriced handling (warn + exclude from total) applies — no rates were invented.Discovery / design
discover-live.md: parseFollowing:/Forked From:frompg:infointoconfig.pg_role/config.pg_leader(Classic path; Advanced-tier add-ons already carry this inadvanced_pools[]).design-mapping.md2B: followers collapse onto the leader (no separate instance; warning +metadata.postgres_followers_collapsed[]), forks get a confirm-with-customer warning, Terraform-only runs get a "cannot tell leaders from followers" warning per multi-DB app.discover-assemble.mdrule 5a + schema:environment_tierper app (pipeline stage first, else name-token heuristic), counts inmetadata.apps_by_environment_tier.design-mapping.mdslug table: new slugs plus a note that deferringscheduleras "Not found in Fast_Path_Table" is a normalization bug.Pricing cache: full Postgres Classic plan lists (Standard/Premium/Private/Shield) with current public prices from elements.heroku.com and devcenter. The Advanced-tier section is untouched.
tests/test_heroku_postgres_advanced_pricing.py's Classic-row guard now compares cell values rather than column padding and expects the correctedprivate-0/shield-0prices.Ported from awslabs/startups#364, which is closed in favour of this PR. Independent of the companion estate-workbook PR; either can land first.
Scope
plugins/aws-startup-advisor/skills/heroku-to-aws/only: fourknowledge/design/*.jsontables,references/phases/design/design-mapping.md,references/phases/discover/discover-live.md,references/phases/discover/discover-assemble.md,references/shared/schema-discover-heroku.md,references/shared/heroku-pricing-cache.md; plus one guard test inplugins/aws-startup-advisor/tests/. No root or.githubchanges.Known limitations: rate rows for RDS MySQL/MariaDB, Cognito, AppFlow and Bedrock are not added to
skills/shared/pricing/aws-infra-pricing.json(shared with azure-to-aws), so those new fast-path targets render as unpriced lines until a separate change adds them. Plan specs and prices were cross-checked against devcenter.heroku.com/articles/heroku-postgres-plans, …/heroku-postgres-and-private-spaces and elements.heroku.com/addons/heroku-redis (Oct 2026); no live run against a customer account is included.Type of Change
Select at least one.
Startup Advisor review checklist
By submitting this PR, I confirm that:
mise -C plugins/aws-startup-advisor run ciand all checks pass.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.