promote ReplayManager to component, replacing Recorder (#1348)

Author: waltjones

src/browser/core.js

@@ -12,9 +12,8 @@ import * as sharedTransforms from '../transforms.js';
 import * as predicates from './predicates.js';
 import * as sharedPredicates from '../predicates.js';
 import errorParser from '../errorParser.js';
-import recorderDefaults from './replay/defaults.js';
+import replayDefaults from './replay/defaults.js';
 import tracingDefaults from '../tracing/defaults.js';
-import ReplayManager from './replay/replayManager.js';
 
 // Used to support global `Rollbar` instance.
 let _instance = null;
@@ -31,7 +30,7 @@ class Rollbar {
     this.scrub = this.components.scrub;
     const truncation = this.components.truncation;
     const Tracing = this.components.tracing;
-    const Recorder = this.components.recorder;
+    const ReplayManager = this.components.replayManager;
 
     const transport = new Transport(truncation);
     const api = new API(this.options, transport, urllib, truncation);
@@ -43,18 +42,16 @@ class Rollbar {
       this.telemeter = new Telemeter(this.options, this.tracing);
     }
 
-    if (Recorder && _.isBrowser()) {
-      const recorderOptions = this.options.recorder;
-      this.recorder = new Recorder(recorderOptions);
+    if (ReplayManager && _.isBrowser()) {
+      const replayOptions = this.options.replay;
       this.replayManager = new ReplayManager({
-        recorder: this.recorder,
-        api: api,
         tracing: this.tracing,
         telemeter: this.telemeter,
+        options: replayOptions,
       });
 
-      if (recorderOptions.enabled && recorderOptions.autoStart) {
-        this.recorder.start();
+      if (replayOptions.enabled && replayOptions.autoStart) {
+        this.replayManager.recorder.start();
       }
     }
 
@@ -129,7 +126,7 @@ class Rollbar {
     );
 
     this.tracing?.configure(this.options);
-    this.recorder?.configure(this.options);
+    this.replayManager?.recorder?.configure(this.options);
     this.client.configure(this.options, payloadData);
     this.instrumenter?.configure(this.options);
     this.setupUnhandledCapture();
@@ -582,7 +579,7 @@ import {
 } from '../defaults.js';
 import browserDefaults from './defaults.js';
 
-var defaultOptions = {
+const defaultOptions = {
   environment: 'unknown',
   version: version,
   scrubFields: browserDefaults.scrubFields,
@@ -599,7 +596,7 @@ var defaultOptions = {
   inspectAnonymousErrors: true,
   ignoreDuplicateErrors: true,
   wrapGlobalEventHandlers: false,
-  recorder: recorderDefaults,
+  replay: replayDefaults,
   tracing: tracingDefaults,
 };
 

src/browser/replay/recorder.js

@@ -27,15 +27,12 @@ export default class Recorder {
    * Creates a new Recorder instance for capturing DOM events
    *
    * @param {Object} options - Configuration options for the recorder
-   * @param {Function} [recordFn=rrwebRecordFn] - The recording function to use
    */
-  constructor(options, recordFn = rrwebRecordFn) {
-    if (!recordFn) {
-      throw new TypeError("Expected 'recordFn' to be provided");
-    }
-
+  constructor(options) {
     this.options = options;
-    this._recordFn = recordFn;
+
+    // Tests inject a custom rrweb record function or mock.
+    this._recordFn = options.recordFn ||rrwebRecordFn;
   }
 
   get isRecording() {

src/browser/replay/replayManager.js

@@ -1,5 +1,6 @@
 import id from '../../tracing/id.js';
 import logger from '../../logger.js';
+import Recorder from './recorder.js';
 import ReplayPredicates from './replayPredicates.js';
 
 /** @typedef {import('./recorder.js').BufferCursor} BufferCursor */
@@ -24,7 +25,6 @@ export default class ReplayManager {
   _map;
   /** @type {Recorder} */
   _recorder;
-  _api;
   _tracing;
   _telemeter;
   _pendingLeading;
@@ -33,32 +33,22 @@ export default class ReplayManager {
   /**
    * Creates a new ReplayManager instance
    *
-   * @param {Object} props - Configuration props
-   * @param {Recorder} props.recorder - The recorder instance that dumps replay data into spans
-   * @param {Object} props.api - The API instance used to send replay payloads to the backend
-   * @param {Object} props.tracing - The tracing instance used to create spans and manage context
+   * @param {Object} [props.tracing] - The tracing instance used to create spans and manage context
+   * @param {Object} [props.telemeter] - Optional telemeter instance for capturing telemetry events
+   * @param {Object} [props.options] - Configuration options
    */
-  constructor({ recorder, api, tracing, telemeter }) {
-    if (!recorder) {
-      throw new TypeError("Expected 'recorder' to be provided");
-    }
-
-    if (!api) {
-      throw new TypeError("Expected 'api' to be provided");
-    }
-
+  constructor({ tracing, telemeter, options }) {
     if (!tracing) {
       throw new TypeError("Expected 'tracing' to be provided");
     }
 
     this._map = new Map();
-    this._recorder = recorder;
-    this._api = api;
+    this._recorder = new Recorder(options);
     this._tracing = tracing;
     this._telemeter = telemeter;
     this._pendingLeading = new Map();
     this._trailingStatus = new Map();
-    this._predicates = new ReplayPredicates(recorder.options);
+    this._predicates = new ReplayPredicates(options);
   }
 
   /**
@@ -196,7 +186,7 @@ export default class ReplayManager {
     switch (trailingStatus) {
       case TrailingStatus.SENT:
         try {
-          await this._api.postSpans(pendingContext.leadingPayload, {
+          await this._tracing.exporter.post(pendingContext.leadingPayload, {
             'X-Rollbar-Replay-Id': replayId,
           });
         } catch (error) {
@@ -350,7 +340,7 @@ export default class ReplayManager {
       throw Error(`ReplayManager.send: No payload found for id: ${replayId}`);
     }
 
-    await this._api.postSpans(payload, { 'X-Rollbar-Replay-Id': replayId });
+    await this._tracing.exporter.post(payload, { 'X-Rollbar-Replay-Id': replayId });
 
     this._trailingStatus.set(replayId, TrailingStatus.SENT);
     await this._sendOrDiscardLeadingReplay(replayId);
@@ -413,6 +403,15 @@ export default class ReplayManager {
     return this._map.size;
   }
 
+  /**
+   * Returns the Recorder instance used by this manager
+   *
+   * @returns {Recorder} The Recorder instance
+   */
+  get recorder() {
+    return this._recorder;
+  }
+
   /**
    * Clears all stored replays without sending them
    */

src/browser/rollbar.js

@@ -5,7 +5,7 @@ import wrapGlobals from './wrapGlobals.js';
 import scrub from '../scrub.js';
 import truncation from '../truncation.js';
 import Tracing from '../tracing/tracing.js';
-import Recorder from './replay/recorder.js';
+import ReplayManager from './replay/replayManager.js';
 
 Rollbar.setComponents({
   telemeter: Telemeter,
@@ -14,7 +14,7 @@ Rollbar.setComponents({
   scrub: scrub,
   truncation: truncation,
   tracing: Tracing,
-  recorder: Recorder,
+  replayManager: ReplayManager,
 });
 
 export default Rollbar;

test/browser.replay.recorder.test.js

@@ -1,5 +1,6 @@
 import { expect } from 'chai';
 import sinon from 'sinon';
+import { record as rrwebRecordFn } from '@rrweb/record';
 import { EventType } from '@rrweb/types';
 
 import Recorder from '../src/browser/replay/recorder.js';
@@ -65,17 +66,16 @@ describe('Recorder', function () {
       });
     });
 
-    it('should throw error if no record function is passed', function () {
-      expect(() => new Recorder({}, null)).to.throw(
-        TypeError,
-        "Expected 'recordFn' to be provided",
-      );
+    it('should use the default recorder if no record function is passed', function () {
+      const recorder = new Recorder({});
+
+      expect(recorder._recordFn).to.equal(rrwebRecordFn);
     });
   });
 
   describe('recording management', function () {
     it('should start recording correctly', function () {
-      const recorder = new Recorder({ enabled: true }, recordFnStub);
+      const recorder = new Recorder({ enabled: true, recordFn: recordFnStub });
       recorder.start();
 
       expect(recorder.isRecording).to.be.true;
@@ -88,23 +88,23 @@ describe('Recorder', function () {
     });
 
     it('should not start if already recording', function () {
-      const recorder = new Recorder({ enabled: true }, recordFnStub);
+      const recorder = new Recorder({ enabled: true, recordFn: recordFnStub });
       recorder.start();
       recorder.start();
 
       expect(recordFnStub.calledOnce).to.be.true;
     });
 
     it('should not start if disabled', function () {
-      const recorder = new Recorder({ enabled: false }, recordFnStub);
+      const recorder = new Recorder({ enabled: false, recordFn: recordFnStub });
       recorder.start();
 
       expect(recorder.isRecording).to.be.false;
       expect(recordFnStub.called).to.be.false;
     });
 
     it('should stop recording correctly', function () {
-      const recorder = new Recorder({ enabled: true }, recordFnStub);
+      const recorder = new Recorder({ enabled: true, recordFn: recordFnStub });
       recorder.start();
       recorder.stop();
 
@@ -124,7 +124,7 @@ describe('Recorder', function () {
 
   describe('event handling', function () {
     it('should handle events correctly', function () {
-      const recorder = new Recorder({}, recordFnStub);
+      const recorder = new Recorder({ recordFn: recordFnStub });
       recorder.start();
 
       const event1 = { timestamp: 1000, type: 'event1', data: { a: 1 } };
@@ -166,7 +166,7 @@ describe('Recorder', function () {
     });
 
     it('should be ready after first full snapshot', function () {
-      const recorder = new Recorder({}, recordFnStub);
+      const recorder = new Recorder({ recordFn: recordFnStub });
       recorder.start();
 
       // First checkout
@@ -181,7 +181,7 @@ describe('Recorder', function () {
     });
 
     it('should handle checkout events correctly', function () {
-      const recorder = new Recorder({}, recordFnStub);
+      const recorder = new Recorder({ recordFn: recordFnStub });
       recorder.start();
 
       // First checkout
@@ -308,7 +308,7 @@ describe('Recorder', function () {
 
   describe('dump functionality', function () {
     it('should create a span with events and return formatted payload', function () {
-      const recorder = new Recorder({}, recordFnStub);
+      const recorder = new Recorder({ recordFn: recordFnStub });
       recorder.start();
 
       emitCallback({ timestamp: 1000, type: 'event1', data: { a: 1 } }, false);
@@ -355,7 +355,7 @@ describe('Recorder', function () {
     });
 
     it('should create a span with the correct span name', function () {
-      const recorder = new Recorder({}, recordFnStub);
+      const recorder = new Recorder({ recordFn: recordFnStub });
       recorder.start();
 
       emitCallback({ timestamp: 1000, type: 'event1', data: { a: 1 } }, false);
@@ -371,7 +371,7 @@ describe('Recorder', function () {
     });
 
     it('should add events with the correct event name and replayId', function () {
-      const recorder = new Recorder({}, recordFnStub);
+      const recorder = new Recorder({ recordFn: recordFnStub });
       recorder.start();
 
       emitCallback(
@@ -402,7 +402,7 @@ describe('Recorder', function () {
     });
 
     it('should handle no events', function () {
-      const recorder = new Recorder({}, recordFnStub);
+      const recorder = new Recorder({ recordFn: recordFnStub });
 
       expect(() => {
         recorder.exportRecordingSpan(mockTracing, {
@@ -417,7 +417,7 @@ describe('Recorder', function () {
 
   describe('configure', function () {
     it('should update options', function () {
-      const recorder = new Recorder({ enabled: true }, recordFnStub);
+      const recorder = new Recorder({ enabled: true, recordFn: recordFnStub });
 
       recorder.configure({ enabled: false, maxSeconds: 20 });
 
@@ -449,7 +449,7 @@ describe('Recorder', function () {
     });
 
     it('should stop recording if enabled set to false', function () {
-      const recorder = new Recorder({ enabled: true }, recordFnStub);
+      const recorder = new Recorder({ enabled: true, recordFn: recordFnStub });
       recorder.start();
 
       expect(recorder.isRecording).to.be.true;

test/browser.rollbar.test.js

@@ -105,7 +105,7 @@ describe('Rollbar()', function () {
     var client = new (TestClientGen())();
     var options = {
       scrubFields: ['foobar'],
-      recorder: {
+      replay: {
         enabled: true,
       },
       tracing: {
@@ -116,8 +116,8 @@ describe('Rollbar()', function () {
 
     expect(rollbar.options.scrubFields).to.contain('foobar');
     expect(rollbar.options.scrubFields).to.contain('password');
-    expect(rollbar.options.recorder.enabled).to.be.true;
-    expect(rollbar.options.recorder.triggers[0].level).to.eql([
+    expect(rollbar.options.replay.enabled).to.be.true;
+    expect(rollbar.options.replay.triggers[0].level).to.eql([
       'error',
       'critical',
     ]);
@@ -869,7 +869,7 @@ describe('log', function () {
 
     expect(body.data.body.trace.exception.message).to.eql('test error');
     expect(body.data.attributes).to.be.an('array');
-    expect(body.data.attributes.length).to.eql(4);
+    expect(body.data.attributes.length).to.eql(3);
     expect(body.data.attributes[0].key).to.eql('session_id');
     expect(body.data.attributes[0].value).to.match(/^[a-f0-9]{32}$/);
     expect(body.data.attributes[1].key).to.eql('span_id');