Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

RUMM-2267 Add Session Replay public API #974

Merged
merged 2 commits into from
Jul 11, 2022
Merged

RUMM-2267 Add Session Replay public API #974

merged 2 commits into from
Jul 11, 2022

Conversation

mariusc83
Copy link
Member

@mariusc83 mariusc83 commented Jul 7, 2022 •
edited
Loading

What does this PR do?

This PR adds the Session Replay Feature and Public API in the dd-sdk-android module.

Note:

  • Because this PR is build on top of the feature/sdkv2 branch the unit tests could not be added as we are waiting for the feature/sdkv2 tests to be fixed first. A new task was added in the backlog for adding the necessary tests when the time comes.
  • This is just the entry point of the feature and all the rest of the code will be leaving in the library/dd-sdk-android-session-replay module.

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)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@mariusc83 mariusc83 force-pushed the mconstantin/rumm-2267/session-replay-public-api branch 2 times, most recently from eda61b7 to 1dfdc06 Compare July 7, 2022 10:47
@mariusc83 mariusc83 self-assigned this Jul 7, 2022
@mariusc83 mariusc83 force-pushed the mconstantin/rumm-2267/session-replay-public-api branch 2 times, most recently from 31fa4de to 1936ced Compare July 7, 2022 12:06
@mariusc83 mariusc83 marked this pull request as ready for review July 7, 2022 12:57
@mariusc83 mariusc83 requested review from a team as code owners July 7, 2022 12:57
@mariusc83 mariusc83 force-pushed the mconstantin/rumm-2267/session-replay-public-api branch from 1936ced to b40943b Compare July 7, 2022 12:59
0xnm
0xnm requested changes Jul 7, 2022
View reviewed changes
dd-sdk-android/build.gradle.kts
@@ -124,6 +124,7 @@ android {
}

dependencies {
api(project(":library:dd-sdk-android-session-replay"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it shouldn't be here, core module shouldn't depend on the feature module

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following the current architecture I was not able to do it differently. I need to be able to inject the SessionReplayActivityLifecycleHook from the SessionReplayFeature. If you want we can have a chat about this maybe we can find a solution to do it differently.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is that later the dependency will change (will be the other way around) once we provide the SDK V2 feature initialization. Have in mind that besides this code all the other Session Replay code will leave in the session replay module and will be easy for us to decouple later.

dd-sdk-android/src/main/kotlin/com/datadog/android/plugin/Feature.kt Outdated
@@ -13,5 +13,6 @@ enum class Feature(internal val featureName: String) {
LOG("Logging"),
CRASH("Crash Reporting"),
TRACE("Tracing"),
RUM("RUM")
RUM("RUM"),
SESSION_REPLAY(featureName = "Session Replay")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
SESSION_REPLAY(featureName = "Session Replay")
SESSION_REPLAY("Session Replay")

@mariusc83 mariusc83 force-pushed the mconstantin/rumm-2267/session-replay-public-api branch from b40943b to 97761af Compare July 7, 2022 15:04
@mariusc83 mariusc83 merged commit ef40e6c into feature/session-replay-vo Jul 11, 2022
@mariusc83 mariusc83 deleted the mconstantin/rumm-2267/session-replay-public-api branch July 11, 2022 07:52
@xgouchet xgouchet added this to the internal milestone Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hestonhoffman hestonhoffman hestonhoffman approved these changes

@0xnm 0xnm 0xnm approved these changes

Assignees

@mariusc83 mariusc83

Labels
None yet
Projects
None yet
Milestone
internal
Development

Successfully merging this pull request may close these issues.
4 participants
@mariusc83 @0xnm @hestonhoffman @xgouchet