From 738c9f10adbcd27336ed48242318b4b55fde5886 Mon Sep 17 00:00:00 2001 From: christopherkarani Date: Fri, 6 Mar 2026 04:59:12 +0300 Subject: [PATCH 1/7] chore(tasks): add PR consolidation checklist --- tasks/todo.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tasks/todo.md b/tasks/todo.md index 112773d..8c7895d 100644 --- a/tasks/todo.md +++ b/tasks/todo.md @@ -1,3 +1,18 @@ +# Open PR Consolidation on Main (2026-03-06) + +- [ ] Create isolated `main`-based integration branch/worktree for PR consolidation. +- [ ] Inventory open PRs and classify them as contained, focused-value, overlapping audit, or broad/stale. +- [ ] Merge or replay focused-value changes from PR #12 while dropping bookkeeping-only hunks. +- [ ] Merge or replay parser hardening from PR #4 while excluding stale TraceMacApp-only changes. +- [ ] Reconcile overlapping OpenTelemetry/install audit PR value into a single coherent implementation on `main`. +- [ ] Selectively salvage still-applicable source/test/doc value from PR #11 without importing stale app/artifact churn. +- [ ] Run targeted verification after each integration step and final `swift test` before completion. +- [ ] Add review notes summarizing preserved value, skipped stale changes, and residual risks. + +## Review + +- Pending. + # TraceMacApp Extraction Verification - [x] Baseline current repo state and identify remaining TraceMacApp coupling. From 8579dd5ce4a969f8299bf599208d08eb94cc2d43 Mon Sep 17 00:00:00 2001 From: christopherkarani Date: Fri, 6 Mar 2026 05:15:43 +0300 Subject: [PATCH 2/7] fix(tracekit): harden OTLP and trace file handling --- Sources/Terra/Terra+OpenTelemetry.swift | 41 +++++++--- Sources/Terra/Terra+Runtime.swift | 7 +- .../HTTPAIInstrumentation.swift | 2 +- Sources/TerraTraceKit/Compression.swift | 8 ++ Sources/TerraTraceKit/OTLPHTTPServer.swift | 79 +++++++++++-------- Sources/TerraTraceKit/TraceFileLocator.swift | 3 +- Sources/TerraTraceKit/TraceFileReader.swift | 29 ++++--- ...OpenTelemetryInstallConcurrencyTests.swift | 16 +++- .../TerraRedactionPolicyTests.swift | 17 ++++ .../OTLPRequestDecoderTests.swift | 15 ++++ Tests/TerraTraceKitTests/TraceKitTests.swift | 17 ++++ 11 files changed, 173 insertions(+), 61 deletions(-) diff --git a/Sources/Terra/Terra+OpenTelemetry.swift b/Sources/Terra/Terra+OpenTelemetry.swift index 8759021..1cff022 100644 --- a/Sources/Terra/Terra+OpenTelemetry.swift +++ b/Sources/Terra/Terra+OpenTelemetry.swift @@ -126,38 +126,39 @@ extension Terra { public static func installOpenTelemetry(_ configuration: OpenTelemetryConfiguration) throws { openTelemetryInstallLock.lock() defer { openTelemetryInstallLock.unlock() } + let normalizedConfiguration = normalize(configuration) if let installed = installedOpenTelemetryConfiguration { - if installed == configuration { + if installed == normalizedConfiguration { return } throw InstallOpenTelemetryError.alreadyInstalled } - installedOpenTelemetryConfiguration = configuration + installedOpenTelemetryConfiguration = normalizedConfiguration do { - if let persistence = configuration.persistence { + if let persistence = normalizedConfiguration.persistence { try FileManager.default.createDirectory(at: persistence.storageURL, withIntermediateDirectories: true, attributes: nil) } - let tracerProviderSdk = try installTracing(configuration: configuration) + let tracerProviderSdk = try installTracing(configuration: normalizedConfiguration) - if configuration.enableSignposts { + if normalizedConfiguration.enableSignposts { installSignposts(tracerProviderSdk: tracerProviderSdk) } - if configuration.enableLogs { - _ = try installLogs(configuration: configuration) + if normalizedConfiguration.enableLogs { + _ = try installLogs(configuration: normalizedConfiguration) } - if configuration.enableSessions { + if normalizedConfiguration.enableSessions { tracerProviderSdk.addSpanProcessor(TerraSessionSpanProcessor()) SessionEventInstrumentation.install() } - if configuration.enableMetrics { - let meterProvider = try installMetrics(configuration: configuration) + if normalizedConfiguration.enableMetrics { + let meterProvider = try installMetrics(configuration: normalizedConfiguration) Terra.install(.init(privacy: Runtime.shared.privacy, meterProvider: meterProvider, registerProvidersAsGlobal: false)) } } catch { @@ -203,8 +204,12 @@ extension Terra { let resource = makeResource(configuration: configuration) let sampler: Sampler? if let ratio = configuration.traceSamplingRatio { - let clamped = min(max(ratio, 0), 1) - sampler = Samplers.parentBased(root: Samplers.traceIdRatio(ratio: clamped)) + if ratio.isFinite { + let clamped = min(max(ratio, 0), 1) + sampler = Samplers.parentBased(root: Samplers.traceIdRatio(ratio: clamped)) + } else { + sampler = nil + } } else { sampler = nil } @@ -253,6 +258,18 @@ extension Terra { #endif } + private static func normalize(_ configuration: OpenTelemetryConfiguration) -> OpenTelemetryConfiguration { + var normalized = configuration + if let ratio = normalized.traceSamplingRatio { + if ratio.isFinite { + normalized.traceSamplingRatio = min(max(ratio, 0), 1) + } else { + normalized.traceSamplingRatio = nil + } + } + return normalized + } + // MARK: - Metrics private static func installMetrics(configuration: OpenTelemetryConfiguration) throws -> MeterProviderSdk { diff --git a/Sources/Terra/Terra+Runtime.swift b/Sources/Terra/Terra+Runtime.swift index cf525c9..be61e95 100644 --- a/Sources/Terra/Terra+Runtime.swift +++ b/Sources/Terra/Terra+Runtime.swift @@ -206,8 +206,11 @@ private extension Runtime { return Data(bytes) } #endif - let seed = UUID().uuidString + UUID().uuidString - return Data(Data(seed.utf8).prefix(anonymizationKeyLengthBytes)) + var generator = SystemRandomNumberGenerator() + let randomBytes = (0.. Data? { diff --git a/Sources/TerraHTTPInstrument/HTTPAIInstrumentation.swift b/Sources/TerraHTTPInstrument/HTTPAIInstrumentation.swift index 9de0225..209326a 100644 --- a/Sources/TerraHTTPInstrument/HTTPAIInstrumentation.swift +++ b/Sources/TerraHTTPInstrument/HTTPAIInstrumentation.swift @@ -26,7 +26,7 @@ public enum HTTPAIInstrumentation { public static func install( hosts: Set = defaultAIHosts, - openClawGatewayHosts: Set = [], + openClawGatewayHosts: Set = defaultOpenClawGatewayHosts, openClawMode: String = "disabled" ) { lock.lock() diff --git a/Sources/TerraTraceKit/Compression.swift b/Sources/TerraTraceKit/Compression.swift index 26effa5..96b9f56 100644 --- a/Sources/TerraTraceKit/Compression.swift +++ b/Sources/TerraTraceKit/Compression.swift @@ -68,6 +68,7 @@ struct OTLPDecompressor { repeat { stream.dst_ptr = dstBuffer stream.dst_size = dstSize + let previousRemainingInput = stream.src_size streamStatus = compression_stream_process(&stream, 0) @@ -82,8 +83,15 @@ struct OTLPDecompressor { if streamStatus == COMPRESSION_STATUS_ERROR { throw OTLPRequestDecoderError.decompressionFailed(reason: "Zlib decompression error") } + if streamStatus == COMPRESSION_STATUS_OK, produced == 0, stream.src_size == previousRemainingInput { + throw OTLPRequestDecoderError.decompressionFailed(reason: "Incomplete zlib payload") + } } while streamStatus == COMPRESSION_STATUS_OK + guard streamStatus == COMPRESSION_STATUS_END else { + throw OTLPRequestDecoderError.decompressionFailed(reason: "Incomplete zlib payload") + } + return output } } diff --git a/Sources/TerraTraceKit/OTLPHTTPServer.swift b/Sources/TerraTraceKit/OTLPHTTPServer.swift index 0cdde2e..a94b63b 100644 --- a/Sources/TerraTraceKit/OTLPHTTPServer.swift +++ b/Sources/TerraTraceKit/OTLPHTTPServer.swift @@ -43,13 +43,16 @@ public final class OTLPHTTPServer: @unchecked Sendable { private static let maxActiveConnections = 64 private let queue = DispatchQueue(label: "terra.trace.otlp.httpserver") + private let queueSpecificKey = DispatchSpecificKey() private var listener: NWListener? private var activeConnections: [ObjectIdentifier: NWConnection] = [:] private var readTimeoutTimers: [ObjectIdentifier: DispatchSourceTimer] = [:] private var decodeTasks: [ObjectIdentifier: Task] = [:] public var port: UInt16 { - listener?.port?.rawValue ?? configuredPort + onQueueSync { + listener?.port?.rawValue ?? configuredPort + } } public init( @@ -66,56 +69,68 @@ public final class OTLPHTTPServer: @unchecked Sendable { self.traceStore = traceStore self.limits = limits self.onSpans = onSpans + self.queue.setSpecific(key: queueSpecificKey, value: ()) } public func start() throws { - guard listener == nil else { return } - - let parameters = NWParameters.tcp - let listener: NWListener - if configuredPort == 0 { - listener = try NWListener(using: parameters) - } else if let port = NWEndpoint.Port(rawValue: configuredPort) { - if shouldBindToHost(host) { - parameters.requiredLocalEndpoint = .hostPort(host: NWEndpoint.Host(host), port: port) + try onQueueSync { + guard listener == nil else { return } + + let parameters = NWParameters.tcp + let listener: NWListener + if configuredPort == 0 { listener = try NWListener(using: parameters) + } else if let port = NWEndpoint.Port(rawValue: configuredPort) { + if shouldBindToHost(host) { + parameters.requiredLocalEndpoint = .hostPort(host: NWEndpoint.Host(host), port: port) + listener = try NWListener(using: parameters) + } else { + listener = try NWListener(using: parameters, on: port) + } } else { - listener = try NWListener(using: parameters, on: port) + throw NSError(domain: "OTLPHTTPServer", code: 1, userInfo: [NSLocalizedDescriptionKey: "Invalid port"]) } - } else { - throw NSError(domain: "OTLPHTTPServer", code: 1, userInfo: [NSLocalizedDescriptionKey: "Invalid port"]) - } - listener.stateUpdateHandler = { [weak self] (state: NWListener.State) in - if case .failed = state { - self?.stop() + listener.stateUpdateHandler = { [weak self] (state: NWListener.State) in + if case .failed = state { + self?.stop() + } } - } - listener.newConnectionHandler = { [weak self] connection in - self?.handle(connection) - } + listener.newConnectionHandler = { [weak self] connection in + self?.handle(connection) + } - self.listener = listener - listener.start(queue: queue) + self.listener = listener + listener.start(queue: queue) + } } public func stop() { - queue.async { - self.listener?.cancel() - self.listener = nil - for id in Array(self.activeConnections.keys) { - self.cleanupConnection(id: id) + onQueueSync { + listener?.cancel() + listener = nil + for id in Array(activeConnections.keys) { + cleanupConnection(id: id) } } } deinit { - listener?.cancel() - listener = nil - for id in Array(activeConnections.keys) { - cleanupConnection(id: id) + onQueueSync { + listener?.cancel() + listener = nil + for id in Array(activeConnections.keys) { + cleanupConnection(id: id) + } + } + } + + private func onQueueSync(_ work: () throws -> T) rethrows -> T { + if DispatchQueue.getSpecific(key: queueSpecificKey) != nil { + return try work() } + return try queue.sync(execute: work) } private func handle(_ connection: NWConnection) { diff --git a/Sources/TerraTraceKit/TraceFileLocator.swift b/Sources/TerraTraceKit/TraceFileLocator.swift index 06e27fa..99a4052 100644 --- a/Sources/TerraTraceKit/TraceFileLocator.swift +++ b/Sources/TerraTraceKit/TraceFileLocator.swift @@ -24,10 +24,9 @@ public struct TraceFileLocator { let files = urls.compactMap { url -> TraceFileReference? in let name = url.lastPathComponent - guard let milliseconds = UInt64(name) else { + guard let timestamp = TraceFileNameParser.timestamp(from: name) else { return nil } - let timestamp = Date(timeIntervalSinceReferenceDate: TimeInterval(milliseconds) / 1000.0) return TraceFileReference(url: url, fileName: name, timestamp: timestamp) } diff --git a/Sources/TerraTraceKit/TraceFileReader.swift b/Sources/TerraTraceKit/TraceFileReader.swift index 079ac02..1b5c6ff 100644 --- a/Sources/TerraTraceKit/TraceFileReader.swift +++ b/Sources/TerraTraceKit/TraceFileReader.swift @@ -25,23 +25,30 @@ public struct TraceFileReader { } do { - let attributes = try fileManager.attributesOfItem(atPath: url.path) - if let sizeValue = attributes[.size] as? NSNumber { - let size = sizeValue.intValue - if size > maxFileSizeBytes { - throw TraceFileError.fileTooLarge(url, actualBytes: size, maxBytes: maxFileSizeBytes) + let handle = try FileHandle(forReadingFrom: url) + defer { + try? handle.close() + } + + var output = Data() + output.reserveCapacity(min(maxFileSizeBytes, 64 * 1024)) + + while true { + let chunk = try handle.read(upToCount: 64 * 1024) ?? Data() + if chunk.isEmpty { + break } + let projectedSize = output.count + chunk.count + if projectedSize > maxFileSizeBytes { + throw TraceFileError.fileTooLarge(url, actualBytes: projectedSize, maxBytes: maxFileSizeBytes) + } + output.append(chunk) } + return output } catch let fileError as TraceFileError { throw fileError } catch { throw TraceFileError.readFailed(url) } - - do { - return try Data(contentsOf: url) - } catch { - throw TraceFileError.readFailed(url) - } } } diff --git a/Tests/TerraTests/TerraOpenTelemetryInstallConcurrencyTests.swift b/Tests/TerraTests/TerraOpenTelemetryInstallConcurrencyTests.swift index 18e4e0f..67d2e54 100644 --- a/Tests/TerraTests/TerraOpenTelemetryInstallConcurrencyTests.swift +++ b/Tests/TerraTests/TerraOpenTelemetryInstallConcurrencyTests.swift @@ -113,7 +113,7 @@ final class TerraOpenTelemetryInstallConcurrencyTests: XCTestCase { if case .success = $0 { return true } return false }.count - XCTAssertGreaterThanOrEqual(successCount, 1, "At least one concurrent same-config install should succeed") + XCTAssertEqual(successCount, 2, "Both concurrent same-config installs should succeed") // None should throw alreadyInstalled -- same config is allowed let alreadyInstalledCount = results.filter { guard case .failure(let error) = $0 else { return false } @@ -124,6 +124,20 @@ final class TerraOpenTelemetryInstallConcurrencyTests: XCTestCase { XCTAssertEqual(alreadyInstalledCount, 0, "Same-config concurrent install should not throw alreadyInstalled") } + func testInstall_withNonFiniteSamplingRatio_doesNotThrow() { + let config = Terra.OpenTelemetryConfiguration( + enableMetrics: false, + enableLogs: false, + enableSignposts: false, + enableSessions: false, + otlpTracesEndpoint: URL(string: "http://localhost:4380/v1/traces")!, + traceSamplingRatio: .nan + ) + + XCTAssertNoThrow(try Terra.installOpenTelemetry(config)) + XCTAssertNoThrow(try Terra.installOpenTelemetry(config)) + } + func testConcurrentInstall_consistentConfiguration() async { let endpoints = [ URL(string: "http://localhost:4331/v1/traces")!, diff --git a/Tests/TerraTests/TerraRedactionPolicyTests.swift b/Tests/TerraTests/TerraRedactionPolicyTests.swift index 65a82d8..d0ac2a9 100644 --- a/Tests/TerraTests/TerraRedactionPolicyTests.swift +++ b/Tests/TerraTests/TerraRedactionPolicyTests.swift @@ -49,6 +49,23 @@ final class TerraRedactionPolicyTests: XCTestCase { XCTAssertFalse(span.attributes.values.contains { $0.description.contains(prompt) }) } + func testDropRedaction_omitsPromptAttributes_withoutRawPrompt() async throws { + Terra.install( + .init(privacy: .init(contentPolicy: .always, redaction: .drop)) + ) + + let prompt = "secret-prompt" + let request = Terra.InferenceRequest(model: "local/llama-3.2-1b", prompt: prompt) + + await Terra.withInferenceSpan(request) { _ in } + + let span = try XCTUnwrap(support.finishedSpans().first) + XCTAssertNil(span.attributes[Terra.Keys.Terra.promptLength]) + XCTAssertNil(span.attributes[Terra.Keys.Terra.promptHMACSHA256]) + XCTAssertNil(span.attributes[Terra.Keys.Terra.promptSHA256]) + XCTAssertFalse(span.attributes.values.contains { $0.description.contains(prompt) }) + } + func testHMACRedaction_recordsHMACAndLength_withoutRawPrompt() async throws { Terra.install( .init(privacy: .init(contentPolicy: .always, redaction: .hashHMACSHA256)) diff --git a/Tests/TerraTraceKitTests/OTLPRequestDecoderTests.swift b/Tests/TerraTraceKitTests/OTLPRequestDecoderTests.swift index b1c9a28..b48c3a2 100644 --- a/Tests/TerraTraceKitTests/OTLPRequestDecoderTests.swift +++ b/Tests/TerraTraceKitTests/OTLPRequestDecoderTests.swift @@ -50,6 +50,21 @@ final class OTLPRequestDecoderTests: XCTestCase { assertDecodedSpans(spans) } + + func testDecodeRejectsTruncatedDeflatePayload() throws { + let body = try OTLPTestFixtures.serializedRequest() + var compressed = try OTLPTestCompression.deflate(body) + compressed.removeLast() + + let decoder = OTLPRequestDecoder( + maxBodyBytes: compressed.count + 64, + maxDecompressedBytes: body.count + 64 + ) + + XCTAssertThrowsError( + try decoder.decode(body: compressed, headers: ["Content-Encoding": "deflate"]) + ) + } #endif func testDecodeRejectsOversizedDecompressedPayload() throws { diff --git a/Tests/TerraTraceKitTests/TraceKitTests.swift b/Tests/TerraTraceKitTests/TraceKitTests.swift index a8bb20e..4110949 100644 --- a/Tests/TerraTraceKitTests/TraceKitTests.swift +++ b/Tests/TerraTraceKitTests/TraceKitTests.swift @@ -73,6 +73,23 @@ func locatorExcludesNonNumericFiles() throws { #expect(files[0].fileName == "1000000") } +@Test("TraceFileLocator accepts composite names with numeric prefixes") +func locatorIncludesCompositePrefixFiles() throws { + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString, isDirectory: true) + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: dir) } + + try Data("[]".utf8).write(to: dir.appendingPathComponent("999000-abcdef1234")) + try Data("[]".utf8).write(to: dir.appendingPathComponent("hello-world")) + + let locator = TraceFileLocator(tracesDirectoryURL: dir) + let files = try locator.listTraceFiles() + #expect(files.count == 1) + #expect(files[0].fileName == "999000-abcdef1234") + #expect(files[0].timestamp == Date(timeIntervalSinceReferenceDate: 999.0)) +} + @Test("TraceFileLocator returns files sorted by timestamp ascending") func locatorSortsByTimestamp() throws { let dir = FileManager.default.temporaryDirectory From 5fefb8bac37352f50168a2eeadeb70464b68821c Mon Sep 17 00:00:00 2001 From: christopherkarani Date: Fri, 6 Mar 2026 05:19:09 +0300 Subject: [PATCH 3/7] fix(http): harden AI request and response parsing --- .../TerraHTTPInstrument/AIRequestParser.swift | 53 ++++++++++++++++--- .../AIResponseParser.swift | 38 ++++++++++--- .../AIRequestParserTests.swift | 37 +++++++++++++ 3 files changed, 116 insertions(+), 12 deletions(-) diff --git a/Sources/TerraHTTPInstrument/AIRequestParser.swift b/Sources/TerraHTTPInstrument/AIRequestParser.swift index a020975..c542438 100644 --- a/Sources/TerraHTTPInstrument/AIRequestParser.swift +++ b/Sources/TerraHTTPInstrument/AIRequestParser.swift @@ -26,21 +26,19 @@ struct AIRequestParser { let model = json["model"] as? String let maxTokens: Int? - if let v = json["max_tokens"] as? Int { + if let v = intValue(json["max_tokens"]) { maxTokens = v - } else if let v = json["max_completion_tokens"] as? Int { + } else if let v = intValue(json["max_completion_tokens"]) { maxTokens = v - } else if let v = json["max_new_tokens"] as? Int { + } else if let v = intValue(json["max_new_tokens"]) { maxTokens = v } else { maxTokens = nil } let temperature: Double? - if let v = json["temperature"] as? Double { + if let v = doubleValue(json["temperature"]) { temperature = v - } else if let v = json["temperature"] as? Int { - temperature = Double(v) } else { temperature = nil } @@ -66,3 +64,46 @@ struct ParsedRequest: Sendable { let temperature: Double? let stream: Bool? } + +private func intValue(_ value: Any?) -> Int? { + guard let number = value as? NSNumber else { + return nil + } + + if isBoolean(number) { + return nil + } + + let doubleValue = number.doubleValue + guard doubleValue.isFinite else { + return nil + } + + let rounded = doubleValue.rounded() + guard rounded == doubleValue else { + return nil + } + + return Int(rounded) +} + +private func doubleValue(_ value: Any?) -> Double? { + guard let number = value as? NSNumber else { + return nil + } + + if isBoolean(number) { + return nil + } + + let doubleValue = number.doubleValue + guard doubleValue.isFinite else { + return nil + } + + return doubleValue +} + +private func isBoolean(_ number: NSNumber) -> Bool { + CFGetTypeID(number) == CFBooleanGetTypeID() +} diff --git a/Sources/TerraHTTPInstrument/AIResponseParser.swift b/Sources/TerraHTTPInstrument/AIResponseParser.swift index 88299e8..f7d6fa1 100644 --- a/Sources/TerraHTTPInstrument/AIResponseParser.swift +++ b/Sources/TerraHTTPInstrument/AIResponseParser.swift @@ -23,27 +23,27 @@ struct AIResponseParser { if let usage = json["usage"] as? [String: Any] { // OpenAI format - if let promptTokens = usage["prompt_tokens"] as? Int { + if let promptTokens = intValue(usage["prompt_tokens"]) { inputTokens = promptTokens } - if let completionTokens = usage["completion_tokens"] as? Int { + if let completionTokens = intValue(usage["completion_tokens"]) { outputTokens = completionTokens } // Anthropic format (overrides if present, or fills in if OpenAI keys absent) - if let v = usage["input_tokens"] as? Int { + if let v = intValue(usage["input_tokens"]) { inputTokens = v } - if let v = usage["output_tokens"] as? Int { + if let v = intValue(usage["output_tokens"]) { outputTokens = v } } // Ollama format (top-level keys) - if let v = json["prompt_eval_count"] as? Int { + if let v = intValue(json["prompt_eval_count"]) { inputTokens = v } - if let v = json["eval_count"] as? Int { + if let v = intValue(json["eval_count"]) { outputTokens = v } @@ -64,3 +64,29 @@ struct ParsedResponse: Sendable { let outputTokens: Int? let model: String? } + +private func intValue(_ value: Any?) -> Int? { + guard let number = value as? NSNumber else { + return nil + } + + if isBoolean(number) { + return nil + } + + let doubleValue = number.doubleValue + guard doubleValue.isFinite else { + return nil + } + + let rounded = doubleValue.rounded() + guard rounded == doubleValue else { + return nil + } + + return Int(rounded) +} + +private func isBoolean(_ number: NSNumber) -> Bool { + CFGetTypeID(number) == CFBooleanGetTypeID() +} diff --git a/Tests/TerraHTTPInstrumentTests/AIRequestParserTests.swift b/Tests/TerraHTTPInstrumentTests/AIRequestParserTests.swift index f7905c8..771296f 100644 --- a/Tests/TerraHTTPInstrumentTests/AIRequestParserTests.swift +++ b/Tests/TerraHTTPInstrumentTests/AIRequestParserTests.swift @@ -80,6 +80,24 @@ func integerTemperatureIsCoerced() throws { #expect(result.temperature == 1.0) } +@Test("Integral floating max_tokens is accepted") +func floatingMaxTokensIsAccepted() throws { + let body = #"{"model": "gpt-4", "max_tokens": 128.0}"# + let data = try #require(body.data(using: .utf8)) + let result = try #require(AIRequestParser.parse(body: data)) + + #expect(result.maxTokens == 128) +} + +@Test("Boolean max_tokens is ignored") +func booleanMaxTokensIsIgnored() throws { + let body = #"{"max_tokens": true}"# + let data = try #require(body.data(using: .utf8)) + let result = AIRequestParser.parse(body: data) + + #expect(result == nil) +} + @Test("Request body larger than 10 MiB is rejected") func oversizedRequestBodyReturnsNil() throws { let oversizedPayload = Data(repeating: 0x61, count: AIRequestParser.maxBodySizeBytes + 1) @@ -165,3 +183,22 @@ func anthropicTokensOverrideOpenAI() throws { #expect(result.inputTokens == 15) #expect(result.outputTokens == 25) } + +@Test("Integral floating usage tokens are accepted") +func floatingUsageTokensAreAccepted() throws { + let body = #"{"model": "gpt-4", "usage": {"prompt_tokens": 5.0, "completion_tokens": 7.0}}"# + let data = try #require(body.data(using: .utf8)) + let result = try #require(AIResponseParser.parse(data: data)) + + #expect(result.inputTokens == 5) + #expect(result.outputTokens == 7) +} + +@Test("Boolean usage tokens are ignored") +func booleanUsageTokensAreIgnored() throws { + let body = #"{"usage": {"prompt_tokens": true}}"# + let data = try #require(body.data(using: .utf8)) + let result = AIResponseParser.parse(data: data) + + #expect(result == nil) +} From 8b2717d87b14885720e2045e4f7cf3503151b886 Mon Sep 17 00:00:00 2001 From: christopherkarani Date: Fri, 6 Mar 2026 05:37:51 +0300 Subject: [PATCH 4/7] fix(telemetry): reconcile install safety and error privacy --- Sources/Terra/Terra+Constants.swift | 4 + Sources/Terra/Terra+OpenTelemetry.swift | 71 ++++++++++----- Sources/Terra/Terra+Runtime.swift | 18 ++-- Sources/Terra/Terra+Scope.swift | 35 +++++++- .../TerraTests/TerraInferenceSpanTests.swift | 15 +++- Tests/TerraTests/TerraMetricsTests.swift | 17 +++- .../TerraOpenTelemetryDefaultsTests.swift | 20 +++++ ...OpenTelemetryInstallConcurrencyTests.swift | 38 ++++++++ ...rraOpenTelemetryInstallRollbackTests.swift | 63 +++++++++++++ .../TerraTests/TerraRuntimeInstallTests.swift | 27 ++++++ .../TerraScopeErrorPrivacyTests.swift | 90 +++++++++++++++++++ 11 files changed, 359 insertions(+), 39 deletions(-) create mode 100644 Tests/TerraTests/TerraOpenTelemetryDefaultsTests.swift create mode 100644 Tests/TerraTests/TerraOpenTelemetryInstallRollbackTests.swift create mode 100644 Tests/TerraTests/TerraRuntimeInstallTests.swift create mode 100644 Tests/TerraTests/TerraScopeErrorPrivacyTests.swift diff --git a/Sources/Terra/Terra+Constants.swift b/Sources/Terra/Terra+Constants.swift index f0401b3..48e9e19 100644 --- a/Sources/Terra/Terra+Constants.swift +++ b/Sources/Terra/Terra+Constants.swift @@ -84,6 +84,10 @@ extension Terra { /// Legacy compatibility attribute during migration to keyed digests. public static let safetySubjectSHA256 = "terra.safety.subject.sha256" public static let anonymizationKeyID = "terra.anonymization.key_id" + public static let errorMessageLength = "terra.error.message.length" + public static let errorMessageHMACSHA256 = "terra.error.message.hmac_sha256" + /// Legacy compatibility attribute during migration to keyed digests. + public static let errorMessageSHA256 = "terra.error.message.sha256" /// Marks spans created by auto-instrumentation (vs. manual `withInferenceSpan`). public static let autoInstrumented = "terra.auto_instrumented" diff --git a/Sources/Terra/Terra+OpenTelemetry.swift b/Sources/Terra/Terra+OpenTelemetry.swift index 1cff022..d62fdd5 100644 --- a/Sources/Terra/Terra+OpenTelemetry.swift +++ b/Sources/Terra/Terra+OpenTelemetry.swift @@ -46,9 +46,9 @@ extension Terra { enableLogs: Bool = false, enableSignposts: Bool = true, enableSessions: Bool = true, - otlpTracesEndpoint: URL = defaultOltpHttpTracesEndpoint(), + otlpTracesEndpoint: URL = defaultOtlpHttpTracesEndpoint(), otlpMetricsEndpoint: URL = defaultOtlpHttpMetricsEndpoint(), - otlpLogsEndpoint: URL = defaultOltpHttpLoggingEndpoint(), + otlpLogsEndpoint: URL = defaultOtlpHttpLogsEndpoint(), metricsExportInterval: TimeInterval = 60, persistence: PersistenceConfiguration? = nil, serviceName: String? = nil, @@ -107,6 +107,33 @@ extension Terra { .appendingPathComponent("terra", isDirectory: true) } + public static func defaultOtlpHttpTracesEndpoint() -> URL { + URL(string: "http://localhost:4318/v1/traces")! + } + + public static func defaultOtlpHttpMetricsEndpoint() -> URL { + URL(string: "http://localhost:4318/v1/metrics")! + } + + public static func defaultOtlpHttpLogsEndpoint() -> URL { + URL(string: "http://localhost:4318/v1/logs")! + } + + @available(*, deprecated, renamed: "defaultOtlpHttpTracesEndpoint") + public static func defaultOltpHttpTracesEndpoint() -> URL { + defaultOtlpHttpTracesEndpoint() + } + + @available(*, deprecated, renamed: "defaultOtlpHttpMetricsEndpoint") + public static func defaultOltpHttpMetricsEndpoint() -> URL { + defaultOtlpHttpMetricsEndpoint() + } + + @available(*, deprecated, renamed: "defaultOtlpHttpLogsEndpoint") + public static func defaultOltpHttpLoggingEndpoint() -> URL { + defaultOtlpHttpLogsEndpoint() + } + public enum InstallOpenTelemetryError: Error { case alreadyInstalled } @@ -134,17 +161,18 @@ extension Terra { } throw InstallOpenTelemetryError.alreadyInstalled } - - installedOpenTelemetryConfiguration = normalizedConfiguration + let previousTracerProvider = OpenTelemetry.instance.tracerProvider + let previousMeterProvider = OpenTelemetry.instance.meterProvider + let previousLoggerProvider = OpenTelemetry.instance.loggerProvider do { if let persistence = normalizedConfiguration.persistence { try FileManager.default.createDirectory(at: persistence.storageURL, withIntermediateDirectories: true, attributes: nil) } - let tracerProviderSdk = try installTracing(configuration: normalizedConfiguration) + let tracerProvider = try installTracing(configuration: normalizedConfiguration) - if normalizedConfiguration.enableSignposts { + if normalizedConfiguration.enableSignposts, let tracerProviderSdk = tracerProvider as? TracerProviderSdk { installSignposts(tracerProviderSdk: tracerProviderSdk) } @@ -152,7 +180,7 @@ extension Terra { _ = try installLogs(configuration: normalizedConfiguration) } - if normalizedConfiguration.enableSessions { + if normalizedConfiguration.enableSessions, let tracerProviderSdk = tracerProvider as? TracerProviderSdk { tracerProviderSdk.addSpanProcessor(TerraSessionSpanProcessor()) SessionEventInstrumentation.install() } @@ -161,8 +189,12 @@ extension Terra { let meterProvider = try installMetrics(configuration: normalizedConfiguration) Terra.install(.init(privacy: Runtime.shared.privacy, meterProvider: meterProvider, registerProvidersAsGlobal: false)) } + installedOpenTelemetryConfiguration = normalizedConfiguration } catch { installedOpenTelemetryConfiguration = nil + OpenTelemetry.registerTracerProvider(tracerProvider: previousTracerProvider) + OpenTelemetry.registerMeterProvider(meterProvider: previousMeterProvider) + OpenTelemetry.registerLoggerProvider(loggerProvider: previousLoggerProvider) throw error } } @@ -180,7 +212,13 @@ extension Terra { return Resource(attributes: attributes) } - private static func installTracing(configuration: OpenTelemetryConfiguration) throws -> TracerProviderSdk { + private static func installTracing(configuration: OpenTelemetryConfiguration) throws -> any TracerProvider { + guard configuration.enableTraces else { + let provider = DefaultTracerProvider.instance + OpenTelemetry.registerTracerProvider(tracerProvider: provider) + return provider + } + func makeExporter() throws -> any SpanExporter { let baseExporter = OtlpHttpTraceExporter(endpoint: configuration.otlpTracesEndpoint) guard let persistence = configuration.persistence else { @@ -194,13 +232,8 @@ extension Terra { ) } - let spanProcessor: SpanProcessor? - if configuration.enableTraces { - let exporter = try makeExporter() - spanProcessor = SimpleSpanProcessor(spanExporter: exporter) - } else { - spanProcessor = nil - } + let exporter = try makeExporter() + let spanProcessor = SimpleSpanProcessor(spanExporter: exporter) let resource = makeResource(configuration: configuration) let sampler: Sampler? if let ratio = configuration.traceSamplingRatio { @@ -222,9 +255,7 @@ extension Terra { existing.updateActiveSampler(sampler) } existing.addSpanProcessor(TerraSpanEnrichmentProcessor()) - if let spanProcessor { - existing.addSpanProcessor(spanProcessor) - } + existing.addSpanProcessor(spanProcessor) return existing } fallthrough @@ -235,9 +266,7 @@ extension Terra { builder = builder.with(sampler: sampler) } builder = builder.add(spanProcessor: TerraSpanEnrichmentProcessor()) - if let spanProcessor { - builder = builder.add(spanProcessor: spanProcessor) - } + builder = builder.add(spanProcessor: spanProcessor) let provider = builder.build() OpenTelemetry.registerTracerProvider(tracerProvider: provider) return provider diff --git a/Sources/Terra/Terra+Runtime.swift b/Sources/Terra/Terra+Runtime.swift index be61e95..1e3390f 100644 --- a/Sources/Terra/Terra+Runtime.swift +++ b/Sources/Terra/Terra+Runtime.swift @@ -61,12 +61,8 @@ final class Runtime { anonymizationKey = providedAnonymizationKey anonymizationKeyID = Runtime.deriveAnonymizationKeyID(from: providedAnonymizationKey) } - if let tracerProvider = installation.tracerProvider { - tracerProviderOverride = tracerProvider - } - if let loggerProvider = installation.loggerProvider { - loggerProviderOverride = loggerProvider - } + tracerProviderOverride = installation.tracerProvider + loggerProviderOverride = installation.loggerProvider if installation.registerProvidersAsGlobal { if let tracerProvider = installation.tracerProvider { @@ -80,9 +76,7 @@ final class Runtime { } } - if let meterProvider = installation.meterProvider { - metrics.configure(meterProvider: meterProvider) - } + metrics.configure(meterProvider: installation.meterProvider ?? OpenTelemetry.instance.meterProvider) } var privacy: Terra.Privacy { @@ -277,8 +271,6 @@ final class TerraMetrics { inferenceDurationMs = meter.histogramBuilder(name: Terra.MetricNames.inferenceDurationMs).build() } - private static let emptyAttributes: [String: OpenTelemetryApi.AttributeValue] = [:] - func recordInference(durationMs: Double) { // Copy references under the lock. OTel SDK instruments are thread-safe, // so we release the lock before calling add/record to avoid holding it @@ -289,7 +281,7 @@ final class TerraMetrics { var inferenceDurationMs = inferenceDurationMs lock.unlock() - inferenceCount?.add(value: 1, attributes: Self.emptyAttributes) - inferenceDurationMs?.record(value: durationMs, attributes: Self.emptyAttributes) + inferenceCount?.add(value: 1, attributes: [:]) + inferenceDurationMs?.record(value: durationMs, attributes: [:]) } } diff --git a/Sources/Terra/Terra+Scope.swift b/Sources/Terra/Terra+Scope.swift index c78ed3e..becf906 100644 --- a/Sources/Terra/Terra+Scope.swift +++ b/Sources/Terra/Terra+Scope.swift @@ -30,13 +30,13 @@ extension Terra { public func recordError(_ error: any Error, captureMessage: Bool = true) { let message = String(describing: error) let exceptionType = String(reflecting: type(of: error)) - underlyingSpan.status = .error(description: captureMessage ? message : exceptionType) + underlyingSpan.status = .error(description: exceptionType) var attributes: [String: AttributeValue] = [ "exception.type": .string(exceptionType), ] if captureMessage { - attributes["exception.message"] = .string(message) + attributes.merge(redactedErrorAttributes(message: message, privacy: Runtime.shared.privacy)) { _, new in new } } underlyingSpan.addEvent( @@ -51,3 +51,34 @@ extension Terra { } } } + +private func redactedErrorAttributes(message: String, privacy: Terra.Privacy) -> [String: AttributeValue] { + switch privacy.redaction { + case .drop: + return [:] + case .lengthOnly: + return [Terra.Keys.Terra.errorMessageLength: .int(message.count)] + case .hashHMACSHA256: + var attributes: [String: AttributeValue] = [ + Terra.Keys.Terra.errorMessageLength: .int(message.count), + ] + if Runtime.isHMACSHA256Available, let digest = Runtime.shared.hmacSHA256Hex(message) { + attributes[Terra.Keys.Terra.errorMessageHMACSHA256] = .string(digest) + if let keyID = Runtime.shared.anonymizationKeyIDValue { + attributes[Terra.Keys.Terra.anonymizationKeyID] = .string(keyID) + } + } + if privacy.emitLegacySHA256Attributes, Runtime.isSHA256Available, let legacyDigest = Runtime.sha256Hex(message) { + attributes[Terra.Keys.Terra.errorMessageSHA256] = .string(legacyDigest) + } + return attributes + case .hashSHA256: + var attributes: [String: AttributeValue] = [ + Terra.Keys.Terra.errorMessageLength: .int(message.count), + ] + if Runtime.isSHA256Available, let digest = Runtime.sha256Hex(message) { + attributes[Terra.Keys.Terra.errorMessageSHA256] = .string(digest) + } + return attributes + } +} diff --git a/Tests/TerraTests/TerraInferenceSpanTests.swift b/Tests/TerraTests/TerraInferenceSpanTests.swift index 673caba..3305bd2 100644 --- a/Tests/TerraTests/TerraInferenceSpanTests.swift +++ b/Tests/TerraTests/TerraInferenceSpanTests.swift @@ -95,10 +95,13 @@ final class TerraInferenceSpanTests: XCTestCase { let span = try XCTUnwrap(support.finishedSpans().first) let exception = try XCTUnwrap(span.events.first(where: { $0.name == "exception" })) XCTAssertNil(exception.attributes["exception.message"]) + XCTAssertNil(exception.attributes[Terra.Keys.Terra.errorMessageLength]) + XCTAssertNil(exception.attributes[Terra.Keys.Terra.errorMessageHMACSHA256]) + XCTAssertNil(exception.attributes[Terra.Keys.Terra.errorMessageSHA256]) XCTAssertEqual(exception.attributes["exception.type"]?.description, String(reflecting: TestFailure.self)) } - func testWithInferenceSpan_privacyAlways_recordsExceptionMessage() async throws { + func testWithInferenceSpan_privacyAlways_lengthOnlyRecordsOnlyRedactedExceptionMetadata() async throws { Terra.install( .init( privacy: .init(contentPolicy: .always, redaction: .lengthOnly), @@ -118,7 +121,15 @@ final class TerraInferenceSpanTests: XCTestCase { let span = try XCTUnwrap(support.finishedSpans().first) let exception = try XCTUnwrap(span.events.first(where: { $0.name == "exception" })) - XCTAssertEqual(exception.attributes["exception.message"]?.description, TestFailure.secret.description) + XCTAssertNil(exception.attributes["exception.message"]) + XCTAssertEqual( + exception.attributes[Terra.Keys.Terra.errorMessageLength]?.description, + String(TestFailure.secret.description.count) + ) + XCTAssertNil(exception.attributes[Terra.Keys.Terra.errorMessageHMACSHA256]) + XCTAssertNil(exception.attributes[Terra.Keys.Terra.errorMessageSHA256]) XCTAssertEqual(exception.attributes["exception.type"]?.description, String(reflecting: TestFailure.self)) + XCTAssertTrue(span.status.description.contains(String(reflecting: TestFailure.self))) + XCTAssertFalse(span.status.description.contains(TestFailure.secret.description)) } } diff --git a/Tests/TerraTests/TerraMetricsTests.swift b/Tests/TerraTests/TerraMetricsTests.swift index 27fb7b4..e7e7aaa 100644 --- a/Tests/TerraTests/TerraMetricsTests.swift +++ b/Tests/TerraTests/TerraMetricsTests.swift @@ -4,7 +4,6 @@ import OpenTelemetrySdk import XCTest final class TerraMetricsTests: XCTestCase { - // MARK: - Spy infrastructure /// Records all counter.add() and histogram.record() calls for verification. @@ -46,4 +45,20 @@ final class TerraMetricsTests: XCTestCase { metrics.recordInference(durationMs: 0) metrics.recordInference(durationMs: 999.9) } + + func testInstall_withoutExplicitMeterProvider_usesGlobalMeterProvider() { + let meterProvider = MeterProviderSdk.builder().build() + let previousMeterProvider = OpenTelemetry.instance.meterProvider + OpenTelemetry.registerMeterProvider(meterProvider: meterProvider) + defer { + OpenTelemetry.registerMeterProvider(meterProvider: previousMeterProvider) + Terra.install(.init(meterProvider: nil, registerProvidersAsGlobal: false)) + } + + Terra.install(.init(meterProvider: nil, registerProvidersAsGlobal: false)) + + let metricsMirror = Mirror(reflecting: Runtime.shared.metrics) + XCTAssertNotNil(metricsMirror.descendant("inferenceCount")) + XCTAssertNotNil(metricsMirror.descendant("inferenceDurationMs")) + } } diff --git a/Tests/TerraTests/TerraOpenTelemetryDefaultsTests.swift b/Tests/TerraTests/TerraOpenTelemetryDefaultsTests.swift new file mode 100644 index 0000000..b5e8f59 --- /dev/null +++ b/Tests/TerraTests/TerraOpenTelemetryDefaultsTests.swift @@ -0,0 +1,20 @@ +import XCTest + +@testable import TerraCore + +final class TerraOpenTelemetryDefaultsTests: XCTestCase { + func testDefaultOtlpHttpEndpoints() { + XCTAssertEqual( + Terra.defaultOtlpHttpTracesEndpoint().absoluteString, + "http://localhost:4318/v1/traces" + ) + XCTAssertEqual( + Terra.defaultOtlpHttpMetricsEndpoint().absoluteString, + "http://localhost:4318/v1/metrics" + ) + XCTAssertEqual( + Terra.defaultOtlpHttpLogsEndpoint().absoluteString, + "http://localhost:4318/v1/logs" + ) + } +} diff --git a/Tests/TerraTests/TerraOpenTelemetryInstallConcurrencyTests.swift b/Tests/TerraTests/TerraOpenTelemetryInstallConcurrencyTests.swift index 67d2e54..91c718e 100644 --- a/Tests/TerraTests/TerraOpenTelemetryInstallConcurrencyTests.swift +++ b/Tests/TerraTests/TerraOpenTelemetryInstallConcurrencyTests.swift @@ -196,4 +196,42 @@ final class TerraOpenTelemetryInstallConcurrencyTests: XCTestCase { XCTFail("Expected alreadyInstalled but got: \(installError)") } } + + func testInstallOpenTelemetry_failureDoesNotBlockFutureInstall() { + let failingConfig = Terra.OpenTelemetryConfiguration( + enableMetrics: false, + enableLogs: false, + enableSignposts: false, + enableSessions: false, + persistence: .init(storageURL: URL(fileURLWithPath: "/dev/null/terra")) + ) + XCTAssertThrowsError(try Terra.installOpenTelemetry(failingConfig)) + + let validConfig = Terra.OpenTelemetryConfiguration( + enableMetrics: false, + enableLogs: false, + enableSignposts: false, + enableSessions: false, + otlpTracesEndpoint: URL(string: "http://localhost:4333/v1/traces")! + ) + XCTAssertNoThrow(try Terra.installOpenTelemetry(validConfig)) + } + + func testInstallOpenTelemetry_whenTracesDisabled_createsNoSpans() async throws { + let support = TerraTestSupport() + defer { support.reset() } + + let config = Terra.OpenTelemetryConfiguration( + enableTraces: false, + enableMetrics: false, + enableLogs: false, + enableSignposts: false, + enableSessions: false + ) + try Terra.installOpenTelemetry(config) + + await Terra.withInferenceSpan(.init(model: "local/llama-3.2-1b")) { _ in } + + XCTAssertTrue(support.finishedSpans().isEmpty) + } } diff --git a/Tests/TerraTests/TerraOpenTelemetryInstallRollbackTests.swift b/Tests/TerraTests/TerraOpenTelemetryInstallRollbackTests.swift new file mode 100644 index 0000000..27433dc --- /dev/null +++ b/Tests/TerraTests/TerraOpenTelemetryInstallRollbackTests.swift @@ -0,0 +1,63 @@ +import OpenTelemetryApi +import XCTest + +@testable import TerraCore + +final class TerraOpenTelemetryInstallRollbackTests: XCTestCase { + override func setUp() { + super.setUp() + Terra.resetOpenTelemetryForTesting() + } + + override func tearDown() { + Terra.resetOpenTelemetryForTesting() + super.tearDown() + } + + func testInstallFailure_rollsBackGlobalProviders_andAllowsRetry() throws { + let previousTracer = OpenTelemetry.instance.tracerProvider + let previousMeter = OpenTelemetry.instance.meterProvider + let previousLogger = OpenTelemetry.instance.loggerProvider + + let storageURL = FileManager.default.temporaryDirectory + .appendingPathComponent("terra-install-rollback-\(UUID().uuidString)", isDirectory: true) + + try FileManager.default.createDirectory(at: storageURL, withIntermediateDirectories: true) + + let tracesPath = storageURL.appendingPathComponent("traces", isDirectory: false) + XCTAssertTrue(FileManager.default.createFile(atPath: tracesPath.path, contents: Data())) + + let failingConfiguration = Terra.OpenTelemetryConfiguration( + enableTraces: true, + enableMetrics: true, + enableLogs: true, + enableSignposts: false, + enableSessions: false, + persistence: .init(storageURL: storageURL) + ) + + XCTAssertThrowsError(try Terra.installOpenTelemetry(failingConfiguration)) + + XCTAssertTrue(isSameInstance(OpenTelemetry.instance.tracerProvider, previousTracer)) + XCTAssertTrue(isSameInstance(OpenTelemetry.instance.meterProvider, previousMeter)) + XCTAssertTrue(isSameInstance(OpenTelemetry.instance.loggerProvider, previousLogger)) + + let retryStorageURL = FileManager.default.temporaryDirectory + .appendingPathComponent("terra-install-retry-\(UUID().uuidString)", isDirectory: true) + + let retryConfiguration = Terra.OpenTelemetryConfiguration( + enableTraces: true, + enableMetrics: true, + enableLogs: true, + enableSignposts: false, + enableSessions: false, + persistence: .init(storageURL: retryStorageURL) + ) + + XCTAssertNoThrow(try Terra.installOpenTelemetry(retryConfiguration)) + } + + private func isSameInstance(_ lhs: T, _ rhs: T) -> Bool { + ObjectIdentifier(lhs as AnyObject) == ObjectIdentifier(rhs as AnyObject) + } +} diff --git a/Tests/TerraTests/TerraRuntimeInstallTests.swift b/Tests/TerraTests/TerraRuntimeInstallTests.swift new file mode 100644 index 0000000..91ac233 --- /dev/null +++ b/Tests/TerraTests/TerraRuntimeInstallTests.swift @@ -0,0 +1,27 @@ +import OpenTelemetrySdk +import XCTest + +@testable import TerraCore + +final class TerraRuntimeInstallTests: XCTestCase { + func testInstall_clearsProviderOverridesWhenNil() { + let tracerProvider = TracerProviderSdk() + let loggerProvider = LoggerProviderSdk() + + Terra.install( + .init( + tracerProvider: tracerProvider, + loggerProvider: loggerProvider, + registerProvidersAsGlobal: false + ) + ) + + XCTAssertNotNil(Runtime.shared.tracerProvider) + XCTAssertNotNil(Runtime.shared.loggerProvider) + + Terra.install(.init(tracerProvider: nil, loggerProvider: nil, registerProvidersAsGlobal: false)) + + XCTAssertNil(Runtime.shared.tracerProvider) + XCTAssertNil(Runtime.shared.loggerProvider) + } +} diff --git a/Tests/TerraTests/TerraScopeErrorPrivacyTests.swift b/Tests/TerraTests/TerraScopeErrorPrivacyTests.swift new file mode 100644 index 0000000..0afba1d --- /dev/null +++ b/Tests/TerraTests/TerraScopeErrorPrivacyTests.swift @@ -0,0 +1,90 @@ +import XCTest + +@testable import TerraCore + +final class TerraScopeErrorPrivacyTests: XCTestCase { + private var support: TerraTestSupport! + + enum SecretError: Error, CustomStringConvertible { + case leaked(String) + + var description: String { + switch self { + case .leaked(let value): + return "sensitive:\(value)" + } + } + } + + override func setUp() { + super.setUp() + support = TerraTestSupport() + Terra.install(.init(tracerProvider: support.tracerProvider, registerProvidersAsGlobal: false)) + } + + override func tearDown() { + support.reset() + super.tearDown() + } + + func testRecordError_defaultPrivacy_doesNotCaptureSensitiveMessage() async throws { + let secret = "private-token" + let request = Terra.InferenceRequest(model: "local/llama-3.2-1b") + + do { + _ = try await Terra.withInferenceSpan(request) { _ in + throw SecretError.leaked(secret) + } + XCTFail("Expected error") + } catch { + // Expected. + } + + let span = try XCTUnwrap(support.finishedSpans().first) + let exceptionEvent = try XCTUnwrap(span.events.first(where: { $0.name == "exception" })) + + XCTAssertNil(exceptionEvent.attributes["exception.message"]) + XCTAssertNil(exceptionEvent.attributes[Terra.Keys.Terra.errorMessageLength]) + XCTAssertNil(exceptionEvent.attributes[Terra.Keys.Terra.errorMessageHMACSHA256]) + XCTAssertNil(exceptionEvent.attributes[Terra.Keys.Terra.errorMessageSHA256]) + XCTAssertFalse(span.status.description.contains(secret)) + } + + func testRecordError_hmacRedaction_recordsLengthAndDigestWithoutRawMessage() async throws { + Terra.install( + .init( + privacy: .init(contentPolicy: .always, redaction: .hashHMACSHA256), + tracerProvider: support.tracerProvider, + registerProvidersAsGlobal: false + ) + ) + + let secret = "private-token" + let request = Terra.InferenceRequest(model: "local/llama-3.2-1b") + + do { + _ = try await Terra.withInferenceSpan(request) { _ in + throw SecretError.leaked(secret) + } + XCTFail("Expected error") + } catch { + // Expected. + } + + let span = try XCTUnwrap(support.finishedSpans().first) + let exceptionEvent = try XCTUnwrap(span.events.first(where: { $0.name == "exception" })) + + let expectedLength = SecretError.leaked(secret).description.count + XCTAssertEqual(exceptionEvent.attributes[Terra.Keys.Terra.errorMessageLength]?.description, String(expectedLength)) + XCTAssertNil(exceptionEvent.attributes["exception.message"]) + XCTAssertFalse(exceptionEvent.attributes.values.contains { $0.description.contains(secret) }) + + #if canImport(CryptoKit) || canImport(Crypto) + let digest = exceptionEvent.attributes[Terra.Keys.Terra.errorMessageHMACSHA256]?.description + XCTAssertEqual(digest?.count, 64) + XCTAssertNotNil(exceptionEvent.attributes[Terra.Keys.Terra.anonymizationKeyID]) + #else + XCTAssertNil(exceptionEvent.attributes[Terra.Keys.Terra.errorMessageHMACSHA256]) + #endif + } +} From 4fdfe86cad31b41a6dc60bf17c51181b2bd8fd97 Mon Sep 17 00:00:00 2001 From: christopherkarani Date: Fri, 6 Mar 2026 05:37:57 +0300 Subject: [PATCH 5/7] fix(tracekit): surface invalid trace filenames explicitly --- Sources/TerraTraceKit/TraceFileLocator.swift | 27 +++++-- Sources/TerraTraceKit/TraceLoader.swift | 7 +- Tests/TerraTraceKitTests/TraceKitTests.swift | 85 ++++++++++++++++++++ 3 files changed, 112 insertions(+), 7 deletions(-) diff --git a/Sources/TerraTraceKit/TraceFileLocator.swift b/Sources/TerraTraceKit/TraceFileLocator.swift index 99a4052..80a2a30 100644 --- a/Sources/TerraTraceKit/TraceFileLocator.swift +++ b/Sources/TerraTraceKit/TraceFileLocator.swift @@ -1,5 +1,10 @@ import Foundation +public struct TraceFileDiscoveryResult { + public let files: [TraceFileReference] + public let invalidFiles: [URL] +} + /// Discovers persisted trace files under the Terra traces directory. public struct TraceFileLocator { public let tracesDirectoryURL: URL @@ -11,9 +16,13 @@ public struct TraceFileLocator { /// Returns trace files with numeric names (milliseconds since reference date), sorted by time. public func listTraceFiles() throws -> [TraceFileReference] { + try listTraceFilesDetailed().files + } + + public func listTraceFilesDetailed() throws -> TraceFileDiscoveryResult { let fileManager = FileManager.default guard fileManager.fileExists(atPath: tracesDirectoryURL.path) else { - return [] + return TraceFileDiscoveryResult(files: [], invalidFiles: []) } let urls = try fileManager.contentsOfDirectory( @@ -22,15 +31,23 @@ public struct TraceFileLocator { options: [.skipsHiddenFiles] ) - let files = urls.compactMap { url -> TraceFileReference? in + var files: [TraceFileReference] = [] + var invalidFiles: [URL] = [] + files.reserveCapacity(urls.count) + + for url in urls { let name = url.lastPathComponent guard let timestamp = TraceFileNameParser.timestamp(from: name) else { - return nil + invalidFiles.append(url) + continue } - return TraceFileReference(url: url, fileName: name, timestamp: timestamp) + files.append(TraceFileReference(url: url, fileName: name, timestamp: timestamp)) } - return files.sorted { $0.timestamp < $1.timestamp } + return TraceFileDiscoveryResult( + files: files.sorted { $0.timestamp < $1.timestamp }, + invalidFiles: invalidFiles.sorted { $0.lastPathComponent < $1.lastPathComponent } + ) } /// Returns the default Terra traces directory in the user caches folder. diff --git a/Sources/TerraTraceKit/TraceLoader.swift b/Sources/TerraTraceKit/TraceLoader.swift index 1b0e252..a1eff76 100644 --- a/Sources/TerraTraceKit/TraceLoader.swift +++ b/Sources/TerraTraceKit/TraceLoader.swift @@ -29,7 +29,8 @@ public struct TraceLoader { /// Loads and groups spans into trace models, reporting per-file failures. public func loadTracesWithFailures(maxFiles: Int? = nil) throws -> TraceLoadResult { - let files = try locator.listTraceFiles() + let discovery = try locator.listTraceFilesDetailed() + let files = discovery.files let filesToProcess: [TraceFileReference] if let maxFiles { let boundedMax = max(0, maxFiles) @@ -39,7 +40,9 @@ public struct TraceLoader { } var traces = [Trace]() - var failures = [(file: URL, error: Error)]() + var failures: [(file: URL, error: Error)] = discovery.invalidFiles.map { invalidFile in + (file: invalidFile, error: TraceModelError.invalidFileName) + } for file in filesToProcess { let spans: [SpanData] diff --git a/Tests/TerraTraceKitTests/TraceKitTests.swift b/Tests/TerraTraceKitTests/TraceKitTests.swift index 4110949..fc2d267 100644 --- a/Tests/TerraTraceKitTests/TraceKitTests.swift +++ b/Tests/TerraTraceKitTests/TraceKitTests.swift @@ -90,6 +90,23 @@ func locatorIncludesCompositePrefixFiles() throws { #expect(files[0].timestamp == Date(timeIntervalSinceReferenceDate: 999.0)) } +@Test("TraceFileLocator detailed listing includes invalid filenames") +func locatorDetailedListingIncludesInvalidFilenames() throws { + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString, isDirectory: true) + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: dir) } + + try Data("[]".utf8).write(to: dir.appendingPathComponent("1000000")) + try Data("[]".utf8).write(to: dir.appendingPathComponent("invalid-name")) + + let locator = TraceFileLocator(tracesDirectoryURL: dir) + let result = try locator.listTraceFilesDetailed() + + #expect(result.files.count == 1) + #expect(result.invalidFiles.map(\.lastPathComponent) == ["invalid-name"]) +} + @Test("TraceFileLocator returns files sorted by timestamp ascending") func locatorSortsByTimestamp() throws { let dir = FileManager.default.temporaryDirectory @@ -238,6 +255,48 @@ func loaderHandlesMixedFiles() throws { #expect(result.totalFileCount == 2) } +@Test("TraceLoader surfaces invalid filenames as explicit failures") +func loaderSurfacesInvalidFilenamesAsFailures() throws { + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString, isDirectory: true) + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: dir) } + + let traceId = TraceId() + let span = makeSpan(name: "root", traceId: traceId) + try writeSpanFile(spans: [span], to: dir.appendingPathComponent("1000000")) + try Data("[]".utf8).write(to: dir.appendingPathComponent("invalid-file-name")) + + let loader = TraceLoader(locator: TraceFileLocator(tracesDirectoryURL: dir)) + let result = try loader.loadTracesWithFailures() + + #expect(result.traces.count == 1) + #expect(result.failures.count == 1) + #expect(result.failures[0].file.lastPathComponent == "invalid-file-name") + #expect(result.failures[0].error as? TraceModelError == .invalidFileName) +} + +@Test("TraceLoader preserves duplicate span ID failures") +func loaderPreservesDuplicateSpanIDFailures() throws { + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString, isDirectory: true) + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: dir) } + + let traceId = TraceId() + let duplicateSpanID = SpanId.random() + let spanA = makeSpan(name: "a", traceId: traceId, spanId: duplicateSpanID) + let spanB = makeSpan(name: "b", traceId: traceId, spanId: duplicateSpanID) + try writeSpanFile(spans: [spanA, spanB], to: dir.appendingPathComponent("1000000")) + + let loader = TraceLoader(locator: TraceFileLocator(tracesDirectoryURL: dir)) + let result = try loader.loadTracesWithFailures() + + #expect(result.traces.isEmpty) + #expect(result.failures.count == 1) + #expect(result.failures[0].error as? TraceModelError == .duplicateSpanIds) +} + @Test("TraceLoader maxFiles loads only the newest files") func loaderRespectsMaxFilesNewestFirst() throws { let dir = FileManager.default.temporaryDirectory @@ -258,6 +317,32 @@ func loaderRespectsMaxFilesNewestFirst() throws { #expect(result.totalFileCount == 2) } +@Test("TraceLoader pagination path stays within performance budget") +func loaderPaginationPerformanceBudget() throws { + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString, isDirectory: true) + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: dir) } + + for index in 0..<250 { + let traceId = TraceId() + let span = makeSpan(name: "span-\(index)", traceId: traceId) + try writeSpanFile( + spans: [span], + to: dir.appendingPathComponent("\(1_000_000 + index)") + ) + } + + let loader = TraceLoader(locator: TraceFileLocator(tracesDirectoryURL: dir)) + let clock = ContinuousClock() + let elapsed = clock.measure { + let result = try? loader.loadTracesWithFailures(maxFiles: 75) + #expect(result?.traces.count == 75) + } + + #expect(elapsed < .seconds(2)) +} + @Test("TraceLoader returns empty for non-existent directory") func loaderReturnsEmptyForMissingDir() throws { let dir = FileManager.default.temporaryDirectory From 18fcb20f5ed9f07106f10699c787c3cfd86d57ec Mon Sep 17 00:00:00 2001 From: christopherkarani Date: Fri, 6 Mar 2026 05:42:40 +0300 Subject: [PATCH 6/7] fix(telemetry): preserve overrides across partial installs --- Sources/Terra/Terra+Runtime.swift | 8 ++++-- .../TerraTests/TerraRuntimeInstallTests.swift | 27 ------------------- tasks/todo.md | 25 ++++++++++------- 3 files changed, 22 insertions(+), 38 deletions(-) delete mode 100644 Tests/TerraTests/TerraRuntimeInstallTests.swift diff --git a/Sources/Terra/Terra+Runtime.swift b/Sources/Terra/Terra+Runtime.swift index 1e3390f..f4d4836 100644 --- a/Sources/Terra/Terra+Runtime.swift +++ b/Sources/Terra/Terra+Runtime.swift @@ -61,8 +61,12 @@ final class Runtime { anonymizationKey = providedAnonymizationKey anonymizationKeyID = Runtime.deriveAnonymizationKeyID(from: providedAnonymizationKey) } - tracerProviderOverride = installation.tracerProvider - loggerProviderOverride = installation.loggerProvider + if let tracerProvider = installation.tracerProvider { + tracerProviderOverride = tracerProvider + } + if let loggerProvider = installation.loggerProvider { + loggerProviderOverride = loggerProvider + } if installation.registerProvidersAsGlobal { if let tracerProvider = installation.tracerProvider { diff --git a/Tests/TerraTests/TerraRuntimeInstallTests.swift b/Tests/TerraTests/TerraRuntimeInstallTests.swift deleted file mode 100644 index 91ac233..0000000 --- a/Tests/TerraTests/TerraRuntimeInstallTests.swift +++ /dev/null @@ -1,27 +0,0 @@ -import OpenTelemetrySdk -import XCTest - -@testable import TerraCore - -final class TerraRuntimeInstallTests: XCTestCase { - func testInstall_clearsProviderOverridesWhenNil() { - let tracerProvider = TracerProviderSdk() - let loggerProvider = LoggerProviderSdk() - - Terra.install( - .init( - tracerProvider: tracerProvider, - loggerProvider: loggerProvider, - registerProvidersAsGlobal: false - ) - ) - - XCTAssertNotNil(Runtime.shared.tracerProvider) - XCTAssertNotNil(Runtime.shared.loggerProvider) - - Terra.install(.init(tracerProvider: nil, loggerProvider: nil, registerProvidersAsGlobal: false)) - - XCTAssertNil(Runtime.shared.tracerProvider) - XCTAssertNil(Runtime.shared.loggerProvider) - } -} diff --git a/tasks/todo.md b/tasks/todo.md index 8c7895d..565108e 100644 --- a/tasks/todo.md +++ b/tasks/todo.md @@ -1,17 +1,24 @@ # Open PR Consolidation on Main (2026-03-06) -- [ ] Create isolated `main`-based integration branch/worktree for PR consolidation. -- [ ] Inventory open PRs and classify them as contained, focused-value, overlapping audit, or broad/stale. -- [ ] Merge or replay focused-value changes from PR #12 while dropping bookkeeping-only hunks. -- [ ] Merge or replay parser hardening from PR #4 while excluding stale TraceMacApp-only changes. -- [ ] Reconcile overlapping OpenTelemetry/install audit PR value into a single coherent implementation on `main`. -- [ ] Selectively salvage still-applicable source/test/doc value from PR #11 without importing stale app/artifact churn. -- [ ] Run targeted verification after each integration step and final `swift test` before completion. -- [ ] Add review notes summarizing preserved value, skipped stale changes, and residual risks. +- [x] Create isolated `main`-based integration branch/worktree for PR consolidation. +- [x] Inventory open PRs and classify them as contained, focused-value, overlapping audit, or broad/stale. +- [x] Merge or replay focused-value changes from PR #12 while dropping bookkeeping-only hunks. +- [x] Merge or replay parser hardening from PR #4 while excluding stale TraceMacApp-only changes. +- [x] Reconcile overlapping OpenTelemetry/install audit PR value into a single coherent implementation on `main`. +- [x] Selectively salvage still-applicable source/test/doc value from PR #11 without importing stale app/artifact churn. +- [x] Run targeted verification after each integration step and final `swift test` before completion. +- [x] Add review notes summarizing preserved value, skipped stale changes, and residual risks. ## Review -- Pending. +- Created isolated consolidation worktree at `/tmp/terra-pr-consolidation` on branch `pr-consolidation-main`, leaving the user’s existing `api-design` branch untouched. +- PR #2 (`macApp`) and PR #7 (`codex/he`) were already effectively contained on `main`; no replay was needed. +- Preserved PR #12 value in `8579dd5` by replaying the live TraceKit/OpenTelemetry hardening and dropping imported task-bookkeeping hunks. +- Preserved PR #4 value in `5fefb8b` by porting request/response parser hardening and skipping stale TraceMacApp-only changes. +- Reconciled overlapping telemetry/install audit value from PRs #3, #5, #6, #8, #9, and #10 in `8b2717d`, then followed up by preserving prior partial-install override semantics so tracer/logger overrides are not cleared by unrelated `Terra.install(.init(...))` calls. +- Selectively salvaged still-relevant TraceKit filename/discovery behavior from PR #11 in `4fdfe86` and intentionally skipped stale app/UI/artifact churn. +- Verification: `swift build` passes and full `swift test` passes on the consolidation branch. +- Residual risk is limited to intentionally skipped stale branch content that no longer matches the current repo layout or would regress current runtime behavior. # TraceMacApp Extraction Verification From 5381f173b2f11185171372640e3ffc495841fbf2 Mon Sep 17 00:00:00 2001 From: christopherkarani Date: Fri, 6 Mar 2026 06:24:49 +0300 Subject: [PATCH 7/7] docs(tasks): record replacement PR closure --- tasks/todo.md | 1 + 1 file changed, 1 insertion(+) diff --git a/tasks/todo.md b/tasks/todo.md index 565108e..44a5458 100644 --- a/tasks/todo.md +++ b/tasks/todo.md @@ -17,6 +17,7 @@ - Preserved PR #4 value in `5fefb8b` by porting request/response parser hardening and skipping stale TraceMacApp-only changes. - Reconciled overlapping telemetry/install audit value from PRs #3, #5, #6, #8, #9, and #10 in `8b2717d`, then followed up by preserving prior partial-install override semantics so tracer/logger overrides are not cleared by unrelated `Terra.install(.init(...))` calls. - Selectively salvaged still-relevant TraceKit filename/discovery behavior from PR #11 in `4fdfe86` and intentionally skipped stale app/UI/artifact churn. +- Opened replacement GitHub PR `#15` from `pr-consolidation-main` and closed superseded PRs `#2` through `#12` with a reference back to `#15`. - Verification: `swift build` passes and full `swift test` passes on the consolidation branch. - Residual risk is limited to intentionally skipped stale branch content that no longer matches the current repo layout or would regress current runtime behavior.