Skip to content

Constrain async ID collection to main thread only #5559

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

Merged
merged 2 commits into from
Apr 10, 2025

Conversation

szegedi
Copy link
Contributor

@szegedi szegedi commented Apr 10, 2025

What does this PR do?

Constrains async ID collection to main thread only.

Motivation

Seems like in worker threads it can cause crashes. Until we can reproduce and fix the crashes we'll limit the use of the feature.

Additional Notes

A typical crash has the stack trace:

_ZN2v87Isolate17GetCurrentContextEv
_ZN4node29AsyncHooksGetExecutionAsyncIdEPN2v87IsolateE
_ZN2dd14GetAsyncIdNoGCEPN2v87IsolateE
_ZN2dd12_GLOBAL__N_113SignalHandler20HandleProfilerSignalEiP9siginfo_tPv
_ZN2v88internal11HandleScope16DeleteExtensionsEPNS0_7IsolateE
_ZN4node11Environment9RunTimersEP10uv_timer_s
uv__run_timers
uv_run
_ZN4node21SpinEventLoopInternalEPNS_11EnvironmentE
_ZN4node6worker6Worker3RunEv
_ZZN4node6worker6Worker11StartThreadERKN2v820FunctionCallbackInfoINS2_5ValueEEEENUlPvE_4_FUNES8_
start_thread
clone

Jira: PROF-11628

Copy link

github-actions bot commented Apr 10, 2025

Overall package size

Self size: 9.28 MB
Deduped: 101.3 MB
No deduping: 101.82 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.5.0 | 29.83 MB | 29.83 MB | | @datadog/native-appsec | 8.5.1 | 19.26 MB | 19.27 MB | | @datadog/native-iast-taint-tracking | 3.3.0 | 13.77 MB | 13.78 MB | | @datadog/pprof | 5.7.1 | 9.51 MB | 9.88 MB | | @opentelemetry/core | 1.30.1 | 908.66 kB | 7.16 MB | | protobufjs | 7.4.0 | 2.77 MB | 5.42 MB | | @datadog/wasm-js-rewriter | 3.1.0 | 2.37 MB | 2.52 MB | | @datadog/native-metrics | 3.1.0 | 1.06 MB | 1.46 MB | | @opentelemetry/api | 1.8.0 | 1.21 MB | 1.21 MB | | import-in-the-middle | 1.13.1 | 117.64 kB | 839.26 kB | | source-map | 0.7.4 | 226 kB | 226 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | lru-cache | 7.18.3 | 133.92 kB | 133.92 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.1 | 109.9 kB | 109.9 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.2 | 53.63 kB | 53.63 kB | | istanbul-lib-coverage | 3.2.0 | 29.34 kB | 29.34 kB | | rfdc | 1.4.1 | 27.15 kB | 27.15 kB | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | dc-polyfill | 0.1.6 | 24.56 kB | 24.56 kB | | shell-quote | 1.8.2 | 23.54 kB | 23.54 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | semifies | 1.0.0 | 15.84 kB | 15.84 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | ttl-set | 1.0.0 | 4.61 kB | 9.69 kB | | path-to-regexp | 0.1.12 | 6.6 kB | 6.6 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | module-details-from-path | 1.0.3 | 4.47 kB | 4.47 kB |

🤖 This report was automatically generated by heaviest-objects-in-the-universe

Copy link

codecov bot commented Apr 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.23%. Comparing base (853152b) to head (5004ebc).
Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5559   +/-   ##
=======================================
  Coverage   79.23%   79.23%           
=======================================
  Files         512      512           
  Lines       23232    23232           
=======================================
  Hits        18409    18409           
  Misses       4823     4823           

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

@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Apr 10, 2025

Datadog Report

Branch report: szegedi/async-id-noworker
Commit report: c9b3829
Test service: dd-trace-js-integration-tests

✅ 0 Failed, 923 Passed, 0 Skipped, 12m 3.45s Total Time

@pr-commenter
Copy link

pr-commenter bot commented Apr 10, 2025

Benchmarks

Benchmark execution time: 2025-04-10 10:21:35

Comparing candidate commit 5004ebc in PR branch szegedi/async-id-noworker with baseline commit 853152b in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 863 metrics, 15 unstable metrics.

@szegedi szegedi enabled auto-merge (squash) April 10, 2025 10:56
@szegedi szegedi merged commit 5fd6690 into master Apr 10, 2025
427 checks passed
@szegedi szegedi deleted the szegedi/async-id-noworker branch April 10, 2025 14:27
This was referenced Apr 10, 2025
@github-actions github-actions bot mentioned this pull request Apr 10, 2025
szegedi added a commit that referenced this pull request Apr 30, 2025
BridgeAR pushed a commit that referenced this pull request Apr 30, 2025
* Revert "Constrain async ID collection to main thread only (#5559)"

This reverts commit 5fd6690.

* Revert "Start collecting async IDs in profiles (#5524)"

This reverts commit 9ec7ef9.

* Restore actually useful test code change
dd-trace-js bot pushed a commit that referenced this pull request May 1, 2025
* Revert "Constrain async ID collection to main thread only (#5559)"

This reverts commit 5fd6690.

* Revert "Start collecting async IDs in profiles (#5524)"

This reverts commit 9ec7ef9.

* Restore actually useful test code change
szegedi added a commit that referenced this pull request May 1, 2025
* Revert "Constrain async ID collection to main thread only (#5559)"

This reverts commit 5fd6690.

* Revert "Start collecting async IDs in profiles (#5524)"

This reverts commit 9ec7ef9.

* Restore actually useful test code change
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants