Crate evaluation for 2017-05-16: log

log evaluation for 2017-05-16

For more information see the internals libz blitz thread.

This post is a wiki. Feel free to edit it.

Links

Pre-review checklist

Do these before the libs team review.

  • Create evaluation thread based on this template
  • Work with author and issue tracker to identify known blockers
  • Compare crate to guidelines by filling in checklist
  • Record other questions and notes about crate
  • Draft several use case statements to serve as cookbook examples
  • Record recommendations for updated guidelines
  • Publish evaluation to coordination thread for review and notify reviewers

Post-review checklist

Do these after the libs team review.

Guidelines checklist

Copied from GitHub - rust-lang/api-guidelines: Rust API guidelines to work through

Legend

  • [y] - guideline is adhered to, no work needed.
  • [n] - guideline may need work, see comments nearby
  • [/] - guideline not applicable

Checklist:

  • Organization (crate is structured in an intelligible way)
    • [y] Crate root re-exports common functionality ([C-REEXPORT])
    • [y] Modules provide a sensible API hierarchy ([C-HIERARCHY])
  • Naming (crate aligns with Rust naming conventions)
    • [y] Casing conforms to RFC 430 ([C-CASE])
    • [y] Ad-hoc conversions follow as_, to_, into_ conventions ([C-CONV])
      • to_log_level_filter, to_log_level
    • [/] Methods on collections that produce iterators follow iter, iter_mut, into_iter ([C-ITER])
    • [/] Iterator type names match the methods that produce them ([C-ITER-TY])
    • [/] Ownership suffixes use _mut and _ref ([C-OWN-SUFFIX])
    • [/] Single-element containers implement appropriate getters ([C-GETTERS])
  • Interoperability (crate interacts nicely with other library functionality)
    • [n] Types eagerly implement common traits ([C-COMMON-TRAITS])
      • Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash Debug, Display, Default
      • see discussion
    • [n] Conversions use the standard traits From, AsRef, AsMut ([C-CONV-TRAITS])
      • It does use FromStr
    • [/] Collections implement FromIterator and Extend ([C-COLLECT])
    • [n] Data structures implement Serde's Serialize, Deserialize ([C-SERDE])
    • [n] Crate has a "serde" cfg option that enables Serde ([C-SERDE-CFG])
      • see discussion
    • [/] Types are Send and Sync where possible ([C-SEND-SYNC])
      • can't tell
    • [y] Error types are Send and Sync ([C-SEND-SYNC-ERR])
      • looks like it. hard to tell
    • [n] Error types are meaningful, not () ([C-MEANINGFUL-ERR])
      • <LogLevel as FromStr>::Err
      • <LogLevelFilter as FromStr>::Err
    • [/] Binary number types provide Hex, Octal, Binary formatting ([C-NUM-FMT])
  • Macros (crate presents well-behaved macros)
    • [y] Input syntax is evocative of the output ([C-EVOCATIVE])
    • [/] Macros compose well with attributes ([C-MACRO-ATTR])
    • [/] Item macros work anywhere that items are allowed ([C-ANYWHERE])
    • [/] Item macros support visibility specifiers ([C-MACRO-VIS])
    • [/] Type fragments are flexible ([C-MACRO-TY])
  • Documentation (crate is abundantly documented)
    • [n] Crate level docs are thorough and include examples ([C-CRATE-DOC])
      • some suggestions in discussion
    • [n] All items have a rustdoc example ([C-EXAMPLE])
    • [/] Examples use ?, not try!, not unwrap ([C-QUESTION-MARK])
    • [n] Function docs include error conditions in "Errors" section ([C-ERROR-DOC])
    • [/] Function docs include panic conditions in "Panics" section ([C-PANIC-DOC])
      • no panic conditions?
    • [n] Prose contains hyperlinks to relevant things ([C-LINK])
    • [n] Cargo.toml publishes CI badges for tier 1 platforms ([C-CI])
    • [n] Cargo.toml includes all common metadata ([C-METADATA])
      • authors, description, license, homepage, documentation, repository, readme, keywords, categories
      • missing keywords
    • [n] Crate sets html_root_url attribute "https://docs.rs/$crate/$version" ([C-HTML-ROOT])
    • [n] Cargo.toml documentation key points to "https://docs.rs/$crate" ([C-DOCS-RS])
  • Predictability (crate enables legible code that acts how it looks)
    • [/] Smart pointers do not add inherent methods ([C-SMART-PTR])
    • [y] Conversions live on the most specific type involved ([C-CONV-SPECIFIC])
    • [y] Functions with a clear receiver are methods ([C-METHOD])
    • [y] Functions do not take out-parameters ([C-NO-OUT])
    • [/] Operator overloads are unsurprising ([C-OVERLOAD])
    • [/] Only smart pointers implement Deref and DerefMut ([C-DEREF])
    • [/] Deref and DerefMut never fail ([C-DEREF-FAIL])
    • [/] Constructors are static, inherent methods ([C-CTOR])
      • no constructors
  • Flexibility (crate supports diverse real-world use cases)
    • [y] Functions expose intermediate results to avoid duplicate work ([C-INTERMEDIATE])
    • [y] Caller decides where to copy and place data ([C-CALLER-CONTROL])
    • [y] Functions minimize assumptions about parameters by using generics ([C-GENERIC])
    • [y] Traits are object-safe if they may be useful as a trait object ([C-OBJECT])
  • Type safety (crate leverages the type system effectively)
    • [y] Newtypes provide static distinctions ([C-NEWTYPE])
    • [/] Arguments convey meaning through types, not bool or Option ([C-CUSTOM-TYPE])
    • [/] Types for a set of flags are bitflags, not enums ([C-BITFLAG])
    • [/] Builders enable construction of complex values ([C-BUILDER])
  • Dependability (crate is unlikely to do the wrong thing)
    • [y] Functions validate their arguments ([C-VALIDATE])
    • [y] Destructors never fail ([C-DTOR-FAIL])
    • [/] Destructors that may block have alternatives ([C-DTOR-BLOCK])
  • Debuggability (crate is conducive to easy debugging)
    • [n] All public types implement Debug ([C-DEBUG])
    • [?] Debug representation is never empty ([C-DEBUG-NONEMPTY])
      • unclear
  • Future proofing (crate is free to improve without breaking users' code)
    • [y] Structs have private fields ([C-STRUCT-PRIVATE])
      • except in one case where it's a noted exception, and claimed to be unstable
    • [/] Newtypes encapsulate implementation details ([C-NEWTYPE-HIDE])
  • Necessities (to whom they matter, they really matter)
    • [/] Public dependencies of a stable crate are stable ([C-STABLE])
      • no deps
    • [y] Crate and its dependencies have a permissive license ([C-PERMISSIVE])

Cookbook use case statements

  • Log a debug message to the console
  • Log an error message to the console
  • Enable log levels per module
  • Log to the Linux system log
  • Log messages to a custom location
    • (implement a custom logger)

More can be added here!

Implemented so far

Crate issues

  • Link to logging sinks other than env_logger in the README/crate docs
  • Support construction of LogRecords and LogMetadata for testing and shim crates
  • Logger shutdown adds some cost/complexity to logging
  • Implement Debug for LogMetadata, LogRecord
  • FromStr impls for LogLevel, LogLevelFilter use () as error type
  • 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
  • ShutdownLoggerError 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 log - Rust
  • Add keywords to Cargo.toml
  • Change html_root_url to https://docs.rs/$crate/$version
  • Improve Debug for LogLocation, right now it has double underscores.

Style guide updates

  • C-CONV-TRAITS should discuss FromStr
  • Macros and importing, should all macro 1.0 macros be selectively importable?

Discussion topics

  • What is the log crate's relation to slog and fern?
  • Should env_logger stay in the repository or should it get split out?
  • Should shutdown of a logger be supported?
  • Colors in env_logger? (see, e.g. pretty_env_logger)
  • env_logger formatting requires creating a String (an allocation)
  • Structured logging?
  • 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?
  • 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
  • What should we do about README Rust libz blitz! - #25 by brson
  • The types here stutter in a way we usually don't do (LogLevel)
    • Should we change?
    • Is this just a legacy mistake that we should leave?
    • Does this impact the guidelines?
  • Should the static default log levels change? Should debug and trace be disabled by default in release mode? (e.g. see slog for an example)
  • Importing just one macro?
  • Migration from 0.3 to 0.4/1.0

Evaluation task assignments

  • naming checklist (brson)
  • interop checklist (brson)
  • documentation checklist (brson)
8 Likes

Thank you for volunteering to be the guinea pig on this one @alexcrichton.

These logging crates are quite mature now. Seems like it’s about time to button them up and get them to 1.0.

First thing I’d like to do is write a list of example use cases for the cookbook, and actually write those examples. We can just edit them write into the op. Examples of existing statements for precedent here. I’ll help with these today. Feel free to just edit the ideas in the op directly.

Edit: I updated the op with some ideas for logging examples.

1 Like

I think this evaluation will also cover env_logger too. Maybe other implementations? There’s an example listed to log to the Linux syslog.

Yeah makes sense to me to cover env_logger here while we’re at it (as it’s in the same repo). It seems reasonable also while we’re taking a look at env_logger to maybe come up with guidelines (if possible) for implementing loggers.

@sfackler are you aware of any outstanding major issues against either log or env_logger? Or if anyone else has issues with these crates, please mention them!

I have a few questions! :smile:

  • What will be our relationship with alternative frameworks (e.g. slog) going forward? I’m concerned that we’ll have another rustc-serialize / serde split when a more advanced solution gains traction.

    • We can work around this via shims that forward from one framework to another, so it might not be as bad as it was with rustc-serialize.
  • On that note, what are our plans around structured logging? It seems like it’ll be a breaking change to add such a feature after-the-fact.

    • I ask because my employer enforces JSON formatted logs for all of its services. While we don’t have a case for Rust just yet, other users might have similar requirements.
  • For env_logger, can color support be added in a backward-compatible way? I guess this issue isn’t as important, since breaking changes here aren’t as disruptive as in the log crate.

6 Likes

Hi @lambda-fairy.

That's a good question. Rust is going to face this situation continuously. There will always be lots of choices. I hope that the relationship between crate authors will be friendly.

When you say "our" I take that to mean "the rust project".

Personally, I want to encourage the most people to write the most and best Rust crates possible, for the ecosystem to be vibrant, and to evolve.

In the case of log for example, I doubt anybody is really in love with that crate, so I hope there will be other solutions. slog looks excellent and people should use it.

Regarding splits in the ecosystem, I think they are sure to happen, and we should expect it and expect to deal with it. We're still pretty much in the middle of the rustc-serialize -> Serde transition, but a rustc-serialize -> Serde post-mortem could make an enlightening thread. Do you see the rust-serialize / serde situation as problematic? My impression is that the situation has been pretty understandable and not so horrible, at least if one's expectations are suitably tempered.

I think we should do our best to walk a fine line between encouraging a proliferation of great Rust crates, and also working to make Rust crates more interoperable and discoverable. I think we can learn how to manage versioning and transitions in the ecosystem in a way that is reliable, and is also tolerant to change.

At least with respect to the log crate, I'd prefer not to add structured logging. My feeling about log is that it is mature and doesn't change much, so let's polish it up for 1.0 and be done. From there interested parties can regroup to think about future directions for logging in Rust. Maybe that leads to log 2.0, or a shift in focus to slog. That's just my opinion, as someone who hasn't maintained any of the log crates. @sfackler or @dpc might have opinions.

I suspect it can, but @sfackler would know better.

1 Like

Anybody interested in helping out on the log crate effort? Don’t worry if it’s not clear to you what to do – @alexcrichton and others will help hook you up with work items. But please leave a comment if you’d like to be involved!

@lambda-fairy

This is a context in which shims are a good way forward. There are some API design questions here about how to enable the shims to produce the various data structures (LogRecord for example) when performing the bridging, but it's pretty straightforward. IIRC slog already has crates to shim in both directions. This is a pretty standard approach in other languages as well (e.g. Log4j Bridge).

JSON-formatted output is basically orthogonal to the logging being structured or not. Structured logging is a pretty small extension on top of "normal" logging - just add on a parameter map to the log event state. See e.g. log4rs::encode::json - Rust for an example of JSON-formatted output from the standard log crate. All that would be added on top of that in a strucured logging world would be an extra params field.

Backwards compatibility is a bit wishy washy here since this isn't a Rust-level API. We could certainly add an extra control bit to turn on colored output, or even to it by default when hooked up to a TTY/console.

@brson[quote="brson, post:6, topic:5185"] In the case of log for example, I doubt anybody is really in love with that crate [/quote]

I think it's pretty solid. Is there any particular badness you're aware of?

Structured logging would not be a particularly large change on top of log - we could probably pull it off in a backwards compatible way right now if we wanted to. The only real design questions are what the structured log value type is (I just use Display for some stuff on an internal framework at work, slog uses a trait that allows you to get primitive types as well as strings) and what the log invocation syntax would look like.

Some other miscellaneous bits:

  • I'd like to remove the ability to shut down the logger. It exists because we used to try to do this with an atexit callback but turned that off because it deadlocks in some cases on Windows. With it gone, we can avoid two atomic add calls on each log event.
  • env_logger is a bit of a weird thing - It's basically just the rustc logger split out into its own crate when I rewrite log into a facade. I don't think anyone on the libs team has a particularly strong vision for what it would be. I'd like to spin it off out of the main log repo and let someone who does feel strongly take ownership over its evolution.
3 Likes

I’ve started to update the OP of this thread (remember, it’s a wiki!) with some of the discussion points coming up (thanks @lambda-fairy and @sfackler!)

@sfackler mind filling in the “Crate issues” section with any known blockers? (or just filling in “none”)

Otherwise if others would like to help out here with the log crate’s evaluation, there’s two primary entry points for doing so:

  • Volunteers for filling out the “Guidelines checklist”
  • Volunteers for creating some cookbook examples

I think for the guidelines checklist we can start out by claiming sections to start off. For example I’ll start out by taking the “Organization” section above and verifying those guidelines, namely C-REEXPORT and C-HIERARCHY. The task here is basically to go over the crate’s API and verify that the corresponding guideline is adhered to. Let’s fill in a [x] if the guideline is respected, and otherwise fill it in with [!] if the guideline needs some work to be fully respected. I’ll synthesize these later into notes for the actual review, and eventually they’ll become issues on the log tracker. For now it’s just checking some checkboxes!

If you’d like to help out with that, please feel free to just leave a comment here with what section you’d like to review, or simply fill in your name next to the section on the top. You’ll find my name’s listed next to the “Organization” topic above.

For cookbook examples recall that the intention here is to provide some substantive examples for working with crates through the ecosystem, focused on everyday tasks that end up going beyond the standard library. The cookbook can be found online for a few examples already. I’ve filled in the “Cookbook use case statements” of the OP with a few examples of examples we could write for log. If you’re interested in fleshing one of those out to a full-fledged cookbook example please feel free!

A good way to organize the cookbook examples I think are to use Integer32’s playground, for example here’s a small hello world. I figure we can basically turn the bullet points above into gists to the playground. If you’re interested in filling one of these out, please feel free to do so! (remember you can edit the OP as well!)

2 Likes

FWIW, I ended up making a pretty_env_logger because I wanted some color in the logs (and it tries to pad the logger name to the longest known name, but it's not perfect...). I could make some changes to allow turning those formatting options on, and/or detecting that stdout is a TTY, and release that as a env_logger 1.0 or something.

1 Like

Hi all,

maybe this is not the place to make such requests, but there are a few things that would really improve my experience with log:

  • I’d need an args() method that return a Arguments instead of &Arguments so I can use it with format_args! and write!
  • I’m eagerly awaiting a release with that commit :wink:
  • I want a custom log formatter that would return a new LogRecord instead of a String (I really have no idea how this could work, but I’m willing to help if it is possible)

Basically, I need to make as few allocations as possible for logging, I want a custom formatter and a custom target, and the current design did not help me there. I ended up copying a lot of code to do what I want, but I’d be happy to help log get there if possible

3 Likes

Now that https://github.com/rust-lang/rfcs/pull/1974 uses a trait, it’s tantalizingly close to mirroring exactly what would be needed to unify the std and no-std approaches with log. Not saying we need block 1.0 on this, but if we don’t then I hope log 2.0 is fine?

This is exactly the place! It'd be great if you want to dive in on some of the other items in the OP as well :slight_smile:

In designing allocationless formatting for fern*, I ended up with a callback solution. It’s not exactly the most ergonomic to use, but it isn’t too bad, and is efficient.

The one thing I would want is a way to create and modify LogRecords. Having a RecordBuilder which could build records from scratch, or modify only one property of an existing record with a From<LogRecord> impl would be super nice for both formatting and testing.

Having it as a builder would keep the ability to add new fields, and allowing use of it to modify an existing record would let one format the message of a log record without having to care about when nev fields are added.

This would also let log back ends have saner testing - right now I basically use cargo test -- --skip test2 and cargo test test2 to test two different configurations, since the only way to send log messages to a logger is having it as the global logger, and that can only happen once in a program.

** fern being a log crate backend, https://crates.io/crates/fern/

1 Like

Count me in for the following :slight_smile:

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.

2 Likes

Thanks for the link! I've added it to the discussion points above. My guess is there's not reason to not have support like this in env_logger itself, though!

Oh note that Arguments implements Copy, so you should be able to just use * to do the promotion.

Excellent constraint! I've updated the discussion topics above. In general I personally feel log is intended to be used in "no allocation" scenarios (e.g. the no_std availability), altough loggers built on top don't always expose all the functionality necessary for this (as you've found).

Awesome, thanks for the info! I'll update the topics above as well. Mind expanding a bit on the technical differences between fern and log as well? It seems related to the "how does slog relate to log" question as well!

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!

Right, maybe I wasn't clear. I meant, since it sounded like some people wanted the feature, and it also sounded like maybe env_logger would be jettisoned from the log repo, that it wouldn't be hard for me to take the work I did there and merge it into some new such env_logger repo and publish.

No, it's just old and very simple.

I'll plan to write some examples this week.

Remember this evaluation is due in just 8 days.

I like to think of it as battle tested and as complex as it needs to be :smile:

2 Likes

Question (which can possibly go into the cookbook eventually): Is there a way to make a logger log both into stdout and into a file at the same time?