-
Notifications
You must be signed in to change notification settings - Fork 133
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
RUMM-1265 Correctly handle RUM events when app in background #504
RUMM-1265 Correctly handle RUM events when app in background #504
Conversation
e1367e3
to
d11bb58
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very good 💪! I left few convention feedbacks.
- Also, please update the PR description 🙏, especially on "how" - as we implement a very custom way of determining if the app is in background (notably, without the app's life cycle events) I think it's worth explaining it for future references.
static let RUM_BACKGROUND_VIEW_URL = "com/datadog/background/view" | ||
static let RUM_BACKGROUND_VIEW_NAME = "Background" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✏️ We don't use capitals for defining constants in Swift. Also, in dd-sdk-ios
we adopt the convention of grouping constants in Constants
struct acting as a namespace:
internal class RUMViewScope: RUMScope, RUMContextProvider {
struct Constants {
static let backgroundViewURL = "com/datadog/background/view"
static let backgroundViewName = "Background"
}
// ...
}
XCTAssertEqual(scope.viewScopes[0].viewPath, RUMViewScope.RUM_BACKGROUND_VIEW_URL) | ||
} | ||
|
||
func testWhenActiveViewScope_andReceivingStartCommand_doesNotCreateNewViewScope() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✏️ missing "it":
func testWhenActiveViewScope_andReceivingStartCommand_doesNotCreateNewViewScope() { | |
func testWhenActiveViewScope_andReceivingStartCommand_itDoesNotCreateNewViewScope() { |
XCTAssertEqual(scope.viewScopes.count, 1) | ||
} | ||
|
||
func testWhenNoActiveViewScope_andReceivingNotValidStartCommand_doesNotCreateNewViewScope() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✏️ missing "it":
func testWhenNoActiveViewScope_andReceivingNotValidStartCommand_doesNotCreateNewViewScope() { | |
func testWhenNoActiveViewScope_andReceivingNotValidStartCommand_itDoesNotCreateNewViewScope() { |
|
||
let scope = RUMSessionScope(parent: parent, dependencies: .mockAny(), samplingRate: 100, startTime: Date()) | ||
_ = scope.process(command: RUMStartViewCommand.mockAny()) | ||
_ = scope.process(command: RUMAddUserActionCommand.mockWith(time: currentTime)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want to call generateRandomValidStartCommand()
here, no?
d11bb58
to
6459555
Compare
6459555
to
6e9aaf8
Compare
💯 👌 |
What and why?
Currently if the application is not in foreground (there is no active View scope), any resource or action event will be ignored as there is no active
View
on which to attach it.In this PR we are adding a change in the way we handle the received commands in the
RUMSessionScope
in order to be able to record RUM events while the application is in background.How?
We are introducing the concept of
RUM background View scope
which by definition is a normal View scope with predefinedname
andurl
. Once theRUMSessionScope
needs to handle aRUMStartAction
orRUMStartResource
command and there is no activeRUMViewScope
, instead of ignoring the command as before it will automatically create a specialRUMViewScope
on which the received resource or action will be attached. For more information please check: RUM Background events RFCReview checklist