DataDog / dd-sdk-ios

Datadog SDK for iOS - Swift and Objective-C.
Apache License 2.0
219 stars 127 forks source link

RUM-1053 Make Logger Sendable for Swift 6 concurrency #2035

Closed ncreated closed 2 months ago

ncreated commented 2 months ago

What and why?

📦 This PR prepares DatadogLogs for compatibility with apps that enable "complete" concurrency checking in Swift 6.

I identified a potential data-race safety issue when passing instances of LoggerProtocol across isolation boundaries, such as to different threads:

import DatadogLogs

let logger = Logger.create(with: .init())

DispatchQueue.global(qos: .userInitiated).async {
    logger.debug("foo") 
    // ^ ❌ Error: Capture of 'logger' with non-sendable type 'any LoggerProtocol' in a `@Sendable` closure
}

How?

This issue is resolved by declaring Sendable conformance for LoggerProtocol:

A conformance to Sendable means the given type is thread safe, and values of the type can be shared across arbitrary isolation domains without introducing a risk of data races.

Since Xcode 15 uses SWIFT_STRICT_CONCURRENCY: minimal, which checks only explicit Sendable conformances, this:

- public protocol LoggerProtocol {
+ public protocol LoggerProtocol: Sendable {

resulted in concurrency warnings elsewhere.

Instead of using @unchecked Sendable for RemoteLogger, ConsoleLogger, and some test types, I chose to address concurrency warnings properly, making RemoteLogger and ConsoleLogger fully checked Sendable types 👌.

This required several major changes:

Smaller adjustments were made as a consequence of spreading Sendable conformance. For example, marking BacktraceReporter as Sendable required resolving issues in Crash Reporting tests, as this type is shared through DatadogInternal.

Review checklist

datadog-datadog-prod-us1[bot] commented 2 months ago

Datadog Report

Branch report: ncreated/RUM-1053/make-logs-api-concurrency-safe Commit report: a3d8423 Test service: dd-sdk-ios

:white_check_mark: 0 Failed, 1880 Passed, 0 Skipped, 1m 16.57s Total Time :small_red_triangle_down: Test Sessions change in coverage: 2 decreased, 5 increased, 1 no change

:small_red_triangle_down: Code Coverage Decreases vs Default Branch (2)