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

RUMM-1322 Allow initializing DDURLSessionDelegate before starting the SDK #483

Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions Datadog/Datadog.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,6 @@
6193DCCE251B6201009B8011 /* RUMTASScreen1ViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6193DCCD251B6201009B8011 /* RUMTASScreen1ViewController.swift */; };
6193DCE1251B692C009B8011 /* RUMTASTableViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6193DCE0251B692C009B8011 /* RUMTASTableViewController.swift */; };
6193DCE8251B9AB1009B8011 /* RUMTASCollectionViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6193DCE7251B9AB1009B8011 /* RUMTASCollectionViewController.swift */; };
61940C7C25668EC600A20043 /* URLSessionInterceptionHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61940C7B25668EC600A20043 /* URLSessionInterceptionHandler.swift */; };
6198D27124C6E3B700493501 /* RUMViewScopeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6198D27024C6E3B700493501 /* RUMViewScopeTests.swift */; };
619E16D82577C1CB00B2516B /* DataProcessor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 619E16D72577C1CB00B2516B /* DataProcessor.swift */; };
619E16E92578E73E00B2516B /* DataMigrator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 619E16E82578E73E00B2516B /* DataMigrator.swift */; };
Expand Down Expand Up @@ -804,7 +803,6 @@
6193DCCD251B6201009B8011 /* RUMTASScreen1ViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RUMTASScreen1ViewController.swift; sourceTree = "<group>"; };
6193DCE0251B692C009B8011 /* RUMTASTableViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RUMTASTableViewController.swift; sourceTree = "<group>"; };
6193DCE7251B9AB1009B8011 /* RUMTASCollectionViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RUMTASCollectionViewController.swift; sourceTree = "<group>"; };
61940C7B25668EC600A20043 /* URLSessionInterceptionHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = URLSessionInterceptionHandler.swift; sourceTree = "<group>"; };
6198D27024C6E3B700493501 /* RUMViewScopeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RUMViewScopeTests.swift; sourceTree = "<group>"; };
619E16D72577C1CB00B2516B /* DataProcessor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataProcessor.swift; sourceTree = "<group>"; };
619E16E82578E73E00B2516B /* DataMigrator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataMigrator.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1903,7 +1901,6 @@
children = (
613F23DC252B05BD006CD2D7 /* URLFiltering */,
618E1337252340810098C6B0 /* URLSessionInterceptor.swift */,
61940C7B25668EC600A20043 /* URLSessionInterceptionHandler.swift */,
61417DC52525CDDE00E2D55C /* TaskInterception.swift */,
);
path = Interception;
Expand Down Expand Up @@ -3293,7 +3290,6 @@
618715F924DC13A100FC0F69 /* RUMDataModelsMapping.swift in Sources */,
61C3638524361E9200C4D4E6 /* Globals.swift in Sources */,
E1D202EA24C065CF00D1AF3A /* ActiveSpansPool.swift in Sources */,
61940C7C25668EC600A20043 /* URLSessionInterceptionHandler.swift in Sources */,
9ED6A6B425F2901800CB2E29 /* AppStateListener.swift in Sources */,
61F3CDA3251118FB00C816E5 /* UIKitRUMViewsHandler.swift in Sources */,
61C5A88824509A0C00DA608C /* Warnings.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ - (void)viewDidLoad {

self.testScenario = SwiftGlobals.currentTestScenario;

self.session = [self.testScenario buildURLSession];
self.session = [self.testScenario getURLSession];
assert(self.testScenario != nil);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ @implementation ObjcSendThirdPartyRequestsViewController
- (void)viewDidLoad {
[super viewDidLoad];
self.testScenario = SwiftGlobals.currentTestScenario;
self.session = [self.testScenario buildURLSession];
self.session = [self.testScenario getURLSession];
assert(self.testScenario != nil);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import Datadog

internal class SendFirstPartyRequestsViewController: UIViewController {
private var testScenario: URLSessionBaseScenario!
private lazy var session = testScenario.buildURLSession()
private lazy var session = testScenario.getURLSession()

override func viewDidLoad() {
super.viewDidLoad()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import Datadog

internal class SendThirdPartyRequestsViewController: UIViewController {
private var testScenario: URLSessionBaseScenario!
private lazy var session = testScenario.buildURLSession()
private lazy var session = testScenario.getURLSession()

override func viewDidLoad() {
super.viewDidLoad()
Expand Down
36 changes: 31 additions & 5 deletions Datadog/Example/Scenarios/URLSession/URLSessionScenarios.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,13 @@ import Datadog
/// calls third party endpoints.
@objc
class URLSessionBaseScenario: NSObject {
/// If yes, instrumented endpoints are passed to `DDURLSessionDelegate`; otherwise, they are passed to `DatadogConfiguration.trackURLSession` method
@objc
let feedAdditionalFirstyPartyHosts: Bool
/// Randomizes the way of passing additional first party hosts.
/// If `true`, instrumented endpoints are passed to `DDURLSessionDelegate`; otherwise, they are passed to `DatadogConfiguration.trackURLSession(...)`.
private let feedAdditionalFirstyPartyHosts: Bool

/// Randomizes the way of creating `URLSession` instrumented with `DDURLSessionDelegate`.
/// If `true`, the session is created after `Datadog.initialize()`; if `false`, it's created before.
private let lazyInitRLSession: Bool
ncreated marked this conversation as resolved.
Show resolved Hide resolved

/// The URL to custom GET resource, observed by Tracing auto instrumentation.
@objc
Expand All @@ -38,8 +42,13 @@ class URLSessionBaseScenario: NSObject {
@objc
let thirdPartyURL: URL

/// Randomized value determining if the `DDURLSessionDelegate` should be initialized before (`false)` or after `Datadog.initialize()` (`true`).
private var ddURLSessionDelegate: DDURLSessionDelegate?

override init() {
feedAdditionalFirstyPartyHosts = Bool.random()
feedAdditionalFirstyPartyHosts = .random()
lazyInitRLSession = .random()

if ProcessInfo.processInfo.arguments.contains("IS_RUNNING_UI_TESTS") {
let serverMockConfiguration = Environment.serverMockConfiguration()!
customGETResourceURL = serverMockConfiguration.instrumentedEndpoints[0]
Expand Down Expand Up @@ -71,6 +80,13 @@ class URLSessionBaseScenario: NSObject {
return request
}()
}
super.init()

if lazyInitRLSession {
self.session = nil // it will be created on lazily, on first access from VC
} else {
self.session = createInstrumentedURLSession()
}
}

func configureSDK(builder: Datadog.Configuration.Builder) {
Expand All @@ -83,8 +99,18 @@ class URLSessionBaseScenario: NSObject {
}
}

private var session: URLSession!

@objc
func buildURLSession() -> URLSession {
func getURLSession() -> URLSession {
if session == nil {
precondition(lazyInitRLSession, "The session is unavailable, but it is not configured for lazy init")
session = createInstrumentedURLSession()
}
return session
}

private func createInstrumentedURLSession() -> URLSession {
let delegate: DDURLSessionDelegate
if feedAdditionalFirstyPartyHosts {
delegate = DDURLSessionDelegate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,44 +28,22 @@ open class DDURLSessionDelegate: NSObject, URLSessionTaskDelegate, URLSessionDat
return self
}

var interceptor: URLSessionInterceptorType?
let firstPartyURLsFilter: FirstPartyURLsFilter?
internal var interceptor: URLSessionInterceptorType? { URLSessionAutoInstrumentation.instance?.interceptor }
private(set) var firstPartyURLsFilter: FirstPartyURLsFilter?
ncreated marked this conversation as resolved.
Show resolved Hide resolved

@objc
override public init() {
Self.datadogInitializationCheck()
firstPartyURLsFilter = nil
interceptor = URLSessionAutoInstrumentation.instance?.interceptor
}

/// Automatically tracked hosts can be customized per instance with this initializer
/// - Parameter additionalFirstPartyHosts: these hosts are tracked **in addition to** what was
/// passed to `DatadogConfiguration.Builder` via `trackURLSession(firstPartyHosts:)`
///
/// **NOTE:** If `trackURLSession(firstPartyHosts:)` is never called, automatic tracking will **not** take place
@objc
public init(additionalFirstPartyHosts: Set<String>) {
// NOTE: RUMM-954 copy&pasting `init()` is a conscious decision.
// otherwise `DDURLSessionDelegateAsSuperclassTests` fails.
// if `init()` was made convenience and call the designated `init` below,
// that would result in potential breaking changes.
// host projects would need to change their `init()`s in subclasses.
// we can fix this in v2.0
Self.datadogInitializationCheck()
firstPartyURLsFilter = FirstPartyURLsFilter(hosts: additionalFirstPartyHosts)
interceptor = URLSessionAutoInstrumentation.instance?.interceptor
}

private static func datadogInitializationCheck() {
if URLSessionAutoInstrumentation.instance?.interceptor == nil {
let error = ProgrammerError(
description: """
`Datadog.initialize()` must be called before initializing the `DDURLSessionDelegate` and
first party hosts must be specified in `Datadog.Configuration`: `trackURLSession(firstPartyHosts:)`
to enable network requests tracking.
"""
)
consolePrint("\(error)")
}
}

open func urlSession(_ session: URLSession, task: URLSessionTask, didFinishCollecting metrics: URLSessionTaskMetrics) {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,29 @@

import Foundation

/// An interface for handling `URLSession` interceptions start and completion.
internal protocol URLSessionInterceptionHandler {
/// Notifies the `URLSessionTask` interception start.
func notify_taskInterceptionStarted(interception: TaskInterception)
/// Notifies the `URLSessionTask` interception completion.
func notify_taskInterceptionCompleted(interception: TaskInterception)
}

/// An interface for processing `URLSession` task interceptions.
internal protocol URLSessionInterceptorType: class {
func modify(request: URLRequest, session: URLSession?) -> URLRequest
func taskCreated(task: URLSessionTask, session: URLSession?)
func taskMetricsCollected(task: URLSessionTask, metrics: URLSessionTaskMetrics)
func taskReceivedData(task: URLSessionTask, data: Data)
func taskCompleted(task: URLSessionTask, error: Error?)

var handler: URLSessionInterceptionHandler { get }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 This adds a bit noise to the URLSessionInterceptorType interface and could be additionally mitigated by decoupling the handler creation to upstream (to URLSessionAutoInstrumentation class). This however seems over engineering if done in the scope of this PR - we may want to refactor this on a stronger reason with more constraints for changing the abstraction.

}

/// An object performing interception of requests sent with `URLSession`.
public class URLSessionInterceptor: URLSessionInterceptorType {
public static var shared: URLSessionInterceptor? {
URLSessionAutoInstrumentation.instance?.interceptor
URLSessionAutoInstrumentation.instance?.interceptor as? URLSessionInterceptor
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 This casting is now required and is tested in existing URLSessionAutoInstrumentationTests tests.

}

/// Filters first party `URLs` defined by the user.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,22 @@ internal class URLSessionAutoInstrumentation {
static var instance: URLSessionAutoInstrumentation?

let swizzler: URLSessionSwizzler
let interceptor: URLSessionInterceptor
let interceptor: URLSessionInterceptorType

init?(
convenience init?(
configuration: FeaturesConfiguration.URLSessionAutoInstrumentation,
dateProvider: DateProvider,
appStateListener: AppStateListening
) {
do {
self.interceptor = URLSessionInterceptor(
configuration: configuration,
dateProvider: dateProvider,
appStateListener: appStateListener
self.init(
swizzler: try URLSessionSwizzler(),
interceptor: URLSessionInterceptor(
configuration: configuration,
dateProvider: dateProvider,
appStateListener: appStateListener
)
)
self.swizzler = try URLSessionSwizzler()
} catch {
consolePrint(
"🔥 Datadog SDK error: automatic tracking of `URLSession` requests can't be set up due to error: \(error)"
Expand All @@ -33,6 +35,11 @@ internal class URLSessionAutoInstrumentation {
}
}

init(swizzler: URLSessionSwizzler, interceptor: URLSessionInterceptorType) {
self.swizzler = swizzler
self.interceptor = interceptor
}

func enable() {
swizzler.swizzle()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,26 @@ class URLSessionInterceptorMock: URLSessionInterceptorType {
taskMetrics.append((task: task, metrics: metrics))
onTaskMetricsCollected?(task, metrics)
}

let handler: URLSessionInterceptionHandler = URLSessionInterceptionHandlerMock()
}

class URLSessionInterceptionHandlerMock: URLSessionInterceptionHandler {
var didNotifyInterceptionStart: ((TaskInterception) -> Void)?
var startedInterceptions: [TaskInterception] = []

func notify_taskInterceptionStarted(interception: TaskInterception) {
startedInterceptions.append(interception)
didNotifyInterceptionStart?(interception)
}

var didNotifyInterceptionCompletion: ((TaskInterception) -> Void)?
var completedInterceptions: [TaskInterception] = []

func notify_taskInterceptionCompleted(interception: TaskInterception) {
completedInterceptions.append(interception)
didNotifyInterceptionCompletion?(interception)
}
}

extension ResourceCompletion {
Expand Down
Loading