Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 5 additions & 0 deletions Sources/InMemoryLogging/InMemoryLogHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,11 @@ public struct InMemoryLogHandler: LogHandler {
mergedMetadata = mergedMetadata.merging(self.metadata) { $1 }
// ..merge in metadata from this log call, overwriting existing keys
mergedMetadata = mergedMetadata.merging(event.metadata ?? [:]) { $1 }
// ..merge in error as metadata, overwriting existing keys
Comment thread
samuelmurray marked this conversation as resolved.
Outdated
if let error = event.error {
mergedMetadata["error.message"] = "\(error)"
mergedMetadata["error.type"] = "\(String(describing: type(of: error)))"
}
Comment thread
kukushechkin marked this conversation as resolved.
Outdated

self.logStore.append(level: event.level, message: event.message, metadata: mergedMetadata)
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/Logging/Docs.docc/Proposals/SLG-0003.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Add standardized way of attaching data from `Error` instances to log entries.

- Proposal: SLG-0003
- Author(s): [Samuel Murray](https://github.com/samuelmurray)
- Status: **Ready for Implementation**
- Status: **Approved**
- Issue: [apple/swift-log#291](https://github.com/apple/swift-log/issues/291)
- Implementation:
- [apple/swift-log#425](https://github.com/apple/swift-log/pull/425)
Expand Down
29 changes: 29 additions & 0 deletions Sources/Logging/LogEvent.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ public struct LogEvent: Sendable {
/// The message of this event.
public var message: Logger.Message

/// The error associated with this event, if any.
public var error: (any Error)?

/// The metadata associated with this event, if any.
public var metadata: Logger.Metadata? {
get { self._metadata }
Expand Down Expand Up @@ -75,9 +78,35 @@ public struct LogEvent: Sendable {
file: String,
function: String,
line: UInt
) {
self.init(level: level, message: message, error: nil, metadata: metadata, source: source, file: file, function: function, line: line)
}

/// Creates a new log event.
///
/// - Parameters:
/// - level: The log level of this event.
/// - message: The message of this event.
/// - error: The error associated with this event, if any.
/// - metadata: The metadata associated with this event, if any.
/// - source: The source where this log event originated. When `nil`, ``source`` is derived lazily
/// from ``file`` on first access so handlers that never read it pay no allocation cost.
/// - file: The file this log event originates from.
/// - function: The function this log event originates from.
/// - line: The line this log event originates from.
public init(
level: Logger.Level,
message: Logger.Message,
error: (any Error)?,
metadata: Logger.Metadata?,
source: String?,
file: String,
function: String,
line: UInt
) {
self.level = level
self.message = message
self.error = error
self._metadata = metadata
self._source = source
self.file = file
Expand Down
281 changes: 272 additions & 9 deletions Sources/Logging/Logging.swift

Large diffs are not rendered by default.

21 changes: 21 additions & 0 deletions Tests/InMemoryLoggingTests/InMemoryLogHandlerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,23 @@ struct InMemoryLogHandlerTests {
)
}

@Test
func errorFromLogEndsUpInEntryAsMetadata() {
let (logHandler, logger) = self.makeTestLogger()
let error = TestError.boom
logger.info("hello", error: error)

#expect(
logHandler.entries == [
InMemoryLogHandler.Entry(
level: .info,
message: "hello",
metadata: ["error.message": "\(error)", "error.type": "\(String(describing: type(of: error)))"]
Comment thread
kukushechkin marked this conversation as resolved.
Outdated
)
]
)
}

@Test
func clear() {
let (logHandler, logger) = self.makeTestLogger()
Expand All @@ -95,4 +112,8 @@ struct InMemoryLogHandlerTests {
)
return (logHandler, logger)
}

enum TestError: Error {
case boom
}
}
53 changes: 51 additions & 2 deletions Tests/LoggingTests/LoggingTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,19 @@ struct LoggingTest {
#expect(logger2.handler is CustomHandler, "expected custom log handler")
}

@Test func errorParameter() {
let testLogging = TestLogging()

let logger = Logger(
label: "\(#function)",
factory: {
testLogging.make(label: $0)
}
)
logger.log(level: .info, "hello world!", error: TestError.boom)
testLogging.history.assertExist(level: .info, message: "hello world!", error: TestError.boom)
}

@Test func allLogLevelsExceptCriticalCanBeBlocked() {
let testLogging = TestLogging()

Expand Down Expand Up @@ -623,7 +636,43 @@ struct LoggingTest {
testLogging.history.assertExist(level: .critical, message: "yes: critical")
}

@Test func allLogLevelByFunctionRefWithSource() {
@Test func allLogLevelByFunctionRefWithSourceAndError() {
let testLogging = TestLogging()

var logger = Logger(
label: "\(#function)",
factory: {
testLogging.make(label: $0)
}
)
logger.logLevel = .trace

let trace = logger.trace(_:error:metadata:source:file:function:line:)
let debug = logger.debug(_:error:metadata:source:file:function:line:)
let info = logger.info(_:error:metadata:source:file:function:line:)
let notice = logger.notice(_:error:metadata:source:file:function:line:)
let warning = logger.warning(_:error:metadata:source:file:function:line:)
let error = logger.error(_:error:metadata:source:file:function:line:)
let critical = logger.critical(_:error:metadata:source:file:function:line:)

trace("yes: trace", TestError.boom, [:], "foo", #file, #function, #line)
debug("yes: debug", TestError.boom, [:], "foo", #file, #function, #line)
info("yes: info", TestError.boom, [:], "foo", #file, #function, #line)
notice("yes: notice", TestError.boom, [:], "foo", #file, #function, #line)
warning("yes: warning", TestError.boom, [:], "foo", #file, #function, #line)
error("yes: error", TestError.boom, [:], "foo", #file, #function, #line)
critical("yes: critical", TestError.boom, [:], "foo", #file, #function, #line)

testLogging.history.assertExist(level: .trace, message: "yes: trace", error: TestError.boom, source: "foo")
testLogging.history.assertExist(level: .debug, message: "yes: debug", error: TestError.boom, source: "foo")
testLogging.history.assertExist(level: .info, message: "yes: info", error: TestError.boom, source: "foo")
testLogging.history.assertExist(level: .notice, message: "yes: notice", error: TestError.boom, source: "foo")
testLogging.history.assertExist(level: .warning, message: "yes: warning", error: TestError.boom, source: "foo")
testLogging.history.assertExist(level: .error, message: "yes: error", error: TestError.boom, source: "foo")
testLogging.history.assertExist(level: .critical, message: "yes: critical", error: TestError.boom, source: "foo")
}

@Test func allLogLevelByFunctionRefWithoWithSourceWithoutError() {
Comment thread
samuelmurray marked this conversation as resolved.
Outdated
let testLogging = TestLogging()

var logger = Logger(
Expand Down Expand Up @@ -659,7 +708,7 @@ struct LoggingTest {
testLogging.history.assertExist(level: .critical, message: "yes: critical", source: "foo")
}

@Test func allLogLevelByFunctionRefWithoutSource() {
@Test func allLogLevelByFunctionRefWithoutSourceOrError() {
let testLogging = TestLogging()

var logger = Logger(
Expand Down
26 changes: 21 additions & 5 deletions Tests/LoggingTests/TestLogger.swift
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should probably get rid of this TestLogger and replace it with the InMemoryLogHandler at some point in the future

Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,15 @@ internal struct TestLogHandler: LogHandler {
metadata.merge(explicitMetadata, uniquingKeysWith: { _, explicit in explicit })
}

if let error = event.error {
Comment thread
kukushechkin marked this conversation as resolved.
Outdated
metadata["error.message"] = "\(error)"
metadata["error.type"] = "\(String(describing: type(of: error)))"
}

self.logger.log(
level: event.level,
event.message,
error: event.error,
metadata: metadata,
source: event.source,
file: event.file,
Expand Down Expand Up @@ -268,6 +274,7 @@ extension History {
func assertExist(
level: Logger.Level,
message: String,
error: (any Error)? = nil,
metadata: Logger.Metadata? = nil,
source: String? = nil,
file: String = #filePath,
Expand All @@ -276,17 +283,18 @@ extension History {
column: Int = #column
) {
let source = source ?? Logger.currentModule(fileID: "\(fileID)")
let entry = self.find(level: level, message: message, metadata: metadata, source: source)
let entry = self.find(level: level, message: message, error: error, metadata: metadata, source: source)
#expect(
entry != nil,
"entry not found: \(level), \(source), \(String(describing: metadata)), \(message)",
"entry not found: \(level), \(source), \(String(describing: metadata)), \(message), \(error)",
sourceLocation: SourceLocation(fileID: fileID, filePath: file, line: line, column: column)
)
}

func assertNotExist(
level: Logger.Level,
message: String,
error: (any Error)? = nil,
metadata: Logger.Metadata? = nil,
source: String? = nil,
file: String = #filePath,
Expand All @@ -295,22 +303,30 @@ extension History {
column: Int = #column
) {
let source = source ?? Logger.currentModule(fileID: "\(fileID)")
let entry = self.find(level: level, message: message, metadata: metadata, source: source)
let entry = self.find(level: level, message: message, error: error, metadata: metadata, source: source)
#expect(
entry == nil,
"entry was found: \(level), \(source), \(String(describing: metadata)), \(message)",
"entry was found: \(level), \(source), \(String(describing: metadata)), \(message), \(error)",
sourceLocation: SourceLocation(fileID: fileID, filePath: file, line: line, column: column)
)
}

func find(level: Logger.Level, message: String, metadata: Logger.Metadata? = nil, source: String) -> LogEntry? {
func find(level: Logger.Level, message: String, error: (any Error)? = nil, metadata: Logger.Metadata? = nil, source: String) -> LogEntry? {
self.entries.first { entry in
if entry.level != level {
return false
}
if entry.message != message {
return false
}
if let error {
guard case .string(let errorMessage) = entry.metadata?["error.message"], errorMessage == "\(error)" else {
return false
}
guard case .string(let errorType) = entry.metadata?["error.type"], errorType == String(describing: type(of: error)) else {
return false
}
}
Comment thread
kukushechkin marked this conversation as resolved.
Outdated
if let lhs = entry.metadata, let rhs = metadata {
if lhs.count != rhs.count {
return false
Expand Down
5 changes: 5 additions & 0 deletions Tests/LoggingTests/TestSendable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,23 @@ struct SendableTest {
let message1 = Logger.Message(stringLiteral: "critical 1")
let message2 = Logger.Message(stringLiteral: "critical 2")
let metadata: Logger.Metadata = ["key": "value"]
let error = TestError()

let task = Task.detached {
logger.info("info")
logger.critical(message1)
logger.critical(message2)
logger.warning(.init(stringLiteral: "warning"), metadata: metadata)
logger.warning("error", error: error)
}

await task.value
testLogging.history.assertExist(level: .info, message: "info", metadata: [:])
testLogging.history.assertExist(level: .critical, message: "critical 1", metadata: [:])
testLogging.history.assertExist(level: .critical, message: "critical 2", metadata: [:])
testLogging.history.assertExist(level: .warning, message: "warning", metadata: metadata)
testLogging.history.assertExist(level: .warning, message: "error", error: error)
}

final class TestError: Error {}
}
Loading