Skip to content

RUM-3669 avoid feature flag spamming events#1932

Merged
xgouchet merged 3 commits into
release/2.7.0from
xgouchet/RUM-3669/feature_flag_batching
Mar 22, 2024
Merged

RUM-3669 avoid feature flag spamming events#1932
xgouchet merged 3 commits into
release/2.7.0from
xgouchet/RUM-3669/feature_flag_batching

Conversation

@xgouchet

Copy link
Copy Markdown
Contributor

What does this PR do?

This PR attempts to improve the performance linked to setting FeatureFlag evaluations on a RUM View. Namely:

  • create an addFeatureFlagBatchEvaluation() method allowing to send a batch of feature flags instead of sending them once at a time. This limits the number of event the Rum View Scope has to process, and the number of view updates written on disk.
  • add a check in the RumViewScope handling the feature flags updates: instead of writing an event update for every call to the addFeatureFlagEvaluation() and addFeatureFlagBatchEvaluation() methods, it only writes an event if at least one feature flag value was changed.

Motivation

For some customer updating many feature flags at once, that would result in a lot of RawRumEvents being processed, and a lot of RUM View updates being written on disk. In some extreme cases it could lead to a lot of backpressure on the executor queues.

@xgouchet
xgouchet requested review from a team as code owners March 21, 2024 20:14
@xgouchet
xgouchet requested a review from ncreated March 21, 2024 20:16
@codecov-commenter

codecov-commenter commented Mar 22, 2024

Copy link
Copy Markdown

Codecov Report

Merging #1932 (8f43597) into release/2.7.0 (9d7e3f9) will decrease coverage by 0.20%.
The diff coverage is 100.00%.

Additional details and impacted files
@@                Coverage Diff                @@
##           release/2.7.0    #1932      +/-   ##
=================================================
- Coverage          83.56%   83.36%   -0.20%     
=================================================
  Files                481      481              
  Lines              17413    17425      +12     
  Branches            2590     2595       +5     
=================================================
- Hits               14550    14526      -24     
- Misses              2177     2186       +9     
- Partials             686      713      +27     
Files Coverage Δ
.../main/kotlin/com/datadog/android/rum/RumMonitor.kt 28.57% <ø> (ø)
...g/android/rum/internal/domain/scope/RumRawEvent.kt 100.00% <100.00%> (ø)
.../android/rum/internal/domain/scope/RumViewScope.kt 93.47% <100.00%> (-0.48%) ⬇️
.../android/rum/internal/monitor/DatadogRumMonitor.kt 85.35% <100.00%> (+0.09%) ⬆️

... and 18 files with indirect coverage changes

0xnm
0xnm previously approved these changes Mar 22, 2024

@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.

I'm ok with the change, we just need to define its shape in terms of the pubilc API and align with iOS, I left an idea regarding that.

Comment thread features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/RumMonitor.kt Outdated
ncreated
ncreated previously approved these changes Mar 22, 2024

@ncreated ncreated 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.

Looks good 👍. Minor suggestion on API naming, non-blocking.

Comment thread features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/RumMonitor.kt Outdated
@xgouchet
xgouchet dismissed stale reviews from ncreated and 0xnm via d9db73b March 22, 2024 09:26
@xgouchet
xgouchet requested review from 0xnm and ncreated March 22, 2024 09:26
0xnm
0xnm previously approved these changes Mar 22, 2024

@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.

lgtm. regarding the release, I think we will need to release 2.8.0 with buildId support soon (maybe in 2 weeks?), so maybe this change can be shipped there instead of hotfix.

@xgouchet
xgouchet merged commit d9b9645 into release/2.7.0 Mar 22, 2024
@xgouchet
xgouchet deleted the xgouchet/RUM-3669/feature_flag_batching branch March 22, 2024 14:05
@xgouchet xgouchet added this to the 2.7.x milestone Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants