From a3b53662ffb3fb5e9f3f581622c418d96356225a Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Mon, 14 Jul 2025 17:17:17 +0200 Subject: [PATCH 01/20] implement logs batcher --- Sources/Sentry/SentrySDK.m | 7 +- Sources/Swift/Tools/SentryLogBatcher.swift | 82 +++++- Tests/SentryTests/SentryLogBatcherTests.swift | 243 +++++++++++++++++- Tests/SentryTests/SentryLoggerTests.swift | 8 +- Tests/SentryTests/SentrySDKTests.swift | 5 +- 5 files changed, 325 insertions(+), 20 deletions(-) diff --git a/Sources/Sentry/SentrySDK.m b/Sources/Sentry/SentrySDK.m index c1b8c6dc2a..33028425d8 100644 --- a/Sources/Sentry/SentrySDK.m +++ b/Sources/Sentry/SentrySDK.m @@ -117,14 +117,19 @@ + (SentryReplayApi *)replay + (SentryLogger *)logger { + @synchronized(currentLoggerLock) { if (currentLogger == nil) { + SentryLogBatcher *batcher; if (nil != currentHub.client && currentHub.client.options.experimental.enableLogs) { - batcher = [[SentryLogBatcher alloc] initWithClient:currentHub.client]; + batcher = [[SentryLogBatcher alloc] + initWithClient:currentHub.client + dispatchQueue:SentryDependencyContainer.sharedInstance.dispatchQueueWrapper]; } else { batcher = nil; } + currentLogger = [[SentryLogger alloc] initWithHub:currentHub dateProvider:SentryDependencyContainer.sharedInstance.dateProvider diff --git a/Sources/Swift/Tools/SentryLogBatcher.swift b/Sources/Swift/Tools/SentryLogBatcher.swift index a64a396e68..32ad3eb81f 100644 --- a/Sources/Swift/Tools/SentryLogBatcher.swift +++ b/Sources/Swift/Tools/SentryLogBatcher.swift @@ -6,14 +6,92 @@ import Foundation @_spi(Private) public class SentryLogBatcher: NSObject { private let client: SentryClient + private let flushTimeout: TimeInterval + private let maxBufferSize: Int + private let dispatchQueue: SentryDispatchQueueWrapper - @_spi(Private) public init(client: SentryClient) { + private var logBuffer: [SentryLog] = [] + private let logBufferLock = NSLock() + private var currentFlushId: UUID? + + @_spi(Private) public init( + client: SentryClient, + flushTimeout: TimeInterval, + maxBufferSize: Int, + dispatchQueue: SentryDispatchQueueWrapper + ) { self.client = client + self.flushTimeout = flushTimeout + self.maxBufferSize = maxBufferSize + self.dispatchQueue = dispatchQueue super.init() } + @_spi(Private) public convenience init(client: SentryClient, dispatchQueue: SentryDispatchQueueWrapper) { + self.init( + client: client, + flushTimeout: 5, + maxBufferSize: 100, + dispatchQueue: dispatchQueue + ) + } + func add(_ log: SentryLog) { - dispatch(logs: [log]) + cancelFlush() + + let shouldFlush = logBufferLock.synchronized { + logBuffer.append(log) + return logBuffer.count >= maxBufferSize + } + + if !shouldFlush { + scheduleFlush() + } else { + flush() + } + } + + @objc + public func flush() { + cancelFlush() + + let logs = logBufferLock.synchronized { + let logs = Array(logBuffer) + logBuffer.removeAll() + return logs + } + + if !logs.isEmpty { + dispatch(logs: logs) + } + } + + private func scheduleFlush() { + let flushId = UUID() + + logBufferLock.synchronized { + currentFlushId = flushId + } + + dispatchQueue.dispatch(after: flushTimeout) { [weak self] in + self?.executeFlushIfMatching(flushId: flushId) + } + } + + private func executeFlushIfMatching(flushId: UUID) { + let shouldFlush = logBufferLock.synchronized { + return currentFlushId == flushId + } + + if shouldFlush { + flush() + } + } + + private func cancelFlush() { + logBufferLock.synchronized { + currentFlushId = nil + } } private func dispatch(logs: [SentryLog]) { diff --git a/Tests/SentryTests/SentryLogBatcherTests.swift b/Tests/SentryTests/SentryLogBatcherTests.swift index 2fc925d46f..24549c9f1f 100644 --- a/Tests/SentryTests/SentryLogBatcherTests.swift +++ b/Tests/SentryTests/SentryLogBatcherTests.swift @@ -6,6 +6,7 @@ class SentryLogBatcherTests: XCTestCase { private var options: Options! private var testClient: TestClient! + private var testDispatchQueue: TestSentryDispatchQueueWrapper! private var sut: SentryLogBatcher! private var scope: Scope! @@ -16,40 +17,262 @@ class SentryLogBatcherTests: XCTestCase { options.experimental.enableLogs = true testClient = TestClient(options: options) - sut = SentryLogBatcher(client: testClient) + testDispatchQueue = TestSentryDispatchQueueWrapper() + sut = SentryLogBatcher( + client: testClient, + flushTimeout: 5.0, + maxBufferSize: 3, + dispatchQueue: testDispatchQueue + ) scope = Scope() } override func tearDown() { super.tearDown() testClient = nil + testDispatchQueue = nil sut = nil scope = nil } - // MARK: - ProcessLog Tests + // MARK: - Basic Functionality Tests + + func testAddMultipleLogs_BatchesTogether() throws { + // Given + let log1 = createTestLog(body: "Log 1") + let log2 = createTestLog(body: "Log 2") + + // When + sut.add(log1) + sut.add(log2) + + // Then - no immediate flush since buffer not full + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 0) + + // Trigger flush manually + sut.flush() + + // Verify both logs are batched together + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) + let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first) + let jsonObject = try XCTUnwrap(JSONSerialization.jsonObject(with: sentData) as? [String: Any]) + let items = try XCTUnwrap(jsonObject["items"] as? [[String: Any]]) + XCTAssertEqual(2, items.count) + } + + // MARK: - Buffer Size Tests - func testAddLog_WithValidLog_CallsCaptureLogsData() throws { + func testBufferReachesMaxSize_FlushesImmediately() throws { + // Given + let log1 = createTestLog(body: "Log 1") + let log2 = createTestLog(body: "Log 2") + let log3 = createTestLog(body: "Log 3") + + // When - add logs up to max buffer size (3) + sut.add(log1) + sut.add(log2) + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 0) + + sut.add(log3) // This should trigger immediate flush + + // Then + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) + + // Verify all three logs are batched together + let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first) + let jsonObject = try XCTUnwrap(JSONSerialization.jsonObject(with: sentData) as? [String: Any]) + let items = try XCTUnwrap(jsonObject["items"] as? [[String: Any]]) + XCTAssertEqual(3, items.count) + } + + // MARK: - Timeout Tests + + func testTimeout_FlushesAfterDelay() throws { // Given let log = createTestLog() // When sut.add(log) - // Then + // Then - no immediate flush + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 0) + + // Called dispatch with correct interval + let invocation = try XCTUnwrap(testDispatchQueue.dispatchAfterInvocations.first) + XCTAssertEqual(invocation.interval, 5.0) + + // Simulate timeout by executing the scheduled block + let scheduledBlock = invocation.block + scheduledBlock() + + // Verify flush occurred XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) + } + + func testMultipleLogsBeforeTimeout_CancelsAndReschedulesFlush() throws { + // Given + let log1 = createTestLog(body: "Log 1") + let log2 = createTestLog(body: "Log 2") + + // When + sut.add(log1) + XCTAssertEqual(testDispatchQueue.dispatchAfterInvocations.count, 1) + + sut.add(log2) + XCTAssertEqual(testDispatchQueue.dispatchAfterInvocations.count, 2) + + // Execute the first scheduled flush (should do nothing due to cancellation) + let firstBlock = testDispatchQueue.dispatchAfterInvocations.invocations[0].block + firstBlock() + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 0, "First flush should be cancelled") + + // Execute second scheduled flush + let secondBlock = testDispatchQueue.dispatchAfterInvocations.invocations[1].block + secondBlock() + // Verify both logs are flushed together + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first) + let jsonObject = try XCTUnwrap(JSONSerialization.jsonObject(with: sentData) as? [String: Any]) + let items = try XCTUnwrap(jsonObject["items"] as? [[String: Any]]) + XCTAssertEqual(2, items.count) + } + + // MARK: - Manual Flush Tests + + func testManualFlush_FlushesImmediately() throws { + // Given + let log1 = createTestLog(body: "Log 1") + let log2 = createTestLog(body: "Log 2") + + // When + sut.add(log1) + sut.add(log2) + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 0) - // Verify the data contains the expected JSON structure + sut.flush() + + // Then + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) + + let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first) let jsonObject = try XCTUnwrap(JSONSerialization.jsonObject(with: sentData) as? [String: Any]) let items = try XCTUnwrap(jsonObject["items"] as? [[String: Any]]) - XCTAssertEqual(1, items.count) + XCTAssertEqual(2, items.count) + } + + func testManualFlush_CancelsScheduledFlush() throws { + // Given + let log = createTestLog() + + // When + sut.add(log) + XCTAssertEqual(testDispatchQueue.dispatchAfterInvocations.count, 1) + + sut.flush() + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1, "Manual flush should work") + + // Execute the scheduled flush (should do nothing since buffer was already flushed) + let scheduledBlock = try XCTUnwrap(testDispatchQueue.dispatchAfterInvocations.invocations.first).block + scheduledBlock() + + // Then - no additional flush should occur (proves cancellation worked) + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1, "Scheduled flush should be cancelled") + } + + func testFlushEmptyBuffer_DoesNothing() { + // When + sut.flush() + + // Then + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 0) + } + + // MARK: - Edge Cases Tests + + func testScheduledFlushAfterBufferAlreadyFlushed_DoesNothing() throws { + // Given + let log1 = createTestLog(body: "Log 1") + let log2 = createTestLog(body: "Log 2") + let log3 = createTestLog(body: "Log 3") + + // When - fill buffer to trigger immediate flush + sut.add(log1) + sut.add(log2) + sut.add(log3) + + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) + + // Simulate a delayed flush callback (should do nothing since buffer is empty) + if let delayedBlock = testDispatchQueue.dispatchAfterInvocations.first?.block { + delayedBlock() + } - let firstLog = items[0] - XCTAssertEqual(1_627_846_801, firstLog["timestamp"] as? TimeInterval) - XCTAssertEqual("info", firstLog["level"] as? String) - XCTAssertEqual("Test log message", firstLog["body"] as? String) + // Then - no additional flush should occur + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) + } + + func testAddLogAfterFlush_StartsNewBatch() throws { + // Given + let log1 = createTestLog(body: "Log 1") + let log2 = createTestLog(body: "Log 2") + + // When + sut.add(log1) + sut.flush() + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) + + sut.add(log2) + sut.flush() + + // Then - should have two separate flush calls + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 2) + + // Verify each flush contains only one log + for (index, data) in testClient.captureLogsDataInvocations.invocations.enumerated() { + let jsonObject = try XCTUnwrap(JSONSerialization.jsonObject(with: data) as? [String: Any]) + let items = try XCTUnwrap(jsonObject["items"] as? [[String: Any]]) + XCTAssertEqual(1, items.count) + XCTAssertEqual("Log \(index + 1)", items[0]["body"] as? String) + } + } + + // MARK: - Thread Safety Tests + + func testConcurrentAdds_ThreadSafe() throws { + // Given + let sutWithRealQueue = SentryLogBatcher( + client: testClient, + flushTimeout: 5, + maxBufferSize: 100, // Large buffer to avoid immediate flushes + dispatchQueue: SentryDispatchQueueWrapper() + ) + + let expectation = XCTestExpectation(description: "Concurrent adds") + expectation.expectedFulfillmentCount = 10 + + // When - add logs concurrently from multiple threads + for i in 0..<10 { + DispatchQueue.global().async { + let log = self.createTestLog(body: "Log \(i)") + sutWithRealQueue.add(log) + expectation.fulfill() + } + } + + wait(for: [expectation], timeout: 5.0) + + // Then - manually flush and verify all logs were added + sutWithRealQueue.flush() + + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) + + // Verify all 10 logs were included in the single batch + let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first) + let jsonObject = try XCTUnwrap(JSONSerialization.jsonObject(with: sentData) as? [String: Any]) + let items = try XCTUnwrap(jsonObject["items"] as? [[String: Any]]) + XCTAssertEqual(10, items.count, "All 10 concurrently added logs should be in the batch") + // Note: We can't verify exact order due to concurrency, but count should be correct } // MARK: - Helper Methods diff --git a/Tests/SentryTests/SentryLoggerTests.swift b/Tests/SentryTests/SentryLoggerTests.swift index c039f2d7c6..213ccaf3d1 100644 --- a/Tests/SentryTests/SentryLoggerTests.swift +++ b/Tests/SentryTests/SentryLoggerTests.swift @@ -20,7 +20,7 @@ final class SentryLoggerTests: XCTestCase { scope = Scope() hub = TestHub(client: client, andScope: scope) dateProvider = TestCurrentDateProvider() - batcher = TestLogBatcher(client: client) + batcher = TestLogBatcher(client: client, dispatchQueue: TestSentryDispatchQueueWrapper()) dateProvider.setDate(date: Date(timeIntervalSince1970: 1_627_846_800.123456)) } @@ -331,11 +331,7 @@ final class SentryLoggerTests: XCTestCase { class TestLogBatcher: SentryLogBatcher { var addInvocations = Invocations() - - override init(client: SentryClient) { - super.init(client: client) - } - + override func add(_ log: SentryLog) { addInvocations.record(log) } diff --git a/Tests/SentryTests/SentrySDKTests.swift b/Tests/SentryTests/SentrySDKTests.swift index 14cdb6bf55..da1bae4216 100644 --- a/Tests/SentryTests/SentrySDKTests.swift +++ b/Tests/SentryTests/SentrySDKTests.swift @@ -908,7 +908,10 @@ class SentrySDKTests: XCTestCase { fixture.client.options.experimental.enableLogs = true givenSdkWithHub() - SentrySDK.logger.error("foo") + for index in (0..<100) { + SentrySDK.logger.error("foo \(index)") + } + XCTAssertEqual(fixture.client.captureLogsDataInvocations.count, 1) } From dd3061673a3962fa387058a29d0ba137523e6c4b Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Mon, 14 Jul 2025 17:26:11 +0200 Subject: [PATCH 02/20] add prnr to changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45e6a84883..051020b85d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Features -- Add experimental support for capturing structured logs via `SentrySDK.logger` (#5532) +- Add experimental support for capturing structured logs via `SentrySDK.logger` (#5532, #5593) ### Improvements From 61175bb7dbfc50b2e62cc22a2412f4b0012f807f Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Wed, 16 Jul 2025 10:28:32 +0200 Subject: [PATCH 03/20] make test class final --- Tests/SentryTests/SentryLogBatcherTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/SentryTests/SentryLogBatcherTests.swift b/Tests/SentryTests/SentryLogBatcherTests.swift index 24549c9f1f..beb4b49f00 100644 --- a/Tests/SentryTests/SentryLogBatcherTests.swift +++ b/Tests/SentryTests/SentryLogBatcherTests.swift @@ -2,7 +2,7 @@ @_spi(Private) import SentryTestUtils import XCTest -class SentryLogBatcherTests: XCTestCase { +final class SentryLogBatcherTests: XCTestCase { private var options: Options! private var testClient: TestClient! From aca9fc46db64412d4612f2d133da0cf04279e3b3 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Thu, 17 Jul 2025 13:28:02 +0200 Subject: [PATCH 04/20] reverse if --- Sources/Swift/Tools/SentryLogBatcher.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/Swift/Tools/SentryLogBatcher.swift b/Sources/Swift/Tools/SentryLogBatcher.swift index 32ad3eb81f..6ee4086310 100644 --- a/Sources/Swift/Tools/SentryLogBatcher.swift +++ b/Sources/Swift/Tools/SentryLogBatcher.swift @@ -44,10 +44,10 @@ import Foundation return logBuffer.count >= maxBufferSize } - if !shouldFlush { - scheduleFlush() - } else { + if shouldFlush { flush() + } else { + scheduleFlush() } } From fb3feae2737a08ee0ec5e19ab622b045c737b4ee Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Thu, 17 Jul 2025 14:39:00 +0200 Subject: [PATCH 05/20] refactor batch logic, add missing item count o envelope --- Sources/Sentry/SentryClient.m | 5 +- Sources/Sentry/include/SentryClient+Logs.h | 2 +- Sources/Swift/Tools/SentryLogBatcher.swift | 134 ++++++++++++--------- 3 files changed, 79 insertions(+), 62 deletions(-) diff --git a/Sources/Sentry/SentryClient.m b/Sources/Sentry/SentryClient.m index 8655a979c1..e2453c2231 100644 --- a/Sources/Sentry/SentryClient.m +++ b/Sources/Sentry/SentryClient.m @@ -1128,12 +1128,13 @@ - (void)removeAttachmentProcessor:(id)attachmen return processedAttachments; } -- (void)captureLogsData:(NSData *)data +- (void)captureLogsData:(NSData *)data with:(NSNumber *)itemCount; { SentryEnvelopeItemHeader *header = [[SentryEnvelopeItemHeader alloc] initWithType:SentryEnvelopeItemTypeLog length:data.length - contentType:@"application/vnd.sentry.items.log+json"]; + contentType:@"application/vnd.sentry.items.log+json" + itemCount: itemCount]; SentryEnvelopeItem *envelopeItem = [[SentryEnvelopeItem alloc] initWithHeader:header data:data]; SentryEnvelope *envelope = [[SentryEnvelope alloc] initWithHeader:[SentryEnvelopeHeader empty] diff --git a/Sources/Sentry/include/SentryClient+Logs.h b/Sources/Sentry/include/SentryClient+Logs.h index b64016f8b5..b25c951d5a 100644 --- a/Sources/Sentry/include/SentryClient+Logs.h +++ b/Sources/Sentry/include/SentryClient+Logs.h @@ -7,7 +7,7 @@ NS_ASSUME_NONNULL_BEGIN /** * Helper to capture encoded logs, as SentryEnvelope can't be used in the Swift SDK. */ -- (void)captureLogsData:(NSData *)data; +- (void)captureLogsData:(NSData *)data with:(NSNumber *)itemCount; @end diff --git a/Sources/Swift/Tools/SentryLogBatcher.swift b/Sources/Swift/Tools/SentryLogBatcher.swift index 6ee4086310..c20e12efd5 100644 --- a/Sources/Swift/Tools/SentryLogBatcher.swift +++ b/Sources/Swift/Tools/SentryLogBatcher.swift @@ -7,22 +7,23 @@ import Foundation private let client: SentryClient private let flushTimeout: TimeInterval - private let maxBufferSize: Int + private let maxBufferSizeBytes: Int private let dispatchQueue: SentryDispatchQueueWrapper - private var logBuffer: [SentryLog] = [] - private let logBufferLock = NSLock() - private var currentFlushId: UUID? - + // All mutable state is accessed from the same dispatch queue. + private var encodedLogs: [Data] = [] + private var encodedLogsSize: Int = 0 + private var isTimerActive: Bool = false + @_spi(Private) public init( client: SentryClient, flushTimeout: TimeInterval, - maxBufferSize: Int, + maxBufferSizeBytes: Int, dispatchQueue: SentryDispatchQueueWrapper ) { self.client = client self.flushTimeout = flushTimeout - self.maxBufferSize = maxBufferSize + self.maxBufferSizeBytes = maxBufferSizeBytes self.dispatchQueue = dispatchQueue super.init() } @@ -31,77 +32,92 @@ import Foundation self.init( client: client, flushTimeout: 5, - maxBufferSize: 100, + maxBufferSizeBytes: 1024 * 1024, // 1MB dispatchQueue: dispatchQueue ) } func add(_ log: SentryLog) { - cancelFlush() - - let shouldFlush = logBufferLock.synchronized { - logBuffer.append(log) - return logBuffer.count >= maxBufferSize + dispatchQueue.dispatchAsync { [weak self] in + self?.encodeAndBuffer(log: log) } - - if shouldFlush { - flush() - } else { - scheduleFlush() + } + + // Helper + + // Called on the dispatch queue. + private func encodeAndBuffer(log: SentryLog) { + do { + let encodedLog = try encodeToJSONData(data: log) + + let wasEmpty = encodedLogs.isEmpty + + encodedLogs.append(encodedLog) + encodedLogsSize += encodedLog.count + + let shouldFlush = encodedLogsSize >= maxBufferSizeBytes + let shouldStartTimer = wasEmpty && !isTimerActive && !shouldFlush + + if shouldStartTimer { + isTimerActive = true + } + + // Need to flush due to max buffer size exceeded. + if shouldFlush { + performFlush() + } else if shouldStartTimer { + dispatchQueue.dispatch(after: flushTimeout) { [weak self] in + self?.performFlush() + } + } + } catch { + SentrySDKLog.error("Failed to encode log: \(error)") } } - + @objc public func flush() { - cancelFlush() - - let logs = logBufferLock.synchronized { - let logs = Array(logBuffer) - logBuffer.removeAll() - return logs - } - - if !logs.isEmpty { - dispatch(logs: logs) + dispatchQueue.dispatchAsync { [weak self] in + self?.performFlush() } } - private func scheduleFlush() { - let flushId = UUID() + // Only ever call this from the dispatch queue. + private func performFlush() { + let encodedLogsToSend = Array(encodedLogs) + + // Reset state. + encodedLogs.removeAll() + encodedLogsSize = 0 + isTimerActive = false - logBufferLock.synchronized { - currentFlushId = flushId - } + // If there are no logs to send, return early. - dispatchQueue.dispatch(after: flushTimeout) { [weak self] in - self?.executeFlushIfMatching(flushId: flushId) + guard encodedLogsToSend.count > 0 else { + return } - } - private func executeFlushIfMatching(flushId: UUID) { - let shouldFlush = logBufferLock.synchronized { - return currentFlushId == flushId - } - - if shouldFlush { - flush() - } - } + // Create the payload. - private func cancelFlush() { - logBufferLock.synchronized { - currentFlushId = nil + let opening = "{\"items\":[".data(using: .utf8), + let comma = ",".data(using: .utf8), + let closing = "]}}".data(using: .utf8) else { + return } - } - - private func dispatch(logs: [SentryLog]) { - do { - let payload = ["items": logs] - let data = try encodeToJSONData(data: payload) - - client.captureLogsData(data) - } catch { - SentrySDKLog.error("Failed to create logs envelope.") + + var payloadData = Data() + payloadData.append(opening) + for (index, encodedLog) in encodedLogsToSend.enumerated() { + if index > 0 { + payloadData.append(comma) + } + payloadData.append(encodedLog) } + payloadData.append(closing) + + // Send the payload. + + client.captureLogsData(payloadData, with: NSNumber(value: encodedLogsToSend.count)) } } + From fa94e7a94fe36bc9d1105a5acc2ab7accfce6ab1 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Thu, 17 Jul 2025 14:54:55 +0200 Subject: [PATCH 06/20] use a cancelable work item --- Sources/Swift/Tools/SentryLogBatcher.swift | 54 ++++++++++++---------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/Sources/Swift/Tools/SentryLogBatcher.swift b/Sources/Swift/Tools/SentryLogBatcher.swift index c20e12efd5..4d39c34eb5 100644 --- a/Sources/Swift/Tools/SentryLogBatcher.swift +++ b/Sources/Swift/Tools/SentryLogBatcher.swift @@ -13,7 +13,7 @@ import Foundation // All mutable state is accessed from the same dispatch queue. private var encodedLogs: [Data] = [] private var encodedLogsSize: Int = 0 - private var isTimerActive: Bool = false + private var timerWorkItem: DispatchWorkItem? @_spi(Private) public init( client: SentryClient, @@ -42,10 +42,17 @@ import Foundation self?.encodeAndBuffer(log: log) } } + + @objc + public func flush() { + dispatchQueue.dispatchAsync { [weak self] in + self?.performFlush() + } + } // Helper - // Called on the dispatch queue. + // Only ever call this from the dispatch queue. private func encodeAndBuffer(log: SentryLog) { do { let encodedLog = try encodeToJSONData(data: log) @@ -55,52 +62,52 @@ import Foundation encodedLogs.append(encodedLog) encodedLogsSize += encodedLog.count - let shouldFlush = encodedLogsSize >= maxBufferSizeBytes - let shouldStartTimer = wasEmpty && !isTimerActive && !shouldFlush + let shouldFlushImmediatley = encodedLogsSize >= maxBufferSizeBytes + let shouldStartTimer = wasEmpty && timerWorkItem == nil && !shouldFlushImmediatley if shouldStartTimer { - isTimerActive = true + startTimer() } - - // Need to flush due to max buffer size exceeded. - if shouldFlush { + if shouldFlushImmediatley { performFlush() - } else if shouldStartTimer { - dispatchQueue.dispatch(after: flushTimeout) { [weak self] in - self?.performFlush() - } } } catch { SentrySDKLog.error("Failed to encode log: \(error)") } } - - @objc - public func flush() { - dispatchQueue.dispatchAsync { [weak self] in + + // Only ever call this from the dispatch queue. + private func startTimer() { + let timerWorkItem = DispatchWorkItem { [weak self] in self?.performFlush() } + self.timerWorkItem = timerWorkItem + dispatchQueue.queue.asyncAfter( + deadline: .now() + flushTimeout, + execute: timerWorkItem + ) } // Only ever call this from the dispatch queue. private func performFlush() { let encodedLogsToSend = Array(encodedLogs) - // Reset state. + // Reset buffer state encodedLogs.removeAll() encodedLogsSize = 0 - isTimerActive = false + + // Reset timer state + timerWorkItem?.cancel() + timerWorkItem = nil // If there are no logs to send, return early. - guard encodedLogsToSend.count > 0 else { return } // Create the payload. - - let opening = "{\"items\":[".data(using: .utf8), - let comma = ",".data(using: .utf8), + guard let opening = "{\"items\":[".data(using: .utf8), + let separator = ",".data(using: .utf8), let closing = "]}}".data(using: .utf8) else { return } @@ -109,7 +116,7 @@ import Foundation payloadData.append(opening) for (index, encodedLog) in encodedLogsToSend.enumerated() { if index > 0 { - payloadData.append(comma) + payloadData.append(separator) } payloadData.append(encodedLog) } @@ -120,4 +127,3 @@ import Foundation client.captureLogsData(payloadData, with: NSNumber(value: encodedLogsToSend.count)) } } - From 5316d913c575377daffe67eb698d58f12eab268a Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Thu, 17 Jul 2025 15:28:07 +0200 Subject: [PATCH 07/20] update tests for new logic --- SentryTestUtils/TestClient.swift | 2 +- Sources/Sentry/SentryClient.m | 2 +- Sources/Swift/Tools/SentryLogBatcher.swift | 4 +- Tests/SentryTests/SentryClientTests.swift | 5 +- Tests/SentryTests/SentryLogBatcherTests.swift | 143 ++++++++++-------- 5 files changed, 89 insertions(+), 67 deletions(-) diff --git a/SentryTestUtils/TestClient.swift b/SentryTestUtils/TestClient.swift index ce88984ef7..32e44cfc98 100644 --- a/SentryTestUtils/TestClient.swift +++ b/SentryTestUtils/TestClient.swift @@ -151,7 +151,7 @@ public class TestClient: SentryClient { } public var captureLogsDataInvocations = Invocations() - public override func captureLogsData(_ data: Data) { + public override func captureLogsData(_ data: Data, with count: NSNumber) { captureLogsDataInvocations.record(data) } } diff --git a/Sources/Sentry/SentryClient.m b/Sources/Sentry/SentryClient.m index e2453c2231..dae7520a95 100644 --- a/Sources/Sentry/SentryClient.m +++ b/Sources/Sentry/SentryClient.m @@ -1134,7 +1134,7 @@ - (void)captureLogsData:(NSData *)data with:(NSNumber *)itemCount; [[SentryEnvelopeItemHeader alloc] initWithType:SentryEnvelopeItemTypeLog length:data.length contentType:@"application/vnd.sentry.items.log+json" - itemCount: itemCount]; + itemCount:itemCount]; SentryEnvelopeItem *envelopeItem = [[SentryEnvelopeItem alloc] initWithHeader:header data:data]; SentryEnvelope *envelope = [[SentryEnvelope alloc] initWithHeader:[SentryEnvelopeHeader empty] diff --git a/Sources/Swift/Tools/SentryLogBatcher.swift b/Sources/Swift/Tools/SentryLogBatcher.swift index 4d39c34eb5..53c88b0480 100644 --- a/Sources/Swift/Tools/SentryLogBatcher.swift +++ b/Sources/Swift/Tools/SentryLogBatcher.swift @@ -32,7 +32,7 @@ import Foundation self.init( client: client, flushTimeout: 5, - maxBufferSizeBytes: 1024 * 1024, // 1MB + maxBufferSizeBytes: 1_024 * 1_024, // 1MB dispatchQueue: dispatchQueue ) } @@ -108,7 +108,7 @@ import Foundation // Create the payload. guard let opening = "{\"items\":[".data(using: .utf8), let separator = ",".data(using: .utf8), - let closing = "]}}".data(using: .utf8) else { + let closing = "]}".data(using: .utf8) else { return } diff --git a/Tests/SentryTests/SentryClientTests.swift b/Tests/SentryTests/SentryClientTests.swift index 52b3718816..1bf715cebc 100644 --- a/Tests/SentryTests/SentryClientTests.swift +++ b/Tests/SentryTests/SentryClientTests.swift @@ -2098,7 +2098,7 @@ class SentryClientTest: XCTestCase { let sut = fixture.getSut() let logData = Data("{\"items\":[{\"timestamp\":1627846801,\"level\":\"info\",\"body\":\"Test log message\"}]}".utf8) - sut.captureLogsData(logData) + sut.captureLogsData(logData, with: NSNumber(value: 1)) // Verify that an envelope was sent XCTAssertEqual(1, fixture.transport.sentEnvelopes.count) @@ -2114,6 +2114,7 @@ class SentryClientTest: XCTestCase { XCTAssertEqual("log", item.header.type) XCTAssertEqual(UInt(logData.count), item.header.length) XCTAssertEqual("application/vnd.sentry.items.log+json", item.header.contentType) + XCTAssertEqual(NSNumber(value: 1), item.header.itemCount) // Verify the envelope item data XCTAssertEqual(logData, item.data) @@ -2123,7 +2124,7 @@ class SentryClientTest: XCTestCase { let sut = fixture.getSutDisabledSdk() let logData = Data("{\"items\":[{\"timestamp\":1627846801,\"level\":\"info\",\"body\":\"Test log message\"}]}".utf8) - sut.captureLogsData(logData) + sut.captureLogsData(logData, with: NSNumber(value: 1)) // Verify that no envelope was sent when client is disabled XCTAssertEqual(0, fixture.transport.sentEnvelopes.count) diff --git a/Tests/SentryTests/SentryLogBatcherTests.swift b/Tests/SentryTests/SentryLogBatcherTests.swift index beb4b49f00..2f1a46409d 100644 --- a/Tests/SentryTests/SentryLogBatcherTests.swift +++ b/Tests/SentryTests/SentryLogBatcherTests.swift @@ -18,10 +18,12 @@ final class SentryLogBatcherTests: XCTestCase { testClient = TestClient(options: options) testDispatchQueue = TestSentryDispatchQueueWrapper() + testDispatchQueue.dispatchAsyncExecutesBlock = true // Execute encoding immediately + sut = SentryLogBatcher( client: testClient, - flushTimeout: 5.0, - maxBufferSize: 3, + flushTimeout: 0.1, // Very small timeout for testing + maxBufferSizeBytes: 500, // Small byte limit for testing dispatchQueue: testDispatchQueue ) scope = Scope() @@ -52,6 +54,9 @@ final class SentryLogBatcherTests: XCTestCase { // Trigger flush manually sut.flush() + // Async, wait a bit. + waitBeforeTimeout() + // Verify both logs are batched together XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first) @@ -63,26 +68,25 @@ final class SentryLogBatcherTests: XCTestCase { // MARK: - Buffer Size Tests func testBufferReachesMaxSize_FlushesImmediately() throws { - // Given - let log1 = createTestLog(body: "Log 1") - let log2 = createTestLog(body: "Log 2") - let log3 = createTestLog(body: "Log 3") + // Given - create a log that will exceed the 500 byte limit + let largeLogBody = String(repeating: "A", count: 600) // Larger than 500 byte limit + let largeLog = createTestLog(body: largeLogBody) - // When - add logs up to max buffer size (3) - sut.add(log1) - sut.add(log2) - XCTAssertEqual(testClient.captureLogsDataInvocations.count, 0) + // When - add a log that exceeds buffer size + sut.add(largeLog) - sut.add(log3) // This should trigger immediate flush + // Async, wait a bit. + waitBeforeTimeout() - // Then + // Then - should trigger immediate flush XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) - // Verify all three logs are batched together + // Verify the large log is sent let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first) let jsonObject = try XCTUnwrap(JSONSerialization.jsonObject(with: sentData) as? [String: Any]) let items = try XCTUnwrap(jsonObject["items"] as? [[String: Any]]) - XCTAssertEqual(3, items.count) + XCTAssertEqual(1, items.count) + XCTAssertEqual(largeLogBody, items[0]["body"] as? String) } // MARK: - Timeout Tests @@ -94,41 +98,36 @@ final class SentryLogBatcherTests: XCTestCase { // When sut.add(log) + // Async, wait a bit. + waitBeforeTimeout() + // Then - no immediate flush XCTAssertEqual(testClient.captureLogsDataInvocations.count, 0) - // Called dispatch with correct interval - let invocation = try XCTUnwrap(testDispatchQueue.dispatchAfterInvocations.first) - XCTAssertEqual(invocation.interval, 5.0) - - // Simulate timeout by executing the scheduled block - let scheduledBlock = invocation.block - scheduledBlock() + // Wait enough time for timet to fire + waitAfterTimeout() // Verify flush occurred XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) + let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first) + let jsonObject = try XCTUnwrap(JSONSerialization.jsonObject(with: sentData) as? [String: Any]) + let items = try XCTUnwrap(jsonObject["items"] as? [[String: Any]]) + XCTAssertEqual(1, items.count) } - func testMultipleLogsBeforeTimeout_CancelsAndReschedulesFlush() throws { + func testAddingLogToEmptyBuffer_StartsTimer() throws { // Given let log1 = createTestLog(body: "Log 1") let log2 = createTestLog(body: "Log 2") - // When + // When - add logs sut.add(log1) - XCTAssertEqual(testDispatchQueue.dispatchAfterInvocations.count, 1) - sut.add(log2) - XCTAssertEqual(testDispatchQueue.dispatchAfterInvocations.count, 2) - // Execute the first scheduled flush (should do nothing due to cancellation) - let firstBlock = testDispatchQueue.dispatchAfterInvocations.invocations[0].block - firstBlock() - XCTAssertEqual(testClient.captureLogsDataInvocations.count, 0, "First flush should be cancelled") + // Should not flush immediately + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 0) - // Execute second scheduled flush - let secondBlock = testDispatchQueue.dispatchAfterInvocations.invocations[1].block - secondBlock() + waitAfterTimeout() // Verify both logs are flushed together XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) @@ -136,9 +135,9 @@ final class SentryLogBatcherTests: XCTestCase { let jsonObject = try XCTUnwrap(JSONSerialization.jsonObject(with: sentData) as? [String: Any]) let items = try XCTUnwrap(jsonObject["items"] as? [[String: Any]]) XCTAssertEqual(2, items.count) - } - - // MARK: - Manual Flush Tests + } + + // MARK: - Manual Flush Tests func testManualFlush_FlushesImmediately() throws { // Given @@ -152,7 +151,10 @@ final class SentryLogBatcherTests: XCTestCase { sut.flush() - // Then + // Async, wait a bit. + waitBeforeTimeout() + + // Then - manual flush dispatches async, so it executes immediately with test setup XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first) @@ -167,23 +169,25 @@ final class SentryLogBatcherTests: XCTestCase { // When sut.add(log) - XCTAssertEqual(testDispatchQueue.dispatchAfterInvocations.count, 1) + // Manual flush immediately sut.flush() XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1, "Manual flush should work") - // Execute the scheduled flush (should do nothing since buffer was already flushed) - let scheduledBlock = try XCTUnwrap(testDispatchQueue.dispatchAfterInvocations.invocations.first).block - scheduledBlock() + // Wait for any timer that might have been scheduled to potentially fire + waitAfterTimeout() - // Then - no additional flush should occur (proves cancellation worked) - XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1, "Scheduled flush should be cancelled") + // Then - no additional flush should occur (timer was cancelled by performFlush) + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1, "Timer should be cancelled") } func testFlushEmptyBuffer_DoesNothing() { // When sut.flush() + // Async, wait a bit. + waitBeforeTimeout() + // Then XCTAssertEqual(testClient.captureLogsDataInvocations.count, 0) } @@ -191,22 +195,21 @@ final class SentryLogBatcherTests: XCTestCase { // MARK: - Edge Cases Tests func testScheduledFlushAfterBufferAlreadyFlushed_DoesNothing() throws { - // Given - let log1 = createTestLog(body: "Log 1") - let log2 = createTestLog(body: "Log 2") - let log3 = createTestLog(body: "Log 3") + // Given - create logs that will trigger size-based flush + let largeLogBody = String(repeating: "B", count: 300) + let log1 = createTestLog(body: largeLogBody) + let log2 = createTestLog(body: largeLogBody) // Together > 500 bytes - // When - fill buffer to trigger immediate flush + // When - add first log (starts timer) sut.add(log1) - sut.add(log2) - sut.add(log3) + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 0) + // Add second log that triggers size-based flush + sut.add(log2) XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) - // Simulate a delayed flush callback (should do nothing since buffer is empty) - if let delayedBlock = testDispatchQueue.dispatchAfterInvocations.first?.block { - delayedBlock() - } + // Wait for any timer that might have been scheduled to potentially fire + waitAfterTimeout() // Then - no additional flush should occur XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) @@ -220,10 +223,13 @@ final class SentryLogBatcherTests: XCTestCase { // When sut.add(log1) sut.flush() + waitBeforeTimeout() + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) sut.add(log2) sut.flush() + waitBeforeTimeout() // Then - should have two separate flush calls XCTAssertEqual(testClient.captureLogsDataInvocations.count, 2) @@ -244,7 +250,7 @@ final class SentryLogBatcherTests: XCTestCase { let sutWithRealQueue = SentryLogBatcher( client: testClient, flushTimeout: 5, - maxBufferSize: 100, // Large buffer to avoid immediate flushes + maxBufferSizeBytes: 10_000, // Large buffer to avoid immediate flushes dispatchQueue: SentryDispatchQueueWrapper() ) @@ -259,13 +265,10 @@ final class SentryLogBatcherTests: XCTestCase { expectation.fulfill() } } - - wait(for: [expectation], timeout: 5.0) - - // Then - manually flush and verify all logs were added + wait(for: [expectation], timeout: 1.0) + sutWithRealQueue.flush() - - XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) + waitBeforeTimeout() // Verify all 10 logs were included in the single batch let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first) @@ -289,4 +292,22 @@ final class SentryLogBatcherTests: XCTestCase { attributes: attributes ) } + + private func waitBeforeTimeout() { + // Wait for timer to fire + let expectation = self.expectation(description: "Timer should flush logs") + DispatchQueue.main.asyncAfter(deadline: .now() + 0.02) { + expectation.fulfill() + } + waitForExpectations(timeout: 0.5) + } + + private func waitAfterTimeout() { + // Wait for timer to fire + let expectation = self.expectation(description: "Timer should flush logs") + DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) { + expectation.fulfill() + } + waitForExpectations(timeout: 0.5) + } } From be27e951ad655d08c806e944aeb5af2e6b49e324 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Thu, 17 Jul 2025 15:34:17 +0200 Subject: [PATCH 08/20] fix linter issues, refactor --- Sources/Swift/Tools/SentryLogBatcher.swift | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/Sources/Swift/Tools/SentryLogBatcher.swift b/Sources/Swift/Tools/SentryLogBatcher.swift index 53c88b0480..a5a5c0fa64 100644 --- a/Sources/Swift/Tools/SentryLogBatcher.swift +++ b/Sources/Swift/Tools/SentryLogBatcher.swift @@ -106,24 +106,20 @@ import Foundation } // Create the payload. - guard let opening = "{\"items\":[".data(using: .utf8), - let separator = ",".data(using: .utf8), - let closing = "]}".data(using: .utf8) else { - return - } - + var payloadData = Data() - payloadData.append(opening) + payloadData.append(Data("{\"items\":[".utf8)) + let separator = Data(",".utf8) for (index, encodedLog) in encodedLogsToSend.enumerated() { if index > 0 { payloadData.append(separator) } payloadData.append(encodedLog) } - payloadData.append(closing) + payloadData.append(Data("]}".utf8)) // Send the payload. - + client.captureLogsData(payloadData, with: NSNumber(value: encodedLogsToSend.count)) } } From 40c8ddba4a149b7506ad3afe039000ab308a4348 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Thu, 17 Jul 2025 15:49:22 +0200 Subject: [PATCH 09/20] fix test expectations --- SentryTestUtils/TestClient.swift | 4 ++-- Tests/SentryTests/SentryLogBatcherTests.swift | 20 +++++++++---------- Tests/SentryTests/SentrySDKTests.swift | 17 +++++++++++++--- 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/SentryTestUtils/TestClient.swift b/SentryTestUtils/TestClient.swift index 32e44cfc98..7a1d5f213b 100644 --- a/SentryTestUtils/TestClient.swift +++ b/SentryTestUtils/TestClient.swift @@ -150,8 +150,8 @@ public class TestClient: SentryClient { flushInvocations.record(timeout) } - public var captureLogsDataInvocations = Invocations() + public var captureLogsDataInvocations = Invocations<(data: Data, count: NSNumber)>() public override func captureLogsData(_ data: Data, with count: NSNumber) { - captureLogsDataInvocations.record(data) + captureLogsDataInvocations.record((data, count)) } } diff --git a/Tests/SentryTests/SentryLogBatcherTests.swift b/Tests/SentryTests/SentryLogBatcherTests.swift index 2f1a46409d..67e935ff75 100644 --- a/Tests/SentryTests/SentryLogBatcherTests.swift +++ b/Tests/SentryTests/SentryLogBatcherTests.swift @@ -59,7 +59,7 @@ final class SentryLogBatcherTests: XCTestCase { // Verify both logs are batched together XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) - let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first) + let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first).data let jsonObject = try XCTUnwrap(JSONSerialization.jsonObject(with: sentData) as? [String: Any]) let items = try XCTUnwrap(jsonObject["items"] as? [[String: Any]]) XCTAssertEqual(2, items.count) @@ -82,7 +82,7 @@ final class SentryLogBatcherTests: XCTestCase { XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) // Verify the large log is sent - let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first) + let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first).data let jsonObject = try XCTUnwrap(JSONSerialization.jsonObject(with: sentData) as? [String: Any]) let items = try XCTUnwrap(jsonObject["items"] as? [[String: Any]]) XCTAssertEqual(1, items.count) @@ -109,7 +109,7 @@ final class SentryLogBatcherTests: XCTestCase { // Verify flush occurred XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) - let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first) + let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first).data let jsonObject = try XCTUnwrap(JSONSerialization.jsonObject(with: sentData) as? [String: Any]) let items = try XCTUnwrap(jsonObject["items"] as? [[String: Any]]) XCTAssertEqual(1, items.count) @@ -131,7 +131,7 @@ final class SentryLogBatcherTests: XCTestCase { // Verify both logs are flushed together XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) - let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first) + let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first).data let jsonObject = try XCTUnwrap(JSONSerialization.jsonObject(with: sentData) as? [String: Any]) let items = try XCTUnwrap(jsonObject["items"] as? [[String: Any]]) XCTAssertEqual(2, items.count) @@ -157,7 +157,7 @@ final class SentryLogBatcherTests: XCTestCase { // Then - manual flush dispatches async, so it executes immediately with test setup XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) - let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first) + let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first).data let jsonObject = try XCTUnwrap(JSONSerialization.jsonObject(with: sentData) as? [String: Any]) let items = try XCTUnwrap(jsonObject["items"] as? [[String: Any]]) XCTAssertEqual(2, items.count) @@ -235,8 +235,8 @@ final class SentryLogBatcherTests: XCTestCase { XCTAssertEqual(testClient.captureLogsDataInvocations.count, 2) // Verify each flush contains only one log - for (index, data) in testClient.captureLogsDataInvocations.invocations.enumerated() { - let jsonObject = try XCTUnwrap(JSONSerialization.jsonObject(with: data) as? [String: Any]) + for (index, invocation) in testClient.captureLogsDataInvocations.invocations.enumerated() { + let jsonObject = try XCTUnwrap(JSONSerialization.jsonObject(with: invocation.data) as? [String: Any]) let items = try XCTUnwrap(jsonObject["items"] as? [[String: Any]]) XCTAssertEqual(1, items.count) XCTAssertEqual("Log \(index + 1)", items[0]["body"] as? String) @@ -271,7 +271,7 @@ final class SentryLogBatcherTests: XCTestCase { waitBeforeTimeout() // Verify all 10 logs were included in the single batch - let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first) + let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first).data let jsonObject = try XCTUnwrap(JSONSerialization.jsonObject(with: sentData) as? [String: Any]) let items = try XCTUnwrap(jsonObject["items"] as? [[String: Any]]) XCTAssertEqual(10, items.count, "All 10 concurrently added logs should be in the batch") @@ -295,7 +295,7 @@ final class SentryLogBatcherTests: XCTestCase { private func waitBeforeTimeout() { // Wait for timer to fire - let expectation = self.expectation(description: "Timer should flush logs") + let expectation = self.expectation(description: "Wait for async add") DispatchQueue.main.asyncAfter(deadline: .now() + 0.02) { expectation.fulfill() } @@ -304,7 +304,7 @@ final class SentryLogBatcherTests: XCTestCase { private func waitAfterTimeout() { // Wait for timer to fire - let expectation = self.expectation(description: "Timer should flush logs") + let expectation = self.expectation(description: "Wait for timer flush") DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) { expectation.fulfill() } diff --git a/Tests/SentryTests/SentrySDKTests.swift b/Tests/SentryTests/SentrySDKTests.swift index da1bae4216..e983f622ed 100644 --- a/Tests/SentryTests/SentrySDKTests.swift +++ b/Tests/SentryTests/SentrySDKTests.swift @@ -908,9 +908,13 @@ class SentrySDKTests: XCTestCase { fixture.client.options.experimental.enableLogs = true givenSdkWithHub() - for index in (0..<100) { - SentrySDK.logger.error("foo \(index)") + SentrySDK.logger.error(String(repeating: "S", count: 1_024 * 1_024)) + + let expectation = self.expectation(description: "Wait for async add.") + DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { + expectation.fulfill() } + waitForExpectations(timeout: 0.5) XCTAssertEqual(fixture.client.captureLogsDataInvocations.count, 1) } @@ -920,7 +924,14 @@ class SentrySDKTests: XCTestCase { let hubWithoutClient = SentryHub(client: nil, andScope: nil) SentrySDK.setCurrentHub(hubWithoutClient) - SentrySDK.logger.error("foo") + SentrySDK.logger.error(String(repeating: "S", count: 1_024 * 1_024)) + + let expectation = self.expectation(description: "Wait for async add.") + DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { + expectation.fulfill() + } + waitForExpectations(timeout: 0.5) + XCTAssertEqual(fixture.client.captureLogsDataInvocations.count, 0) } From 99d1416b924d99c1b71a3550e8ec3c2261f4c6a5 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Fri, 18 Jul 2025 10:24:43 +0200 Subject: [PATCH 10/20] remove comment, remove copy of logs with direct access --- Sources/Swift/Tools/SentryLogBatcher.swift | 21 ++++++++++--------- Tests/SentryTests/SentryLogBatcherTests.swift | 2 +- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/Sources/Swift/Tools/SentryLogBatcher.swift b/Sources/Swift/Tools/SentryLogBatcher.swift index a5a5c0fa64..3e59d4ffa6 100644 --- a/Sources/Swift/Tools/SentryLogBatcher.swift +++ b/Sources/Swift/Tools/SentryLogBatcher.swift @@ -90,18 +90,17 @@ import Foundation // Only ever call this from the dispatch queue. private func performFlush() { - let encodedLogsToSend = Array(encodedLogs) - - // Reset buffer state - encodedLogs.removeAll() - encodedLogsSize = 0 - + // Reset logs on function exit + defer { + encodedLogs.removeAll() + encodedLogsSize = 0 + } + // Reset timer state timerWorkItem?.cancel() timerWorkItem = nil - // If there are no logs to send, return early. - guard encodedLogsToSend.count > 0 else { + guard encodedLogs.count > 0 else { return } @@ -110,7 +109,7 @@ import Foundation var payloadData = Data() payloadData.append(Data("{\"items\":[".utf8)) let separator = Data(",".utf8) - for (index, encodedLog) in encodedLogsToSend.enumerated() { + for (index, encodedLog) in encodedLogs.enumerated() { if index > 0 { payloadData.append(separator) } @@ -120,6 +119,8 @@ import Foundation // Send the payload. - client.captureLogsData(payloadData, with: NSNumber(value: encodedLogsToSend.count)) + client.captureLogsData(payloadData, with: NSNumber(value: encodedLogs.count)) + + } } diff --git a/Tests/SentryTests/SentryLogBatcherTests.swift b/Tests/SentryTests/SentryLogBatcherTests.swift index 67e935ff75..b1bdf763d5 100644 --- a/Tests/SentryTests/SentryLogBatcherTests.swift +++ b/Tests/SentryTests/SentryLogBatcherTests.swift @@ -135,7 +135,7 @@ final class SentryLogBatcherTests: XCTestCase { let jsonObject = try XCTUnwrap(JSONSerialization.jsonObject(with: sentData) as? [String: Any]) let items = try XCTUnwrap(jsonObject["items"] as? [[String: Any]]) XCTAssertEqual(2, items.count) - } + } // MARK: - Manual Flush Tests From ead2dbe2544ea86ce28f296130f6f7a6f290f92b Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Fri, 18 Jul 2025 10:29:00 +0200 Subject: [PATCH 11/20] add additional docs --- Sources/Swift/Tools/SentryLogBatcher.swift | 27 +++++++++++++++------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/Sources/Swift/Tools/SentryLogBatcher.swift b/Sources/Swift/Tools/SentryLogBatcher.swift index 3e59d4ffa6..478dbe5c2c 100644 --- a/Sources/Swift/Tools/SentryLogBatcher.swift +++ b/Sources/Swift/Tools/SentryLogBatcher.swift @@ -10,11 +10,19 @@ import Foundation private let maxBufferSizeBytes: Int private let dispatchQueue: SentryDispatchQueueWrapper - // All mutable state is accessed from the same dispatch queue. + // All mutable state is accessed from the same serial dispatch queue. + + // Every logs data is added sepratley. They are flushed together in an envelope. private var encodedLogs: [Data] = [] private var encodedLogsSize: Int = 0 private var timerWorkItem: DispatchWorkItem? + /// Initializes a new SentryLogBatcher. + /// - Parameters: + /// - client: The SentryClient to use for sending logs + /// - flushTimeout: The timeout interval after which buffered logs will be flushed + /// - maxBufferSizeBytes: The maximum buffer size in bytes before triggering an immediate flush + /// - dispatchQueue: A serial dispatch queue wrapper for thread-safe access to mutable state @_spi(Private) public init( client: SentryClient, flushTimeout: TimeInterval, @@ -28,6 +36,11 @@ import Foundation super.init() } + + /// Convenience initializer with default flush timeout and buffer size. + /// - Parameters: + /// - client: The SentryClient to use for sending logs + /// - dispatchQueue: A serial dispatch queue wrapper for thread-safe access to mutable state @_spi(Private) public convenience init(client: SentryClient, dispatchQueue: SentryDispatchQueueWrapper) { self.init( client: client, @@ -37,14 +50,14 @@ import Foundation ) } - func add(_ log: SentryLog) { + @_spi(Private) func add(_ log: SentryLog) { dispatchQueue.dispatchAsync { [weak self] in self?.encodeAndBuffer(log: log) } } @objc - public func flush() { + @_spi(Private) func flush() { dispatchQueue.dispatchAsync { [weak self] in self?.performFlush() } @@ -52,7 +65,7 @@ import Foundation // Helper - // Only ever call this from the dispatch queue. + // Only ever call this from the serial dispatch queue. private func encodeAndBuffer(log: SentryLog) { do { let encodedLog = try encodeToJSONData(data: log) @@ -76,7 +89,7 @@ import Foundation } } - // Only ever call this from the dispatch queue. + // Only ever call this from the serial dispatch queue. private func startTimer() { let timerWorkItem = DispatchWorkItem { [weak self] in self?.performFlush() @@ -88,7 +101,7 @@ import Foundation ) } - // Only ever call this from the dispatch queue. + // Only ever call this from the serial dispatch queue. private func performFlush() { // Reset logs on function exit defer { @@ -120,7 +133,5 @@ import Foundation // Send the payload. client.captureLogsData(payloadData, with: NSNumber(value: encodedLogs.count)) - - } } From 1554b479658af098a8208051b8033411b41281fe Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Fri, 18 Jul 2025 10:39:06 +0200 Subject: [PATCH 12/20] simplify fush/dispatch condition --- Sources/Swift/Tools/SentryLogBatcher.swift | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/Sources/Swift/Tools/SentryLogBatcher.swift b/Sources/Swift/Tools/SentryLogBatcher.swift index 478dbe5c2c..d3128cf6df 100644 --- a/Sources/Swift/Tools/SentryLogBatcher.swift +++ b/Sources/Swift/Tools/SentryLogBatcher.swift @@ -70,19 +70,15 @@ import Foundation do { let encodedLog = try encodeToJSONData(data: log) - let wasEmpty = encodedLogs.isEmpty + let encodedLogsWereEmpty = encodedLogs.isEmpty encodedLogs.append(encodedLog) encodedLogsSize += encodedLog.count - let shouldFlushImmediatley = encodedLogsSize >= maxBufferSizeBytes - let shouldStartTimer = wasEmpty && timerWorkItem == nil && !shouldFlushImmediatley - - if shouldStartTimer { - startTimer() - } - if shouldFlushImmediatley { + if encodedLogsSize >= maxBufferSizeBytes { performFlush() + } else if encodedLogsWereEmpty && timerWorkItem == nil { + startTimer() } } catch { SentrySDKLog.error("Failed to encode log: \(error)") From 33ae73c8960cd319bf34f444841ec01dbfe54dc8 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Fri, 18 Jul 2025 11:10:37 +0200 Subject: [PATCH 13/20] add dispatch with workitem to wrapper, update tests --- .../TestSentryDispatchQueueWrapper.swift | 14 +++ .../Helper/SentryDispatchQueueWrapper.swift | 6 ++ Sources/Swift/Tools/SentryLogBatcher.swift | 6 +- Tests/SentryTests/SentryLogBatcherTests.swift | 85 ++++++++----------- 4 files changed, 58 insertions(+), 53 deletions(-) diff --git a/SentryTestUtils/TestSentryDispatchQueueWrapper.swift b/SentryTestUtils/TestSentryDispatchQueueWrapper.swift index 5f1788447c..6417d11566 100644 --- a/SentryTestUtils/TestSentryDispatchQueueWrapper.swift +++ b/SentryTestUtils/TestSentryDispatchQueueWrapper.swift @@ -62,6 +62,20 @@ import Foundation dispatchAfterInvocations.invocations.last?.block() } + public var dispatchAfterWorkItemInvocations = Invocations<(interval: TimeInterval, workItem: DispatchWorkItem)>() + public override func dispatch(after interval: TimeInterval, workItem: DispatchWorkItem) { + dispatchAfterWorkItemInvocations.record((interval, workItem)) + if blockBeforeMainBlock() { + if dispatchAfterExecutesBlock { + workItem.perform() + } + } + } + + public func invokeLastDispatchAfterWorkItem() { + dispatchAfterWorkItemInvocations.invocations.last?.workItem.perform() + } + public var dispatchCancelInvocations = 0 public override var shouldDispatchCancel: Bool { dispatchCancelInvocations += 1 diff --git a/Sources/Swift/Helper/SentryDispatchQueueWrapper.swift b/Sources/Swift/Helper/SentryDispatchQueueWrapper.swift index 2691df0da8..eee18c1ba6 100644 --- a/Sources/Swift/Helper/SentryDispatchQueueWrapper.swift +++ b/Sources/Swift/Helper/SentryDispatchQueueWrapper.swift @@ -46,6 +46,12 @@ internalWrapper.dispatch(after: interval, block: block) } + public func dispatch(after interval: TimeInterval, workItem: DispatchWorkItem) { + internalWrapper.dispatch(after: interval) { + workItem.perform() + } + } + public func dispatchOnce(_ predicate: UnsafeMutablePointer, block: @escaping () -> Void) { internalWrapper.dispatchOnce(predicate, block: block) } diff --git a/Sources/Swift/Tools/SentryLogBatcher.swift b/Sources/Swift/Tools/SentryLogBatcher.swift index d3128cf6df..2dd5905d49 100644 --- a/Sources/Swift/Tools/SentryLogBatcher.swift +++ b/Sources/Swift/Tools/SentryLogBatcher.swift @@ -36,7 +36,6 @@ import Foundation super.init() } - /// Convenience initializer with default flush timeout and buffer size. /// - Parameters: /// - client: The SentryClient to use for sending logs @@ -91,10 +90,7 @@ import Foundation self?.performFlush() } self.timerWorkItem = timerWorkItem - dispatchQueue.queue.asyncAfter( - deadline: .now() + flushTimeout, - execute: timerWorkItem - ) + dispatchQueue.dispatch(after: flushTimeout, workItem: timerWorkItem) } // Only ever call this from the serial dispatch queue. diff --git a/Tests/SentryTests/SentryLogBatcherTests.swift b/Tests/SentryTests/SentryLogBatcherTests.swift index b1bdf763d5..c492fd693f 100644 --- a/Tests/SentryTests/SentryLogBatcherTests.swift +++ b/Tests/SentryTests/SentryLogBatcherTests.swift @@ -54,9 +54,6 @@ final class SentryLogBatcherTests: XCTestCase { // Trigger flush manually sut.flush() - // Async, wait a bit. - waitBeforeTimeout() - // Verify both logs are batched together XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first).data @@ -75,9 +72,6 @@ final class SentryLogBatcherTests: XCTestCase { // When - add a log that exceeds buffer size sut.add(largeLog) - // Async, wait a bit. - waitBeforeTimeout() - // Then - should trigger immediate flush XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) @@ -98,14 +92,13 @@ final class SentryLogBatcherTests: XCTestCase { // When sut.add(log) - // Async, wait a bit. - waitBeforeTimeout() - - // Then - no immediate flush + // Then - no immediate flush but timer should be started XCTAssertEqual(testClient.captureLogsDataInvocations.count, 0) + XCTAssertEqual(testDispatchQueue.dispatchAfterWorkItemInvocations.count, 1) + XCTAssertEqual(testDispatchQueue.dispatchAfterWorkItemInvocations.first?.interval, 0.1) - // Wait enough time for timet to fire - waitAfterTimeout() + // Manually trigger the timer to simulate timeout + testDispatchQueue.invokeLastDispatchAfterWorkItem() // Verify flush occurred XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) @@ -120,14 +113,24 @@ final class SentryLogBatcherTests: XCTestCase { let log1 = createTestLog(body: "Log 1") let log2 = createTestLog(body: "Log 2") - // When - add logs + // When - add first log to empty buffer sut.add(log1) + + // Then - timer should be started for first log + XCTAssertEqual(testDispatchQueue.dispatchAfterWorkItemInvocations.count, 1) + XCTAssertEqual(testDispatchQueue.dispatchAfterWorkItemInvocations.first?.interval, 0.1) + + // When - add second log to non-empty buffer sut.add(log2) + // Then - no additional timer should be started + XCTAssertEqual(testDispatchQueue.dispatchAfterWorkItemInvocations.count, 1) + // Should not flush immediately XCTAssertEqual(testClient.captureLogsDataInvocations.count, 0) - waitAfterTimeout() + // Manually trigger the timer + testDispatchQueue.invokeLastDispatchAfterWorkItem() // Verify both logs are flushed together XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) @@ -151,10 +154,7 @@ final class SentryLogBatcherTests: XCTestCase { sut.flush() - // Async, wait a bit. - waitBeforeTimeout() - - // Then - manual flush dispatches async, so it executes immediately with test setup + // Then XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first).data @@ -170,12 +170,16 @@ final class SentryLogBatcherTests: XCTestCase { // When sut.add(log) + // Then - timer should be started + XCTAssertEqual(testDispatchQueue.dispatchAfterWorkItemInvocations.count, 1) + let timerWorkItem = try XCTUnwrap(testDispatchQueue.dispatchAfterWorkItemInvocations.first?.workItem) + // Manual flush immediately sut.flush() XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1, "Manual flush should work") - // Wait for any timer that might have been scheduled to potentially fire - waitAfterTimeout() + // Try to trigger the timer work item (should not flush again since timer was cancelled) + timerWorkItem.perform() // Then - no additional flush should occur (timer was cancelled by performFlush) XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1, "Timer should be cancelled") @@ -185,13 +189,10 @@ final class SentryLogBatcherTests: XCTestCase { // When sut.flush() - // Async, wait a bit. - waitBeforeTimeout() - // Then XCTAssertEqual(testClient.captureLogsDataInvocations.count, 0) } - + // MARK: - Edge Cases Tests func testScheduledFlushAfterBufferAlreadyFlushed_DoesNothing() throws { @@ -203,13 +204,15 @@ final class SentryLogBatcherTests: XCTestCase { // When - add first log (starts timer) sut.add(log1) XCTAssertEqual(testClient.captureLogsDataInvocations.count, 0) + XCTAssertEqual(testDispatchQueue.dispatchAfterWorkItemInvocations.count, 1) + let timerWorkItem = try XCTUnwrap(testDispatchQueue.dispatchAfterWorkItemInvocations.first?.workItem) // Add second log that triggers size-based flush sut.add(log2) XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) - // Wait for any timer that might have been scheduled to potentially fire - waitAfterTimeout() + // Try to trigger the original timer work item (should not flush again) + timerWorkItem.perform() // Then - no additional flush should occur XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) @@ -223,13 +226,11 @@ final class SentryLogBatcherTests: XCTestCase { // When sut.add(log1) sut.flush() - waitBeforeTimeout() XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1) sut.add(log2) sut.flush() - waitBeforeTimeout() // Then - should have two separate flush calls XCTAssertEqual(testClient.captureLogsDataInvocations.count, 2) @@ -251,7 +252,7 @@ final class SentryLogBatcherTests: XCTestCase { client: testClient, flushTimeout: 5, maxBufferSizeBytes: 10_000, // Large buffer to avoid immediate flushes - dispatchQueue: SentryDispatchQueueWrapper() + dispatchQueue: SentryDispatchQueueWrapper() // Real dispatch queue ) let expectation = XCTestExpectation(description: "Concurrent adds") @@ -268,7 +269,13 @@ final class SentryLogBatcherTests: XCTestCase { wait(for: [expectation], timeout: 1.0) sutWithRealQueue.flush() - waitBeforeTimeout() + + // Need to wait a bit for flush to complete since this uses a real queue + let flushExpectation = self.expectation(description: "Wait for flush") + DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { + flushExpectation.fulfill() + } + waitForExpectations(timeout: 0.5) // Verify all 10 logs were included in the single batch let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first).data @@ -292,22 +299,4 @@ final class SentryLogBatcherTests: XCTestCase { attributes: attributes ) } - - private func waitBeforeTimeout() { - // Wait for timer to fire - let expectation = self.expectation(description: "Wait for async add") - DispatchQueue.main.asyncAfter(deadline: .now() + 0.02) { - expectation.fulfill() - } - waitForExpectations(timeout: 0.5) - } - - private func waitAfterTimeout() { - // Wait for timer to fire - let expectation = self.expectation(description: "Wait for timer flush") - DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) { - expectation.fulfill() - } - waitForExpectations(timeout: 0.5) - } } From 7c0a081a6365daf36d56823c5ccd018bb410f7b1 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Fri, 18 Jul 2025 11:13:07 +0200 Subject: [PATCH 14/20] more explicit warning for injected queue --- Sources/Swift/Tools/SentryLogBatcher.swift | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Sources/Swift/Tools/SentryLogBatcher.swift b/Sources/Swift/Tools/SentryLogBatcher.swift index 2dd5905d49..6c99acaf0a 100644 --- a/Sources/Swift/Tools/SentryLogBatcher.swift +++ b/Sources/Swift/Tools/SentryLogBatcher.swift @@ -22,7 +22,10 @@ import Foundation /// - client: The SentryClient to use for sending logs /// - flushTimeout: The timeout interval after which buffered logs will be flushed /// - maxBufferSizeBytes: The maximum buffer size in bytes before triggering an immediate flush - /// - dispatchQueue: A serial dispatch queue wrapper for thread-safe access to mutable state + /// - dispatchQueue: A **serial** dispatch queue wrapper for thread-safe access to mutable state + /// + /// - Important: The `dispatchQueue` parameter MUST be a serial queue to ensure thread safety. + /// Passing a concurrent queue will result in undefined behavior and potential data races. @_spi(Private) public init( client: SentryClient, flushTimeout: TimeInterval, @@ -39,7 +42,10 @@ import Foundation /// Convenience initializer with default flush timeout and buffer size. /// - Parameters: /// - client: The SentryClient to use for sending logs - /// - dispatchQueue: A serial dispatch queue wrapper for thread-safe access to mutable state + /// - dispatchQueue: A **serial** dispatch queue wrapper for thread-safe access to mutable state + /// + /// - Important: The `dispatchQueue` parameter MUST be a serial queue to ensure thread safety. + /// Passing a concurrent queue will result in undefined behavior and potential data races. @_spi(Private) public convenience init(client: SentryClient, dispatchQueue: SentryDispatchQueueWrapper) { self.init( client: client, From f7539bb0af36da16e71434a6896158359211d98a Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Fri, 18 Jul 2025 11:19:44 +0200 Subject: [PATCH 15/20] add debug messages --- Sources/Swift/Tools/SentryLogBatcher.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Sources/Swift/Tools/SentryLogBatcher.swift b/Sources/Swift/Tools/SentryLogBatcher.swift index 6c99acaf0a..2701105f1f 100644 --- a/Sources/Swift/Tools/SentryLogBatcher.swift +++ b/Sources/Swift/Tools/SentryLogBatcher.swift @@ -93,6 +93,7 @@ import Foundation // Only ever call this from the serial dispatch queue. private func startTimer() { let timerWorkItem = DispatchWorkItem { [weak self] in + SentrySDKLog.debug("SentryLogBatcher: Timer fired, calling performFlush().") self?.performFlush() } self.timerWorkItem = timerWorkItem @@ -112,6 +113,7 @@ import Foundation timerWorkItem = nil guard encodedLogs.count > 0 else { + SentrySDKLog.debug("SentryLogBatcher: No logs to flush.") return } From fad5c54549ba67dbaaf4a2310697875748b9c877 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Fri, 18 Jul 2025 14:00:43 +0200 Subject: [PATCH 16/20] Use internal wrapper queue --- .../Helper/SentryDispatchQueueWrapper.swift | 4 +-- Tests/SentryTests/SentryLogBatcherTests.swift | 36 ++++++++++++++++++- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/Sources/Swift/Helper/SentryDispatchQueueWrapper.swift b/Sources/Swift/Helper/SentryDispatchQueueWrapper.swift index eee18c1ba6..5926572125 100644 --- a/Sources/Swift/Helper/SentryDispatchQueueWrapper.swift +++ b/Sources/Swift/Helper/SentryDispatchQueueWrapper.swift @@ -47,9 +47,7 @@ } public func dispatch(after interval: TimeInterval, workItem: DispatchWorkItem) { - internalWrapper.dispatch(after: interval) { - workItem.perform() - } + internalWrapper.queue.asyncAfter(deadline: .now() + interval, execute: workItem) } public func dispatchOnce(_ predicate: UnsafeMutablePointer, block: @escaping () -> Void) { diff --git a/Tests/SentryTests/SentryLogBatcherTests.swift b/Tests/SentryTests/SentryLogBatcherTests.swift index c492fd693f..cb22dac05c 100644 --- a/Tests/SentryTests/SentryLogBatcherTests.swift +++ b/Tests/SentryTests/SentryLogBatcherTests.swift @@ -244,7 +244,7 @@ final class SentryLogBatcherTests: XCTestCase { } } - // MARK: - Thread Safety Tests + // MARK: - IntegrationTests func testConcurrentAdds_ThreadSafe() throws { // Given @@ -284,6 +284,40 @@ final class SentryLogBatcherTests: XCTestCase { XCTAssertEqual(10, items.count, "All 10 concurrently added logs should be in the batch") // Note: We can't verify exact order due to concurrency, but count should be correct } + + func testDispatchAfterTimeoutWithRealDispatchQueue() throws { + // Given - create batcher with real dispatch queue and short timeout + let sutWithRealQueue = SentryLogBatcher( + client: testClient, + flushTimeout: 0.2, // Short but realistic timeout + maxBufferSizeBytes: 10_000, // Large buffer to avoid size-based flush + dispatchQueue: SentryDispatchQueueWrapper() // Real dispatch queue + ) + + let log = createTestLog(body: "Real timeout test log") + let expectation = XCTestExpectation(description: "Real timeout flush") + + // When - add log and wait for real timeout + sutWithRealQueue.add(log) + + // Initially no flush should have occurred + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 0) + + // Wait for timeout to trigger flush + DispatchQueue.main.asyncAfter(deadline: .now() + 0.3) { // Wait longer than timeout + expectation.fulfill() + } + wait(for: [expectation], timeout: 1.0) + + // Then - verify flush occurred due to timeout + XCTAssertEqual(testClient.captureLogsDataInvocations.count, 1, "Timeout should trigger flush") + + let sentData = try XCTUnwrap(testClient.captureLogsDataInvocations.first).data + let jsonObject = try XCTUnwrap(JSONSerialization.jsonObject(with: sentData) as? [String: Any]) + let items = try XCTUnwrap(jsonObject["items"] as? [[String: Any]]) + XCTAssertEqual(1, items.count, "Should contain exactly one log") + XCTAssertEqual("Real timeout test log", items[0]["body"] as? String) + } // MARK: - Helper Methods From 6e0f43a8e3449c82ab92eb781658bf16bad12167 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Mon, 21 Jul 2025 10:30:15 +0200 Subject: [PATCH 17/20] add comment and make internal --- Sources/Swift/Helper/SentryDispatchQueueWrapper.swift | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Sources/Swift/Helper/SentryDispatchQueueWrapper.swift b/Sources/Swift/Helper/SentryDispatchQueueWrapper.swift index 5926572125..bafd1523bc 100644 --- a/Sources/Swift/Helper/SentryDispatchQueueWrapper.swift +++ b/Sources/Swift/Helper/SentryDispatchQueueWrapper.swift @@ -45,15 +45,16 @@ public func dispatch(after interval: TimeInterval, block: @escaping () -> Void) { internalWrapper.dispatch(after: interval, block: block) } - - public func dispatch(after interval: TimeInterval, workItem: DispatchWorkItem) { - internalWrapper.queue.asyncAfter(deadline: .now() + interval, execute: workItem) - } - + public func dispatchOnce(_ predicate: UnsafeMutablePointer, block: @escaping () -> Void) { internalWrapper.dispatchOnce(predicate, block: block) } + @_spi(Private) public func dispatch(after interval: TimeInterval, workItem: DispatchWorkItem) { + // Swift only API, so we need to call the internal queue directly. + internalWrapper.queue.asyncAfter(deadline: .now() + interval, execute: workItem) + } + // The ObjC version of this code wrapped `dispatch_cancel` and `dispatch_block_create` // However dispatch_block is not accessible in Swift. Unit tests rely on stubbing out // the creation and cancelation of dispatch blocks, so these two variables allow From a253552e66b84e831718d61bded913a58d158ecd Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Mon, 21 Jul 2025 14:22:08 +0200 Subject: [PATCH 18/20] increase diffmax --- CHANGELOG.md | 2 +- Tests/Perf/metrics-test.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e538e4655..fa9c9ae92f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Features -- Add experimental support for capturing structured logs via `SentrySDK.logger` (#5532, #5593) +- Add experimental support for capturing structured logs via `SentrySDK.logger` (#5532, #5593, #5628) ### Improvements diff --git a/Tests/Perf/metrics-test.yml b/Tests/Perf/metrics-test.yml index e7c3cdb2b2..e98b1ed532 100644 --- a/Tests/Perf/metrics-test.yml +++ b/Tests/Perf/metrics-test.yml @@ -11,4 +11,4 @@ startupTimeTest: binarySizeTest: diffMin: 200 KiB - diffMax: 875 KiB + diffMax: 880 KiB From e4614939d40d9e023b0d5494cf3804628cc2d395 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Mon, 21 Jul 2025 15:11:13 +0200 Subject: [PATCH 19/20] increase timeout in test --- Samples/iOS-SwiftUI/iOS-SwiftUI-UITests/LaunchUITests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Samples/iOS-SwiftUI/iOS-SwiftUI-UITests/LaunchUITests.swift b/Samples/iOS-SwiftUI/iOS-SwiftUI-UITests/LaunchUITests.swift index 3aecae1e52..af23971f58 100644 --- a/Samples/iOS-SwiftUI/iOS-SwiftUI-UITests/LaunchUITests.swift +++ b/Samples/iOS-SwiftUI/iOS-SwiftUI-UITests/LaunchUITests.swift @@ -15,7 +15,7 @@ class LaunchUITests: XCTestCase { let transactionName = app.staticTexts["TRANSACTION_NAME"] let transactionId = app.staticTexts["TRANSACTION_ID"] - if !transactionName.waitForExistence(timeout: 1) { + if !transactionName.waitForExistence(timeout: 5) { XCTFail("Span operation label not found") } From 8290c16dbd3fda64534b902c6741f5c5970fa3fe Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Mon, 21 Jul 2025 15:15:05 +0200 Subject: [PATCH 20/20] fix cl --- CHANGELOG.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c37b18aef..c5459eed3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,7 @@ ### Features -- Add experimental support for capturing structured logs via `SentrySDK.logger` (#5532, #5593, #5628) -- Add experimental support for capturing structured logs via `SentrySDK.logger` (#5532, #5593, #5639) -- Add experimental support for capturing structured logs via `SentrySDK.logger` (#5532, #5593) +- Add experimental support for capturing structured logs via `SentrySDK.logger` (#5532, #5593, #5639, #5628) ### Improvements