-
Notifications
You must be signed in to change notification settings - Fork 340
[SLG-0006]: task-local logger implementation #459
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
891aec2
0ed85c1
a4f45ab
5767644
302b19c
de192c4
c4e28e1
accf3af
38dad4f
eb974cb
d825a7a
378b7ce
6274dfa
a237876
87c618f
87e88e0
9c4e977
1cfb2f7
fe4d4c8
af561ae
b221b81
ebbc525
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,20 +1,30 @@ | ||
| # 003: Accepting loggers in libraries | ||
| # 003: Logger propagation in libraries | ||
|
|
||
| Accept loggers through method parameters to ensure proper metadata propagation. | ||
| Propagate caller context by accepting a `Logger` parameter or reading the task-local | ||
| ``Logger/current`` — never by constructing your own logger. | ||
|
|
||
| ## Overview | ||
|
|
||
| Libraries should accept logger instances through method parameters rather than | ||
| storing them as instance variables. This practice ensures metadata (such as | ||
| correlation IDs) is properly propagated down the call stack, while giving | ||
| applications control over logging configuration. | ||
| Libraries should obtain a `Logger` in one of two ways: accept one through a method or | ||
| initializer parameter, or read ``Logger/current`` from the task-local. Both approaches | ||
| preserve the caller's metadata, log level, and handler choice. Constructing a logger | ||
| inside a library — via ``Logger/init(label:)`` — takes those choices away from the | ||
| application and breaks metadata propagation. | ||
|
|
||
| ### Motivation | ||
|
|
||
| When libraries accept loggers as method parameters, they enable automatic | ||
| propagation of contextual metadata attached to the logger instance. This is | ||
| especially important for distributed systems where correlation IDs must flow | ||
| through the entire request processing pipeline. | ||
| The application controls logging: which backend, which log level, which metadata | ||
| travels with each request. Libraries that participate in this picture obtain a logger | ||
| the application has set up, rather than constructing their own from scratch. This | ||
| ensures correlation IDs and other contextual metadata flow through the entire call | ||
| stack, and gives the application a single place to redirect or filter all log output. | ||
|
|
||
| Two propagation mechanisms are available, and they coexist. Choose the explicit | ||
| parameter when the library's API already accepts a `Logger` (or it's natural to add | ||
| one) — the call site stays declarative about what gets logged where. Choose the | ||
| task-local when adding a `logger:` parameter would pollute an API that otherwise has | ||
| no logging concern in its signature. Application code drives the task-local binding; | ||
| library code reads it. | ||
|
|
||
| ### Example | ||
|
|
||
|
|
@@ -31,70 +41,141 @@ struct RequestProcessor { | |
| logger[metadataKey: "request.id"] = "\(request.id)" | ||
|
|
||
| logger.debug("Processing request") | ||
|
|
||
| // Pass the logger down to maintain metadata context. | ||
| let validatedData = try validateRequest(request, logger: logger) | ||
| let result = try await executeBusinessLogic(validatedData, logger: logger) | ||
|
|
||
| logger.debug("Request processed successfully") | ||
| return result | ||
| } | ||
|
|
||
| private func validateRequest(_ request: HTTPRequest, logger: Logger) throws -> ValidatedRequest { | ||
| logger.debug("Validating request parameters") | ||
| // Include validation logic that uses the same logger context. | ||
| return ValidatedRequest(request) | ||
| } | ||
|
|
||
| private func executeBusinessLogic(_ data: ValidatedRequest, logger: Logger) async throws -> HTTPResponse { | ||
| logger.debug("Executing business logic") | ||
|
|
||
| // Further propagate the logger to other services. | ||
| let dbResult = try await databaseService.query(data.query, logger: logger) | ||
|
|
||
| logger.debug("Business logic completed") | ||
| return HTTPResponse(data: dbResult) | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| #### Alternative: Accept logger through initializer when appropriate | ||
| #### Recommended: Accept logger through initializer for long-lived components | ||
|
|
||
| ```swift | ||
| // ✅ Acceptable: Logger through initializer for long-lived components | ||
| final class BackgroundJobProcessor { | ||
| private let logger: Logger | ||
|
|
||
| init(logger: Logger) { | ||
| self.logger = logger | ||
| } | ||
|
|
||
| func run() async { | ||
| // Execute some long running work | ||
|
kukushechkin marked this conversation as resolved.
|
||
| logger.debug("Update about long running work") | ||
| // Execute some more long running work | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| #### Recommended: Read ``Logger/current`` from the task-local | ||
|
|
||
| When a `logger:` parameter would clutter an otherwise logger-unrelated API, read the | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reworded as just whenever there is no logger in the API for whatever reasons. |
||
| task-local. The application's accumulated metadata (`request.id`, etc.) flows in | ||
| automatically without an explicit hand-off. | ||
|
|
||
| ```swift | ||
| // ✅ Good: Library reads Logger.current; caller scopes context via withLogger. | ||
| public struct AnalyticsClient { | ||
| public func track(_ event: String) { | ||
| Logger.current.info("event", metadata: ["event.name": "\(event)"]) | ||
| } | ||
| } | ||
|
|
||
| // Application binds at @main and scopes per-request metadata. | ||
| @main | ||
| struct MyServer { | ||
| static func main() async throws { | ||
| let logger = Logger(label: "my-server") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This example is missing the bootstrap. I am wondering if we want a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it will be the same technically, but confusing semantically —
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right. Realistically, an application get completely get rid of |
||
| try await withLogger(logger) { _ in | ||
| try await runServices() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func handleRequest(_ req: HTTPRequest) async throws { | ||
| try await withLogger(mergingMetadata: ["request.id": "\(req.id)"]) { _ in | ||
| AnalyticsClient().track("request.received") // sees request.id automatically | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| For per-statement metadata, pass it via the `metadata:` parameter on the log call — | ||
| it does not propagate and does not leak into other code: | ||
|
kukushechkin marked this conversation as resolved.
Outdated
|
||
|
|
||
| ```swift | ||
| Logger.current.info("step", metadata: ["step.name": "validate"]) | ||
| ``` | ||
|
|
||
| For a few back-to-back lines, take a local copy and stamp metadata there — also no | ||
| propagation: | ||
|
|
||
| ```swift | ||
| var local = Logger.current | ||
| local[metadataKey: "step.name"] = "validate" | ||
| local.info("entering") | ||
| local.info("done") | ||
| ``` | ||
|
|
||
| #### Avoid: Libraries creating their own loggers | ||
|
|
||
| Libraries might create their own loggers; however, this leads to two problems. | ||
| First, users of the library can't inject their own loggers which means they have | ||
| no control in customizing the log level or log handler. Secondly, it breaks the | ||
| metadata propagation since users can't pass in a logger with already attached | ||
| metadata. | ||
| Constructing ``Logger/init(label:)`` inside a library takes control over the handler, log | ||
| level, and base metadata away from the application. The application cannot redirect, | ||
| filter, or silence the library's output. | ||
|
|
||
| ```swift | ||
| // ❌ Bad: Library creates its own logger | ||
| // ❌ Bad: Library creates its own logger — loses caller's context. | ||
| final class MyLibrary { | ||
| private let logger = Logger(label: "MyLibrary") // Loses all context | ||
| private let logger = Logger(label: "MyLibrary") | ||
| } | ||
| ``` | ||
|
|
||
| // ✅ Good: Library accepts logger from caller | ||
| final class MyLibrary { | ||
| func operation(logger: Logger) { | ||
| // Maintains caller's context and metadata | ||
| } | ||
| ``Logger/init(label:)`` is for the application — typically at `@main`, paired with | ||
| ``LoggingSystem/bootstrap(_:)``. Library code, including internal application modules, | ||
| should not construct loggers. | ||
|
|
||
| #### Avoid: Relying on ``Logger/current`` across non-Task boundaries | ||
|
|
||
| ``Logger/current`` is backed by a `TaskLocal` and propagates through Swift's structured | ||
| concurrency model only. Callbacks invoked on non-Task threads — GCD blocks, | ||
| `URLSession` completion handlers, delegate methods dispatched onto specific queues, | ||
| `NotificationCenter` observers, C-API callbacks — see the *default* fallback logger, | ||
| not the bound one. Metadata bound by the calling `Task` is invisible inside those | ||
| callbacks. | ||
|
|
||
| ```swift | ||
| // ❌ Bad: completion handler runs without the Task context; Logger.current is the fallback. | ||
| try await withLogger(mergingMetadata: ["request.id": "r1"]) { _ in | ||
| URLSession.shared.dataTask(with: req) { data, _, _ in | ||
| Logger.current.info("response") // empty-label fallback, no request.id | ||
| }.resume() | ||
| } | ||
| ``` | ||
|
|
||
| For libraries with async completion-handler APIs, accept an explicit `Logger` parameter. | ||
| If capturing and rebinding across the boundary is the only option: | ||
|
|
||
| ```swift | ||
| // ✅ Good: capture before the boundary, rebind on the other side. | ||
| try await withLogger(mergingMetadata: ["request.id": "r1"]) { _ in | ||
| let captured = Logger.current | ||
| URLSession.shared.dataTask(with: req) { data, _, _ in | ||
| withLogger(captured) { logger in | ||
| logger.info("response") // request.id preserved | ||
| } | ||
| }.resume() | ||
| } | ||
| ``` | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did this become recommended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, let's keep it just "Alternative", not a recommended approach.