Skip to content
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

Closed
felix91gr opened this issue Jun 14, 2018 · 9 comments
Closed
Assignees

Comments

@felix91gr
Copy link

felix91gr commented Jun 14, 2018

Okay. So I've asked in the forums about how to tackle the issue that @tonystone presented at #44 here:

I chose to keep it separate for one primary reason which is that I don't want to force all Linux users to have to install systemd-dev just to use TraceLog. There are many applications that do not have a need for that Writer and just want to link to TraceLog and go.

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, with libsystemd-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:

  • Include OS logging in both (Darwin, Linux) platforms by default.
  • Not include OS logging in both platforms, by default, and include it with a compile-time option (e.g by reading an environment variable at the Package manifest).

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 🙂

@tonystone
Copy link
Owner

tonystone commented Jun 14, 2018

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

  1. Universal: With TraceLog you are not locked into one type of logging system, as a matter of fact, you can choose to use a combination of loggers to write to various endpoints and systems (more on this later).
  2. Flexible: With TraceLog you can filter messages at development time as well as production time. Choose whatever combination of Writers and filters that work for your particular use case. Write your own custom loggers to plug into TraceLog for customized use-cases. In one customer use, they had 35+ different modules/libraries that made up their app (each using TraceLog). With TraceLog, each module team could filter out all other module output so they could focus on debugging their particular subsystem.
  3. Portable: At this writing, TraceLog is one of the few logging systems that was designed to run on all swift supported platforms and run in 2 languages (Swift and Objective-C).
  4. Lightweight: TraceLog's footprint is small and efficient. It's designed and meant to be as efficient on resources as can be and also optimize itself out if required by the use case.
  5. Easy to use: TraceLog can be used right out of the box with No setup or special dependencies. This was designed in and we've worked hard to maintain that over the years. You can literally link to it and start adding log statements to your app and get useful output on any platform. There is no need to even call configure unless you require the features that configure offers or you require it to read the environment on startup. In the latter case, we kept it as simple as TraceLog.configure() with no params. This forces TraceLog to read the process environment for it's filtering commands.

Some known use cases

Here are some actual companies and how they are using TraceLog.

  • Company A: iOS: Using a ConsoleWriter, a custom FileWriter, and a custom endpoint logger that logs to their Spark stream in real time for their app.
  • Company B: iOS: Uses a straight ConsoleWriter for inline development of their application. It was meant for tracing through the application while in development to give the developers visibility into the threading and workings of the app. In production, they use TraceLog's feature which optimizes out TraceLog completely so there is no overhead for the production release.
  • Company C: iOS: Using File and Console logging. File logs are shippable from the application to support for diagnosis.

Possible use cases

  • Scripts: Your own use in scripts was interesting and useful. I'd like to see the use of this as simple as possible. In a normal Swift script, it could be as simple as:
	TraceLog.confgure(mode: .direct)

	logInfo { "Starting script MyScript..."  }

In this case on Linux, you would not want the user to have to install systemd-dev just to compile and run the scripts.

  • Portable application using the vendor encouraged logger on each (your use case): Having TraceLog include the SDJournalWriter on Linux would definitely be an advantage in this case.

  • Server-side systems (resembling Apache, NGINX, Tomcat, etc): These systems typically write to various outputs and can include syslog but traditionally they capture their logs into a file. NGINX, in particular, has stdout, file, and syslog with a file being the default. The others write to a file as default as well as their defaults.

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?

@tonystone
Copy link
Owner

tonystone commented Jun 14, 2018

I believe I found a solution, the following can be placed at the beginning of the Package.swift file. I'm not sure if this is legal or how fragile it will be though so I'd like to research.

   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 sd-journal.h is available and include JournalWriter if it is. Of course, we'd update that function to search the header include path for the sd-journal.h file instead of using a hardcoded path.

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.

Note: on Linux when you print to the screen it gives error: manifest parse error(s): sd-journal available.. If you don't print though it compiles cleanly.

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 SystemLogWriter (I think this is what you mean by OSLog) that adapts its internals to whatever platform it's running on (in other words combine the 2 Writers into one). This essentially moves the abstraction from TraceLogs core to the Writer itself. It could get ugly internally, and the complication with the public interface (the constructor) is that all platforms have different requirements such as log priority/level mapping and tags.

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.

@felix91gr
Copy link
Author

felix91gr commented Jun 16, 2018

Unless we made a single SystemLogWriter (I think this is what you mean by OSLog) that adapts its internals to whatever platform it's running on (in other words combine the 2 Writers into one). This essentially moves the abstraction from TraceLogs core to the Writer itself.

Yes! This is what I think is the best option for the users. Specially if we do it like you say:

if we can keep it as a separate module so we don't have the systemd dependency in TraceLogs core.

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?

  • TraceLog: logging library with instant cross-platform support for all of its features.
  • TraceLogSysLogWriter: small library that expands TraceLog, giving you a cross-platform SystemLogWriter for your OS.

When a user wants to use TraceLog for stdout, they just work with the base library. When they want to add System Logging to their apps/libraries, they add TraceLogSysLogWriter as a dependency (and install packages like systemd-dev if the OS requires it), and in the configure step they use something like the snippet you wrote: they import the TraceLogSysLogWriter library and configure TraceLog to use a SystemLogWriter for the logging output.

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 SysLogWriter. Not that it wasn't like that already 😆. But I think a separate TraceLog Writer module fits the design of TraceLog itself very well.


Regarding your observations:

It could get ugly internally, and the complication with the public interface (the constructor) is that all platforms have different requirements such as log priority/level mapping and tags.

We would also have to work out a mapping for the various platforms that is portable.

I think both are fair, and we should have those in mind if we're gonna do it like this.

What about this approach?

  • default: a default conversion scheme from TraceLog's logging levels and the OS' logging levels. This is included in the SysLogWriter class' implementation.
  • optional: an optional input in the SysLogWriter constructor, that maps TraceLog's logging levels in a different way. This requires a user that knows what they want in the different syslogs, and therefore it makes sense that they would need to use #if, #elseif for creating the different OS-specific dictionaries. It would help if there was a static, readonly dictionary that exposed how the default conversion is, such that the user could just modify a copy of it instead of making it from scratch. But that's just an implementation detail.

@tonystone
Copy link
Owner

@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 SystemLogWriter because I believe there is a need for a SysLogWriter (syslog is available on all platforms and is still widely used) as well and the names would be too close.

For now, I've called it the AdaptiveWriter, any thoughts on alternatives for a name?

@felix91gr
Copy link
Author

I haven't forgotten about this, been busy with work.

That's alright! I've been busy with uni as well 😅 it has been a very assignment-heavy couple of weeks.

I'm having some trouble with naming this. I want to avoid SystemLogWriter because I believe there is a need for a SysLogWriter (syslog is available on all platforms and is still widely used) as well and the names would be too close.

For now, I've called it the AdaptiveWriter, any thoughts on alternatives for a name?

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 os.log journal? Maybe we could call the writer something like OS<<journalName>>Writer. Another idea would be to call it just OSLogWriter, because that's what it does. Or AdaptiveOSLogWriter.

@tonystone
Copy link
Owner

tonystone commented Jun 26, 2018

@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.

@tonystone tonystone self-assigned this Jun 27, 2018
@felix91gr
Copy link
Author

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 ☺️

@tonystone
Copy link
Owner

tonystone commented Jul 23, 2018

@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.

@tonystone
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants