Skip to content

build: run perfetto build and test on GHA - #64721

Open
legendecas wants to merge 4 commits into
nodejs:mainfrom
legendecas:ci-perfetto
Open

build: run perfetto build and test on GHA#64721
legendecas wants to merge 4 commits into
nodejs:mainfrom
legendecas:ci-perfetto

Conversation

@legendecas

@legendecas legendecas commented Jul 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

This ensures that the src/lib changes (like emitting trace events) are compatible with perfetto, and compatible with V8 compiled with perfetto.

Refs: #64565
Fixes: nodejs/diagnostics#654

@nodejs-github-bot

nodejs-github-bot commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/actions

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Jul 24, 2026
@legendecas legendecas added dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. dont-land-on-v24.x PRs that should not land on the v24.x-staging branch and should not be released in v24.x. labels Jul 24, 2026
avivkeller
avivkeller reviewed Jul 24, 2026
View reviewed changes
Comment thread .github/workflows/test-linux-perfetto.yml Outdated
Comment on lines +44 to +48
path: node
- name: Install Clang ${{ env.CLANG_VERSION }}
uses: ./node/.github/actions/install-clang
with:
clang-version: ${{ env.CLANG_VERSION }}

@avivkeller avivkeller Jul 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
path: node
- name: Install Clang ${{ env.CLANG_VERSION }}
uses: ./node/.github/actions/install-clang
with:
clang-version: ${{ env.CLANG_VERSION }}
- name: Install Clang ${{ env.CLANG_VERSION }}
uses: .github/actions/install-clang
with:
clang-version: ${{ env.CLANG_VERSION }}

nit: use the CWD

@legendecas legendecas Jul 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I replicated this from https://github.com/nodejs/node/blob/main/.github/workflows/test-linux-quic.yml. Maybe it was replicated from https://github.com/nodejs/node/blob/main/.github/workflows/test-linux.yml

Comment thread .github/workflows/test-linux-perfetto.yml Outdated
Comment thread .github/workflows/test-linux-perfetto.yml Outdated
Comment thread .github/workflows/test-linux-perfetto.yml Outdated
avivkeller
avivkeller approved these changes Jul 24, 2026
View reviewed changes

@avivkeller avivkeller left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, we should probably update the other workflows to use the CWD, IMO it's cleaner

@Renegade334

Renegade334 commented Jul 25, 2026

Copy link
Copy Markdown
Member

Could we enable this for the existing Linux workflow rather than adding an additional run?

@Renegade334
Renegade334 requested a review from aduh95 July 25, 2026 02:09
@legendecas

legendecas commented Jul 27, 2026

Copy link
Copy Markdown
Member Author

Could we enable this for the existing Linux workflow rather than adding an additional run?

We could. Still, I think in the near future, we might want to keep testing both legacy tracing and perfetto tracing, until we can fully deprecate the legacy one.

aduh95
aduh95 reviewed Jul 27, 2026
View reviewed changes
Comment thread .github/workflows/test-linux-perfetto.yml Outdated

@aduh95 aduh95 Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For other flags (except QUIC, but the QUIC job being restricted to a very limited set of files, that's less of a concern), we put this as part of the test-shared workflow. Can we do that here as well? Having a dedicated job is probably a waste and won't scale well.

@legendecas legendecas Jul 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, updated!

@codecov

codecov Bot commented Jul 27, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.17%. Comparing base (f8f8aa2) to head (38a1810).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
src/tracing/trace_event_helper.cc 92.30% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #64721      +/-   ##
==========================================
+ Coverage   90.14%   90.17%   +0.03%     
==========================================
  Files         746      746              
  Lines      242818   242764      -54     
  Branches    45765    45736      -29     
==========================================
+ Hits       218884   218911      +27     
+ Misses      15417    15345      -72     
+ Partials     8517     8508       -9
Files with missing lines Coverage Δ
src/debug_utils.h 80.00% <ø> (ø)
src/node_trace_events.cc 82.30% <ø> (ø)
src/tracing/node_trace_writer.cc 77.50% <100.00%> (-1.12%) ⬇️
src/tracing/trace_event_helper.h 100.00% <ø> (ø)
src/tracing/trace_event_helper.cc 95.65% <92.30%> (-4.35%) ⬇️

... and 57 files with indirect coverage changes

🚀 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.

@legendecas
legendecas force-pushed the ci-perfetto branch 4 times, most recently from 494f038 to dd064f0 Compare July 29, 2026 13:54
@legendecas legendecas added test Issues and PRs related to the tests. lib / src Issues and PRs related to general changes in the lib or src directory. labels Jul 29, 2026
@legendecas
src: fix perfetto build on GetTraceFilePath
7742a03 
Signed-off-by: Chengzhong Wu <cwu631@bloomberg.net>
@legendecas

legendecas commented Jul 29, 2026

Copy link
Copy Markdown
Member Author

Rebased on the latest main branch to fix conflicts and build failures.

@legendecas legendecas added the trace_events Issues and PRs related to V8, Node.js core, and userspace code trace events. label Jul 29, 2026
@legendecas
src,test: disable trace events tests when perfetto is enabled
d757cc9 
Signed-off-by: Chengzhong Wu <cwu631@bloomberg.net>
@legendecas
build: fix v8_use_perfetto source scraping
d099a60 
Signed-off-by: Chengzhong Wu <cwu631@bloomberg.net>
@legendecas
build: run perfetto build and test on GHA
38a1810 
Signed-off-by: Chengzhong Wu <cwu631@bloomberg.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aduh95 aduh95 aduh95 left review comments
@anonrig anonrig anonrig approved these changes
@avivkeller avivkeller avivkeller approved these changes

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. dont-land-on-v24.x PRs that should not land on the v24.x-staging branch and should not be released in v24.x. lib / src Issues and PRs related to general changes in the lib or src directory. meta Issues and PRs related to the general management of the project. test Issues and PRs related to the tests. trace_events Issues and PRs related to V8, Node.js core, and userspace code trace events.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Reopening Perfetto in Node discussion

6 participants

@legendecas @nodejs-github-bot @Renegade334 @anonrig @aduh95 @avivkeller