PROF-9791: Use heuristics to start the profiler when requested through SSI #4322

szegedi · 2024-05-17T10:35:57Z

What does this PR do?

With this change, when DD_INJECTION_ENABLED has profiler among its comma-separated values then profiler will be started as soon as the application both created a tracing span and has reached a 30 second lifetime. This will ensure SSI profiling enablement doesn't profile either tracing-unaware applications or those that run for only a short period of time.

Motivation

Adding profiler integration to SSI.

github-actions · 2024-05-17T10:36:35Z

Overall package size

Self size: 6.54 MB
Deduped: 61.8 MB
No deduping: 62.08 MB

Dependency sizes

name	version	self size	total size
@datadog/native-appsec	8.0.1	15.59 MB	15.6 MB
@datadog/native-iast-taint-tracking	2.1.0	14.91 MB	14.92 MB
@datadog/pprof	5.3.0	9.85 MB	10.22 MB
protobufjs	7.2.5	2.77 MB	6.56 MB
@datadog/native-iast-rewriter	2.3.1	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.8.0	1.21 MB	1.21 MB
import-in-the-middle	1.7.4	70.19 kB	739.86 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
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
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
module-details-from-path	1.0.3	4.47 kB	4.47 kB

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

codecov · 2024-05-17T10:38:25Z

Codecov Report

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

Project coverage is 62.86%. Comparing base (8bedb23) to head (96b13bb).
Report is 5 commits behind head on master.

❗ Current head 96b13bb differs from pull request most recent head 9ca48ce

Please upload reports for the commit 9ca48ce to get more accurate results.

Files	Patch %	Lines
packages/dd-trace/src/profiling/ssi-heuristics.js	36.36%	14 Missing ⚠️
packages/dd-trace/src/proxy.js	52.94%	8 Missing ⚠️
packages/dd-trace/src/config.js	90.00%	1 Missing ⚠️
packages/dd-trace/src/profiler.js	0.00%	1 Missing ⚠️
packages/dd-trace/src/profiling/config.js	0.00%	1 Missing ⚠️
packages/dd-trace/src/profiling/profiler.js	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #4322       +/-   ##
===========================================
+ Coverage   18.61%   62.86%   +44.24%     
===========================================
  Files           6      243      +237     
  Lines         736    10308     +9572     
  Branches       33       33               
===========================================
+ Hits          137     6480     +6343     
- Misses        599     3828     +3229

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

pr-commenter · 2024-05-22T10:07:20Z

Benchmarks

Benchmark execution time: 2024-05-29 12:42:08

Comparing candidate commit 9ca48ce in PR branch szegedi/profiler-heuristics with baseline commit ad081db in branch master.

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

Also make sure mock profiler event emitter runs until heuristics trigger.

nsavoire · 2024-05-29T08:17:08Z

packages/dd-trace/src/config.js

@@ -663,6 +667,17 @@ class Config {
    this._setBoolean(env, 'profiling.enabled', coalesce(DD_EXPERIMENTAL_PROFILING_ENABLED, DD_PROFILING_ENABLED))
    this._setString(env, 'profiling.exporters', DD_PROFILING_EXPORTERS)
    this._setBoolean(env, 'profiling.sourceMap', DD_PROFILING_SOURCE_MAP && !isFalse(DD_PROFILING_SOURCE_MAP))
+    if (DD_INJECTION_ENABLED) {


Is it ok to put logic here? this seems to stray from the simple this._setBoolean/Value sequence of statement.

I think yes. There's some other if statements in the method already.

nsavoire · 2024-05-29T08:19:37Z

packages/dd-trace/src/config.js

+      if (DD_INJECTION_ENABLED.split(',').includes('profiler')) {
+        this._setBoolean(env, 'profiling.heuristicsEnabled', true)
+      }
+      if (DD_INTERNAL_PROFILING_LONG_LIVED_THRESHOLD) {


Having two different naming for the same thing (long lived / short lived threshold) feels weird.

I know – it already was there as "short lived" for telemetry, but we usually mentioned a "long-lived" process so this felt more natural. I was vacillating on renaming the internal variable but then thought I might do it in a separate step later to keep this smaller. I might add a commit to fix this though.

nsavoire · 2024-05-29T09:01:33Z

packages/dd-trace/src/profiling/ssi-heuristics.js

@@ -13,17 +11,16 @@ const EnablementChoice = {
  MANUALLY_ENABLED: Symbol('SSITelemetry.EnablementChoice.MANUALLY_ENABLED'),
  SSI_ENABLED: Symbol('SSITelemetry.EnablementChoice.SSI_ENABLED'),
  SSI_NOT_ENABLED: Symbol('SSITelemetry.EnablementChoice.SSI_NOT_ENABLED'),
-  DISABLED: Symbol('SSITelemetry.EnablementChoice.MANUALLY_DISABLED')
+  DISABLED: Symbol('SSITelemetry.EnablementChoice.DISABLED')


There is a MANUALLY_DISABLED reference left in the file

…h SSI (#4322)

szegedi added profiling semver-minor labels May 17, 2024

szegedi force-pushed the szegedi/profiler-heuristics branch from dd67713 to 1bdd8b6 Compare May 22, 2024 09:59

szegedi force-pushed the szegedi/profiler-heuristics branch 3 times, most recently from 62af38e to 96b13bb Compare May 23, 2024 10:44

szegedi mentioned this pull request May 23, 2024

PROF-9791: Allow None as default for profiling_enabled DataDog/system-tests#2479

Merged

8 tasks

szegedi marked this pull request as ready for review May 24, 2024 14:32

szegedi requested a review from a team as a code owner May 24, 2024 14:32

szegedi force-pushed the szegedi/profiler-heuristics branch from 96b13bb to cddcbd6 Compare May 27, 2024 12:27

szegedi added 8 commits May 28, 2024 15:11

Simplify mock telemetry emitter, make sure it runs with unref'd timer

5126bfc

Use an unref'd timer for short livedness determination

5f018fe

Derive config options from environment, later only rely on them

6596c65

Use config instead of env for SSI telemetry

b61a677

Add onHeuristicsTriggered callback functionality to SSITelemetry

a569529

Extract profiler start code, soon we'll be using it twice

e3b5af9

Logic for heuristic-triggered profiler start.

da6b782

Also make sure mock profiler event emitter runs until heuristics trigger.

Rename "SSITelemetry" to "SSIHeuristics"

6339755

szegedi force-pushed the szegedi/profiler-heuristics branch from cddcbd6 to 33003e2 Compare May 28, 2024 13:12

Add integration tests for SSI profiler activation

e76c79e

szegedi force-pushed the szegedi/profiler-heuristics branch from 33003e2 to e76c79e Compare May 28, 2024 14:10

szegedi requested review from Qard and nsavoire May 28, 2024 14:30

nsavoire reviewed May 29, 2024

View reviewed changes

szegedi added 2 commits May 29, 2024 14:19

Fix a leftover MANUALLY_DISABLED reference

4fcbf02

Change "short lived threshold" to "long lived threshold" everywhere

9ca48ce

szegedi requested a review from nsavoire May 29, 2024 12:34

nsavoire approved these changes May 29, 2024

View reviewed changes

szegedi enabled auto-merge (squash) May 30, 2024 12:32

szegedi merged commit 2f90a64 into master May 30, 2024
115 checks passed

szegedi deleted the szegedi/profiler-heuristics branch May 30, 2024 12:33

juan-fernandez pushed a commit that referenced this pull request Jun 4, 2024

PROF-9791: Use heuristics to start the profiler when requested throug…

5f1398c

…h SSI (#4322)

juan-fernandez pushed a commit that referenced this pull request Jun 4, 2024

PROF-9791: Use heuristics to start the profiler when requested throug…

8eda7c8

…h SSI (#4322)

juan-fernandez pushed a commit that referenced this pull request Jun 5, 2024

PROF-9791: Use heuristics to start the profiler when requested throug…

c86bd14

…h SSI (#4322)

juan-fernandez pushed a commit that referenced this pull request Jun 5, 2024

PROF-9791: Use heuristics to start the profiler when requested throug…

ebaa6db

…h SSI (#4322)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PROF-9791: Use heuristics to start the profiler when requested through SSI #4322

PROF-9791: Use heuristics to start the profiler when requested through SSI #4322

szegedi commented May 17, 2024 •

edited

github-actions bot commented May 17, 2024 •

edited

codecov bot commented May 17, 2024 •

edited

pr-commenter bot commented May 22, 2024 •

edited

nsavoire May 29, 2024

szegedi May 29, 2024

nsavoire May 29, 2024

szegedi May 29, 2024

nsavoire May 29, 2024

PROF-9791: Use heuristics to start the profiler when requested through SSI #4322

PROF-9791: Use heuristics to start the profiler when requested through SSI #4322

Conversation

szegedi commented May 17, 2024 • edited

What does this PR do?

Motivation

github-actions bot commented May 17, 2024 • edited

Overall package size

Dependency sizes

codecov bot commented May 17, 2024 • edited

Codecov Report

pr-commenter bot commented May 22, 2024 • edited

Benchmarks

nsavoire May 29, 2024

Choose a reason for hiding this comment

szegedi May 29, 2024

Choose a reason for hiding this comment

nsavoire May 29, 2024

Choose a reason for hiding this comment

szegedi May 29, 2024

Choose a reason for hiding this comment

nsavoire May 29, 2024

Choose a reason for hiding this comment

szegedi commented May 17, 2024 •

edited

github-actions bot commented May 17, 2024 •

edited

codecov bot commented May 17, 2024 •

edited

pr-commenter bot commented May 22, 2024 •

edited