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

RFC: Add capability to configure loggers inline #243

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Mordil
Copy link

@Mordil Mordil commented Dec 22, 2022

Adds initializers and methods to create Logger instances with metadata inline.

Motivation:

Frequently it's needed to create copies of loggers, or a new logger, with attached metadata or specific log levels, but the current method requires first creating an instance, as a var, and then configuring the instance.

This leaves you to have a mutable value existing in the scope even if you don't want to.

This particularly has issues with async code when capturing the instance in a Task because it's non-isolated

var logger = Logger(label: "\(#function)")
logger[metadataKey: metadataKey] = "original"

Task {
    logger.trace("won't work")
       // ^^^^^ Reference to captured var 'logger' in concurrently-executing code
 }

The best way to get around this is either

// 1 immediately invoked closure
let logger = {
    let inner = Logger(...)
    // configuration
    return inner
}()

// 2 variable rebinding
let retainedLogger = logger
Task {
    retainedLogger.trace("will work")
}

// 3 explicit capture
Task { [logger] in
    logger.trace("will also work")
}

Modifications:

  • Add: makeCopy(with:mergePolicy:) to allow just setting inline the metadata you want the new instance to have, with control on how to merge with existing metadata keys
  • Add: makeCopy(_:) which passes an inout reference to the new instance, allowing you to set more options such as logLevel as part of creation
  • Add: init(label:metadata:) that allows creation of instances directly inline that directly sets the metadata
  • Add: init(label:_:) that allows creation of instances with a reference to the object for inline configuration much like makeCopy(_:)

Result:

Developers will now be able to make Logger instances, or "children" copies, directly inline with full control over the value's mutability - making it safe to send across Task boundaries

let logger = Logger(label: "myLabel") {
    $0.logLevel = .info
    $0[metadataKey: "my_key"] = "my value"
}

Task {
    logger.info("this works")
}

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@Mordil Mordil marked this pull request as draft December 22, 2022 04:29
@Mordil Mordil changed the title Add capability to configure loggers inline RFC: Add capability to configure loggers inline Dec 22, 2022
Comment on lines +670 to +697
public func makeCopy(with metadata: Metadata, mergePolicy: Metadata.MergingPolicy = .retainExistingValues) -> Logger {
let originalMetadata = self.handler.metadata
var child = self
child.handler.metadata = originalMetadata.merging(metadata, uniquingKeysWith: { old, new in
switch mergePolicy.policy {
case .replaceValues: return new
case .retainExistingValues: return old
}
})
return child
}

@inlinable
public func makeCopy(_ configure: (inout Logger) -> Void) -> Logger {
var child = self
configure(&child)
return child
}

public init(label: String, metadata: Metadata) {
self.init(label: label)
self.handler.metadata = metadata
}

public init(label: String, _ configure: (inout Logger) -> Void) {
self.init(label: label)
configure(&self)
}
Copy link
Author

Choose a reason for hiding this comment

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

I'm definitely expecting some bikeshedding on the names & labels of all of these - hence the lack of symbol docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants