Make logged metadata a little more readable (#201)

* The usual package updates. Bump Swift min to 5.8 * Fix a couple Sendable warnings * Make the "without quotes" part of metadata logging more true than before by descending recursively into dictionary and array metadata. * Fix logging tests so that the logged line number is deterministic despite changes to the file. * Fix typo, thanks @weissi!
vapor · May 30, 2024 · 9f7932f · 9f7932f
1 parent 7cf58ad
commit 9f7932f
Show file tree

Hide file tree

Showing 9 changed files with 66 additions and 42 deletions.
diff --git a/Package.swift b/Package.swift
@@ -1,4 +1,4 @@
-// swift-tools-version:5.7
+// swift-tools-version:5.8
 import PackageDescription
 
 let package = Package(
@@ -16,57 +16,71 @@ let package = Package(
     ],
     dependencies: [
         .package(url: "https://github.com/apple/swift-log.git", from: "1.5.3"),
-        .package(url: "https://github.com/apple/swift-nio.git", from: "2.56.0"),
+        .package(url: "https://github.com/apple/swift-nio.git", from: "2.62.0"),
     ],
     targets: [
         .target(
             name: "ConsoleKit",
             dependencies: [
                 .target(name: "ConsoleKitCommands"),
                 .target(name: "ConsoleKitTerminal"),
-            ]
+            ],
+            swiftSettings: swiftSettings
         ),
         .target(
             name: "ConsoleKitCommands",
             dependencies: [
                 .product(name: "Logging", package: "swift-log"),
                 .product(name: "NIOConcurrencyHelpers", package: "swift-nio"),
                 .target(name: "ConsoleKitTerminal"),
-            ]
+            ],
+            swiftSettings: swiftSettings
         ),
         .target(
             name: "ConsoleKitTerminal",
             dependencies: [
                 .product(name: "Logging", package: "swift-log"),
                 .product(name: "NIOConcurrencyHelpers", package: "swift-nio"),
-            ]
+            ],
+            swiftSettings: swiftSettings
         ),
         .testTarget(
             name: "ConsoleKitTests",
-            dependencies: [.target(name: "ConsoleKit")]
+            dependencies: [.target(name: "ConsoleKit")],
+            swiftSettings: swiftSettings
         ),
         .testTarget(
             name: "AsyncConsoleKitTests",
-            dependencies: [.target(name: "ConsoleKit")]
+            dependencies: [.target(name: "ConsoleKit")],
+            swiftSettings: swiftSettings
         ),
         .testTarget(
             name: "ConsoleKitPerformanceTests",
-            dependencies: [.target(name: "ConsoleKit")]
+            dependencies: [.target(name: "ConsoleKit")],
+            swiftSettings: swiftSettings
         ),
         .executableTarget(
             name: "ConsoleKitExample",
-            dependencies: [.target(name: "ConsoleKit")]
+            dependencies: [.target(name: "ConsoleKit")],
+            swiftSettings: swiftSettings
         ),
         .executableTarget(
             name: "ConsoleKitAsyncExample",
-            dependencies: [.target(name: "ConsoleKit")]
+            dependencies: [.target(name: "ConsoleKit")],
+            swiftSettings: swiftSettings
         ),
         .executableTarget(
             name: "ConsoleLoggerExample",
             dependencies: [
                 .target(name: "ConsoleKit"),
                 .product(name: "Logging", package: "swift-log"),
-            ]
+            ],
+            swiftSettings: swiftSettings
         ),
     ]
 )
+
+var swiftSettings: [SwiftSetting] { [
+    .enableUpcomingFeature("ForwardTrailingClosures"),
+    .enableUpcomingFeature("ConciseMagicFile"),
+] }
diff --git a/[email protected] b/[email protected]
@@ -1,14 +1,6 @@
 // swift-tools-version:5.9
 import PackageDescription
 
-let swiftSettings: [PackageDescription.SwiftSetting] = [
-    .enableExperimentalFeature("StrictConcurrency=complete"),
-    .enableUpcomingFeature("ExistentialAny"),
-    .enableUpcomingFeature("ForwardTrailingClosures"),
-    .enableUpcomingFeature("ConciseMagicFile"),
-    .enableUpcomingFeature("DisableOutwardActorInference"),
-]
-
 let package = Package(
     name: "console-kit",
     platforms: [
@@ -87,3 +79,11 @@ let package = Package(
         ),
     ]
 )
+
+var swiftSettings: [SwiftSetting] { [
+    .enableUpcomingFeature("ExistentialAny"),
+    .enableUpcomingFeature("ForwardTrailingClosures"),
+    .enableUpcomingFeature("ConciseMagicFile"),
+    .enableUpcomingFeature("DisableOutwardActorInference"),
+    .enableExperimentalFeature("StrictConcurrency=complete"),
+] }
diff --git a/README.md b/README.md
@@ -9,8 +9,8 @@
 <a href="https://docs.vapor.codes/4.0/"><img src="https://design.vapor.codes/images/readthedocs.svg" alt="Documentation"></a>
 <a href="https://discord.gg/vapor"><img src="https://design.vapor.codes/images/discordchat.svg" alt="Team Chat"></a>
 <a href="LICENSE"><img src="https://design.vapor.codes/images/mitlicense.svg" alt="MIT License"></a>
-<a href="https://github.com/vapor/console-kit/actions/workflows/test.yml"><img src="https://img.shields.io/github/actions/workflow/status/vapor/console-kit/test.yml?event=push&style=plastic&logo=github&label=test&logoColor=%23ccc" alt="Continuous Integration"></a>
-<a href="https://codecov.io/gh/vapor/console-kit"><img src="https://img.shields.io/codecov/c/gh/vapor/console-kit?style=plastic&logo=codecov&label=Codecov&token=FroD9hgbSC"></a>
-<a href="https://swift.org"><img src="https://design.vapor.codes/images/swift57up.svg" alt="Swift 5.7+"></a>
+<a href="https://github.com/vapor/console-kit/actions/workflows/test.yml"><img src="https://img.shields.io/github/actions/workflow/status/vapor/console-kit/test.yml?event=push&style=plastic&logo=github&label=tests&logoColor=%23ccc" alt="Continuous Integration"></a>
+<a href="https://codecov.io/gh/vapor/console-kit"><img src="https://img.shields.io/codecov/c/gh/vapor/console-kit?style=plastic&logo=codecov&label=codecov&token=FroD9hgbSC"></a>
+<a href="https://swift.org"><img src="https://design.vapor.codes/images/swift58up.svg" alt="Swift 5.8+"></a>
 </p>
 <br>
diff --git a/Sources/ConsoleKit/Docs.docc/theme-settings.json b/Sources/ConsoleKit/Docs.docc/theme-settings.json
@@ -1,6 +1,6 @@
 {
   "theme": {
-    "aside": { "border-radius": "6px", "border-style": "double", "border-width": "3px" },
+    "aside": { "border-radius": "16px", "border-style": "double", "border-width": "3px" },
     "border-radius": "0",
     "button": { "border-radius": "16px", "border-width": "1px", "border-style": "solid" },
     "code":   { "border-radius": "16px", "border-width": "1px", "border-style": "solid" },

diff --git a/Sources/ConsoleKitCommands/Docs.docc/theme-settings.json b/Sources/ConsoleKitCommands/Docs.docc/theme-settings.json
@@ -1,6 +1,6 @@
 {
   "theme": {
-    "aside": { "border-radius": "6px", "border-style": "double", "border-width": "3px" },
+    "aside": { "border-radius": "16px", "border-style": "double", "border-width": "3px" },
     "border-radius": "0",
     "button": { "border-radius": "16px", "border-width": "1px", "border-style": "solid" },
     "code":   { "border-radius": "16px", "border-width": "1px", "border-style": "solid" },

diff --git a/Sources/ConsoleKitTerminal/Docs.docc/theme-settings.json b/Sources/ConsoleKitTerminal/Docs.docc/theme-settings.json
@@ -1,6 +1,6 @@
 {
   "theme": {
-    "aside": { "border-radius": "6px", "border-style": "double", "border-width": "3px" },
+    "aside": { "border-radius": "16px", "border-style": "double", "border-width": "3px" },
     "border-radius": "0",
     "button": { "border-radius": "16px", "border-width": "1px", "border-style": "solid" },
     "code":   { "border-radius": "16px", "border-width": "1px", "border-style": "solid" },

diff --git a/Sources/ConsoleKitTerminal/Terminal/readpassphrase_linux.swift b/Sources/ConsoleKitTerminal/Terminal/readpassphrase_linux.swift
@@ -119,7 +119,7 @@ fileprivate let linux_readpassphrase_signos: VeryUnsafeMutableSigAtomicBufferPoi
 /// deliberately. Swift has no other model for doing this kind of thing yet.
 ///
 /// If you think you want to use this for something, you're wrong.
-fileprivate struct VeryUnsafeMutableSigAtomicBufferPointer {
+fileprivate struct VeryUnsafeMutableSigAtomicBufferPointer: @unchecked Sendable {
     let capacity: Int
     let baseAddress: UnsafeMutablePointer<sig_atomic_t>
 
@@ -139,11 +139,7 @@ fileprivate struct VeryUnsafeMutableSigAtomicBufferPointer {
     }
 
     func reset() {
-#if swift(<5.8)
-        self.baseAddress.assign(repeating: 0, count: self.capacity)
-#else
         self.baseAddress.update(repeating: 0, count: self.capacity)
-#endif
     }
 }
 #endif

diff --git a/Sources/ConsoleKitTerminal/Utilities/LoggerFragment.swift b/Sources/ConsoleKitTerminal/Utilities/LoggerFragment.swift
@@ -327,11 +327,22 @@ public struct TimestampFragment<S: TimestampSource>: LoggerFragment {
     }
 }
 
+private extension Logger.MetadataValue {
+    var descriptionWithoutExcessQuotes: String {
+        switch self {
+        case .array(let array): return "[\(array.map(\.descriptionWithoutExcessQuotes).joined(separator: ", "))]"
+        case .dictionary(let dict): return "[\(dict.map { "\($0): \($1.descriptionWithoutExcessQuotes)" }.joined(separator: ", "))]"
+        case .string(let str): return str
+        case .stringConvertible(let conv): return "\(conv)"
+        }
+    }
+}
+
 private extension Logger.Metadata {
     var sortedDescriptionWithoutQuotes: String {
         let contents = Array(self)
             .sorted(by: { $0.0 < $1.0 })
-            .map { "\($0.description): \($1)" }
+            .map { "\($0): \($1.descriptionWithoutExcessQuotes)" }
             .joined(separator: ", ")
         return "[\(contents)]"
     }

diff --git a/Tests/ConsoleKitTests/LoggingTests.swift b/Tests/ConsoleKitTests/LoggingTests.swift
@@ -63,6 +63,9 @@ final class ConsoleLoggerTests: XCTestCase {
 
         logger.info("info", metadata: ["meta1": "overridden"])
         XCTAssertLog(console, .info, "info [meta1: overridden]")
+
+        logger.info("info", metadata: ["meta1": "test1", "meta2": .stringConvertible(CommandError.missingCommand), "meta3": ["hello", "wor\"ld"], "meta4": ["hello": "wor\"ld"]])
+        XCTAssertLog(console, .info, #"info [meta1: test1, meta2: Missing command, meta3: [hello, wor"ld], meta4: [hello: wor"ld]]"#)
     }
 
     func testSourceLocation() {
@@ -71,8 +74,8 @@ final class ConsoleLoggerTests: XCTestCase {
             ConsoleLogger(label: label, console: console, level: .debug)
         }
 
-        logger.debug("debug")
-        XCTAssertLog(console, .debug, "debug (ConsoleKitTests/LoggingTests.swift:74)")
+        logger.debug("debug", line: 1)
+        XCTAssertLog(console, .debug, "debug (ConsoleKitTests/LoggingTests.swift:1)")
     }
 
     func testMetadataProviders() {
@@ -89,15 +92,15 @@ final class ConsoleLoggerTests: XCTestCase {
         }
 
         TraceNamespace.$simpleTraceID.withValue("1234-5678") {
-            logger.debug("debug")
+            logger.debug("debug", line: 1)
         }
-        XCTAssertLog(console, .debug, "debug [simple-trace-id: 1234-5678] (ConsoleKitTests/LoggingTests.swift:92)")
+        XCTAssertLog(console, .debug, "debug [simple-trace-id: 1234-5678] (ConsoleKitTests/LoggingTests.swift:1)")
     }
 
     func testTimestampFragment() {
         let console = TestConsole()
 
-        struct ConstantTimestampSource: TimestampSource {
+        struct ConstantTimestampSource: TimestampSource, @unchecked Sendable {
             let time: tm
 
             func now() -> tm {
@@ -121,7 +124,7 @@ final class ConsoleLoggerTests: XCTestCase {
             )
         }
 
-        logger.info("logged")
+        logger.info("logged", line: 1)
 
         var logged = console.testOutputQueue.first!
         let expect = "2000-06-04T03:02:01"
@@ -131,7 +134,7 @@ final class ConsoleLoggerTests: XCTestCase {
         // Remove the timezone, since there doesn't appear to be a good way to mock it with strftime.
         while logged.removeFirst() != " " { }
 
-        XCTAssertEqual(logged, "[ \(Logger.Level.info.name) ] logged (ConsoleKitTests/LoggingTests.swift:124)\n")
+        XCTAssertEqual(logged, "[ \(Logger.Level.info.name) ] logged (ConsoleKitTests/LoggingTests.swift:1)\n")
     }
 
     func testSourceFragment() {
@@ -145,14 +148,14 @@ final class ConsoleLoggerTests: XCTestCase {
             )
         }
 
-        logger.info("logged")
+        logger.info("logged", line: 1)
 
-        XCTAssertEqual(console.testOutputQueue.first, "ConsoleKitTests [ \(Logger.Level.info.name) ] logged (ConsoleKitTests/LoggingTests.swift:148)\n")
+        XCTAssertEqual(console.testOutputQueue.first, "ConsoleKitTests [ \(Logger.Level.info.name) ] logged (ConsoleKitTests/LoggingTests.swift:1)\n")
     }
 }
 
-private func XCTAssertLog(_ console: TestConsole, _ level: Logger.Level, _ message: String, file: StaticString = #file, line: UInt = #line) {
-    XCTAssertEqual(console.testOutputQueue.first, "[ \(level.name) ] \(message)\n", file: (file), line: line)
+private func XCTAssertLog(_ console: TestConsole, _ level: Logger.Level, _ message: String, file: StaticString = #filePath, line: UInt = #line) {
+    XCTAssertEqual(console.testOutputQueue.first ?? "", "[ \(level.name) ] \(message)\n", file: file, line: line)
 }
 
 enum TraceNamespace {