Skip to content

feat(lib): allow disabling creation stacks#215

Merged
jsteinich merged 1 commit into
open-constructs:mainfrom
gabegorelick:toggle-creation-stack
Jun 1, 2026
Merged

feat(lib): allow disabling creation stacks#215
jsteinich merged 1 commit into
open-constructs:mainfrom
gabegorelick:toggle-creation-stack

Conversation

@gabegorelick
Copy link
Copy Markdown
Contributor

@gabegorelick gabegorelick commented May 26, 2026 •
edited
Loading

Allow setting CDKTN_DISABLE_CREATION_STACK environment variable to disable capturing stack traces when constructing IResolvables.

Capturing stack traces is fairly slow in Node, so disabling them saves a lot of time when synthing large stacks.

Related issue

Fixes #209

Description

Don't capture stack traces if the CDKTN_DISABLE_CREATION_STACK env var is set.

Checklist

  • I have updated the PR title to match CDKTN's style guide
  • I have run the linter on my code locally
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation if applicable
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works if applicable
  • New and existing unit tests pass locally with my changes

gabegorelick
gabegorelick commented May 26, 2026
View reviewed changes
Comment thread packages/cdktn/src/tokens/private/intrinsic.ts
gabegorelick
gabegorelick commented May 26, 2026
View reviewed changes
Comment thread packages/cdktn/src/errors.ts
@gabegorelick gabegorelick mentioned this pull request May 26, 2026
Closed
1 task
@gabegorelick gabegorelick marked this pull request as ready for review May 26, 2026 03:33
@gabegorelick gabegorelick requested a review from a team as a code owner May 26, 2026 03:33
@gabegorelick gabegorelick changed the title feat(cdktn): allow disabling creation stacks feat(lib): allow disabling creation stacks May 26, 2026
@gabegorelick gabegorelick force-pushed the toggle-creation-stack branch from 512dc9a to 905459e Compare May 26, 2026 04:01
@jsteinich
Copy link
Copy Markdown
Contributor

jsteinich commented May 27, 2026

Found a couple pieces from the AWS CDK that make me wonder if we should just take this further:

Basically, they flat out removed them from tokens. That would be a breaking change, though I wonder if anyone would actually miss the behavior.

@gabegorelick
Copy link
Copy Markdown
Contributor Author

gabegorelick commented May 27, 2026

For aws/aws-cdk#11170, it looks like they added a CDK_DISABLE_STACK_TRACE env var and made stack traces dependent on CDK_DEBUG being enabled. That's pretty much what this PR does (minus the CDK_DEBUG gate).

But then in aws/aws-cdk#37642, they removed the stack traces completely. I am fine removing them from cdktn as well, if that's what the consensus decides. But like you say, it's more of a breaking change. The change in this PR is non-breaking.

@jsteinich
Copy link
Copy Markdown
Contributor

jsteinich commented May 28, 2026

For aws/aws-cdk#11170, it looks like they added a CDK_DISABLE_STACK_TRACE env var and made stack traces dependent on CDK_DEBUG being enabled. That's pretty much what this PR does (minus the CDK_DEBUG gate).

But then in aws/aws-cdk#37642, they removed the stack traces completely. I am fine removing them from cdktn as well, if that's what the consensus decides. But like you say, it's more of a breaking change. The change in this PR is non-breaking.

I expect we'll have a release with breaking changes in the next month (potentially the next release), so I don't mind a breaking change.

It is probably worth writing up a bit of detail on what we'd lose and post to slack to see if there's any pushback before going forward with that path though.

@gabegorelick
Copy link
Copy Markdown
Contributor Author

gabegorelick commented May 28, 2026

Should we merge this PR and then discuss whether it makes sense to remove the stack traces completely?

I'd hate for this to get blocked indefinitely while we figure out if we can delete the stack traces. But if you think we can come to a quick consensus, that's fine.

@gabegorelick gabegorelick mentioned this pull request May 28, 2026
Draft
8 tasks
@gabegorelick
Copy link
Copy Markdown
Contributor Author

gabegorelick commented May 28, 2026

In any case, here's a PR to delete creations stacks completely: #219

I will start a discussion on Slack.

@so0k so0k mentioned this pull request May 30, 2026
Open
so0k
so0k approved these changes May 30, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@so0k so0k left a comment

Choose a reason for hiding this comment

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

LGTM, only tagged this PR in the config surface tracking issue - but it's a new capability in CDKTN so feels fine to use the CDKNT_ prefix

I agree that merging this (potentially for a performance improving patch release with the fast stringify?)

@gabegorelick @so0k
feat(lib): allow disabling creation stacks
83fe106 
Allow setting `CDKTN_DISABLE_CREATION_STACK` environment variable to
disable capturing stack traces when constructing IResolvables.

Capturing stack traces is fairly slow in Node, so disabling them saves a
lot of time when synthing large stacks.

Fixes open-constructs#209
@so0k so0k force-pushed the toggle-creation-stack branch from 905459e to 83fe106 Compare May 30, 2026 15:51
@jsteinich jsteinich enabled auto-merge (squash) June 1, 2026 13:08
@jsteinich jsteinich merged commit 9f7d29f into open-constructs:main Jun 1, 2026
508 of 510 checks passed
@gabegorelick gabegorelick deleted the toggle-creation-stack branch June 1, 2026 13:37
@jsteinich jsteinich mentioned this pull request Jun 5, 2026
Merged
so0k added a commit that referenced this pull request Jun 5, 2026
@jsteinich @so0k @sakul-learning
chore: prepare v0.23.3 release (#240)
c27aa7f 
## 0.23.3

### chore

- chore(gha): run integration tests on Depot runners
[\#238](#238)
- chore(deps): bump typescript to 5.9.3 across the repo
[\#231](#231)
- chore: Upgrade jsii
[\#223](#223)
- chore: Revise README links to documentation and language support
[\#221](#221)
- chore: Pin node version to 22.22.2 via .nvmrc
[\#212](#212)
- chore: Add gradle cache to the examples and integration CI workflows
[\#211](#211)
- chore: Add PR workflow concurrency group and CI Label Filter job
[\#204](#204)
- chore(tests): add verdaccio to package.json
[\#202](#202)
- chore(tests): don't require other language builds
[\#201](#201)
- chore(tests): Ensure we terminate Verdaccio
[\#199](#199)
- chore: rewrite https://cdk.tf links
[\#197](#197)
- chore: Update examples help files
[\#194](#194)

### feat

- feat: faster JSON stringify
[\#224](#224)
- feat(lib): allow disabling creation stacks
[\#215](#215)

### fix

- fix(lib): surface stderr/stdout on exec() failures via toString()
[\#207](#207)

---------

Co-authored-by: so0k <vincent.drl@gmail.com>
Co-authored-by: sakul-learning <lucas@sheesh.host>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@so0k so0k so0k approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

cdktn: excessive cost of captureStackTrace

3 participants

@gabegorelick @jsteinich @so0k