-
Notifications
You must be signed in to change notification settings - Fork 15
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
On the topic of including (or not, and how) the OSLoggers for a common API #49
Comments
So I do like the idea of the OS Logger but feel it wouldn't meet the design philosohy we have worked hard to maintain for so many years. TraceLog is designed to be a universal, flexible, portable, lightweight, and easy to use logging and trace facility. Let's expand on these one at a time: TraceLog Design Philosophy
Some known use casesHere are some actual companies and how they are using TraceLog.
Possible use cases
In this case on Linux, you would not want the user to have to install systemd-dev just to compile and run the scripts.
None of these examples, spanning multiple platforms, use the Journal or Unified Logging as their default. I know that Apple, in the Darwin community, will have a hard sell trying to get people to use Unified Logging directly. With Swift in place, developers don't want to be totally locked to Apple's direct method calls especially since they change their mind (remember ASL -> Unified Logging). I'd like to find a solution that meets all the design criteria TraceLog has been following. Perhaps we explore the compiler flag option you suggested earlier for explicitly specifying inclusion of the Journal writer when needed? Other ideas? |
I believe I found a solution, the following can be placed at the beginning of the import Foundation
func canInclude(_ file: String) -> Bool {
return FileManager.default.fileExists(atPath: file)
}
if canInclude("/usr/include/systemd/sd-journal.h") {
print("sd-journal available.")
} else {
print("sd-journal NOT available.")
} We could use that to conditionally see if We need to do more research on this but I've tested the code above on both OSX and Linux 16.04 and it runs.
Do you have any cycles to research this more and find out about its future compatibility? Also, note that there are issues with this solution as well. 1) the JournalWriter would have to still be a module so we could conditionally compile or not compile the module. 2) Being a module means that the developer will have to import the module specifically in his code. 3) For portable applications, you will still require #if in order to import and configure TraceLog (but only at the configure step). So startup code may look like this import TraceLog
#if canImport(TraceLogJournalWriter)
import TraceLogJournalWriter
#endif
#if canImport(TraceLogJournalWriter)
TraceLog.configure(writers: [JournalWriter()])
#else
TraceLog.configure(writers: [<Some other writer goes here>])
#endif The second part of that code (using #if for configuring) will have to be done no matter what solution we select when you are using Writers that are not universal between platforms. Unless we made a single The design of this is also more fragile as Swift is ported to new platforms. A system logger may not be available or have vastly different configuration options required. The major benefit is that it would allow you for all platforms to simply write: import TraceLog
import TraceLogSystemLogWriter /// This import only required for the configure, all other files just need to import TraceLog
TraceLog.configure(writers: [SystemLogWriter()]) I believe it would still make sense to keep it a separate module given the dependency, but since its portable, there are no conditions that have to be put into the package file for the developer and none in the code. 🥇 But, I'm liking this option more and more if we can keep it as a separate module so we don't have the systemd dependency in TraceLogs core. We would also have to work out a mapping for the various platforms that is portable. |
Yes! This is what I think is the best option for the users. Specially if we do it like you say:
Which should be the best approach. If I understood what you meant here correctly: import TraceLog
import TraceLogSystemLogWriter /// This import only required for the configure, all other files just need to import TraceLog
TraceLog.configure(writers: [SystemLogWriter()]) That means using this design?
When a user wants to use Is that so? I really like that design! I think it's actually very clever to keep having TraceLog itself be lightweight while allowing for a very easy extension of it in user code, by e.g. just instantiating a Regarding your observations:
I think both are fair, and we should have those in mind if we're gonna do it like this. What about this approach?
|
@felix91gr I haven't forgotten about this, been busy with work. I have a new version finished that combines these, still needs a little refinement. I'll release shortly in beta for you. I'm currently working on a test harness for tracelog and this writer. I'm having some trouble with naming this. I want to avoid For now, I've called it the |
That's alright! I've been busy with uni as well 😅 it has been a very assignment-heavy couple of weeks.
I think staying away of a "syslog"-esque name is a good idea. Hmm. The name should reflect what it does. How do you call the |
@felix91gr @RLovelett, the AdaptiveWriter is ready for you to integrate into langserver_swift. You can find it here TraceLogAdaptiveWriter 1.0.0-beta.4 Should be ready to go on both Linux and OSX. |
Tony, thanks for the AdaptiveWriter. I've been away for a while, sorry about that. I just didn't know how to talk about this... so I'll just say it: I'm leaving the Swift community, for personal reasons. I don't want to talk about it here in the open, but if you wanna have a private chat I can give you more details. Sorry for not reporting earlier. I really didn't know how to address the issue and just ended up procrastinating it. Thank you for your commitment and care in making TraceLog's System Writer cross-platform. I'm sure Ryan and many others will make good use of it 🙌 As for me, this is farewell. Have a good one |
@felix91gr Very sorry to see you go, it was great working with you. Let's connect privately on LinkedIn https://www.linkedin.com/in/tonystone/. Send me a request to link. |
Closing this issue as the AdaptiveWriter is now complete and accessible at https://github.com/tonystone/tracelog-adaptive-writer. @felix91gr Thank you for driving this to a portable logger for Linux and Darwin and for all the great help!!! Loved working with you on this. |
Okay. So I've asked in the forums about how to tackle the issue that @tonystone presented at #44 here:
I think that you're right, Tony. But I feel like it's better to have it be included by default. If you agree, the method provided in the forums opens up a (mostly) clear way.
But here's my justification for wanting it be included by default: in a server environment, where you'd be more restrictive on what packages you install, you'd (almost certainly) want to put your log entries in the system journal so that's alright.
That leaves us with the desktop users, both in Mac and Linux, and the users in mobile Darwin (iOS, WatchOS) platforms.
Let's first tackle where the Darwin (both desktop and mobile) users lie right now. As it stands, the 4.0.0 release gives them
os_log
logging. If an app or library developer chooses to use it, no problem! And if they don't... good as well! The current TraceLog code wouldn't change at all.That leaves us with the Linux desktop users. If you were in Windows, you'd be reticent to install anything and with good reason. But in Linux, specially if you're using
apt-get
(Debian) or another trusted, signed repository, you know it doesn't contain malicious software, and can verify the integrity of the binaries. The two main concerns would then be (a) Breaking another package or (b) Binary size. In the case of Python packages, (a) is completely justified because setting it up correctly requires a bit of black magic. In this case, withlibsystemd-dev
being a OS-provided library, that should never happen.(b) Is the last concern. I went and looked up how much
systemd-dev
weighs, and it's less than 1 MB!, which is actually less than I expected 😅.So all in all, I think it makes sense for it to be included by default.
Now from an API standpoint, I think we should either:
This would make the API uniform in all platforms, which brings a lot of benefits. Take for example a library author who wants to use TraceLog in the current configuration for OS logging, who might be testing it only on MacOS first. Then when they try to port it to Linux, they find themselves having to use a bunch of
#if os(...)
,#elseif
statements because the TraceLog API they were using is not available under Linux.If instead the API was uniform, when they started porting it to Linux they would install
systemd-dev
and after that everything would behave in exactly the same way as it does on MacOS. And that, I think, would be a great relief 😌I will write up on ideas of how to implement this afterwards because my hands are very tired at the moment 😅
Please, lemme know what you think 🙂
The text was updated successfully, but these errors were encountered: