[flags] FFL-1113: RUM Evaluation Tracking#2926
Conversation
e904a4b to
8263dbf
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feature/feature-flagging #2926 +/- ##
============================================================
- Coverage 70.76% 70.75% -0.02%
============================================================
Files 835 837 +2
Lines 30388 30430 +42
Branches 5132 5135 +3
============================================================
+ Hits 21504 21528 +24
- Misses 7431 7439 +8
- Partials 1453 1463 +10
🚀 New features to boost your workflow:
|
2697635 to
ba4a606
Compare
jonathanmos
changed the title
2f7a86c to
82fe14a
Compare
jonathanmos
changed the title
| * This is enabled by default. | ||
| * @param enabled whether flag evaluation events are logged with RUM. | ||
| */ | ||
| fun rumIntegrationEnabled(enabled: Boolean): Builder { |
There was a problem hiding this comment.
I think we need to check this with iOS if it should be rumIntegrationEnabled or enableRumIntegration.
There was a problem hiding this comment.
enableRumIntegration would match the active voice of the other builder methods, ex: useCustom..., trackExposures.
My vote is for consistency across the SDKs first, but preferably there is also internal consistency
| * @param flagKey name of the flag. | ||
| * @param value value of the flag. | ||
| */ | ||
| data class RumFlagEvaluationMessage( |
There was a problem hiding this comment.
minor: if we expect value to be only one of the given types (like String, Int, Double) we can use sealed class here.
There was a problem hiding this comment.
I'll make a note to do this, but will defer for a future pr
82fe14a to
01ccba9
Compare
01ccba9 to
fafab47
Compare
typotter
left a comment
typotter
left a comment
There was a problem hiding this comment.
lgtm. If we concur on logging to RUM when the default is returned, we can make that change in a followup
| * This is enabled by default. | ||
| * @param enabled whether flag evaluation events are logged with RUM. | ||
| */ | ||
| fun rumIntegrationEnabled(enabled: Boolean): Builder { |
There was a problem hiding this comment.
enableRumIntegration would match the active voice of the other builder methods, ex: useCustom..., trackExposures.
My vote is for consistency across the SDKs first, but preferably there is also internal consistency
| } else { | ||
| if (flagsConfiguration.trackExposures) { |
There was a problem hiding this comment.
could this be } else if ... {
4 participants
What does this PR do?
• Evaluation tracking through RUM
This is controlled by the
rumIntegrationEnabledflag inFlagsConfiguration(enabled by default). No deduplication is performed for these requests.Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
Review checklist (to be filled by reviewers)