feat(replay): Streamline replay options #6641
Conversation
This streamlines options to be passed to replay a bit: First, we deprecate passing arbitrary rrweb config through. Instead, you can provide `recordingOptions` directly, which will be passed through. For now, the old way still works, but we will remove this eventually. Secondly, we explicitly defined the config options we _do_ support. This should cover most things that users are actually using, thus most users shouldn't need to actually do anything. Finally, this also separates the options we pass to the integration from the options we store on the replay container. There is no need to keep all the rrweb-specific config there, we only need this at init-time, and can then discard it.
size-limit report 📦
|
| ...recordingOptions | ||
| ignoreClass = 'sentry-ignore', | ||
| _experiments = {}, | ||
| recordingOptions = {}, |
There was a problem hiding this comment.
I would prefer if we pulled out the rrweb options that we want to allow. I still think having recordingOptions is a bit arbitrary when we have options like the above that could be considered recording options as well.
There was a problem hiding this comment.
Yeah, that's basically what we are doing here anyhow :) All the options we really support are on the top level, and recordingOptions is really more of an escape hatch to allow passing arbitrary other options (for which we do not provide any guarantees). That was also the original idea for calling this option rrweb, as that is really rrweb-specific stuff that we do not explicitly support.
There was a problem hiding this comment.
Yeah my suggestion is to kill the escape hatch is we are going to change the API anyway. It should either be a first class option or not supported at all. I'm gonna go through the rrweb options and make a list of what I think we should support. Thoughts on that?
There was a problem hiding this comment.
I'm also generally fine with that, although we may get into the situation where one customer wants to do some kind of weird thing that rrweb supports but we do not want to support. But I'm also happy with just saying "no, sorry, can't do that" in such a case!
|
This pull request 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 "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
Closing this in favor of other stuff done in recent weeks! |
This streamlines options to be passed to replay a bit:
First, we deprecate passing arbitrary rrweb config through. Instead, you can provide
recordingOptionsdirectly, which will be passed through. For now, the old way still works, but we will remove this eventually.Secondly, we explicitly defined the config options we do support. This should cover most things that users are actually using, thus most users shouldn't need to actually do anything.
Finally, this also separates the options we pass to the integration from the options we store on the replay container. There is no need to keep all the rrweb-specific config there, we only need this at init-time, and can then discard it.
Closes #6390
Note that this is technically a breaking change, because the type signature of
replay.getOptions()is technically public, but this shouldn't really affect any user. Any other config users could use before should continue to work for the time being, until we remove the deprecated functionality.