[SLG-0006]: task-local logger implementation#459
Conversation
c5471e9 to
16bce4e
Compare
16bce4e to
80e5dfd
Compare
80e5dfd to
0ed85c1
Compare
…ithub.com/kukushechkin/swift-log into SLG-0006-task-local-logger-implementation
| } | ||
| ``` | ||
|
|
||
| #### Alternative: Accept logger through initializer when appropriate |
There was a problem hiding this comment.
Why did this become recommended?
There was a problem hiding this comment.
Shouldn't be recommended. The thinking was if you want a specific logger for the lifetime of an object, caching the logger might be a preferred way — the task-local one can be different over time.
There was a problem hiding this comment.
I wouldn’t recommend that because caching the logger for the lifetime of an object means the metadata is not inherited down the call stack. I would consider storing the logger actually a bad practice now with task local propagation
There was a problem hiding this comment.
Yeah, let's keep it just "Alternative", not a recommended approach.
|
|
||
| #### Recommended: Read ``Logger/current`` from the task-local | ||
|
|
||
| When a `logger:` parameter would clutter an otherwise logger-unrelated API, read the |
There was a problem hiding this comment.
I would recommend not saying “clutter” but instead say that if an API doesn’t want to make Logging part of its public API I.e. to hide the dependency
There was a problem hiding this comment.
Reworded as just whenever there is no logger in the API for whatever reasons.
| @main | ||
| struct MyServer { | ||
| static func main() async throws { | ||
| let logger = Logger(label: "my-server") |
There was a problem hiding this comment.
This example is missing the bootstrap. I am wondering if we want a bootstrap method that also binds the take local
There was a problem hiding this comment.
I think it will be the same technically, but confusing semantically — LoggingSystem.bootstrap(logger) { } vs withLogger(logger) {}. Can you call bootstrap only once? What is the difference with just withLogger?
There was a problem hiding this comment.
I am mostly thinking of how we recommend application developers to structure their application Main method. Now we recommend calling bootstrap and then withLogger right afterwards maybe providing a 2-1 solution will make it easier
There was a problem hiding this comment.
Right. Realistically, an application get completely get rid of bootstrap if it itself uses task-local or explicit loggers AND no dependencies create loggers (== do not rely on the global factory bootstrap). This is needed if application has to deal with loggers construction. Changed the paragraph in the corresponding section.
This proposal adds task-local logger storage so that metadata can accumulate across async
call stacks without threading a
Loggerthrough every function signature.Motivation:
Metadata propagation requires threading a logger through every layer. Every function on
the path from request ingress to the bottom of the call stack has to accept, mutate, and
forward a
Logger. Libraries must choose between polluting their public API with alogger:parameter andlosing all the caller's context. There is no third option today.
Modifications:
@TaskLocalLoggerinstance added.Logger.currentAPI.Result:
Users can now automatically propagate accumulated metadata into libraries without explicitly passing loggers through the API boundaries.