Skip to content

RUM-4098: Add TLVFormat DataStore persistence#2038

Merged
jonathanmos merged 14 commits into
developfrom
jmoskovich/rum-4098/add-persistence
Jun 24, 2024
Merged

RUM-4098: Add TLVFormat DataStore persistence#2038
jonathanmos merged 14 commits into
developfrom
jmoskovich/rum-4098/add-persistence

Conversation

@jonathanmos

@jonathanmos jonathanmos commented May 16, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

What does this PR do?

Adds the capability to write/read files in TLVformat, and defines a DataStore type that features can use to persist data between sessions.

Motivation

Part of the effort to persist bitmap hashes between sessions. Integrating the datastore with SR will be in a second PR.

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)

@jonathanmos
jonathanmos force-pushed the jmoskovich/rum-4098/add-persistence branch 6 times, most recently from 410e07f to 4d6dc78 Compare May 20, 2024 13:04
@codecov-commenter

codecov-commenter commented May 21, 2024 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 90.62500% with 24 lines in your changes missing coverage. Please review.

Project coverage is 83.23%. Comparing base (d3f69b6) to head (d25e520).
Report is 3 commits behind head on develop.

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #2038      +/-   ##
===========================================
+ Coverage    83.19%   83.23%   +0.04%     
===========================================
  Files          495      508      +13     
  Lines        17773    18004     +231     
  Branches      2685     2699      +14     
===========================================
+ Hits         14785    14985     +200     
- Misses        2256     2274      +18     
- Partials       732      745      +13
Files Coverage Δ
...in/com/datadog/android/api/feature/FeatureScope.kt 0.00% <ø> (ø)
...in/com/datadog/android/core/internal/SdkFeature.kt 90.86% <100.00%> (+1.76%) ⬆️
...rnal/persistence/datastore/DataStoreFileHandler.kt 100.00% <100.00%> (ø)
...rnal/persistence/datastore/NoOpDataStoreHandler.kt 100.00% <100.00%> (ø)
.../core/internal/persistence/datastore/ext/IntExt.kt 100.00% <100.00%> (ø)
...id/core/internal/persistence/tlvformat/TLVBlock.kt 100.00% <100.00%> (ø)
...ternal/persistence/tlvformat/TLVBlockFileReader.kt 100.00% <100.00%> (ø)
...ore/internal/persistence/tlvformat/TLVBlockType.kt 100.00% <100.00%> (ø)
...oid/core/persistence/datastore/DataStoreContent.kt 100.00% <100.00%> (ø)
...ernal/persistence/datastore/DatastoreFileWriter.kt 97.87% <97.87%> (ø)
... and 5 more

... and 31 files with indirect coverage changes

@jonathanmos
jonathanmos marked this pull request as ready for review May 21, 2024 10:02
@jonathanmos
jonathanmos requested review from a team as code owners May 21, 2024 10:02
mariusc83
mariusc83 reviewed May 21, 2024
View reviewed changes
Comment thread .../main/kotlin/com/datadog/android/core/internal/persistence/datastore/FileDataStoreHandler.kt Outdated
Comment thread .../main/kotlin/com/datadog/android/core/internal/persistence/datastore/FileDataStoreHandler.kt Outdated
Comment thread ...rc/main/kotlin/com/datadog/android/core/internal/persistence/tlvformat/FileTLVBlockReader.kt Outdated
0xnm
0xnm requested changes May 21, 2024
View reviewed changes

@0xnm 0xnm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

good start! I reviewed production files so far, didn't go through the tests yet. I left some questions/suggestions.

Comment thread dd-sdk-android-core/api/apiSurface Outdated
Comment thread dd-sdk-android-core/src/main/kotlin/com/datadog/android/api/feature/FeatureScope.kt Outdated
Comment thread dd-sdk-android-core/src/main/kotlin/com/datadog/android/api/feature/FeatureScope.kt Outdated
Comment thread dd-sdk-android-core/src/main/kotlin/com/datadog/android/api/feature/FeatureScope.kt Outdated
Comment thread dd-sdk-android-core/src/main/kotlin/com/datadog/android/api/feature/FeatureScope.kt Outdated
Comment thread ...rc/main/kotlin/com/datadog/android/core/internal/persistence/tlvformat/FileTLVBlockReader.kt Outdated
Comment thread ...rc/main/kotlin/com/datadog/android/core/internal/persistence/tlvformat/FileTLVBlockReader.kt Outdated
Comment thread ...rc/main/kotlin/com/datadog/android/core/internal/persistence/tlvformat/FileTLVBlockReader.kt Outdated
Comment thread ...rc/main/kotlin/com/datadog/android/core/internal/persistence/tlvformat/FileTLVBlockReader.kt Outdated
Comment thread dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/SdkFeature.kt Outdated
@jonathanmos
jonathanmos force-pushed the jmoskovich/rum-4098/add-persistence branch from 6279225 to 564eb64 Compare May 22, 2024 08:16
mariusc83
mariusc83 requested changes May 22, 2024
View reviewed changes
Comment thread dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/SdkFeature.kt Outdated
Comment thread dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/SdkFeature.kt Outdated
Comment thread .../main/kotlin/com/datadog/android/core/internal/persistence/datastore/FileDataStoreHandler.kt Outdated
@jonathanmos
jonathanmos marked this pull request as draft May 26, 2024 18:50
@jonathanmos
jonathanmos force-pushed the jmoskovich/rum-4098/add-persistence branch 13 times, most recently from 875464b to 459dd96 Compare June 2, 2024 18:04
@jonathanmos
jonathanmos marked this pull request as ready for review June 4, 2024 07:32
@jonathanmos
jonathanmos requested a review from mariusc83 June 4, 2024 07:33
@jonathanmos
jonathanmos requested a review from 0xnm June 4, 2024 07:33
0xnm
0xnm reviewed Jun 5, 2024
View reviewed changes
Comment thread dd-sdk-android-core/src/main/kotlin/com/datadog/android/api/feature/FeatureScope.kt Outdated
Comment thread .../main/kotlin/com/datadog/android/core/internal/persistence/datastore/DataStoreFileHandler.kt Outdated
Comment thread ...c/main/kotlin/com/datadog/android/core/internal/persistence/datastore/DataStoreFileHelper.kt
Comment thread ...c/main/kotlin/com/datadog/android/core/internal/persistence/datastore/DatastoreFileReader.kt Outdated
Comment thread ...c/main/kotlin/com/datadog/android/core/internal/persistence/datastore/DatastoreFileReader.kt Outdated
Comment thread detekt_custom.yml Outdated
Comment thread dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/SdkFeatureTest.kt Outdated
Comment thread dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/SdkFeatureTest.kt Outdated
Comment thread ...oid-core/src/main/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlock.kt
Comment thread ...oid-core/src/main/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlock.kt Outdated
@jonathanmos
jonathanmos force-pushed the jmoskovich/rum-4098/add-persistence branch from 05dcf33 to 1c3f646 Compare June 5, 2024 12:16
@jonathanmos
jonathanmos requested a review from 0xnm June 6, 2024 09:06
mariusc83
mariusc83 approved these changes Jun 6, 2024
View reviewed changes

@mariusc83 mariusc83 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM !!

0xnm
0xnm requested changes Jun 19, 2024
View reviewed changes
Comment thread dd-sdk-android-core/src/main/kotlin/com/datadog/android/api/feature/FeatureScope.kt Outdated
Comment thread dd-sdk-android-core/src/main/kotlin/com/datadog/android/api/feature/FeatureScope.kt Outdated
Comment thread ...-android-core/src/main/kotlin/com/datadog/android/api/storage/datastore/DataStoreCallback.kt Outdated
Comment thread ...-android-core/src/main/kotlin/com/datadog/android/api/storage/datastore/DataStoreCallback.kt
Comment thread ...-android-core/src/main/kotlin/com/datadog/android/api/storage/datastore/DataStoreCallback.kt Outdated
Comment thread ...est/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockFileReaderTest.kt Outdated
Comment thread ...est/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockFileReaderTest.kt Outdated
Comment thread ...core/src/test/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockTest.kt Outdated
Comment thread .../src/test/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockTypeTest.kt Outdated
Comment thread detekt_custom.yml
@jonathanmos
jonathanmos requested a review from 0xnm June 20, 2024 07:33
0xnm
0xnm approved these changes Jun 20, 2024
View reviewed changes
Comment thread ...tlin/com/datadog/android/core/internal/persistence/file/datastore/DataStoreFileReaderTest.kt Outdated
Comment thread ...est/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockFileReaderTest.kt Outdated
Comment thread ...est/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockFileReaderTest.kt Outdated
@jonathanmos
jonathanmos force-pushed the jmoskovich/rum-4098/add-persistence branch from ba49a80 to cc27469 Compare June 20, 2024 14:10
@jonathanmos
jonathanmos merged commit 29692c8 into develop Jun 24, 2024
@jonathanmos
jonathanmos deleted the jmoskovich/rum-4098/add-persistence branch June 24, 2024 14:03
@xgouchet xgouchet added this to the 2.12.x milestone Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mariusc83 mariusc83 mariusc83 approved these changes
@0xnm 0xnm 0xnm approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2.12.x

Development

Successfully merging this pull request may close these issues.

5 participants

@jonathanmos @codecov-commenter @mariusc83 @0xnm @xgouchet