-
Notifications
You must be signed in to change notification settings - Fork 301
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
Emit correct metadata from StreamLogHandler #251
Conversation
Ouch, nice catch -- thanks @slashmo. We should have a test that covers this, can you please add the following test case in LoggingTests around the func testStreamLogHandlerWritesIncludeMetadataProviderMetadata() {
let interceptStream = InterceptStream()
LoggingSystem.bootstrapInternal({ _, metadataProvider in
StreamLogHandler(label: "test", stream: interceptStream, metadataProvider: metadataProvider)
}, metadataProvider: .exampleMetadataProvider)
let log = Logger(label: "test")
let testString = "my message is better than yours"
log.critical("\(testString)")
let messageSucceeded = interceptStream.interceptedText?.trimmingCharacters(in: .whitespacesAndNewlines).hasSuffix(testString)
XCTAssertTrue(messageSucceeded ?? false)
XCTAssertEqual(interceptStream.strings.count, 1)
let message = interceptStream.strings.first!
XCTAssertTrue(message.contains("example=example-value"), "message must contain metadata, was: \(message)")
} Thank you! |
Good point, I'll add it. |
cf0b53e
to
2ddc2b3
Compare
2ddc2b3
to
7942833
Compare
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.
LGTM, thanks for adding the test and spotting the issue
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [apple/swift-log](https://togithub.com/apple/swift-log) | minor | `from: "1.4.4"` -> `from: "1.5.2"` | --- ### Release Notes <details> <summary>apple/swift-log</summary> ### [`v1.5.2`](https://togithub.com/apple/swift-log/releases/tag/1.5.2) [Compare Source](https://togithub.com/apple/swift-log/compare/1.5.1...1.5.2) #### Primary change Address too aggressive warning logging on LogHandlers that do not support `MetadataProvider`. The warning would be emitted too frequently, resulting in flooding logs with warnings. Instead, the warning is now emitted once per log handler type. #### What's Changed - Avoid logging warnings when handler does not support metadataproviders by [@​ktoso](https://togithub.com/ktoso) in [https://github.com/apple/swift-log/pull/252](https://togithub.com/apple/swift-log/pull/252) - Handle providers properly in multiplex log handler by [@​ktoso](https://togithub.com/ktoso) in [https://github.com/apple/swift-log/pull/254](https://togithub.com/apple/swift-log/pull/254) - Add CI for Swift 5.8 and update nightly to Ubuntu 22.04 by [@​yim-lee](https://togithub.com/yim-lee) in [https://github.com/apple/swift-log/pull/255](https://togithub.com/apple/swift-log/pull/255) **Full Changelog**: apple/swift-log@1.5.1...1.5.2 ### [`v1.5.1`](https://togithub.com/apple/swift-log/releases/tag/1.5.1) [Compare Source](https://togithub.com/apple/swift-log/compare/1.5.0...1.5.1) #### Summary This patch release focuses on minor cleanups to ergonomics of setting metadata providers with the default stream log handlers, and fixes a bug in the default handler not printing the provided extra metadata by default (it does now). Thank you to [@​slashmo](https://togithub.com/slashmo) for quickly noticing and providing a patch for the latter! #### What's Changed - Allow passing explicit provider into the stream handlers by [@​ktoso](https://togithub.com/ktoso) in [https://github.com/apple/swift-log/pull/250](https://togithub.com/apple/swift-log/pull/250) - Emit correct metadata from StreamLogHandler by [@​slashmo](https://togithub.com/slashmo) in [https://github.com/apple/swift-log/pull/251](https://togithub.com/apple/swift-log/pull/251) **Full Changelog**: apple/swift-log@1.5.0...1.5.1 ### [`v1.5.0`](https://togithub.com/apple/swift-log/releases/tag/1.5.0) [Compare Source](https://togithub.com/apple/swift-log/compare/1.4.4...1.5.0) ### Changes #### Swift version support This release drops support for Swift 5.0. Swift 5.1+ remain supported for the time being. #### Logger.MetadataProvider This release introduces metadata providers! They are an additional way to add metadata to your log statements automatically whenever a log statement is about to be made. This works extremely well with systems like distributed tracing, that may pick up trace identifiers and other information from the task-local context from where the log statement is being made. The feature came with a [swift evolution style proposal](https://togithub.com/apple/swift-log/blob/main/proposals/0001-metadata-providers.md) introduction to the "why?" and "how?" of this feature you may find interesting. Metadata providers are used like this: ```swift import Logging enum Namespace { @​TaskLocal static var simpleTraceID: String? } let simpleTraceIDMetadataProvider = Logger.MetadataProvider { guard let traceID = Namespace.simpleTraceID else { return [:] } return ["simple-trace-id": .string(traceID)] } LoggingSystem.bootstrap({ label, metadataProvider in myCoolLogHandler(label: label, metadataProvider: metadataProvider) }, metadataProvider: simpleTraceIDMetadataProvider) ``` which in turn makes every `Logger` on this `LoggingSystem` add this contextual metadata to log statements automatically: ```swift let log = Logger(label: "hello") Namespace.$simpleTraceID.withValue("1234-5678") { test() } func test() { log.info("test log statement") } // [info] [simple-trace-id: 1234-5678] test log statement ``` ##### Adoption in `LogHandler`s In order to support this new feature in your log handlers, please make it accept a `MetadataProvider?` at creation, and store it as: ```swift struct MyHandler: LogHandler { // ... public var metadataProvider: Logger.MetadataProvider? // ... } ``` #### What's Changed ##### Highlight - Metadata Providers (e.g. for Distributed Tracing) in LogHandlers by [@​ktoso](https://togithub.com/ktoso) in [https://github.com/apple/swift-log/pull/238](https://togithub.com/apple/swift-log/pull/238) ##### Other changes - \[docs] Minimal docc setup and landing page by [@​ktoso](https://togithub.com/ktoso) in [https://github.com/apple/swift-log/pull/226](https://togithub.com/apple/swift-log/pull/226) - \=docc Make docs use symbol references by [@​ktoso](https://togithub.com/ktoso) in [https://github.com/apple/swift-log/pull/230](https://togithub.com/apple/swift-log/pull/230) - \=docc Move to multiple Package.swift files by [@​ktoso](https://togithub.com/ktoso) in [https://github.com/apple/swift-log/pull/231](https://togithub.com/apple/swift-log/pull/231) - Undo 5.7 package files, not needed yet by [@​ktoso](https://togithub.com/ktoso) in [https://github.com/apple/swift-log/pull/232](https://togithub.com/apple/swift-log/pull/232) - Update README: Add missing Source param by [@​Rusik](https://togithub.com/Rusik) in [https://github.com/apple/swift-log/pull/233](https://togithub.com/apple/swift-log/pull/233) - Fix build for wasm by [@​ahti](https://togithub.com/ahti) in [https://github.com/apple/swift-log/pull/236](https://togithub.com/apple/swift-log/pull/236) - Add .spi.yml for Swift Package Index DocC support by [@​yim-lee](https://togithub.com/yim-lee) in [https://github.com/apple/swift-log/pull/240](https://togithub.com/apple/swift-log/pull/240) - Fixes link to Supabase repository in README.md by [@​timobollwerk](https://togithub.com/timobollwerk) in [https://github.com/apple/swift-log/pull/245](https://togithub.com/apple/swift-log/pull/245) #### New Contributors - [@​Rusik](https://togithub.com/Rusik) made their first contribution in [https://github.com/apple/swift-log/pull/233](https://togithub.com/apple/swift-log/pull/233) - [@​ahti](https://togithub.com/ahti) made their first contribution in [https://github.com/apple/swift-log/pull/236](https://togithub.com/apple/swift-log/pull/236) - [@​timobollwerk](https://togithub.com/timobollwerk) made their first contribution in [https://github.com/apple/swift-log/pull/245](https://togithub.com/apple/swift-log/pull/245) **Full Changelog**: apple/swift-log@1.4.4...1.5.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/cgrindel/swift_bazel). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4yMi4xIiwidXBkYXRlZEluVmVyIjoiMzUuMjIuMSJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Return the correct metadata to log from the
StreamLogHandler
.Motivation:
At the moment,
StreamLogHandler
always returns the explicitly passed metadata as the metadata to log. This throws away any metadata provided by the registeredMetadataProvider
.Modifications:
Return the merged metadata, both explicit and provided.
Result:
When using the
StreamLogHandler
, metadata set throughMetadataProvider
s will now correctly show up in the logs.