Feature Request: Change AirshipLogger to a protocol with default implementation that can be overridden by app developer

[SwiftNativeDeveloper]

What Airship dependencies are you using?

AirshipCore

What are the versions of any relevant development tools you are using?

Airship 16.6 Xcode Latest iOS 14+

Report

What unexpected behavior are you seeing?

Airship logs go to system console and NOT something the app can view in production

What is the expected behavior?

AirshipLogger should allow customization for integrator to choose where logs go, i.e. AirshipLogger should be an interface, with an extension by default to route to os.Logger

Inspiration Swift-Log's interface

What are the steps to reproduce the unexpected behavior?

Use the code, have ANY log level set.

Do you have logging for the issue?

N/A

[rlepinski]

Could you propose an API ? What exactly are the goals of that? Would something like this work:

public class AirshipLogger {

    public static var logWriter: LogWriter = DefaultLogWriter()

    static func trace(_ message: String,
                      file: String = #file,
                      line: Int = #line,
                      function: String = #function) {

        log(logLevel: UALogLevel.trace,
            message: message,
            file:file,
            line: line,
            function: function)
    }

    static func debug(_ message: String,
                      file: String = #file,
                      line: Int = #line,
                      function: String = #function) {

        log(logLevel: UALogLevel.debug,
            message: message,
            file:file,
            line: line,
            function: function)
    }

    static func info(_ message: String,
                     file: String = #file,
                     line: Int = #line,
                     function: String = #function) {
        log(logLevel: UALogLevel.trace,
            message: message,
            file:file,
            line: line,
            function: function)
    }

    static func warn(_ message: String,
                     file: String = #file,
                     line: Int = #line,
                     function: String = #function) {
        log(logLevel: UALogLevel.trace,
            message: message,
            file:file,
            line: line,
            function: function)
    }

    static func error(_ message: String,
                      file: String = #file,
                      line: Int = #line,
                      function: String = #function) {

        log(logLevel: UALogLevel.trace,
            message: message,
            file:file,
            line: line,
            function: function)
    }

    private static func log(logLevel: UALogLevel,
                            message: String,
                            file: String = #file,
                            line: Int = #line,
                            function: String = #function) {

        if (uaLoggingEnabled.boolValue && logLevel.rawValue >= uaLogLevel.rawValue) {
            logWriter.log(logLevel: logLevel,
                          message: message,
                          file: file,
                          line: line,
                          function: function)
        }
    }
}

public protocol LogWriter {
    func log(logLevel: UALogLevel, message: String, file: String, line: Int, function: String)
}

Then you can set the LogWriter that is used by AirshipLogger?

[SwiftNativeDeveloper]

@rlepinski

For creating an interface, I would look at this protocol. If you're not familiar with the SwiftLog package, it is fantastic. https://github.com/apple/swift-log/blob/main/Sources/Logging/LogHandler.swift#L116

Your "LogWriter" protocol is very similar to that in SwiftLog, so not much to comment on there. Maybe line as UInt? I know #line is an Int, but for some reason Apple's interface in this package uses UInt (makes sense, can't have a negative line number.)

I'd also suggest enhancing all of the log functions within the AirshipLogger to use an AutoClosure for the message parameter. That way if the level is too low, the auto closure won't even compute the message. https://github.com/apple/swift-log/blob/main/Sources/Logging/Logging.swift#L73

As for setting a custom handler, I'm not sure if a static implementor vs a Configuration / Airship takeoff parameter would be better. Either would do fine as long as the implementation gets some love in the documentation. AirshipLogger type was marked as "internal use only".

For what its worth, the default Logger(subsystem: Bundle.main.bundleIdentifier ?? "", category: "Airship") seems like a good fit for default and developer discovery, IF you have console app running, or use the new OSLogStore for iOS 15 to get historical logs that launch.

[rlepinski]

Thanks for the links. Autoclosure is neat, didnt know it existed.

I will probably make AirshipLogger internal only, I don't really want people to log using our logger, but being able to route our logs seems reasonable. We are currently in a weird state since some of our modules are swift and other are obj-c, so not all logs go through this class right now. Its not really ready to be public.

Ill probably expose custom settings on Airship directly to hide AirshipLogger from the world.

[SwiftNativeDeveloper]

@rlepinski sounds good! Happy to review a PR if you want when you get there.

[rlepinski]

Sweet, ill post a PR on this public repo then. Thanks for the input!

[rlepinski]

Had some time today and put this together - https://github.com/urbanairship/ios-library/pull/322 I need to do some more work on it but if you want to provide any feedback please do. Part of the PR is bridging module logs that are still in obj-c to the new logger so we log the same everywhere. Its a real pain with multi modules so I had to jump through some hoops to make it work.

[rlepinski]

Added in 16.8.0