[Replay] Move rrweb-specific configuration into recordingOptions

[mydea]

Currently, replay is configured like this:

new Replay({
  // replay specific config
  useCompression: true,
  stickySession: true,

  // this is passed directly to rrweb
  blockClass: 'sentry-block',
  ignoreClass: 'sentry-ignore',
  maskTextClass: 'sentry-mask'
});

From a code perspective, this happens with a rest operator:

constructor({
    flushMinDelay = 5000,
    flushMaxDelay = 15000,
    initialFlushDelay = 5000,
    stickySession = true,
    useCompression = true,
    sessionSampleRate = DEFAULT_SESSION_SAMPLE_RATE,
    errorSampleRate = DEFAULT_ERROR_SAMPLE_RATE,
    maskAllText = true,
    maskAllInputs = true,
    blockAllMedia = true,
    blockClass = 'sentry-block',
    ignoreClass = 'sentry-ignore',
    maskTextClass = 'sentry-mask',
    blockSelector = '[data-sentry-block]',
    ...recordingOptions, // <-- this is the important part
  }: ReplayConfiguration = {}) {

This basically means that any config passed to new Replay() that is not defined above will be passed to rrweb.

In order to streamline this and make this more explicit, I propose to change this API to:

new Replay({
  // replay specific config
  useCompression: true,
  stickySession: true,

  recordingOptions: {
    blockClass: 'sentry-block',
    ignoreClass: 'sentry-ignore',
    maskTextClass: 'sentry-mask'
  }
});

This would make it much more explicit which options are only related to rrweb.
We can still promote all options we care about/think are important to the top level and internall pass them to recordingOptions, e.g.:

new Replay({
  // replay specific config
  useCompression: true,
  stickySession: true,

  // Explicit rrweb options we care about - name can match, but could also be different
  blockClass: 'myClass',
  otherNameForIgnoreClass: 'ignore'

  recordingOptions: {}
});

For now, I propose to add these options to the top level:

• ignoreClass: string | string[] one or multiple classes to ignore. We can internally convert this for ignoreClass
• blockClass: string | string[]: one or multiple classes to block
• blockSelector: string | string[]: One or multiple selectors to block
• maskTextClass: string | string[]: one or multiple classes to mask text for
• inlineImages: If true, inline images as data URIs
• recordCanvas: If true, record canvas element content

Which covers the most relevant stuff, and also the stuff we currently provide default values for.

For the initial release, we can still handle the "old" way of config, warn when we detect "unknown" config, and still make it work. And then remove this deprecated functionality in the near future.

[billyvg]

FWIW we changed this in getsentry/sentry-replay#226 -- my reasoning at the time is that from the user's perspective, they shouldn't need to decide what is a "recordingOption" or not. Also in a hypothetical world where we do not use rrweb, would it make sense to still have a recordingOption object?

[mydea]

I generally agree, and I would advocate to actually make the "main" configuration options explicit on our end - e.g. users could still directly configure blockClass directly in new Replay(). This is really more about the "... other" options that could be passed through, which leads to us not really "owning" the public API surface of Replay (for example, the options to rrweb change in v2! this would mean we have to do a breaking change when we upgrade, because the available options to pass change).

I was also thinking about simply naming this rrweb to make it explicit. Maybe that makes more sense - from our perspective, that would mean:

• Promote the options we care about/think are important to top level on our end
• Allow to pass rrweb really more as an escape hatch for obscure options or similar

WDYT?

[mydea]

FYI this issue: #6464 is another, very good, reason to actually do this - we don't want to expose any rrweb-specific types to our users.

[github-actions]

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!

---

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

[billyvg]

Closed by #6645