Hi all,
The proposal SLG-0005: LogEvent LogHandler API for swift-log is now up and In Review.
The review period will run until March 16th — please feel free to post your feedback as a reply to this thread.
Thanks!
Hi all,
The proposal SLG-0005: LogEvent LogHandler API for swift-log is now up and In Review.
The review period will run until March 16th — please feel free to post your feedback as a reply to this thread.
Thanks!
I think this sounds like a good change. However, I’m not sure I fully understand how it affects our ability to extend the API in the future.
Future proposals can add new fields to
LogEventwith sensible defaults — a richer metadata type, or an attachedError(as a follow-up for SLG-0003 proposal) — without touching theLogHandlerprotocol and without requiring any changes to existing handler implementations. This is the primary motivation for this proposal.The SLG-0004 proposal introduces an
AttributedMetadatatype that wrapsLogger.Metadatawith additional context. Adding it toLogEventas a new optional field would be source-compatible, and the existingmetadatafield could become a computed property that projects fromattributedMetadatawhen present, keeping old handlers fully functional without modification.
From your second example, I take it that we want old handlers to get a computed metadata with both “regular” and attributed metadata merged. But an updated LogHandler that handles the new attributedMetadata would want metadata to return nil if info(message:attributedMetada:...) was called, right? Otherwise they would need to first check attributedMetadata and only if that is nil, check metadata.
And for the error attribute in SLG-0003, we are considering adding a default serialization for old LogHandlers, but this should of course not be done for updated LogHandlers. How would such a fallback be implemented using LogEvent?
Yeah that’s a good idea, it’ll make less painful future evolution.
Minor staging question if we want to get the error proposal done first or not, but it could go either way since the impact of the error one is pretty clear here.
We’d have some overloads less to worry about if we land this first though which could be nice.
Thanks for the idea!
Thanks for sharing the proposal. The swift-log ecosystem has grown a lot over time, so it’s interesting to see discussions around evolving the LogHandler API.
From a quick read, the idea of introducing a LogEvent type seems like a nice way to group together the metadata, message, level, and context into a single structure. It feels like it could make handler implementations a bit cleaner and easier to reason about, especially when writing custom log backends.
I’m also curious how this might affect existing LogHandler implementations in terms of migration. If the transition path is straightforward, it could make the API feel more cohesive without adding too much complexity.
Looking forward to seeing more feedback from others in the community during the review period.
attributedMetadata is meant to replace plain metadata if it is available. I was thinking about making both metadata and attributedMetadata computed. Either plain or attributed metadata can be used in a log instruction (merging with metadata provider and LogHandler metadata is the responsibility of the LogHandler) and computed vars would do transformation between plain and attributed metadata, so LogHandler can keep using either.
For LogHandler not yet supporting LogEvent, the fallback would be as spelled in the SLG-0003 — add special metadata values. For LogHandler already supporting LogEvent, but not yet supporting the error field of it — we have no knowledge if a concrete LogHandler using the logEvent.error property, neither we provide any feedback. A reasonable question now, as I've written this, — do we want to provide fallback to older LogHandler, or make errors propagation a new feature of the LogEvent only.
I think it's reasonable that we only ever "move forward" here, so to get the error param you have to move to the LogEvent model.
I think we should only have attributedMetadata for the new LogEvent and use this opportunity to move over. Assuming we accept the privacy labels proposal.
I think this is the one downside of this proposal. We cannot easily implement fallbacks for new fields of the log event. However, I think it is okay to expect LogHandlers to be updated to support new fields in log events. This is similar to many other API evolution stories. Concretely this means any log handler that doesn't take the error field into consideration will just not emit any errors details.
I think we can. When we introduce a new LogEvent initializer with the new field, we add the fallback behavior into the old initializer that we deprecate in the same step.
How would this be done concretely? If I read the proposal correct, Logger is responsible for instantiating LogEvent, which will then be passed to the new log(event: LogEvent) func in LogHandler. But how can Logger decide on which LogEvent initializer to use? It can't know what "version" of LogEvent the LogHandler supports, right?
I'm not saying this is a deal breaker; as other people have stated it could be expected of a LogHandler to release new versions to keep up with API additions, but I think we should at least be aware of it.
Yeah, as there are three active proposals in various stages affecting the API between Logger and LogHandler, maybe it's wise to discuss what the release strategy should be. We already said that error should not be implemented before this one - does that mean that we make one release with all three additions/changes? This would be a somewhat large release, but could help avoid a cluttered API.
I think Error support in LogEvent is additive. AttributedMetadata can be additive as well, if we keep both plain and attributed metadata.
I would still discuss if LogEvent should have only attributed metadata, or both plain and attributed metadata. This would also couple this proposal to the SLG-0004, which indeed introduces complexity.
We've talked more offline with @FranzBusch about potentially having only .attributedMetadata in the LogEvent. Given there is no consensus on the SLG-0004 atm, we accept this proposal with the plain .metadata (making it a computed property + stored private property to futureproof the API from changing a stored to a computed property). If/when there is a decision on the attributed metadata we can add attributed metadata to the LogEvent separately. If, miraculously, we come up with an attributed metadata design that can replace plain metadata, and LogEvent is not yet implemented or released, we discuss replacing it in a PR.
Tank you for the discussion! The proposal will now move to the "Ready for Implementation".