Skip to content

RUMM-2324: Let user to disable RUM view tracking strategy#981

Merged
0xnm merged 2 commits into
developfrom
nogorodnikov/rumm-2324/let-user-disable-view-tracking-strategy
Jul 13, 2022
Merged

RUMM-2324: Let user to disable RUM view tracking strategy#981
0xnm merged 2 commits into
developfrom
nogorodnikov/rumm-2324/let-user-disable-view-tracking-strategy

Conversation

@0xnm

@0xnm 0xnm commented Jul 12, 2022

Copy link
Copy Markdown
Member

What does this PR do?

This change allows user to disable the default view tracking strategy if there is a need/wish to do view tracking manually by using RumMonitor on the user side.

This is achieved by letting call Configuration#useViewTrackingStrategy with null argument, which will make SDK use NoOpViewTrackingStrategy.

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)

@0xnm
0xnm requested a review from a team as a code owner July 12, 2022 12:02
Comment thread dd-sdk-android/apiSurface
fun trackInteractions(Array<com.datadog.android.rum.tracking.ViewAttributesProvider> = emptyArray(), com.datadog.android.rum.tracking.InteractionPredicate = NoOpInteractionPredicate()): Builder
fun trackLongTasks(Long = DEFAULT_LONG_TASK_THRESHOLD_MS): Builder
fun useViewTrackingStrategy(com.datadog.android.rum.tracking.ViewTrackingStrategy): Builder
fun useViewTrackingStrategy(com.datadog.android.rum.tracking.ViewTrackingStrategy?): Builder

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.

should we do this or should we have it null by default in the configuration and have this method as before ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think we should have it ActivityViewTrackingStrategy in the configuration by default, to ease onboarding.

@codecov-commenter

codecov-commenter commented Jul 12, 2022

Copy link
Copy Markdown

Codecov Report

Merging #981 (8c3e8f3) into develop (5fb1fdf) will decrease coverage by 0.03%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #981      +/-   ##
===========================================
- Coverage    83.08%   83.05%   -0.03%     
===========================================
  Files          268      268              
  Lines         9213     9213              
  Branches      1484     1484              
===========================================
- Hits          7654     7651       -3     
- Misses        1151     1153       +2     
- Partials       408      409       +1     
Impacted Files Coverage Δ
...atadog/android/core/configuration/Configuration.kt 98.68% <ø> (ø)
...android/log/internal/logger/TelemetryLogHandler.kt 75.00% <0.00%> (-25.00%) ⬇️
...ndroid/core/internal/persistence/file/EventMeta.kt 80.00% <0.00%> (-10.00%) ⬇️
...droid/rum/tracking/ActivityViewTrackingStrategy.kt 86.54% <0.00%> (-1.92%) ⬇️
...rc/main/java/com/datadog/opentracing/DDTracer.java 55.65% <0.00%> (-0.42%) ⬇️
...android/rum/internal/ndk/DatadogNdkCrashHandler.kt 88.04% <0.00%> (ø)
...droid/rum/tracking/FragmentViewTrackingStrategy.kt 86.96% <0.00%> (+4.35%) ⬆️

@xgouchet xgouchet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@0xnm
0xnm merged commit 774b2a4 into develop Jul 13, 2022
@0xnm
0xnm deleted the nogorodnikov/rumm-2324/let-user-disable-view-tracking-strategy branch July 13, 2022 07:05
@xgouchet xgouchet added this to the 1.14.0 milestone Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants