Introduce _RumInternalProxy.setRumSessionTypeOverride#2776
Conversation
33c11be to
3b18911
Compare
| } else { | ||
| ActionEvent.ActionEventSessionType.SYNTHETICS | ||
|
|
||
| val sessionType = when { |
There was a problem hiding this comment.
If we have rumSessionType use it. If we are inside a synthetic test, use sessionType=SYNTHETICS, otherwise sessionType=USER
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2776 +/- ##
===========================================
- Coverage 69.97% 69.83% -0.14%
===========================================
Files 824 826 +2
Lines 30881 30930 +49
Branches 5195 5207 +12
===========================================
- Hits 21606 21597 -9
- Misses 7828 7858 +30
- Partials 1447 1475 +28
🚀 New features to boost your workflow:
|
ef249ff to
fa65f79
Compare
0xnm
left a comment
0xnm
left a comment
There was a problem hiding this comment.
looks good 👍
since we have many lines changed here, it would be nice to have one more review.
| val additionalConfig: Map<String, Any>, | ||
| val trackAnonymousUser: Boolean | ||
| val trackAnonymousUser: Boolean, | ||
| val rumSessionType: RumSessionType? |
There was a problem hiding this comment.
I'm wondering if we should call this something like rumSessionTypeOverride, because we will have set it up from the intent attributes and this value is just what overrides the normal flow. Or maybe add some comment.
Although in the symmetric PR for iOS SDK suffix Override is used only in configuration https://github.com/DataDog/dd-sdk-ios/pull/2383/files.
There was a problem hiding this comment.
I'm wondering if we should call this something like rumSessionTypeOverride
done
Or maybe add some comment.
I added a comment here
Although in the symmetric PR for iOS SDK suffix Override is used only in configuration
Looks like on iOS the situation is slightly different, they are able to determine final rumSessionType in one place and propagate the value to all rum events link. So it is not "override", it is the final one.
On Android we create first RUM scopes (RumApplicationScope for example) before this is called and account for the synthetic test attributes only when the events are created (for example like this). So for us is is kind of "override" indeed.
fa65f79 to
04acd57
Compare
aleksandr-gringauz
changed the title
4 participants
What does this PR do?
We want to be able to force
sessionType=userfor sessions that happen in synthetic tests.Introducing internal api
_RumInternalProxy.setRumSessionTypethat setssessionTypethat will be propagated to all RUM events regardless of whether we have a synthetic test or not.The pr is big, but most of the changes are related to using named arguments in various places. For example this constructor has a lot of arguments of primitive types (
bool,long), one can easily make mistake in order of the items. Also some of them have default values and if I create a new argument in this constructor and place it in the end, I have to provide the values for previous arguments manually.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)