swift-log
swift-log copied to clipboard
apple
Make Logger.Metadata.Key strongly typed
Make Logger.Metadata.Key strongly typed by using a separate struct.
Motivation:
Using String as keys makes it hard to use constants for metadata keys. They have to be defined in some arbitrary namespace. Or if they don't, they're messy to change and even harder to "look up" (e.g. what keys exist on that logger? what key stores what information) since there is no central definition place.
enum SomeMetadataKeyNamespace {
static let myMetadataKey = "awesome-key"
}
var logger = Logger(label: "some.great.logger")
logger[metadataKey: SomeMetadataKeyNamespace. myMetadataKey] = "what a beautiful value"
// Later on in e.g. a different file (a `LogHandler` implementation for example)
if case .string(let str)? = logger[metadataKey: SomeMetadataKeyNamespace. myMetadataKey], str.contains("beautiful") {
print("It's beautiful!")
} else {
print("Not beautiful...")
}
Modifications:
- Add
Logger.MetadataKeyand change thetypealiasforLogger.Metadatato useLogger.MetadataKeyas key instead of String. - Change the protocol requirement in
LogHandlerto takeLogger.Metadata.Keyinstead ofStringinsubscript(metadataKey:). Logger.MetadataKeyconforms to:RawRepresentable(RawValue = String)HashableCustomStringConvertible(descriptionbeingrawValue)ExpressibleByStringLiteral(StringLiteralType = String)ExpressibleByStringInterpolation(StringInterpolation=DefaultStringInterpolation)
Result:
Using a separate struct for Logger.Metadata.Key allows static constants on it making metadata "nicer" to use:
extension Logger.Metadata.Key {
static let myMetadataKey = Logger.Metadata.Key(rawValue: "awesome-key")
}
var logger = Logger(label: "some.great.logger")
logger[metadataKey: .myMetadataKey] = "what a beautiful value"
// Later on in e.g. a different file (a `LogHandler` implementation for example)
if case .string(let str)? = logger[metadataKey: .myMetadataKey], str.contains("beautiful") {
print("It's beautiful!")
} else {
print("Not beautiful...")
}
Source Compatibility
I'm not sure this change can be made in a source compatible way - especially since the subscript(metadataKey:) was directly requiring String instead of Logger.Metadata.Key. Most implementations would probably be fine with the deprecated overload I added (similar than what has been done for source - although I've read that this wasn't perfect either). But those implementations that use String APIs will definitively break. Thus this will likely need a new major version - in which case I could remove the "attempted backwards compatibility" 🙂
Thanks for the PR/idea @ffried, I'll give it a deeper look soon.
It may be worth having a look at the big picture as well, as the topic of type-safety of such metadata properties is solved in other ways already in baggage context and swift tracing (both eventually to become SSWG standard libraries). I wonder if we were to change anything, if we rather should try to align all those styles into one style how to access/set metadata.
Note: I'm not really opposed to your idea, just saying that we should explore things a bit more since a similar pattern (even more type-safe) exists in those other libs!
Specifically, we're working on introducing a baggage context type:
- where we use typesafe keys already, and the baggage is really the type to set all your metadata on, if it should be carried around, rather than setting it in the logger metadata (long story, but it matters for tracing),
- see here how the keys work: https://github.com/slashmo/gsoc-swift-baggage-context/blob/main/Tests/BaggageTests/BaggageContextTests.swift#L69-L82
We have found better ways to express this though, but they are limited to newer swift versions:
as well as tracing which handles attributes in a very typesafe and well defined way:
- https://github.com/slashmo/gsoc-swift-tracing/pull/104
- allows for
span.attributes.http.something = onlyTheRightTypewhere http.something are actually defined via extensions; so it is even nicer for use sites than[metadataKey: .metadataKey]; this requires Swift 5.1 / 5.2+ though, it'd be additive.
- allows for
And finally, if we decide this is a nice thing to do... we need to see how to introduce it in a source-compatible way :)
Let me know your thoughts!
@ktoso Thanks for the insights you gave there!
The approach gsoc-baggage-context takes, leads to a lot of new types that should actually be constants. Vapor 4 uses the same approach for its storage and lock keys. IMHO it's hard to justify having to introduce a type where you actually only want to define a constant. I think it's kind of similar to the current situation in swift-log where you'd have to introduce a namespace-type. The only benefit from this is, that types are automatically unique, eliminating the risk of duplicate constants.
The approach taken in gsoc-swift-tracing looks neat, though! I've only had a quick glance at it, but it looks very promising. The only thing I'm not sure is the amount of "overhead" it generates. E.g. what is NestedSpanAttributesProtocol vs. SpanAttributeNamespace? Why does it need a protocol for this at all - couldn't it be done similar to what SwiftUI does with Environment?
But these questions might clear up once I take a better look at it - which I definitively will!
As for source compatibility: The latter approach might even be easier in that regard, since it (as far as I've seen) could be a purely additive change.
I'll see if I can revisit this next week and maybe come up with a different (better? 🙂) implementation that is similar to what gosc-swift-tracing does.
I'm going to close this as we're working on proposing baggage (from tracing) integration into swift-log, we'll share a proposal soon.
@ktoso I'm sorry that I haven't found time to take another look at this. And thanks for taking this on!