-
Notifications
You must be signed in to change notification settings - Fork 135
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-1785 DatadogEventBridge implemented #674
RUMM-1785 DatadogEventBridge implemented #674
Conversation
f42703d
to
afd46de
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.
I left few questions, about the architecture plan (vs upcoming V2 efforts), threading and testing.
IMO this can't be single-threaded feature, as Browser events are equally complex to mobile events and we should process them on a background queue.
Sources/Datadog/RUM/WebView/WKUserContentController+Datadog.swift
Outdated
Show resolved
Hide resolved
Sources/Datadog/RUM/WebView/WKUserContentController+Datadog.swift
Outdated
Show resolved
Hide resolved
Tests/DatadogTests/Datadog/RUM/WebView/DatadogEventBridgeTests.swift
Outdated
Show resolved
Hide resolved
Tests/DatadogTests/Datadog/RUM/WebView/DatadogEventBridgeTests.swift
Outdated
Show resolved
Hide resolved
|
||
init(eventBridge: DatadogEventBridge) { | ||
self.eventBridge = eventBridge | ||
} |
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.
What will be the lifecycle of DatadogMessageHandler
? As I understand, it will be created with public API similar to:
controller.addDatadogMessageHandler(allowedWebViewHosts: ["datadoghq.dev"])
and destroyed when? I'm trying to guess the lifecycle of underlying queue
- assuming it will be destroyed when WebView disappears, it should be fine.
Sources/Datadog/FeaturesIntegration/WebView/DatadogEventBridge.swift
Outdated
Show resolved
Hide resolved
Sources/Datadog/FeaturesIntegration/WebView/WKUserContentController+Datadog.swift
Outdated
Show resolved
Hide resolved
Sources/Datadog/FeaturesIntegration/WebView/WKUserContentController+Datadog.swift
Show resolved
Hide resolved
Sources/Datadog/FeaturesIntegration/WebView/DatadogEventBridge.swift
Outdated
Show resolved
Hide resolved
Sources/Datadog/FeaturesIntegration/WebView/WKUserContentController+Datadog.swift
Outdated
Show resolved
Hide resolved
HostsSanitizer refactored out and shared
What and why?
As part of
WKWebView
tracking, Datadog iOS SDK needs to communicate with Datadog Browser SDK.How?
DatadogEventBridge
is implemented as a mean of communication between those two platforms.It receives messages from Browser SDK, parses them into JSON and forwards to corresponding
WebEventConsumer
(Log
orRUM
).NOTE
Also, an extension of
WKUserContentController
is added in order to injectDatadogEventBridge
into theWKWebView
. However, I can't find a way to test this extension in unit tests. I triedwebView.evaluateJavaScript(...)
API to call JS methods within the webview, yet that didn't work in unit tests (although it works inExample
app).We may need to test this extension in integration tests.
Review checklist