sushichop / puppy Goto Github PK

Once the LoggingSystem has been bootstrapped, adding a Loggerable to Puppy has no effect. This means it is no longer possible to add and remove loggers to a running system. This was possible in 0.5.0 but is broken in 0.7.0. It may be due to Sendable where the struct is copied when the LogHandler is created. However, the contract to add and remove loggers still exists but has no effect.

On macOS (Big Sur) with Apple Silicon chipset (m1), when a log file is rotated, when the new empty log file is created, it is created with empty permissions (------------). This causes the application to crash with an unexpected nil optional for the file handle (failed to open file for writing?).

NOTE: This looks like a macOS bug where the 'umask' is not taken into account, or is lost at the point of log rotation, because initially a file with proper permissions is created (if not, we could not get to the first point of log rotation).

NOTE: THIS ONLY HAPPENS ON APPLE SILICON.

Changing the openFile function as follows fixes the issue:

let successful = FileManager.default.createFile(atPath: fileURL.path, contents: nil, attributes: [FileAttributeKey.posixPermissions: 0o660])

Obviously a better solution would be to allow the attributes to be specified via some other means, eg: FileLoggerDelegate protocol.

Thanks. Still a great framework :-)

When

I add puppy like private var puppy = Puppy.default

Then

This only happens when I do swift build -c release with xcode 14.0.1 or in terminal with swift-driver version: 1.62.8 Apple Swift version 5.7 (swiftlang-5.7.0.127.4 clang-1400.0.29.50) Target: arm64-apple-macosx12.0

The whole output

~/.build/arm64-apple-macosx/release/Puppy.build/module.modulemap:2:12: error: header '~/.build/arm64-apple-macosx/release/Puppy.build/Puppy-Swift.h' not found
    header "~/.build/arm64-apple-macosx/release/Puppy.build/Puppy-Swift.h"
           ^
~/Sources/Utilities/Logging.swift:3:8: error: could not build Objective-C module 'Puppy'
import Puppy
       ^
[504/510] Compiling Utilities Logging.swift

It does not happen when I build Puppy on its own.

I have mac with info

Software:

    System Software Overview:

      System Version: macOS 12.6 (21G115)
      Kernel Version: Darwin 21.6.0
      Boot Volume: Macintosh HD
      Boot Mode: Normal
      User Name: Stijn Willems (stijnwillems)
      Secure Virtual Memory: Enabled
      System Integrity Protection: Enabled
      Time since boot: 47 minutes

Hardware:

    Hardware Overview:

      Model Name: MacBook Pro
      Model Identifier: MacBookPro18,2
      Chip: Apple M1 Max
      Total Number of Cores: 10 (8 performance and 2 efficiency)
      Memory: 64 GB
      System Firmware Version: 7459.141.1
      OS Loader Version: 7459.141.1
      Serial Number (system): Y63LWPG9VF
      Hardware UUID: 1B4FA833-0112-5EC9-B392-86A2D549DBE0
      Provisioning UDID: 00006001-0008712E3402801E
      Activation Lock Status: Enabled

Workaround

adding puppy like private var puppy = Puppy() fixes the issue 🥴

Hi!

In FileRotationLogger.swift the function rotateFiles() fails at line 109 during the following instruction:

try FileManager.default.moveItem(at: fileURL, to: archivedFileURL)

This happens because in Formatter.swift the function dateFormatter(_:, locale:, dateFormat:, timeZone:) creates a timestamp that includes colons (:) which the file system does not like.

One potential solution is changing the dateFormat in line 13 of Formatter.swift from "yyyy-MM-dd'T'HH:mm:ss.SSSZZZZZ" to "yyyy-MM-dd'T'HH-mm-ss.SSS". This removes the explicit colons in the original date format string, as well as an implicit colon which is added by ZZZZZ which expands to an HH:MM style presentation of the timezone.

You might also consider reducing the complexity of this string further, since I'd argue if a detailed timestamp is necessary then it can be placed inside the log file, rather than in the file name.

One alternative could be to structure like this:

myLog.log
myLog.log.1
myLog.log.2

This proposal would simplify the implementation of the rest of the rotateFiles method that is concerned with removing old archived files.

• Consider an option to keep file extension using FileRoationLogger.

FileRotationLogger's log method calls append with the default writeMode of force.
Seeing some crashes at that point and don't see a way to adjust the writeMode.

class SampleFileRotationDelegate: FileRotationLoggerDeletate {

should be updated to:

class SampleFileRotationDelegate: FileRotationLoggerDelegate {

There is a bit of a gap with Vapor logging to a file on the server. If you search logging in the Vapor Discord, there are quite a few people over the years asking for a way to log to a file and the answer is usually to use any swift-log backend (Vapor's logging is built on top of swift-log)... Unfortunately most of that list is only built for iOS or is too big a dependency to add for simple file logging. After some trial and error with different packages, Puppy was the package that was straight forward and just worked. This has been tested on Xcode 14.1 and Ubuntu 18.04.6 with no issues.

In a Vapor 4 project, add the following to your Sources/Run/main.swift file:

import App
import Vapor
import Puppy

class fileRotationDelegate: FileRotationLoggerDelegate {
    func fileRotationLogger(
        _ fileRotationLogger: FileRotationLogger,
        didArchiveFileURL: URL,
        toFileURL: URL
    ) {
        print("didArchiveFileURL: \(didArchiveFileURL), toFileURL: \(toFileURL)")
    }

    func fileRotationLogger(
        _ fileRotationLogger: FileRotationLogger,
        didRemoveArchivedFileURL: URL
    ) {
        print("didRemoveArchivedFileURL: \(didRemoveArchivedFileURL)")
    }
}

let fileURL = URL(fileURLWithPath: "./Logs/info.log").absoluteURL
let fileRotation = try FileRotationLogger(
    "com.example.yourapp.file",
    fileURL: fileURL,
    filePermission: "600"
)

let delegate = fileRotationDelegate()
fileRotation.delegate = delegate

var puppy = Puppy.default
puppy.add(fileRotation)

var env = try Environment.detect()

try LoggingSystem.bootstrap(from: &env) { (logLevel) -> (String) -> LogHandler in
    return { label -> LogHandler in
        var handler = PuppyLogHandler(label: label, puppy: puppy)
        handler.logLevel = .info
        return handler
    }
}

let app = Application(env)
defer { app.shutdown() }
try configure(app)
try app.run()

Using Puppy in a swift-log context with multiple handlers (console and file rotation) only the first handler added to a Puppy instance is invoked. For example:

https://github.com/sfomuseum/swift-image-emboss-grpc/blob/main/Scripts/image-emboss-grpc-server/Server.swift#L48-L73

The ordering of the handlers does not seem to matter. If I add the console handler first then messages are logged to the console but not the file handler (though the file is created) and vice versa.

This particular application has a somewhat convoluted logging setup necessitated by the steps required to pass metadata (in this case a remote address property) in a gRPC server context and the need to create a custom swift-log wrapper that can be updated (with the remote address metadata) and not trigger compiler warnings which necessitates making the wrapper @unchecked Sendable.

https://github.com/sfomuseum/swift-image-emboss-grpc/blob/main/Sources/ImageEmbossGRPC/Interceptors.swift
https://github.com/sfomuseum/swift-image-emboss-grpc/blob/main/Sources/ImageEmbossGRPC/Logger.swift
https://github.com/sfomuseum/swift-image-emboss-grpc/blob/main/Sources/ImageEmbossGRPC/Embosser.swift#L21

I don't think that's the problem but I am also not super familiar with Sendable yet. I am pretty sure I doing everything as explained in the docs but maybe I am missing something?

Thanks,

Hi,
I have problem after adding the swift package with Puppy to my application. Could anyone please tell me what I am doing wrong?
I am trying on macOS on Intel machine with XCode Version 13.4 (13F17a).

Undefined symbols for architecture x86_64:
"type metadata accessor for Puppy.FileLogger", referenced from:
Action.metoda() -> () in Logger.o
"Puppy.FileLogger.__allocating_init(_: Swift.String, fileURL: Foundation.URL, filePermission: Swift.String, flushMode: Puppy.FileLogger.FlushMode) throws -> Puppy.FileLogger", referenced from:
Action.metoda() -> () in Logger.o
"Puppy.Puppy.__allocating_init() -> Puppy.Puppy", referenced from:
one-time initialization function for log in Logger.o
"type metadata accessor for Puppy.Puppy", referenced from:
one-time initialization function for log in Logger.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

I think that it would be beneficial if the Puppy class were open, which essentially allows an easier custom logger with the provided .debug(...), .trace(...), etc. logging functions and would allow custom initializers to easily add console/file loggers with all the other provided functions.

This can of course be overcome by copy-pasting those functions into a custom logger but seems more beneficial and cleaner this way. I understand if this isn't the intended purpose of the Puppy class and can be dismissed.

With Xcode 14.1 I have found that running the basic usage example from the readme is quite temperamental. Either both the messages are logged or nothing is logged. Adding a fatalError or print("hi") or sleep(10) to the end of the example all seem to fix the issue. This leads me to believe that it is something to do with batching of logs not working correctly when the last bit of code to run is a line that logs a message.

I have provided the simple Swift package that reproduces this issue. To reproduce the issue just run swift run multiple times and eventually you should get lucky and no log messages will be printed. Let me know if you need any additional context or you can't reproduce the issue yourself.

LogTest.zip

Since FileRotationLogger.init() takes a LogLevel (Puppy) as constructor paremeter, perhaps this extension should be public?
Or maybe FileRotationLogger can can take Log.Level (swift-log) as parameter instead?

Hey,

I've been implementing a nice Codable extension to some of Puppy's stuff to eliminate a bunch of boilerplate from our server-side swift project.

Apparently the referenced code is internal, and I've been boggled the whole day whether it's a design decision or just hapaned to be that way?

Of-course, if the answer is the latter, if you don't mind I would love to open a PR to open that field to the public... should be safe.... it's a let and it's a minor change, non-breaking

I'd like to make a custom logger that would log to a new target, specifically in my case Firebase Crashlytics. Looking at ConsoleLogger which inherits from BaseLogger, I would think that all I needed to do would be to define my own CrashlyticsLogger with an overridden log() function.

However, since the BaseLogger class is defined as public instead of open I'm not able to do so, with the compiler error: Cannot inherit from non-open class 'ConsoleLogger' outside of its defining module.

Is there some other mechanism that I should be using to make custom loggers? 🤔

For some reason I have to create my own copy of PuppyLogHandler so I can enable/disable debug logging centrally.
Whenever I try to use PuppyLogHandler (a single instance for all Loggers) and then set the logLevel, it's not taken into account.
However, If I try my cloned class, it works.
Pretty strange.

The only issue is that I can't use the puppy.logMessage(...) method because it's not public.

Is it possible to expose it? Or alternatively allow the individual methods (error, trace,...) to also pass swiftLogInfo?

Here's how I'm using it:

private static var handler: PuppyLogHandler?

private static func initLogging() {
// ... (more code)
        let puppy = Puppy(loggers: loggers)
        handler = PuppyLogHandler(label: "\(LOGGING_PREFIX).PuppyLogHandler", puppy: puppy)

        // Set the default logging level.
        #if DEBUG
        handler?.logLevel = .debug
        #else
        handler?.logLevel = .info
        #endif

        LoggingSystem.bootstrap { _ in
            return handler!
        }
    }
    
    static func enableDebugLogging(_ enableDebugLogging: Bool) {
        handler?.logLevel = enableDebugLogging ? .debug : .info
    }

Hey @sushichop 👋🏻

First of all, thank you for working on Puppy! Puppy is my go-to Swift logging lib, I really appreciate the amount of work you are investing in it.

Now to the issue. There is a section in the README, which shows how to use LogFormattable. In this example, the date value is returned by the dateFormatter function:

struct LogFormatter: LogFormattable {
    func formatMessage(_ level: LogLevel, message: String, tag: String, function: String,
                       file: String, line: UInt, swiftLogInfo: [String : String],
                       label: String, date: Date, threadID: UInt64) -> String {
        let date = dateFormatter(date)
        let fileName = fileName(file)
        let moduleName = moduleName(file)
        return "\(date) \(threadID) [\(level.emoji) \(level)] \(swiftLogInfo) \(moduleName)/\(fileName)#L.\(line) \(function) \(message)".colorize(level.color)
    }
}

dateFormatter looks like this:

@Sendable
public func dateFormatter(_ date: Date, locale: String = "en_US_POSIX", dateFormat: String = "yyyy-MM-dd'T'HH:mm:ss.SSSZZZZZ", timeZone: String = "") -> String {
    let dateFormatter = DateFormatter()
    dateFormatter.locale = Locale(identifier: locale)
    dateFormatter.dateFormat = dateFormat
    dateFormatter.timeZone = TimeZone(identifier: timeZone)
    return dateFormatter.string(from: date)
}

You see, calling the DateFormatter() constructor is a really expensive operation, and in this case, it is being called on each log event. Performance can be greatly improved by pre-initialising the DateFormatter() object, and injecting it into the formatMessage function.

There are multiple ways to address this, would you like me to create a PR?

• pattern 1

  • myLog.log
  • myLog.log.1
  • myLog.log.2

• pattern 2

  • myLog.log.20210419T1234.567Z
  • myLog.log.20210520T2345.678Z
  • myLog.log.20210621T1123.123Z

• pattern 3

  • myLog.log0_.20210419T1234.567Z
  • myLog.log.1_20210520T2345.678Z
  • myLog.log.2_.20210621T1123.123Z

Build error:
:0: error: missing required module 'CPuppy'

When using an existing directory for log files, Puppy will try to delete all files in the directory when rotating logs, not just the Puppy logs.
To solve this I put my logs into a specific directory. This is fine, but generally people may want to put logs into a generic 'log' directory.

But .... puppy is great. File rotation logging is why I use it.
Thanks.