Skip to content

refactor: use gritql for cdk and terraform metrics transforms#447

Open
cogwirrel wants to merge 3 commits intomainfrom
chore/gritql-metrics
Open

refactor: use gritql for cdk and terraform metrics transforms#447
cogwirrel wants to merge 3 commits intomainfrom
chore/gritql-metrics

Conversation

@cogwirrel
Copy link
Contributor

@cogwirrel cogwirrel commented Mar 1, 2026

Reason for this change

GritQL is a query language for modifying source code and supports typescript, terraform and python, and so the intention is to gradually migrate our ast manipulation to use this instead so that we can perform python ast updates, and have a consistent way to do this regardless of language.

Description of changes

Use GritQL to transform the metrics aspect for cdk and metrics cloudformation stack for Terraform.

Also replace illegible ast construction code for cognito auth with readable gritql! :)

Currently, the only way we can invoke gritql is to wrap the cli, published to npm as @getgrit/cli, but if biomejs/gritql#617 is merged we can switch to use napi bindings which is faster and more reliable - only napi bindings for mac are published right now so we can't use it yet unfortunately.

Description of how you validated changes

Unit tests, integration tests

Checklist


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@cogwirrel cogwirrel force-pushed the chore/gritql-metrics branch 6 times, most recently from ef02aa6 to 57d8c26 Compare March 2, 2026 02:13
@codecov-commenter
Copy link

codecov-commenter commented Mar 2, 2026

Codecov Report

❌ Patch coverage is 83.33333% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.47%. Comparing base (3b0f999) to head (51c1dba).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
packages/nx-plugin/src/utils/ast.ts 77.77% 9 Missing and 1 partial ⚠️
...gin/src/ts/react-website/cognito-auth/generator.ts 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #447      +/-   ##
==========================================
- Coverage   91.51%   91.47%   -0.04%     
==========================================
  Files          86       86              
  Lines        2957     3016      +59     
  Branches      646      664      +18     
==========================================
+ Hits         2706     2759      +53     
- Misses         98      106       +8     
+ Partials      153      151       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cogwirrel cogwirrel force-pushed the chore/gritql-metrics branch 5 times, most recently from 044ec30 to 820c289 Compare March 9, 2026 00:23
@cogwirrel cogwirrel marked this pull request as draft March 9, 2026 00:37
@cogwirrel cogwirrel force-pushed the chore/gritql-metrics branch from 820c289 to e0d7dc7 Compare March 9, 2026 02:25
@cogwirrel cogwirrel marked this pull request as ready for review March 9, 2026 03:08
// Exponential back-off: ~50ms, ~150ms, ~350ms
const delay = 50 * Math.pow(2, attempt) + Math.random() * 50;
const until = Date.now() + delay;
while (Date.now() < until) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we just do this with timeouts/sleep instead of busy wait?

await (new Promise(resolve => setTimeout(resolve, delay)))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah good shout! :)

await applyGritQLTransform(
tree,
METRICS_ASPECT_FILE_PATH,
`\`const tags: string[] = $old\`` +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just want to confirm this does the following:

  • Will only work if within a metric aspect body (where { ${WITHIN_METRICS_ASPECT},)
  • Will do nothing if it already contains the metric ($old <: not contains \'${info.metric}'\)
  • Will set $old to [$info.metric] if $old is []
  • Will set $old to [$items, $info.metric] if $old has $items

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep that's right! :) <: reads as "matches". This is a bit harder to read than most gritql queries as the += is mutative. Unfortunately multi-line arrays with trailing commas break what I'd intuitively do:

const tags: string[] = [$items] => const tags: string[] = [$items, info.metric]

@cogwirrel cogwirrel force-pushed the chore/gritql-metrics branch 2 times, most recently from 6c755b2 to 51c1dba Compare March 11, 2026 05:45
Use GritQL to transform the metrics aspect for cdk and metrics cloudformation stack for Terraform.
GritQL is a query language for modifying source code and supports typescript, terraform and python,
and so the intention is to gradually migrate our ast manipulation to use this instead so that we can
perform python ast updates, and have a consistent way to do this regardless of language.
@cogwirrel cogwirrel force-pushed the chore/gritql-metrics branch from 51c1dba to f390b40 Compare March 11, 2026 07:08
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.

3 participants