StdoutLogHandler should be public

[allenhumphreys]

I'm looking at being an early adopter of SwiftLog because it looks good and I'm tired of solving the logging issue every time I start a new project. It occurs to me though, that StdoutLogHandler should be public but it is marked as internal.

The StdoutLogHandler is great because it provides out-of-the-box console logging to get up and moving fast and prevents the need to re-implement that, but being marked internal means that as soon as the use-case gets a little more interesting, I'd have to write my own console log handlers. The use case I'm referring to really just looks like this:

LoggingSystem.bootstrap {
  MultiplexLogHandler([StdoutLogHandler(label: $0), RemoteLogHandler(label: $0)])
}

I suppose MultiplexLogHandler could have some kind of configurability to implicitly create a StdoutLogHandler in addition to whichever log handlers are passed in. This could be an alternative to exposing StdoutLogHandler if that is truly the desire.

It's also worth pointing out that StdoutLogHandler's initializer is explicitly marked public. So I'm wondering if this has been an oversight.

I can open a PR to make a change if needed.

[ktoso]

Hi Allen, really cool that you're looking to adopt the API, and thanks for chiming in!

As you notice, it is early days here, and not many implementations of this API exist yet. Our hope is that soon someone would implement a much better standard out logger that would support formatting etc. It is true though that the built in one provides a nice out of the box default...

I think it sounds good on making it public, it seems to have been intended to be so; and it is accessible in any case for using it via "not configuring anything"; We could also address the docs comment on it while we're at it (perhaps something more clear that it's "boring" :-)), mentioning that it is intended to be a bare-minimum implementation and proper log handlers with formatting are expected to be external modules?

We'll see what @weissi and @tomerd think, but sounds to me like a plan to have it publicly constructable, since we "ship it" and it is "to be used by users" anyway.

[weissi]

I'd be okay with that, but we should name it StderrHandler and make it log to stderr :). It's no good logging to stdout but I forgot to fix it...

[ianpartridge]

Why stderr? https://12factor.net/logs

[weissi]

Why stderr? https://12factor.net/logs

Command line apps are pretty unusable if you log to stdout. Imagine you do

cat myImage.HEIC | my-swift-heic-to-jpg-transcoder > myImage.jpg

that'd break horribly if you log to stdout. Obviously we could say in that case people need to install their own log handler...

[ianpartridge]

I dug around a bit.

kubectl logs aggregates stdout and stderr. Cloud Foundry does the same - https://docs.cloudfoundry.org/devguide/deploy-apps/streaming-logs.html Mesos likewise - http://mesos.apache.org/documentation/latest/logging/#Containers

Maybe the 12-factor requirement of stdout isn't realistic in practice..

[weissi]

I dug around a bit.

kubectl logs aggregates stdout and stderr. Cloud Foundry does the same - https://docs.cloudfoundry.org/devguide/deploy-apps/streaming-logs.html Mesos likewise - http://mesos.apache.org/documentation/latest/logging/#Containers

Maybe the 12-factor requirement of stdout isn't realistic in practice..

Yeah, I find it a bit odd that they require stdout... Anyway, I guess the best argument for leaving it stdout is that we shipped 1.0.0 with it and you could say it's a breaking change...

[ianpartridge]

Yeah. I do think we should make StdoutLogHandler public - for the 12-factor style of app it's useful.

[weissi]

@ianpartridge 2>&1 would make a stderr-logging thing 12-factor again, so I don't think whatever 12-factor came up with should be the deciding argument here.

All real-world deployments will definitely have to take anything on stderr too because that's where Swift crashes get written too.

[ianpartridge]

Maybe we should ship StdoutLogHandler and StderrLogHandler.

[weissi]

Maybe we should ship StdoutLogHandler and StderrLogHandler.

yeah, could just have a LogStdioHandler that takes a FILE * or a file descriptor... And make the default either LogStdioHandler(stdout) or LogStdioHandler(stderr)

[ktoso]

👍 on shipping both, makes sense and does not widen the scope of the lib too much; this still counts as "bare minimum" etc. I think.

Btw. re 12-factor apps... the rules are ok-ish... but in reality some of the hints are very weird (the configuration one + security springs to mind), so it's good as general inspiration but I don't think one should follow those when designing APIs like a bible of app design :-)

[allenhumphreys]

Thanks for all the feedback here. Just wanted to say I can make StdoutLogHandler public via a PR, but the other proposals seems a bit out of scope for me!

[ianpartridge]

@allenhumphreys I think we'd welcome that first PR :)

[allenhumphreys]

@ianpartridge I went to commit and was following CONTRIBUTING.md's advice to use dev/git.commit.template as the commit template, but found that seems to be a holdover from where this project started (SwiftNIO?).

As part of my update I could: 1) Add the template so the instructions are accurate 2) Update the instructions so they don't reference the template

[ianpartridge]

Ah, thanks for letting us know. I think we should fix that in a separate PR - @weissi?

[weissi]

I think we should fix that in a separate PR - @weissi?

Yes please :)

[tomerd]

I think we should fix that in a separate PR -

https://github.com/apple/swift-log/pull/62

[weissi]

fixed by #61

[ktoso]

Seems we missed to close this issue; Seems solved ( by #61 and see also https://github.com/apple/swift-log/blob/master/Sources/Logging/Logging.swift#L539-L549 )

[ktoso]

Assigned to 1.0.1 milestone, and closed the 1.0.0 milestone as well since it seems to have been still open even tho we tagged 1.0.0.