Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from
Sep 10, 2024
24 changes: 24 additions & 0 deletions Datadog/Datadog.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,14 @@
615CC40C2694A56D0005F08C /* SwiftExtensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 615CC40B2694A56D0005F08C /* SwiftExtensions.swift */; };
615CC4102694A64D0005F08C /* SwiftExtensionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 615CC40F2694A64D0005F08C /* SwiftExtensionTests.swift */; };
615CC4132695957C0005F08C /* CrashReportTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 615CC4122695957C0005F08C /* CrashReportTests.swift */; };
615D52B82C888C1F00F8B8FC /* SynchronizedAttributes.swift in Sources */ = {isa = PBXBuildFile; fileRef = 615D52B72C888C1F00F8B8FC /* SynchronizedAttributes.swift */; };
615D52B92C888C1F00F8B8FC /* SynchronizedAttributes.swift in Sources */ = {isa = PBXBuildFile; fileRef = 615D52B72C888C1F00F8B8FC /* SynchronizedAttributes.swift */; };
615D52BB2C88A83A00F8B8FC /* SynchronizedTags.swift in Sources */ = {isa = PBXBuildFile; fileRef = 615D52BA2C88A83A00F8B8FC /* SynchronizedTags.swift */; };
615D52BC2C88A83A00F8B8FC /* SynchronizedTags.swift in Sources */ = {isa = PBXBuildFile; fileRef = 615D52BA2C88A83A00F8B8FC /* SynchronizedTags.swift */; };
615D52BE2C88A98300F8B8FC /* SynchronizedTagsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 615D52BD2C88A98300F8B8FC /* SynchronizedTagsTests.swift */; };
615D52BF2C88A98300F8B8FC /* SynchronizedTagsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 615D52BD2C88A98300F8B8FC /* SynchronizedTagsTests.swift */; };
615D52C12C88AB1E00F8B8FC /* SynchronizedAttributesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 615D52C02C88AB1E00F8B8FC /* SynchronizedAttributesTests.swift */; };
615D52C22C88AB1E00F8B8FC /* SynchronizedAttributesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 615D52C02C88AB1E00F8B8FC /* SynchronizedAttributesTests.swift */; };
6167C79326665D6900D4CF07 /* E2EUtils.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6167C79226665D6900D4CF07 /* E2EUtils.swift */; };
6167C7952666622800D4CF07 /* LoggingE2EHelpers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6167C7942666622800D4CF07 /* LoggingE2EHelpers.swift */; };
6167E6D32B7F8B3300C3CA2D /* AppHangsMonitor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6167E6D22B7F8B3300C3CA2D /* AppHangsMonitor.swift */; };
Expand Down Expand Up @@ -2449,6 +2457,10 @@
615CC40B2694A56D0005F08C /* SwiftExtensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SwiftExtensions.swift; sourceTree = "<group>"; };
615CC40F2694A64D0005F08C /* SwiftExtensionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SwiftExtensionTests.swift; sourceTree = "<group>"; };
615CC4122695957C0005F08C /* CrashReportTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CrashReportTests.swift; sourceTree = "<group>"; };
615D52B72C888C1F00F8B8FC /* SynchronizedAttributes.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SynchronizedAttributes.swift; sourceTree = "<group>"; };
615D52BA2C88A83A00F8B8FC /* SynchronizedTags.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SynchronizedTags.swift; sourceTree = "<group>"; };
615D52BD2C88A98300F8B8FC /* SynchronizedTagsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SynchronizedTagsTests.swift; sourceTree = "<group>"; };
615D52C02C88AB1E00F8B8FC /* SynchronizedAttributesTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SynchronizedAttributesTests.swift; sourceTree = "<group>"; };
615D9E2626048EAF006DC6D1 /* DatadogCrashReporting.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = DatadogCrashReporting.xcconfig; sourceTree = "<group>"; };
615F197B25B5A64B00BE14B5 /* UIKitExtensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UIKitExtensions.swift; sourceTree = "<group>"; };
6161247825CA9CA6009901BE /* CrashReporting.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CrashReporting.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -4225,6 +4237,8 @@
61133BC22423979B00786299 /* LogEventEncoder.swift */,
61133BC32423979B00786299 /* LogEventBuilder.swift */,
61133BC42423979B00786299 /* LogEventSanitizer.swift */,
615D52B72C888C1F00F8B8FC /* SynchronizedAttributes.swift */,
615D52BA2C88A83A00F8B8FC /* SynchronizedTags.swift */,
);
path = Log;
sourceTree = "<group>";
Expand Down Expand Up @@ -4423,6 +4437,8 @@
children = (
61133C3B2423990D00786299 /* LogEventBuilderTests.swift */,
61133C3C2423990D00786299 /* LogSanitizerTests.swift */,
615D52BD2C88A98300F8B8FC /* SynchronizedTagsTests.swift */,
615D52C02C88AB1E00F8B8FC /* SynchronizedAttributesTests.swift */,
);
path = Log;
sourceTree = "<group>";
Expand Down Expand Up @@ -8584,7 +8600,9 @@
D207319529A522F600ECBF94 /* LogsFeature.swift in Sources */,
D242C29E2A14D6A6004B4980 /* RemoteLogger.swift in Sources */,
D20731B529A528DA00ECBF94 /* LogEventBuilder.swift in Sources */,
615D52BB2C88A83A00F8B8FC /* SynchronizedTags.swift in Sources */,
D243BBF529A620CC000B9CEC /* MessageReceivers.swift in Sources */,
615D52B82C888C1F00F8B8FC /* SynchronizedAttributes.swift in Sources */,
D22C5BC92A98A0B30024CC1F /* Baggages.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
Expand All @@ -8595,13 +8613,15 @@
files = (
D2A783E829A53468003B03BB /* ConsoleLoggerTests.swift in Sources */,
D2A783EB29A53468003B03BB /* LogSanitizerTests.swift in Sources */,
615D52BE2C88A98300F8B8FC /* SynchronizedTagsTests.swift in Sources */,
D2D30E602A40CD310020C553 /* LogsTests.swift in Sources */,
D2A783E729A53468003B03BB /* LogEventBuilderTests.swift in Sources */,
D242C2A12A14D747004B4980 /* RemoteLoggerTests.swift in Sources */,
D20FD9CF2AC6FF42004D3569 /* WebViewLogReceiverTests.swift in Sources */,
D2A783ED29A534F2003B03BB /* LoggingFeatureMocks.swift in Sources */,
D2B249972A45E10500DD4F9F /* LoggerTests.swift in Sources */,
D2A783EA29A53468003B03BB /* LogMessageReceiverTests.swift in Sources */,
615D52C12C88AB1E00F8B8FC /* SynchronizedAttributesTests.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand All @@ -8622,7 +8642,9 @@
D20731AB29A5279D00ECBF94 /* LogsFeature.swift in Sources */,
D242C29F2A14D6A7004B4980 /* RemoteLogger.swift in Sources */,
D20731B629A528DA00ECBF94 /* LogEventBuilder.swift in Sources */,
615D52BC2C88A83A00F8B8FC /* SynchronizedTags.swift in Sources */,
D243BBF629A620CC000B9CEC /* MessageReceivers.swift in Sources */,
615D52B92C888C1F00F8B8FC /* SynchronizedAttributes.swift in Sources */,
D22C5BC82A98A0B20024CC1F /* Baggages.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
Expand Down Expand Up @@ -9226,13 +9248,15 @@
files = (
D2A783F329A534F9003B03BB /* ConsoleLoggerTests.swift in Sources */,
D2A783F429A534F9003B03BB /* LogSanitizerTests.swift in Sources */,
615D52BF2C88A98300F8B8FC /* SynchronizedTagsTests.swift in Sources */,
D2D30E612A40CD310020C553 /* LogsTests.swift in Sources */,
D2A783F529A534F9003B03BB /* LogEventBuilderTests.swift in Sources */,
D242C2A22A14D747004B4980 /* RemoteLoggerTests.swift in Sources */,
D20FD9D02AC6FF42004D3569 /* WebViewLogReceiverTests.swift in Sources */,
D2A783F629A534F9003B03BB /* LoggingFeatureMocks.swift in Sources */,
D2B249982A45E10500DD4F9F /* LoggerTests.swift in Sources */,
D2A783F729A534F9003B03BB /* LogMessageReceiverTests.swift in Sources */,
615D52C22C88AB1E00F8B8FC /* SynchronizedAttributesTests.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down
2 changes: 1 addition & 1 deletion DatadogCore/Sources/Core/DatadogCore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ extension DatadogCore: DatadogCoreProtocol {
}
}

internal class CoreFeatureScope<Feature>: FeatureScope where Feature: DatadogFeature {
internal class CoreFeatureScope<Feature>: @unchecked Sendable, FeatureScope where Feature: DatadogFeature {
private weak var core: DatadogCore?
private let store: FeatureDataStore

Expand Down
4 changes: 2 additions & 2 deletions DatadogCore/Tests/Datadog/LoggerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -986,7 +986,7 @@ class LoggerTests: XCTestCase {
)
XCTAssertEqual(
dd.logger.criticalLog?.error?.message,
"πŸ”₯ Datadog SDK usage error: `Datadog.initialize()` must be called prior to `Logger.builder.build()`."
"πŸ”₯ Datadog SDK usage error: `Datadog.initialize()` must be called prior to `Logger.create()`."
)
XCTAssertTrue(logger is NOPLogger)
}
Expand All @@ -1009,7 +1009,7 @@ class LoggerTests: XCTestCase {
)
XCTAssertEqual(
dd.logger.criticalLog?.error?.message,
"πŸ”₯ Datadog SDK usage error: `Logger.builder.build()` produces a non-functional logger, as the logging feature is disabled."
"πŸ”₯ Datadog SDK usage error: `Logger.create()` produces a non-functional logger because the `Logs` feature was not enabled."
)
XCTAssertTrue(logger is NOPLogger)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ private struct FeatureScopeProxy: FeatureScope {
}
}

private class FeatureScopeInterceptor {
private final class FeatureScopeInterceptor: @unchecked Sendable {
struct InterceptingWriter: Writer {
static let jsonEncoder = JSONEncoder.dd.default()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import Foundation
import DatadogInternal
import CrashReporter
@preconcurrency import CrashReporter

internal extension PLCrashReporterConfig {
/// `PLCR` configuration used for `DatadogCrashReporting`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import Foundation
import DatadogInternal

/// An interface of 3rd party crash reporter used by the DatadogCrashReporting.
internal protocol ThirdPartyCrashReporter {
internal protocol ThirdPartyCrashReporter: Sendable {
/// Initializes and enables the crash reporter.
init() throws

Expand Down
19 changes: 10 additions & 9 deletions DatadogCrashReporting/Tests/CrashReportingPluginTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,14 @@ class CrashReportingPluginTests: XCTestCase {

// MARK: - Handling Errors

private let printFunction = PrintFunctionMock()

func testGivenPendingCrashReport_whenItsLoadingFails_itPrintsError() throws {
let expectation = self.expectation(description: "No Crash Report was delivered to the caller.")
var errorPrinted: String?

consolePrint = { message, _ in errorPrinted = message }
defer { consolePrint = { message, _ in print(message) } }
let previousPrint = consolePrint
consolePrint = printFunction.print
defer { consolePrint = previousPrint }

let crashReporter = try ThirdPartyCrashReporterMock()
let plugin = PLCrashReporterPlugin { crashReporter }
Expand All @@ -126,16 +128,15 @@ class CrashReportingPluginTests: XCTestCase {
waitForExpectations(timeout: 0.5, handler: nil)
XCTAssertFalse(crashReporter.hasPurgedPendingCrashReport)
XCTAssertEqual(
errorPrinted,
printFunction.printedMessage,
"πŸ”₯ DatadogCrashReporting error: failed to load crash report: Reading error"
)
}

func testWhenCrashReporterCannotBeEnabled_itPrintsError() {
var errorPrinted: String?

consolePrint = { message, _ in errorPrinted = message }
defer { consolePrint = { message, _ in print(message) } }
let previousPrint = consolePrint
consolePrint = printFunction.print
defer { consolePrint = previousPrint }

// When
ThirdPartyCrashReporterMock.initializationError = ErrorMock("Initialization error")
Expand All @@ -145,7 +146,7 @@ class CrashReportingPluginTests: XCTestCase {
_ = PLCrashReporterPlugin { try ThirdPartyCrashReporterMock() }

XCTAssertEqual(
errorPrinted,
printFunction.printedMessage,
"πŸ”₯ DatadogCrashReporting error: failed to enable crash reporter: Initialization error"
)
}
Expand Down
3 changes: 1 addition & 2 deletions DatadogCrashReporting/Tests/Mocks.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@

import DatadogInternal
import CrashReporter

@testable import DatadogCrashReporting

internal class ThirdPartyCrashReporterMock: ThirdPartyCrashReporter {
internal final class ThirdPartyCrashReporterMock: ThirdPartyCrashReporter, @unchecked Sendable {
static var initializationError: Error?

var pendingCrashReport: DDCrashReport?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public extension Thread {
}

/// A protocol for types capable of generating backtrace reports.
public protocol BacktraceReporting {
public protocol BacktraceReporting: Sendable {
/// Generates a backtrace report for given thread ID.
///
/// The thread given by `threadID` will be promoted in the main stack of returned `BacktraceReport` (`report.stack`).
Expand All @@ -41,7 +41,7 @@ public extension BacktraceReporting {
}
}

internal struct CoreBacktraceReporter: BacktraceReporting {
internal struct CoreBacktraceReporter: BacktraceReporting, @unchecked Sendable {
/// A weak core reference.
private weak var core: DatadogCoreProtocol?

Expand Down
2 changes: 1 addition & 1 deletion DatadogInternal/Sources/Context/DateProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import Foundation

/// Provides current device time information.
public protocol DateProvider {
public protocol DateProvider: Sendable {
/// Current device time.
///
/// A specific point in time, independent of any calendar or time zone.
Expand Down
2 changes: 1 addition & 1 deletion DatadogInternal/Sources/DD.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import OSLog
#endif

/// Function printing `String` content to console.
public var consolePrint: (String, CoreLoggerLevel) -> Void = { message, level in
public var consolePrint: @Sendable (String, CoreLoggerLevel) -> Void = { message, level in
#if canImport(OSLog)
if #available(iOS 14.0, tvOS 14.0, *) {
switch level {
Expand Down
2 changes: 1 addition & 1 deletion DatadogInternal/Sources/DatadogCoreProtocol.swift
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ extension BaggageSharing {
}

/// Feature scope provides a context and a writer to build a record event.
public protocol FeatureScope: MessageSending, BaggageSharing {
public protocol FeatureScope: MessageSending, BaggageSharing, Sendable {
/// Retrieve the core context and event writer.
///
/// The Feature scope provides the current Datadog context and event writer for building and recording events.
Expand Down
23 changes: 6 additions & 17 deletions DatadogInternal/Sources/Utils/DateFormatting.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,31 +6,20 @@

import Foundation

public protocol DateFormatterType {
public protocol DateFormatterType: Sendable {
func string(from date: Date) -> String
func date(from string: String) -> Date?
}

extension ISO8601DateFormatter: DateFormatterType {}
extension DateFormatter: DateFormatterType {}
extension ISO8601DateFormatter: DateFormatterType, @unchecked Sendable {}
extension DateFormatter: DateFormatterType, @unchecked Sendable {}

/// Date formatter producing `ISO8601` string representation of a given date.
/// Should be used to encode dates in messages send to the server.
public let iso8601DateFormatter: DateFormatterType = {
// As there is a known crash in iOS 11.0 and 11.1 when using `.withFractionalSeconds` option in `ISO8601DateFormatter`,
// we use different `DateFormatterType` implementation depending on the OS version. The problem was fixed by Apple in iOS 11.2.
if #available(iOS 11.2, *) {
let formatter = ISO8601DateFormatter()
formatter.formatOptions.insert(.withFractionalSeconds)
return formatter
} else {
let iso8601Formatter = DateFormatter()
iso8601Formatter.locale = Locale(identifier: "en_US_POSIX")
iso8601Formatter.timeZone = TimeZone(abbreviation: "UTC")! // swiftlint:disable:this force_unwrapping
iso8601Formatter.calendar = Calendar(identifier: .gregorian)
iso8601Formatter.dateFormat = "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'" // ISO8601 format
return iso8601Formatter
}
Comment on lines -26 to -33
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ’‘ not needed anymore after we dropped iOS 11

let formatter = ISO8601DateFormatter()
formatter.formatOptions.insert(.withFractionalSeconds)
return formatter
}()

/// Date formatter producing string representation of a given date for user-facing features (like console output).
Expand Down
4 changes: 2 additions & 2 deletions DatadogLogs/Sources/ConsoleLogger.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ internal final class ConsoleLogger: LoggerProtocol {
/// The prefix to use when rendering log.
private let prefix: String
/// The function used to render log.
private let printFunction: (String, CoreLoggerLevel) -> Void
private let printFunction: @Sendable (String, CoreLoggerLevel) -> Void

init(
configuration: Configuration,
dateProvider: DateProvider,
printFunction: @escaping (String, CoreLoggerLevel) -> Void
printFunction: @escaping @Sendable (String, CoreLoggerLevel) -> Void
) {
self.dateProvider = dateProvider
self.timeFormatter = presentationDateFormatter(withTimeZone: configuration.timeZone)
Expand Down
17 changes: 3 additions & 14 deletions DatadogLogs/Sources/Feature/LogsFeature.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ internal struct LogsFeature: DatadogRemoteFeature {

let backtraceReporter: BacktraceReporting?

@ReadWriteLock
private var attributes: [String: Encodable] = [:]
/// Global attributes attached to every log event.
let attributes: SynchronizedAttributes

/// Time provider.
let dateProvider: DateProvider
Expand Down Expand Up @@ -59,17 +59,6 @@ internal struct LogsFeature: DatadogRemoteFeature {
self.messageReceiver = messageReceiver
self.dateProvider = dateProvider
self.backtraceReporter = backtraceReporter
}

internal func addAttribute(forKey key: AttributeKey, value: AttributeValue) {
_attributes.mutate { $0[key] = value }
}

internal func removeAttribute(forKey key: AttributeKey) {
_attributes.mutate { $0.removeValue(forKey: key) }
}

internal func getAttributes() -> [String: Encodable] {
return attributes
self.attributes = SynchronizedAttributes(attributes: [:])
}
}
47 changes: 47 additions & 0 deletions DatadogLogs/Sources/Log/SynchronizedAttributes.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0.
* This product includes software developed at Datadog (https://www.datadoghq.com/).
* Copyright 2019-Present Datadog, Inc.
*/

import Foundation
import DatadogInternal

/// A thread-safe container for managing attributes in a key-value format.
/// This class allows concurrent access and modification of attributes, ensuring data consistency
/// through the use of a `ReadWriteLock`. It is designed to be used in scenarios where attributes
/// need to be safely managed across multiple threads or tasks.
internal final class SynchronizedAttributes: Sendable {
/// The underlying dictionary of attributes, wrapped in a `ReadWriteLock` to ensure thread safety.
private let attributes: ReadWriteLock<[String: Encodable]>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/question We are not using the ReadWriteLock as a property wrapper in multiple places, is it because it would require it to be a var? Can't this be solved by making ReadWriteLock Sendable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ReadWriteLock is Sendable already - it was done in this PR. And yes, if using property wrapper, this will become var, raising concurrency issue. I agree with what was discussed here - property wrappers become quite unusable with strict concurrency.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, makes sense πŸ‘


/// Initializes a new instance of `SynchronizedAttributes` with the provided dictionary.
///
/// - Parameter attributes: A dictionary of initial attributes.
init(attributes: [String: Encodable]) {
self.attributes = .init(wrappedValue: attributes)
}

/// Adds or updates an attribute in the container.
///
/// - Parameters:
/// - key: The key associated with the attribute.
/// - value: The value to associate with the key.
func addAttribute(key: AttributeKey, value: AttributeValue) {
attributes.mutate { $0[key] = value }
}

/// Removes an attribute from the container.
///
/// - Parameter key: The key of the attribute to remove.
func removeAttribute(forKey key: AttributeKey) {
attributes.mutate { $0.removeValue(forKey: key) }
}

/// Retrieves the current dictionary of attributes.
///
/// - Returns: A dictionary containing all the attributes.
func getAttributes() -> [String: Encodable] {
return attributes.wrappedValue
}
}
Loading