Set log level

[iffyio]

Currently we do not explicitly set a log level and as a result we log debug messages to stdout today.
Ideally we would want to make the log level configurable via an endpoint on the admin server but for this ticket we can hard code the log level to INFO to avoid the proxy producing chatty logs.

We currently create the base logger in two places today - one for the actual proxy and the other one for tests.
Not sure what the difference is atm but if possible we can remove the one used in tests and have only one function for creating logger.
Also, the logger creating functions are currently done in an unrelated files (proxy/builder.rs)

• Add a dedicated logger module under src/logger.rs and move the logger creating functions over to it
• Hard code any created logger to log at level INFO
• If feasible, remove the function entirely for creating loggers in tests, and reuse one function across the code base

[markmandel]

So this is interesting, because I assumed this was controlled by Rust compilation settings more than anything else.

I.e. a release build of Quilkin does not output debug logging (I actually assumed the debug statements don't even get included in the binary).

[markmandel]

So coming back around to this:

• By default, slog log levels are controlled by the build release process. So if you cargo build --release on info level or above log messages are output. Not against being able to overwrite the log level on a release build though. The other option is to have a debug binary and a production binary.
• I've always liked that the log level was defaulted based on the compilation level the binary was built at - debug gets debug logging, release gets info.
• If we go down this path slog_atomic might be nice so that we can switch on debugging on an already running process.
• There are two base loggers because the production logger is async for performance reasons. In tests this can be problematic when using println statements to debug, as it is non deterministic the ordering output will present in - so a sync logger is much more convenient. (Also I think the formatting for tests is structured, but not json, as that is more human readable)

Thoughts?

[iffyio]

Thanks for clarifying the test loggger scenario! The issue with compiling away log levels is mainly that at runtime we can't e.g temporarily enable them. From the slog docs, only trace level is compiled away by default both for debug/release binaries which sounds fine to rely on while debug->error is toggle-able at runtime, we can log at a hardcoded default level INFO and later use slog_atomic?

[markmandel]

To copy paste (source):

Note: At compile time slog by default removes trace and debug level statements in release builds, and trace level records in debug builds. This makes trace and debug level logging records practically free, which should encourage using them freely. If you want to enable trace/debug messages or raise the compile time logging level limit, use the following in your Cargo.toml:

It looks like debug logging is removed in the release binaries, but not in debug builds - which makes sense.

So actually in release builds won't be able to have debug build output anyway, unless we explicitly allow it (which I don't think we should really do - but we should have debug builds available)

[iffyio]

Ah I see, I misread the other one as trace for both. I don't see a reason to disable debug for release atm, since then there's no alternative to e.g temporarily fetch detailed log info at runtime. We can not compile debug level away?

[markmandel]

I don't see a reason to disable debug for release atm

By default, we compile away debug for the release build. So release is maximum performance.

So reverting what I said previously - let's not have log level be switchable at runtime. I would advocate you either run the debug build or the release build, and that's how you control the log levels.

I'm not sure, but I have a feeling we are in furious agreement here? 😄

[iffyio]

The use cases I had in mind re switching at runtime were scenarios where something is wrong in production and you want logging of what's going on, that won't be possible currently since those log lines don't exist. I guess if that's actually a thing that someone needs down the line we can have it switchable at runtime and until then we can use the default behavior of compiling debug away for release.

[markmandel]

I guess if that's actually a thing that someone needs down the line we can have it switchable at runtime and until then we can use the default behavior of compiling debug away for release.

Yeah, I think that makes sense - let's see what usage patterns should be.

Do we have agreement that we should ship both prod and debug builds though?

[iffyio]

Do we have agreement that we should ship both prod and debug builds though?

Ah I was thinking release vs debug builds means when a user or dev compiles it themself. But rather multiple official builds of the same version for example, yeah that could be nice to have

[markmandel]

Ooh, can this also be closed now, since we have both release and debug builds?

[iffyio]

Yes we can! closing...