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

RUM-3669 avoid feature flag spamming events #1932

Merged
merged 3 commits into from
Mar 22, 2024

Conversation

xgouchet
Copy link
Member

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
Copy link

codecov-commenter commented Mar 22, 2024

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
Copy link
Member

@0xnm 0xnm left a comment

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.

ncreated
ncreated previously approved these changes Mar 22, 2024
Copy link
Member

@ncreated ncreated left a comment

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.

@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
Copy link
Member

@0xnm 0xnm left a comment

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
23 checks passed
@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