Consider switching OSLog to Logging

[pfandrade]

Hello,

As of writing, SyncEngine can be parameterized with a Logger instance from OSLog. While great OSLog doesn't provide much customization such as providing different backends.

swift-log however, is a more customizable logging system.

In my particular case, I have logging backends configured with swift-log that allow me to log to file, console, etc. And have other features in place that allow, for example, to ask a user for his logs and the app will zip the log files and send them my way.

Ideally, I'd like the SyncEngine logs to also be written to my current logging infrastructure. But alas, that's not currently possible.

So, is switching to the more general swift-log something you would consider? There's an OSLog backend for it as well.

[stephencelis]

@pfandrade We are open to this and it has been PR'd a couple times when we didn't have a chance to help steer it across the finish line.

• Swift Log Support #122 by @mhayes853
• Add injectable logger support for CloudKit sync operations #251 by @jakubwarchol

Not sure if either is interested in moving things forward now.

I think our main goal with such a PR would be for it to:

• Introduce a SQLiteDataLogging trait, which gates the dependency on swift-log

• Minimize #if SQLiteDataLogging conditionals to imports, e.g.:

  #if SQLiteDataLogging
    import Logging
  #else
    import os
  #endif

Because swift-log and os have very similar interfaces, the hope is that the actual code invoking the logger can be the same, statically.

Thoughts on this approach?

[pfandrade]

Thanks for mentioning those PRs. I did not know about them.

Your approach sounds fine to me and PR #122 seems like a good place to start. However, when I started the discussion I was thinking more about the SyncEngine itself.

The rest of SQLiteData offers fine-grained control over SQL operations, so my logs are more than enough. But the SyncEngine does a lot of stuff behind my back and that's where its logs come in handy.

Having said that, I guess that if we're adding swift-log as a possibility it does make sense to add it to the entire package. Not just the sync engine.

Given the sync engine already receives a Logger as a parameter, a client could just pass a different logger there (with a different label), and thus have a way to distinguish between SyncEngine logs and everything else.

[mbrandonw]

However, when I started the discussion I was thinking more about the SyncEngine itself.

The library does not currently perform any logging except in the SyncEngine. So I believe Stephen's suggestion is equivalent to what you want. And if in the future we added additional logging, I can't think of any reason we would want that logging to behave differently from the logging in SyncEngine.

This approach also feels like it generalizes to really any library that uses logging. By default a package can use the native system logging capabilities (i.e. OSLog), and if people want to use their own logging system they can enable the trait.

[mbrandonw]

@pfandrade @mhayes853 @jakubwarchol Do any of yall have interest in taking Stephen's idea and running with it?

[pfandrade]

I might be able to take a stab at it in a couple of weeks. Can we keep this discussion open a bit longer?

[mbrandonw]

Yep no need to close the discussion!

[mhayes853]

Looking back on #122, one of the things I tried doing was to make use of Swift Log's metadata system, albeit it probably could've been done in a simpler manner without needing to have entirely separate log messages for OSLog and Swift Log.

Perhaps when the SQLiteDataLogging trait is enabled, the metadata can be computed for Swift Log whilst keeping the original log messages intact. Since it probably wouldn't be possible to pass the metadata directly into the logger methods while keeping the logging code statically the same, a custom MetadataProvider could be used instead. This should avoid the need to duplicate the log messages themselves.

Eg. Something like this.

#if SQLiteDataLogging
  extension SyncEngine {
    @TaskLocal static var currentMetadata: Logger.Metadata = [:]

    public static let metadataProvider = Logger.MetadataProvider { currentMetadata }
  }
#endif

// In Logging.swift

@available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
extension Logger {
  func log(_ event: SyncEngine.Event, syncEngine: any SyncEngineProtocol) {
    #if SQLiteDataLogging
      SyncEngine.$currentMetadata.withValue(computeMetadata(event, syncEngine: syncEngine)) {
        _log(event, syncEngine: syncEngine)
      }
    #else 
      _log(event, syncEngine: syncEngine)
    #endif
  }

  private func _log(_ event: SyncEngine.Event, syncEngine: any SyncEngineProtocol) {
    // Existing Logging Code Here...
  }

  #if SQLiteDataLogging
    private func computeMetadata(_ event: SyncEngine.Event, syncEngine: any SyncEngineProtocol) -> Logger.Metadata {
      // ...
    }
  #endif
}

Then when bootstrapping the LoggingSystem from application code, you can pass SyncEngine.metadataProvider to get access to the metadata.

import Logging
import SQLiteData

LoggingSystem.bootstrap(
  { label, metadataProvider in /* Construct Log Handler */ },
  metadataProvider: SyncEngine.metadataProvider
)

// If you have other metadata providers you want to use alongside the one from SyncEngine

LoggingSystem.bootstrap(
  { label, metadataProvider in /* Construct Log Handler */ },
  metadataProvider: .multiplex([SyncEngine.metadataProvider, otherProvider])
)