-
Notifications
You must be signed in to change notification settings - Fork 39
feat(telemetry): e2e test suite + create-blocks-app worker parity + cross-platform console #133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
sarayev
wants to merge
44
commits into
main
Choose a base branch
from
feat/telemetry-e2e-tests-clean
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,676
−105
Open
Changes from all commits
Commits
Show all changes
44 commits
Select commit
Hold shift + click to select a range
8100bcb
test(telemetry): add isolated comprehensive e2e telemetry suite
sarayev 3b6cf80
ci: add Telemetry E2E job to PR checks workflow
sarayev bcf6bc7
fix(telemetry): inherit worker stderr when NODE_DEBUG=blocks-telemetr…
sarayev d07e9b0
refactor(telemetry-e2e): rewrite tests to invoke real scripts instead…
sarayev af16467
fix: update changeset to include @aws-blocks/core patch
sarayev e56328c
fix(create-blocks-app): remove CI telemetry suppression to match core…
sarayev 6a88007
test(telemetry): add console.ts script and use it in e2e test
sarayev d640f31
fix(telemetry-e2e): fix stderr capture timing for delivery assertions
sarayev 9246009
fix(telemetry-e2e): add AWS creds to CI job, increase timeouts
sarayev fd9bc0e
fix(ci): add environment: publish for OIDC role assumption in telemet…
sarayev b2d1237
fix(telemetry-e2e): add diagnostic output on failure, fix projectId s…
sarayev be87eb9
fix(telemetry-e2e): add index.cdk.ts and fix script paths for CDK com…
sarayev 72b41a8
fix(telemetry-e2e): run create-blocks-app from APP_ROOT so npx resolv…
sarayev 6ee51e8
fix(ci): increase telemetry e2e job timeout to 40min for CDK deploy t…
sarayev cb8074a
fix(create-blocks-app): port telemetry to detached subprocess worker
sarayev 4625de7
fix(ci): add BLOCKS_STACK_SUFFIX and cleanup step for telemetry e2e
sarayev a3cafa3
fix(telemetry-e2e): add stderr to sandbox SUCCESS failure diagnostic
sarayev 634a5ce
fix(telemetry-e2e): fix create-blocks-app resolution and stack conflicts
sarayev d0c0ebb
fix(telemetry-e2e): add AWS credential preflight check before deploy …
sarayev 1cb0d15
fix(telemetry-e2e): remove HOME override — use real HOME with seeded …
sarayev c8a1f2b
fix(telemetry-e2e): add pre-cleanup + deployOnly to match comprehensi…
sarayev 4a8de53
fix(telemetry-e2e): scope SSM param by stack id + run create-blocks-a…
sarayev 455af4d
fix(telemetry-e2e): drop in-script pre-cleanup destroy; raise suite t…
sarayev dc96860
fix(console): cross-platform, headless-safe browser open
sarayev b28bea6
test(telemetry-e2e): surface console command output on failure
sarayev 70633ad
test(telemetry): add missing test categories from reference suite
sarayev cc66b92
test(telemetry): add missing test categories + fix test correctness i…
sarayev 10422db
fix(telemetry-e2e): run blocks-telemetry CLI from built dist via node
sarayev 3d4dfc1
fix(telemetry-e2e): address review findings — docs, port reliability,…
sarayev c5b6739
fix(telemetry-e2e): fix getNextPort — use sequential counter instead …
sarayev f071e54
fix(telemetry-e2e): fix vendorize npx resolution + relax delivery ass…
sarayev 0194f96
fix(telemetry-e2e): vendorize uses process.exit — assert no telemetry…
sarayev 6a77900
fix(telemetry-e2e): stop killing process groups — use SIGTERM on chil…
sarayev 34a8e5c
fix(telemetry-e2e): remove detached:true from spawnDevServer
sarayev b175c51
fix(telemetry-e2e): restore detached:true + process group kill
sarayev f102023
fix(telemetry-e2e): don't kill dev server — disconnect and let it die…
sarayev 5d0abb1
fix(dev-server): never reclaim a port owned by self or an ancestor
sarayev 4010bcc
revert(dev-server): undo self/ancestor reclaim guard (keep PR test-only)
sarayev 92bed51
test(telemetry-e2e): dev FAIL via privileged-port bind (no in-runner …
sarayev bb2b03a
Merge branch 'main' into feat/telemetry-e2e-tests-clean
sarayev d9c1ffb
Merge remote-tracking branch 'aws-blocks/main' into feat/telemetry-e2…
sarayev eb1db0a
Merge branch 'main' into feat/telemetry-e2e-tests-clean
sarayev 88d9373
Merge branch 'main' into feat/telemetry-e2e-tests-clean
sarayev 47ef470
chore(telemetry): address reviewer NIT/MINOR feedback
sarayev File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| --- | ||
| "@aws-blocks/core": patch | ||
| "@aws-blocks/create-blocks-app": patch | ||
| --- | ||
|
|
||
| fix(telemetry): inherit worker stderr in debug mode; enable telemetry in CI for create-blocks-app | ||
|
|
||
| - When `NODE_DEBUG=blocks-telemetry` is set, the telemetry worker subprocess | ||
| inherits the parent's stderr so delivery confirmation is observable. Silent | ||
| by default. | ||
| - Remove CI telemetry suppression from create-blocks-app to match core behavior. | ||
| Telemetry is now enabled in CI (same as all other CLI commands). | ||
| - Make `console` cross-platform and headless-safe: pick the OS browser opener | ||
| (`open`/`xdg-open`/`start`), resolve the region from the environment, and treat | ||
| a missing opener (CI / remote shells) as best-effort success instead of failing. | ||
| - Add an isolated E2E telemetry test suite (`test-apps/telemetry`) that verifies | ||
| payload structure, delivery to the real endpoint, disable mechanisms, and | ||
| per-command success/failure events. |
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[major] Both destroys end in
|| true, so a failed teardown is swallowed. Stacks are unique per run (BLOCKS_STACK_SUFFIX: telemetry-<pr>-<attempt>+ a random sandbox id), so leaks are distinct and cumulative -> ongoing AWS cost. Fail the job (or emit a warning annotation) on cleanup error, and/or add a scheduled sweeper for stalebb-telemetry-e2e-*stacks (or tagblocks:purpose=telemetry-e2e).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The best-effort teardown here mirrors the existing convention: e2e-hosting's cleanup step uses the identical
if: always()+|| truepattern (line 475), and the other e2e jobs rely on in-script cleanup + the separate cleanup-stacks.yml sweeper. Stacks are uniquely named per run, so a genuinely stronger story (fail-on-cleanup-error and/or a scheduled sweeper for bb-telemetry-e2e-*) is a reasonable improvement — but to stay consistent with the repo's current approach I'd do that as a follow-up (ideally across all e2e jobs, not just this one). Tracking it.