Skip to content
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

PROF-9250: Enable timeline and CPU profiling by default #4149

Merged
merged 6 commits into from
Apr 8, 2024

Conversation

szegedi
Copy link
Contributor

@szegedi szegedi commented Mar 7, 2024

What does this PR do?

  • Makes both timeline and CPU profiling on by default.
  • Introduces a non-experimental env var for CPU profiling.
  • Explicitly disables timeline events gathering if CPU or Wall profiling are not enabled.

Motivation

Timeline and CPU profiling have now been extensively tested, and it is considered safe to have them on by default. While adjusting tests for the on-by-default behavior, I also noticed some edge cases in the behavior that I fixed, namely:

  • it was possible to gather timeline events without a wall/CPU profiler, but we don't want to support that

@szegedi szegedi requested a review from a team as a code owner March 7, 2024 17:11
Copy link

github-actions bot commented Mar 7, 2024

Overall package size

Self size: 6.26 MB
Deduped: 60.76 MB
No deduping: 61.04 MB

Dependency sizes

name version self size total size
@datadog/native-iast-taint-tracking 1.7.0 16.71 MB 16.72 MB
@datadog/native-appsec 7.1.1 14.39 MB 14.4 MB
@datadog/pprof 5.2.0 8.84 MB 9.21 MB
protobufjs 7.2.5 2.77 MB 6.56 MB
@datadog/native-iast-rewriter 2.3.0 2.15 MB 2.24 MB
@opentelemetry/core 1.14.0 872.87 kB 1.47 MB
@datadog/native-metrics 2.0.0 898.77 kB 1.3 MB
@opentelemetry/api 1.4.1 780.32 kB 780.32 kB
import-in-the-middle 1.7.3 67.62 kB 731.01 kB
msgpack-lite 0.1.26 201.16 kB 281.59 kB
opentracing 0.14.7 194.81 kB 194.81 kB
semver 7.5.4 93.4 kB 123.8 kB
pprof-format 2.1.0 111.69 kB 111.69 kB
@datadog/sketches-js 2.1.0 109.9 kB 109.9 kB
lodash.sortby 4.7.0 75.76 kB 75.76 kB
lru-cache 7.14.0 74.95 kB 74.95 kB
ipaddr.js 2.1.0 60.23 kB 60.23 kB
ignore 5.2.4 51.22 kB 51.22 kB
int64-buffer 0.1.10 49.18 kB 49.18 kB
shell-quote 1.8.1 44.96 kB 44.96 kB
istanbul-lib-coverage 3.2.0 29.34 kB 29.34 kB
tlhunter-sorted-set 0.1.0 24.94 kB 24.94 kB
limiter 1.1.5 23.17 kB 23.17 kB
dc-polyfill 0.1.4 23.1 kB 23.1 kB
retry 0.13.1 18.85 kB 18.85 kB
node-abort-controller 3.1.1 16.89 kB 16.89 kB
jest-docblock 29.7.0 8.99 kB 12.76 kB
crypto-randomuuid 1.0.0 11.18 kB 11.18 kB
path-to-regexp 0.1.7 6.78 kB 6.78 kB
koalas 1.0.2 6.47 kB 6.47 kB
methods 1.1.2 5.29 kB 5.29 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 Mar 7, 2024

Codecov Report

Attention: Patch coverage is 20.00000% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 85.31%. Comparing base (3094779) to head (80c63c5).

❗ Current head 80c63c5 differs from pull request most recent head 5e1275c. Consider uploading reports for the commit 5e1275c to get more accurate results

Files Patch % Lines
...ackages/dd-trace/src/profiling/profilers/events.js 0.00% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4149      +/-   ##
==========================================
+ Coverage   85.23%   85.31%   +0.07%     
==========================================
  Files         247      247              
  Lines       10961    10940      -21     
  Branches       33       33              
==========================================
- Hits         9343     9333      -10     
+ Misses       1618     1607      -11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@szegedi szegedi requested a review from nsavoire March 12, 2024 11:51
@nsavoire
Copy link
Collaborator

nsavoire commented Mar 12, 2024

Some profiler tests are still failing on Windows, probably because timeline is not available here.

@szegedi szegedi force-pushed the szegedi/enable-timeline-and-cpu branch 2 times, most recently from ac5e0ea to 80c63c5 Compare March 12, 2024 16:06
nsavoire
nsavoire previously approved these changes Mar 13, 2024
@szegedi szegedi force-pushed the szegedi/enable-timeline-and-cpu branch from 80c63c5 to 5e1275c Compare April 8, 2024 12:27
@szegedi szegedi requested a review from nsavoire April 8, 2024 12:29
@szegedi
Copy link
Contributor Author

szegedi commented Apr 8, 2024

Rebased to fix merge conflicts; 4 of the previous 10 commits were now empty 'cause they were incorporated into master earlier; we're left with 6.

@pr-commenter
Copy link

pr-commenter bot commented Apr 8, 2024

Benchmarks

Benchmark execution time: 2024-04-08 12:35:44

Comparing candidate commit 5e1275c in PR branch szegedi/enable-timeline-and-cpu with baseline commit 3094779 in branch master.

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

@szegedi szegedi merged commit 731ee34 into master Apr 8, 2024
107 of 109 checks passed
@szegedi szegedi deleted the szegedi/enable-timeline-and-cpu branch April 8, 2024 15:23
juan-fernandez pushed a commit that referenced this pull request Apr 22, 2024
* Add non-experimental DD_PROFILING_CPU_ENABLED

* Turn timelines and CPU profile on by default on non-Windows platforms
juan-fernandez pushed a commit that referenced this pull request Apr 22, 2024
* Add non-experimental DD_PROFILING_CPU_ENABLED

* Turn timelines and CPU profile on by default on non-Windows platforms
juan-fernandez pushed a commit that referenced this pull request Apr 22, 2024
* Add non-experimental DD_PROFILING_CPU_ENABLED

* Turn timelines and CPU profile on by default on non-Windows platforms
This was referenced Apr 22, 2024
juan-fernandez pushed a commit that referenced this pull request Apr 23, 2024
* Add non-experimental DD_PROFILING_CPU_ENABLED

* Turn timelines and CPU profile on by default on non-Windows platforms
juan-fernandez pushed a commit that referenced this pull request Apr 23, 2024
* Add non-experimental DD_PROFILING_CPU_ENABLED

* Turn timelines and CPU profile on by default on non-Windows platforms
juan-fernandez pushed a commit that referenced this pull request Apr 23, 2024
* Add non-experimental DD_PROFILING_CPU_ENABLED

* Turn timelines and CPU profile on by default on non-Windows platforms
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