Crate evaluation for 2017-05-16: log

[budziq]

Awesome thanks so much @gamazeps! If you’ve got any questions please feel free to check in here and ask. And if anyone else would like to also work on cookbook examples here, having multiple examples to work with is always a great “problem” to have!

Hi, I do not know if this is the place for such questions/suggestions.

I would be really eager to participate in the cookbook but as someone completely new to rust, its community and discourse I find it currently a little hard to approach (even though I have already submitted few examples) .

What is the canonical way to submit cookbook ideas and notify about ones willingness to implement them? Is it via replying to lib-blitz discourse threads or via github issues as mentioned in CONTRIBUTING.md?

I for one would opt for a centralized github based approach similar to the already created Url crate tracking issue: https://github.com/brson/rust-cookbook/issues/40. It makes it a little more approachable for newcomers like myself and helps to avoid work duplication (I would not know about log examples being ready for implementation if I did not stumble here by chance).

I am really looking forward to helping out and learning rust along the way!

[Nemo157]

While structured logging is nice, the big thing I can see that slog provides over log is contextual loggers. Even more importantly these drastically affect the API surface. From slog's docs:

• contextual - slog's Logger objects carry set of key-value data pairs that contains the context of logging - information that otherwise would have to be repeated in every logging statement.

I have to admit, I have not used either of these crates, trying them both out is still on my todo list. But I have worked with contextual loggers in the past in a C# web application, and found them super useful in terms of being able to inject data into log lines from other libraries. At the start of a request a logger containing the current request id is created, then that is passed everywhere that might log during the request so all log lines can be correlated with the request. Especially when using a multi-threaded service with async I/O this is really helpful as an individual request would bounce around between threads as different async operations complete.

[brson]

I’ve created a cookbook issue for log examples, and posted a PR with placeholders for them.

I did not create a placeholder for the syslog example because I don’t know which crate implements log for the syslog.

I also updated the op to include an issue for adding links to crates that implement the log interface to the README and crate docs. The only logging implementation I actually know of is env_logger!

[luser]

I have to admit, I have not used either of these crates, trying them both out is still on my todo list. But I have worked with contextual loggers in the past in a C# web application, and found them super useful in terms of being able to inject data into log lines from other libraries. At the start of a request a logger containing the current request id is created, then that is passed everywhere that might log during the request so all log lines can be correlated with the request. Especially when using a multi-threaded service with async I/O this is really helpful as an individual request would bounce around between threads as different async operations complete.

We’re using log + env_logger in sccache, and having some sort of contextual logger would be incredibly useful. The sccache server uses tokio and can handle multiple requests at a time, so we wind up prefixing all log messages with the object file or crate name being compiled, like so. Since we’re using Futures all over the place we wind up cloning that String everywhere. It would be nice to have a more straightforward way to do that.

[sfackler]

We should certainly make sure the structured stuff is designed such that this can be done in a separate crate. I want to keep log itself as minimal as possible - it should be very low cost for any library crate anywhere to depend on it and log things.

[alexcrichton]

Ok I’ve filled out the evaluation template for C-REEXPORT and C-HIERARCHY, turned out to be not too hard as there’s no public modules in the log crate!

[brson]

It’s not clear to me whether some incomplete evaluation tasks have been claimed or not, so I added a section to the end of the template for listing tasks that people have volunteered for, and I added it to this op, and signed myself up for the “naming” and “interop” sections, which I will do shortly.

I also think that for simplicity’s sake we should stick to the log crate here, and worry about env_logger later. Otherwise we basically have to do the evaluation twice and create a whole second checklist. I’ll update the blitz thread to reflect this.

[brson]

I did the “naming” and “interop” sections and updated the op with the following:

Crate issues

• Implement Debug for LogMetadata, LogRecord
• FromStr impls for LogLevel, LogLevelFilter use () as error type

Guidelines changes

• C-CONV-TRAITS should discuss FromStr

Discussion topics

• Can any of these Debug representations be empty, per C-DEBUG-NONEMPTY?
• Possible trait impls
  • LogLocation: Eq/PartialEq, Ord/PartialOrd, Hash, Clone
  • LogMetadata: Eq/ParialEq, Ord/PartialOrd, Hash
  • LogRecord: Eq/PartialEq, Ord/PartialOrd, Hash
  • LogLevel: Hash
  • LogLevelFilter: Hash
• Should anything here implement serde?
  • If not what does that say about the guidelines C-SERDE?

[brson]

I’m going to do the docs checklist next.

[brson]

I also did “debuggability”.

[brson]

Updated the op with the following for documentation:

Crate issues

• Document what happens when log is initialized more than once
  • crate docs say “may only be initalized once”, not why, or what happens if you initialize twice
• Use env_logger instead of my_logger in example
  • env_logger is something a user can try themselves
• Crate docs should mention the logging levels and corresponding macros
• Add example for log! macro
  • using both variants
• Document max_level_*, release_max_level_* features in crate docs
• Add examples to debug!, error!, info!, trace!, warn!
  • Both pattern variants
• Expand log! docs for max_level_*
  • Seems like a non-sequiter here, but other logging macros have better explanations of this feature
• Add example for second log_enabled! pattern variant
  • There’s an example, but it only covers one variant of the macro
• LogLocation doc improvements
  • How are these values created?
  • Are these for logger implementors or log user?
  • Add an example
• LogMetadata doc improvements
  • How are these values created?
  • Are these for logger implementors or log user?
  • Add an example
• LogRecord doc improvements
  • How are these values created?
  • Are these for logger implementors or log user?
  • What’s the difference between the ‘level’/‘target’ accessors here and on the attached LogMetadata?
  • Add an example
• SetLoggerError doc improvements
  • Add links to functions
• ShutdownLeggerError doc improvements
  • Add links to functions
• LogLevel doc improvements
  • Do end-users use this type or just loggers?
  • Add link to LogLevelFilter
• LogLevelFilter doc improvements
  • Do end-users use this type or just loggers?
  • Add link to LogLevel
• max_log_level
  • Add links
• set_logger docs
  • Add links
  • Add “Errors” section
  • Add example
• set_logger_raw docs
  • Add links
  • Add “Errors” section
  • Add example
• shutdown_logger docs
  • Add links
  • Add “Errors” section
  • Add example
• shutdown_logger_raw docs
  • Add links
  • Add “Errors” section
  • Add example
• Add links to crate docs
• Add CI badges to Cargo.toml
• Change Cargo.toml ocumentation key to point to https://docs.rs/log
• Add keywords to Cargo.toml
• Change html_root_url to https://docs.rs/$crate/$version

Guidelines changes

Discussion topics

• Are there any panic conditions in this lib? Not documented.
• There are several “musts” about usage here that are kinda scary
  • should we say more about why?
  • e.g. “This function may only be called once in the lifetime of a program, and may not be called before set_logger”
    • This is a safe function and I sure can do what is says I can’t

[seanmonstar]

A question I’d add for the discussion: do all the types need the Log prefix? LogLevel, LogRecord, LogMetadata, etc? Usually we like to say that there shouldn’t be a stutter, that a crate or module name is fine for differentiation (log::LogRecord vs log::Record).

[brson]

I filled out the “future proofing” and “necessities” sections. And I discovered something that confused me really badly about the documentation on docs.rs: the README is also displayed there. I didn’t bother evaluating the README as we have no guidelines, but we need to figure out how we think README should be handled.

[brson]

@seanmonstar I added it to the discussion points.

[daboross]

One question I would have: is there any possibility in adding another crate “below” the log crate as an implementation detail that would handle inter-op between multiple versions of the log crate?

Changing log::LogRecord and log::LogMetaData to log::Record and log::Metadata is something I support, but having a breaking change in log would definitely split the ecosystem with users being unable to have loggers which receive records from both log 0.3 and 0.4 crates.

Could we create a log_dist crate which is meant only for use from within log which provides a very minimal API matching that of the log crate?

This way the log crate itself could be upgraded with incompatible changes (or to a 1.0 state after it’s been fully checked out), and it’d just have to call the same things in log_dist. It could be a non-breaking upgrade to add log_dist as a backend to log 0.3.x.

[sfackler]

Handling the 0.3 to 0.4 or 1.0 or whatever bump is definitely really important. Hopefully we can have 0.3 loggers “just work” via some hacky magic.

[dpc]

Have anyone thought about changing the default compile time logging level?

In slog I made release builds by default ignore trace and debug and debug builds ignore trace. The rationale is: applications that need debug logging level in production can just enable it in Cargo.toml. Most apps probably don’t need it, and at times the performance hit can be substantial (especially as async logging is not implemented). Check here: https://github.com/rust-lang-nursery/log/issues/112 . 20% perf drop by using env_logger.

With trace and debug being by default disabled, developers are encouraged to use trace and debug and leave it inside the code without fear of slowing things down.

[gamazeps]

I’ll try to get them done by then end of the week or during the weekend in the worst case scenario if that sounds good to you.

Awesome thanks so much @gamazeps! If you’ve got any questions please feel free to check in here and ask. And if anyone else would like to also work on cookbook examples here, having multiple examples to work with is always a great “problem” to have!

Well the case was worst than I thought, should be done by Wednesday I think (I got caught by administrative things).

[LilianMoraru]

Just throwing an example of a non-traditional(from log's API point of view) logging ?style?: I was also working(sadly, no plans to work on it in the next 3 months) on a logging library based on log: dlt-rs. For context?: DLT is very popular in the Automotive industry. In DLT you usually register a context(Ctid in the image from the link I posted - used for filtering) and when calling the log level, we also pass the Context ID(C++ example):

qCDebug(MyContext) << "Hello";

More info about the initialization steps, log levels, etc…: https://at.projects.genivi.org/wiki/display/PROJ/DLT+cheatsheet

At work we use DLT through Qt’s logging framework. Calling qCDebug would actually call QMessageLogger(links mostly for inspiration). Observe that the log levels also have function overloads with QLoggingCategory as a parameter - this covers DLT’s Context ID use-case. qCDebug by default calls Qt’s logging implementation(think env_logger) but we swap it with our implementation(dlt-rs?) with 1 header include.

Now to the issue: At the moment, log's macros are not very friendly to this use-case. There are workarounds of course - own macros for example. I am not even sure if log needs to cover such a use-case - just wanted to throw the info out there.

[alexcrichton]

Ok it’s now almost time for the review of the log crate! We’ll convene tomorrow 2016-05-16 at 1pm PST for a recorded video session to discuss this crate. I’ll be sure to post a link to the recording once it’s available.

In the meantime I’ve filled out the remaining minor points of evaluation (thanks @brson for filling out big chunks!) and I think we’re ready to go. I’ve copied everything and started trimming it down a bit into https://public.etherpad-mozilla.org/p/rust-crate-eval-log where we’ll take notes during the meeting. I’ll be sure to copy everything new back to this thread though for continuing the discussion.

Thanks again to everyone who’s participated so far! We’ve got a great list of discussion topics and there’s steady progress implementing examples in the cookbook. In the meantime I’ve added “migration away from 0.3” and “default log levels” to the discussion points, thanks for bringing those up @daboross and @dpc!