Merge pull request #3096 from DataDog/fix/telemetry-error-encoding

[RUM-17396] Standardize and sanitize error reporting for internal SDK telemetry

Author: ncreated

AGENTS.md

@@ -20,9 +20,11 @@ CONTRIBUTING.md          ← General contribution guidelines
 
 docs/
 ├── ARCHITECTURE.md      ← Module structure, data flow, key abstractions, protocols,
-│                          error handling, thread safety, HTTP upload, dependencies
+│                          thread safety, HTTP upload, dependencies
 ├── CONVENTIONS.md       ← Naming, SwiftLint rules, conditional compilation,
 │                          generated models, file headers, commit/PR format
+├── ERROR_HANDLING.md    ← Customer-facing error safety + internal telemetry
+│                          error reporting (TelemetrySanitizableError)
 ├── DEVELOPMENT.md       ← Recipes for adding features/commands/providers,
 │                          RFC process, build & test quick reference
 ├── TESTING.md           ← Test conventions, mock infrastructure (.mockAny(),
@@ -50,6 +52,7 @@ Feature-specific docs (in each module directory):
 | Write or fix tests | `docs/TESTING.md` |
 | Check naming, lint, commit format | `docs/CONVENTIONS.md` |
 | Touch swizzling code | `docs/SWIZZLING.md` |
+| Report an error to internal telemetry | `docs/ERROR_HANDLING.md` |
 | Modify a fragile area | `docs/KNOWN_CONCERNS.md` |
 | Work on RUM specifically | `DatadogRUM/RUM_FEATURE.md` |
 | Work on Session Replay specifically | `DatadogSessionReplay/SESSION_REPLAY_FEATURE.md` |
@@ -71,6 +74,7 @@ Feature-specific docs (in each module directory):
 - **Do NOT name branches with `codex`.** Use repo branch naming conventions instead.
 - **Never mention AI assistant names** (Claude, ChatGPT, Cursor, Copilot, etc.) in commit messages, PR descriptions, code comments, or co-author tags.
 - **Never write or modify a swizzle without reading `docs/SWIZZLING.md` first.** Past incidents have caused production crashes.
+- **Never report a raw `Error` to internal telemetry.** `"\(error)"` can embed customer data (e.g. `EncodingError.invalidValue` embeds the offending value). Always go through `Telemetry.error(_:)`, which sanitizes via `TelemetrySanitizableError`; see `docs/ERROR_HANDLING.md`. Past incidents have leaked customer PII and auth tokens this way.
 
 ## Quick Reference
 

Datadog/Datadog.xcodeproj/project.pbxproj

@@ -1014,6 +1014,8 @@
 		D23039F8298D5236001A1FA3 /* InternalLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = D23039CE298D5235001A1FA3 /* InternalLogger.swift */; };
 		D23039F9298D5236001A1FA3 /* CoreLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = D23039CF298D5235001A1FA3 /* CoreLogger.swift */; };
 		D23039FA298D5236001A1FA3 /* Telemetry.swift in Sources */ = {isa = PBXBuildFile; fileRef = D23039D0298D5235001A1FA3 /* Telemetry.swift */; };
+		6165D7353013888F0069D0E4 /* TelemetrySanitizableError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6165D7343013888F0069D0E4 /* TelemetrySanitizableError.swift */; };
+		6165D7373017AA010069D0E4 /* TelemetrySanitizableErrorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6165D7363017AA010069D0E4 /* TelemetrySanitizableErrorTests.swift */; };
 		D23039FB298D5236001A1FA3 /* URLRequestBuilder.swift in Sources */ = {isa = PBXBuildFile; fileRef = D23039D2298D5235001A1FA3 /* URLRequestBuilder.swift */; };
 		D23039FC298D5236001A1FA3 /* DataFormat.swift in Sources */ = {isa = PBXBuildFile; fileRef = D23039D3298D5235001A1FA3 /* DataFormat.swift */; };
 		D23039FD298D5236001A1FA3 /* DataCompression.swift in Sources */ = {isa = PBXBuildFile; fileRef = D23039D4298D5235001A1FA3 /* DataCompression.swift */; };
@@ -2975,6 +2977,8 @@
 		D23039CE298D5235001A1FA3 /* InternalLogger.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = InternalLogger.swift; sourceTree = "<group>"; };
 		D23039CF298D5235001A1FA3 /* CoreLogger.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CoreLogger.swift; sourceTree = "<group>"; };
 		D23039D0298D5235001A1FA3 /* Telemetry.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Telemetry.swift; sourceTree = "<group>"; };
+		6165D7343013888F0069D0E4 /* TelemetrySanitizableError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TelemetrySanitizableError.swift; sourceTree = "<group>"; };
+		6165D7363017AA010069D0E4 /* TelemetrySanitizableErrorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TelemetrySanitizableErrorTests.swift; sourceTree = "<group>"; };
 		D23039D2298D5235001A1FA3 /* URLRequestBuilder.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = URLRequestBuilder.swift; sourceTree = "<group>"; };
 		D23039D3298D5235001A1FA3 /* DataFormat.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DataFormat.swift; sourceTree = "<group>"; };
 		D23039D4298D5235001A1FA3 /* DataCompression.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DataCompression.swift; sourceTree = "<group>"; };
@@ -6300,6 +6304,7 @@
 			isa = PBXGroup;
 			children = (
 				D21AE6BB29E5EDAF0064BF29 /* TelemetryTests.swift */,
+				6165D7363017AA010069D0E4 /* TelemetrySanitizableErrorTests.swift */,
 			);
 			path = Telemetry;
 			sourceTree = "<group>";
@@ -6503,6 +6508,7 @@
 				D23039CE298D5235001A1FA3 /* InternalLogger.swift */,
 				11EA5C322DC5077000E8DFA2 /* InternalLogger+objc.swift */,
 				D23039D0298D5235001A1FA3 /* Telemetry.swift */,
+				6165D7343013888F0069D0E4 /* TelemetrySanitizableError.swift */,
 			);
 			path = Telemetry;
 			sourceTree = "<group>";
@@ -8968,6 +8974,7 @@
 				D23039EE298D5236001A1FA3 /* FeatureMessageReceiver.swift in Sources */,
 				D23039DE298D5235001A1FA3 /* Writer.swift in Sources */,
 				D23039FA298D5236001A1FA3 /* Telemetry.swift in Sources */,
+				6165D7353013888F0069D0E4 /* TelemetrySanitizableError.swift in Sources */,
 				D23039FC298D5236001A1FA3 /* DataFormat.swift in Sources */,
 				D2160CED29C0E0E600FAA9A5 /* DatadogURLSessionHandler.swift in Sources */,
 				1166C5A52F76EC0B008E34BC /* OperationOptions.swift in Sources */,
@@ -9500,6 +9507,7 @@
 				61F3E3662BC595F600C7881E /* HTTPHeadersReaderTests.swift in Sources */,
 				D2EBEE3C29BA163E00B15732 /* B3HTTPHeadersWriterTests.swift in Sources */,
 				D21AE6BC29E5EDAF0064BF29 /* TelemetryTests.swift in Sources */,
+				6165D7373017AA010069D0E4 /* TelemetrySanitizableErrorTests.swift in Sources */,
 				D2DA23A3298D58F400C6C7E6 /* AnyEncodableTests.swift in Sources */,
 				3CCECDAF2BC688120013C125 /* SpanIDGeneratorTests.swift in Sources */,
 				D263BCB429DB014900FA0E21 /* FixedWidthInteger+ConvenienceTests.swift in Sources */,

DatadogCore/Sources/Core/DataStore/DataStoreFileWriter.swift

@@ -12,6 +12,25 @@ internal enum DataStoreFileWritingError: Error {
     case failedToEncodeData(Error)
 }
 
+extension DataStoreFileWritingError: TelemetrySanitizableError {
+    func sanitize() -> TelemetrySanitizedError {
+        let kind: String
+        let wrapped: Error
+        switch self {
+        case .failedToEncodeVersion(let error):
+            kind = "failedToEncodeVersion"
+            wrapped = error
+        case .failedToEncodeData(let error):
+            kind = "failedToEncodeData"
+            wrapped = error
+        }
+        return TelemetrySanitizedError(
+            kind: "DataStoreFileWritingError",
+            message: "\(kind)(\(TelemetrySanitizedError(sanitizing: wrapped).message))"
+        )
+    }
+}
+
 internal struct DataStoreFileWriter {
     let file: File
 

DatadogCore/Sources/Core/DataStore/FeatureDataStore.swift

@@ -50,7 +50,7 @@ internal final class FeatureDataStore: DataStore {
                 try self.write(data: value, forKey: key, version: version)
             } catch let error {
                 DD.logger.error("[Data Store] Error on setting `\(key)` value for `\(self.feature)`", error: error)
-                self.telemetry.error("[Data Store] Error on setting `\(key)` value for `\(self.feature)`", error: DDError(error: error))
+                self.telemetry.error("[Data Store] Error on setting `\(key)` value for `\(self.feature)`", error: error)
             }
         }
     }
@@ -67,7 +67,7 @@ internal final class FeatureDataStore: DataStore {
             } catch let error {
                 callback(.error(error))
                 DD.logger.error("[Data Store] Error on getting `\(key)` value for `\(self.feature)`", error: error)
-                self.telemetry.error("[Data Store] Error on getting `\(key)` value for `\(self.feature)`", error: DDError(error: error))
+                self.telemetry.error("[Data Store] Error on getting `\(key)` value for `\(self.feature)`", error: error)
             }
         }
     }
@@ -82,7 +82,7 @@ internal final class FeatureDataStore: DataStore {
                 try self.deleteData(forKey: key)
             } catch let error {
                 DD.logger.error("[Data Store] Error on deleting `\(key)` value for `\(self.feature)`", error: error)
-                self.telemetry.error("[Data Store] Error on deleting `\(key)` value for `\(self.feature)`", error: DDError(error: error))
+                self.telemetry.error("[Data Store] Error on deleting `\(key)` value for `\(self.feature)`", error: error)
             }
         }
     }
@@ -94,7 +94,7 @@ internal final class FeatureDataStore: DataStore {
                 try directory?.deleteAllFiles()
             } catch let error {
                 DD.logger.error("[Data Store] Error on clearing all data for `\(self.feature)`", error: error)
-                self.telemetry.error("[Data Store] Error on clearing all data for `\(self.feature)`", error: DDError(error: error))
+                self.telemetry.error("[Data Store] Error on clearing all data for `\(self.feature)`", error: error)
             }
         }
     }

DatadogCore/Sources/Core/TLV/TLVBlockReader.swift

@@ -5,6 +5,7 @@
  */
 
 import Foundation
+import DatadogInternal
 
 /// A block reader can read TLV formatted blocks from a data input.
 ///
@@ -148,11 +149,24 @@ internal final class TLVBlockReader<BlockType> where BlockType: RawRepresentable
         return try read(length: Int(length))
     }
 }
+extension TLVBlockError: TelemetrySanitizableError {
+    /// Every case only ever describes block types, sizes, limits and stream status codes - never the
+    /// raw bytes or decoded content of a block - so the full description is safe to report as-is.
+    /// `readOperationFailed`'s `streamError` is the one exception: it's an arbitrary, foreign `Error`
+    /// (typically an `NSError`), and `NSError.userInfo` content depends on the error's domain - some
+    /// domains keep it minimal, others (e.g. `NSCocoaErrorDomain` file errors) can include the file
+    /// path - so `description` routes it through `TelemetrySanitizedError.init(sanitizing:)` rather than
+    /// assuming this particular source is safe to interpolate directly.
+    func sanitize() -> TelemetrySanitizedError {
+        TelemetrySanitizedError(unsafelyDescribing: self)
+    }
+}
+
 extension TLVBlockError: CustomStringConvertible {
     var description: String {
         switch self {
         case .readOperationFailed(let status, let error):
-            let error = error.map { "\($0)" } ?? "(null)"
+            let error = error.map { TelemetrySanitizedError(sanitizing: $0) }.map { "\($0.kind): \($0.message)" } ?? "(null)"
             return "DataBlock read operation failed with stream status: \(status.rawValue), error: \(error)"
         case .invalidDataType(let type):
             return "Invalid DataBlock type: \(type)"

DatadogCore/Tests/Datadog/Core/TLV/TLVBlockTests.swift

@@ -64,4 +64,26 @@ class TLVBlockTests: XCTestCase {
             )
         }
     }
+
+    func testSanitizingReadOperationFailed_neverReportsRawStreamError() {
+        // Given
+        // `streamError`'s `userInfo` content depends on its domain - `NSCocoaErrorDomain` file errors,
+        // for instance, can include the file path via `NSFilePathErrorKey` - so it must go through the
+        // same central `NSError` sanitization as any other foreign error, not be interpolated directly
+        // into `TLVBlockError`'s description.
+        let sensitiveFilePath = "/var/mobile/Containers/session-\(String.mockRandom(length: 16))/file.dat"
+        let streamError = NSError(
+            domain: NSCocoaErrorDomain,
+            code: 260,
+            userInfo: [NSFilePathErrorKey: sensitiveFilePath]
+        )
+        let error = TLVBlockError.readOperationFailed(streamStatus: .error, streamError: streamError)
+
+        // When
+        let sanitized = error.sanitize()
+
+        // Then
+        XCTAssertFalse(sanitized.message.contains(sensitiveFilePath))
+        XCTAssertTrue(sanitized.message.contains("domain: \(NSCocoaErrorDomain), code: 260"))
+    }
 }

DatadogCore/Tests/Datadog/Core/Upload/DataUploadWorkerTests.swift

@@ -712,7 +712,7 @@ class DataUploadWorkerTests: XCTestCase {
         XCTAssertEqual(telemetry.messages.count, 2)
 
         let error = try XCTUnwrap(telemetry.messages.firstError(), "An error should be send to `telemetry`.")
-        XCTAssertEqual(error.message, #"Data upload finished with error - Error Domain=abc Code=0 "(null)""#)
+        XCTAssertEqual(error.message, "Data upload finished with error - domain: abc, code: 0")
 
         let metric = try XCTUnwrap(telemetry.messages.firstMetric(named: "upload_quality"), "An upload quality metric should be send to `telemetry`.")
         XCTAssertEqual(metric.attributes["failure"] as? String, "\(nserror.code)")
@@ -753,7 +753,7 @@ class DataUploadWorkerTests: XCTestCase {
         XCTAssertEqual(telemetry.messages.count, 2)
 
         let error = try XCTUnwrap(telemetry.messages.firstError(), "An error should be send to `telemetry`.")
-        XCTAssertEqual(error.message, #"Failed to initiate 'some-feature' data upload - Failed to prepare upload"#)
+        XCTAssertEqual(error.message, "Failed to initiate 'some-feature' data upload - ErrorMock does not conform to TelemetrySanitizableError — reporting type name only")
 
         let metric = try XCTUnwrap(telemetry.messages.firstMetric(named: "upload_quality"), "An upload quality metric should be send to `telemetry`.")
         XCTAssertEqual(metric.attributes["failure"] as? String, "invalid")

DatadogInternal/Sources/Telemetry/Telemetry.swift

@@ -321,50 +321,68 @@ extension Telemetry {
             id: "\(file):\(line):\(message)",
             message: message,
             kind: kind ?? "\(file)",
-            stack: stack ?? "\(file):\(line)"
+            stack: stack.map { "\(file):\(line)\n\($0)" } ?? "\(file):\(line)"
         )
     }
 
     /// Collect execution error.
     ///
+    /// - Note: Not exposed publicly — accepts only `TelemetrySanitizedError`, so a raw, unsanitized
+    ///   error description can never reach this sink. Callers should use `error(_ error: Error, ...)`
+    ///   instead, which sanitizes the error via `TelemetrySanitizedError.init(sanitizing:)` before
+    ///   forwarding here.
+    ///
     /// - Parameters:
-    ///   - error: The error.
+    ///   - error: The sanitized error.
     ///   - file: The current file name.
     ///   - line: The line number in file.
-    public func error(_ error: DDError, file: String = #fileID, line: Int = #line) {
-        self.error(error.message, kind: error.type, stack: error.stack, file: file, line: line)
+    func error(_ error: TelemetrySanitizedError, file: String = #fileID, line: Int = #line) {
+        self.error(error.message, kind: error.kind, stack: error.stack, file: file, line: line)
     }
 
     /// Collect execution error.
     ///
+    /// - Note: Not exposed publicly — accepts only `TelemetrySanitizedError`, so a raw, unsanitized
+    ///   error description can never reach this sink. Callers should use
+    ///   `error(_ message: String, error: Error, ...)` instead, which sanitizes the error via
+    ///   `TelemetrySanitizedError.init(sanitizing:)` before forwarding here.
+    ///
     /// - Parameters:
     ///   - message: The error message.
-    ///   - error: The error.
+    ///   - error: The sanitized error.
     ///   - file: The current file name.
     ///   - line: The line number in file.
-    public func error(_ message: String, error: DDError, file: String = #fileID, line: Int = #line) {
-        self.error("\(message) - \(error.message)", kind: error.type, stack: error.stack, file: file, line: line)
+    func error(_ message: String, error: TelemetrySanitizedError, file: String = #fileID, line: Int = #line) {
+        self.error("\(message) - \(error.message)", kind: error.kind, stack: error.stack, file: file, line: line)
     }
 
     /// Collect execution error.
     ///
+    /// - Note: If `error` conforms to `TelemetrySanitizableError`, its own sanitized context is reported.
+    ///   Otherwise, it falls back to Telemetry's default sanitization, which may drop most contextual
+    ///   information to avoid leaking sensitive data.
+    ///
     /// - Parameters:
     ///   - error: The error.
     ///   - file: The current file name.
     ///   - line: The line number in file.
     public func error(_ error: Error, file: String = #fileID, line: Int = #line) {
-        self.error(DDError(error: error), file: file, line: line)
+        self.error(TelemetrySanitizedError(sanitizing: error), file: file, line: line)
     }
 
     /// Collect execution error.
     ///
+    /// - Note: If `error` conforms to `TelemetrySanitizableError`, its own sanitized context is reported.
+    ///   Otherwise, it falls back to Telemetry's default sanitization, which may drop most contextual
+    ///   information to avoid leaking sensitive data.
+    ///
     /// - Parameters:
     ///   - message: The error message.
     ///   - error: The error.
     ///   - file: The current file name.
     ///   - line: The line number in file.
     public func error(_ message: String, error: Error, file: String = #fileID, line: Int = #line) {
-        self.error(message, error: DDError(error: error), file: file, line: line)
+        self.error(message, error: TelemetrySanitizedError(sanitizing: error), file: file, line: line)
     }
 
     /// Report a Configuration Telemetry.