Add config tests for OOM heap profiler (#2943)

* Use isTrue in profiling config

* Add OOM heap profiler config tests

* Deduplicate OOM export strategies

Author: nsavoire

packages/dd-trace/src/profiling/config.js

@@ -12,31 +12,32 @@ const WallProfiler = require('./profilers/wall')
 const SpaceProfiler = require('./profilers/space')
 const { oomExportStrategies, snapshotKinds } = require('./constants')
 const { tagger } = require('./tagger')
-
-const {
-  DD_PROFILING_ENABLED,
-  DD_PROFILING_PROFILERS,
-  DD_PROFILING_ENDPOINT_COLLECTION_ENABLED,
-  DD_ENV,
-  DD_TAGS,
-  DD_SERVICE,
-  DD_VERSION,
-  DD_TRACE_AGENT_URL,
-  DD_AGENT_HOST,
-  DD_TRACE_AGENT_PORT,
-  DD_PROFILING_UPLOAD_TIMEOUT,
-  DD_PROFILING_SOURCE_MAP,
-  DD_PROFILING_UPLOAD_PERIOD,
-  DD_PROFILING_PPROF_PREFIX,
-  DD_PROFILING_EXPERIMENTAL_OOM_MONITORING_ENABLED,
-  DD_PROFILING_EXPERIMENTAL_OOM_HEAP_LIMIT_EXTENSION_SIZE,
-  DD_PROFILING_EXPERIMENTAL_OOM_MAX_HEAP_EXTENSION_COUNT,
-  DD_PROFILING_EXPERIMENTAL_OOM_EXPORT_STRATEGIES
-} = process.env
+const { isTrue } = require('../util')
 
 class Config {
   constructor (options = {}) {
-    const enabled = coalesce(options.enabled, DD_PROFILING_ENABLED, true)
+    const {
+      DD_PROFILING_ENABLED,
+      DD_PROFILING_PROFILERS,
+      DD_PROFILING_ENDPOINT_COLLECTION_ENABLED,
+      DD_ENV,
+      DD_TAGS,
+      DD_SERVICE,
+      DD_VERSION,
+      DD_TRACE_AGENT_URL,
+      DD_AGENT_HOST,
+      DD_TRACE_AGENT_PORT,
+      DD_PROFILING_UPLOAD_TIMEOUT,
+      DD_PROFILING_SOURCE_MAP,
+      DD_PROFILING_UPLOAD_PERIOD,
+      DD_PROFILING_PPROF_PREFIX,
+      DD_PROFILING_EXPERIMENTAL_OOM_MONITORING_ENABLED,
+      DD_PROFILING_EXPERIMENTAL_OOM_HEAP_LIMIT_EXTENSION_SIZE,
+      DD_PROFILING_EXPERIMENTAL_OOM_MAX_HEAP_EXTENSION_COUNT,
+      DD_PROFILING_EXPERIMENTAL_OOM_EXPORT_STRATEGIES
+    } = process.env
+
+    const enabled = isTrue(coalesce(options.enabled, DD_PROFILING_ENABLED, true))
     const env = coalesce(options.env, DD_ENV)
     const service = options.service || DD_SERVICE || 'node'
     const host = os.hostname()
@@ -53,7 +54,7 @@ class Config {
     const pprofPrefix = coalesce(options.pprofPrefix,
       DD_PROFILING_PPROF_PREFIX)
 
-    this.enabled = String(enabled) !== 'false'
+    this.enabled = enabled
     this.service = service
     this.env = env
     this.host = host
@@ -84,8 +85,8 @@ class Config {
       new AgentExporter(this)
     ], this)
 
-    const oomMonitoringEnabled = coalesce(options.oomMonitoring,
-      DD_PROFILING_EXPERIMENTAL_OOM_MONITORING_ENABLED, false)
+    const oomMonitoringEnabled = isTrue(coalesce(options.oomMonitoring,
+      DD_PROFILING_EXPERIMENTAL_OOM_MONITORING_ENABLED, false))
     const heapLimitExtensionSize = coalesce(options.oomHeapLimitExtensionSize,
       Number(DD_PROFILING_EXPERIMENTAL_OOM_HEAP_LIMIT_EXTENSION_SIZE), 0)
     const maxHeapExtensionCount = coalesce(options.oomMaxHeapExtensionCount,
@@ -136,7 +137,7 @@ function ensureOOMExportStrategies (strategies, options) {
     }
   }
 
-  return strategies
+  return [ ...new Set(strategies) ]
 }
 
 function getExporter (name, options) {

packages/dd-trace/test/profiling/config.spec.js

@@ -4,6 +4,7 @@ require('../setup/tap')
 
 const { expect } = require('chai')
 const os = require('os')
+const path = require('path')
 const { AgentExporter } = require('../../src/profiling/exporters/agent')
 const { FileExporter } = require('../../src/profiling/exporters/file')
 const CpuProfiler = require('../../src/profiling/profilers/cpu')
@@ -13,9 +14,16 @@ const { ConsoleLogger } = require('../../src/profiling/loggers/console')
 
 describe('config', () => {
   let Config
+  let env
 
   beforeEach(() => {
     Config = require('../../src/profiling/config').Config
+    env = process.env
+    process.env = {}
+  })
+
+  afterEach(() => {
+    process.env = env
   })
 
   it('should have the correct defaults', () => {
@@ -136,4 +144,55 @@ describe('config', () => {
 
     expect(exporterUrl).to.equal(expectedUrl)
   })
+
+  it('should disable OOM heap profiler by default', () => {
+    const config = new Config()
+    expect(config.oomMonitoring).to.deep.equal({
+      enabled: false,
+      heapLimitExtensionSize: 0,
+      maxHeapExtensionCount: 0,
+      exportStrategies: [],
+      exportCommand: undefined
+    })
+  })
+
+  it('should support OOM heap profiler configuration', () => {
+    process.env = {
+      DD_PROFILING_EXPERIMENTAL_OOM_MONITORING_ENABLED: 'false'
+    }
+    const config = new Config({})
+
+    expect(config.oomMonitoring).to.deep.equal({
+      enabled: false,
+      heapLimitExtensionSize: 0,
+      maxHeapExtensionCount: 0,
+      exportStrategies: [],
+      exportCommand: undefined
+    })
+  })
+
+  it('should support OOM heap profiler configuration', () => {
+    process.env = {
+      DD_PROFILING_EXPERIMENTAL_OOM_MONITORING_ENABLED: '1',
+      DD_PROFILING_EXPERIMENTAL_OOM_HEAP_LIMIT_EXTENSION_SIZE: '1000000',
+      DD_PROFILING_EXPERIMENTAL_OOM_MAX_HEAP_EXTENSION_COUNT: '2',
+      DD_PROFILING_EXPERIMENTAL_OOM_EXPORT_STRATEGIES: 'process,interrupt,async,interrupt'
+    }
+
+    const config = new Config({})
+
+    expect(config.oomMonitoring).to.deep.equal({
+      enabled: true,
+      heapLimitExtensionSize: 1000000,
+      maxHeapExtensionCount: 2,
+      exportStrategies: ['process', 'interrupt', 'async'],
+      exportCommand: [
+        process.execPath,
+        path.normalize(path.join(__dirname, '../../src/profiling', 'exporter_cli.js')),
+        'http://localhost:8126/',
+        `host:${config.host},service:node,snapshot:on_oom`,
+        'space'
+      ]
+    })
+  })
 })