feat(Session Replay): ReplayEvent, ReplayRecording and Envelope handling

[brustolin]

Added SentryReplayEven, SentryReplayRecording and Envelope handling for this new types.

#skip-changelog

Relates to:

• [Epic] Mobile Replay: Private Alpha Release sentry#63255

[github-actions]

	Messages
📖	Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 53b3089

[github-actions]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1203.16 ms	1216.26 ms	13.09 ms
Size	21.58 KiB	429.58 KiB	408.00 KiB

Previous results on branch: feat(SR)/ReplayEvent

Startup times

Revision	Plain	With Sentry	Diff
6381b5a	1239.04 ms	1243.80 ms	4.76 ms
20a0fe7	1204.59 ms	1211.98 ms	7.39 ms
91877f9	1196.71 ms	1212.69 ms	15.98 ms
90ce64a	1204.37 ms	1212.98 ms	8.61 ms
39743bf	1234.49 ms	1258.06 ms	23.57 ms

App size

Revision	Plain	With Sentry	Diff
6381b5a	21.58 KiB	423.65 KiB	402.06 KiB
20a0fe7	21.58 KiB	429.51 KiB	407.92 KiB
91877f9	21.58 KiB	422.76 KiB	401.17 KiB
90ce64a	21.58 KiB	424.51 KiB	402.93 KiB
39743bf	21.58 KiB	429.46 KiB	407.88 KiB

[philipphofmann]

That looks great. I'm excited to see session reply land on Cocoa soonish 😃. A few points to consider:

1. Some new classes miss NS_ASSUME_NONNULL_BEGIN and NS_ASSUME_NONNULL_END. Please add them. I think we should write a linter for that.
2. You could consider writing classes without dependencies in Swift.
3. I think you could set the base of this PR to main, so we include it sooner in the code base. Less potential for merge conflicts. I'm OK with the tradeoff that there is a bit of code that isn't used yet in there.

[vaind]

LGTM so far

[philipphofmann]

A few comments

[philipphofmann]

m: I think it would make sense to add some sample files in the message pack format and use them for extra tests to ensure our serialization works properly.

[brustolin]

We do have tests with files

[philipphofmann]

No, I mean using some predefined dictionary with a couple of different values, serializing it with another message pack library, store the output to a file, add this file to our tests, and then comparing the results of the predefined dictionary with the stored file.

[brustolin]

Although the reading works in another message pack library (I tested it). The writing is different since I didn't optimise as explained in the SentryMsgPackSerializer.m line 35

        // We will always use the 4 bytes data length for simplicity.
        // Worst case we're losing 3 bytes.

As long we dont change the tests in this file that check for format, there is nothing to worry about.

[philipphofmann]

Then maybe add a snapshot. Take some sample data, serialize it with SentryMsgPackSerializer, validate the output against another message pack library, store the contents to a file, and use the file in the tests.

[philipphofmann]

No, I mean using some predefined dictionary with a couple of different values, serializing it with another message pack library, store the output to a file, add this file to our tests, and then comparing the results of the predefined dictionary with the stored file.

[romtsn]

we probably also got to nullify other stuff that replays do not support currently:

replayEvent.extra = nil;
replayEvent.threads = nil;
replayEvent.debugMeta = nil;

[romtsn]

and also this part which will be executed for the replay event I believe, but it shouldn't:

sentry-cocoa/Sources/Sentry/SentryClient.m

Lines 557 to 562 in 7b5ebd9

	// Transactions have their own sampleRate
	if (eventIsNotATransaction && [self isSampled:self.options.sampleRate]) {
	SENTRY_LOG_DEBUG(@"Event got sampled, will not send the event");
	[self recordLostEvent:kSentryDataCategoryError reason:kSentryDiscardReasonSampleRate];
	return nil;
	}

[brustolin]

Good catch. I will fix this.

[brustolin]

I believe it's best to open a new PR to address prepareEvent rather than further inflating this PR

[romtsn]

sure! just pointed it out so we don't miss that

[romtsn]

LGTM 🎉

[philipphofmann]

LGTM