Skip to content

Replace symbolicator-cli with a profiler-edit node tool#5965

Merged
mstange merged 1 commit intofirefox-devtools:mainfrom
mstange:push-kqwkpkspuxxt
Apr 24, 2026
Merged

Replace symbolicator-cli with a profiler-edit node tool#5965
mstange merged 1 commit intofirefox-devtools:mainfrom
mstange:push-kqwkpkspuxxt

Conversation

@mstange
Copy link
Copy Markdown
Contributor

@mstange mstange commented Apr 23, 2026

Before:

  node dist/symbolicator-cli.js \
    --input samply-profile.json \
    --output profile-symbolicated.json \
    --server http://localhost:3000

After:

  node node-tools-dist/profiler-edit.js \
    -i samply-profile.json \
    -o profile-symbolicated.json \
    --symbolicate-with-server http://localhost:3000

The new tool also accepts a URL or profile storage hash as input, and can write gzip-compressed output:

  node node-tools-dist/profiler-edit.js \
    --from-hash w1spyw917hg... \
    -o out.json.gz \
    --symbolicate-with-server http://localhost:8001/abcdef/

The reason for this change is that I want to add further ways to modify profiles in the future (such as an --insert-label-frames argument) and it doesn't really make sense to have a separate tool for each modification.

This commit also moves the script file to /src/node-tools/ and the output to /node-tools-dist/. I'm planning to add more scripts of this type in the future.

@mstange mstange requested review from canova and fatadel as code owners April 23, 2026 20:05
@mstange mstange force-pushed the push-kqwkpkspuxxt branch from 4f6398f to 8132e6c Compare April 23, 2026 20:07
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 23, 2026

Codecov Report

❌ Patch coverage is 84.04255% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.33%. Comparing base (fde75e3) to head (a802dc1).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/node-tools/profiler-edit.ts 84.04% 15 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5965      +/-   ##
==========================================
+ Coverage   85.32%   85.33%   +0.01%     
==========================================
  Files         323      323              
  Lines       32169    32214      +45     
  Branches     8849     8871      +22     
==========================================
+ Hits        27448    27490      +42     
- Misses       4290     4293       +3     
  Partials      431      431              

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

@fatadel
Copy link
Copy Markdown
Contributor

fatadel commented Apr 24, 2026

What a branch name 😃

Copy link
Copy Markdown
Contributor

@fatadel fatadel left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I've added some minor comments but LGTM overall 👍🏻
Also, additional integration tests could improve the test coverage. Namely, the two new capabilities are not exercised end-to-end:

  • Gzip output: no test writes to out.json.gz. A quick integration test that outputs .gz, reads the bytes, checks the gzip magic (or round-trips through decompress) and compares to the uncompressed snapshot would be valuable.
  • URL / HASH loading: only the argv parsing is tested; the actual fetch paths are not. A window.fetchMock test paralleling the existing symbolication test would close that gap cheaply.

Comment thread src/node-tools/profiler-edit.ts Outdated
Comment thread src/node-tools/profiler-edit.ts Outdated
Comment thread src/node-tools/profiler-edit.ts
Comment thread src/node-tools/profiler-edit.ts Outdated
Comment thread src/node-tools/profiler-edit.ts Outdated
@mstange mstange force-pushed the push-kqwkpkspuxxt branch 2 times, most recently from 3ff7557 to 2092294 Compare April 24, 2026 18:42
Before:

```
  node dist/symbolicator-cli.js \
    --input samply-profile.json \
    --output profile-symbolicated.json \
    --server http://localhost:3000
```

After:

```
  node node-tools-dist/profiler-edit.js \
    -i samply-profile.json \
    -o profile-symbolicated.json \
    --symbolicate-with-server http://localhost:3000
```

The new tool also accepts a URL or profile storage hash as input, and can
write gzip-compressed output:

```
  node node-tools-dist/profiler-edit.js \
    --from-hash w1spyw917hg... \
    -o out.json.gz \
    --symbolicate-with-server http://localhost:8001/abcdef/
```

The reason for this change is that I want to add further ways to
modify profiles in the future (such as an `--insert-label-frames`
argument) and it doesn't really make sense to have a separate tool
for each modification.

This commit also moves the script file to /src/node-tools/ and the output to /node-tools-dist/.
I'm planning to add more scripts of this type in the future.
@mstange mstange force-pushed the push-kqwkpkspuxxt branch from 2092294 to a802dc1 Compare April 24, 2026 18:42
@mstange
Copy link
Copy Markdown
Contributor Author

mstange commented Apr 24, 2026

Thanks for the review! I've addressed all the comments and added the missing tests.

@mstange mstange enabled auto-merge April 24, 2026 18:43
@mstange mstange merged commit 8c0afb1 into firefox-devtools:main Apr 24, 2026
19 checks passed
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.

2 participants