Skip to content

Commit cd68fed

Browse files
maxepmaxep
committed
RUM-8042 Move pending_batches to dedicated aggregation
1 parent b649964 commit cd68fed

File tree

3 files changed

+89
-31
lines changed

3 files changed

+89
-31
lines changed

‎DatadogCore/Sources/Core/Storage/FilesOrchestrator.swift‎

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,6 @@ internal class FilesOrchestrator: FilesOrchestratorType {
6060
/// An extra information to include in metrics or `nil` if metrics should not be reported for this orchestrator.
6161
let metricsData: MetricsData?
6262

63-
/// Tracks number of pending batches in the track's directory
64-
@ReadWriteLock
65-
private var pendingBatches: Int = 0
66-
6763
var trackName: String {
6864
metricsData?.trackName ?? "Unknown"
6965
}
@@ -133,9 +129,8 @@ internal class FilesOrchestrator: FilesOrchestratorType {
133129
lastWritableFileObjectsCount = 1
134130
lastWritableFileApproximatedSize = writeSize
135131
lastWritableFileLastWriteDate = dateProvider.now
136-
137-
// Increment pending batches for telemetry
138-
_pendingBatches.mutate { $0 += 1 }
132+
// Increment pending batches in telemetry
133+
incrementPendingBatches()
139134
return newFile
140135
}
141136

@@ -193,7 +188,7 @@ internal class FilesOrchestrator: FilesOrchestratorType {
193188
let files = try directory.files()
194189

195190
// Reset pending batches for telemetry
196-
pendingBatches = files.count
191+
recordPendingBatches(count: files.count)
197192

198193
let filesFromOldest = try files
199194
.compactMap { try deleteFileIfItsObsolete(file: $0, fileCreationDate: fileCreationDateFrom(fileName: $0.name)) }
@@ -228,7 +223,7 @@ internal class FilesOrchestrator: FilesOrchestratorType {
228223
#endif
229224
try readableFile.delete()
230225
// Decrement pending batches at each batch deletion
231-
_pendingBatches.mutate { $0 -= 1 }
226+
incrementPendingBatches(by: -1)
232227
sendBatchDeletedMetric(batchFile: readableFile, deletionReason: deletionReason)
233228
} catch {
234229
telemetry.error("Failed to delete file", error: error)
@@ -259,7 +254,7 @@ internal class FilesOrchestrator: FilesOrchestratorType {
259254
let fileWithSize = filesWithSizeSortedByCreationDate.removeFirst()
260255
try fileWithSize.file.delete()
261256
// Decrement pending batches at each batch deletion
262-
_pendingBatches.mutate { $0 -= 1 }
257+
incrementPendingBatches(by: -1)
263258
sendBatchDeletedMetric(batchFile: fileWithSize.file, deletionReason: .purged)
264259
sizeFreed += fileWithSize.size
265260
}
@@ -272,7 +267,7 @@ internal class FilesOrchestrator: FilesOrchestratorType {
272267
if fileAge > performance.maxFileAgeForRead {
273268
try file.delete()
274269
// Decrement pending batches at each batch deletion
275-
_pendingBatches.mutate { $0 -= 1 }
270+
incrementPendingBatches(by: -1)
276271
sendBatchDeletedMetric(batchFile: file, deletionReason: .obsolete)
277272
return nil
278273
} else {
@@ -309,13 +304,40 @@ internal class FilesOrchestrator: FilesOrchestratorType {
309304
BatchDeletedMetric.batchAgeKey: batchAge.toMilliseconds,
310305
BatchDeletedMetric.batchRemovalReasonKey: deletionReason.toString(),
311306
BatchDeletedMetric.inBackgroundKey: false,
312-
BatchDeletedMetric.backgroundTasksEnabled: metricsData.backgroundTasksEnabled,
313-
BatchDeletedMetric.pendingBatches: pendingBatches
307+
BatchDeletedMetric.backgroundTasksEnabled: metricsData.backgroundTasksEnabled
314308
],
315309
sampleRate: BatchDeletedMetric.sampleRate
316310
)
317311
}
318312

313+
private func incrementPendingBatches(by increment: Double = 1) {
314+
guard let metricsData = metricsData else {
315+
return
316+
}
317+
318+
telemetry.increment(
319+
metric: PendingBatchMetric.typeValue,
320+
by: increment,
321+
cardinalities: [
322+
BatchMetric.trackKey: .string(metricsData.trackName)
323+
]
324+
)
325+
}
326+
327+
private func recordPendingBatches(count: Int) {
328+
guard let metricsData = metricsData else {
329+
return
330+
}
331+
332+
telemetry.record(
333+
metric: PendingBatchMetric.typeValue,
334+
value: count,
335+
cardinalities: [
336+
BatchMetric.trackKey: .string(metricsData.trackName)
337+
]
338+
)
339+
}
340+
319341
/// Sends "Batch Closed" telemetry log.
320342
/// - Parameters:
321343
/// - fileName: The name of the batch that was closed.

‎DatadogCore/Sources/SDKMetrics/BatchMetrics.swift‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,3 +140,9 @@ internal enum BatchBlockedMetric {
140140
/// The blocking failure reason.
141141
static let failure = "failure"
142142
}
143+
144+
/// Definition of "Pending Batches" telemetry.
145+
internal enum PendingBatchMetric {
146+
/// Metric type value.
147+
static let typeValue = "pending batches"
148+
}

‎DatadogCore/Tests/Datadog/Core/Persistence/FilesOrchestrator+MetricsTests.swift‎

Lines changed: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ class FilesOrchestrator_MetricsTests: XCTestCase {
4646

4747
// MARK: - "Batch Deleted" Metric
4848

49-
func testWhenReadableFileIsDeleted_itSendsBatchDeletedMetric() throws {
49+
func testWhenReadableFileIsDeleted_itSendsTelemetryMetric() throws {
5050
// Given
5151
let orchestrator = createOrchestrator()
5252
let expectedBatchAge = storage.minFileAgeForRead + 1
@@ -63,8 +63,8 @@ class FilesOrchestrator_MetricsTests: XCTestCase {
6363
orchestrator.delete(readableFile: file, deletionReason: .intakeCode(responseCode: 202))
6464

6565
// Then
66-
let metric = try XCTUnwrap(telemetry.messages.firstMetricReport(named: "Batch Deleted"))
67-
DDAssertJSONEqual(metric.attributes, [
66+
let batchDeleted = try XCTUnwrap(telemetry.messages.firstMetricReport(named: "Batch Deleted"))
67+
DDAssertJSONEqual(batchDeleted.attributes, [
6868
"metric_type": "batch deleted",
6969
"track": "track name",
7070
"consent": "consent value",
@@ -76,13 +76,20 @@ class FilesOrchestrator_MetricsTests: XCTestCase {
7676
"in_background": false,
7777
"background_tasks_enabled": false,
7878
"batch_age": expectedBatchAge.toMilliseconds,
79-
"batch_removal_reason": "intake-code-202",
80-
"pending_batches": 1
79+
"batch_removal_reason": "intake-code-202"
8180
])
82-
XCTAssertEqual(metric.sampleRate, BatchDeletedMetric.sampleRate)
81+
XCTAssertEqual(batchDeleted.sampleRate, BatchDeletedMetric.sampleRate)
82+
83+
let pendingBatches = telemetry.messages.compactMap { $0.asMetricIncrement }.reduce(0) { count, metric in
84+
XCTAssertEqual(metric.metric, "pending batches")
85+
XCTAssertEqual(metric.cardinalities["track"], .string("track name"))
86+
return count + metric.increment
87+
}
88+
89+
XCTAssertEqual(pendingBatches, 1)
8390
}
8491

85-
func testWhenObsoleteFileIsDeleted_itSendsBatchDeletedMetric() throws {
92+
func testWhenObsoleteFileIsDeleted_itSendsTelemetryMetric() throws {
8693
// Given:
8794
// - request some batch to be created
8895
let orchestrator = createOrchestrator()
@@ -95,8 +102,8 @@ class FilesOrchestrator_MetricsTests: XCTestCase {
95102
_ = orchestrator.getReadableFiles()
96103

97104
// Then
98-
let metric = try XCTUnwrap(telemetry.messages.firstMetricReport(named: "Batch Deleted"))
99-
DDAssertJSONEqual(metric.attributes, [
105+
let batchDeleted = try XCTUnwrap(telemetry.messages.firstMetricReport(named: "Batch Deleted"))
106+
DDAssertJSONEqual(batchDeleted.attributes, [
100107
"metric_type": "batch deleted",
101108
"track": "track name",
102109
"consent": "consent value",
@@ -108,13 +115,29 @@ class FilesOrchestrator_MetricsTests: XCTestCase {
108115
"in_background": false,
109116
"background_tasks_enabled": false,
110117
"batch_age": (storage.maxFileAgeForRead + 1).toMilliseconds,
111-
"batch_removal_reason": "obsolete",
112-
"pending_batches": 0
118+
"batch_removal_reason": "obsolete"
113119
])
114-
XCTAssertEqual(metric.sampleRate, BatchDeletedMetric.sampleRate)
120+
XCTAssertEqual(batchDeleted.sampleRate, BatchDeletedMetric.sampleRate)
121+
122+
let pendingBatches = telemetry.messages.reduce(0) { count, message in
123+
switch message {
124+
case let .metric(.record(metric, value, cardinalities)):
125+
XCTAssertEqual(metric, "pending batches")
126+
XCTAssertEqual(cardinalities["track"], .string("track name"))
127+
return value
128+
case let .metric(.increment(metric, value, cardinalities)):
129+
XCTAssertEqual(metric, "pending batches")
130+
XCTAssertEqual(cardinalities["track"], .string("track name"))
131+
return count + value
132+
default:
133+
return count
134+
}
135+
}
136+
137+
XCTAssertEqual(pendingBatches, 0)
115138
}
116139

117-
func testWhenDirectoryIsPurged_itSendsBatchDeletedMetrics() throws {
140+
func testWhenDirectoryIsPurged_itSendsTelemetryMetrics() throws {
118141
// Given: some batch
119142
// - request batch to be created
120143
// - write more data than allowed directory size limit
@@ -130,8 +153,8 @@ class FilesOrchestrator_MetricsTests: XCTestCase {
130153
_ = try orchestrator.getWritableFile(writeSize: 1)
131154

132155
// Then
133-
let metric = try XCTUnwrap(telemetry.messages.firstMetricReport(named: "Batch Deleted"))
134-
DDAssertJSONEqual(metric.attributes, [
156+
let batchDeleted = try XCTUnwrap(telemetry.messages.firstMetricReport(named: "Batch Deleted"))
157+
DDAssertJSONEqual(batchDeleted.attributes, [
135158
"metric_type": "batch deleted",
136159
"track": "track name",
137160
"consent": "consent value",
@@ -143,10 +166,17 @@ class FilesOrchestrator_MetricsTests: XCTestCase {
143166
"in_background": false,
144167
"background_tasks_enabled": false,
145168
"batch_age": expectedBatchAge.toMilliseconds,
146-
"batch_removal_reason": "purged",
147-
"pending_batches": 0
169+
"batch_removal_reason": "purged"
148170
])
149-
XCTAssertEqual(metric.sampleRate, BatchDeletedMetric.sampleRate)
171+
XCTAssertEqual(batchDeleted.sampleRate, BatchDeletedMetric.sampleRate)
172+
173+
let pendingBatches = telemetry.messages.compactMap { $0.asMetricIncrement }.reduce(0) { count, metric in
174+
XCTAssertEqual(metric.metric, "pending batches")
175+
XCTAssertEqual(metric.cardinalities["track"], .string("track name"))
176+
return count + metric.increment
177+
}
178+
179+
XCTAssertEqual(pendingBatches, 1)
150180
}
151181

152182
// MARK: - "Batch Closed" Metric

0 commit comments

Comments
 (0)