enable log collection & log calls review (#4932)

* Remove template literals * Enable log collection by default * Add a message to some log.error(err) * remove template lit * Add messages to request errors * Fix llmobs test * Some more messages * do not send 'Generic Error' with empty stack * Remove error type from message * A bunch more messages * Missing cypress plugin messages
DataDog · Dec 12, 2024 · c6defbc · c6defbc
1 parent 95b6f95
commit c6defbc
Show file tree

Hide file tree

Showing 61 changed files with 165 additions and 142 deletions.
diff --git a/packages/datadog-instrumentations/src/helpers/bundler-register.js b/packages/datadog-instrumentations/src/helpers/bundler-register.js
@@ -30,12 +30,12 @@ dc.subscribe(CHANNEL, (payload) => {
   try {
     hooks[payload.package]()
   } catch (err) {
-    log.error(`esbuild-wrapped ${payload.package} missing in list of hooks`)
+    log.error('esbuild-wrapped %s missing in list of hooks', payload.package)
     throw err
   }
 
   if (!instrumentations[payload.package]) {
-    log.error(`esbuild-wrapped ${payload.package} missing in list of instrumentations`)
+    log.error('esbuild-wrapped %s missing in list of instrumentations', payload.package)
     return
   }
 
@@ -47,7 +47,7 @@ dc.subscribe(CHANNEL, (payload) => {
       loadChannel.publish({ name, version: payload.version, file })
       payload.module = hook(payload.module, payload.version)
     } catch (e) {
-      log.error(e)
+      log.error('Error executing bundler hook', e)
     }
   }
 })
diff --git a/packages/datadog-instrumentations/src/helpers/register.js b/packages/datadog-instrumentations/src/helpers/register.js
@@ -103,8 +103,7 @@ for (const packageName of names) {
         try {
           version = version || getVersion(moduleBaseDir)
         } catch (e) {
-          log.error(`Error getting version for "${name}": ${e.message}`)
-          log.error(e)
+          log.error('Error getting version for "%s": %s', name, e.message, e)
           continue
         }
         if (typeof namesAndSuccesses[`${name}@${version}`] === 'undefined') {
@@ -146,7 +145,7 @@ for (const packageName of names) {
           `integration:${name}`,
           `integration_version:${version}`
         ])
-        log.info(`Found incompatible integration version: ${nameVersion}`)
+        log.info('Found incompatible integration version: %s', nameVersion)
         seenCombo.add(nameVersion)
       }
     }

diff --git a/packages/datadog-instrumentations/src/http/client.js b/packages/datadog-instrumentations/src/http/client.js
@@ -39,7 +39,7 @@ function patch (http, methodName) {
       try {
         args = normalizeArgs.apply(null, arguments)
       } catch (e) {
-        log.error(e)
+        log.error('Error normalising http req arguments', e)
         return request.apply(this, arguments)
       }
 

diff --git a/packages/datadog-instrumentations/src/jest.js b/packages/datadog-instrumentations/src/jest.js
@@ -451,7 +451,7 @@ function cliWrapper (cli, jestVersion) {
         earlyFlakeDetectionFaultyThreshold = libraryConfig.earlyFlakeDetectionFaultyThreshold
       }
     } catch (err) {
-      log.error(err)
+      log.error('Jest library configuration error', err)
     }
 
     if (isEarlyFlakeDetectionEnabled) {
@@ -472,7 +472,7 @@ function cliWrapper (cli, jestVersion) {
           isEarlyFlakeDetectionEnabled = false
         }
       } catch (err) {
-        log.error(err)
+        log.error('Jest known tests error', err)
       }
     }
 
@@ -491,7 +491,7 @@ function cliWrapper (cli, jestVersion) {
           skippableSuites = receivedSkippableSuites
         }
       } catch (err) {
-        log.error(err)
+        log.error('Jest test-suite skippable error', err)
       }
     }
 

diff --git a/packages/datadog-instrumentations/src/playwright.js b/packages/datadog-instrumentations/src/playwright.js
@@ -425,7 +425,7 @@ function runnerHook (runnerExport, playwrightVersion) {
       }
     } catch (e) {
       isEarlyFlakeDetectionEnabled = false
-      log.error(e)
+      log.error('Playwright session start error', e)
     }
 
     if (isEarlyFlakeDetectionEnabled && semver.gte(playwrightVersion, MINIMUM_SUPPORTED_VERSION_EFD)) {
@@ -438,7 +438,7 @@ function runnerHook (runnerExport, playwrightVersion) {
         }
       } catch (err) {
         isEarlyFlakeDetectionEnabled = false
-        log.error(err)
+        log.error('Playwright known tests error', err)
       }
     }
 

diff --git a/packages/datadog-plugin-avsc/src/schema_iterator.js b/packages/datadog-plugin-avsc/src/schema_iterator.js
@@ -110,7 +110,7 @@ class SchemaExtractor {
       }
       for (const field of schema.fields) {
         if (!this.extractProperty(field, schemaName, field.name, builder, depth)) {
-          log.warn(`DSM: Unable to extract field with name: ${field.name} from Avro schema with name: ${schemaName}`)
+          log.warn('DSM: Unable to extract field with name: %s from Avro schema with name: %s', field.name, schemaName)
         }
       }
     }

diff --git a/packages/datadog-plugin-aws-sdk/src/services/eventbridge.js b/packages/datadog-plugin-aws-sdk/src/services/eventbridge.js
@@ -45,7 +45,7 @@ class EventBridge extends BaseAwsSdkPlugin {
         }
         request.params.Entries[0].Detail = finalData
       } catch (e) {
-        log.error(e)
+        log.error('EventBridge error injecting request', e)
       }
     }
   }

diff --git a/packages/datadog-plugin-aws-sdk/src/services/kinesis.js b/packages/datadog-plugin-aws-sdk/src/services/kinesis.js
@@ -97,7 +97,7 @@ class Kinesis extends BaseAwsSdkPlugin {
         parsedAttributes: decodedData._datadog
       }
     } catch (e) {
-      log.error(e)
+      log.error('Kinesis error extracting response', e)
     }
   }
 

diff --git a/packages/datadog-plugin-aws-sdk/src/services/lambda.js b/packages/datadog-plugin-aws-sdk/src/services/lambda.js
@@ -43,7 +43,7 @@ class Lambda extends BaseAwsSdkPlugin {
           const newContextBase64 = Buffer.from(JSON.stringify(clientContext)).toString('base64')
           request.params.ClientContext = newContextBase64
         } catch (err) {
-          log.error(err)
+          log.error('Lambda error injecting request', err)
         }
       }
     }

diff --git a/packages/datadog-plugin-aws-sdk/src/services/sqs.js b/packages/datadog-plugin-aws-sdk/src/services/sqs.js
@@ -163,7 +163,7 @@ class Sqs extends BaseAwsSdkPlugin {
         return JSON.parse(buffer)
       }
     } catch (e) {
-      log.error(e)
+      log.error('Sqs error parsing DD attributes', e)
     }
   }
 

diff --git a/packages/datadog-plugin-cypress/src/cypress-plugin.js b/packages/datadog-plugin-cypress/src/cypress-plugin.js
@@ -223,7 +223,7 @@ class CypressPlugin {
     this.libraryConfigurationPromise = getLibraryConfiguration(this.tracer, this.testConfiguration)
       .then((libraryConfigurationResponse) => {
         if (libraryConfigurationResponse.err) {
-          log.error(libraryConfigurationResponse.err)
+          log.error('Cypress plugin library config response error', libraryConfigurationResponse.err)
         } else {
           const {
             libraryConfig: {
@@ -360,7 +360,7 @@ class CypressPlugin {
         this.testConfiguration
       )
       if (knownTestsResponse.err) {
-        log.error(knownTestsResponse.err)
+        log.error('Cypress known tests response error', knownTestsResponse.err)
         this.isEarlyFlakeDetectionEnabled = false
       } else {
         // We use TEST_FRAMEWORK_NAME for the name of the module
@@ -374,7 +374,7 @@ class CypressPlugin {
         this.testConfiguration
       )
       if (skippableTestsResponse.err) {
-        log.error(skippableTestsResponse.err)
+        log.error('Cypress skippable tests response error', skippableTestsResponse.err)
       } else {
         const { skippableTests, correlationId } = skippableTestsResponse
         this.testsToSkip = skippableTests || []

diff --git a/packages/datadog-plugin-grpc/src/util.js b/packages/datadog-plugin-grpc/src/util.js
@@ -54,7 +54,7 @@ module.exports = {
     }
 
     if (config.hasOwnProperty(filter)) {
-      log.error(`Expected '${filter}' to be an array or function.`)
+      log.error('Expected \'%s\' to be an array or function.', filter)
     }
 
     return () => ({})

diff --git a/packages/datadog-plugin-oracledb/src/index.js b/packages/datadog-plugin-oracledb/src/index.js
@@ -33,7 +33,7 @@ function getUrl (connectString) {
   try {
     return new URL(`http://${connectString}`)
   } catch (e) {
-    log.error(e)
+    log.error('Invalid oracle connection string', e)
     return {}
   }
 }

diff --git a/packages/datadog-shimmer/src/shimmer.js b/packages/datadog-shimmer/src/shimmer.js
@@ -136,15 +136,15 @@ function wrapMethod (target, name, wrapper, noAssert) {
       if (callState.completed) {
         // error was thrown after original function returned/resolved, so
         // it was us. log it.
-        log.error(e)
+        log.error('Shimmer error was thrown after original function returned/resolved', e)
         // original ran and returned something. return it.
         return callState.retVal
       }
 
       if (!callState.called) {
         // error was thrown before original function was called, so
         // it was us. log it.
-        log.error(e)
+        log.error('Shimmer error was thrown before original function was called', e)
         // original never ran. call it unwrapped.
         return original.apply(this, args)
       }

diff --git a/packages/dd-trace/src/ci-visibility/dynamic-instrumentation/index.js b/packages/dd-trace/src/ci-visibility/dynamic-instrumentation/index.js
@@ -90,8 +90,8 @@ class TestVisDynamicInstrumentation {
       }
     }).unref()
 
-    this.worker.on('error', (err) => log.error(err))
-    this.worker.on('messageerror', (err) => log.error(err))
+    this.worker.on('error', (err) => log.error('ci-visibility DI worker error', err))
+    this.worker.on('messageerror', (err) => log.error('ci-visibility DI worker messageerror', err))
   }
 }
 

diff --git a/packages/dd-trace/src/ci-visibility/exporters/agentless/coverage-writer.js b/packages/dd-trace/src/ci-visibility/exporters/agentless/coverage-writer.js
@@ -63,7 +63,7 @@ class Writer extends BaseWriter {
           TELEMETRY_ENDPOINT_PAYLOAD_DROPPED,
           { endpoint: 'code_coverage' }
         )
-        log.error(err)
+        log.error('Error sending CI coverage payload', err)
         done()
         return
       }

diff --git a/packages/dd-trace/src/ci-visibility/exporters/agentless/di-logs-writer.js b/packages/dd-trace/src/ci-visibility/exporters/agentless/di-logs-writer.js
@@ -40,7 +40,7 @@ class DynamicInstrumentationLogsWriter extends BaseWriter {
 
     request(data, options, (err, res) => {
       if (err) {
-        log.error(err)
+        log.error('Error sending DI logs payload', err)
         done()
         return
       }

diff --git a/packages/dd-trace/src/ci-visibility/exporters/agentless/index.js b/packages/dd-trace/src/ci-visibility/exporters/agentless/index.js
@@ -38,7 +38,7 @@ class AgentlessCiVisibilityExporter extends CiVisibilityExporter {
       apiUrl = new URL(apiUrl)
       this._apiUrl = apiUrl
     } catch (e) {
-      log.error(e)
+      log.error('Error setting CI exporter api url', e)
     }
   }
 

diff --git a/packages/dd-trace/src/ci-visibility/exporters/agentless/writer.js b/packages/dd-trace/src/ci-visibility/exporters/agentless/writer.js
@@ -64,7 +64,7 @@ class Writer extends BaseWriter {
           TELEMETRY_ENDPOINT_PAYLOAD_DROPPED,
           { endpoint: 'test_cycle' }
         )
-        log.error(err)
+        log.error('Error sending CI agentless payload', err)
         done()
         return
       }

diff --git a/packages/dd-trace/src/ci-visibility/exporters/ci-visibility-exporter.js b/packages/dd-trace/src/ci-visibility/exporters/ci-visibility-exporter.js
@@ -225,7 +225,7 @@ class CiVisibilityExporter extends AgentInfoExporter {
         repositoryUrl,
         (err) => {
           if (err) {
-            log.error(`Error uploading git metadata: ${err.message}`)
+            log.error('Error uploading git metadata: %s', err.message)
           } else {
             log.debug('Successfully uploaded git metadata')
           }
@@ -345,7 +345,7 @@ class CiVisibilityExporter extends AgentInfoExporter {
       this._writer.setUrl(url)
       this._coverageWriter.setUrl(coverageUrl)
     } catch (e) {
-      log.error(e)
+      log.error('Error setting CI exporter url', e)
     }
   }
 

diff --git a/packages/dd-trace/src/config.js b/packages/dd-trace/src/config.js
@@ -145,7 +145,7 @@ function maybeFile (filepath) {
   try {
     return fs.readFileSync(filepath, 'utf8')
   } catch (e) {
-    log.error(e)
+    log.error('Error reading file %s', filepath, e)
     return undefined
   }
 }
@@ -378,7 +378,7 @@ class Config {
         } catch (e) {
           // Only log error if the user has set a git.properties path
           if (process.env.DD_GIT_PROPERTIES_FILE) {
-            log.error(e)
+            log.error('Error reading DD_GIT_PROPERTIES_FILE: %s', DD_GIT_PROPERTIES_FILE, e)
           }
         }
         if (gitPropertiesString) {
@@ -553,7 +553,7 @@ class Config {
     this._setValue(defaults, 'telemetry.dependencyCollection', true)
     this._setValue(defaults, 'telemetry.enabled', true)
     this._setValue(defaults, 'telemetry.heartbeatInterval', 60000)
-    this._setValue(defaults, 'telemetry.logCollection', false)
+    this._setValue(defaults, 'telemetry.logCollection', true)
     this._setValue(defaults, 'telemetry.metrics', true)
     this._setValue(defaults, 'traceEnabled', true)
     this._setValue(defaults, 'traceId128BitGenerationEnabled', true)
@@ -1143,12 +1143,6 @@ class Config {
       calc['tracePropagationStyle.extract'] = calc['tracePropagationStyle.extract'] || defaultPropagationStyle
     }
 
-    const iastEnabled = coalesce(this._options['iast.enabled'], this._env['iast.enabled'])
-    const profilingEnabled = coalesce(this._options['profiling.enabled'], this._env['profiling.enabled'])
-    const injectionIncludesProfiler = (this._env.injectionEnabled || []).includes('profiler')
-    if (iastEnabled || ['auto', 'true'].includes(profilingEnabled) || injectionIncludesProfiler) {
-      this._setBoolean(calc, 'telemetry.logCollection', true)
-    }
     if (this._env.injectionEnabled?.length > 0) {
       this._setBoolean(calc, 'crashtracking.enabled', true)
     }

diff --git a/packages/dd-trace/src/crashtracking/crashtracker.js b/packages/dd-trace/src/crashtracking/crashtracker.js
@@ -20,7 +20,7 @@ class Crashtracker {
       binding.updateConfig(this._getConfig(config))
       binding.updateMetadata(this._getMetadata(config))
     } catch (e) {
-      log.error(e)
+      log.error('Error configuring crashtracker', e)
     }
   }
 
@@ -36,7 +36,7 @@ class Crashtracker {
         this._getMetadata(config)
       )
     } catch (e) {
-      log.error(e)
+      log.error('Error initialising crashtracker', e)
     }
   }
 

diff --git a/packages/dd-trace/src/datastreams/writer.js b/packages/dd-trace/src/datastreams/writer.js
@@ -45,13 +45,13 @@ class DataStreamsWriter {
 
     zlib.gzip(encodedPayload, { level: 1 }, (err, compressedData) => {
       if (err) {
-        log.error(err)
+        log.error('Error zipping datastream', err)
         return
       }
       makeRequest(compressedData, this._url, (err, res) => {
         log.debug(`Response from the agent: ${res}`)
         if (err) {
-          log.error(err)
+          log.error('Error sending datastream', err)
         }
       })
     })

diff --git a/packages/dd-trace/src/debugger/devtools_client/config.js b/packages/dd-trace/src/debugger/devtools_client/config.js
@@ -15,7 +15,7 @@ const config = module.exports = {
 updateUrl(parentConfig)
 
 configPort.on('message', updateUrl)
-configPort.on('messageerror', (err) => log.error(err))
+configPort.on('messageerror', (err) => log.error('Debugger config messageerror', err))
 
 function updateUrl (updates) {
   config.url = updates.url || format({

diff --git a/packages/dd-trace/src/debugger/devtools_client/index.js b/packages/dd-trace/src/debugger/devtools_client/index.js
@@ -93,7 +93,7 @@ session.on('Debugger.paused', async ({ params }) => {
 
     // TODO: Process template (DEBUG-2628)
     send(probe.template, logger, snapshot, (err) => {
-      if (err) log.error(err)
+      if (err) log.error('Debugger error', err)
       else ackEmitting(probe)
     })
   }

diff --git a/packages/dd-trace/src/debugger/devtools_client/remote_config.js b/packages/dd-trace/src/debugger/devtools_client/remote_config.js
@@ -41,7 +41,7 @@ rcPort.on('message', async ({ action, conf: probe, ackId }) => {
     ackError(err, probe)
   }
 })
-rcPort.on('messageerror', (err) => log.error(err))
+rcPort.on('messageerror', (err) => log.error('Debugger RC message error', err))
 
 async function processMsg (action, probe) {
   log.debug(`Received request to ${action} ${probe.type} probe (id: ${probe.id}, version: ${probe.version})`)

diff --git a/packages/dd-trace/src/debugger/devtools_client/status.js b/packages/dd-trace/src/debugger/devtools_client/status.js
@@ -55,7 +55,7 @@ function ackEmitting ({ id: probeId, version }) {
 }
 
 function ackError (err, { id: probeId, version }) {
-  log.error(err)
+  log.error('Debugger ackError', err)
 
   onlyUniqueUpdates(STATUSES.ERROR, probeId, version, () => {
     const payload = statusPayload(probeId, version, STATUSES.ERROR)
@@ -87,7 +87,7 @@ function send (payload) {
   }
 
   request(form, options, (err) => {
-    if (err) log.error(err)
+    if (err) log.error('Error sending debugger payload', err)
   })
 }