fix: pr comments

Author: cjr125

packages/rum-core/src/performance-monitoring/span.js

@@ -40,17 +40,8 @@ class Span extends SpanBase {
     }
     this.sync = this.options.sync
 
-    if (
-      this.options.spanContextCallback &&
-      typeof this.options.spanContextCallback === 'function'
-    ) {
-      let tags
-      try {
-        tags = this.options.spanContextCallback()
-        this.addLabels(tags)
-      } catch (e) {
-        console.error('Failed to execute span context callback', e)
-      }
+    if (this.options.spanContextCallback) {
+      this.addLabels(this.options.spanContextCallback())
     }
   }
 

packages/rum-core/src/performance-monitoring/transaction-service.js

@@ -104,17 +104,32 @@ class TransactionService {
     let presetOptions = {
       transactionSampleRate: config.transactionSampleRate
     }
-    if (config.transactionContextCallback) {
-      presetOptions = {
-        ...presetOptions,
-        transactionContextCallback: config.transactionContextCallback
+    if (typeof config.transactionContextCallback === 'function') {
+      const tCC = function () {
+        let tags = {}
+        try {
+          tags = config.transactionContextCallback()
+        } catch (err) {
+          this._logger.error(
+            'Failed to execute transaction context callback',
+            err
+          )
+        }
+        return tags
       }
+      presetOptions.transactionContextCallback = tCC.bind(this)
     }
-    if (config.spanContextCallback) {
-      presetOptions = {
-        ...presetOptions,
-        spanContextCallback: config.spanContextCallback
+    if (typeof config.spanContextCallback === 'function') {
+      const sCC = function () {
+        let tags = {}
+        try {
+          tags = config.spanContextCallback()
+        } catch (err) {
+          this._logger.error('Failed to execute span context callback', err)
+        }
+        return tags
       }
+      presetOptions.spanContextCallback = sCC.bind(this)
     }
     let perfOptions = extend(presetOptions, options)
     if (perfOptions.managed) {
@@ -298,7 +313,7 @@ class TransactionService {
           if (name === NAME_UNKNOWN && pageLoadTransactionName) {
             tr.name = pageLoadTransactionName
           }
-          
+
           /**
            * Capture the TBT as span after observing for all long task entries
            * and once performance observer is disconnected

packages/rum-core/src/performance-monitoring/transaction.js

@@ -55,17 +55,8 @@ class Transaction extends SpanBase {
     this.sampleRate = this.options.transactionSampleRate
     this.sampled = Math.random() <= this.sampleRate
 
-    if (
-      this.options.transactionContextCallback &&
-      typeof this.options.transactionContextCallback === 'function'
-    ) {
-      let tags
-      try {
-        tags = this.options.transactionContextCallback()
-        this.addLabels(tags)
-      } catch (e) {
-        console.error('Failed to execute transaction context callback', e)
-      }
+    if (this.options.transactionContextCallback) {
+      this.addLabels(this.options.transactionContextCallback())
     }
   }
 
@@ -111,14 +102,8 @@ class Transaction extends SpanBase {
     }
     let opts = extend({}, options)
 
-    if (
-      this.options.spanContextCallback &&
-      typeof this.options.spanContextCallback === 'function'
-    ) {
-      opts = {
-        ...opts,
-        spanContextCallback: this.options.spanContextCallback
-      }
+    if (this.options.spanContextCallback) {
+      opts.spanContextCallback = this.options.spanContextCallback
     }
 
     opts.onEnd = trc => {

packages/rum-core/test/performance-monitoring/transaction-service.spec.js

@@ -706,6 +706,40 @@ describe('TransactionService', function () {
       }
       sp1.end()
     })
+
+    it('should safely catch and log errors for an invalid callback', () => {
+      logger = new LoggingService()
+      spyOn(logger, 'error')
+
+      config.setConfig({
+        transactionContextCallback: () => {
+          throw new Error('Error in transaction callback')
+        },
+        spanContextCallback: () => {
+          throw new Error('Error in span callback')
+        }
+      })
+      const transactionService = new TransactionService(logger, config)
+
+      const tr1 = transactionService.startTransaction(
+        'transaction1',
+        'transaction'
+      )
+      expect(logger.error).toHaveBeenCalledWith(
+        'Failed to execute transaction context callback',
+        new Error('Error in transaction callback')
+      )
+      logger.error.calls.reset()
+
+      const sp1 = tr1.startSpan('span1', 'span')
+      expect(logger.error).toHaveBeenCalledWith(
+        'Failed to execute span context callback',
+        new Error('Error in span callback')
+      )
+
+      sp1.end()
+      tr1.end()
+    })
   })
 
   it('should truncate active spans after transaction ends', () => {

packages/rum/src/index.d.ts

@@ -107,8 +107,8 @@ declare module '@elastic/apm-rum' {
       method: string
       payload?: string
       headers?: Record<string, string>
-    }) => boolean,
-    transactionContextCallback?: (...args: any[]) => any,
+    }) => boolean
+    transactionContextCallback?: (...args: any[]) => any
     spanContextCallback?: (...args: any[]) => any
   }