Profiler tracking PR (DataDog#1422)

* update profiler to default to pprof module (DataDog#1291) * add sirun benchmark for the profiler (DataDog#1318) * add profiler and profiling to keywords in package.json * Make profiler use normal enable option (DataDog#1437) * Remove DD_PROFILING_INTERVAL env var (DataDog#1454) * Do prebuilds on profiler branch * Fix profiler url config (DataDog#1460) * Prevent agent 502 errors (DataDog#1461) If a request to the agent starts within 55 seconds of the last ending the agent will reject with a 502 error. This bumps the reporting time up slightly and ensures the response is discarded to avoid that. * Expand profiler logging (DataDog#1511) * Make pprof required to satisfy check_licenses (DataDog#1513) It's probably better for it to be required while using a separate branch anyway. * Improve profiler logging and behaviour when profilers fail to start (DataDog#1515) * Prevent next failures on profiler branch (DataDog#1523) * Use forked pprof module with prebuilds (DataDog#1529) * Add additional profiler tests (DataDog#1530) This fixes what should be a rejection on the exporter failure test and adds a test for skipping submit when no profiles received. * Fix some missing code coverage (DataDog#1531) * Reduce delta of code changes from profiler branch to master (DataDog#1537) * Minor cleanup * Ensure fresh start time is used for each profiler capture (DataDog#1543) Co-authored-by: rochdev <[email protected]>
atif-saddique-deel · Aug 6, 2021 · e745f2a · e745f2a
1 parent cc3952d
commit e745f2a
Show file tree

Hide file tree

Showing 36 changed files with 620 additions and 5,180 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -104,7 +104,6 @@ node-bench-sirun-base: &node-bench-sirun-base
         paths:
           - benchmark/sirun/*-sirun-output.ndjson
 
-
 node-integration-base: &node-integration-base
   resource_class: small
   working_directory: ~/dd-trace-js
@@ -270,6 +269,7 @@ prebuild-job: &prebuild-job
     branches:
       only:
         - master
+        - profiler
         - /v\d+\.\d+/
         - /v\d+\.x/
         - /.*native.*/
@@ -396,6 +396,13 @@ jobs:
         environment:
           - SIRUN_TEST_DIR=plugin-net
 
+  node-bench-sirun-profiler-latest:
+    <<: *node-bench-sirun-base
+    docker:
+      - image: node
+        environment:
+          - SIRUN_TEST_DIR=profiler
+
   node-bench-sirun-all:
     docker:
       - image: node
@@ -1403,6 +1410,7 @@ workflows:
       - node-bench-sirun-plugin-net-latest
       - node-bench-sirun-scope-latest
       - node-bench-sirun-exporting-pipeline-latest
+      - node-bench-sirun-profiler-latest
       - node-bench-sirun-all:
           requires:
             - node-bench-sirun-startup-latest
@@ -1416,6 +1424,7 @@ workflows:
             - node-bench-sirun-plugin-net-latest
             - node-bench-sirun-scope-latest
             - node-bench-sirun-exporting-pipeline-latest
+            - node-bench-sirun-profiler-latest
   upstream:
     jobs:
       # - node-upstream-node
@@ -1521,6 +1530,7 @@ workflows:
             branches:
               only:
                 - master
+                - profiler
     jobs:
       - lint
       - node-core-12
@@ -1591,6 +1601,7 @@ workflows:
             branches:
               only:
                 - master
+                - profiler
     jobs:
       - node-bench-latest
       - node-bench-profiler-latest
@@ -1606,6 +1617,7 @@ workflows:
       - node-bench-sirun-plugin-net-latest
       - node-bench-sirun-scope-latest
       - node-bench-sirun-exporting-pipeline-latest
+      - node-bench-sirun-profiler-latest
       - node-bench-sirun-all:
           requires:
             - node-bench-sirun-startup-latest
@@ -1619,6 +1631,7 @@ workflows:
             - node-bench-sirun-plugin-net-latest
             - node-bench-sirun-scope-latest
             - node-bench-sirun-exporting-pipeline-latest
+            - node-bench-sirun-profiler-latest
   nightly-upstream:
     triggers:
       - schedule:
@@ -1627,6 +1640,7 @@ workflows:
             branches:
               only:
                 - master
+                - profiler
     jobs:
       # - node-upstream-node
       - node-upstream-amqp10

diff --git a/.eslintignore b/.eslintignore
@@ -3,7 +3,6 @@ dist
 docs
 out
 node_modules
-protobuf
 versions
 acmeair-nodejs
 vendor
diff --git a/.npmignore b/.npmignore
@@ -4,7 +4,6 @@
 !packages/*/src/**/*
 !packages/*/index.js
 !prebuilds/**/*
-!protobuf/**/*
 !scripts/**/*
 !vendor/**/*
 !CONTRIBUTING.md

diff --git a/LICENSE-3rdparty.csv b/LICENSE-3rdparty.csv
@@ -1,4 +1,5 @@
 Component,Origin,License,Copyright
+require,@datadog/pprof,Apache license 2.0,Copyright 2019 Google Inc.
 require,@datadog/sketches-js,Apache license 2.0,Copyright 2020 Datadog Inc.
 require,@types/node,MIT,Copyright Authors
 require,form-data,MIT,Copyright 2012 Felix Geisendörfer and contributors
@@ -16,7 +17,6 @@ require,node-gyp-build,MIT,Copyright 2017 Mathias Buus
 require,opentracing,MIT,Copyright 2016 Resonance Labs Inc
 require,path-to-regexp,MIT,Copyright 2014 Blake Embrey
 require,performance-now,MIT,Copyright 2013 Braveg1rl
-require,protobufjs,BSD-3-Clause,Copyright 2016 Daniel Wirtz
 require,semver,ISC,Copyright Isaac Z. Schlueter and Contributors
 require,shimmer,BSD-2-Clause,Copyright Forrest L Norvell
 require,source-map,BSD-3-Clause,Copyright 2009-2011 Mozilla Foundation and contributors

diff --git a/benchmark/sirun/profiler/index.js b/benchmark/sirun/profiler/index.js
@@ -0,0 +1,33 @@
+'use strict'
+
+const {
+  profiler,
+  CpuProfiler,
+  HeapProfiler
+} = require('../../../packages/dd-trace/src/profiling')
+
+const { PROFILER } = process.env
+
+const profilers = []
+
+if (PROFILER === 'cpu' || PROFILER === 'all') {
+  profilers.push(new CpuProfiler())
+}
+if (PROFILER === 'heap' || PROFILER === 'all') {
+  profilers.push(new HeapProfiler())
+}
+
+const exporters = [{
+  export () {
+    profiler.stop()
+    return Promise.resolve()
+  }
+}]
+
+profiler.start({
+  profilers,
+  exporters,
+  interval: 0
+})
+
+profiler._timer.ref()
diff --git a/benchmark/sirun/profiler/meta.json b/benchmark/sirun/profiler/meta.json
@@ -0,0 +1,28 @@
+{
+  "name": "profiler",
+  "run": "node -r ../monitor index.js",
+  "cachegrind": true,
+  "iterations": 10,
+  "variants": {
+    "control": {
+      "env": {
+        "PROFILER": ""
+      }
+    },
+    "with-cpu-profiler": {
+      "env": {
+        "PROFILER": "cpu"
+      }
+    },
+    "with-heap-profiler": {
+      "env": {
+        "PROFILER": "heap"
+      }
+    },
+    "with-all-profilers": {
+      "env": {
+        "PROFILER": "all"
+      }
+    }
+  }
+}
diff --git a/package.json b/package.json
@@ -10,7 +10,6 @@
     "rebuild": "node-gyp rebuild --jobs=max",
     "prebuild": "node scripts/prebuild.js",
     "prebuilds": "node scripts/prebuilds.js",
-    "protobuf": "pbjs -t static-module -w commonjs -o protobuf/profile.js protobuf/profile.proto && pbts -o protobuf/profile.d.ts protobuf/profile.js",
     "prepublishOnly": "node scripts/prepublish.js",
     "postpublish": "node scripts/postpublish.js",
     "bench": "node benchmark",
@@ -27,6 +26,7 @@
     "test:plugins": "mocha --exit --file \"packages/dd-trace/test/setup/core.js\" \"packages/datadog-plugin-@($(echo $PLUGINS))/test/**/*.spec.js\"",
     "test:plugins:ci": "yarn services && nyc --include \"packages/datadog-plugin-@($(echo $PLUGINS))/src/**/*.js\" -- npm run test:plugins -- --reporter mocha-multi-reporters --reporter-options configFile=mocha-reporter-config.json",
     "test:plugins:upstream": "node ./packages/dd-trace/test/plugins/suite.js",
+    "test:profiler": "mocha --exit --file \"packages/dd-trace/test/setup/core.js\" \"packages/dd-trace/test/profiling/**/*.spec.js\"",
     "test:integration": "mocha --timeout 30000 \"integration-tests/**/*.spec.js\"",
     "leak:core": "node ./scripts/install_plugin_modules && (cd packages/memwatch && yarn) && NODE_PATH=./packages/memwatch/node_modules node --no-warnings ./node_modules/.bin/tape 'packages/dd-trace/test/leak/**/*.js'",
     "leak:plugins": "yarn services && (cd packages/memwatch && yarn) && NODE_PATH=./packages/memwatch/node_modules node --no-warnings ./node_modules/.bin/tape \"packages/datadog-plugin-@($(echo $PLUGINS))/test/leak.js\"",
@@ -43,6 +43,9 @@
     "datadog",
     "trace",
     "tracing",
+    "profile",
+    "profiler",
+    "profiling",
     "opentracing",
     "apm"
   ],
@@ -56,6 +59,7 @@
     "node": ">=12"
   },
   "dependencies": {
+    "@datadog/pprof": "^0.1.3",
     "@datadog/sketches-js": "^1.0.4",
     "@types/node": "^10.12.18",
     "form-data": "^3.0.0",
@@ -73,7 +77,6 @@
     "opentracing": ">=0.12.1",
     "path-to-regexp": "^0.1.2",
     "performance-now": "^2.1.0",
-    "protobufjs": "^6.9.0",
     "semver": "^5.5.0",
     "shimmer": "1.2.1",
     "source-map": "^0.7.3",
@@ -112,7 +115,7 @@
     "proxyquire": "^1.8.0",
     "retry": "^0.10.1",
     "rimraf": "^3.0.0",
-    "sinon": "^8.0.4",
+    "sinon": "^11.1.2",
     "sinon-chai": "^3.4.0",
     "tape": "^4.9.1",
     "tar": "^4.4.8",

diff --git a/packages/datadog-plugin-next/test/index.spec.js b/packages/datadog-plugin-next/test/index.spec.js
@@ -38,6 +38,7 @@ describe('Plugin', function () {
           execSync('node build', {
             cwd: __dirname,
             env: {
+              ...process.env,
               version,
               // needed for webpack 5
               NODE_PATH: [

diff --git a/packages/dd-trace/src/config.js b/packages/dd-trace/src/config.js
@@ -26,9 +26,9 @@ class Config {
 
     // Temporary disabled
     const DD_PROFILING_ENABLED = coalesce(
-      // options.profiling,
-      // process.env.DD_PROFILING_ENABLED,
+      options.profiling,
       process.env.DD_EXPERIMENTAL_PROFILING_ENABLED,
+      process.env.DD_PROFILING_ENABLED,
       false
     )
     const DD_PROFILING_EXPORTERS = coalesce(

diff --git a/packages/dd-trace/src/profiling/config.js b/packages/dd-trace/src/profiling/config.js
@@ -5,9 +5,9 @@ const os = require('os')
 const { URL } = require('url')
 const { AgentExporter } = require('./exporters/agent')
 const { FileExporter } = require('./exporters/file')
-const { InspectorCpuProfiler } = require('./profilers/inspector/cpu')
-const { InspectorHeapProfiler } = require('./profilers/inspector/heap')
 const { ConsoleLogger } = require('./loggers/console')
+const CpuProfiler = require('./profilers/cpu')
+const HeapProfiler = require('./profilers/heap')
 const { tagger } = require('./tagger')
 
 const {
@@ -28,7 +28,8 @@ class Config {
     const service = options.service || DD_SERVICE || 'node'
     const host = os.hostname()
     const version = coalesce(options.version, DD_VERSION)
-    const flushInterval = 60 * 1000
+    // Must be longer than one minute so pad with five seconds
+    const flushInterval = coalesce(options.interval, 65 * 1000)
 
     this.enabled = String(enabled) !== 'false'
     this.service = service
@@ -46,16 +47,16 @@ class Config {
 
     const hostname = coalesce(options.hostname, DD_AGENT_HOST, 'localhost')
     const port = coalesce(options.port, DD_TRACE_AGENT_PORT, 8126)
-    const url = new URL(coalesce(options.url, DD_TRACE_AGENT_URL,
+    this.url = new URL(coalesce(options.url, DD_TRACE_AGENT_URL,
       `http://${hostname || 'localhost'}:${port || 8126}`))
 
     this.exporters = ensureExporters(options.exporters || [
-      new AgentExporter({ url })
-    ], options)
+      new AgentExporter(this)
+    ], this)
 
     this.profilers = options.profilers || [
-      new InspectorCpuProfiler(),
-      new InspectorHeapProfiler()
+      new CpuProfiler(),
+      new HeapProfiler()
     ]
   }
 }

diff --git a/packages/dd-trace/src/profiling/encoders/pprof.js b/packages/dd-trace/src/profiling/encoders/pprof.js