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.

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

RUMM-2536: Use SDK v2 configuration interfaces for features #1079

Merged

Conversation

0xnm
Copy link
Contributor

@0xnm 0xnm commented Oct 11, 2022

What does this PR do?

This change does the following decouples SdkFeature from RUM, Logs, etc. features. SdkFeature will reside in the core module and won't be exposed as part of the public API, it is responsible for Storage and Uploader creation. RUM, Logs, etc. features will be in the independent modules.

For now they take Storage as a constructor argument, but later on they will only depend on SdkCore and will do the writing operations using Event Write Context via SdkCore#getFeature -> FeatureScope#withWriteContext call. SdkFeature implements FeatureScope.

The idea is that RumFeature, LogsFeature, etc. will be handling data serialization (since it is not the responsibility of the core) and will write the binary data in the EWC, so for their own PersistenceStrategy interface we are leaving DataWriter part only.

DatadogFeature class is removed in favour of SdkFeature.

Features lifecycle API will be decided later.

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)

@codecov-commenter
Copy link

codecov-commenter commented Oct 11, 2022

Codecov Report

Merging #1079 (3b40dae) into feature/sdkv2 (3200133) will decrease coverage by 0.01%.
The diff coverage is 81.77%.

@@                Coverage Diff                @@
##           feature/sdkv2    #1079      +/-   ##
=================================================
- Coverage          82.57%   82.56%   -0.01%     
=================================================
  Files                347      346       -1     
  Lines              11202    11157      -45     
  Branches            1845     1831      -14     
=================================================
- Hits                9249     9211      -38     
- Misses              1378     1383       +5     
+ Partials             575      563      -12     
Impacted Files Coverage Δ
...oid/src/main/kotlin/com/datadog/android/Datadog.kt 71.43% <0.00%> (-1.12%) ⬇️
...istence/file/batch/BatchFilePersistenceStrategy.kt 100.00% <ø> (ø)
...ror/internal/CrashReportFilePersistenceStrategy.kt 100.00% <ø> (ø)
.../src/main/kotlin/com/datadog/android/log/Logger.kt 93.89% <ø> (ø)
.../log/internal/domain/LogFilePersistenceStrategy.kt 100.00% <ø> (ø)
...c/main/kotlin/com/datadog/android/rum/GlobalRum.kt 63.04% <0.00%> (-5.85%) ⬇️
...droid/rum/tracking/FragmentViewTrackingStrategy.kt 78.85% <0.00%> (ø)
...oid/rum/tracking/NavigationViewTrackingStrategy.kt 84.48% <0.00%> (-0.26%) ⬇️
...l/domain/SessionReplayRecordPersistenceStrategy.kt 100.00% <ø> (ø)
...g/internal/domain/TracesFilePersistenceStrategy.kt 100.00% <ø> (ø)
... and 28 more

@0xnm 0xnm marked this pull request as ready for review October 11, 2022 14:27
@0xnm 0xnm requested a review from a team as a code owner October 11, 2022 14:27
@0xnm 0xnm force-pushed the nogorodnikov/rumm-2536/use-v2-configuration-interface branch from f710ed7 to 3b40dae Compare October 11, 2022 14:37
@xgouchet xgouchet added the size-large This PR is large sized label Oct 11, 2022
context: Context,
storage: Storage,
configuration: Configuration.Feature.CrashReport
private fun createPersistenceStrategy(
Copy link
Collaborator

Choose a reason for hiding this comment

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

for now we are still doing the writing through the persistenceStrategy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for now - yes. In the later PRs this will change: interface of DataWriter will change (and it won't be a single interface in the core) and for cross-feature interaction we will have message bus instead of getting the writer of another feature.

@0xnm 0xnm merged commit 946cc7e into feature/sdkv2 Oct 12, 2022
@0xnm 0xnm deleted the nogorodnikov/rumm-2536/use-v2-configuration-interface branch October 12, 2022 08:14
@xgouchet xgouchet added this to the 1.16.0 milestone Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size-large This PR is large sized
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants