Skip to content

Commit

Permalink
RUM-1000 chore: improve telemetry messages for file I/O errors
Browse files Browse the repository at this point in the history
  • Loading branch information
ganeshnj committed Jun 28, 2024
1 parent 380bdf4 commit b1ce789
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 16 deletions.
5 changes: 5 additions & 0 deletions DatadogCore/Sources/Core/Storage/FilesOrchestrator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ internal protocol FilesOrchestratorType: AnyObject {
func delete(readableFile: ReadableFile, deletionReason: BatchDeletedMetric.RemovalReason)

var ignoreFilesAgeWhenReading: Bool { get set }
var trackName: String { get }
}

/// Orchestrates files in a single directory.
Expand Down Expand Up @@ -52,6 +53,10 @@ internal class FilesOrchestrator: FilesOrchestratorType {
/// An extra information to include in metrics or `nil` if metrics should not be reported for this orchestrator.
let metricsData: MetricsData?

var trackName: String {
metricsData?.trackName ?? "Unknown"
}

init(
directory: Directory,
performance: StoragePerformancePreset,
Expand Down
39 changes: 29 additions & 10 deletions DatadogCore/Sources/Core/Storage/Writing/FileWriter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,25 +37,44 @@ internal struct FileWriter: Writer {
/// - value: Encodable value to write.
/// - metadata: Encodable metadata to write.
func write<T: Encodable, M: Encodable>(value: T, metadata: M?) {
do {
var encoded: Data = .init()
if let metadata = metadata {
var encoded: Data = .init()
if let metadata = metadata {
do {
let encodedMetadata = try encode(value: metadata, blockType: .eventMetadata)
encoded.append(encodedMetadata)
} catch {
DD.logger.error("(\(orchestrator.trackName)) Failed to encode metadata", error: error)
telemetry.error("(\(orchestrator.trackName)) Failed to encode metadata", error: error)
}
}

do {
let encodedValue = try encode(value: value, blockType: .event)
encoded.append(encodedValue)
} catch {
DD.logger.error("(\(orchestrator.trackName)) Failed to encode value", error: error)
telemetry.error("(\(orchestrator.trackName)) Failed to encode value", error: error)
return
}

// Make sure both event and event metadata are written to the same file.
// This is to avoid a situation where event is written to one file and event metadata to another.
// If this happens, the reader will not be able to match event with its metadata.
let writeSize = UInt64(encoded.count)
let file = try orchestrator.getWritableFile(writeSize: writeSize)
// Make sure both event and event metadata are written to the same file.
// This is to avoid a situation where event is written to one file and event metadata to another.
// If this happens, the reader will not be able to match event with its metadata.
let writeSize = UInt64(encoded.count)
let file: WritableFile
do {
file = try orchestrator.getWritableFile(writeSize: writeSize)
} catch {
DD.logger.error("(\(orchestrator.trackName)) Failed to get writable file for \(writeSize) bytes", error: error)
telemetry.error("(\(orchestrator.trackName)) Failed to get writable file for \(writeSize) bytes", error: error)
return
}

do {
try file.append(data: encoded)
} catch {
DD.logger.error("Failed to write data", error: error)
telemetry.error("Failed to write data to file", error: error)
DD.logger.error("(\(orchestrator.trackName)) Failed to write \(writeSize) bytes to file", error: error)
telemetry.error("(\(orchestrator.trackName)) Failed to write \(writeSize) bytes to file", error: error)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ class FileWriterTests: XCTestCase {
maxObjectSize: 23 // 23 bytes is enough for TLV with {"key1":"value1"} JSON
),
dateProvider: SystemDateProvider(),
telemetry: NOPTelemetry()
telemetry: NOPTelemetry(),
metricsData: .init(trackName: "rum", consentLabel: .mockAny(), uploaderPerformance: UploadPerformanceMock.noOp)
),
encryption: nil,
telemetry: NOPTelemetry()
Expand All @@ -168,7 +169,7 @@ class FileWriterTests: XCTestCase {
reader = try BatchDataBlockReader(input: directory.files()[0].stream())
blocks = try XCTUnwrap(reader.all())
XCTAssertEqual(blocks.count, 1) // same content as before
XCTAssertEqual(dd.logger.errorLog?.message, "Failed to write data")
XCTAssertEqual(dd.logger.errorLog?.message, "(rum) Failed to encode value")
XCTAssertEqual(dd.logger.errorLog?.error?.message, "DataBlock length exceeds limit of 23 bytes")
}

Expand All @@ -181,15 +182,16 @@ class FileWriterTests: XCTestCase {
directory: directory,
performance: PerformancePreset.mockAny(),
dateProvider: SystemDateProvider(),
telemetry: NOPTelemetry()
telemetry: NOPTelemetry(),
metricsData: .init(trackName: "rum", consentLabel: .mockAny(), uploaderPerformance: UploadPerformanceMock.noOp)
),
encryption: nil,
telemetry: NOPTelemetry()
)

writer.write(value: FailingEncodableMock(errorMessage: "failed to encode `FailingEncodable`."))

XCTAssertEqual(dd.logger.errorLog?.message, "Failed to write data")
XCTAssertEqual(dd.logger.errorLog?.message, "(rum) Failed to encode value")
XCTAssertEqual(dd.logger.errorLog?.error?.message, "failed to encode `FailingEncodable`.")
}

Expand All @@ -202,7 +204,8 @@ class FileWriterTests: XCTestCase {
directory: directory,
performance: PerformancePreset.mockAny(),
dateProvider: SystemDateProvider(),
telemetry: NOPTelemetry()
telemetry: NOPTelemetry(),
metricsData: .init(trackName: "rum", consentLabel: .mockAny(), uploaderPerformance: UploadPerformanceMock.noOp)
),
encryption: nil,
telemetry: NOPTelemetry()
Expand All @@ -213,7 +216,7 @@ class FileWriterTests: XCTestCase {
writer.write(value: ["won't be written"])
try? directory.files()[0].makeReadWrite()

XCTAssertEqual(dd.logger.errorLog?.message, "Failed to write data")
XCTAssertEqual(dd.logger.errorLog?.message, "(rum) Failed to write 26 bytes to file")
XCTAssertTrue(dd.logger.errorLog!.error!.message.contains("You don’t have permission"))
}

Expand Down
1 change: 1 addition & 0 deletions DatadogCore/Tests/Datadog/Mocks/CoreMocks.swift
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ internal class NOPFilesOrchestrator: FilesOrchestratorType {
func delete(readableFile: ReadableFile, deletionReason: BatchDeletedMetric.RemovalReason) { }

var ignoreFilesAgeWhenReading = false
var trackName: String = "nop"
}

extension DataFormat {
Expand Down

0 comments on commit b1ce789

Please sign in to comment.