From f59ef7292fdf0f6075e498df1517fb7009dd6a21 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Mon, 14 Sep 2020 21:08:25 +0900 Subject: [PATCH 01/14] Final API proposal: Less is More / Simple is Better --- Package.swift | 21 +- Sources/Baggage/Baggage.swift | 237 +++++++++++++++ Sources/Baggage/BaggageContext.swift | 239 --------------- Sources/Baggage/BaggageContextCarrier.swift | 54 ---- Sources/Baggage/BaggageKey.swift | 74 +++++ Sources/BaggageContext/Context.swift | 287 ++++++++++++++++++ .../ContextLogHandler.swift} | 25 +- .../ArgParser.swift | 0 .../BenchmarkCategory.swift | 0 .../BenchmarkTools.swift | 0 .../DriverUtils.swift | 0 .../README_SWIFT.md | 0 .../BaggagePassingBenchmarks.swift | 33 +- .../locks.swift | 0 .../main.swift | 2 +- .../Logger+BaggageContext.swift | 29 -- .../LoggingBaggageContextCarrier.swift | 44 --- .../BaggageContextTests+XCTest.swift} | 13 +- .../BaggageContextTests.swift | 213 +++++++++++++ .../TestLogger.swift | 0 .../LoggingBaggageContextCarrierTests.swift | 145 --------- ...XCTest.swift => ContextTests+XCTest.swift} | 6 +- ...eContextTests.swift => ContextTests.swift} | 33 +- ...ift => FrameworkContextTests+XCTest.swift} | 7 +- ...ests.swift => FrameworkContextTests.swift} | 38 ++- Tests/LinuxMain.swift | 6 +- 26 files changed, 915 insertions(+), 591 deletions(-) create mode 100644 Sources/Baggage/Baggage.swift delete mode 100644 Sources/Baggage/BaggageContext.swift delete mode 100644 Sources/Baggage/BaggageContextCarrier.swift create mode 100644 Sources/Baggage/BaggageKey.swift create mode 100644 Sources/BaggageContext/Context.swift rename Sources/{BaggageLogging/BaggageMetadataLogHandler.swift => BaggageContext/ContextLogHandler.swift} (80%) rename Sources/{BaggageBenchmarkTools => BaggageContextBenchmarkTools}/ArgParser.swift (100%) rename Sources/{BaggageBenchmarkTools => BaggageContextBenchmarkTools}/BenchmarkCategory.swift (100%) rename Sources/{BaggageBenchmarkTools => BaggageContextBenchmarkTools}/BenchmarkTools.swift (100%) rename Sources/{BaggageBenchmarkTools => BaggageContextBenchmarkTools}/DriverUtils.swift (100%) rename Sources/{BaggageBenchmarkTools => BaggageContextBenchmarkTools}/README_SWIFT.md (100%) rename Sources/{BaggageBenchmarks => BaggageContextBenchmarks}/BaggagePassingBenchmarks.swift (86%) rename Sources/{BaggageBenchmarks => BaggageContextBenchmarks}/locks.swift (100%) rename Sources/{BaggageBenchmarks => BaggageContextBenchmarks}/main.swift (97%) delete mode 100644 Sources/BaggageLogging/Logger+BaggageContext.swift delete mode 100644 Sources/BaggageLogging/LoggingBaggageContextCarrier.swift rename Tests/{BaggageLoggingTests/LoggingBaggageContextCarrierTests+XCTest.swift => BaggageContextTests/BaggageContextTests+XCTest.swift} (54%) create mode 100644 Tests/BaggageContextTests/BaggageContextTests.swift rename Tests/{BaggageLoggingTests => BaggageContextTests}/TestLogger.swift (100%) delete mode 100644 Tests/BaggageLoggingTests/LoggingBaggageContextCarrierTests.swift rename Tests/BaggageTests/{BaggageContextTests+XCTest.swift => ContextTests+XCTest.swift} (90%) rename Tests/BaggageTests/{BaggageContextTests.swift => ContextTests.swift} (75%) rename Tests/BaggageTests/{BaggageContextCarrierTests+XCTest.swift => FrameworkContextTests+XCTest.swift} (79%) rename Tests/BaggageTests/{BaggageContextCarrierTests.swift => FrameworkContextTests.swift} (57%) diff --git a/Package.swift b/Package.swift index f7b8008..96eabcc 100644 --- a/Package.swift +++ b/Package.swift @@ -2,7 +2,7 @@ import PackageDescription let package = Package( - name: "swift-baggage-context", + name: "swift-context", products: [ .library( name: "Baggage", @@ -11,9 +11,9 @@ let package = Package( ] ), .library( - name: "BaggageLogging", + name: "BaggageContext", targets: [ - "BaggageLogging", + "BaggageContext", ] ), ], @@ -27,7 +27,7 @@ let package = Package( ), .target( - name: "BaggageLogging", + name: "BaggageContext", dependencies: [ "Baggage", .product(name: "Logging", package: "swift-log"), @@ -45,10 +45,10 @@ let package = Package( ), .testTarget( - name: "BaggageLoggingTests", + name: "BaggageContextTests", dependencies: [ "Baggage", - "BaggageLogging", + "BaggageContext", ] ), @@ -56,15 +56,14 @@ let package = Package( // MARK: Performance / Benchmarks .target( - name: "BaggageBenchmarks", + name: "BaggageContextBenchmarks", dependencies: [ - "Baggage", - "BaggageLogging", - "BaggageBenchmarkTools", + "BaggageContext", + "BaggageContextBenchmarkTools", ] ), .target( - name: "BaggageBenchmarkTools", + name: "BaggageContextBenchmarkTools", dependencies: [] ), ] diff --git a/Sources/Baggage/Baggage.swift b/Sources/Baggage/Baggage.swift new file mode 100644 index 0000000..93f5c4e --- /dev/null +++ b/Sources/Baggage/Baggage.swift @@ -0,0 +1,237 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift Baggage Context open source project +// +// Copyright (c) 2020 Moritz Lang and the Swift Baggage Context project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +// ==== ---------------------------------------------------------------------------------------------------------------- +// MARK: Baggage + +/// A `Baggage` is a heterogeneous storage type with value semantics for keyed values in a type-safe +/// fashion. +/// +/// Its values are uniquely identified via `BaggageKey`s (by type identity). These keys also dictate the type of +/// value allowed for a specific key-value pair through their associated type `Value`. +/// +/// ## Defining keys and accessing values +/// Baggage keys are defined as types, most commonly case-less enums (as no actual instances are actually required) +/// which conform to the `Baggage.Key` protocol: +/// +/// private enum TestIDKey: BaggageKey { +/// typealias Value = String +/// } +/// +/// While defining a key, one should also immediately declare an extension on `BaggageProtocol`, +/// to allow convenient and discoverable ways to interact with the baggage item, the extension should take the form of: +/// +/// extension BaggageProtocol { +/// var testID: TestIDKey.Value? { +/// get { +/// self[TestIDKey.self] +/// } set { +/// self[TestIDKey.self] = newValue +/// } +/// } +/// } +/// +/// For consistency, it is recommended to name key types with the `...Key` suffix (e.g. `SomethingKey`) and the property +/// used to access a value identifier by such key the prefix of the key (e.g. `something`). Please also observe the usual +/// Swift naming conventions, e.g. prefer `ID` to `Id` etc. +/// +/// ## Usage +/// Using a baggage container is fairly straight forward, as it boils down to using the prepared computed properties: +/// +/// var context = Baggage.background +/// // set a new value +/// context.testID = "abc" +/// // retrieve a stored value +/// let testID = context.testID ?? "default" +/// // remove a stored value +/// context[TestIDKey.self] = nil +/// +/// Note that normally a baggage should not be "created" ad-hoc by user code, but rather it should be passed to it from +/// a runtime. For example, when working in an HTTP server framework, it is most likely that the baggage is already passed +/// directly or indirectly (e.g. in a `BaggageContext` or `FrameworkContext`) +/// +/// ### Accessing all values +/// +/// The only way to access "all" values in a baggage context is by using the `forEachBaggageItem` function. +/// The baggage container on purpose does not expose more functions to prevent abuse and treating it as too much of an +/// arbitrary value smuggling container, but only make it convenient for tracing and instrumentation systems which need +/// to access either specific or all items carried inside a baggage. +public struct Baggage: BaggageProtocol { + public typealias Key = BaggageKey + + private var _storage = [AnyBaggageKey: Any]() + + /// Internal on purpose, please use `Baggage.TODO` or `Baggage.background` to create an "empty" context, + /// which carries more meaning to other developers why an empty context was used. + init() {} + + public var baggage: Baggage { + get { + return self + } + set { + self = newValue + } + } +} + +extension Baggage { + /// Creates a new empty baggage, generally used for background processing tasks or an "initial" baggage to be immediately + /// populated with some values by a framework or runtime. + /// + /// Typically, this would only be called in a "top" or "background" setting, such as the main function, initialization, + /// tests, beginning of some background task or some other top-level baggage to be immediately populated with incoming request/message information. + /// + /// ## Usage in frameworks and libraries + /// This function is really only intended to be used frameworks and libraries, at the "top-level" where a request's, + /// message's or task's processing is initiated. For example, a framework handling requests, should create an empty + /// context when handling a request only to immediately populate it with useful trace information extracted from e.g. + /// request headers. + /// + /// ## Usage in applications + /// Application code should never have to create an empty context during the processing lifetime of any request, + /// and only should create contexts if some processing is performed in the background - thus the naming of this property. + /// + /// Usually, a framework such as an HTTP server or similar "request handler" would already provide users + /// with a context to be passed along through subsequent calls. + /// + /// If unsure where to obtain a context from, prefer using `.TODO("Not sure where I should get a context from here?")`, + /// in order to inform other developers that the lack of context passing was not done on purpose, but rather because either + /// not being sure where to obtain a context from, or other framework limitations -- e.g. the outer framework not being + /// baggage context aware just yet. + public static var background: Baggage { + return Baggage() + } +} + +extension Baggage { + /// A baggage intended as a placeholder until a real value can be passed through a function call. + /// + /// It should ONLY be used while prototyping or when the passing of the proper context is not yet possible, + /// e.g. because an external library did not pass it correctly and has to be fixed before the proper context + /// can be obtained where the TO-DO is currently used. + /// + /// ## Crashing on TO-DO context creation + /// You may set the `BAGGAGE_CRASH_TODOS` variable while compiling a project in order to make calls to this function crash + /// with a fatal error, indicating where a to-do baggage context was used. This comes in handy when wanting to ensure that + /// a project never ends up using with code initially was written as "was lazy, did not pass context", yet the + /// project requires context passing to be done correctly throughout the application. Similar checks can be performed + /// at compile time easily using linters (not yet implemented), since it is always valid enough to detect a to-do context + /// being passed as illegal and warn or error when spotted. + /// + /// ## Example + /// + /// frameworkHandler { what in + /// hello(who: "World", baggage: .TODO("The framework XYZ should be modified to pass us a context here, and we'd pass it along")) + /// } + /// + /// - Parameters: + /// - reason: Informational reason for developers, why a placeholder context was used instead of a proper one, + /// - Returns: Empty "to-do" baggage context which should be eventually replaced with a carried through one, or `background`. + public static func TODO(_ reason: StaticString? = "", function: String = #function, file: String = #file, line: UInt = #line) -> Baggage { + var context = Baggage.background + #if BAGGAGE_CRASH_TODOS + fatalError("BAGGAGE_CRASH_TODOS: at \(file):\(line) (function \(function)), reason: \(reason)") + #else + context[TODOKey.self] = .init(file: file, line: line) + return context + #endif + } + + private enum TODOKey: BaggageKey { + typealias Value = TODOLocation + static var name: String? { + return "todo" + } + } +} + +/// The `BaggageProtocol` should not directly be used in APIs. +/// +/// Prefer accepting the `ContextProtocol` in APIs which need to accept a baggage context. +/// If `Context` can not be used for some reason, it is preferable to accept a `Baggage` directly rather than this protocol. +public protocol BaggageProtocol { + /// Get the `Baggage` container. + var baggage: Baggage { get set } + + /// Provides type-safe access to the baggage's values. + /// + /// Rather than using this subscript directly, users SHOULD offer a convenience accessor to their values, + /// using the following pattern: + /// + /// extension BaggageProtocol { + /// var testID: TestIDKey.Value? { + /// get { + /// self[TestIDKey.self] + /// } + /// set { + /// self[TestIDKey.self] = newValue + /// } + /// } + /// } + /// + /// Note that specific baggage and context types MAY (and usually do), offer also a way to set baggage values, + /// however in the most general case it is not required, as some frameworks may only be able to offer reading. + subscript(_ key: Key.Type) -> Key.Value? { get set } + + /// Calls the given closure for each item contained in the underlying `Baggage`. + /// + /// Order of those invocations is NOT guaranteed and should not be relied on. + /// + /// - Parameter body: A closure invoked with the type erased key and value stored for the key in this baggage. + func forEachBaggageItem(_ body: (AnyBaggageKey, Any) throws -> Void) rethrows +} + +/// Writable `BaggageProtocol`. +/// +/// - SeeAlso: `_WritableBaggageProtocol` +public protocol _WritableBaggageProtocol: BaggageProtocol { + var baggage: Baggage { get set } +} + +extension Baggage { + public subscript(_ key: Key.Type) -> Key.Value? { + get { + guard let value = self._storage[AnyBaggageKey(key)] else { return nil } + // safe to force-cast as this subscript is the only way to set a value. + return (value as! Key.Value) + } + set { + self._storage[AnyBaggageKey(key)] = newValue + } + } + + public func forEachBaggageItem(_ body: (AnyBaggageKey, Any) throws -> Void) rethrows { + try self._storage.forEach { key, value in + try body(key, value) + } + } +} + +extension Baggage: CustomStringConvertible { + /// A context's description prints only keys of the contained values. + /// This is in order to prevent spilling a lot of detailed information of carried values accidentally. + /// + /// `Baggage`s are not intended to be printed "raw" but rather inter-operate with tracing, logging and other systems, + /// which can use the `forEach` function providing access to its underlying values. + public var description: String { + return "\(type(of: self).self)(keys: \(self._storage.map { $0.key.name }))" + } +} + +/// Carried automatically by a "to do" baggage context. +/// It can be used to track where a context originated and which "to do" context must be fixed into a real one to avoid this. +public struct TODOLocation { + let file: String + let line: UInt +} diff --git a/Sources/Baggage/BaggageContext.swift b/Sources/Baggage/BaggageContext.swift deleted file mode 100644 index 13a5502..0000000 --- a/Sources/Baggage/BaggageContext.swift +++ /dev/null @@ -1,239 +0,0 @@ -//===----------------------------------------------------------------------===// -// -// This source file is part of the Swift Baggage Context open source project -// -// Copyright (c) 2020 Moritz Lang and the Swift Baggage Context project authors -// Licensed under Apache License v2.0 -// -// See LICENSE.txt for license information -// -// SPDX-License-Identifier: Apache-2.0 -// -//===----------------------------------------------------------------------===// - -/// A `BaggageContext` is a heterogeneous storage type with value semantics for keyed values in a type-safe -/// fashion. Its values are uniquely identified via `BaggageContextKey`s. These keys also dictate the type of -/// value allowed for a specific key-value pair through their associated type `Value`. -/// -/// ## Subscript access -/// You may access the stored values by subscripting with a key type conforming to `BaggageContextKey`. -/// -/// enum TestIDKey: BaggageContextKey { -/// typealias Value = String -/// } -/// -/// var context = BaggageContext.background -/// // set a new value -/// context[TestIDKey.self] = "abc" -/// // retrieve a stored value -/// context[TestIDKey.self] ?? "default" -/// // remove a stored value -/// context[TestIDKey.self] = nil -/// -/// ## Convenience extensions -/// -/// Libraries may also want to provide an extension, offering the values that users are expected to reach for -/// using the following pattern: -/// -/// extension BaggageContextProtocol { -/// var testID: TestIDKey.Value? { -/// get { -/// self[TestIDKey.self] -/// } set { -/// self[TestIDKey.self] = newValue -/// } -/// } -/// } -public struct BaggageContext: BaggageContextProtocol { - private var _storage = [AnyBaggageContextKey: Any]() - - /// Internal on purpose, please use `TODO` or `.background` to create an "empty" context, - /// which carries more meaning to other developers why an empty context was used. - init() {} - - public subscript(_ key: Key.Type) -> Key.Value? { - get { - guard let value = self._storage[AnyBaggageContextKey(key)] else { return nil } - // safe to force-cast as this subscript is the only way to set a value. - return (value as! Key.Value) - } set { - self._storage[AnyBaggageContextKey(key)] = newValue - } - } - - public func forEach(_ body: (AnyBaggageContextKey, Any) throws -> Void) rethrows { - try self._storage.forEach { key, value in - try body(key, value) - } - } -} - -extension BaggageContext: CustomStringConvertible { - /// A context's description prints only keys of the contained values. - /// This is in order to prevent spilling a lot of detailed information of carried values accidentally. - /// - /// `BaggageContext`s are not intended to be printed "raw" but rather inter-operate with tracing, logging and other systems, - /// which can use the `forEach` function providing access to its underlying values. - public var description: String { - return "\(type(of: self).self)(keys: \(self._storage.map { $0.key.name }))" - } -} - -public protocol BaggageContextProtocol { - /// Provides type-safe access to the baggage's values. - /// - /// Rather than using this subscript directly, users are encouraged to offer a convenience accessor to their values, - /// using the following pattern: - /// - /// extension BaggageContextProtocol { - /// var testID: TestIDKey.Value? { - /// get { - /// self[TestIDKey.self] - /// } set { - /// self[TestIDKey.self] = newValue - /// } - /// } - /// } - subscript(_ key: Key.Type) -> Key.Value? { get set } - - /// Calls the given closure on each key/value pair in the `BaggageContext`. - /// - /// - Parameter body: A closure invoked with the type erased key and value stored for the key in this baggage. - func forEach(_ body: (AnyBaggageContextKey, Any) throws -> Void) rethrows -} - -// ==== ------------------------------------------------------------------------ -// MARK: Baggage keys - -/// `BaggageContextKey`s are used as keys in a `BaggageContext`. Their associated type `Value` guarantees type-safety. -/// To give your `BaggageContextKey` an explicit name you may override the `name` property. -/// -/// In general, `BaggageContextKey`s should be `internal` to the part of a system using it. It is strongly recommended to do -/// convenience extensions on `BaggageContextProtocol`, using the keys directly is considered an anti-pattern. -/// -/// extension BaggageContextProtocol { -/// var testID: TestIDKey.Value? { -/// get { -/// self[TestIDKey.self] -/// } set { -/// self[TestIDKey.self] = newValue -/// } -/// } -/// } -public protocol BaggageContextKey { - /// The type of `Value` uniquely identified by this key. - associatedtype Value - - /// The human-readable name of this key. Defaults to `nil`. - static var name: String? { get } -} - -extension BaggageContextKey { - public static var name: String? { return nil } -} - -/// A type-erased `BaggageContextKey` used when iterating through the `BaggageContext` using its `forEach` method. -public struct AnyBaggageContextKey { - /// The key's type represented erased to an `Any.Type`. - public let keyType: Any.Type - - private let _name: String? - - /// A human-readable String representation of the underlying key. - /// If no explicit name has been set on the wrapped key the type name is used. - public var name: String { - return self._name ?? String(describing: self.keyType.self) - } - - init(_ keyType: Key.Type) where Key: BaggageContextKey { - self.keyType = keyType - self._name = keyType.name - } -} - -extension AnyBaggageContextKey: Hashable { - public static func == (lhs: AnyBaggageContextKey, rhs: AnyBaggageContextKey) -> Bool { - return ObjectIdentifier(lhs.keyType) == ObjectIdentifier(rhs.keyType) - } - - public func hash(into hasher: inout Hasher) { - hasher.combine(ObjectIdentifier(self.keyType)) - } -} - -// ==== ---------------------------------------------------------------------------------------------------------------- -// MARK: Background BaggageContext - -extension BaggageContextProtocol { - /// An empty baggage context intended as the "root" or "initial" baggage context background processing tasks, or as the "root" baggage context. - /// - /// It is never canceled, has no values, and has no deadline. - /// It is typically used by the main function, initialization, and tests, and as the top-level Context for incoming requests. - /// - /// ### Usage in frameworks and libraries - /// This function is really only intended to be used frameworks and libraries, at the "top-level" where a request's, - /// message's or task's processing is initiated. For example, a framework handling requests, should create an empty - /// context when handling a request only to immediately populate it with useful trace information extracted from e.g. - /// request headers. - /// - /// ### Usage in applications - /// Application code should never have to create an empty context during the processing lifetime of any request, - /// and only should create contexts if some processing is performed in the background - thus the naming of this property. - /// - /// Usually, a framework such as an HTTP server or similar "request handler" would already provide users - /// with a context to be passed along through subsequent calls. - /// - /// If unsure where to obtain a context from, prefer using `.TODO("Not sure where I should get a context from here?")`, - /// such that other developers are informed that the lack of context was not done on purpose, but rather because either - /// not being sure where to obtain a context from, or other framework limitations -- e.g. the outer framework not being - /// context aware just yet. - public static var background: BaggageContext { - return BaggageContext() - } -} - -// ==== ---------------------------------------------------------------------------------------------------------------- -// MARK: "TO DO" BaggageContext - -extension BaggageContextProtocol { - /// A baggage context intended as a placeholder until a real value can be passed through a function call. - /// - /// It should ONLY be used while prototyping or when the passing of the proper context is not yet possible, - /// e.g. because an external library did not pass it correctly and has to be fixed before the proper context - /// can be obtained where the TO-DO is currently used. - /// - /// ### Crashing on TO-DO context creation - /// You may set the `BAGGAGE_CRASH_TODOS` variable while compiling a project in order to make calls to this function crash - /// with a fatal error, indicating where a to-do baggage context was used. This comes in handy when wanting to ensure that - /// a project never ends up using with code initially was written as "was lazy, did not pass context", yet the - /// project requires context passing to be done correctly throughout the application. Similar checks can be performed - /// at compile time easily using linters (not yet implemented), since it is always valid enough to detect a to-do context - /// being passed as illegal and warn or error when spotted. - /// - /// - Parameters: - /// - reason: Informational reason for developers, why a placeholder context was used instead of a proper one, - /// - Returns: Empty "to-do" baggage context which should be eventually replaced with a carried through one, or `background`. - public static func TODO(_ reason: StaticString? = "", function: String = #function, file: String = #file, line: UInt = #line) -> BaggageContext { - var context = BaggageContext.background - #if BAGGAGE_CRASH_TODOS - fatalError("BAGGAGE_CRASH_TODOS: at \(file):\(line) (function \(function)), reason: \(reason)") - #else - context[TODOKey.self] = .init(file: file, line: line) - return context - #endif - } -} - -internal enum TODOKey: BaggageContextKey { - typealias Value = TODOLocation - static var name: String? { - return "todo" - } -} - -/// Carried automatically by a "to do" baggage context. -/// It can be used to track where a context originated and which "to do" context must be fixed into a real one to avoid this. -public struct TODOLocation { - let file: String - let line: UInt -} diff --git a/Sources/Baggage/BaggageContextCarrier.swift b/Sources/Baggage/BaggageContextCarrier.swift deleted file mode 100644 index 135935b..0000000 --- a/Sources/Baggage/BaggageContextCarrier.swift +++ /dev/null @@ -1,54 +0,0 @@ -//===----------------------------------------------------------------------===// -// -// This source file is part of the Swift Baggage Context open source project -// -// Copyright (c) 2020 Moritz Lang and the Swift Baggage Context project authors -// Licensed under Apache License v2.0 -// -// See LICENSE.txt for license information -// -// SPDX-License-Identifier: Apache-2.0 -// -//===----------------------------------------------------------------------===// - -// ==== ---------------------------------------------------------------------------------------------------------------- -// MARK: Framework Context Protocols - -/// Framework context protocols may conform to this protocol if they are used to carry a baggage object. -/// -/// Notice that the baggage context property is spelled as `baggage`, this is purposefully designed in order to read well -/// with framework context's which often will be passed as `context: FrameworkContext` and used as `context.baggage`. -/// -/// Such carrier protocol also conforms to `BaggageContextProtocol` meaning that it has the same convenient accessors -/// as the actual baggage type. Users should be able to use the `context.myValue` the same way if a raw baggage context, -/// or a framework context was passed around as `context` parameter, allowing for easier migrations between those two when needed. -public protocol BaggageContextCarrier: BaggageContextProtocol { - /// The underlying `BaggageContext`. - var baggage: BaggageContext { get set } -} - -extension BaggageContextCarrier { - public subscript(baggageKey: Key.Type) -> Key.Value? { - get { - return self.baggage[baggageKey] - } set { - self.baggage[baggageKey] = newValue - } - } - - public func forEach(_ body: (AnyBaggageContextKey, Any) throws -> Void) rethrows { - try self.baggage.forEach(body) - } -} - -/// A baggage itself also is a carrier of _itself_. -extension BaggageContext: BaggageContextCarrier { - public var baggage: BaggageContext { - get { - return self - } - set { - self = newValue - } - } -} diff --git a/Sources/Baggage/BaggageKey.swift b/Sources/Baggage/BaggageKey.swift new file mode 100644 index 0000000..d3960b2 --- /dev/null +++ b/Sources/Baggage/BaggageKey.swift @@ -0,0 +1,74 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift Baggage Context open source project +// +// Copyright (c) 2020 Moritz Lang and the Swift Baggage Context project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +/// `BaggageKey`s are used as keys in a `Baggage`. Their associated type `Value` guarantees type-safety. +/// To give your `BaggageKey` an explicit name you may override the `name` property. +/// +/// In general, `BaggageKey`s should be `internal` or `private` to the part of a system using it. It is strongly recommended to do +/// convenience extensions on `BaggageProtocol`, using the keys directly is considered an anti-pattern. +/// +/// private enum TestIDKey: Baggage.Key { +/// typealias Value = String +/// static var name: String? { "test-id" } +/// } +/// +/// extension BaggageProtocol { +/// var testID: TestIDKey.Value? { +/// get { +/// self[TestIDKey.self] +/// } set { +/// self[TestIDKey.self] = newValue +/// } +/// } +/// } +public protocol BaggageKey { + /// The type of `Value` uniquely identified by this key. + associatedtype Value + + /// The human-readable name of this key. Defaults to `nil`. + /// May be used as key during serialization of the baggage item. + static var name: String? { get } +} + +extension BaggageKey { + public static var name: String? { return nil } +} + +/// A type-erased `BaggageKey` used when iterating through the `Baggage` using its `forEach` method. +public struct AnyBaggageKey { + /// The key's type represented erased to an `Any.Type`. + public let keyType: Any.Type + + private let _name: String? + + /// A human-readable String representation of the underlying key. + /// If no explicit name has been set on the wrapped key the type name is used. + public var name: String { + return self._name ?? String(describing: self.keyType.self) + } + + init(_ keyType: Key.Type) where Key: BaggageKey { + self.keyType = keyType + self._name = keyType.name + } +} + +extension AnyBaggageKey: Hashable { + public static func == (lhs: AnyBaggageKey, rhs: AnyBaggageKey) -> Bool { + return ObjectIdentifier(lhs.keyType) == ObjectIdentifier(rhs.keyType) + } + + public func hash(into hasher: inout Hasher) { + hasher.combine(ObjectIdentifier(self.keyType)) + } +} diff --git a/Sources/BaggageContext/Context.swift b/Sources/BaggageContext/Context.swift new file mode 100644 index 0000000..2e46d00 --- /dev/null +++ b/Sources/BaggageContext/Context.swift @@ -0,0 +1,287 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift Baggage Context open source project +// +// Copyright (c) 2020 Moritz Lang and the Swift Baggage Context project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +import Baggage +import Logging + +/// A default `Context` type. +/// +/// It is a carrier of contextual `Baggage` and related `Logger`, allowing to log and trace throughout a system. +/// +/// Any values set on the `baggage` will be made accessible to the logger as call-site metadata, allowing it to log those. +/// +/// ### Logged Metadata and Baggage Items +/// +/// Please refer to your configured log handler documentation about how to configure which metadata values should be logged +/// and which not, as each log handler may handle and configure those differently. The default implementations log *all* +/// metadata/baggage values present, which often is the right thing, however in larger systems one may want to choose a +/// log handler which allows for configuring these details. +/// +/// ### Accepting context types in APIs +/// +/// It is preferred to accept values of `ContextProtocol` in library APIs, as this yields a more flexible API shape, +/// to which other libraries/frameworks may pass their specific context objects. +/// +/// - SeeAlso: `Baggage` from the Baggage module. +/// - SeeAlso: `Logger` from the SwiftLog package. +public struct Context: ContextProtocol { + /// The `Baggage` carried with this context. + /// It's values will automatically be made available to the `logger` as metadata when logging. + /// + /// Baggage values are different from plain logging metadata in that they are intended to be + /// carried across process and node boundaries (serialized and deserialized) and are made + /// available to instruments using `swift-distributed-tracing`. + public var baggage: Baggage + + public var logger: Logger { + get { + return self._logger.with(self.baggage) + } + set { + self._logger = newValue + } + } + + private var _logger: Logger + + public init(baggage: Baggage, logger underlying: Logger) { + self.baggage = baggage + self._logger = underlying + } + + public var asBaggageContext: Context { + return self + } +} + +// ==== ---------------------------------------------------------------------------------------------------------------- +// MARK: Context Protocol + +/// The `ContextProtocol` MAY be adopted by specific "framework contexts" such as e.g. `CoolFramework.Context` in +/// order to allow users to pass such context directly to libraries accepting any context. +/// +/// This allows frameworks and library authors to offer APIs which compose more easily. +/// Please refer to the "Reference Implementation" notes on each of the requirements to know how to implement this protocol correctly. +/// +/// ### Relation to `BaggageProtocol` +/// The `ContextProtocol` does NOT extend `BaggageProtocol`. +/// +/// A custom context implementation however `MAY` extend both `ContextProtocol` and `BaggageProtocol`. +/// Doing so enables a custom context to obtain the baggage protocol's capability of getting *and setting* +/// properties on a baggage directly, like so: `myContext.traceID = "111-222"` without having to type out the baggage +/// explicitly, as would normally be the case (`myContext.baggage.traceID = "111-222"`). +/// +/// This difference in capabilities is purposefully designed as such, in order to allow context objects implement `ContextProtocol` +/// without having to force them to allow _modifying_ the baggage. Some frameworks or libraries may choose to create immutable +/// context objects, including the baggage they offer there. Users, when wanting to add more contextual information need then to +/// perform the `asBaggageContext`, mutate the resulting context OR use the `with...` fluent-API functions to obtain a mutated context. +/// +/// Please note that if a context does conform to `BaggageProtocol` as well, it will most-likely also offer all properties that +/// users have provided via extensions on it - so be watchful of potential naming conflicts on variable names. +public protocol ContextProtocol { + /// Get the `Baggage` container. + var baggage: Baggage { get } + + /// The `Logger` associated with this context carrier. + /// + /// It automatically populates the loggers metadata based on the `BaggageContext` associated with this context object. + /// + /// ### Implementation note + /// + /// Libraries and/or frameworks which conform to this protocol with their "Framework Context" types, + /// SHOULD implement this logger by wrapping the "raw" logger associated with `_logger.with(self.baggage)` function, + /// which efficiently handles the bridging of baggage to logging metadata values. + /// + /// ### Reference Implementation + /// + /// Writes to the `logger` metadata SHOULD NOT be reflected in the `baggage`, + /// however writes to the underlying `baggage` SHOULD be reflected in the `logger`. + /// + /// struct MyFrameworkContext: ContextProtocol { + /// var baggage: Baggage + /// private let _logger: Logger + /// + /// var logger: Logger { + /// return self._logger.with(self.baggage) + /// } + /// } + var logger: Logger { get } + + /// Get a baggage `Context` struct, regardless of the representation that this context protocol is implemented by. + var asBaggageContext: Context { get } +} + +// ==== ---------------------------------------------------------------------------------------------------------------- +// MARK: `with...` functions + +extension Context { + /// Fluent API allowing for modification of underlying logger when passing the context to other functions. + /// + /// - Parameter logger: Logger that should replace the underlying logger of this context. + /// - Returns: new context, with the passed in `logger` + public func withLogger(_ logger: Logger) -> Context { + var copy = self + copy.logger = logger + return copy + } + + /// Fluent API allowing for modification of underlying log level when passing the context to other functions. + /// + /// - Parameter logLevel: New log level which should be used to create the new context + /// - Returns: new context, with the passed in `logLevel` used for the underlying logger + public func withLogLevel(_ logLevel: Logger.Level) -> Context { + var copy = self + copy.logger.logLevel = logLevel + return copy + } + + /// Fluent API allowing for modification a few baggage values when passing the context to other functions, e.g. + /// + /// makeRequest(url, context: context.withBaggage { + /// $0.traceID = "fake-value" + /// $0.calledFrom = #function + /// }) + /// + /// - Parameter function: + public func withBaggage(_ function: (inout Baggage) -> Void) -> Context { + var baggage = self.baggage + function(&baggage) + return self.withBaggage(baggage) + } + + /// Fluent API allowing for replacement of underlying baggage when passing the context to other functions. + /// + /// - Warning: Use with caution, generally it is not recommended to modify an entire baggage, but rather only add a few values to it. + /// + /// - Parameter baggage: baggage that should *replace* the context's current baggage. + /// - Returns: new context, with the passed in baggage + public func withBaggage(_ baggage: Baggage) -> Context { + var copy = self + copy.baggage = baggage + return copy + } +} + +// ==== ---------------------------------------------------------------------------------------------------------------- +// MARK: Context Initializers + +extension Context { + /// An empty baggage context intended as the "root" or "initial" baggage context background processing tasks, or as the "root" baggage context. + /// + /// It is never canceled, has no values, and has no deadline. + /// It is typically used by the main function, initialization, and tests, and as the top-level Context for incoming requests. + /// + /// ### Usage in frameworks and libraries + /// This function is really only intended to be used frameworks and libraries, at the "top-level" where a request's, + /// message's or task's processing is initiated. For example, a framework handling requests, should create an empty + /// context when handling a request only to immediately populate it with useful trace information extracted from e.g. + /// request headers. + /// + /// ### Usage in applications + /// Application code should never have to create an empty context during the processing lifetime of any request, + /// and only should create contexts if some processing is performed in the background - thus the naming of this property. + /// + /// Usually, a framework such as an HTTP server or similar "request handler" would already provide users + /// with a context to be passed along through subsequent calls. + /// + /// If unsure where to obtain a context from, prefer using `.TODO("Not sure where I should get a context from here?")`, + /// such that other developers are informed that the lack of context was not done on purpose, but rather because either + /// not being sure where to obtain a context from, or other framework limitations -- e.g. the outer framework not being + /// context aware just yet. + public static func background(logger: Logger) -> Context { + return .init(baggage: .background, logger: logger) + } + + /// A baggage context intended as a placeholder until a real value can be passed through a function call. + /// + /// It should ONLY be used while prototyping or when the passing of the proper context is not yet possible, + /// e.g. because an external library did not pass it correctly and has to be fixed before the proper context + /// can be obtained where the TO-DO is currently used. + /// + /// ### Crashing on TO-DO context creation + /// You may set the `BAGGAGE_CRASH_TODOS` variable while compiling a project in order to make calls to this function crash + /// with a fatal error, indicating where a to-do baggage context was used. This comes in handy when wanting to ensure that + /// a project never ends up using with code initially was written as "was lazy, did not pass context", yet the + /// project requires context passing to be done correctly throughout the application. Similar checks can be performed + /// at compile time easily using linters (not yet implemented), since it is always valid enough to detect a to-do context + /// being passed as illegal and warn or error when spotted. + /// + /// - Parameters: + /// - reason: Informational reason for developers, why a placeholder context was used instead of a proper one, + /// - Returns: Empty "to-do" baggage context which should be eventually replaced with a carried through one, or `background`. + public static func background(label loggerLabel: String) -> Context { + return .background(logger: Logger(label: loggerLabel)) + } +} + +extension Context { + /// A baggage context intended as a placeholder until a real value can be passed through a function call. + /// + /// It should ONLY be used while prototyping or when the passing of the proper context is not yet possible, + /// e.g. because an external library did not pass it correctly and has to be fixed before the proper context + /// can be obtained where the TO-DO is currently used. + /// + /// ## Crashing on TO-DO context creation + /// You may set the `BAGGAGE_CRASH_TODOS` variable while compiling a project in order to make calls to this function crash + /// with a fatal error, indicating where a to-do baggage context was used. This comes in handy when wanting to ensure that + /// a project never ends up using with code initially was written as "was lazy, did not pass context", yet the + /// project requires context passing to be done correctly throughout the application. Similar checks can be performed + /// at compile time easily using linters (not yet implemented), since it is always valid enough to detect a to-do context + /// being passed as illegal and warn or error when spotted. + /// + /// ## Example + /// + /// frameworkHandler { what in + /// hello(who: "World", baggage: .TODO(logger: logger, "The framework XYZ should be modified to pass us a context here, and we'd pass it along")) + /// } + /// + /// - Parameters: + /// - reason: Informational reason for developers, why a placeholder context was used instead of a proper one, + /// - Returns: Empty "to-do" baggage context which should be eventually replaced with a carried through one, or `background`. + public static func TODO(logger: Logger, _ reason: StaticString? = "", function: String = #function, file: String = #file, line: UInt = #line) -> Context { + let baggage = Baggage.TODO(reason, function: function, file: file, line: line) + #if BAGGAGE_CRASH_TODOS + fatalError("BAGGAGE_CRASH_TODOS: at \(file):\(line) (function \(function)), reason: \(reason)", file: file, line: line) + #else + return .init(baggage: baggage, logger: logger) + #endif + } + + /// A baggage context intended as a placeholder until a real value can be passed through a function call. + /// + /// It should ONLY be used while prototyping or when the passing of the proper context is not yet possible, + /// e.g. because an external library did not pass it correctly and has to be fixed before the proper context + /// can be obtained where the TO-DO is currently used. + /// + /// ## Crashing on TO-DO context creation + /// You may set the `BAGGAGE_CRASH_TODOS` variable while compiling a project in order to make calls to this function crash + /// with a fatal error, indicating where a to-do baggage context was used. This comes in handy when wanting to ensure that + /// a project never ends up using with code initially was written as "was lazy, did not pass context", yet the + /// project requires context passing to be done correctly throughout the application. Similar checks can be performed + /// at compile time easily using linters (not yet implemented), since it is always valid enough to detect a to-do context + /// being passed as illegal and warn or error when spotted. + /// + /// ## Example + /// + /// frameworkHandler { what in + /// hello(who: "World", baggage: .TODO(label: "MyLib", "The framework XYZ should be modified to pass us a context here, and we'd pass it along")) + /// } + /// + /// - Parameters: + /// - label: Label to be used to obtain a globally bootstrapped `Logger` with. + /// - reason: Informational reason for developers, why a placeholder context was used instead of a proper one, + /// - Returns: Empty "to-do" baggage context which should be eventually replaced with a carried through one, or `background`. + public static func TODO(label loggerLabel: String, _ reason: StaticString? = "", function: String = #function, file: String = #file, line: UInt = #line) -> Context { + return .TODO(logger: Logger(label: loggerLabel), reason, function: function, file: file, line: line) + } +} diff --git a/Sources/BaggageLogging/BaggageMetadataLogHandler.swift b/Sources/BaggageContext/ContextLogHandler.swift similarity index 80% rename from Sources/BaggageLogging/BaggageMetadataLogHandler.swift rename to Sources/BaggageContext/ContextLogHandler.swift index 8b48c50..77d8c2f 100644 --- a/Sources/BaggageLogging/BaggageMetadataLogHandler.swift +++ b/Sources/BaggageContext/ContextLogHandler.swift @@ -14,6 +14,23 @@ import Baggage import Logging +// ==== ---------------------------------------------------------------------------------------------------------------- +// MARK: Logger with Baggage + +extension Logger { + /// Returns a logger that in addition to any explicit metadata passed to log statements, + /// also includes the `BaggageContext` adapted into metadata values. + /// + /// The rendering of baggage values into metadata values is performed on demand, + /// whenever a log statement is effective (i.e. will be logged, according to active `logLevel`). + public func with(_ baggage: Baggage) -> Logger { + return Logger( + label: self.label, + factory: { _ in BaggageMetadataLogHandler(logger: self, baggage: baggage) } + ) + } +} + // ==== ---------------------------------------------------------------------------------------------------------------- // MARK: BaggageContext (as additional Logger.Metadata) LogHandler @@ -23,11 +40,11 @@ import Logging /// the `BaggageContext` values are preferred. public struct BaggageMetadataLogHandler: LogHandler { private var underlying: Logger - private let context: BaggageContext + private let baggage: Baggage - public init(logger underlying: Logger, context: BaggageContext) { + public init(logger underlying: Logger, baggage: Baggage) { self.underlying = underlying - self.context = context + self.baggage = baggage } public var logLevel: Logger.Level { @@ -86,7 +103,7 @@ public struct BaggageMetadataLogHandler: LogHandler { private func baggageAsMetadata() -> Logger.Metadata { var effectiveMetadata: Logger.Metadata = [:] - self.context.forEach { key, value in + self.baggage.forEachBaggageItem { key, value in if let convertible = value as? String { effectiveMetadata[key.name] = .string(convertible) } else if let convertible = value as? CustomStringConvertible { diff --git a/Sources/BaggageBenchmarkTools/ArgParser.swift b/Sources/BaggageContextBenchmarkTools/ArgParser.swift similarity index 100% rename from Sources/BaggageBenchmarkTools/ArgParser.swift rename to Sources/BaggageContextBenchmarkTools/ArgParser.swift diff --git a/Sources/BaggageBenchmarkTools/BenchmarkCategory.swift b/Sources/BaggageContextBenchmarkTools/BenchmarkCategory.swift similarity index 100% rename from Sources/BaggageBenchmarkTools/BenchmarkCategory.swift rename to Sources/BaggageContextBenchmarkTools/BenchmarkCategory.swift diff --git a/Sources/BaggageBenchmarkTools/BenchmarkTools.swift b/Sources/BaggageContextBenchmarkTools/BenchmarkTools.swift similarity index 100% rename from Sources/BaggageBenchmarkTools/BenchmarkTools.swift rename to Sources/BaggageContextBenchmarkTools/BenchmarkTools.swift diff --git a/Sources/BaggageBenchmarkTools/DriverUtils.swift b/Sources/BaggageContextBenchmarkTools/DriverUtils.swift similarity index 100% rename from Sources/BaggageBenchmarkTools/DriverUtils.swift rename to Sources/BaggageContextBenchmarkTools/DriverUtils.swift diff --git a/Sources/BaggageBenchmarkTools/README_SWIFT.md b/Sources/BaggageContextBenchmarkTools/README_SWIFT.md similarity index 100% rename from Sources/BaggageBenchmarkTools/README_SWIFT.md rename to Sources/BaggageContextBenchmarkTools/README_SWIFT.md diff --git a/Sources/BaggageBenchmarks/BaggagePassingBenchmarks.swift b/Sources/BaggageContextBenchmarks/BaggagePassingBenchmarks.swift similarity index 86% rename from Sources/BaggageBenchmarks/BaggagePassingBenchmarks.swift rename to Sources/BaggageContextBenchmarks/BaggagePassingBenchmarks.swift index ac0e041..2b55b11 100644 --- a/Sources/BaggageBenchmarks/BaggagePassingBenchmarks.swift +++ b/Sources/BaggageContextBenchmarks/BaggagePassingBenchmarks.swift @@ -12,16 +12,17 @@ //===----------------------------------------------------------------------===// import Baggage -import BaggageBenchmarkTools +import BaggageContextBenchmarkTools import Dispatch import class Foundation.NSLock + public let BaggagePassingBenchmarks: [BenchmarkInfo] = [ // ==== ---------------------------------------------------------------------------------------------------------------- // MARK: "Read only" context passing around BenchmarkInfo( name: "BaggagePassingBenchmarks.pass_async_empty_100_000 ", runFunction: { _ in - let context = BaggageContext.background + let context = Baggage.background pass_async(context: context, times: 100_000) }, tags: [], @@ -31,7 +32,7 @@ public let BaggagePassingBenchmarks: [BenchmarkInfo] = [ BenchmarkInfo( name: "BaggagePassingBenchmarks.pass_async_smol_100_000 ", runFunction: { _ in - var context = BaggageContext.background + var context = Baggage.background context.k1 = "one" context.k2 = "two" context.k3 = "three" @@ -45,7 +46,7 @@ public let BaggagePassingBenchmarks: [BenchmarkInfo] = [ BenchmarkInfo( name: "BaggagePassingBenchmarks.pass_async_small_nonconst_100_000", runFunction: { _ in - var context = BaggageContext.background + var context = Baggage.background context.k1 = "\(Int.random(in: 1 ... Int.max))" context.k2 = "\(Int.random(in: 1 ... Int.max))" context.k3 = "\(Int.random(in: 1 ... Int.max))" @@ -65,7 +66,7 @@ public let BaggagePassingBenchmarks: [BenchmarkInfo] = [ BenchmarkInfo( name: "BaggagePassingBenchmarks.pass_mut_async_small_100_000 ", runFunction: { _ in - var context = BaggageContext.background + var context = Baggage.background context.k1 = "\(Int.random(in: 1 ... Int.max))" context.k2 = "\(Int.random(in: 1 ... Int.max))" context.k3 = "\(Int.random(in: 1 ... Int.max))" @@ -87,10 +88,10 @@ private func tearDown() { } @inline(never) -func pass_async(context: BaggageContext, times remaining: Int) { +func pass_async(context: Baggage, times remaining: Int) { let latch = CountDownLatch(from: 1) - func pass_async0(context: BaggageContext, times remaining: Int) { + func pass_async0(context: Baggage, times remaining: Int) { if remaining == 0 { latch.countDown() } @@ -105,11 +106,11 @@ func pass_async(context: BaggageContext, times remaining: Int) { } @inline(never) -func pass_mut_async(context: BaggageContext, times remaining: Int) { +func pass_mut_async(context: Baggage, times remaining: Int) { var context = context let latch = CountDownLatch(from: 1) - func pass_async0(context: BaggageContext, times remaining: Int) { + func pass_async0(context: Baggage, times remaining: Int) { if remaining == 0 { latch.countDown() } @@ -132,31 +133,31 @@ func pass_mut_async(context: BaggageContext, times remaining: Int) { // ==== ---------------------------------------------------------------------------------------------------------------- // MARK: Baggage Keys -private enum TestPassCounterKey: BaggageContextKey { +private enum TestPassCounterKey: BaggageKey { typealias Value = Int } -private enum TestK1: BaggageContextKey { +private enum TestK1: BaggageKey { typealias Value = String } -private enum TestK2: BaggageContextKey { +private enum TestK2: BaggageKey { typealias Value = String } -private enum TestK3: BaggageContextKey { +private enum TestK3: BaggageKey { typealias Value = String } -private enum TestK4: BaggageContextKey { +private enum TestK4: BaggageKey { typealias Value = String } -private enum TestKD1: BaggageContextKey { +private enum TestKD1: BaggageKey { typealias Value = [String: String] } -extension BaggageContext { +extension Baggage { fileprivate var passCounter: TestPassCounterKey.Value { get { return self[TestPassCounterKey.self] ?? 0 } set { self[TestPassCounterKey.self] = newValue } diff --git a/Sources/BaggageBenchmarks/locks.swift b/Sources/BaggageContextBenchmarks/locks.swift similarity index 100% rename from Sources/BaggageBenchmarks/locks.swift rename to Sources/BaggageContextBenchmarks/locks.swift diff --git a/Sources/BaggageBenchmarks/main.swift b/Sources/BaggageContextBenchmarks/main.swift similarity index 97% rename from Sources/BaggageBenchmarks/main.swift rename to Sources/BaggageContextBenchmarks/main.swift index 6715d74..8a9ce44 100644 --- a/Sources/BaggageBenchmarks/main.swift +++ b/Sources/BaggageContextBenchmarks/main.swift @@ -11,7 +11,7 @@ // //===----------------------------------------------------------------------===// -import BaggageBenchmarkTools +import BaggageContextBenchmarkTools assert({ print("===========================================================================") diff --git a/Sources/BaggageLogging/Logger+BaggageContext.swift b/Sources/BaggageLogging/Logger+BaggageContext.swift deleted file mode 100644 index c778ccf..0000000 --- a/Sources/BaggageLogging/Logger+BaggageContext.swift +++ /dev/null @@ -1,29 +0,0 @@ -//===----------------------------------------------------------------------===// -// -// This source file is part of the Swift Baggage Context open source project -// -// Copyright (c) 2020 Moritz Lang and the Swift Baggage Context project authors -// Licensed under Apache License v2.0 -// -// See LICENSE.txt for license information -// -// SPDX-License-Identifier: Apache-2.0 -// -//===----------------------------------------------------------------------===// - -import Baggage -import Logging - -extension Logger { - /// Returns a logger that in addition to any explicit metadata passed to log statements, - /// also includes the `BaggageContext` adapted into metadata values. - /// - /// The rendering of baggage values into metadata values is performed on demand, - /// whenever a log statement is effective (i.e. will be logged, according to active `logLevel`). - public func with(context: BaggageContext) -> Logger { - return Logger( - label: self.label, - factory: { _ in BaggageMetadataLogHandler(logger: self, context: context) } - ) - } -} diff --git a/Sources/BaggageLogging/LoggingBaggageContextCarrier.swift b/Sources/BaggageLogging/LoggingBaggageContextCarrier.swift deleted file mode 100644 index 98d873f..0000000 --- a/Sources/BaggageLogging/LoggingBaggageContextCarrier.swift +++ /dev/null @@ -1,44 +0,0 @@ -//===----------------------------------------------------------------------===// -// -// This source file is part of the Swift Baggage Context open source project -// -// Copyright (c) 2020 Moritz Lang and the Swift Baggage Context project authors -// Licensed under Apache License v2.0 -// -// See LICENSE.txt for license information -// -// SPDX-License-Identifier: Apache-2.0 -// -//===----------------------------------------------------------------------===// - -import Baggage -import Logging - -/// A `LoggingBaggageContextCarrier` purpose is to be adopted by frameworks which already provide a "FrameworkContext", -/// and to such frameworks to pass their context as `BaggageContextCarrier`. -public protocol LoggingBaggageContextCarrier: BaggageContextCarrier { - /// The logger associated with this context carrier. - /// - /// It should automatically populate the loggers metadata based on the `BaggageContext` associated with this context object. - /// - /// ### Implementation note - /// - /// Libraries and/or frameworks which conform to this protocol with their "Framework Context" types, - /// SHOULD implement this logger by wrapping the "raw" logger associated with this context with the `logger.with(BaggageContext:)` function, - /// which efficiently handles the bridging of baggage to logging metadata values. - /// - /// ### Example implementation - /// - /// Writes to the `logger` metadata SHOULD NOT be reflected in the `baggage`, - /// however writes to the underlying `baggage` SHOULD be reflected in the `logger`. - /// - /// struct MyFrameworkContext: LoggingBaggageContextCarrier { - /// var baggage = BaggageContext() - /// private let _logger: Logger - /// - /// var logger: Logger { - /// return self._logger.with(context: self.baggage) - /// } - /// } - var logger: Logger { get } -} diff --git a/Tests/BaggageLoggingTests/LoggingBaggageContextCarrierTests+XCTest.swift b/Tests/BaggageContextTests/BaggageContextTests+XCTest.swift similarity index 54% rename from Tests/BaggageLoggingTests/LoggingBaggageContextCarrierTests+XCTest.swift rename to Tests/BaggageContextTests/BaggageContextTests+XCTest.swift index 2c9d7fa..99ef608 100644 --- a/Tests/BaggageLoggingTests/LoggingBaggageContextCarrierTests+XCTest.swift +++ b/Tests/BaggageContextTests/BaggageContextTests+XCTest.swift @@ -11,7 +11,7 @@ // //===----------------------------------------------------------------------===// // -// LoggingBaggageContextCarrierTests+XCTest.swift +// BaggageContextTests+XCTest.swift // import XCTest /// @@ -20,14 +20,15 @@ import XCTest /// Do NOT edit this file directly as it will be regenerated automatically when needed. /// -extension LoggingBaggageContextCarrierTests { +extension BaggageContextTests { @available(*, deprecated, message: "not actually deprecated. Just deprecated to allow deprecated tests (which test deprecated functionality) without warnings") - static var allTests : [(String, (LoggingBaggageContextCarrierTests) -> () throws -> Void)] { + static var allTests : [(String, (BaggageContextTests) -> () throws -> Void)] { return [ - ("test_ContextWithLogger_dumpBaggage", test_ContextWithLogger_dumpBaggage), - ("test_ContextWithLogger_log_withBaggage", test_ContextWithLogger_log_withBaggage), - ("test_ContextWithLogger_log_prefersBaggageContextOverExistingLoggerMetadata", test_ContextWithLogger_log_prefersBaggageContextOverExistingLoggerMetadata), + ("test_ExampleFrameworkContext_dumpBaggage", test_ExampleFrameworkContext_dumpBaggage), + ("test_ExampleMutableFrameworkContext_dumpBaggage", test_ExampleMutableFrameworkContext_dumpBaggage), + ("test_ExampleMutableFrameworkContext_log_withBaggage", test_ExampleMutableFrameworkContext_log_withBaggage), + ("test_ExampleMutableFrameworkContext_log_prefersBaggageContextOverExistingLoggerMetadata", test_ExampleMutableFrameworkContext_log_prefersBaggageContextOverExistingLoggerMetadata), ] } } diff --git a/Tests/BaggageContextTests/BaggageContextTests.swift b/Tests/BaggageContextTests/BaggageContextTests.swift new file mode 100644 index 0000000..c862b87 --- /dev/null +++ b/Tests/BaggageContextTests/BaggageContextTests.swift @@ -0,0 +1,213 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift Baggage Context open source project +// +// Copyright (c) 2020 Moritz Lang and the Swift Baggage Context project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +import Baggage +import BaggageContext +import Logging +import XCTest + +final class BaggageContextTests: XCTestCase { + func test_ExampleFrameworkContext_dumpBaggage() throws { + var baggage = Baggage.background + let logger = Logger(label: "TheLogger") + + baggage.testID = 42 + let context = ExampleFrameworkContext(context: baggage, logger: logger) + + func frameworkFunctionDumpsBaggage(param: String, context: ContextProtocol) -> String { + var s = "" + context.baggage.forEachBaggageItem { key, item in + s += "\(key.name): \(item)\n" + } + return s + } + + let result = frameworkFunctionDumpsBaggage(param: "x", context: context) + XCTAssertEqual( + result, + """ + TestIDKey: 42 + + """ + ) + } + + func test_ExampleMutableFrameworkContext_dumpBaggage() throws { + let baggage = Baggage.background + let logger = Logger(label: "TheLogger") + + var context: ContextProtocol & BaggageProtocol = ExampleMutableFrameworkContext(context: baggage, logger: logger) + context.testID = 42 + + func frameworkFunctionDumpsBaggage(param: String, context: ContextProtocol & BaggageProtocol) -> String { + var s = "" + context.forEachBaggageItem { key, item in + s += "\(key.name): \(item)\n" + } + return s + } + + let result = frameworkFunctionDumpsBaggage(param: "x", context: context) + XCTAssertEqual( + result, + """ + TestIDKey: 42 + + """ + ) + } + + func test_ExampleMutableFrameworkContext_log_withBaggage() throws { + let baggage = Baggage.background + let logging = TestLogging() + let logger = Logger(label: "TheLogger", factory: { label in logging.make(label: label) }) + + var context = ExampleMutableFrameworkContext(context: baggage, logger: logger) + + context.secondTestID = "value" + context.testID = 42 + context.logger.info("Hello") + + context.testID = nil + context.logger.warning("World") + + logging.history.assertExist(level: .info, message: "Hello", metadata: [ + "TestIDKey": .stringConvertible(42), + "secondIDExplicitlyNamed": "value", + ]) + logging.history.assertExist(level: .warning, message: "World", metadata: [ + "secondIDExplicitlyNamed": "value", + ]) + } + + func test_ExampleMutableFrameworkContext_log_prefersBaggageContextOverExistingLoggerMetadata() { + let baggage = Baggage.background + let logging = TestLogging() + var logger = Logger(label: "TheLogger", factory: { label in logging.make(label: label) }) + logger[metadataKey: "secondIDExplicitlyNamed"] = "set on logger" + + var context = ExampleMutableFrameworkContext(context: baggage, logger: logger) + + context.secondTestID = "set on baggage" + + context.logger.info("Hello") + + logging.history.assertExist(level: .info, message: "Hello", metadata: [ + "secondIDExplicitlyNamed": "set on baggage", + ]) + } +} + +struct ExampleFrameworkContext: BaggageContext.ContextProtocol { + let baggage: Baggage + let logger: Logger + + init(context baggage: Baggage, logger: Logger) { + self.baggage = baggage + self.logger = logger.with(self.baggage) + } + + var asBaggageContext: Context { + return .init(baggage: self.baggage, logger: self.logger) + } +} + +struct ExampleMutableFrameworkContext: ContextProtocol, BaggageProtocol { + var baggage: Baggage + + private var _logger: Logger + var logger: Logger { + return self._logger.with(self.baggage) + } + + init(context baggage: Baggage, logger: Logger) { + self.baggage = baggage + self._logger = logger + } + + subscript(key: Key.Type) -> Key.Value? { + get { + return self.baggage[key] + } + set { + self.baggage[key] = newValue + } + } + + func forEachBaggageItem(_ body: (AnyBaggageKey, Any) throws -> Void) rethrows { + return try self.baggage.forEachBaggageItem(body) + } + + var asBaggageContext: Context { + return .init(baggage: self.baggage, logger: self._logger) + } +} + +struct CoolFrameworkContext: BaggageContext.ContextProtocol { + private var _logger: Logger = Logger(label: "some frameworks logger") + var logger: Logger { + return self._logger.with(self.baggage) + } + + var baggage: Baggage = .background + + // framework context defines other values as well + let frameworkField: String = "" + + // including the popular eventLoop + let eventLoop: FakeEventLoop + + subscript(key: Key.Type) -> Key.Value? { + return self.baggage[key] + } + + func forEachBaggageItem(_ body: (AnyBaggageKey, Any) throws -> Void) rethrows { + return try self.baggage.forEachBaggageItem(body) + } + + var asBaggageContext: Context { + return .init(baggage: self.baggage, logger: self._logger) + } +} + +struct FakeEventLoop {} + +private extension BaggageProtocol { + var testID: Int? { + get { + return self[TestIDKey.self] + } + set { + self[TestIDKey.self] = newValue + } + } + + var secondTestID: String? { + get { + return self[SecondTestIDKey.self] + } + set { + self[SecondTestIDKey.self] = newValue + } + } +} + +private enum TestIDKey: Baggage.Key { + typealias Value = Int +} + +private enum SecondTestIDKey: Baggage.Key { + typealias Value = String + + static let name: String? = "secondIDExplicitlyNamed" +} diff --git a/Tests/BaggageLoggingTests/TestLogger.swift b/Tests/BaggageContextTests/TestLogger.swift similarity index 100% rename from Tests/BaggageLoggingTests/TestLogger.swift rename to Tests/BaggageContextTests/TestLogger.swift diff --git a/Tests/BaggageLoggingTests/LoggingBaggageContextCarrierTests.swift b/Tests/BaggageLoggingTests/LoggingBaggageContextCarrierTests.swift deleted file mode 100644 index 49c79a4..0000000 --- a/Tests/BaggageLoggingTests/LoggingBaggageContextCarrierTests.swift +++ /dev/null @@ -1,145 +0,0 @@ -//===----------------------------------------------------------------------===// -// -// This source file is part of the Swift Baggage Context open source project -// -// Copyright (c) 2020 Moritz Lang and the Swift Baggage Context project authors -// Licensed under Apache License v2.0 -// -// See LICENSE.txt for license information -// -// SPDX-License-Identifier: Apache-2.0 -// -//===----------------------------------------------------------------------===// - -import Baggage -import BaggageLogging -import Logging -import XCTest - -final class LoggingBaggageContextCarrierTests: XCTestCase { - func test_ContextWithLogger_dumpBaggage() throws { - let baggage = BaggageContext.background - let logger = Logger(label: "TheLogger") - - var context: LoggingBaggageContextCarrier = ExampleFrameworkContext(context: baggage, logger: logger) - context.testID = 42 - - func frameworkFunctionDumpsBaggage(param: String, context: LoggingBaggageContextCarrier) -> String { - var s = "" - context.baggage.forEach { key, item in - s += "\(key.name): \(item)\n" - } - return s - } - - let result = frameworkFunctionDumpsBaggage(param: "x", context: context) - XCTAssertEqual( - result, - """ - TestIDKey: 42 - - """ - ) - } - - func test_ContextWithLogger_log_withBaggage() throws { - let baggage = BaggageContext.background - let logging = TestLogging() - let logger = Logger(label: "TheLogger", factory: { label in logging.make(label: label) }) - - var context: LoggingBaggageContextCarrier = ExampleFrameworkContext(context: baggage, logger: logger) - - context.secondTestID = "value" - context.testID = 42 - context.logger.info("Hello") - - context.testID = nil - context.logger.warning("World") - - logging.history.assertExist(level: .info, message: "Hello", metadata: [ - "TestIDKey": .stringConvertible(42), - "secondIDExplicitlyNamed": "value", - ]) - logging.history.assertExist(level: .warning, message: "World", metadata: [ - "secondIDExplicitlyNamed": "value", - ]) - } - - func test_ContextWithLogger_log_prefersBaggageContextOverExistingLoggerMetadata() { - let baggage = BaggageContext.background - let logging = TestLogging() - var logger = Logger(label: "TheLogger", factory: { label in logging.make(label: label) }) - logger[metadataKey: "secondIDExplicitlyNamed"] = "set on logger" - - var context: LoggingBaggageContextCarrier = ExampleFrameworkContext(context: baggage, logger: logger) - - context.secondTestID = "set on baggage" - - context.logger.info("Hello") - - logging.history.assertExist(level: .info, message: "Hello", metadata: [ - "secondIDExplicitlyNamed": "set on baggage", - ]) - } -} - -struct ExampleFrameworkContext: LoggingBaggageContextCarrier { - var baggage: BaggageContext - - private var _logger: Logger - var logger: Logger { - return self._logger.with(context: self.baggage) - } - - init(context baggage: BaggageContext, logger: Logger) { - self.baggage = baggage - self._logger = logger - } -} - -struct CoolFrameworkContext: LoggingBaggageContextCarrier { - private var _logger: Logger = Logger(label: "some frameworks logger") - var logger: Logger { - return self._logger.with(context: self.baggage) - } - - var baggage: BaggageContext = .background - - // framework context defines other values as well - let frameworkField: String = "" - - // including the popular eventLoop - let eventLoop: FakeEventLoop -} - -struct FakeEventLoop {} - -private extension BaggageContextProtocol { - var testID: Int? { - get { - return self[TestIDKey.self] - } - set { - self[TestIDKey.self] = newValue - } - } - - var secondTestID: String? { - get { - return self[SecondTestIDKey.self] - } - set { - self[SecondTestIDKey.self] = newValue - } - } -} - -private enum TestIDKey: BaggageContextKey { - typealias Value = Int -} - -private enum SecondTestIDKey: BaggageContextKey { - typealias Value = String - - static let name: String? = "secondIDExplicitlyNamed" -} diff --git a/Tests/BaggageTests/BaggageContextTests+XCTest.swift b/Tests/BaggageTests/ContextTests+XCTest.swift similarity index 90% rename from Tests/BaggageTests/BaggageContextTests+XCTest.swift rename to Tests/BaggageTests/ContextTests+XCTest.swift index 62a9861..5dff63f 100644 --- a/Tests/BaggageTests/BaggageContextTests+XCTest.swift +++ b/Tests/BaggageTests/ContextTests+XCTest.swift @@ -11,7 +11,7 @@ // //===----------------------------------------------------------------------===// // -// BaggageContextTests+XCTest.swift +// ContextTests+XCTest.swift // import XCTest /// @@ -20,10 +20,10 @@ import XCTest /// Do NOT edit this file directly as it will be regenerated automatically when needed. /// -extension BaggageContextTests { +extension ContextTests { @available(*, deprecated, message: "not actually deprecated. Just deprecated to allow deprecated tests (which test deprecated functionality) without warnings") - static var allTests : [(String, (BaggageContextTests) -> () throws -> Void)] { + static var allTests : [(String, (ContextTests) -> () throws -> Void)] { return [ ("testSubscriptAccess", testSubscriptAccess), ("testRecommendedConvenienceExtension", testRecommendedConvenienceExtension), diff --git a/Tests/BaggageTests/BaggageContextTests.swift b/Tests/BaggageTests/ContextTests.swift similarity index 75% rename from Tests/BaggageTests/BaggageContextTests.swift rename to Tests/BaggageTests/ContextTests.swift index dd2f48f..b058cc8 100644 --- a/Tests/BaggageTests/BaggageContextTests.swift +++ b/Tests/BaggageTests/ContextTests.swift @@ -14,11 +14,11 @@ import Baggage import XCTest -final class BaggageContextTests: XCTestCase { +final class ContextTests: XCTestCase { func testSubscriptAccess() { let testID = 42 - var baggage = BaggageContext.background + var baggage = Baggage.background XCTAssertNil(baggage[TestIDKey.self]) baggage[TestIDKey.self] = testID @@ -31,7 +31,7 @@ final class BaggageContextTests: XCTestCase { func testRecommendedConvenienceExtension() { let testID = 42 - var baggage = BaggageContext.background + var baggage = Baggage.background XCTAssertNil(baggage.testID) baggage.testID = testID @@ -42,26 +42,26 @@ final class BaggageContextTests: XCTestCase { } func testEmptyBaggageDescription() { - XCTAssertEqual(String(describing: BaggageContext.background), "BaggageContext(keys: [])") + XCTAssertEqual(String(describing: Baggage.background), "Baggage(keys: [])") } func testSingleKeyBaggageDescription() { - var baggage = BaggageContext.background + var baggage = Baggage.background baggage.testID = 42 - XCTAssertEqual(String(describing: baggage), #"BaggageContext(keys: ["TestIDKey"])"#) + XCTAssertEqual(String(describing: baggage), #"Baggage(keys: ["TestIDKey"])"#) } func testMultiKeysBaggageDescription() { - var baggage = BaggageContext.background + var baggage = Baggage.background baggage.testID = 42 baggage[SecondTestIDKey.self] = "test" let description = String(describing: baggage) - XCTAssert(description.starts(with: "BaggageContext(keys: [")) + XCTAssert(description.starts(with: "Baggage(keys: ["), "Was: \(description)") // use contains instead of `XCTAssertEqual` because the order is non-predictable (Dictionary) - XCTAssert(description.contains("TestIDKey")) - XCTAssert(description.contains("ExplicitKeyName")) + XCTAssert(description.contains("TestIDKey"), "Was: \(description)") + XCTAssert(description.contains("ExplicitKeyName"), "Was: \(description)") } // ==== ------------------------------------------------------------------------------------------------------------ @@ -69,7 +69,7 @@ final class BaggageContextTests: XCTestCase { func test_todo_context() { // the to-do context can be used to record intentions for why a context could not be passed through - let context = BaggageContext.TODO("#1245 Some other library should be adjusted to pass us context") + let context = Baggage.TODO("#1245 Some other library should be adjusted to pass us context") _ = context // avoid "not used" warning // TODO: Can't work with protocols; re-consider the entire carrier approach... Context being a Baggage + Logger, and a specific type. @@ -80,7 +80,7 @@ final class BaggageContextTests: XCTestCase { } func test_todo_empty() { - let context = BaggageContext.background + let context = Baggage.background _ = context // avoid "not used" warning // TODO: Can't work with protocols; re-consider the entire carrier approach... Context being a Baggage + Logger, and a specific type. @@ -92,21 +92,22 @@ final class BaggageContextTests: XCTestCase { } } -private enum TestIDKey: BaggageContextKey { +private enum TestIDKey: Baggage.Key { typealias Value = Int } -private extension BaggageContext { +private extension Baggage { var testID: Int? { get { return self[TestIDKey.self] - } set { + } + set { self[TestIDKey.self] = newValue } } } -private enum SecondTestIDKey: BaggageContextKey { +private enum SecondTestIDKey: Baggage.Key { typealias Value = String static let name: String? = "ExplicitKeyName" diff --git a/Tests/BaggageTests/BaggageContextCarrierTests+XCTest.swift b/Tests/BaggageTests/FrameworkContextTests+XCTest.swift similarity index 79% rename from Tests/BaggageTests/BaggageContextCarrierTests+XCTest.swift rename to Tests/BaggageTests/FrameworkContextTests+XCTest.swift index 886f7de..e1fdbf2 100644 --- a/Tests/BaggageTests/BaggageContextCarrierTests+XCTest.swift +++ b/Tests/BaggageTests/FrameworkContextTests+XCTest.swift @@ -11,7 +11,7 @@ // //===----------------------------------------------------------------------===// // -// BaggageContextCarrierTests+XCTest.swift +// FrameworkContextTests+XCTest.swift // import XCTest /// @@ -20,14 +20,13 @@ import XCTest /// Do NOT edit this file directly as it will be regenerated automatically when needed. /// -extension BaggageContextCarrierTests { +extension FrameworkBaggageContextTests { @available(*, deprecated, message: "not actually deprecated. Just deprecated to allow deprecated tests (which test deprecated functionality) without warnings") - static var allTests : [(String, (BaggageContextCarrierTests) -> () throws -> Void)] { + static var allTests : [(String, (FrameworkBaggageContextTests) -> () throws -> Void)] { return [ ("testBaggageContextSubscript", testBaggageContextSubscript), ("testBaggageContextForEach", testBaggageContextForEach), - ("testBaggageContextCarriesItself", testBaggageContextCarriesItself), ] } } diff --git a/Tests/BaggageTests/BaggageContextCarrierTests.swift b/Tests/BaggageTests/FrameworkContextTests.swift similarity index 57% rename from Tests/BaggageTests/BaggageContextCarrierTests.swift rename to Tests/BaggageTests/FrameworkContextTests.swift index 77747ff..1d4c090 100644 --- a/Tests/BaggageTests/BaggageContextCarrierTests.swift +++ b/Tests/BaggageTests/FrameworkContextTests.swift @@ -14,7 +14,7 @@ @testable import Baggage import XCTest -final class BaggageContextCarrierTests: XCTestCase { +final class FrameworkBaggageContextTests: XCTestCase { func testBaggageContextSubscript() { var carrier = TestFrameworkContext() @@ -30,38 +30,44 @@ final class BaggageContextCarrierTests: XCTestCase { } func testBaggageContextForEach() { - var contents = [AnyBaggageContextKey: Any]() + var contents = [AnyBaggageKey: Any]() var carrier = TestFrameworkContext() carrier[TestKey.self] = 42 carrier[OtherKey.self] = "test" - carrier.forEach { key, value in + carrier.forEachBaggageItem { key, value in contents[key] = value } - XCTAssertNotNil(contents[AnyBaggageContextKey(TestKey.self)]) - XCTAssertEqual(contents[AnyBaggageContextKey(TestKey.self)] as? Int, 42) - XCTAssertNotNil(contents[AnyBaggageContextKey(OtherKey.self)]) - XCTAssertEqual(contents[AnyBaggageContextKey(OtherKey.self)] as? String, "test") + XCTAssertNotNil(contents[AnyBaggageKey(TestKey.self)]) + XCTAssertEqual(contents[AnyBaggageKey(TestKey.self)] as? Int, 42) + XCTAssertNotNil(contents[AnyBaggageKey(OtherKey.self)]) + XCTAssertEqual(contents[AnyBaggageKey(OtherKey.self)] as? String, "test") } +} - func testBaggageContextCarriesItself() { - var context: BaggageContextCarrier = BaggageContext() +private struct TestFrameworkContext: BaggageProtocol { + var baggage = Baggage() - context.baggage[TestKey.self] = 42 - XCTAssertEqual(context.baggage[TestKey.self], 42) + subscript(_ key: Key.Type) -> Key.Value? { + get { + return self.baggage[key] + } + set { + self.baggage[key] = newValue + } } -} -private struct TestFrameworkContext: BaggageContextCarrier { - var baggage = BaggageContext() + func forEachBaggageItem(_ body: (AnyBaggageKey, Any) throws -> Void) rethrows { + return try self.baggage.forEachBaggageItem(body) + } } -private enum TestKey: BaggageContextKey { +private enum TestKey: Baggage.Key { typealias Value = Int } -private enum OtherKey: BaggageContextKey { +private enum OtherKey: Baggage.Key { typealias Value = String } diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index 92c2ff7..e7a87cc 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -21,7 +21,7 @@ import XCTest /// #if os(Linux) || os(FreeBSD) - @testable import BaggageLoggingTests + @testable import BaggageContextTests @testable import BaggageTests // This protocol is necessary to we can call the 'run' method (on an existential of this protocol) @@ -33,9 +33,9 @@ class LinuxMainRunnerImpl: LinuxMainRunner { @available(*, deprecated, message: "not actually deprecated. Just deprecated to allow deprecated tests (which test deprecated functionality) without warnings") func run() { XCTMain([ - testCase(BaggageContextCarrierTests.allTests), testCase(BaggageContextTests.allTests), - testCase(LoggingBaggageContextCarrierTests.allTests), + testCase(ContextTests.allTests), + testCase(FrameworkBaggageContextTests.allTests), ]) } } From fe6c4c49abf00a48648460d05f4a0bbc1778c5f9 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 15 Sep 2020 10:40:34 +0900 Subject: [PATCH 02/14] Context is the protocol --- Sources/BaggageContext/Context.swift | 125 +++++++++--------- .../BaggageContextTests.swift | 18 +-- 2 files changed, 72 insertions(+), 71 deletions(-) diff --git a/Sources/BaggageContext/Context.swift b/Sources/BaggageContext/Context.swift index 2e46d00..1cfed25 100644 --- a/Sources/BaggageContext/Context.swift +++ b/Sources/BaggageContext/Context.swift @@ -14,56 +14,6 @@ import Baggage import Logging -/// A default `Context` type. -/// -/// It is a carrier of contextual `Baggage` and related `Logger`, allowing to log and trace throughout a system. -/// -/// Any values set on the `baggage` will be made accessible to the logger as call-site metadata, allowing it to log those. -/// -/// ### Logged Metadata and Baggage Items -/// -/// Please refer to your configured log handler documentation about how to configure which metadata values should be logged -/// and which not, as each log handler may handle and configure those differently. The default implementations log *all* -/// metadata/baggage values present, which often is the right thing, however in larger systems one may want to choose a -/// log handler which allows for configuring these details. -/// -/// ### Accepting context types in APIs -/// -/// It is preferred to accept values of `ContextProtocol` in library APIs, as this yields a more flexible API shape, -/// to which other libraries/frameworks may pass their specific context objects. -/// -/// - SeeAlso: `Baggage` from the Baggage module. -/// - SeeAlso: `Logger` from the SwiftLog package. -public struct Context: ContextProtocol { - /// The `Baggage` carried with this context. - /// It's values will automatically be made available to the `logger` as metadata when logging. - /// - /// Baggage values are different from plain logging metadata in that they are intended to be - /// carried across process and node boundaries (serialized and deserialized) and are made - /// available to instruments using `swift-distributed-tracing`. - public var baggage: Baggage - - public var logger: Logger { - get { - return self._logger.with(self.baggage) - } - set { - self._logger = newValue - } - } - - private var _logger: Logger - - public init(baggage: Baggage, logger underlying: Logger) { - self.baggage = baggage - self._logger = underlying - } - - public var asBaggageContext: Context { - return self - } -} - // ==== ---------------------------------------------------------------------------------------------------------------- // MARK: Context Protocol @@ -88,7 +38,7 @@ public struct Context: ContextProtocol { /// /// Please note that if a context does conform to `BaggageProtocol` as well, it will most-likely also offer all properties that /// users have provided via extensions on it - so be watchful of potential naming conflicts on variable names. -public protocol ContextProtocol { +public protocol Context { /// Get the `Baggage` container. var baggage: Baggage { get } @@ -118,7 +68,58 @@ public protocol ContextProtocol { var logger: Logger { get } /// Get a baggage `Context` struct, regardless of the representation that this context protocol is implemented by. - var asBaggageContext: Context { get } + var asBaggageContext: DefaultContext { get } +} + + +/// A default `Context` type. +/// +/// It is a carrier of contextual `Baggage` and related `Logger`, allowing to log and trace throughout a system. +/// +/// Any values set on the `baggage` will be made accessible to the logger as call-site metadata, allowing it to log those. +/// +/// ### Logged Metadata and Baggage Items +/// +/// Please refer to your configured log handler documentation about how to configure which metadata values should be logged +/// and which not, as each log handler may handle and configure those differently. The default implementations log *all* +/// metadata/baggage values present, which often is the right thing, however in larger systems one may want to choose a +/// log handler which allows for configuring these details. +/// +/// ### Accepting context types in APIs +/// +/// It is preferred to accept values of `ContextProtocol` in library APIs, as this yields a more flexible API shape, +/// to which other libraries/frameworks may pass their specific context objects. +/// +/// - SeeAlso: `Baggage` from the Baggage module. +/// - SeeAlso: `Logger` from the SwiftLog package. +public struct DefaultContext: Context { + /// The `Baggage` carried with this context. + /// It's values will automatically be made available to the `logger` as metadata when logging. + /// + /// Baggage values are different from plain logging metadata in that they are intended to be + /// carried across process and node boundaries (serialized and deserialized) and are made + /// available to instruments using `swift-distributed-tracing`. + public var baggage: Baggage + + public var logger: Logger { + get { + return self._logger.with(self.baggage) + } + set { + self._logger = newValue + } + } + + private var _logger: Logger + + public init(baggage: Baggage, logger underlying: Logger) { + self.baggage = baggage + self._logger = underlying + } + + public var asBaggageContext: DefaultContext { + return self + } } // ==== ---------------------------------------------------------------------------------------------------------------- @@ -129,7 +130,7 @@ extension Context { /// /// - Parameter logger: Logger that should replace the underlying logger of this context. /// - Returns: new context, with the passed in `logger` - public func withLogger(_ logger: Logger) -> Context { + public func withLogger(_ logger: Logger) -> DefaultContext { var copy = self copy.logger = logger return copy @@ -139,7 +140,7 @@ extension Context { /// /// - Parameter logLevel: New log level which should be used to create the new context /// - Returns: new context, with the passed in `logLevel` used for the underlying logger - public func withLogLevel(_ logLevel: Logger.Level) -> Context { + public func withLogLevel(_ logLevel: Logger.Level) -> DefaultContext { var copy = self copy.logger.logLevel = logLevel return copy @@ -153,7 +154,7 @@ extension Context { /// }) /// /// - Parameter function: - public func withBaggage(_ function: (inout Baggage) -> Void) -> Context { + public func withBaggage(_ function: (inout Baggage) -> Void) -> DefaultContext { var baggage = self.baggage function(&baggage) return self.withBaggage(baggage) @@ -165,7 +166,7 @@ extension Context { /// /// - Parameter baggage: baggage that should *replace* the context's current baggage. /// - Returns: new context, with the passed in baggage - public func withBaggage(_ baggage: Baggage) -> Context { + public func withBaggage(_ baggage: Baggage) -> DefaultContext { var copy = self copy.baggage = baggage return copy @@ -175,7 +176,7 @@ extension Context { // ==== ---------------------------------------------------------------------------------------------------------------- // MARK: Context Initializers -extension Context { +extension DefaultContext { /// An empty baggage context intended as the "root" or "initial" baggage context background processing tasks, or as the "root" baggage context. /// /// It is never canceled, has no values, and has no deadline. @@ -198,7 +199,7 @@ extension Context { /// such that other developers are informed that the lack of context was not done on purpose, but rather because either /// not being sure where to obtain a context from, or other framework limitations -- e.g. the outer framework not being /// context aware just yet. - public static func background(logger: Logger) -> Context { + public static func background(logger: Logger) -> DefaultContext { return .init(baggage: .background, logger: logger) } @@ -219,12 +220,12 @@ extension Context { /// - Parameters: /// - reason: Informational reason for developers, why a placeholder context was used instead of a proper one, /// - Returns: Empty "to-do" baggage context which should be eventually replaced with a carried through one, or `background`. - public static func background(label loggerLabel: String) -> Context { + public static func background(label loggerLabel: String) -> DefaultContext { return .background(logger: Logger(label: loggerLabel)) } } -extension Context { +extension DefaultContext { /// A baggage context intended as a placeholder until a real value can be passed through a function call. /// /// It should ONLY be used while prototyping or when the passing of the proper context is not yet possible, @@ -248,7 +249,7 @@ extension Context { /// - Parameters: /// - reason: Informational reason for developers, why a placeholder context was used instead of a proper one, /// - Returns: Empty "to-do" baggage context which should be eventually replaced with a carried through one, or `background`. - public static func TODO(logger: Logger, _ reason: StaticString? = "", function: String = #function, file: String = #file, line: UInt = #line) -> Context { + public static func TODO(logger: Logger, _ reason: StaticString? = "", function: String = #function, file: String = #file, line: UInt = #line) -> DefaultContext { let baggage = Baggage.TODO(reason, function: function, file: file, line: line) #if BAGGAGE_CRASH_TODOS fatalError("BAGGAGE_CRASH_TODOS: at \(file):\(line) (function \(function)), reason: \(reason)", file: file, line: line) @@ -281,7 +282,7 @@ extension Context { /// - label: Label to be used to obtain a globally bootstrapped `Logger` with. /// - reason: Informational reason for developers, why a placeholder context was used instead of a proper one, /// - Returns: Empty "to-do" baggage context which should be eventually replaced with a carried through one, or `background`. - public static func TODO(label loggerLabel: String, _ reason: StaticString? = "", function: String = #function, file: String = #file, line: UInt = #line) -> Context { + public static func TODO(label loggerLabel: String, _ reason: StaticString? = "", function: String = #function, file: String = #file, line: UInt = #line) -> DefaultContext { return .TODO(logger: Logger(label: loggerLabel), reason, function: function, file: file, line: line) } } diff --git a/Tests/BaggageContextTests/BaggageContextTests.swift b/Tests/BaggageContextTests/BaggageContextTests.swift index c862b87..e97ee56 100644 --- a/Tests/BaggageContextTests/BaggageContextTests.swift +++ b/Tests/BaggageContextTests/BaggageContextTests.swift @@ -24,7 +24,7 @@ final class BaggageContextTests: XCTestCase { baggage.testID = 42 let context = ExampleFrameworkContext(context: baggage, logger: logger) - func frameworkFunctionDumpsBaggage(param: String, context: ContextProtocol) -> String { + func frameworkFunctionDumpsBaggage(param: String, context: Context) -> String { var s = "" context.baggage.forEachBaggageItem { key, item in s += "\(key.name): \(item)\n" @@ -46,10 +46,10 @@ final class BaggageContextTests: XCTestCase { let baggage = Baggage.background let logger = Logger(label: "TheLogger") - var context: ContextProtocol & BaggageProtocol = ExampleMutableFrameworkContext(context: baggage, logger: logger) + var context: Context & BaggageProtocol = ExampleMutableFrameworkContext(context: baggage, logger: logger) context.testID = 42 - func frameworkFunctionDumpsBaggage(param: String, context: ContextProtocol & BaggageProtocol) -> String { + func frameworkFunctionDumpsBaggage(param: String, context: Context & BaggageProtocol) -> String { var s = "" context.forEachBaggageItem { key, item in s += "\(key.name): \(item)\n" @@ -108,7 +108,7 @@ final class BaggageContextTests: XCTestCase { } } -struct ExampleFrameworkContext: BaggageContext.ContextProtocol { +struct ExampleFrameworkContext: BaggageContext.Context { let baggage: Baggage let logger: Logger @@ -117,12 +117,12 @@ struct ExampleFrameworkContext: BaggageContext.ContextProtocol { self.logger = logger.with(self.baggage) } - var asBaggageContext: Context { + var asBaggageContext: DefaultContext { return .init(baggage: self.baggage, logger: self.logger) } } -struct ExampleMutableFrameworkContext: ContextProtocol, BaggageProtocol { +struct ExampleMutableFrameworkContext: Context, BaggageProtocol { var baggage: Baggage private var _logger: Logger @@ -148,12 +148,12 @@ struct ExampleMutableFrameworkContext: ContextProtocol, BaggageProtocol { return try self.baggage.forEachBaggageItem(body) } - var asBaggageContext: Context { + var asBaggageContext: DefaultContext { return .init(baggage: self.baggage, logger: self._logger) } } -struct CoolFrameworkContext: BaggageContext.ContextProtocol { +struct CoolFrameworkContext: BaggageContext.Context { private var _logger: Logger = Logger(label: "some frameworks logger") var logger: Logger { return self._logger.with(self.baggage) @@ -175,7 +175,7 @@ struct CoolFrameworkContext: BaggageContext.ContextProtocol { return try self.baggage.forEachBaggageItem(body) } - var asBaggageContext: Context { + var asBaggageContext: DefaultContext { return .init(baggage: self.baggage, logger: self._logger) } } From 358b3c91627a5d5f3afffae52016cf269caabbdc Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 15 Sep 2020 12:22:47 +0900 Subject: [PATCH 03/14] remove BaggageProtocol, remove label overloads for background context --- Sources/Baggage/Baggage.swift | 58 ++++--------- Sources/Baggage/BaggageKey.swift | 29 ++++--- Sources/BaggageContext/Context.swift | 84 ++----------------- .../BaggageContextTests+XCTest.swift | 1 - .../BaggageContextTests.swift | 49 ++--------- .../FrameworkContextTests+XCTest.swift | 0 .../FrameworkContextTests.swift | 59 ++++++++----- ...XCTest.swift => BaggageTests+XCTest.swift} | 6 +- ...{ContextTests.swift => BaggageTests.swift} | 2 +- Tests/LinuxMain.swift | 2 +- 10 files changed, 88 insertions(+), 202 deletions(-) rename Tests/{BaggageTests => BaggageContextTests}/FrameworkContextTests+XCTest.swift (100%) rename Tests/{BaggageTests => BaggageContextTests}/FrameworkContextTests.swift (56%) rename Tests/BaggageTests/{ContextTests+XCTest.swift => BaggageTests+XCTest.swift} (92%) rename Tests/BaggageTests/{ContextTests.swift => BaggageTests.swift} (98%) diff --git a/Sources/Baggage/Baggage.swift b/Sources/Baggage/Baggage.swift index 93f5c4e..b9b2a54 100644 --- a/Sources/Baggage/Baggage.swift +++ b/Sources/Baggage/Baggage.swift @@ -14,8 +14,7 @@ // ==== ---------------------------------------------------------------------------------------------------------------- // MARK: Baggage -/// A `Baggage` is a heterogeneous storage type with value semantics for keyed values in a type-safe -/// fashion. +/// A `Baggage` is a heterogeneous storage type with value semantics for keyed values in a type-safe fashion. /// /// Its values are uniquely identified via `BaggageKey`s (by type identity). These keys also dictate the type of /// value allowed for a specific key-value pair through their associated type `Value`. @@ -28,10 +27,10 @@ /// typealias Value = String /// } /// -/// While defining a key, one should also immediately declare an extension on `BaggageProtocol`, +/// While defining a key, one should also immediately declare an extension on `Baggage`, /// to allow convenient and discoverable ways to interact with the baggage item, the extension should take the form of: /// -/// extension BaggageProtocol { +/// extension Baggage { /// var testID: TestIDKey.Value? { /// get { /// self[TestIDKey.self] @@ -66,7 +65,7 @@ /// The baggage container on purpose does not expose more functions to prevent abuse and treating it as too much of an /// arbitrary value smuggling container, but only make it convenient for tracing and instrumentation systems which need /// to access either specific or all items carried inside a baggage. -public struct Baggage: BaggageProtocol { +public struct Baggage { public typealias Key = BaggageKey private var _storage = [AnyBaggageKey: Any]() @@ -74,15 +73,6 @@ public struct Baggage: BaggageProtocol { /// Internal on purpose, please use `Baggage.TODO` or `Baggage.background` to create an "empty" context, /// which carries more meaning to other developers why an empty context was used. init() {} - - public var baggage: Baggage { - get { - return self - } - set { - self = newValue - } - } } extension Baggage { @@ -156,20 +146,13 @@ extension Baggage { } } -/// The `BaggageProtocol` should not directly be used in APIs. -/// -/// Prefer accepting the `ContextProtocol` in APIs which need to accept a baggage context. -/// If `Context` can not be used for some reason, it is preferable to accept a `Baggage` directly rather than this protocol. -public protocol BaggageProtocol { - /// Get the `Baggage` container. - var baggage: Baggage { get set } - +extension Baggage { /// Provides type-safe access to the baggage's values. /// /// Rather than using this subscript directly, users SHOULD offer a convenience accessor to their values, /// using the following pattern: /// - /// extension BaggageProtocol { + /// extension Baggage { /// var testID: TestIDKey.Value? { /// get { /// self[TestIDKey.self] @@ -182,24 +165,6 @@ public protocol BaggageProtocol { /// /// Note that specific baggage and context types MAY (and usually do), offer also a way to set baggage values, /// however in the most general case it is not required, as some frameworks may only be able to offer reading. - subscript(_ key: Key.Type) -> Key.Value? { get set } - - /// Calls the given closure for each item contained in the underlying `Baggage`. - /// - /// Order of those invocations is NOT guaranteed and should not be relied on. - /// - /// - Parameter body: A closure invoked with the type erased key and value stored for the key in this baggage. - func forEachBaggageItem(_ body: (AnyBaggageKey, Any) throws -> Void) rethrows -} - -/// Writable `BaggageProtocol`. -/// -/// - SeeAlso: `_WritableBaggageProtocol` -public protocol _WritableBaggageProtocol: BaggageProtocol { - var baggage: Baggage { get set } -} - -extension Baggage { public subscript(_ key: Key.Type) -> Key.Value? { get { guard let value = self._storage[AnyBaggageKey(key)] else { return nil } @@ -211,6 +176,11 @@ extension Baggage { } } + /// Calls the given closure for each item contained in the underlying `Baggage`. + /// + /// Order of those invocations is NOT guaranteed and should not be relied on. + /// + /// - Parameter body: A closure invoked with the type erased key and value stored for the key in this baggage. public func forEachBaggageItem(_ body: (AnyBaggageKey, Any) throws -> Void) rethrows { try self._storage.forEach { key, value in try body(key, value) @@ -232,6 +202,8 @@ extension Baggage: CustomStringConvertible { /// Carried automatically by a "to do" baggage context. /// It can be used to track where a context originated and which "to do" context must be fixed into a real one to avoid this. public struct TODOLocation { - let file: String - let line: UInt + /// Source file location where the to-do `Baggage` was created + public let file: String + /// Source line location where the to-do `Baggage` was created + public let line: UInt } diff --git a/Sources/Baggage/BaggageKey.swift b/Sources/Baggage/BaggageKey.swift index d3960b2..be30ec1 100644 --- a/Sources/Baggage/BaggageKey.swift +++ b/Sources/Baggage/BaggageKey.swift @@ -14,29 +14,34 @@ /// `BaggageKey`s are used as keys in a `Baggage`. Their associated type `Value` guarantees type-safety. /// To give your `BaggageKey` an explicit name you may override the `name` property. /// -/// In general, `BaggageKey`s should be `internal` or `private` to the part of a system using it. It is strongly recommended to do -/// convenience extensions on `BaggageProtocol`, using the keys directly is considered an anti-pattern. +/// In general, `BaggageKey`s should be `internal` or `private` to the part of a system using it. +/// +/// All access to baggage items should be performed through an accessor computed property defined as shown below: /// /// private enum TestIDKey: Baggage.Key { -/// typealias Value = String -/// static var name: String? { "test-id" } +/// typealias Value = String +/// static var name: String? { "test-id" } /// } /// -/// extension BaggageProtocol { -/// var testID: TestIDKey.Value? { -/// get { -/// self[TestIDKey.self] -/// } set { -/// self[TestIDKey.self] = newValue +/// extension Baggage { +/// /// This is some useful property documentation. +/// var testID: String? { +/// get { +/// self[TestIDKey.self] +/// } +/// set { +/// self[TestIDKey.self] = newValue +/// } /// } -/// } /// } public protocol BaggageKey { /// The type of `Value` uniquely identified by this key. associatedtype Value - /// The human-readable name of this key. Defaults to `nil`. + /// The human-readable name of this key. /// May be used as key during serialization of the baggage item. + /// + /// Defaults to `nil`. static var name: String? { get } } diff --git a/Sources/BaggageContext/Context.swift b/Sources/BaggageContext/Context.swift index 1cfed25..96a1401 100644 --- a/Sources/BaggageContext/Context.swift +++ b/Sources/BaggageContext/Context.swift @@ -22,22 +22,6 @@ import Logging /// /// This allows frameworks and library authors to offer APIs which compose more easily. /// Please refer to the "Reference Implementation" notes on each of the requirements to know how to implement this protocol correctly. -/// -/// ### Relation to `BaggageProtocol` -/// The `ContextProtocol` does NOT extend `BaggageProtocol`. -/// -/// A custom context implementation however `MAY` extend both `ContextProtocol` and `BaggageProtocol`. -/// Doing so enables a custom context to obtain the baggage protocol's capability of getting *and setting* -/// properties on a baggage directly, like so: `myContext.traceID = "111-222"` without having to type out the baggage -/// explicitly, as would normally be the case (`myContext.baggage.traceID = "111-222"`). -/// -/// This difference in capabilities is purposefully designed as such, in order to allow context objects implement `ContextProtocol` -/// without having to force them to allow _modifying_ the baggage. Some frameworks or libraries may choose to create immutable -/// context objects, including the baggage they offer there. Users, when wanting to add more contextual information need then to -/// perform the `asBaggageContext`, mutate the resulting context OR use the `with...` fluent-API functions to obtain a mutated context. -/// -/// Please note that if a context does conform to `BaggageProtocol` as well, it will most-likely also offer all properties that -/// users have provided via extensions on it - so be watchful of potential naming conflicts on variable names. public protocol Context { /// Get the `Baggage` container. var baggage: Baggage { get } @@ -66,12 +50,8 @@ public protocol Context { /// } /// } var logger: Logger { get } - - /// Get a baggage `Context` struct, regardless of the representation that this context protocol is implemented by. - var asBaggageContext: DefaultContext { get } } - /// A default `Context` type. /// /// It is a carrier of contextual `Baggage` and related `Logger`, allowing to log and trace throughout a system. @@ -117,23 +97,24 @@ public struct DefaultContext: Context { self._logger = underlying } - public var asBaggageContext: DefaultContext { - return self + public init(context: C) where C: Context { + self._logger = context.logger + self.baggage = context.baggage } } // ==== ---------------------------------------------------------------------------------------------------------------- // MARK: `with...` functions -extension Context { +extension DefaultContext { /// Fluent API allowing for modification of underlying logger when passing the context to other functions. /// /// - Parameter logger: Logger that should replace the underlying logger of this context. /// - Returns: new context, with the passed in `logger` - public func withLogger(_ logger: Logger) -> DefaultContext { - var copy = self - copy.logger = logger - return copy + public func withLogger(_ function: (inout Logger) -> Void) -> DefaultContext { + var logger = self._logger + function(&logger) + return .init(baggage: self.baggage, logger: logger) } /// Fluent API allowing for modification of underlying log level when passing the context to other functions. @@ -202,27 +183,6 @@ extension DefaultContext { public static func background(logger: Logger) -> DefaultContext { return .init(baggage: .background, logger: logger) } - - /// A baggage context intended as a placeholder until a real value can be passed through a function call. - /// - /// It should ONLY be used while prototyping or when the passing of the proper context is not yet possible, - /// e.g. because an external library did not pass it correctly and has to be fixed before the proper context - /// can be obtained where the TO-DO is currently used. - /// - /// ### Crashing on TO-DO context creation - /// You may set the `BAGGAGE_CRASH_TODOS` variable while compiling a project in order to make calls to this function crash - /// with a fatal error, indicating where a to-do baggage context was used. This comes in handy when wanting to ensure that - /// a project never ends up using with code initially was written as "was lazy, did not pass context", yet the - /// project requires context passing to be done correctly throughout the application. Similar checks can be performed - /// at compile time easily using linters (not yet implemented), since it is always valid enough to detect a to-do context - /// being passed as illegal and warn or error when spotted. - /// - /// - Parameters: - /// - reason: Informational reason for developers, why a placeholder context was used instead of a proper one, - /// - Returns: Empty "to-do" baggage context which should be eventually replaced with a carried through one, or `background`. - public static func background(label loggerLabel: String) -> DefaultContext { - return .background(logger: Logger(label: loggerLabel)) - } } extension DefaultContext { @@ -257,32 +217,4 @@ extension DefaultContext { return .init(baggage: baggage, logger: logger) #endif } - - /// A baggage context intended as a placeholder until a real value can be passed through a function call. - /// - /// It should ONLY be used while prototyping or when the passing of the proper context is not yet possible, - /// e.g. because an external library did not pass it correctly and has to be fixed before the proper context - /// can be obtained where the TO-DO is currently used. - /// - /// ## Crashing on TO-DO context creation - /// You may set the `BAGGAGE_CRASH_TODOS` variable while compiling a project in order to make calls to this function crash - /// with a fatal error, indicating where a to-do baggage context was used. This comes in handy when wanting to ensure that - /// a project never ends up using with code initially was written as "was lazy, did not pass context", yet the - /// project requires context passing to be done correctly throughout the application. Similar checks can be performed - /// at compile time easily using linters (not yet implemented), since it is always valid enough to detect a to-do context - /// being passed as illegal and warn or error when spotted. - /// - /// ## Example - /// - /// frameworkHandler { what in - /// hello(who: "World", baggage: .TODO(label: "MyLib", "The framework XYZ should be modified to pass us a context here, and we'd pass it along")) - /// } - /// - /// - Parameters: - /// - label: Label to be used to obtain a globally bootstrapped `Logger` with. - /// - reason: Informational reason for developers, why a placeholder context was used instead of a proper one, - /// - Returns: Empty "to-do" baggage context which should be eventually replaced with a carried through one, or `background`. - public static func TODO(label loggerLabel: String, _ reason: StaticString? = "", function: String = #function, file: String = #file, line: UInt = #line) -> DefaultContext { - return .TODO(logger: Logger(label: loggerLabel), reason, function: function, file: file, line: line) - } } diff --git a/Tests/BaggageContextTests/BaggageContextTests+XCTest.swift b/Tests/BaggageContextTests/BaggageContextTests+XCTest.swift index 99ef608..2cdd030 100644 --- a/Tests/BaggageContextTests/BaggageContextTests+XCTest.swift +++ b/Tests/BaggageContextTests/BaggageContextTests+XCTest.swift @@ -26,7 +26,6 @@ extension BaggageContextTests { static var allTests : [(String, (BaggageContextTests) -> () throws -> Void)] { return [ ("test_ExampleFrameworkContext_dumpBaggage", test_ExampleFrameworkContext_dumpBaggage), - ("test_ExampleMutableFrameworkContext_dumpBaggage", test_ExampleMutableFrameworkContext_dumpBaggage), ("test_ExampleMutableFrameworkContext_log_withBaggage", test_ExampleMutableFrameworkContext_log_withBaggage), ("test_ExampleMutableFrameworkContext_log_prefersBaggageContextOverExistingLoggerMetadata", test_ExampleMutableFrameworkContext_log_prefersBaggageContextOverExistingLoggerMetadata), ] diff --git a/Tests/BaggageContextTests/BaggageContextTests.swift b/Tests/BaggageContextTests/BaggageContextTests.swift index e97ee56..10abf1c 100644 --- a/Tests/BaggageContextTests/BaggageContextTests.swift +++ b/Tests/BaggageContextTests/BaggageContextTests.swift @@ -42,31 +42,6 @@ final class BaggageContextTests: XCTestCase { ) } - func test_ExampleMutableFrameworkContext_dumpBaggage() throws { - let baggage = Baggage.background - let logger = Logger(label: "TheLogger") - - var context: Context & BaggageProtocol = ExampleMutableFrameworkContext(context: baggage, logger: logger) - context.testID = 42 - - func frameworkFunctionDumpsBaggage(param: String, context: Context & BaggageProtocol) -> String { - var s = "" - context.forEachBaggageItem { key, item in - s += "\(key.name): \(item)\n" - } - return s - } - - let result = frameworkFunctionDumpsBaggage(param: "x", context: context) - XCTAssertEqual( - result, - """ - TestIDKey: 42 - - """ - ) - } - func test_ExampleMutableFrameworkContext_log_withBaggage() throws { let baggage = Baggage.background let logging = TestLogging() @@ -74,11 +49,11 @@ final class BaggageContextTests: XCTestCase { var context = ExampleMutableFrameworkContext(context: baggage, logger: logger) - context.secondTestID = "value" - context.testID = 42 + context.baggage.secondTestID = "value" + context.baggage.testID = 42 context.logger.info("Hello") - context.testID = nil + context.baggage.testID = nil context.logger.warning("World") logging.history.assertExist(level: .info, message: "Hello", metadata: [ @@ -98,7 +73,7 @@ final class BaggageContextTests: XCTestCase { var context = ExampleMutableFrameworkContext(context: baggage, logger: logger) - context.secondTestID = "set on baggage" + context.baggage.secondTestID = "set on baggage" context.logger.info("Hello") @@ -116,13 +91,9 @@ struct ExampleFrameworkContext: BaggageContext.Context { self.baggage = baggage self.logger = logger.with(self.baggage) } - - var asBaggageContext: DefaultContext { - return .init(baggage: self.baggage, logger: self.logger) - } } -struct ExampleMutableFrameworkContext: Context, BaggageProtocol { +struct ExampleMutableFrameworkContext: Context { var baggage: Baggage private var _logger: Logger @@ -147,10 +118,6 @@ struct ExampleMutableFrameworkContext: Context, BaggageProtocol { func forEachBaggageItem(_ body: (AnyBaggageKey, Any) throws -> Void) rethrows { return try self.baggage.forEachBaggageItem(body) } - - var asBaggageContext: DefaultContext { - return .init(baggage: self.baggage, logger: self._logger) - } } struct CoolFrameworkContext: BaggageContext.Context { @@ -174,15 +141,11 @@ struct CoolFrameworkContext: BaggageContext.Context { func forEachBaggageItem(_ body: (AnyBaggageKey, Any) throws -> Void) rethrows { return try self.baggage.forEachBaggageItem(body) } - - var asBaggageContext: DefaultContext { - return .init(baggage: self.baggage, logger: self._logger) - } } struct FakeEventLoop {} -private extension BaggageProtocol { +private extension Baggage { var testID: Int? { get { return self[TestIDKey.self] diff --git a/Tests/BaggageTests/FrameworkContextTests+XCTest.swift b/Tests/BaggageContextTests/FrameworkContextTests+XCTest.swift similarity index 100% rename from Tests/BaggageTests/FrameworkContextTests+XCTest.swift rename to Tests/BaggageContextTests/FrameworkContextTests+XCTest.swift diff --git a/Tests/BaggageTests/FrameworkContextTests.swift b/Tests/BaggageContextTests/FrameworkContextTests.swift similarity index 56% rename from Tests/BaggageTests/FrameworkContextTests.swift rename to Tests/BaggageContextTests/FrameworkContextTests.swift index 1d4c090..50e537d 100644 --- a/Tests/BaggageTests/FrameworkContextTests.swift +++ b/Tests/BaggageContextTests/FrameworkContextTests.swift @@ -12,31 +12,27 @@ //===----------------------------------------------------------------------===// @testable import Baggage +import BaggageContext +import Logging import XCTest final class FrameworkBaggageContextTests: XCTestCase { func testBaggageContextSubscript() { - var carrier = TestFrameworkContext() - - // mutate baggage context through carrier - carrier[TestKey.self] = 42 - XCTAssertEqual(carrier[TestKey.self], 42) - XCTAssertEqual(carrier.baggage[TestKey.self], 42) + var context = TestFrameworkContext() // mutate baggage context directly - carrier.baggage[OtherKey.self] = "test" - XCTAssertEqual(carrier.baggage[OtherKey.self], "test") - XCTAssertEqual(carrier[OtherKey.self], "test") + context.baggage[OtherKey.self] = "test" + XCTAssertEqual(context.baggage.otherKey, "test") } func testBaggageContextForEach() { var contents = [AnyBaggageKey: Any]() - var carrier = TestFrameworkContext() + var context = TestFrameworkContext() - carrier[TestKey.self] = 42 - carrier[OtherKey.self] = "test" + context.baggage.testKey = 42 + context.baggage.otherKey = "test" - carrier.forEachBaggageItem { key, value in + context.baggage.forEachBaggageItem { key, value in contents[key] = value } @@ -47,27 +43,46 @@ final class FrameworkBaggageContextTests: XCTestCase { } } -private struct TestFrameworkContext: BaggageProtocol { - var baggage = Baggage() +private struct TestFrameworkContext: Context { + var baggage = Baggage.background - subscript(_ key: Key.Type) -> Key.Value? { + private var _logger = Logger(label: "test") + var logger: Logger { get { - return self.baggage[key] + return self._logger.with(self.baggage) } set { - self.baggage[key] = newValue + self._logger = newValue } } - - func forEachBaggageItem(_ body: (AnyBaggageKey, Any) throws -> Void) rethrows { - return try self.baggage.forEachBaggageItem(body) - } } private enum TestKey: Baggage.Key { typealias Value = Int } +extension Baggage { + var testKey: Int? { + get { + return self[TestKey.self] + } + set { + self[TestKey.self] = newValue + } + } +} + private enum OtherKey: Baggage.Key { typealias Value = String } + +extension Baggage { + var otherKey: String? { + get { + return self[OtherKey.self] + } + set { + self[OtherKey.self] = newValue + } + } +} diff --git a/Tests/BaggageTests/ContextTests+XCTest.swift b/Tests/BaggageTests/BaggageTests+XCTest.swift similarity index 92% rename from Tests/BaggageTests/ContextTests+XCTest.swift rename to Tests/BaggageTests/BaggageTests+XCTest.swift index 5dff63f..4b53bde 100644 --- a/Tests/BaggageTests/ContextTests+XCTest.swift +++ b/Tests/BaggageTests/BaggageTests+XCTest.swift @@ -11,7 +11,7 @@ // //===----------------------------------------------------------------------===// // -// ContextTests+XCTest.swift +// BaggageTests+XCTest.swift // import XCTest /// @@ -20,10 +20,10 @@ import XCTest /// Do NOT edit this file directly as it will be regenerated automatically when needed. /// -extension ContextTests { +extension BaggageTests { @available(*, deprecated, message: "not actually deprecated. Just deprecated to allow deprecated tests (which test deprecated functionality) without warnings") - static var allTests : [(String, (ContextTests) -> () throws -> Void)] { + static var allTests : [(String, (BaggageTests) -> () throws -> Void)] { return [ ("testSubscriptAccess", testSubscriptAccess), ("testRecommendedConvenienceExtension", testRecommendedConvenienceExtension), diff --git a/Tests/BaggageTests/ContextTests.swift b/Tests/BaggageTests/BaggageTests.swift similarity index 98% rename from Tests/BaggageTests/ContextTests.swift rename to Tests/BaggageTests/BaggageTests.swift index b058cc8..867932a 100644 --- a/Tests/BaggageTests/ContextTests.swift +++ b/Tests/BaggageTests/BaggageTests.swift @@ -14,7 +14,7 @@ import Baggage import XCTest -final class ContextTests: XCTestCase { +final class BaggageTests: XCTestCase { func testSubscriptAccess() { let testID = 42 diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index e7a87cc..44c5e22 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -34,7 +34,7 @@ class LinuxMainRunnerImpl: LinuxMainRunner { func run() { XCTMain([ testCase(BaggageContextTests.allTests), - testCase(ContextTests.allTests), + testCase(BaggageTests.allTests), testCase(FrameworkBaggageContextTests.allTests), ]) } From dbbe0348fbf0b31ac8e293a284dda4b5dae129e4 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 15 Sep 2020 13:19:48 +0900 Subject: [PATCH 04/14] comment cleanups --- Sources/Baggage/Baggage.swift | 20 ++++++++++--------- Sources/BaggageContext/Context.swift | 2 +- .../BaggageContext/ContextLogHandler.swift | 12 +++++------ 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/Sources/Baggage/Baggage.swift b/Sources/Baggage/Baggage.swift index b9b2a54..d05adcf 100644 --- a/Sources/Baggage/Baggage.swift +++ b/Sources/Baggage/Baggage.swift @@ -16,14 +16,14 @@ /// A `Baggage` is a heterogeneous storage type with value semantics for keyed values in a type-safe fashion. /// -/// Its values are uniquely identified via `BaggageKey`s (by type identity). These keys also dictate the type of +/// Its values are uniquely identified via `Baggage.Key`s (by type identity). These keys also dictate the type of /// value allowed for a specific key-value pair through their associated type `Value`. /// /// ## Defining keys and accessing values /// Baggage keys are defined as types, most commonly case-less enums (as no actual instances are actually required) /// which conform to the `Baggage.Key` protocol: /// -/// private enum TestIDKey: BaggageKey { +/// private enum TestIDKey: Baggage.Key { /// typealias Value = String /// } /// @@ -31,7 +31,7 @@ /// to allow convenient and discoverable ways to interact with the baggage item, the extension should take the form of: /// /// extension Baggage { -/// var testID: TestIDKey.Value? { +/// var testID: String? { /// get { /// self[TestIDKey.self] /// } set { @@ -53,11 +53,11 @@ /// // retrieve a stored value /// let testID = context.testID ?? "default" /// // remove a stored value -/// context[TestIDKey.self] = nil +/// context.testIDKey = nil /// /// Note that normally a baggage should not be "created" ad-hoc by user code, but rather it should be passed to it from /// a runtime. For example, when working in an HTTP server framework, it is most likely that the baggage is already passed -/// directly or indirectly (e.g. in a `BaggageContext` or `FrameworkContext`) +/// directly or indirectly (e.g. in a `FrameworkContext`) /// /// ### Accessing all values /// @@ -121,9 +121,7 @@ extension Baggage { /// /// ## Example /// - /// frameworkHandler { what in - /// hello(who: "World", baggage: .TODO("The framework XYZ should be modified to pass us a context here, and we'd pass it along")) - /// } + /// let baggage = Baggage.TODO("The framework XYZ should be modified to pass us a context here, and we'd pass it along")) /// /// - Parameters: /// - reason: Informational reason for developers, why a placeholder context was used instead of a proper one, @@ -152,8 +150,12 @@ extension Baggage { /// Rather than using this subscript directly, users SHOULD offer a convenience accessor to their values, /// using the following pattern: /// + /// internal enum TestID: Baggage.Key { + /// typealias Value = TestID + /// } + /// /// extension Baggage { - /// var testID: TestIDKey.Value? { + /// var testID: TestID? { /// get { /// self[TestIDKey.self] /// } diff --git a/Sources/BaggageContext/Context.swift b/Sources/BaggageContext/Context.swift index 96a1401..619f8d4 100644 --- a/Sources/BaggageContext/Context.swift +++ b/Sources/BaggageContext/Context.swift @@ -28,7 +28,7 @@ public protocol Context { /// The `Logger` associated with this context carrier. /// - /// It automatically populates the loggers metadata based on the `BaggageContext` associated with this context object. + /// It automatically populates the loggers metadata based on the `Baggage` associated with this context object. /// /// ### Implementation note /// diff --git a/Sources/BaggageContext/ContextLogHandler.swift b/Sources/BaggageContext/ContextLogHandler.swift index 77d8c2f..aebfaa3 100644 --- a/Sources/BaggageContext/ContextLogHandler.swift +++ b/Sources/BaggageContext/ContextLogHandler.swift @@ -19,7 +19,7 @@ import Logging extension Logger { /// Returns a logger that in addition to any explicit metadata passed to log statements, - /// also includes the `BaggageContext` adapted into metadata values. + /// also includes the `Baggage` adapted into metadata values. /// /// The rendering of baggage values into metadata values is performed on demand, /// whenever a log statement is effective (i.e. will be logged, according to active `logLevel`). @@ -32,12 +32,12 @@ extension Logger { } // ==== ---------------------------------------------------------------------------------------------------------------- -// MARK: BaggageContext (as additional Logger.Metadata) LogHandler +// MARK: Baggage (as additional Logger.Metadata) LogHandler -/// Proxying log handler which adds `BaggageContext` as metadata when log events are to be emitted. +/// Proxying log handler which adds `Baggage` as metadata when log events are to be emitted. /// -/// The values stored in the `BaggageContext` are merged with the existing metadata on the logger. If both contain values for the same key, -/// the `BaggageContext` values are preferred. +/// The values stored in the `Baggage` are merged with the existing metadata on the logger. If both contain values for the same key, +/// the `Baggage` values are preferred. public struct BaggageMetadataLogHandler: LogHandler { private var underlying: Logger private let baggage: Baggage @@ -91,7 +91,7 @@ public struct BaggageMetadataLogHandler: LogHandler { /// /// This is because a context lookup either has to use the specific type key, or iterate over all keys to locate one by name, /// which may be incorrect still, thus rather than making an potentially slightly incorrect lookup, we do not implement peeking - /// into a baggage with String keys through this handler (as that is not a capability `BaggageContext` offers in any case. + /// into a baggage with String keys through this handler (as that is not a capability `Baggage` offers in any case. public subscript(metadataKey metadataKey: Logger.Metadata.Key) -> Logger.Metadata.Value? { get { return self.underlying[metadataKey: metadataKey] From 9ad00d67e2e05484def549146fa6c700ea9cc1c1 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 15 Sep 2020 13:20:40 +0900 Subject: [PATCH 05/14] since BaggageProtocol is gone, no need for long name of forEach --- Sources/Baggage/Baggage.swift | 4 ++-- Sources/BaggageContext/ContextLogHandler.swift | 2 +- Tests/BaggageContextTests/BaggageContextTests.swift | 6 +++--- Tests/BaggageContextTests/FrameworkContextTests.swift | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Sources/Baggage/Baggage.swift b/Sources/Baggage/Baggage.swift index d05adcf..fc7c910 100644 --- a/Sources/Baggage/Baggage.swift +++ b/Sources/Baggage/Baggage.swift @@ -61,7 +61,7 @@ /// /// ### Accessing all values /// -/// The only way to access "all" values in a baggage context is by using the `forEachBaggageItem` function. +/// The only way to access "all" values in a baggage context is by using the `forEach` function. /// The baggage container on purpose does not expose more functions to prevent abuse and treating it as too much of an /// arbitrary value smuggling container, but only make it convenient for tracing and instrumentation systems which need /// to access either specific or all items carried inside a baggage. @@ -183,7 +183,7 @@ extension Baggage { /// Order of those invocations is NOT guaranteed and should not be relied on. /// /// - Parameter body: A closure invoked with the type erased key and value stored for the key in this baggage. - public func forEachBaggageItem(_ body: (AnyBaggageKey, Any) throws -> Void) rethrows { + public func forEach(_ body: (AnyBaggageKey, Any) throws -> Void) rethrows { try self._storage.forEach { key, value in try body(key, value) } diff --git a/Sources/BaggageContext/ContextLogHandler.swift b/Sources/BaggageContext/ContextLogHandler.swift index aebfaa3..61ebb9c 100644 --- a/Sources/BaggageContext/ContextLogHandler.swift +++ b/Sources/BaggageContext/ContextLogHandler.swift @@ -103,7 +103,7 @@ public struct BaggageMetadataLogHandler: LogHandler { private func baggageAsMetadata() -> Logger.Metadata { var effectiveMetadata: Logger.Metadata = [:] - self.baggage.forEachBaggageItem { key, value in + self.baggage.forEach { key, value in if let convertible = value as? String { effectiveMetadata[key.name] = .string(convertible) } else if let convertible = value as? CustomStringConvertible { diff --git a/Tests/BaggageContextTests/BaggageContextTests.swift b/Tests/BaggageContextTests/BaggageContextTests.swift index 10abf1c..a776c6d 100644 --- a/Tests/BaggageContextTests/BaggageContextTests.swift +++ b/Tests/BaggageContextTests/BaggageContextTests.swift @@ -26,7 +26,7 @@ final class BaggageContextTests: XCTestCase { func frameworkFunctionDumpsBaggage(param: String, context: Context) -> String { var s = "" - context.baggage.forEachBaggageItem { key, item in + context.baggage.forEach { key, item in s += "\(key.name): \(item)\n" } return s @@ -116,7 +116,7 @@ struct ExampleMutableFrameworkContext: Context { } func forEachBaggageItem(_ body: (AnyBaggageKey, Any) throws -> Void) rethrows { - return try self.baggage.forEachBaggageItem(body) + return try self.baggage.forEach(body) } } @@ -139,7 +139,7 @@ struct CoolFrameworkContext: BaggageContext.Context { } func forEachBaggageItem(_ body: (AnyBaggageKey, Any) throws -> Void) rethrows { - return try self.baggage.forEachBaggageItem(body) + return try self.baggage.forEach(body) } } diff --git a/Tests/BaggageContextTests/FrameworkContextTests.swift b/Tests/BaggageContextTests/FrameworkContextTests.swift index 50e537d..2c28be2 100644 --- a/Tests/BaggageContextTests/FrameworkContextTests.swift +++ b/Tests/BaggageContextTests/FrameworkContextTests.swift @@ -32,7 +32,7 @@ final class FrameworkBaggageContextTests: XCTestCase { context.baggage.testKey = 42 context.baggage.otherKey = "test" - context.baggage.forEachBaggageItem { key, value in + context.baggage.forEach { key, value in contents[key] = value } From 1318b835bf227b09841c708da911ab7e97db9b18 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Fri, 18 Sep 2020 17:56:30 +0900 Subject: [PATCH 06/14] Change DefaultContext implementation to update logger on baggage change, rather than each time, which would lead to terrible logging performance if many log statements are made. Generally, there are fewer baggage changes than there are log statements so this tradeoff makes sense. The logger modification amortizes over time if it is not changed again and again. --- Package.swift | 2 +- Sources/Baggage/Baggage.swift | 7 +- Sources/BaggageContext/Context.swift | 40 +- .../BaggageContext/ContextLogHandler.swift | 15 + .../BenchmarkCategory.swift | 3 + .../BaggageLoggingBenchmarks.swift | 379 ++++++++++++++++++ Sources/BaggageContextBenchmarks/main.swift | 1 + 7 files changed, 432 insertions(+), 15 deletions(-) create mode 100644 Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift diff --git a/Package.swift b/Package.swift index 96eabcc..e57ecd5 100644 --- a/Package.swift +++ b/Package.swift @@ -18,7 +18,7 @@ let package = Package( ), ], dependencies: [ - .package(url: "https://github.com/apple/swift-log.git", from: "1.3.0"), + .package(url: "https://github.com/apple/swift-log.git", from: "1.4.0"), ], targets: [ .target( diff --git a/Sources/Baggage/Baggage.swift b/Sources/Baggage/Baggage.swift index fc7c910..f90554a 100644 --- a/Sources/Baggage/Baggage.swift +++ b/Sources/Baggage/Baggage.swift @@ -178,6 +178,11 @@ extension Baggage { } } + /// Number of contained baggage items. + public var count: Int { + return self._storage.count + } + /// Calls the given closure for each item contained in the underlying `Baggage`. /// /// Order of those invocations is NOT guaranteed and should not be relied on. @@ -201,7 +206,7 @@ extension Baggage: CustomStringConvertible { } } -/// Carried automatically by a "to do" baggage context. +/// Carried automatically by a "to do" baggage. /// It can be used to track where a context originated and which "to do" context must be fixed into a real one to avoid this. public struct TODOLocation { /// Source file location where the to-do `Baggage` was created diff --git a/Sources/BaggageContext/Context.swift b/Sources/BaggageContext/Context.swift index 619f8d4..f5a711c 100644 --- a/Sources/BaggageContext/Context.swift +++ b/Sources/BaggageContext/Context.swift @@ -11,7 +11,7 @@ // //===----------------------------------------------------------------------===// -import Baggage +@_exported import Baggage import Logging // ==== ---------------------------------------------------------------------------------------------------------------- @@ -79,27 +79,41 @@ public struct DefaultContext: Context { /// Baggage values are different from plain logging metadata in that they are intended to be /// carried across process and node boundaries (serialized and deserialized) and are made /// available to instruments using `swift-distributed-tracing`. - public var baggage: Baggage + public var baggage: Baggage { + willSet { + // every time the baggage changes, we need to update the logger; + // values removed from the baggage are also removed from the logger metadata. + // + // TODO: optimally, logger could some day accept baggage directly, without ever having to map it into `Metadata`, + // then we would not have to make those mappings at all and passing the logger.with(baggage) would be cheap. + // + // This implementation generally is a tradeoff, we bet on logging being performed far more often than baggage + // being changed; We do this logger update eagerly, so even if we never log anything, the logger has to be updated. + // Systems which never or rarely log will take the hit for it here. The alternative tradeoff to map lazily as `logger.with(baggage)` + // is available as well, but users would have to build their own context and specifically make use of that then -- that approach + // allows to not pay the mapping cost up front, but only if a log statement is made (but then again, the cost is paid every time we log something). + self.logger.update(previous: self.baggage, latest: newValue) + } + } public var logger: Logger { - get { - return self._logger.with(self.baggage) - } - set { - self._logger = newValue + didSet { + // Since someone could have completely replaced the logger (not just changed the log level), + // we have to update the baggage again, since perhaps the new logger has empty metadata. + self.logger.update(previous: .background, latest: self.baggage) } } - private var _logger: Logger - - public init(baggage: Baggage, logger underlying: Logger) { + public init(baggage: Baggage, logger: Logger) { self.baggage = baggage - self._logger = underlying + self.logger = logger + self.logger.update(previous: .background, latest: baggage) } public init(context: C) where C: Context { - self._logger = context.logger self.baggage = context.baggage + self.logger = context.logger + self.logger.update(previous: .background, latest: self.baggage) } } @@ -112,7 +126,7 @@ extension DefaultContext { /// - Parameter logger: Logger that should replace the underlying logger of this context. /// - Returns: new context, with the passed in `logger` public func withLogger(_ function: (inout Logger) -> Void) -> DefaultContext { - var logger = self._logger + var logger = self.logger function(&logger) return .init(baggage: self.baggage, logger: logger) } diff --git a/Sources/BaggageContext/ContextLogHandler.swift b/Sources/BaggageContext/ContextLogHandler.swift index 61ebb9c..951c951 100644 --- a/Sources/BaggageContext/ContextLogHandler.swift +++ b/Sources/BaggageContext/ContextLogHandler.swift @@ -29,6 +29,21 @@ extension Logger { factory: { _ in BaggageMetadataLogHandler(logger: self, baggage: baggage) } ) } + + public mutating func update(previous: Baggage, latest: Baggage) { + var removedKeys: Set = [] + removedKeys.reserveCapacity(previous.count) + previous.forEach { key, _ in + removedKeys.insert(key) + } + latest.forEach { key, value in + removedKeys.remove(key) + self[metadataKey: key.name] = "\(value)" + } + removedKeys.forEach { removedKey in + self[metadataKey: removedKey.name] = nil + } + } } // ==== ---------------------------------------------------------------------------------------------------------------- diff --git a/Sources/BaggageContextBenchmarkTools/BenchmarkCategory.swift b/Sources/BaggageContextBenchmarkTools/BenchmarkCategory.swift index 821880e..4a978ad 100644 --- a/Sources/BaggageContextBenchmarkTools/BenchmarkCategory.swift +++ b/Sources/BaggageContextBenchmarkTools/BenchmarkCategory.swift @@ -25,4 +25,7 @@ public enum BenchmarkCategory: String { // Explicit skip marker case skip + + // --- custom --- + case logging } diff --git a/Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift b/Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift new file mode 100644 index 0000000..8bb92ba --- /dev/null +++ b/Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift @@ -0,0 +1,379 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift Baggage Context open source project +// +// Copyright (c) 2020 Moritz Lang and the Swift Baggage Context project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +import Baggage +import BaggageContext +import BaggageContextBenchmarkTools +import Dispatch +import class Foundation.NSLock +import Logging + +private let message: Logger.Message = "Hello world how are you" + +func pad(_ label: String) -> String { + "\(label)\(String(repeating: " ", count: max(0, 80 - label.count)))" +} + +public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ + // ==== ---------------------------------------------------------------------------------------------------------------- + // MARK: Baseline + BenchmarkInfo( + name: pad("BaggageLoggingBenchmarks.0_log_noop_baseline_empty"), + runFunction: { iters in + let logger = Logger(label: "0_log_noop_baseline_empty", factory: { _ in SwiftLogNoOpLogHandler() }) + log_baseline(logger: logger, iters: iters) + }, + tags: [ + .logging, + ], + setUpFunction: { setUp() }, + tearDownFunction: tearDown + ), + BenchmarkInfo( + name: pad("BaggageLoggingBenchmarks.0_log_noop_baseline_smallMetadata"), + runFunction: { iters in + var logger = Logger(label: "0_log_noop_baseline_smallMetadata", factory: { _ in SwiftLogNoOpLogHandler() }) + logger[metadataKey: "k1"] = "k1-value" + logger[metadataKey: "k2"] = "k2-value" + logger[metadataKey: "k3"] = "k3-value" + log_baseline(logger: logger, iters: iters) + }, + tags: [ + .logging, + ], + setUpFunction: { setUp() }, + tearDownFunction: tearDown + ), + + // ==== ---------------------------------------------------------------------------------------------------------------- + // MARK: Context / Baggage (Really do log) + + BenchmarkInfo( + name: pad("BaggageLoggingBenchmarks.0_log_noop_loggerWithBaggage_small"), + runFunction: { iters in + let logger = Logger(label: "0_log_noop_loggerWithBaggage_small", factory: { _ in SwiftLogNoOpLogHandler() }) + var baggage = Baggage.background + baggage[TestK1.self] = "k1-value" + baggage[TestK2.self] = "k2-value" + baggage[TestK3.self] = "k3-value" + log_loggerWithBaggage(logger: logger, baggage: baggage, iters: iters) + }, + tags: [ + .logging, + ], + setUpFunction: { setUp() }, + tearDownFunction: tearDown + ), + BenchmarkInfo( + name: pad("BaggageLoggingBenchmarks.0_log_noop_context_with_baggage_small"), + runFunction: { iters in + var context = DefaultContext.background(logger: Logger(label: "0_log_noop_context_with_baggage_small", factory: { _ in SwiftLogNoOpLogHandler() })) + context.baggage[TestK1.self] = "k1-value" + context.baggage[TestK2.self] = "k2-value" + context.baggage[TestK3.self] = "k3-value" + log_throughContext(context: context, iters: iters) + }, + tags: [ + .logging, + ], + setUpFunction: { setUp() }, + tearDownFunction: tearDown + ), + + // ==== ---------------------------------------------------------------------------------------------------------------- + // MARK: Context / Baggage (do actually emit the logs) + + BenchmarkInfo( + name: pad("BaggageLoggingBenchmarks.1_log_real_baseline_empty"), + runFunction: { iters in + let logger = Logger(label: "1_log_real_baseline_empty", factory: StreamLogHandler.standardError) + log_baseline(logger: logger, iters: iters) + }, + tags: [ + .logging, + ], + setUpFunction: { setUp() }, + tearDownFunction: tearDown + ), + BenchmarkInfo( + name: pad("BaggageLoggingBenchmarks.1_log_real_baseline_smallMetadata"), + runFunction: { iters in + var logger = Logger(label: "1_log_real_baseline_smallMetadata", factory: StreamLogHandler.standardError) + logger[metadataKey: "k1"] = "k1-value" + logger[metadataKey: "k2"] = "k2-value" + logger[metadataKey: "k3"] = "k3-value" + log_baseline(logger: logger, iters: iters) + }, + tags: [ + .logging, + ], + setUpFunction: { setUp() }, + tearDownFunction: tearDown + ), + + BenchmarkInfo( + name: pad("BaggageLoggingBenchmarks.1_log_real_loggerWithBaggage_small"), + runFunction: { iters in + let logger = Logger(label: "1_log_real_loggerWithBaggage_small", factory: StreamLogHandler.standardError) + var baggage = Baggage.background + baggage[TestK1.self] = "k1-value" + baggage[TestK2.self] = "k2-value" + baggage[TestK3.self] = "k3-value" + log_loggerWithBaggage(logger: logger, baggage: baggage, iters: iters) + }, + tags: [ + .logging, + ], + setUpFunction: { setUp() }, + tearDownFunction: tearDown + ), + BenchmarkInfo( + name: pad("BaggageLoggingBenchmarks.1_log_real_context_with_baggage_small"), + runFunction: { iters in + var context = DefaultContext.background(logger: Logger(label: "1_log_real_context_with_baggage_small", factory: StreamLogHandler.standardError)) + context.baggage[TestK1.self] = "k1-value" + context.baggage[TestK2.self] = "k2-value" + context.baggage[TestK3.self] = "k3-value" + log_throughContext(context: context, iters: iters) + }, + tags: [ + .logging, + ], + setUpFunction: { setUp() }, + tearDownFunction: tearDown + ), + + // ==== ---------------------------------------------------------------------------------------------------------------- + // MARK: Context / Baggage (log not emitted because logLevel) + + BenchmarkInfo( + name: pad("BaggageLoggingBenchmarks.2_log_real-trace_baseline_empty"), + runFunction: { iters in + let logger = Logger(label: "trace_baseline_empty", factory: StreamLogHandler.standardError) + log_baseline_trace(logger: logger, iters: iters) + }, + tags: [ + .logging, + ], + setUpFunction: { setUp() }, + tearDownFunction: tearDown + ), + BenchmarkInfo( + name: pad("BaggageLoggingBenchmarks.2_log_real-trace_baseline_smallMetadata"), + runFunction: { iters in + var logger = Logger(label: "2_log_real-trace_baseline_smallMetadata", factory: StreamLogHandler.standardError) + logger[metadataKey: "k1"] = "k1-value" + logger[metadataKey: "k2"] = "k2-value" + logger[metadataKey: "k3"] = "k3-value" + log_baseline_trace(logger: logger, iters: iters) + }, + tags: [ + .logging, + ], + setUpFunction: { setUp() }, + tearDownFunction: tearDown + ), + + BenchmarkInfo( + name: pad("BaggageLoggingBenchmarks.2_log_real-trace_loggerWithBaggage_small"), + runFunction: { iters in + let logger = Logger(label: "2_log_real-trace_loggerWithBaggage_small", factory: StreamLogHandler.standardError) + var baggage = Baggage.background + baggage[TestK1.self] = "k1-value" + baggage[TestK2.self] = "k2-value" + baggage[TestK3.self] = "k3-value" + log_loggerWithBaggage_trace(logger: logger, baggage: baggage, iters: iters) + }, + tags: [ + .logging, + ], + setUpFunction: { setUp() }, + tearDownFunction: tearDown + ), + BenchmarkInfo( + name: pad("BaggageLoggingBenchmarks.2_log_real-trace_context_with_baggage_small"), + runFunction: { iters in + var context = DefaultContext.background(logger: Logger(label: "2_log_real-trace_context_with_baggage_small", factory: StreamLogHandler.standardError)) + context.baggage[TestK1.self] = "k1-value" + context.baggage[TestK2.self] = "k2-value" + context.baggage[TestK3.self] = "k3-value" + log_throughContext_trace(context: context, iters: iters) + }, + tags: [ + .logging, + ], + setUpFunction: { setUp() }, + tearDownFunction: tearDown + ), + + // ==== ---------------------------------------------------------------------------------------------------------------- + // MARK: materialize once once + + BenchmarkInfo( + name: pad("BaggageLoggingBenchmarks.3_log_real_small_context_materializeOnce"), + runFunction: { iters in + var context = DefaultContext.background(logger: Logger(label: "3_log_real_context_materializeOnce", factory: StreamLogHandler.standardError)) + context.baggage[TestK1.self] = "k1-value" + context.baggage[TestK2.self] = "k2-value" + context.baggage[TestK3.self] = "k3-value" + log_materializeOnce(context: context, iters: iters) + }, + tags: [ + .logging, + ], + setUpFunction: { setUp() }, + tearDownFunction: tearDown + ), + BenchmarkInfo( + name: pad("BaggageLoggingBenchmarks.3_log_real-trace_small_context_materializeOnce"), + runFunction: { iters in + var context = DefaultContext.background(logger: Logger(label: "3_log_real_context_materializeOnce", factory: StreamLogHandler.standardError)) + context.baggage[TestK1.self] = "k1-value" + context.baggage[TestK2.self] = "k2-value" + context.baggage[TestK3.self] = "k3-value" + log_materializeOnce_trace(context: context, iters: iters) + }, + tags: [ + .logging, + ], + setUpFunction: { setUp() }, + tearDownFunction: tearDown + ), +] + +private func setUp() { + // ... +} + +private func tearDown() { + // ... +} + +// ==== ---------------------------------------------------------------------------------------------------------------- +// MARK: Benchmarks + +@inline(never) +func log_baseline(logger: Logger, iters remaining: Int) { + for _ in 0 ..< remaining { + logger.warning(message) + } +} + +@inline(never) +func log_baseline_trace(logger: Logger, iters remaining: Int) { + for _ in 0 ..< remaining { + logger.trace(message) + } +} + +@inline(never) +func log_loggerWithBaggage(logger: Logger, baggage: Baggage, iters remaining: Int) { + for _ in 0 ..< remaining { + logger.with(baggage).warning(message) + } +} + +@inline(never) +func log_throughContext(context: Context, iters remaining: Int) { + for _ in 0 ..< remaining { + context.logger.warning(message) + } +} + +@inline(never) +func log_loggerWithBaggage_trace(logger: Logger, baggage: Baggage, iters remaining: Int) { + for _ in 0 ..< remaining { + logger.with(baggage).trace(message) + } +} + +@inline(never) +func log_throughContext_trace(context: Context, iters remaining: Int) { + for _ in 0 ..< remaining { + context.logger.trace(message) + } +} + +@inline(never) +func log_materializeOnce_trace(context: Context, iters remaining: Int) { + var logger = context.logger + context.baggage.forEach { key, value in + logger[metadataKey: key.name] = "\(value)" + } + + for _ in 0 ..< remaining { + logger.trace(message) + } +} + +@inline(never) +func log_materializeOnce(context: Context, iters remaining: Int) { + var logger = context.logger + context.baggage.forEach { key, value in + logger[metadataKey: key.name] = "\(value)" + } + + for _ in 0 ..< remaining { + logger.warning(message) + } +} + +// ==== ---------------------------------------------------------------------------------------------------------------- +// MARK: Baggage Keys + +private enum TestK1: BaggageKey { + typealias Value = String +} + +private enum TestK2: BaggageKey { + typealias Value = String +} + +private enum TestK3: BaggageKey { + typealias Value = String +} + +private enum TestK4: BaggageKey { + typealias Value = String +} + +private enum TestKD1: BaggageKey { + typealias Value = [String: String] +} + +extension Baggage { + fileprivate var k1: TestK1.Value? { + get { return self[TestK1.self] } + set { self[TestK1.self] = newValue } + } + + fileprivate var k2: TestK2.Value? { + get { return self[TestK2.self] } + set { self[TestK2.self] = newValue } + } + + fileprivate var k3: TestK3.Value? { + get { return self[TestK3.self] } + set { self[TestK3.self] = newValue } + } + + fileprivate var k4: TestK4.Value? { + get { return self[TestK4.self] } + set { self[TestK4.self] = newValue } + } + + fileprivate var kd1: TestKD1.Value? { + get { return self[TestKD1.self] } + set { self[TestKD1.self] = newValue } + } +} diff --git a/Sources/BaggageContextBenchmarks/main.swift b/Sources/BaggageContextBenchmarks/main.swift index 8a9ce44..8844ba4 100644 --- a/Sources/BaggageContextBenchmarks/main.swift +++ b/Sources/BaggageContextBenchmarks/main.swift @@ -37,5 +37,6 @@ private func registerBenchmark(_ name: String, _ function: @escaping (Int) -> Vo } registerBenchmark(BaggagePassingBenchmarks) +registerBenchmark(BaggageLoggingBenchmarks) main() From a1c6cd53081f0e19463065a679517a9b5292b98d Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Fri, 18 Sep 2020 18:10:04 +0900 Subject: [PATCH 07/14] review feedback: background -> topLevel rename, as it makes more sense yet keeps the "huh?" effect of "this likely is wrong" if it appears in the middle of a codebase --- Sources/Baggage/Baggage.swift | 17 ++++++++--------- Sources/BaggageContext/Context.swift | 16 +++++++++------- .../BaggageLoggingBenchmarks.swift | 16 ++++++++-------- .../BaggagePassingBenchmarks.swift | 8 ++++---- .../BaggageContextTests.swift | 8 ++++---- .../FrameworkContextTests.swift | 2 +- Tests/BaggageTests/BaggageTests.swift | 14 +++++++------- 7 files changed, 41 insertions(+), 40 deletions(-) diff --git a/Sources/Baggage/Baggage.swift b/Sources/Baggage/Baggage.swift index f90554a..ee2f2dd 100644 --- a/Sources/Baggage/Baggage.swift +++ b/Sources/Baggage/Baggage.swift @@ -47,7 +47,7 @@ /// ## Usage /// Using a baggage container is fairly straight forward, as it boils down to using the prepared computed properties: /// -/// var context = Baggage.background +/// var context = Baggage.topLevel /// // set a new value /// context.testID = "abc" /// // retrieve a stored value @@ -70,17 +70,16 @@ public struct Baggage { private var _storage = [AnyBaggageKey: Any]() - /// Internal on purpose, please use `Baggage.TODO` or `Baggage.background` to create an "empty" context, + /// Internal on purpose, please use `Baggage.TODO` or `Baggage.topLevel` to create an "empty" context, /// which carries more meaning to other developers why an empty context was used. init() {} } extension Baggage { - /// Creates a new empty baggage, generally used for background processing tasks or an "initial" baggage to be immediately - /// populated with some values by a framework or runtime. - /// - /// Typically, this would only be called in a "top" or "background" setting, such as the main function, initialization, - /// tests, beginning of some background task or some other top-level baggage to be immediately populated with incoming request/message information. + /// Creates a new empty "top level" baggage, generally used as an "initial" baggage to immediately be populated with + /// some values by a framework or runtime. Another use case is for tasks starting in the "background" (e.g. on a timer), + /// which don't have a "request context" per se that they can pick up, and as such they have to create a "top level" + /// baggage for their work. /// /// ## Usage in frameworks and libraries /// This function is really only intended to be used frameworks and libraries, at the "top-level" where a request's, @@ -99,7 +98,7 @@ extension Baggage { /// in order to inform other developers that the lack of context passing was not done on purpose, but rather because either /// not being sure where to obtain a context from, or other framework limitations -- e.g. the outer framework not being /// baggage context aware just yet. - public static var background: Baggage { + public static var topLevel: Baggage { return Baggage() } } @@ -127,7 +126,7 @@ extension Baggage { /// - reason: Informational reason for developers, why a placeholder context was used instead of a proper one, /// - Returns: Empty "to-do" baggage context which should be eventually replaced with a carried through one, or `background`. public static func TODO(_ reason: StaticString? = "", function: String = #function, file: String = #file, line: UInt = #line) -> Baggage { - var context = Baggage.background + var context = Baggage.topLevel #if BAGGAGE_CRASH_TODOS fatalError("BAGGAGE_CRASH_TODOS: at \(file):\(line) (function \(function)), reason: \(reason)") #else diff --git a/Sources/BaggageContext/Context.swift b/Sources/BaggageContext/Context.swift index f5a711c..c92e948 100644 --- a/Sources/BaggageContext/Context.swift +++ b/Sources/BaggageContext/Context.swift @@ -100,20 +100,20 @@ public struct DefaultContext: Context { didSet { // Since someone could have completely replaced the logger (not just changed the log level), // we have to update the baggage again, since perhaps the new logger has empty metadata. - self.logger.update(previous: .background, latest: self.baggage) + self.logger.update(previous: .topLevel, latest: self.baggage) } } public init(baggage: Baggage, logger: Logger) { self.baggage = baggage self.logger = logger - self.logger.update(previous: .background, latest: baggage) + self.logger.update(previous: .topLevel, latest: baggage) } public init(context: C) where C: Context { self.baggage = context.baggage self.logger = context.logger - self.logger.update(previous: .background, latest: self.baggage) + self.logger.update(previous: .topLevel, latest: self.baggage) } } @@ -172,9 +172,11 @@ extension DefaultContext { // MARK: Context Initializers extension DefaultContext { - /// An empty baggage context intended as the "root" or "initial" baggage context background processing tasks, or as the "root" baggage context. + /// Creates a new empty "top level" default baggage context, generally used as an "initial" context to immediately be populated with + /// some values by a framework or runtime. Another use case is for tasks starting in the "background" (e.g. on a timer), + /// which don't have a "request context" per se that they can pick up, and as such they have to create a "top level" + /// baggage for their work. /// - /// It is never canceled, has no values, and has no deadline. /// It is typically used by the main function, initialization, and tests, and as the top-level Context for incoming requests. /// /// ### Usage in frameworks and libraries @@ -194,8 +196,8 @@ extension DefaultContext { /// such that other developers are informed that the lack of context was not done on purpose, but rather because either /// not being sure where to obtain a context from, or other framework limitations -- e.g. the outer framework not being /// context aware just yet. - public static func background(logger: Logger) -> DefaultContext { - return .init(baggage: .background, logger: logger) + public static func topLevel(logger: Logger) -> DefaultContext { + return .init(baggage: .topLevel, logger: logger) } } diff --git a/Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift b/Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift index 8bb92ba..9c88907 100644 --- a/Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift +++ b/Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift @@ -62,7 +62,7 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ name: pad("BaggageLoggingBenchmarks.0_log_noop_loggerWithBaggage_small"), runFunction: { iters in let logger = Logger(label: "0_log_noop_loggerWithBaggage_small", factory: { _ in SwiftLogNoOpLogHandler() }) - var baggage = Baggage.background + var baggage = Baggage.topLevel baggage[TestK1.self] = "k1-value" baggage[TestK2.self] = "k2-value" baggage[TestK3.self] = "k3-value" @@ -77,7 +77,7 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ BenchmarkInfo( name: pad("BaggageLoggingBenchmarks.0_log_noop_context_with_baggage_small"), runFunction: { iters in - var context = DefaultContext.background(logger: Logger(label: "0_log_noop_context_with_baggage_small", factory: { _ in SwiftLogNoOpLogHandler() })) + var context = DefaultContext.topLevel(logger: Logger(label: "0_log_noop_context_with_baggage_small", factory: { _ in SwiftLogNoOpLogHandler() })) context.baggage[TestK1.self] = "k1-value" context.baggage[TestK2.self] = "k2-value" context.baggage[TestK3.self] = "k3-value" @@ -125,7 +125,7 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ name: pad("BaggageLoggingBenchmarks.1_log_real_loggerWithBaggage_small"), runFunction: { iters in let logger = Logger(label: "1_log_real_loggerWithBaggage_small", factory: StreamLogHandler.standardError) - var baggage = Baggage.background + var baggage = Baggage.topLevel baggage[TestK1.self] = "k1-value" baggage[TestK2.self] = "k2-value" baggage[TestK3.self] = "k3-value" @@ -140,7 +140,7 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ BenchmarkInfo( name: pad("BaggageLoggingBenchmarks.1_log_real_context_with_baggage_small"), runFunction: { iters in - var context = DefaultContext.background(logger: Logger(label: "1_log_real_context_with_baggage_small", factory: StreamLogHandler.standardError)) + var context = DefaultContext.topLevel(logger: Logger(label: "1_log_real_context_with_baggage_small", factory: StreamLogHandler.standardError)) context.baggage[TestK1.self] = "k1-value" context.baggage[TestK2.self] = "k2-value" context.baggage[TestK3.self] = "k3-value" @@ -188,7 +188,7 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ name: pad("BaggageLoggingBenchmarks.2_log_real-trace_loggerWithBaggage_small"), runFunction: { iters in let logger = Logger(label: "2_log_real-trace_loggerWithBaggage_small", factory: StreamLogHandler.standardError) - var baggage = Baggage.background + var baggage = Baggage.topLevel baggage[TestK1.self] = "k1-value" baggage[TestK2.self] = "k2-value" baggage[TestK3.self] = "k3-value" @@ -203,7 +203,7 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ BenchmarkInfo( name: pad("BaggageLoggingBenchmarks.2_log_real-trace_context_with_baggage_small"), runFunction: { iters in - var context = DefaultContext.background(logger: Logger(label: "2_log_real-trace_context_with_baggage_small", factory: StreamLogHandler.standardError)) + var context = DefaultContext.topLevel(logger: Logger(label: "2_log_real-trace_context_with_baggage_small", factory: StreamLogHandler.standardError)) context.baggage[TestK1.self] = "k1-value" context.baggage[TestK2.self] = "k2-value" context.baggage[TestK3.self] = "k3-value" @@ -222,7 +222,7 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ BenchmarkInfo( name: pad("BaggageLoggingBenchmarks.3_log_real_small_context_materializeOnce"), runFunction: { iters in - var context = DefaultContext.background(logger: Logger(label: "3_log_real_context_materializeOnce", factory: StreamLogHandler.standardError)) + var context = DefaultContext.topLevel(logger: Logger(label: "3_log_real_context_materializeOnce", factory: StreamLogHandler.standardError)) context.baggage[TestK1.self] = "k1-value" context.baggage[TestK2.self] = "k2-value" context.baggage[TestK3.self] = "k3-value" @@ -237,7 +237,7 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ BenchmarkInfo( name: pad("BaggageLoggingBenchmarks.3_log_real-trace_small_context_materializeOnce"), runFunction: { iters in - var context = DefaultContext.background(logger: Logger(label: "3_log_real_context_materializeOnce", factory: StreamLogHandler.standardError)) + var context = DefaultContext.topLevel(logger: Logger(label: "3_log_real_context_materializeOnce", factory: StreamLogHandler.standardError)) context.baggage[TestK1.self] = "k1-value" context.baggage[TestK2.self] = "k2-value" context.baggage[TestK3.self] = "k3-value" diff --git a/Sources/BaggageContextBenchmarks/BaggagePassingBenchmarks.swift b/Sources/BaggageContextBenchmarks/BaggagePassingBenchmarks.swift index 2b55b11..c6f919a 100644 --- a/Sources/BaggageContextBenchmarks/BaggagePassingBenchmarks.swift +++ b/Sources/BaggageContextBenchmarks/BaggagePassingBenchmarks.swift @@ -22,7 +22,7 @@ public let BaggagePassingBenchmarks: [BenchmarkInfo] = [ BenchmarkInfo( name: "BaggagePassingBenchmarks.pass_async_empty_100_000 ", runFunction: { _ in - let context = Baggage.background + let context = Baggage.topLevel pass_async(context: context, times: 100_000) }, tags: [], @@ -32,7 +32,7 @@ public let BaggagePassingBenchmarks: [BenchmarkInfo] = [ BenchmarkInfo( name: "BaggagePassingBenchmarks.pass_async_smol_100_000 ", runFunction: { _ in - var context = Baggage.background + var context = Baggage.topLevel context.k1 = "one" context.k2 = "two" context.k3 = "three" @@ -46,7 +46,7 @@ public let BaggagePassingBenchmarks: [BenchmarkInfo] = [ BenchmarkInfo( name: "BaggagePassingBenchmarks.pass_async_small_nonconst_100_000", runFunction: { _ in - var context = Baggage.background + var context = Baggage.topLevel context.k1 = "\(Int.random(in: 1 ... Int.max))" context.k2 = "\(Int.random(in: 1 ... Int.max))" context.k3 = "\(Int.random(in: 1 ... Int.max))" @@ -66,7 +66,7 @@ public let BaggagePassingBenchmarks: [BenchmarkInfo] = [ BenchmarkInfo( name: "BaggagePassingBenchmarks.pass_mut_async_small_100_000 ", runFunction: { _ in - var context = Baggage.background + var context = Baggage.topLevel context.k1 = "\(Int.random(in: 1 ... Int.max))" context.k2 = "\(Int.random(in: 1 ... Int.max))" context.k3 = "\(Int.random(in: 1 ... Int.max))" diff --git a/Tests/BaggageContextTests/BaggageContextTests.swift b/Tests/BaggageContextTests/BaggageContextTests.swift index a776c6d..be70d1f 100644 --- a/Tests/BaggageContextTests/BaggageContextTests.swift +++ b/Tests/BaggageContextTests/BaggageContextTests.swift @@ -18,7 +18,7 @@ import XCTest final class BaggageContextTests: XCTestCase { func test_ExampleFrameworkContext_dumpBaggage() throws { - var baggage = Baggage.background + var baggage = Baggage.topLevel let logger = Logger(label: "TheLogger") baggage.testID = 42 @@ -43,7 +43,7 @@ final class BaggageContextTests: XCTestCase { } func test_ExampleMutableFrameworkContext_log_withBaggage() throws { - let baggage = Baggage.background + let baggage = Baggage.topLevel let logging = TestLogging() let logger = Logger(label: "TheLogger", factory: { label in logging.make(label: label) }) @@ -66,7 +66,7 @@ final class BaggageContextTests: XCTestCase { } func test_ExampleMutableFrameworkContext_log_prefersBaggageContextOverExistingLoggerMetadata() { - let baggage = Baggage.background + let baggage = Baggage.topLevel let logging = TestLogging() var logger = Logger(label: "TheLogger", factory: { label in logging.make(label: label) }) logger[metadataKey: "secondIDExplicitlyNamed"] = "set on logger" @@ -126,7 +126,7 @@ struct CoolFrameworkContext: BaggageContext.Context { return self._logger.with(self.baggage) } - var baggage: Baggage = .background + var baggage: Baggage = .topLevel // framework context defines other values as well let frameworkField: String = "" diff --git a/Tests/BaggageContextTests/FrameworkContextTests.swift b/Tests/BaggageContextTests/FrameworkContextTests.swift index 2c28be2..ca21ed5 100644 --- a/Tests/BaggageContextTests/FrameworkContextTests.swift +++ b/Tests/BaggageContextTests/FrameworkContextTests.swift @@ -44,7 +44,7 @@ final class FrameworkBaggageContextTests: XCTestCase { } private struct TestFrameworkContext: Context { - var baggage = Baggage.background + var baggage = Baggage.topLevel private var _logger = Logger(label: "test") var logger: Logger { diff --git a/Tests/BaggageTests/BaggageTests.swift b/Tests/BaggageTests/BaggageTests.swift index 867932a..6e884d8 100644 --- a/Tests/BaggageTests/BaggageTests.swift +++ b/Tests/BaggageTests/BaggageTests.swift @@ -18,7 +18,7 @@ final class BaggageTests: XCTestCase { func testSubscriptAccess() { let testID = 42 - var baggage = Baggage.background + var baggage = Baggage.topLevel XCTAssertNil(baggage[TestIDKey.self]) baggage[TestIDKey.self] = testID @@ -31,7 +31,7 @@ final class BaggageTests: XCTestCase { func testRecommendedConvenienceExtension() { let testID = 42 - var baggage = Baggage.background + var baggage = Baggage.topLevel XCTAssertNil(baggage.testID) baggage.testID = testID @@ -42,18 +42,18 @@ final class BaggageTests: XCTestCase { } func testEmptyBaggageDescription() { - XCTAssertEqual(String(describing: Baggage.background), "Baggage(keys: [])") + XCTAssertEqual(String(describing: Baggage.topLevel), "Baggage(keys: [])") } func testSingleKeyBaggageDescription() { - var baggage = Baggage.background + var baggage = Baggage.topLevel baggage.testID = 42 XCTAssertEqual(String(describing: baggage), #"Baggage(keys: ["TestIDKey"])"#) } func testMultiKeysBaggageDescription() { - var baggage = Baggage.background + var baggage = Baggage.topLevel baggage.testID = 42 baggage[SecondTestIDKey.self] = "test" @@ -80,7 +80,7 @@ final class BaggageTests: XCTestCase { } func test_todo_empty() { - let context = Baggage.background + let context = Baggage.topLevel _ = context // avoid "not used" warning // TODO: Can't work with protocols; re-consider the entire carrier approach... Context being a Baggage + Logger, and a specific type. @@ -88,7 +88,7 @@ final class BaggageTests: XCTestCase { // func take(context: BaggageContextProtocol) { // _ = context // ignore // } -// take(context: .background) +// take(context: .topLevel) } } From b983972690735a5cc86883ab84748a0b6ad29854 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Fri, 18 Sep 2020 19:03:13 +0900 Subject: [PATCH 08/14] correct the DefaultContext implementation such that value removal on metadata itself also just works --- Sources/BaggageContext/Context.swift | 88 +++++++++++---- .../BaggageContext/ContextLogHandler.swift | 40 +++++-- .../BaggageContextTests+XCTest.swift | 4 +- .../BaggageContextTests.swift | 106 ++++++++++++------ Tests/BaggageContextTests/TestLogger.swift | 4 - 5 files changed, 170 insertions(+), 72 deletions(-) diff --git a/Sources/BaggageContext/Context.swift b/Sources/BaggageContext/Context.swift index c92e948..31a1504 100644 --- a/Sources/BaggageContext/Context.swift +++ b/Sources/BaggageContext/Context.swift @@ -22,34 +22,74 @@ import Logging /// /// This allows frameworks and library authors to offer APIs which compose more easily. /// Please refer to the "Reference Implementation" notes on each of the requirements to know how to implement this protocol correctly. +/// +/// ### Implementation notes +/// It is STRONGLY encouraged that a context type should exhibit Value Semantics (i.e. be a pure `struct`, or implement +/// the Copy-on-Write pattern), in order to implement the `set` requirements of the baggage and logger effectively, +/// and also for their user's sanity, as a reference semantics context type can be very confusing to use when shared +/// between multiple threads, as often is the case in server side environments. +/// +/// It is STRONGLY encouraged to use the `DefaultContext` as inspiration for a correct implementation of a `Context`, +/// as the relationship between `Logger` and `Baggage` can be tricky to wrap your head around at first. public protocol Context { + /// Get the `Baggage` container. - var baggage: Baggage { get } + /// + /// ### Implementation notes + /// Libraries and/or frameworks which conform to this protocol with their "Framework Context" types MUST + /// ensure that a modification of the baggage is properly represented in the associated `logger`. Users expect values + /// from the baggage be visible in their log statements issued via `context.logger.info()`. + /// + /// Please refer to `DefaultContext`'s implementation for a reference implementation, + /// here a short snippet of how the baggage itself should be implemented: + /// + /// public var baggage: Baggage { + /// willSet { + /// self._logger.updateMetadata(previous: self.baggage, latest: newValue) + /// } + /// } + /// + /// #### Thread Safety + /// Implementations / MUST take care of thread-safety of modifications of the baggage. They can achieve this by such + /// context type being a pure `struct` or by implementing Copy-on-Write semantics for their type, the latter gives + /// many benefits, allowing the context to avoid being copied unless needed to (e.g. if the context type contains + /// many other values, in addition to the baggage). + var baggage: Baggage { get set } /// The `Logger` associated with this context carrier. /// /// It automatically populates the loggers metadata based on the `Baggage` associated with this context object. /// - /// ### Implementation note - /// + /// ### Implementation notes /// Libraries and/or frameworks which conform to this protocol with their "Framework Context" types, /// SHOULD implement this logger by wrapping the "raw" logger associated with `_logger.with(self.baggage)` function, /// which efficiently handles the bridging of baggage to logging metadata values. /// - /// ### Reference Implementation - /// - /// Writes to the `logger` metadata SHOULD NOT be reflected in the `baggage`, - /// however writes to the underlying `baggage` SHOULD be reflected in the `logger`. + /// If a new logger is set, it MUST populate itself with the latest (current) baggage of the context, + /// this is to ensure that even if users set a new logger (completely "fresh") here, the metadata from the baggage + /// still will properly be logged in other pieces of the application where the context might be passed to. /// - /// struct MyFrameworkContext: ContextProtocol { - /// var baggage: Baggage - /// private let _logger: Logger + /// A correct implementation might look like the following: /// - /// var logger: Logger { - /// return self._logger.with(self.baggage) - /// } + /// public var _logger: Logger + /// public var logger: Logger { + /// get { + /// return self._logger + /// } + /// set { + /// self._logger = newValue + /// // Since someone could have completely replaced the logger (not just changed the log level), + /// // we have to update the baggage again, since perhaps the new logger has empty metadata. + /// self._logger.updateMetadata(previous: .topLevel, latest: self.baggage) + /// } + /// } /// } - var logger: Logger { get } + /// + /// + /// #### Thread Safety + /// Implementations MUST ensure the thread-safety of mutating the logger. This is usually handled best by the + /// framework context itself being a Copy-on-Write type, however the exact safety mechanism is left up to the libraries. + var logger: Logger { get set } } /// A default `Context` type. @@ -92,28 +132,34 @@ public struct DefaultContext: Context { // Systems which never or rarely log will take the hit for it here. The alternative tradeoff to map lazily as `logger.with(baggage)` // is available as well, but users would have to build their own context and specifically make use of that then -- that approach // allows to not pay the mapping cost up front, but only if a log statement is made (but then again, the cost is paid every time we log something). - self.logger.update(previous: self.baggage, latest: newValue) + self._logger.updateMetadata(previous: self.baggage, latest: newValue) } } + // We need to store the logger as `_logger` in order to avoid cyclic updates triggering when baggage changes + public var _logger: Logger public var logger: Logger { - didSet { + get { + return self._logger + } + set { + self._logger = newValue // Since someone could have completely replaced the logger (not just changed the log level), // we have to update the baggage again, since perhaps the new logger has empty metadata. - self.logger.update(previous: .topLevel, latest: self.baggage) + self._logger.updateMetadata(previous: .topLevel, latest: self.baggage) } } public init(baggage: Baggage, logger: Logger) { self.baggage = baggage - self.logger = logger - self.logger.update(previous: .topLevel, latest: baggage) + self._logger = logger + self._logger.updateMetadata(previous: .topLevel, latest: baggage) } public init(context: C) where C: Context { self.baggage = context.baggage - self.logger = context.logger - self.logger.update(previous: .topLevel, latest: self.baggage) + self._logger = context.logger + self._logger.updateMetadata(previous: .topLevel, latest: self.baggage) } } diff --git a/Sources/BaggageContext/ContextLogHandler.swift b/Sources/BaggageContext/ContextLogHandler.swift index 951c951..2d7ae8e 100644 --- a/Sources/BaggageContext/ContextLogHandler.swift +++ b/Sources/BaggageContext/ContextLogHandler.swift @@ -23,6 +23,11 @@ extension Logger { /// /// The rendering of baggage values into metadata values is performed on demand, /// whenever a log statement is effective (i.e. will be logged, according to active `logLevel`). + /// + /// Note that when it is known that multiple log statements will be performed with the baggage it is preferable to + /// modify the logger's metadata by issuing `logger.updateMetadata(previous: baggage, latest: baggage)` instead. + /// + /// - SeeAlso: public func with(_ baggage: Baggage) -> Logger { return Logger( label: self.label, @@ -30,7 +35,15 @@ extension Logger { ) } - public mutating func update(previous: Baggage, latest: Baggage) { + /// Update the logger's metadata in accordance to the passed in baggage. + /// + /// Items which were previously present in the baggage but are now removed will also be removed from the logger's + /// metadata. + /// + /// - Parameters: + /// - previous: baggage which was previously used to set metadata on this logger (pass `.topLevel` if unknown or none) + /// - latest: the current, latest, state of the baggage container; all of it's loggable values will be set as the `Logger`'s metadata + public mutating func updateMetadata(previous: Baggage, latest: Baggage) { var removedKeys: Set = [] removedKeys.reserveCapacity(previous.count) previous.forEach { key, _ in @@ -38,7 +51,13 @@ extension Logger { } latest.forEach { key, value in removedKeys.remove(key) - self[metadataKey: key.name] = "\(value)" + if let convertible = value as? String { + self[metadataKey: key.name] = .string(convertible) + } else if let convertible = value as? CustomStringConvertible { + self[metadataKey: key.name] = .stringConvertible(convertible) + } else { + self[metadataKey: key.name] = .stringConvertible(BaggageValueCustomStringConvertible(value)) + } } removedKeys.forEach { removedKey in self[metadataKey: removedKey.name] = nil @@ -131,15 +150,16 @@ public struct BaggageMetadataLogHandler: LogHandler { return effectiveMetadata } - struct BaggageValueCustomStringConvertible: CustomStringConvertible { - let value: Any +} - init(_ value: Any) { - self.value = value - } +struct BaggageValueCustomStringConvertible: CustomStringConvertible { + let value: Any - var description: String { - return "\(self.value)" - } + init(_ value: Any) { + self.value = value + } + + var description: String { + return "\(self.value)" } } diff --git a/Tests/BaggageContextTests/BaggageContextTests+XCTest.swift b/Tests/BaggageContextTests/BaggageContextTests+XCTest.swift index 2cdd030..5b72ee0 100644 --- a/Tests/BaggageContextTests/BaggageContextTests+XCTest.swift +++ b/Tests/BaggageContextTests/BaggageContextTests+XCTest.swift @@ -26,8 +26,8 @@ extension BaggageContextTests { static var allTests : [(String, (BaggageContextTests) -> () throws -> Void)] { return [ ("test_ExampleFrameworkContext_dumpBaggage", test_ExampleFrameworkContext_dumpBaggage), - ("test_ExampleMutableFrameworkContext_log_withBaggage", test_ExampleMutableFrameworkContext_log_withBaggage), - ("test_ExampleMutableFrameworkContext_log_prefersBaggageContextOverExistingLoggerMetadata", test_ExampleMutableFrameworkContext_log_prefersBaggageContextOverExistingLoggerMetadata), + ("test_ExampleFrameworkContext_log_withBaggage", test_ExampleFrameworkContext_log_withBaggage), + ("test_ExampleFrameworkContext_log_prefersBaggageContextOverExistingLoggerMetadata", test_ExampleFrameworkContext_log_prefersBaggageContextOverExistingLoggerMetadata), ] } } diff --git a/Tests/BaggageContextTests/BaggageContextTests.swift b/Tests/BaggageContextTests/BaggageContextTests.swift index be70d1f..d76d2d9 100644 --- a/Tests/BaggageContextTests/BaggageContextTests.swift +++ b/Tests/BaggageContextTests/BaggageContextTests.swift @@ -42,12 +42,12 @@ final class BaggageContextTests: XCTestCase { ) } - func test_ExampleMutableFrameworkContext_log_withBaggage() throws { + func test_ExampleFrameworkContext_log_withBaggage() throws { let baggage = Baggage.topLevel let logging = TestLogging() let logger = Logger(label: "TheLogger", factory: { label in logging.make(label: label) }) - var context = ExampleMutableFrameworkContext(context: baggage, logger: logger) + var context = ExampleFrameworkContext(context: baggage, logger: logger) context.baggage.secondTestID = "value" context.baggage.testID = 42 @@ -56,6 +56,42 @@ final class BaggageContextTests: XCTestCase { context.baggage.testID = nil context.logger.warning("World") + context.baggage.secondTestID = nil + context.logger[metadataKey: "metadata"] = "on-logger" + context.logger.warning("!") + + // These implicitly exercise logger.updateMetadata + + logging.history.assertExist(level: .info, message: "Hello", metadata: [ + "TestIDKey": .stringConvertible(42), + "secondIDExplicitlyNamed": "value", + ]) + logging.history.assertExist(level: .warning, message: "World", metadata: [ + "secondIDExplicitlyNamed": "value", + ]) + logging.history.assertExist(level: .warning, message: "!", metadata: [ + "metadata": "on-logger", + ]) + } + func test_DefaultContext_log_withBaggage() throws { + let logging = TestLogging() + let logger = Logger(label: "TheLogger", factory: { label in logging.make(label: label) }) + + var context = DefaultContext.topLevel(logger: logger) + + context.baggage.secondTestID = "value" + context.baggage.testID = 42 + context.logger.info("Hello") + + context.baggage.testID = nil + context.logger.warning("World") + + context.baggage.secondTestID = nil + context.logger[metadataKey: "metadata"] = "on-logger" + context.logger.warning("!") + + // These implicitly exercise logger.updateMetadata + logging.history.assertExist(level: .info, message: "Hello", metadata: [ "TestIDKey": .stringConvertible(42), "secondIDExplicitlyNamed": "value", @@ -63,15 +99,18 @@ final class BaggageContextTests: XCTestCase { logging.history.assertExist(level: .warning, message: "World", metadata: [ "secondIDExplicitlyNamed": "value", ]) + logging.history.assertExist(level: .warning, message: "!", metadata: [ + "metadata": "on-logger", + ]) } - func test_ExampleMutableFrameworkContext_log_prefersBaggageContextOverExistingLoggerMetadata() { + func test_ExampleFrameworkContext_log_prefersBaggageContextOverExistingLoggerMetadata() { let baggage = Baggage.topLevel let logging = TestLogging() var logger = Logger(label: "TheLogger", factory: { label in logging.make(label: label) }) logger[metadataKey: "secondIDExplicitlyNamed"] = "set on logger" - var context = ExampleMutableFrameworkContext(context: baggage, logger: logger) + var context = ExampleFrameworkContext(context: baggage, logger: logger) context.baggage.secondTestID = "set on baggage" @@ -81,61 +120,58 @@ final class BaggageContextTests: XCTestCase { "secondIDExplicitlyNamed": "set on baggage", ]) } + } struct ExampleFrameworkContext: BaggageContext.Context { - let baggage: Baggage - let logger: Logger - - init(context baggage: Baggage, logger: Logger) { - self.baggage = baggage - self.logger = logger.with(self.baggage) + var baggage: Baggage { + willSet { + self._logger.updateMetadata(previous: self.baggage, latest: newValue) + } } -} - -struct ExampleMutableFrameworkContext: Context { - var baggage: Baggage - private var _logger: Logger + var _logger: Logger var logger: Logger { - return self._logger.with(self.baggage) - } - - init(context baggage: Baggage, logger: Logger) { - self.baggage = baggage - self._logger = logger - } - - subscript(key: Key.Type) -> Key.Value? { get { - return self.baggage[key] + return self._logger } set { - self.baggage[key] = newValue + self._logger = newValue + self._logger.updateMetadata(previous: self.baggage, latest: self.baggage) } } - func forEachBaggageItem(_ body: (AnyBaggageKey, Any) throws -> Void) rethrows { - return try self.baggage.forEach(body) + init(context baggage: Baggage, logger: Logger) { + self.baggage = baggage + self._logger = logger + self._logger.updateMetadata(previous: .topLevel, latest: baggage) } } struct CoolFrameworkContext: BaggageContext.Context { - private var _logger: Logger = Logger(label: "some frameworks logger") + var baggage: Baggage { + willSet { + self.logger.updateMetadata(previous: self.baggage, latest: newValue) + } + } var logger: Logger { - return self._logger.with(self.baggage) + didSet { + self.logger.updateMetadata(previous: self.baggage, latest: self.baggage) + } } - var baggage: Baggage = .topLevel - // framework context defines other values as well - let frameworkField: String = "" + let frameworkField: String // including the popular eventLoop let eventLoop: FakeEventLoop - subscript(key: Key.Type) -> Key.Value? { - return self.baggage[key] + init() { + self.baggage = .topLevel + self.logger = Logger(label: "some-framework-logger") + self.eventLoop = FakeEventLoop() + self.frameworkField = "" + self.logger.updateMetadata(previous: .topLevel, latest: self.baggage) } func forEachBaggageItem(_ body: (AnyBaggageKey, Any) throws -> Void) rethrows { diff --git a/Tests/BaggageContextTests/TestLogger.swift b/Tests/BaggageContextTests/TestLogger.swift index b15aeb7..4eceb48 100644 --- a/Tests/BaggageContextTests/TestLogger.swift +++ b/Tests/BaggageContextTests/TestLogger.swift @@ -84,11 +84,9 @@ internal struct TestLogHandler: LogHandler { public var metadata: Logger.Metadata { get { - // return self.logger.metadata return self.metadataLock.withLock { self._metadata } } set { - // self.logger.metadata = newValue self.metadataLock.withLock { self._metadata = newValue } } } @@ -96,11 +94,9 @@ internal struct TestLogHandler: LogHandler { // TODO: would be nice to delegate to local copy of logger but StdoutLogger is a reference type. why? subscript(metadataKey metadataKey: Logger.Metadata.Key) -> Logger.Metadata.Value? { get { - // return self.logger[metadataKey: metadataKey] return self.metadataLock.withLock { self._metadata[metadataKey] } } set { - // return logger[metadataKey: metadataKey] = newValue self.metadataLock.withLock { self._metadata[metadataKey] = newValue } From 0e09b91fe60b9f4e575727452531ecdb86d5e6e3 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Fri, 18 Sep 2020 19:18:50 +0900 Subject: [PATCH 09/14] review: make it more explicit that subscript[_key] is only to be used by privileged authors "those who create keys" --- Sources/Baggage/Baggage.swift | 19 +++--- Sources/Baggage/BaggageKey.swift | 4 +- Sources/BaggageContext/Context.swift | 1 - .../BaggageContext/ContextLogHandler.swift | 1 - .../BaggageLoggingBenchmarks.swift | 68 +++++++++---------- .../BaggagePassingBenchmarks.swift | 24 +++---- .../BaggageContextTests+XCTest.swift | 1 + .../BaggageContextTests.swift | 11 +-- .../FrameworkContextTests.swift | 10 +-- Tests/BaggageTests/BaggageTests.swift | 18 ++--- 10 files changed, 80 insertions(+), 77 deletions(-) diff --git a/Sources/Baggage/Baggage.swift b/Sources/Baggage/Baggage.swift index ee2f2dd..817fc5e 100644 --- a/Sources/Baggage/Baggage.swift +++ b/Sources/Baggage/Baggage.swift @@ -33,9 +33,9 @@ /// extension Baggage { /// var testID: String? { /// get { -/// self[TestIDKey.self] +/// self[_key: TestIDKey.self] /// } set { -/// self[TestIDKey.self] = newValue +/// self[_key: TestIDKey.self] = newValue /// } /// } /// } @@ -130,7 +130,7 @@ extension Baggage { #if BAGGAGE_CRASH_TODOS fatalError("BAGGAGE_CRASH_TODOS: at \(file):\(line) (function \(function)), reason: \(reason)") #else - context[TODOKey.self] = .init(file: file, line: line) + context[_key: TODOKey.self] = .init(file: file, line: line) return context #endif } @@ -145,9 +145,9 @@ extension Baggage { extension Baggage { /// Provides type-safe access to the baggage's values. + /// This API should ONLY be used inside of accessor implementations. /// - /// Rather than using this subscript directly, users SHOULD offer a convenience accessor to their values, - /// using the following pattern: + /// End users rather than using this subscript should use "accessors" the key's author MUST define, following this pattern: /// /// internal enum TestID: Baggage.Key { /// typealias Value = TestID @@ -156,17 +156,20 @@ extension Baggage { /// extension Baggage { /// var testID: TestID? { /// get { - /// self[TestIDKey.self] + /// self[_key: TestIDKey.self] /// } /// set { - /// self[TestIDKey.self] = newValue + /// self[_key: TestIDKey.self] = newValue /// } /// } /// } /// + /// This is in order to enforce a consistent style across projects and also allow for fine grained control over + /// who may set and who may get such property. Just access control to the Key type itself lacks such fidelity. + /// /// Note that specific baggage and context types MAY (and usually do), offer also a way to set baggage values, /// however in the most general case it is not required, as some frameworks may only be able to offer reading. - public subscript(_ key: Key.Type) -> Key.Value? { + public subscript(_key key: Key.Type) -> Key.Value? { get { guard let value = self._storage[AnyBaggageKey(key)] else { return nil } // safe to force-cast as this subscript is the only way to set a value. diff --git a/Sources/Baggage/BaggageKey.swift b/Sources/Baggage/BaggageKey.swift index be30ec1..a92fb3b 100644 --- a/Sources/Baggage/BaggageKey.swift +++ b/Sources/Baggage/BaggageKey.swift @@ -27,10 +27,10 @@ /// /// This is some useful property documentation. /// var testID: String? { /// get { -/// self[TestIDKey.self] +/// self[_key: TestIDKey.self] /// } /// set { -/// self[TestIDKey.self] = newValue +/// self[_key: TestIDKey.self] = newValue /// } /// } /// } diff --git a/Sources/BaggageContext/Context.swift b/Sources/BaggageContext/Context.swift index 31a1504..4f039f9 100644 --- a/Sources/BaggageContext/Context.swift +++ b/Sources/BaggageContext/Context.swift @@ -32,7 +32,6 @@ import Logging /// It is STRONGLY encouraged to use the `DefaultContext` as inspiration for a correct implementation of a `Context`, /// as the relationship between `Logger` and `Baggage` can be tricky to wrap your head around at first. public protocol Context { - /// Get the `Baggage` container. /// /// ### Implementation notes diff --git a/Sources/BaggageContext/ContextLogHandler.swift b/Sources/BaggageContext/ContextLogHandler.swift index 2d7ae8e..8b4bb6e 100644 --- a/Sources/BaggageContext/ContextLogHandler.swift +++ b/Sources/BaggageContext/ContextLogHandler.swift @@ -149,7 +149,6 @@ public struct BaggageMetadataLogHandler: LogHandler { return effectiveMetadata } - } struct BaggageValueCustomStringConvertible: CustomStringConvertible { diff --git a/Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift b/Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift index 9c88907..e9572cc 100644 --- a/Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift +++ b/Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift @@ -63,9 +63,9 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ runFunction: { iters in let logger = Logger(label: "0_log_noop_loggerWithBaggage_small", factory: { _ in SwiftLogNoOpLogHandler() }) var baggage = Baggage.topLevel - baggage[TestK1.self] = "k1-value" - baggage[TestK2.self] = "k2-value" - baggage[TestK3.self] = "k3-value" + baggage[_key: TestK1.self] = "k1-value" + baggage[_key: TestK2.self] = "k2-value" + baggage[_key: TestK3.self] = "k3-value" log_loggerWithBaggage(logger: logger, baggage: baggage, iters: iters) }, tags: [ @@ -78,9 +78,9 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ name: pad("BaggageLoggingBenchmarks.0_log_noop_context_with_baggage_small"), runFunction: { iters in var context = DefaultContext.topLevel(logger: Logger(label: "0_log_noop_context_with_baggage_small", factory: { _ in SwiftLogNoOpLogHandler() })) - context.baggage[TestK1.self] = "k1-value" - context.baggage[TestK2.self] = "k2-value" - context.baggage[TestK3.self] = "k3-value" + context.baggage[_key: TestK1.self] = "k1-value" + context.baggage[_key: TestK2.self] = "k2-value" + context.baggage[_key: TestK3.self] = "k3-value" log_throughContext(context: context, iters: iters) }, tags: [ @@ -126,9 +126,9 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ runFunction: { iters in let logger = Logger(label: "1_log_real_loggerWithBaggage_small", factory: StreamLogHandler.standardError) var baggage = Baggage.topLevel - baggage[TestK1.self] = "k1-value" - baggage[TestK2.self] = "k2-value" - baggage[TestK3.self] = "k3-value" + baggage[_key: TestK1.self] = "k1-value" + baggage[_key: TestK2.self] = "k2-value" + baggage[_key: TestK3.self] = "k3-value" log_loggerWithBaggage(logger: logger, baggage: baggage, iters: iters) }, tags: [ @@ -141,9 +141,9 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ name: pad("BaggageLoggingBenchmarks.1_log_real_context_with_baggage_small"), runFunction: { iters in var context = DefaultContext.topLevel(logger: Logger(label: "1_log_real_context_with_baggage_small", factory: StreamLogHandler.standardError)) - context.baggage[TestK1.self] = "k1-value" - context.baggage[TestK2.self] = "k2-value" - context.baggage[TestK3.self] = "k3-value" + context.baggage[_key: TestK1.self] = "k1-value" + context.baggage[_key: TestK2.self] = "k2-value" + context.baggage[_key: TestK3.self] = "k3-value" log_throughContext(context: context, iters: iters) }, tags: [ @@ -189,9 +189,9 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ runFunction: { iters in let logger = Logger(label: "2_log_real-trace_loggerWithBaggage_small", factory: StreamLogHandler.standardError) var baggage = Baggage.topLevel - baggage[TestK1.self] = "k1-value" - baggage[TestK2.self] = "k2-value" - baggage[TestK3.self] = "k3-value" + baggage[_key: TestK1.self] = "k1-value" + baggage[_key: TestK2.self] = "k2-value" + baggage[_key: TestK3.self] = "k3-value" log_loggerWithBaggage_trace(logger: logger, baggage: baggage, iters: iters) }, tags: [ @@ -204,9 +204,9 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ name: pad("BaggageLoggingBenchmarks.2_log_real-trace_context_with_baggage_small"), runFunction: { iters in var context = DefaultContext.topLevel(logger: Logger(label: "2_log_real-trace_context_with_baggage_small", factory: StreamLogHandler.standardError)) - context.baggage[TestK1.self] = "k1-value" - context.baggage[TestK2.self] = "k2-value" - context.baggage[TestK3.self] = "k3-value" + context.baggage[_key: TestK1.self] = "k1-value" + context.baggage[_key: TestK2.self] = "k2-value" + context.baggage[_key: TestK3.self] = "k3-value" log_throughContext_trace(context: context, iters: iters) }, tags: [ @@ -223,9 +223,9 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ name: pad("BaggageLoggingBenchmarks.3_log_real_small_context_materializeOnce"), runFunction: { iters in var context = DefaultContext.topLevel(logger: Logger(label: "3_log_real_context_materializeOnce", factory: StreamLogHandler.standardError)) - context.baggage[TestK1.self] = "k1-value" - context.baggage[TestK2.self] = "k2-value" - context.baggage[TestK3.self] = "k3-value" + context.baggage[_key: TestK1.self] = "k1-value" + context.baggage[_key: TestK2.self] = "k2-value" + context.baggage[_key: TestK3.self] = "k3-value" log_materializeOnce(context: context, iters: iters) }, tags: [ @@ -238,9 +238,9 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ name: pad("BaggageLoggingBenchmarks.3_log_real-trace_small_context_materializeOnce"), runFunction: { iters in var context = DefaultContext.topLevel(logger: Logger(label: "3_log_real_context_materializeOnce", factory: StreamLogHandler.standardError)) - context.baggage[TestK1.self] = "k1-value" - context.baggage[TestK2.self] = "k2-value" - context.baggage[TestK3.self] = "k3-value" + context.baggage[_key: TestK1.self] = "k1-value" + context.baggage[_key: TestK2.self] = "k2-value" + context.baggage[_key: TestK3.self] = "k3-value" log_materializeOnce_trace(context: context, iters: iters) }, tags: [ @@ -353,27 +353,27 @@ private enum TestKD1: BaggageKey { extension Baggage { fileprivate var k1: TestK1.Value? { - get { return self[TestK1.self] } - set { self[TestK1.self] = newValue } + get { return self[_key: TestK1.self] } + set { self[_key: TestK1.self] = newValue } } fileprivate var k2: TestK2.Value? { - get { return self[TestK2.self] } - set { self[TestK2.self] = newValue } + get { return self[_key: TestK2.self] } + set { self[_key: TestK2.self] = newValue } } fileprivate var k3: TestK3.Value? { - get { return self[TestK3.self] } - set { self[TestK3.self] = newValue } + get { return self[_key: TestK3.self] } + set { self[_key: TestK3.self] = newValue } } fileprivate var k4: TestK4.Value? { - get { return self[TestK4.self] } - set { self[TestK4.self] = newValue } + get { return self[_key: TestK4.self] } + set { self[_key: TestK4.self] = newValue } } fileprivate var kd1: TestKD1.Value? { - get { return self[TestKD1.self] } - set { self[TestKD1.self] = newValue } + get { return self[_key: TestKD1.self] } + set { self[_key: TestKD1.self] = newValue } } } diff --git a/Sources/BaggageContextBenchmarks/BaggagePassingBenchmarks.swift b/Sources/BaggageContextBenchmarks/BaggagePassingBenchmarks.swift index c6f919a..6e96303 100644 --- a/Sources/BaggageContextBenchmarks/BaggagePassingBenchmarks.swift +++ b/Sources/BaggageContextBenchmarks/BaggagePassingBenchmarks.swift @@ -159,32 +159,32 @@ private enum TestKD1: BaggageKey { extension Baggage { fileprivate var passCounter: TestPassCounterKey.Value { - get { return self[TestPassCounterKey.self] ?? 0 } - set { self[TestPassCounterKey.self] = newValue } + get { return self[_key: TestPassCounterKey.self] ?? 0 } + set { self[_key: TestPassCounterKey.self] = newValue } } fileprivate var k1: TestK1.Value? { - get { return self[TestK1.self] } - set { self[TestK1.self] = newValue } + get { return self[_key: TestK1.self] } + set { self[_key: TestK1.self] = newValue } } fileprivate var k2: TestK2.Value? { - get { return self[TestK2.self] } - set { self[TestK2.self] = newValue } + get { return self[_key: TestK2.self] } + set { self[_key: TestK2.self] = newValue } } fileprivate var k3: TestK3.Value? { - get { return self[TestK3.self] } - set { self[TestK3.self] = newValue } + get { return self[_key: TestK3.self] } + set { self[_key: TestK3.self] = newValue } } fileprivate var k4: TestK4.Value? { - get { return self[TestK4.self] } - set { self[TestK4.self] = newValue } + get { return self[_key: TestK4.self] } + set { self[_key: TestK4.self] = newValue } } fileprivate var kd1: TestKD1.Value? { - get { return self[TestKD1.self] } - set { self[TestKD1.self] = newValue } + get { return self[_key: TestKD1.self] } + set { self[_key: TestKD1.self] = newValue } } } diff --git a/Tests/BaggageContextTests/BaggageContextTests+XCTest.swift b/Tests/BaggageContextTests/BaggageContextTests+XCTest.swift index 5b72ee0..ab44fa0 100644 --- a/Tests/BaggageContextTests/BaggageContextTests+XCTest.swift +++ b/Tests/BaggageContextTests/BaggageContextTests+XCTest.swift @@ -27,6 +27,7 @@ extension BaggageContextTests { return [ ("test_ExampleFrameworkContext_dumpBaggage", test_ExampleFrameworkContext_dumpBaggage), ("test_ExampleFrameworkContext_log_withBaggage", test_ExampleFrameworkContext_log_withBaggage), + ("test_DefaultContext_log_withBaggage", test_DefaultContext_log_withBaggage), ("test_ExampleFrameworkContext_log_prefersBaggageContextOverExistingLoggerMetadata", test_ExampleFrameworkContext_log_prefersBaggageContextOverExistingLoggerMetadata), ] } diff --git a/Tests/BaggageContextTests/BaggageContextTests.swift b/Tests/BaggageContextTests/BaggageContextTests.swift index d76d2d9..021a7b6 100644 --- a/Tests/BaggageContextTests/BaggageContextTests.swift +++ b/Tests/BaggageContextTests/BaggageContextTests.swift @@ -73,6 +73,7 @@ final class BaggageContextTests: XCTestCase { "metadata": "on-logger", ]) } + func test_DefaultContext_log_withBaggage() throws { let logging = TestLogging() let logger = Logger(label: "TheLogger", factory: { label in logging.make(label: label) }) @@ -120,7 +121,6 @@ final class BaggageContextTests: XCTestCase { "secondIDExplicitlyNamed": "set on baggage", ]) } - } struct ExampleFrameworkContext: BaggageContext.Context { @@ -154,6 +154,7 @@ struct CoolFrameworkContext: BaggageContext.Context { self.logger.updateMetadata(previous: self.baggage, latest: newValue) } } + var logger: Logger { didSet { self.logger.updateMetadata(previous: self.baggage, latest: self.baggage) @@ -184,19 +185,19 @@ struct FakeEventLoop {} private extension Baggage { var testID: Int? { get { - return self[TestIDKey.self] + return self[_key: TestIDKey.self] } set { - self[TestIDKey.self] = newValue + self[_key: TestIDKey.self] = newValue } } var secondTestID: String? { get { - return self[SecondTestIDKey.self] + return self[_key: SecondTestIDKey.self] } set { - self[SecondTestIDKey.self] = newValue + self[_key: SecondTestIDKey.self] = newValue } } } diff --git a/Tests/BaggageContextTests/FrameworkContextTests.swift b/Tests/BaggageContextTests/FrameworkContextTests.swift index ca21ed5..831ebf2 100644 --- a/Tests/BaggageContextTests/FrameworkContextTests.swift +++ b/Tests/BaggageContextTests/FrameworkContextTests.swift @@ -21,7 +21,7 @@ final class FrameworkBaggageContextTests: XCTestCase { var context = TestFrameworkContext() // mutate baggage context directly - context.baggage[OtherKey.self] = "test" + context.baggage[_key: OtherKey.self] = "test" XCTAssertEqual(context.baggage.otherKey, "test") } @@ -64,10 +64,10 @@ private enum TestKey: Baggage.Key { extension Baggage { var testKey: Int? { get { - return self[TestKey.self] + return self[_key: TestKey.self] } set { - self[TestKey.self] = newValue + self[_key: TestKey.self] = newValue } } } @@ -79,10 +79,10 @@ private enum OtherKey: Baggage.Key { extension Baggage { var otherKey: String? { get { - return self[OtherKey.self] + return self[_key: OtherKey.self] } set { - self[OtherKey.self] = newValue + self[_key: OtherKey.self] = newValue } } } diff --git a/Tests/BaggageTests/BaggageTests.swift b/Tests/BaggageTests/BaggageTests.swift index 6e884d8..11c6d76 100644 --- a/Tests/BaggageTests/BaggageTests.swift +++ b/Tests/BaggageTests/BaggageTests.swift @@ -19,13 +19,13 @@ final class BaggageTests: XCTestCase { let testID = 42 var baggage = Baggage.topLevel - XCTAssertNil(baggage[TestIDKey.self]) + XCTAssertNil(baggage[_key: TestIDKey.self]) - baggage[TestIDKey.self] = testID - XCTAssertEqual(baggage[TestIDKey.self], testID) + baggage[_key: TestIDKey.self] = testID + XCTAssertEqual(baggage[_key: TestIDKey.self], testID) - baggage[TestIDKey.self] = nil - XCTAssertNil(baggage[TestIDKey.self]) + baggage[_key: TestIDKey.self] = nil + XCTAssertNil(baggage[_key: TestIDKey.self]) } func testRecommendedConvenienceExtension() { @@ -37,7 +37,7 @@ final class BaggageTests: XCTestCase { baggage.testID = testID XCTAssertEqual(baggage.testID, testID) - baggage[TestIDKey.self] = nil + baggage[_key: TestIDKey.self] = nil XCTAssertNil(baggage.testID) } @@ -55,7 +55,7 @@ final class BaggageTests: XCTestCase { func testMultiKeysBaggageDescription() { var baggage = Baggage.topLevel baggage.testID = 42 - baggage[SecondTestIDKey.self] = "test" + baggage[_key: SecondTestIDKey.self] = "test" let description = String(describing: baggage) XCTAssert(description.starts(with: "Baggage(keys: ["), "Was: \(description)") @@ -99,10 +99,10 @@ private enum TestIDKey: Baggage.Key { private extension Baggage { var testID: Int? { get { - return self[TestIDKey.self] + return self[_key: TestIDKey.self] } set { - self[TestIDKey.self] = newValue + self[_key: TestIDKey.self] = newValue } } } From 79c60c24a8f151cfc9ebd955ecdddebaa600ccaf Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Fri, 18 Sep 2020 19:29:25 +0900 Subject: [PATCH 10/14] syntax fix --- Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift b/Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift index e9572cc..1ed88ee 100644 --- a/Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift +++ b/Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift @@ -21,7 +21,7 @@ import Logging private let message: Logger.Message = "Hello world how are you" func pad(_ label: String) -> String { - "\(label)\(String(repeating: " ", count: max(0, 80 - label.count)))" + return "\(label)\(String(repeating: " ", count: max(0, 80 - label.count)))" } public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ From 4e766b3d53465cddfd38b71f7b9687bf8981d60f Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Wed, 23 Sep 2020 15:00:23 +0900 Subject: [PATCH 11/14] name -> name override --- Sources/Baggage/Baggage.swift | 16 ++--- Sources/Baggage/BaggageKey.swift | 32 ++++++--- .../BaggageLoggingBenchmarks.swift | 68 +++++++++---------- .../BaggagePassingBenchmarks.swift | 24 +++---- .../BaggageContextTests.swift | 8 +-- .../FrameworkContextTests.swift | 10 +-- Tests/BaggageTests/BaggageTests.swift | 35 +++------- 7 files changed, 95 insertions(+), 98 deletions(-) diff --git a/Sources/Baggage/Baggage.swift b/Sources/Baggage/Baggage.swift index 817fc5e..9a3c2de 100644 --- a/Sources/Baggage/Baggage.swift +++ b/Sources/Baggage/Baggage.swift @@ -33,9 +33,9 @@ /// extension Baggage { /// var testID: String? { /// get { -/// self[_key: TestIDKey.self] +/// self[TestIDKey.self] /// } set { -/// self[_key: TestIDKey.self] = newValue +/// self[TestIDKey.self] = newValue /// } /// } /// } @@ -130,14 +130,14 @@ extension Baggage { #if BAGGAGE_CRASH_TODOS fatalError("BAGGAGE_CRASH_TODOS: at \(file):\(line) (function \(function)), reason: \(reason)") #else - context[_key: TODOKey.self] = .init(file: file, line: line) + context[TODOKey.self] = .init(file: file, line: line) return context #endif } private enum TODOKey: BaggageKey { typealias Value = TODOLocation - static var name: String? { + static var nameOverride: String? { return "todo" } } @@ -154,12 +154,12 @@ extension Baggage { /// } /// /// extension Baggage { - /// var testID: TestID? { + /// public internal(set) var testID: TestID? { /// get { - /// self[_key: TestIDKey.self] + /// self[TestIDKey.self] /// } /// set { - /// self[_key: TestIDKey.self] = newValue + /// self[TestIDKey.self] = newValue /// } /// } /// } @@ -169,7 +169,7 @@ extension Baggage { /// /// Note that specific baggage and context types MAY (and usually do), offer also a way to set baggage values, /// however in the most general case it is not required, as some frameworks may only be able to offer reading. - public subscript(_key key: Key.Type) -> Key.Value? { + public subscript(_ key: Key.Type) -> Key.Value? { get { guard let value = self._storage[AnyBaggageKey(key)] else { return nil } // safe to force-cast as this subscript is the only way to set a value. diff --git a/Sources/Baggage/BaggageKey.swift b/Sources/Baggage/BaggageKey.swift index a92fb3b..2b96c0d 100644 --- a/Sources/Baggage/BaggageKey.swift +++ b/Sources/Baggage/BaggageKey.swift @@ -18,35 +18,45 @@ /// /// All access to baggage items should be performed through an accessor computed property defined as shown below: /// -/// private enum TestIDKey: Baggage.Key { +/// /// The Key type should be internal (or private). +/// enum TestIDKey: Baggage.Key { /// typealias Value = String -/// static var name: String? { "test-id" } +/// static var nameOverride: String? { "test-id" } /// } /// /// extension Baggage { /// /// This is some useful property documentation. -/// var testID: String? { +/// public internal(set) var testID: String? { /// get { -/// self[_key: TestIDKey.self] +/// self[TestIDKey.self] /// } /// set { -/// self[_key: TestIDKey.self] = newValue +/// self[TestIDKey.self] = newValue /// } /// } /// } +/// +/// This pattern allows library authors fine-grained control over which values may be set, and whic only get by end-users. public protocol BaggageKey { /// The type of `Value` uniquely identified by this key. associatedtype Value /// The human-readable name of this key. - /// May be used as key during serialization of the baggage item. + /// This name will be used instead of the type name when a value is printed. + /// + /// It MAY also be picked up by an instrument (from Swift Tracing) which serializes baggage items and e.g. used as + /// header name for carried metadata. Though generally speaking header names are NOT required to use the nameOverride, + /// and MAY use their well known names for header names etc, as it depends on the specific transport and instrument used. + /// + /// For example, a baggage key representing the W3C "trace-state" header may want to return "trace-state" here, + /// in order to achieve a consistent look and feel of this baggage item throughout logging and tracing systems. /// /// Defaults to `nil`. - static var name: String? { get } + static var nameOverride: String? { get } } extension BaggageKey { - public static var name: String? { return nil } + public static var nameOverride: String? { return nil } } /// A type-erased `BaggageKey` used when iterating through the `Baggage` using its `forEach` method. @@ -54,17 +64,17 @@ public struct AnyBaggageKey { /// The key's type represented erased to an `Any.Type`. public let keyType: Any.Type - private let _name: String? + private let _nameOverride: String? /// A human-readable String representation of the underlying key. /// If no explicit name has been set on the wrapped key the type name is used. public var name: String { - return self._name ?? String(describing: self.keyType.self) + return self._nameOverride ?? String(describing: self.keyType.self) } init(_ keyType: Key.Type) where Key: BaggageKey { self.keyType = keyType - self._name = keyType.name + self._nameOverride = keyType.nameOverride } } diff --git a/Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift b/Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift index 1ed88ee..136e968 100644 --- a/Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift +++ b/Sources/BaggageContextBenchmarks/BaggageLoggingBenchmarks.swift @@ -63,9 +63,9 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ runFunction: { iters in let logger = Logger(label: "0_log_noop_loggerWithBaggage_small", factory: { _ in SwiftLogNoOpLogHandler() }) var baggage = Baggage.topLevel - baggage[_key: TestK1.self] = "k1-value" - baggage[_key: TestK2.self] = "k2-value" - baggage[_key: TestK3.self] = "k3-value" + baggage[TestK1.self] = "k1-value" + baggage[TestK2.self] = "k2-value" + baggage[TestK3.self] = "k3-value" log_loggerWithBaggage(logger: logger, baggage: baggage, iters: iters) }, tags: [ @@ -78,9 +78,9 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ name: pad("BaggageLoggingBenchmarks.0_log_noop_context_with_baggage_small"), runFunction: { iters in var context = DefaultContext.topLevel(logger: Logger(label: "0_log_noop_context_with_baggage_small", factory: { _ in SwiftLogNoOpLogHandler() })) - context.baggage[_key: TestK1.self] = "k1-value" - context.baggage[_key: TestK2.self] = "k2-value" - context.baggage[_key: TestK3.self] = "k3-value" + context.baggage[TestK1.self] = "k1-value" + context.baggage[TestK2.self] = "k2-value" + context.baggage[TestK3.self] = "k3-value" log_throughContext(context: context, iters: iters) }, tags: [ @@ -126,9 +126,9 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ runFunction: { iters in let logger = Logger(label: "1_log_real_loggerWithBaggage_small", factory: StreamLogHandler.standardError) var baggage = Baggage.topLevel - baggage[_key: TestK1.self] = "k1-value" - baggage[_key: TestK2.self] = "k2-value" - baggage[_key: TestK3.self] = "k3-value" + baggage[TestK1.self] = "k1-value" + baggage[TestK2.self] = "k2-value" + baggage[TestK3.self] = "k3-value" log_loggerWithBaggage(logger: logger, baggage: baggage, iters: iters) }, tags: [ @@ -141,9 +141,9 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ name: pad("BaggageLoggingBenchmarks.1_log_real_context_with_baggage_small"), runFunction: { iters in var context = DefaultContext.topLevel(logger: Logger(label: "1_log_real_context_with_baggage_small", factory: StreamLogHandler.standardError)) - context.baggage[_key: TestK1.self] = "k1-value" - context.baggage[_key: TestK2.self] = "k2-value" - context.baggage[_key: TestK3.self] = "k3-value" + context.baggage[TestK1.self] = "k1-value" + context.baggage[TestK2.self] = "k2-value" + context.baggage[TestK3.self] = "k3-value" log_throughContext(context: context, iters: iters) }, tags: [ @@ -189,9 +189,9 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ runFunction: { iters in let logger = Logger(label: "2_log_real-trace_loggerWithBaggage_small", factory: StreamLogHandler.standardError) var baggage = Baggage.topLevel - baggage[_key: TestK1.self] = "k1-value" - baggage[_key: TestK2.self] = "k2-value" - baggage[_key: TestK3.self] = "k3-value" + baggage[TestK1.self] = "k1-value" + baggage[TestK2.self] = "k2-value" + baggage[TestK3.self] = "k3-value" log_loggerWithBaggage_trace(logger: logger, baggage: baggage, iters: iters) }, tags: [ @@ -204,9 +204,9 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ name: pad("BaggageLoggingBenchmarks.2_log_real-trace_context_with_baggage_small"), runFunction: { iters in var context = DefaultContext.topLevel(logger: Logger(label: "2_log_real-trace_context_with_baggage_small", factory: StreamLogHandler.standardError)) - context.baggage[_key: TestK1.self] = "k1-value" - context.baggage[_key: TestK2.self] = "k2-value" - context.baggage[_key: TestK3.self] = "k3-value" + context.baggage[TestK1.self] = "k1-value" + context.baggage[TestK2.self] = "k2-value" + context.baggage[TestK3.self] = "k3-value" log_throughContext_trace(context: context, iters: iters) }, tags: [ @@ -223,9 +223,9 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ name: pad("BaggageLoggingBenchmarks.3_log_real_small_context_materializeOnce"), runFunction: { iters in var context = DefaultContext.topLevel(logger: Logger(label: "3_log_real_context_materializeOnce", factory: StreamLogHandler.standardError)) - context.baggage[_key: TestK1.self] = "k1-value" - context.baggage[_key: TestK2.self] = "k2-value" - context.baggage[_key: TestK3.self] = "k3-value" + context.baggage[TestK1.self] = "k1-value" + context.baggage[TestK2.self] = "k2-value" + context.baggage[TestK3.self] = "k3-value" log_materializeOnce(context: context, iters: iters) }, tags: [ @@ -238,9 +238,9 @@ public let BaggageLoggingBenchmarks: [BenchmarkInfo] = [ name: pad("BaggageLoggingBenchmarks.3_log_real-trace_small_context_materializeOnce"), runFunction: { iters in var context = DefaultContext.topLevel(logger: Logger(label: "3_log_real_context_materializeOnce", factory: StreamLogHandler.standardError)) - context.baggage[_key: TestK1.self] = "k1-value" - context.baggage[_key: TestK2.self] = "k2-value" - context.baggage[_key: TestK3.self] = "k3-value" + context.baggage[TestK1.self] = "k1-value" + context.baggage[TestK2.self] = "k2-value" + context.baggage[TestK3.self] = "k3-value" log_materializeOnce_trace(context: context, iters: iters) }, tags: [ @@ -353,27 +353,27 @@ private enum TestKD1: BaggageKey { extension Baggage { fileprivate var k1: TestK1.Value? { - get { return self[_key: TestK1.self] } - set { self[_key: TestK1.self] = newValue } + get { return self[TestK1.self] } + set { self[TestK1.self] = newValue } } fileprivate var k2: TestK2.Value? { - get { return self[_key: TestK2.self] } - set { self[_key: TestK2.self] = newValue } + get { return self[TestK2.self] } + set { self[TestK2.self] = newValue } } fileprivate var k3: TestK3.Value? { - get { return self[_key: TestK3.self] } - set { self[_key: TestK3.self] = newValue } + get { return self[TestK3.self] } + set { self[TestK3.self] = newValue } } fileprivate var k4: TestK4.Value? { - get { return self[_key: TestK4.self] } - set { self[_key: TestK4.self] = newValue } + get { return self[TestK4.self] } + set { self[TestK4.self] = newValue } } fileprivate var kd1: TestKD1.Value? { - get { return self[_key: TestKD1.self] } - set { self[_key: TestKD1.self] = newValue } + get { return self[TestKD1.self] } + set { self[TestKD1.self] = newValue } } } diff --git a/Sources/BaggageContextBenchmarks/BaggagePassingBenchmarks.swift b/Sources/BaggageContextBenchmarks/BaggagePassingBenchmarks.swift index 6e96303..c6f919a 100644 --- a/Sources/BaggageContextBenchmarks/BaggagePassingBenchmarks.swift +++ b/Sources/BaggageContextBenchmarks/BaggagePassingBenchmarks.swift @@ -159,32 +159,32 @@ private enum TestKD1: BaggageKey { extension Baggage { fileprivate var passCounter: TestPassCounterKey.Value { - get { return self[_key: TestPassCounterKey.self] ?? 0 } - set { self[_key: TestPassCounterKey.self] = newValue } + get { return self[TestPassCounterKey.self] ?? 0 } + set { self[TestPassCounterKey.self] = newValue } } fileprivate var k1: TestK1.Value? { - get { return self[_key: TestK1.self] } - set { self[_key: TestK1.self] = newValue } + get { return self[TestK1.self] } + set { self[TestK1.self] = newValue } } fileprivate var k2: TestK2.Value? { - get { return self[_key: TestK2.self] } - set { self[_key: TestK2.self] = newValue } + get { return self[TestK2.self] } + set { self[TestK2.self] = newValue } } fileprivate var k3: TestK3.Value? { - get { return self[_key: TestK3.self] } - set { self[_key: TestK3.self] = newValue } + get { return self[TestK3.self] } + set { self[TestK3.self] = newValue } } fileprivate var k4: TestK4.Value? { - get { return self[_key: TestK4.self] } - set { self[_key: TestK4.self] = newValue } + get { return self[TestK4.self] } + set { self[TestK4.self] = newValue } } fileprivate var kd1: TestKD1.Value? { - get { return self[_key: TestKD1.self] } - set { self[_key: TestKD1.self] = newValue } + get { return self[TestKD1.self] } + set { self[TestKD1.self] = newValue } } } diff --git a/Tests/BaggageContextTests/BaggageContextTests.swift b/Tests/BaggageContextTests/BaggageContextTests.swift index 021a7b6..f05bdbb 100644 --- a/Tests/BaggageContextTests/BaggageContextTests.swift +++ b/Tests/BaggageContextTests/BaggageContextTests.swift @@ -185,19 +185,19 @@ struct FakeEventLoop {} private extension Baggage { var testID: Int? { get { - return self[_key: TestIDKey.self] + return self[TestIDKey.self] } set { - self[_key: TestIDKey.self] = newValue + self[TestIDKey.self] = newValue } } var secondTestID: String? { get { - return self[_key: SecondTestIDKey.self] + return self[SecondTestIDKey.self] } set { - self[_key: SecondTestIDKey.self] = newValue + self[SecondTestIDKey.self] = newValue } } } diff --git a/Tests/BaggageContextTests/FrameworkContextTests.swift b/Tests/BaggageContextTests/FrameworkContextTests.swift index 831ebf2..ca21ed5 100644 --- a/Tests/BaggageContextTests/FrameworkContextTests.swift +++ b/Tests/BaggageContextTests/FrameworkContextTests.swift @@ -21,7 +21,7 @@ final class FrameworkBaggageContextTests: XCTestCase { var context = TestFrameworkContext() // mutate baggage context directly - context.baggage[_key: OtherKey.self] = "test" + context.baggage[OtherKey.self] = "test" XCTAssertEqual(context.baggage.otherKey, "test") } @@ -64,10 +64,10 @@ private enum TestKey: Baggage.Key { extension Baggage { var testKey: Int? { get { - return self[_key: TestKey.self] + return self[TestKey.self] } set { - self[_key: TestKey.self] = newValue + self[TestKey.self] = newValue } } } @@ -79,10 +79,10 @@ private enum OtherKey: Baggage.Key { extension Baggage { var otherKey: String? { get { - return self[_key: OtherKey.self] + return self[OtherKey.self] } set { - self[_key: OtherKey.self] = newValue + self[OtherKey.self] = newValue } } } diff --git a/Tests/BaggageTests/BaggageTests.swift b/Tests/BaggageTests/BaggageTests.swift index 11c6d76..3dcc013 100644 --- a/Tests/BaggageTests/BaggageTests.swift +++ b/Tests/BaggageTests/BaggageTests.swift @@ -19,13 +19,13 @@ final class BaggageTests: XCTestCase { let testID = 42 var baggage = Baggage.topLevel - XCTAssertNil(baggage[_key: TestIDKey.self]) + XCTAssertNil(baggage[TestIDKey.self]) - baggage[_key: TestIDKey.self] = testID - XCTAssertEqual(baggage[_key: TestIDKey.self], testID) + baggage[TestIDKey.self] = testID + XCTAssertEqual(baggage[TestIDKey.self], testID) - baggage[_key: TestIDKey.self] = nil - XCTAssertNil(baggage[_key: TestIDKey.self]) + baggage[TestIDKey.self] = nil + XCTAssertNil(baggage[TestIDKey.self]) } func testRecommendedConvenienceExtension() { @@ -37,7 +37,7 @@ final class BaggageTests: XCTestCase { baggage.testID = testID XCTAssertEqual(baggage.testID, testID) - baggage[_key: TestIDKey.self] = nil + baggage[TestIDKey.self] = nil XCTAssertNil(baggage.testID) } @@ -55,7 +55,7 @@ final class BaggageTests: XCTestCase { func testMultiKeysBaggageDescription() { var baggage = Baggage.topLevel baggage.testID = 42 - baggage[_key: SecondTestIDKey.self] = "test" + baggage[SecondTestIDKey.self] = "test" let description = String(describing: baggage) XCTAssert(description.starts(with: "Baggage(keys: ["), "Was: \(description)") @@ -71,24 +71,11 @@ final class BaggageTests: XCTestCase { // the to-do context can be used to record intentions for why a context could not be passed through let context = Baggage.TODO("#1245 Some other library should be adjusted to pass us context") _ = context // avoid "not used" warning - - // TODO: Can't work with protocols; re-consider the entire carrier approach... Context being a Baggage + Logger, and a specific type. -// func take(context: BaggageContextProtocol) { -// _ = context // ignore -// } -// take(context: .TODO("pass from request instead")) } - func test_todo_empty() { + func test_topLevel() { let context = Baggage.topLevel _ = context // avoid "not used" warning - - // TODO: Can't work with protocols; re-consider the entire carrier approach... Context being a Baggage + Logger, and a specific type. - // static member 'empty' cannot be used on protocol metatype 'BaggageContextProtocol.Protocol' -// func take(context: BaggageContextProtocol) { -// _ = context // ignore -// } -// take(context: .topLevel) } } @@ -97,12 +84,12 @@ private enum TestIDKey: Baggage.Key { } private extension Baggage { - var testID: Int? { + public internal(set) var testID: Int? { get { - return self[_key: TestIDKey.self] + return self[TestIDKey.self] } set { - self[_key: TestIDKey.self] = newValue + self[TestIDKey.self] = newValue } } } From d5c1316dc24a6ce0314be4ac70c8e97470fe2944 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Thu, 24 Sep 2020 00:37:38 +0900 Subject: [PATCH 12/14] added missing withlogger --- Sources/BaggageContext/Context.swift | 10 ++++++++++ Tests/BaggageTests/BaggageTests+XCTest.swift | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/Sources/BaggageContext/Context.swift b/Sources/BaggageContext/Context.swift index 4f039f9..03c5ee3 100644 --- a/Sources/BaggageContext/Context.swift +++ b/Sources/BaggageContext/Context.swift @@ -166,6 +166,16 @@ public struct DefaultContext: Context { // MARK: `with...` functions extension DefaultContext { + /// Fluent API allowing for modification of underlying logger when passing the context to other functions. + /// + /// - Parameter logger: Logger that should replace the underlying logger of this context. + /// - Returns: new context, with the passed in `logger` + public func withLogger(_ logger: Logger) -> DefaultContext { + var copy = self + copy.logger = logger + return copy + } + /// Fluent API allowing for modification of underlying logger when passing the context to other functions. /// /// - Parameter logger: Logger that should replace the underlying logger of this context. diff --git a/Tests/BaggageTests/BaggageTests+XCTest.swift b/Tests/BaggageTests/BaggageTests+XCTest.swift index 4b53bde..73af853 100644 --- a/Tests/BaggageTests/BaggageTests+XCTest.swift +++ b/Tests/BaggageTests/BaggageTests+XCTest.swift @@ -31,7 +31,7 @@ extension BaggageTests { ("testSingleKeyBaggageDescription", testSingleKeyBaggageDescription), ("testMultiKeysBaggageDescription", testMultiKeysBaggageDescription), ("test_todo_context", test_todo_context), - ("test_todo_empty", test_todo_empty), + ("test_topLevel", test_topLevel), ] } } From 2e7a9ecb3c456d13bb82e75582a90f6966701fed Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Thu, 24 Sep 2020 00:47:48 +0900 Subject: [PATCH 13/14] nameOverride typo --- Package.resolved | 16 ++++++++++++++++ .../BaggageContextTests.swift | 2 +- Tests/BaggageTests/BaggageTests.swift | 4 ++-- 3 files changed, 19 insertions(+), 3 deletions(-) create mode 100644 Package.resolved diff --git a/Package.resolved b/Package.resolved new file mode 100644 index 0000000..36ab510 --- /dev/null +++ b/Package.resolved @@ -0,0 +1,16 @@ +{ + "object": { + "pins": [ + { + "package": "swift-log", + "repositoryURL": "https://github.com/apple/swift-log.git", + "state": { + "branch": null, + "revision": "173f567a2dfec11d74588eea82cecea555bdc0bc", + "version": "1.4.0" + } + } + ] + }, + "version": 1 +} diff --git a/Tests/BaggageContextTests/BaggageContextTests.swift b/Tests/BaggageContextTests/BaggageContextTests.swift index f05bdbb..b6e12f1 100644 --- a/Tests/BaggageContextTests/BaggageContextTests.swift +++ b/Tests/BaggageContextTests/BaggageContextTests.swift @@ -209,5 +209,5 @@ private enum TestIDKey: Baggage.Key { private enum SecondTestIDKey: Baggage.Key { typealias Value = String - static let name: String? = "secondIDExplicitlyNamed" + static let nameOverride: String? = "secondIDExplicitlyNamed" } diff --git a/Tests/BaggageTests/BaggageTests.swift b/Tests/BaggageTests/BaggageTests.swift index 3dcc013..ae91c9c 100644 --- a/Tests/BaggageTests/BaggageTests.swift +++ b/Tests/BaggageTests/BaggageTests.swift @@ -84,7 +84,7 @@ private enum TestIDKey: Baggage.Key { } private extension Baggage { - public internal(set) var testID: Int? { + var testID: Int? { get { return self[TestIDKey.self] } @@ -97,5 +97,5 @@ private extension Baggage { private enum SecondTestIDKey: Baggage.Key { typealias Value = String - static let name: String? = "ExplicitKeyName" + static let nameOverride: String? = "ExplicitKeyName" } From 416dc0569e703196103dcf0329d4338da2d533d0 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Wed, 30 Sep 2020 16:17:34 +0900 Subject: [PATCH 14/14] amend comment --- Sources/Baggage/Baggage.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/Baggage/Baggage.swift b/Sources/Baggage/Baggage.swift index 9a3c2de..c81f749 100644 --- a/Sources/Baggage/Baggage.swift +++ b/Sources/Baggage/Baggage.swift @@ -47,13 +47,13 @@ /// ## Usage /// Using a baggage container is fairly straight forward, as it boils down to using the prepared computed properties: /// -/// var context = Baggage.topLevel +/// var baggage = Baggage.topLevel /// // set a new value -/// context.testID = "abc" +/// baggage.testID = "abc" /// // retrieve a stored value -/// let testID = context.testID ?? "default" +/// let testID = baggage.testID ?? "default" /// // remove a stored value -/// context.testIDKey = nil +/// baggage.testIDKey = nil /// /// Note that normally a baggage should not be "created" ad-hoc by user code, but rather it should be passed to it from /// a runtime. For example, when working in an HTTP server framework, it is most likely that the baggage is already passed