Skip to content

Commit 866d2a3

Browse files
buranmertburanmert
committed
RUMM-1064 PR comments addressed
1 parent 857edc6 commit 866d2a3

File tree

14 files changed

+74
-118
lines changed

14 files changed

+74
-118
lines changed

‎Sources/Datadog/Core/System/AppStateListener.swift

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,13 @@ import class UIKit.UIApplication
1010
/// A data structure to represent recorded app states in a given period of time
1111
internal struct AppStateHistory: Equatable {
1212
/// Snapshot of the app state at `date`
13-
struct Snapshot: Equatable, Comparable {
13+
struct Snapshot: Equatable {
1414
let isActive: Bool
1515
let date: Date
16-
17-
static func < (lhs: Snapshot, rhs: Snapshot) -> Bool {
18-
return lhs.date < rhs.date
19-
}
2016
}
2117

2218
var initialState: Snapshot
23-
private(set) var changes = [Snapshot]()
24-
// NOTE: RUMM-1064 changes.last.date > finalDate case isn't handled as not realistic
19+
var changes = [Snapshot]()
2520
var finalDate: Date
2621
var finalState: Snapshot {
2722
return Snapshot(
@@ -30,11 +25,6 @@ internal struct AppStateHistory: Equatable {
3025
)
3126
}
3227

33-
mutating func add(change: Snapshot) {
34-
changes.append(change)
35-
changes.sort()
36-
}
37-
3828
/// Limits or extrapolates app state history to the given range
3929
/// This is useful when you record between 0...3t but you are concerned of t...2t only
4030
/// - Parameter range: if outside of `initialState` and `finalState`, it extrapolates; otherwise it limits
@@ -82,14 +72,14 @@ internal struct AppStateHistory: Equatable {
8272
// and no change after final state
8373
return finalState.isActive
8474
}
85-
var active = initialState.isActive
75+
var active = initialState
8676
for change in changes {
8777
if date < change.date {
8878
break
8979
}
90-
active = change.isActive
80+
active = change
9181
}
92-
return active
82+
return active.isActive
9383
}
9484
}
9585

@@ -134,15 +124,15 @@ internal class AppStateListener: AppStateListening {
134124
@objc
135125
private func appWillResignActive() {
136126
let now = dateProvider.currentDate()
137-
publisher.mutateAsync {
138-
$0.add(change: Snapshot(isActive: false, date: now))
139-
}
127+
var value = publisher.currentValue
128+
value.changes.append(Snapshot(isActive: false, date: now))
129+
publisher.publishAsync(value)
140130
}
141131
@objc
142132
private func appDidBecomeActive() {
143133
let now = dateProvider.currentDate()
144-
publisher.mutateAsync {
145-
$0.add(change: Snapshot(isActive: true, date: now))
146-
}
134+
var value = publisher.currentValue
135+
value.changes.append(Snapshot(isActive: true, date: now))
136+
publisher.publishAsync(value)
147137
}
148138
}

‎Sources/Datadog/Core/Utils/ValuePublisher.swift

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,6 @@ internal class ValuePublisher<Value> {
6666
concurrentQueue.async(flags: .barrier) { self.unsafeValue = newValue }
6767
}
6868

69-
/// Mutates the `currentValue` asynchronously, without blocking the caller thread.
70-
func mutateAsync(_ block: @escaping (inout Value) -> Void) {
71-
concurrentQueue.async(flags: .barrier) { block(&self.unsafeValue) }
72-
}
73-
7469
/// Registers an observer that will be notified on all value changes.
7570
/// All calls to the `observer` will be synchronised using internal concurrent queue.
7671
func subscribe<Observer: ValueObserver>(_ observer: Observer) where Observer.ObservedValue == Value {

‎Sources/Datadog/Tracer.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@ public struct DDTags {
1515
///
1616
/// Expects `String` value set for a tag.
1717
public static let resource = "resource.name"
18+
/// Internal tag. `Integer` value. Measures elapsed time at app's foreground state in nanoseconds.
19+
/// (duration - foregroundDuration) gives you the elapsed time while the app wasn't active (probably at background)
20+
internal static let foregroundDuration = "foreground_duration"
21+
/// Internal tag. `Bool` value.
22+
/// `true` if span was started or ended while the app was not active, `false` otherwise.
23+
internal static let isBackground = "is_background"
1824

1925
/// Those keys used to encode information received from the user through `OpenTracingLogFields`, `OpenTracingTagKeys` or custom fields.
2026
/// Supported by Datadog platform.

‎Sources/Datadog/Tracing/AutoInstrumentation/URLSessionTracingHandler.swift

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,13 @@
77
import Foundation
88

99
internal class URLSessionTracingHandler: URLSessionInterceptionHandler {
10+
/// Listening to app state changes and use it to report `foreground_duration`
11+
let appStateListener: AppStateListening
12+
13+
init(appStateListener: AppStateListening) {
14+
self.appStateListener = appStateListener
15+
}
16+
1017
// MARK: - URLSessionInterceptionHandler
1118

1219
func notify_taskInterceptionStarted(interception: TaskInterception) {
@@ -68,12 +75,12 @@ internal class URLSessionTracingHandler: URLSessionInterceptionHandler {
6875
}
6976
}
7077
}
71-
let appStateHistory = interception.appStateListener.history.take(
78+
let appStateHistory = appStateListener.history.take(
7279
between: resourceMetrics.fetch.start...resourceMetrics.fetch.end
7380
)
7481
// TODO: RUMM-1064 tag doesn't seem like the best fit but can't find anything better
75-
span.setTag(key: "foreground_duration", value: "\(appStateHistory.foregroundDuration)")
76-
span.setTag(key: "is_background", value: "\(appStateHistory.didRunInBackground)")
82+
span.setTag(key: DDTags.foregroundDuration, value: appStateHistory.foregroundDuration.toNanoseconds)
83+
span.setTag(key: DDTags.isBackground, value: appStateHistory.didRunInBackground)
7784

7885
span.finish(at: resourceMetrics.fetch.end)
7986
}

‎Sources/Datadog/URLSessionAutoInstrumentation/Interception/TaskInterception.swift

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,14 @@ internal class TaskInterception {
2121
/// Trace information propagated with the task. Not available when Tracing is disabled
2222
/// or when the task was created through `URLSession.dataTask(with:url)` on some iOS13+.
2323
private(set) var spanContext: DDSpanContext?
24-
/// Listening to app state changes and use it to report `foreground_duration`
25-
let appStateListener: AppStateListening
2624

2725
init(
2826
request: URLRequest,
29-
isFirstParty: Bool,
30-
appStateListener: AppStateListening
27+
isFirstParty: Bool
3128
) {
3229
self.identifier = UUID()
3330
self.request = request
3431
self.isFirstPartyRequest = isFirstParty
35-
self.appStateListener = appStateListener
3632
}
3733

3834
func register(metrics: ResourceMetrics) {

‎Sources/Datadog/URLSessionAutoInstrumentation/Interception/URLSessionInterceptor.swift

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,6 @@ public class URLSessionInterceptor: URLSessionInterceptorType {
2323
private let defaultFirstPartyURLsFilter: FirstPartyURLsFilter
2424
/// Filters internal `URLs` used by the SDK.
2525
private let internalURLsFilter: InternalURLsFilter
26-
/// Listening to app state changes and use it to report `foreground_duration`
27-
internal let appStateListener: AppStateListening
28-
2926
/// Handles resources interception.
3027
/// Depending on which instrumentation is enabled, this can be either RUM or Tracing handler sending respectively: RUM Resource or tracing Span.
3128
internal let handler: URLSessionInterceptionHandler
@@ -48,7 +45,7 @@ public class URLSessionInterceptor: URLSessionInterceptorType {
4845
if configuration.instrumentRUM {
4946
handler = URLSessionRUMResourcesHandler(dateProvider: dateProvider)
5047
} else {
51-
handler = URLSessionTracingHandler()
48+
handler = URLSessionTracingHandler(appStateListener: appStateListener)
5249
}
5350

5451
self.init(configuration: configuration, handler: handler, appStateListener: appStateListener)
@@ -62,7 +59,6 @@ public class URLSessionInterceptor: URLSessionInterceptorType {
6259
self.defaultFirstPartyURLsFilter = FirstPartyURLsFilter(hosts: configuration.userDefinedFirstPartyHosts)
6360
self.internalURLsFilter = InternalURLsFilter(urls: configuration.sdkInternalURLs)
6461
self.handler = handler
65-
self.appStateListener = appStateListener
6662

6763
if configuration.instrumentTracing {
6864
self.injectTracingHeadersToFirstPartyRequests = true
@@ -118,8 +114,7 @@ public class URLSessionInterceptor: URLSessionInterceptorType {
118114
let isFirstPartyRequest = self.isFirstParty(request: request, for: session)
119115
let interception = TaskInterception(
120116
request: request,
121-
isFirstParty: isFirstPartyRequest,
122-
appStateListener: self.appStateListener
117+
isFirstParty: isFirstPartyRequest
123118
)
124119
self.interceptionByTask[task] = interception
125120

‎Tests/DatadogTests/Datadog/Core/Utils/ValuePublisherTests.swift

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -55,32 +55,6 @@ class ValuePublisherTests: XCTestCase {
5555
waitForExpectations(timeout: 1, handler: nil)
5656
}
5757

58-
func testWhenValueMutates_itNotifiesObservers() {
59-
let numberOfObservers = 10
60-
let initialValue: Int = .mockRandom()
61-
let newValue: Int = .mockRandom()
62-
63-
let expectation = self.expectation(description: "All observers received new value")
64-
expectation.expectedFulfillmentCount = numberOfObservers
65-
66-
let observers: [ValueObserverMock<Int>] = (0..<numberOfObservers).map { _ in
67-
ValueObserverMock<Int> { old, new in
68-
XCTAssertEqual(old, initialValue)
69-
XCTAssertEqual(new, newValue)
70-
expectation.fulfill()
71-
}
72-
}
73-
74-
let publisher = ValuePublisher<Int>(initialValue: initialValue)
75-
observers.forEach { publisher.subscribe($0) }
76-
77-
// When
78-
publisher.mutateAsync { $0 = newValue }
79-
80-
// Then
81-
waitForExpectations(timeout: 1, handler: nil)
82-
}
83-
8458
func testWhenNonEquatableValueChanges_itNotifiesObserversOnAllChanges() {
8559
struct NonEquatableValue {
8660
let value: Int

‎Tests/DatadogTests/Datadog/RUM/AutoInstrumentation/Resources/URLSessionRUMResourcesHandlerTests.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class URLSessionRUMResourcesHandlerTests: XCTestCase {
2626
// Given
2727
var request = URLRequest(url: .mockRandom())
2828
request.httpMethod = ["GET", "POST", "PUT", "DELETE"].randomElement()!
29-
let taskInterception = TaskInterception(request: request, isFirstParty: .random(), appStateListener: AppStateListener.mockAny())
29+
let taskInterception = TaskInterception(request: request, isFirstParty: .random())
3030
XCTAssertNil(taskInterception.spanContext)
3131

3232
// When
@@ -49,7 +49,7 @@ class URLSessionRUMResourcesHandlerTests: XCTestCase {
4949
commandSubscriber.onCommandReceived = { _ in receiveCommand.fulfill() }
5050

5151
// Given
52-
let taskInterception = TaskInterception(request: .mockAny(), isFirstParty: true, appStateListener: AppStateListener.mockAny())
52+
let taskInterception = TaskInterception(request: .mockAny(), isFirstParty: true)
5353

5454
// When
5555
handler.notify_taskInterceptionStarted(interception: taskInterception)
@@ -66,7 +66,7 @@ class URLSessionRUMResourcesHandlerTests: XCTestCase {
6666
commandSubscriber.onCommandReceived = { _ in receiveCommand.fulfill() }
6767

6868
// Given
69-
let taskInterception = TaskInterception(request: .mockAny(), isFirstParty: false, appStateListener: AppStateListener.mockAny())
69+
let taskInterception = TaskInterception(request: .mockAny(), isFirstParty: false)
7070

7171
// When
7272
handler.notify_taskInterceptionStarted(interception: taskInterception)
@@ -83,7 +83,7 @@ class URLSessionRUMResourcesHandlerTests: XCTestCase {
8383
commandSubscriber.onCommandReceived = { _ in receiveCommand.fulfill() }
8484

8585
// Given
86-
let taskInterception = TaskInterception(request: .mockAny(), isFirstParty: .random(), appStateListener: AppStateListener.mockAny())
86+
let taskInterception = TaskInterception(request: .mockAny(), isFirstParty: .random())
8787
taskInterception.register(spanContext: .mockWith(traceID: 1, spanID: 2))
8888
XCTAssertNotNil(taskInterception.spanContext)
8989

@@ -108,7 +108,7 @@ class URLSessionRUMResourcesHandlerTests: XCTestCase {
108108
}
109109

110110
// Given
111-
let taskInterception = TaskInterception(request: .mockAny(), isFirstParty: .random(), appStateListener: AppStateListener.mockAny())
111+
let taskInterception = TaskInterception(request: .mockAny(), isFirstParty: .random())
112112
let resourceMetrics: ResourceMetrics = .mockAny()
113113
let resourceCompletion: ResourceCompletion = .mockWith(response: .mockResponseWith(statusCode: 200), error: nil)
114114
taskInterception.register(metrics: resourceMetrics)
@@ -145,7 +145,7 @@ class URLSessionRUMResourcesHandlerTests: XCTestCase {
145145
}
146146

147147
// Given
148-
let taskInterception = TaskInterception(request: .mockAny(), isFirstParty: .random(), appStateListener: AppStateListener.mockAny())
148+
let taskInterception = TaskInterception(request: .mockAny(), isFirstParty: .random())
149149
let taskError = NSError(domain: "domain", code: 123, userInfo: [NSLocalizedDescriptionKey: "network error"])
150150
let resourceMetrics: ResourceMetrics = .mockAny()
151151
let resourceCompletion: ResourceCompletion = .mockWith(response: nil, error: taskError)

‎Tests/DatadogTests/Datadog/RUMMonitorTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1136,7 +1136,7 @@ class RUMMonitorTests: XCTestCase {
11361136
XCTAssertTrue(Global.rum is DDNoopRUMMonitor)
11371137

11381138
// Then
1139-
resourcesHandler.notify_taskInterceptionCompleted(interception: TaskInterception(request: .mockAny(), isFirstParty: .mockAny(), appStateListener: AppStateListener.mockAny()))
1139+
resourcesHandler.notify_taskInterceptionCompleted(interception: TaskInterception(request: .mockAny(), isFirstParty: .mockAny()))
11401140
XCTAssertEqual(output.recordedLog?.status, .warn)
11411141
XCTAssertEqual(
11421142
output.recordedLog?.message,

‎Tests/DatadogTests/Datadog/TracerTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1100,7 +1100,7 @@ class TracerTests: XCTestCase {
11001100
XCTAssertTrue(Global.sharedTracer is DDNoopTracer)
11011101

11021102
// Then
1103-
tracingHandler.notify_taskInterceptionCompleted(interception: TaskInterception(request: .mockAny(), isFirstParty: true, appStateListener: AppStateListener.mockAny()))
1103+
tracingHandler.notify_taskInterceptionCompleted(interception: TaskInterception(request: .mockAny(), isFirstParty: true))
11041104
XCTAssertEqual(output.recordedLog?.status, .warn)
11051105
XCTAssertEqual(
11061106
output.recordedLog?.message,

‎Tests/DatadogTests/Datadog/Tracing/Autoinstrumentation/URLSessionTracingHandlerTests.swift

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,17 @@
77
import XCTest
88
@testable import Datadog
99

10+
private class MockAppStateListener: AppStateListening {
11+
let history = AppStateHistory(
12+
initialState: .init(isActive: true, date: .mockDecember15th2019At10AMUTC()),
13+
finalDate: .mockDecember15th2019At10AMUTC() + 10
14+
)
15+
}
16+
1017
class URLSessionTracingHandlerTests: XCTestCase {
1118
private let spanOutput = SpanOutputMock()
1219
private let logOutput = LogOutputMock()
13-
private let handler = URLSessionTracingHandler()
20+
private let handler = URLSessionTracingHandler(appStateListener: MockAppStateListener())
1421

1522
override func setUp() {
1623
Global.sharedTracer = Tracer.mockWith(
@@ -33,7 +40,7 @@ class URLSessionTracingHandlerTests: XCTestCase {
3340
spanOutput.onSpanRecorded = { _ in spanSentExpectation.fulfill() }
3441

3542
// Given
36-
let interception = TaskInterception(request: .mockAny(), isFirstParty: true, appStateListener: AppStateListener.mockAny())
43+
let interception = TaskInterception(request: .mockAny(), isFirstParty: true)
3744
interception.register(completion: .mockAny())
3845
interception.register(
3946
metrics: .mockWith(
@@ -70,7 +77,7 @@ class URLSessionTracingHandlerTests: XCTestCase {
7077

7178
// Given
7279
let request: URLRequest = .mockWith(httpMethod: "POST")
73-
let interception = TaskInterception(request: request, isFirstParty: true, appStateListener: AppStateListener.mockAny())
80+
let interception = TaskInterception(request: request, isFirstParty: true)
7481
interception.register(completion: .mockWith(response: .mockResponseWith(statusCode: 200), error: nil))
7582
interception.register(
7683
metrics: .mockWith(
@@ -108,7 +115,7 @@ class URLSessionTracingHandlerTests: XCTestCase {
108115
// Given
109116
let request: URLRequest = .mockWith(httpMethod: "GET")
110117
let error = NSError(domain: "domain", code: 123, userInfo: [NSLocalizedDescriptionKey: "network error"])
111-
let interception = TaskInterception(request: request, isFirstParty: true, appStateListener: AppStateListener.mockAny())
118+
let interception = TaskInterception(request: request, isFirstParty: true)
112119
interception.register(completion: .mockWith(response: nil, error: error))
113120
interception.register(
114121
metrics: .mockWith(
@@ -170,7 +177,7 @@ class URLSessionTracingHandlerTests: XCTestCase {
170177

171178
// Given
172179
let request: URLRequest = .mockWith(httpMethod: "GET")
173-
let interception = TaskInterception(request: request, isFirstParty: true, appStateListener: AppStateListener.mockAny())
180+
let interception = TaskInterception(request: request, isFirstParty: true)
174181
interception.register(completion: .mockWith(response: .mockResponseWith(statusCode: 404), error: nil))
175182
interception.register(
176183
metrics: .mockWith(
@@ -233,7 +240,7 @@ class URLSessionTracingHandlerTests: XCTestCase {
233240
spanOutput.onSpanRecorded = { _ in spanNotSentExpectation.fulfill() }
234241

235242
// Given
236-
let incompleteInterception = TaskInterception(request: .mockAny(), isFirstParty: true, appStateListener: AppStateListener.mockAny())
243+
let incompleteInterception = TaskInterception(request: .mockAny(), isFirstParty: true)
237244
// `incompleteInterception` has no metrics and no completion
238245

239246
// When
@@ -250,7 +257,7 @@ class URLSessionTracingHandlerTests: XCTestCase {
250257
spanOutput.onSpanRecorded = { _ in spanNotSentExpectation.fulfill() }
251258

252259
// Given
253-
let interception = TaskInterception(request: .mockAny(), isFirstParty: false, appStateListener: AppStateListener.mockAny())
260+
let interception = TaskInterception(request: .mockAny(), isFirstParty: false)
254261
interception.register(completion: .mockAny())
255262
interception.register(
256263
metrics: .mockWith(
@@ -271,19 +278,8 @@ class URLSessionTracingHandlerTests: XCTestCase {
271278
}
272279

273280
func testGivenAnyInterception_itAddsAppStateInformationToSpan() throws {
274-
class MockAppStateListener: AppStateListening {
275-
let history = AppStateHistory(
276-
initialState: .init(isActive: true, date: .mockDecember15th2019At10AMUTC()),
277-
finalDate: .mockDecember15th2019At10AMUTC() + 10
278-
)
279-
}
280-
281281
// Given
282-
let interception = TaskInterception(
283-
request: .mockAny(),
284-
isFirstParty: true,
285-
appStateListener: MockAppStateListener()
286-
)
282+
let interception = TaskInterception(request: .mockAny(), isFirstParty: true)
287283
interception.register(completion: .mockAny())
288284
interception.register(
289285
metrics: .mockWith(
@@ -299,7 +295,7 @@ class URLSessionTracingHandlerTests: XCTestCase {
299295

300296
// Then
301297
let recordedSpan = try XCTUnwrap(spanOutput.recordedSpan)
302-
XCTAssertEqual(recordedSpan.tags["foreground_duration"]?.encodable.value as? String, "10.0")
303-
XCTAssertEqual(recordedSpan.tags["is_background"]?.encodable.value as? String, "false")
298+
XCTAssertEqual(recordedSpan.tags[DDTags.foregroundDuration]?.encodable.value as? UInt64, 10_000_000_000)
299+
XCTAssertEqual(recordedSpan.tags[DDTags.isBackground]?.encodable.value as? Bool, false)
304300
}
305301
}

0 commit comments

Comments
 (0)