Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Make logged metadata a little more readable #201

Merged
merged 5 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 25 additions & 11 deletions Package.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version:5.7
// swift-tools-version:5.8
import PackageDescription

let package = Package(
Expand All @@ -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"),
] }
Comment on lines +83 to +86

Choose a reason for hiding this comment

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

Mostly curiosity, is there a reason we are opting for a computed property here rather than a constant?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because it lets me put the settings at the bottom of the file instead of the top (and I must admit it was @MahdiBM who came up with that).

16 changes: 8 additions & 8 deletions [email protected]
Original file line number Diff line number Diff line change
@@ -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: [
Expand Down Expand Up @@ -87,3 +79,11 @@ let package = Package(
),
]
)

var swiftSettings: [SwiftSetting] { [
.enableUpcomingFeature("ExistentialAny"),
.enableUpcomingFeature("ForwardTrailingClosures"),
.enableUpcomingFeature("ConciseMagicFile"),
.enableUpcomingFeature("DisableOutwardActorInference"),
.enableExperimentalFeature("StrictConcurrency=complete"),
] }
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>
2 changes: 1 addition & 1 deletion Sources/ConsoleKit/Docs.docc/theme-settings.json
Original file line number Diff line number Diff line change
@@ -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" },
Expand Down
2 changes: 1 addition & 1 deletion Sources/ConsoleKitCommands/Docs.docc/theme-settings.json
Original file line number Diff line number Diff line change
@@ -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" },
Expand Down
2 changes: 1 addition & 1 deletion Sources/ConsoleKitTerminal/Docs.docc/theme-settings.json
Original file line number Diff line number Diff line change
@@ -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" },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand All @@ -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
Expand Down
13 changes: 12 additions & 1 deletion Sources/ConsoleKitTerminal/Utilities/LoggerFragment.swift
Original file line number Diff line number Diff line change
Expand Up @@ -327,11 +327,22 @@ public struct TimestampFragment<S: TimestampSource>: LoggerFragment {
}
}

private extension Logger.MetadataValue {
var descriptionWithoutExcessQuotes: String {
Copy link
Member

Choose a reason for hiding this comment

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

😂

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)]"
}
Expand Down
25 changes: 14 additions & 11 deletions Tests/ConsoleKitTests/LoggingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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() {
Expand All @@ -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 {
Expand All @@ -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"
Expand All @@ -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() {
Expand All @@ -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 {
Expand Down