feat: measure real GCP usage in deployml costs - #74
Open
andrewtclim wants to merge 6 commits into
Open
andrewtclim wants to merge 6 commits into
andrewtclim wants to merge 6 commits into
Conversation
…sts commands - Rewrites infracost.py for v2 CLI (scan + inspect --json, dropped deprecated breakdown command) - Adds deployml estimate: pre-deploy cost prediction from config, no GCP credentials needed - Adds deployml costs: cost check against live deployed Terraform workspace - Adds check_infracost_authenticated() with macOS + Linux credential path support - Fixes copy_modules_to_workspace to include cloud_sql_postgres when mlflow uses postgresql - Updates deployml doctor to show infracost install + auth status - Adds 14 unit tests for infracost.py - Updates docs/features/costs.md and tutorial with estimate/costs commands
Infracost defaults usage-based services (Cloud Run, BigQuery, GCS) to zero usage, so the previous estimate only surfaced the always-on Cloud SQL cost and showed $0 for everything else -- misleading for students. The estimate command now feeds infracost a realistic usage profile (--profile light|heavy, default light) and renders a two-bucket view: ALWAYS-ON (fixed 24/7) vs USAGE-BASED (scales with activity), each line labelled in plain English, plus a 'biggest lever' call-out. - add usage_profiles.py: LIGHT/HEAVY dicts + render_usage_yaml() - infracost.py: ResourceCost dataclass, run_infracost_scan_with_usage(), fetch_resource_costs_detailed(), display_estimate(), run_estimate_analysis(); pin 'infracost inspect --file <scan>' instead of the global scan cache - cli.py: estimate gains --profile and calls the new flow - tests: 22 passing
Brings the branch up to date with 50 commits of main (GCP Cloud Run hardening, Windows compatibility, Grafana Secret Manager, repo cleanup). Conflict resolutions: - utils/infracost.py: main routed external tool calls through run_tool from platform_compat so .cmd wrappers work on Windows. Git aligned main's v1 run_infracost_breakdown against this branch's v2 run_infracost_scan_with_usage because both end in a subprocess.run call; they are different functions and the v1 one was intentionally removed here. Kept the v2 scan logic and converted every infracost invocation in this file to run_tool, including the two call sites git did not flag as conflicts. - tests/test_infracost.py: repointed the five subprocess.run patches at run_tool to match the new call path, following the convention main used in test_helpers.py, and updated the inspect argv assertion since run_tool takes the tool name separately from its args. - docs/features/costs.md: kept main's per-service price breakdown and Cost Optimization section, folding in this branch's two concrete tips (the backend_store_uri: sqlite config key and the explicit deployml destroy step). - docs/tutorials/gcp-cloud-run.md: adopted main's section renumbering, keeping the estimate step as 3.5 and renumbering the costs step to 9.5. Main dropped the prerequisites bullet list in favour of deployml doctor; preserved only the infracost install note, which doctor checks but does not install. config.yaml is no longer tracked, per main's cleanup in #73.
`deployml costs` re-ran the zero-usage infracost scan against the deployed
terraform, so it reported the same misleading numbers the estimate redesign
existed to fix: Cloud Run, BigQuery and GCS at $0, only Cloud SQL real. It
answered "what would this cost if nobody used it", which nobody is asking.
It now measures what the student actually did. New utils/measured_usage.py
reads Cloud Monitoring over a window (--days, default 30), maps each metric
onto the infracost usage key it corresponds to, and keys it to the terraform
address via `terraform show -json`, so per-service usage prices per service
instead of charging every Cloud Run service the whole stack's traffic.
Deliberately no billing integration: GCP has no API for "my spend so far", and
the BigQuery billing export needs billing-account admin plus a ~24h lag, which
is out of reach for a student. Measuring usage needs only monitoring.viewer.
Design rule is that nothing is invented. The scan gets an empty profile and
measured per-address usage only, so anything Monitoring could not report stays
at zero and the reason prints under "About these numbers". A silent $0 is the
bug being fixed; an explained $0 is fine. The three approximations (GCS class
A/B inferred from method names, project-scoped BigQuery bytes split evenly,
Cloud Run concurrency left at infracost's default) are disclosed the same way.
Measured usage also retires the guessed profiles: classify_usage_profile
compares real request volume against light/heavy on a log scale, so those stop
being pricing inputs and become a label ("you are a LIGHT user").
Also folded in, since they are the same code path:
- deploy's confirmation prompt was built from the zero-usage scan, i.e. it
asked students to approve a number we know is wrong. It now uses the same
usage-aware estimate, which let run_infracost_scan, run_infracost_analysis,
display_cost_breakdown and fetch_resource_costs be deleted as duplicates.
- check_infracost_authenticated can only prove a token file exists, so an
expired token passed pre-flight and failed at scan time with raw JSON
stderr. The failure is now recognised and answered with "infracost auth
login". Verified against a real expired token.
- the `bigquery` module description won for both the dataset and the tables, so
storage rows were labelled as query cost. Split into per-type descriptions.
- google_cloud_run_v2_job (teardown, offline_scoring, explainability) was in
neither label map and printed as a raw google_* type name.
- a failed `infracost inspect` returned [] silently, rendering as "nothing
costs money" under a non-zero total. It now says so.
- costs defaulted to workspace "development" while deploy/destroy/get-urls use
"default", so it could look in a directory deploy never wrote.
- costs and estimate bypassed _load_config_or_exit, turning a missing key into
" Estimate failed: 'provider'".
Metric and aligner pairs were validated against the live Monitoring API.
database/up is GAUGE/INT64 rather than BOOL, so ALIGN_FRACTION_TRUE is
rejected with HTTP 400; ALIGN_MEAN over a 0/1 gauge is the same uptime
fraction and is pinned by a test.
Tests: 85 -> 106, all passing.
docs/api/cli-commands.md never mentioned either cost command. Both now have a section with options, sample output, and their prerequisites. docs/features/costs.md claimed both commands show $0 for usage-based services, which stopped being true when estimate became usage-aware and is now the whole point of costs. Rewritten around the two questions the commands answer -- what will this cost (assumed usage) versus what is it costing me (measured usage) -- with the fixed-vs-usage distinction up front, since that is what makes the numbers legible: how hard you use the stack barely matters next to whether the database is running. tests/README.md described only the helpers tests. Now lists all six test modules, the mocking conventions, and the regression guards worth not breaking.
Verified the one load-bearing assumption in the measured-usage path: that the terraform address `terraform show -json` reports is byte-identical to the address infracost keys `resource_usage` on. If they disagree the usage file is silently ignored and every usage line prices at $0 -- the exact bug measured usage exists to fix, just relocated. Checked against a real cached `infracost scan` of the rendered deployml Cloud Run stack (infracost records each resource's address in its "name" field) and diffed it against build_resource_map() run on the deployed state. All 10 costable addresses match exactly, count indices included: module.experiment_tracking_mlflow[0].google_cloud_run_service.mlflow[0] google_storage_bucket.artifact_tracking_mlflow_artifact ... Those strings are now pinned in a test, since the tempting "cleanup" here is to normalise or strip the count index, which would break the binding silently. Reading the real scan also turned up an unescaped-quote bug. for_each addresses embed their own quotes, e.g. module.cloud_sql_postgres.google_project_service.required["cloudkms.googleapis.com"] and render_usage_yaml wrapped addresses with a naive f-string, so such an address closed the YAML string early and made the whole usage file unparseable. Not reachable today -- no measured type uses for_each -- but it would have been a silent trap for whoever added one. Now emitted via json.dumps, whose escaping is valid YAML double-quoted style. Tests: 106 -> 108.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
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.
Supersedes #64 (its commits
da56a5fanded65724are carried over here, so nothing is lost). Closing that one.deployml estimategot usable in #64: it feeds infracost a light/heavy usage profile and splits the output into always-on vs usage-based. Butdeployml costsnever got the same treatment — it just re-ran the zero-usage scan against the deployed Terraform, so it printed exactly the misleading numbers the redesign existed to kill: Cloud Run, BigQuery and GCS at $0, only Cloud SQL real. It answered "what would this cost if nobody used it", which isn't a question anyone has. The follow-up section of #64 named this as the next step.This finishes the pair:
estimate= before you deploy,costs= what you're actually running at.summary
deployml costsnow measures real usage instead of assuming it. Newsrc/deployml/utils/measured_usage.pyreads Cloud Monitoring over a window (--days, default 30), maps each metric to the infracost usage key it corresponds to, and keys it to the Terraform address. Why: once the stack exists we don't have to guess, and the student's actual behaviour is the only number that answers "what is this costing me".render_usage_yaml()grew aresource_usage:block keyed by Terraform address. Why: infracost'sresource_type_default_usageapplies one number to every resource of a type, so 50k requests measured across three Cloud Run services would bill 3×50k.classify_usage_profile()compares real request volume against light/heavy on a log scale. Why: light/heavy stop being pricing inputs the student is billed for and become a label — "you are a LIGHT user".requestswas already a dep, and the Monitoring REST API is called with an ADC token fromgcloud auth application-default print-access-token. Why: matches the repo'srun_toolshell-out style rather than pulling ingoogle-cloud-monitoring.deploy's confirmation prompt no longer shows a number we know is wrong. It used the zero-usage scan; it now uses the same usage-aware estimate. Why: it was asking students to approve a misleading figure. This also letrun_infracost_scan,run_infracost_analysis,display_cost_breakdownandfetch_resource_costsbe deleted as duplicates (~143 lines).check_infracost_authenticated()can only prove a token file exists, so an expired token passed pre-flight and failed later with raw JSON stderr._report_scan_failure()recognises the auth case and answersinfracost auth login. Why: hit this exact false positive twice while working on this.bigquerymodule description won for both the dataset and the tables, so storage rows were labelled as query cost;google_cloud_run_v2_job(teardown/offline_scoring/explainability) was in neither label map and printed as a rawgoogle_*type; a failedinfracost inspectreturned[]silently, rendering as "nothing costs money" under a non-zero total;costsdefaulted to workspace"development"while deploy/destroy/get-urls use"default", so it could look in a directorydeploynever wrote;costsandestimatebypassed_load_config_or_exit.roles/monitoring.viewertoREQUIRED_GCP_IAM_ROLESsodoctor --project-idchecks it for free (monitoring.googleapis.comwas already inREQUIRED_GCP_APIS); added thecost_analysisblock toconfig.example.yaml, which three commands read and no example documented; rewrotedocs/features/costs.md, which still claimed usage-based services show $0; addedestimateandcoststodocs/api/cli-commands.md, where neither had appeared at all.Sample output:
test plan
PYTHONPATH=src pytest tests/— 108 passing, up from 85.namefield) againstbuild_resource_map()on the deployed state: all 10 costable addresses match exactly, count indices included (module.experiment_tracking_mlflow[0].google_cloud_run_service.mlflow[0]). Those strings are now pinned in a test, since the tempting cleanup is to normalise the index away.cloudsql.googleapis.com/database/upis GAUGE/INT64, not BOOL, soALIGN_FRACTION_TRUEreturns HTTP 400 and the uptime line degraded to a "could not read" note.ALIGN_MEANover a 0/1 gauge is the same fraction; a test pins the aligner.build_resource_map()against a genuine 145KBterraform.tfstatefrom a real deploy. Caught thatterraform show -jsonincludes data sources (module.*.data.google_project.current), which inflated the resource map from 10 to 15; now filtered onmode == "managed"._cloudbuildstaging bucket, and it is correctly excluded because it isn't in the stack's state. A student's unrelated resources can't inflate their deployml cost.--daysout of range; missing config; malformed config; expired infracost token (verified against a realoauth2: "invalid_grant"failure while the pre-check still returnedTrue).costsagainst a live deployment with real traffic. Not run — the test stack is destroyed and standing it up bills Cloud SQL. The measurement layer is verified against the live Monitoring API and the pricing layer against a real cached scan, but the two have not been exercised together on a running stack.deploy's new estimate prompt. Not run — needs a real deploy.follow-up (not in this PR)
INFORMATION_SCHEMA.JOBS_BY_PROJECTwould fix this properly.check_infracost_authenticated()still can't prove a token is valid without making a live call. The failure is now handled where it surfaces, but the pre-flight check remains a file-existence test.--compare(sqlite vs postgres) from the original proposal is still unbuilt. Given that Cloud SQL is ~96% of the bill, a one-command "what if I switched" is probably the highest-value thing left.