RUMM-2273 Create the Session Replay Writer component #1041
Conversation
mariusc83
force-pushed
the
mconstantin/rumm-2273/create-session-replay-writer
branch
from
3a24960 to
59fc648
Compare
Codecov Report
@@ Coverage Diff @@
## feature/session-replay-vo #1041 +/- ##
=============================================================
+ Coverage 83.26% 83.31% +0.04%
=============================================================
Files 332 334 +2
Lines 10569 10591 +22
Branches 1773 1774 +1
=============================================================
+ Hits 8800 8823 +23
+ Misses 1219 1218 -1
Partials 550 550
|
| // TODO: This will be switched to a Serializer<Record> once the models | ||
| // will be in place. RUMM-2330" | ||
| return null | ||
| internal class SessionReplayRecordSerializer : Serializer<String> { |
There was a problem hiding this comment.
by some reason github doesn't show any syntax highlight for this file. Is this file ok?
There was a problem hiding this comment.
yeah, I will recreate this file, have the same issue in my editor.
| override fun write(record: EnrichedRecord) { | ||
| // TODO: RUMM-2273 Create the Session Replay Writer | ||
| jsonStringWriter.write(record.toJson()) |
There was a problem hiding this comment.
should we call toJson in Serializer?
There was a problem hiding this comment.
actually not because the EnrichedRecord lives in the dd-dk-android-session-replay module. I don't want to expose anything from there so that's the reason I want to just delegate to a jsonStringWriter which is provided from the dd-sdk-android. If I'll do it in the SessionReplayRecordSerializer I'd need to expose the EnrichedRecord to that module and I don't want that. Better to keep everything abstract.
| } | ||
|
|
||
| return EnrichedRecord( | ||
| forge.getForgery<UUID>().toString(), |
There was a problem hiding this comment.
minor: it is better to add named arguments, not clear what these are (same applies to all other new forgeries).
| import android.os.Bundle | ||
| import com.datadog.android.sessionreplay.LifecycleCallback | ||
|
|
||
| internal class NoOpLifecycleCallback : LifecycleCallback { |
There was a problem hiding this comment.
Why don't you use the automatic NoOpFactory?
There was a problem hiding this comment.
Mostly because the LifecycleCallback lives in the dd-dk-android-session-replay module and NoOpLifecycleCallback is only relevant in the dd-sdk-android module where is actually being used. If I used the NoOpFactory it will be generated in the dd-sdk-android-session-replay and I want to avoid that.
mariusc83
force-pushed
the
mconstantin/rumm-2273/create-session-replay-writer
branch
from
59fc648 to
78d2fbe
Compare
mariusc83
deleted the
mconstantin/rumm-2273/create-session-replay-writer
branch
What does this PR do?
In this PR we are introducing the Session Replay
RecordWritercomponent in thedd-sdk-android-session-replaymodule which delegates to theSessionReplaySerializedRecordWriterin thedd-sdk-androidmodule. The latter delegates all its functionality to theDataWriter<String>component provided by theSessionReplayPersistenceStrategy. Later the plan is to switch to theStorageimplementation of the SDK V2 persistence layer.Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
Review checklist (to be filled by reviewers)