Merge branch 'master' into new_user_collection

DataDog · Dec 13, 2024 · b1ba64a · b1ba64a
2 parents 172da58 + 4304684
commit b1ba64a
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 4 deletions.
diff --git a/packages/datadog-shimmer/src/shimmer.js b/packages/datadog-shimmer/src/shimmer.js
@@ -6,8 +6,12 @@ const log = require('../../dd-trace/src/log')
 const unwrappers = new WeakMap()
 
 function copyProperties (original, wrapped) {
-  Object.setPrototypeOf(wrapped, original)
-
+  // TODO getPrototypeOf is not fast. Should we instead do this in specific
+  // instrumentations where needed?
+  const proto = Object.getPrototypeOf(original)
+  if (proto !== Function.prototype) {
+    Object.setPrototypeOf(wrapped, proto)
+  }
   const props = Object.getOwnPropertyDescriptors(original)
   const keys = Reflect.ownKeys(props)
 

diff --git a/packages/dd-trace/src/telemetry/logs/log-collector.js b/packages/dd-trace/src/telemetry/logs/log-collector.js
@@ -3,7 +3,7 @@
 const log = require('../../log')
 const { calculateDDBasePath } = require('../../util')
 
-const logs = new Map()
+const logs = new Map() // hash -> log
 
 // NOTE: Is this a reasonable number?
 let maxEntries = 10000
@@ -79,8 +79,10 @@ const logCollector = {
       }
       const hash = createHash(logEntry)
       if (!logs.has(hash)) {
-        logs.set(hash, logEntry)
+        logs.set(hash, errorCopy(logEntry))
         return true
+      } else {
+        logs.get(hash).count++
       }
     } catch (e) {
       log.error('Unable to add log to logCollector: %s', e.message)
@@ -120,6 +122,19 @@ const logCollector = {
   }
 }
 
+// clone an Error object to later serialize and transmit
+// { ...error } doesn't work
+// also users can add arbitrary fields to an error
+function errorCopy (error) {
+  const keys = Object.getOwnPropertyNames(error)
+  const obj = {}
+  for (const key of keys) {
+    obj[key] = error[key]
+  }
+  obj.count = 1
+  return obj
+}
+
 logCollector.reset()
 
 module.exports = logCollector
diff --git a/packages/dd-trace/src/util.js b/packages/dd-trace/src/util.js
@@ -66,6 +66,8 @@ function globMatch (pattern, subject) {
   return true
 }
 
+// TODO: this adds stack traces relative to packages/
+// shouldn't paths be relative to the root of dd-trace?
 function calculateDDBasePath (dirname) {
   const dirSteps = dirname.split(path.sep)
   const packagesIndex = dirSteps.lastIndexOf('packages')

diff --git a/packages/dd-trace/test/telemetry/logs/log-collector.spec.js b/packages/dd-trace/test/telemetry/logs/log-collector.spec.js
@@ -126,5 +126,24 @@ describe('telemetry log collector', () => {
       expect(logs.length).to.be.equal(4)
       expect(logs[3]).to.deep.eq({ message: 'Omitted 2 entries due to overflowing', level: 'ERROR' })
     })
+
+    it('duplicated errors should send incremented count values', () => {
+      const err1 = new Error('oh no')
+      err1.level = 'ERROR'
+
+      const err2 = new Error('foo buzz')
+      err2.level = 'ERROR'
+
+      logCollector.add(err1)
+      logCollector.add(err2)
+      logCollector.add(err1)
+      logCollector.add(err2)
+      logCollector.add(err1)
+
+      const drainedErrors = logCollector.drain()
+      expect(drainedErrors.length).to.be.equal(2)
+      expect(drainedErrors[0].count).to.be.equal(3)
+      expect(drainedErrors[1].count).to.be.equal(2)
+    })
   })
 })