Add logs support

[soywod]

It could be a great and useful addition to add the tracing crate behind a feature gate and to emit logs. I can propose a PR if you are interested in seach feature.

[brotskydotcom]

Hi @soywod, I'm afraid I'm not convinced of the utility of this, and the samples in your PR are not convincing me otherwise. It's not like the internals of keyring are interesting from a client point of view, except maybe from performance perspective, and no one's been complaining.

Is there a use case you have in mind?

[soywod]

IMHO, adding logs to a lib is really a must. It gives lib consumers more context about what is happening internally, and at a bigger scale (let's say at app level), it also helps to debug. Adding few logs here and there in keyring-rs looks insignificant at lib level, but it really help at app level.

My main motivation comes from an issue one of my user opened: passwords were not persisting. I could not reproduce the issue. App logs showed that everything was fine, but he just could not get his freshly-defined password. After a long research I assumed that it was internally using the mock keystore (potentially due to a missing cargo feature). Having just a simple log create mock keystore entry would have changed everything. Since only keyring-rs has the information of the selected keystore, logging it would be more than helpful.

Other arguments are:

• tracing is the reference in term of structure logging. It has also a compatible layer with log for text-based logging.
• Logging is a common practice. It costs nothing (especially if behind a feature gate), does not alter the API and helps debugging at app level.
• Lower-level crates than keyring-rs use logs: rust-keyutils, security-framework etc. By supporting logs yourself you also enable logs for them.

Side note: it could be great that other tools made by you guys support logs as well: secret-service, dbus-secret-service etc.

[soywod]

Another argument for #222: logs let us know if a secret/password has been found in the keyutils cache or not, with the underlaying error.

if let Some(keyutils) = &self.keyutils {
    if let Ok(password) = keyutils.get_password() {
        return Ok(password);
    }
}

becomes:

if let Some(keyutils) = &self.keyutils {
    let _res = keyutils.get_password();

    #[cfg(feature = "tracing")]
    match _res {
        Ok(password) => {
	    tracing::debug!("password found from keyutils");
	    return Ok(password)
	}
	Err(err) => {
	    tracing::debug!(?err, "cannot find password from keyutils");
        }
    }
}

[brotskydotcom]

OK, I see where we are talking past each other here. :)

If you are suggesting that keyutils should do debug logging, for example of the default keystore choice, and what the computed target and other attributes are for each entry, then yes, by all means, I've been meaning to do that for a long time but have just not gotten around to it. I would love to see a PR that introduces logging (using the logs crate), and I would be very grateful for it!

IMHO, the beauty of adding logging in this way is that it leaves it up to the application client to decide how they want to be handling logs, including whether they want to be doing tracing and set up spans for each call into keyring. The examples you gave above are all, in the language of the tracing crate, events not spans, so can be output via logging.

Would you be willing to contribute logging rather than tracing?

[soywod]

I'm not sure to understand the reason you propose log over tracing. AFAIK, tracing does everything log does, but better:

• tracing is really active, it's built by tokio team
• Because it's built by tokio it handles better async code (not a concern for now, but costs nothing to be ready for it)
• It supports structured logs out of the box
• It supports spans, see their example
• It even proposes the same API as logs, so both are compatible. I started my project with log, then switched to tracing with no effort.

The tracing crate, like log, is just an interface. To collect logs, final users need to subscribe to the interface so then can do whatever they want with them.

I would really advise to go with tracing as it is the way to go now, but if have really sth against I can use log. I invite you to read a bit more about tracing before taking the decision.

[soywod]

I just read your comment in the PR #223 (review), I just want to precise that in my example, debug!(…) does not mean log is consumed straight. Like with log, it just emit an even from a span. It the final user does not subscribe to tracing, it will receive and do nothing. I added the interface behind a cargo feature to just remove a dependency but technically it's just an interface and should be fine without. I personally did not find any resource on this particular question (whenever tracing should be behind a feature gate). I put it in my app but I plan to remove it soon.

Also, I apologize in advance if I misunderstand your arguments for log over tracing. I experienced both and I don't see a single thing log does that tracing does not.

[brotskydotcom]

Hi Clément, I know a lot about tracing, and I use it in several apps. I do feel strongly that we use logs instead. Here's why:

1. It keeps our footprint small - logs is already a dependency of many of our dependents
2. Tracing is designed to be introduced at the application (not the library) level, introducing it at the library level can lead to configuration confusion. So we would need a feature gate, which I don't want to introduce.
3. Using logs provides freedom for application developers to use one of the many other competing libraries that are compatible with logs (rather than tracing).

Hope this helps clarify.

[soywod]

1. It keeps our footprint small - logs is already a dependency of many of our dependents

With the actual code, log is not a dependency of a single dependent:

$ cargo tree --all-features -i log
warning: nothing to print.

To find dependencies that require specific target platforms, try to use option `--target all` first, and then narrow your search scope accordingly.

Which is not the case of tracing, that seems to be already a dependent:

$ cargo tree --all-features -i tracing
tracing v0.1.40
├── async-io v2.3.4
│   └── zbus v4.4.0
│       ├── keyring v3.4.0 (./keyring-rs)
│       └── secret-service v4.0.0
│           └── keyring v3.4.0 (./keyring-rs)
├── polling v3.7.3
│   └── async-io v2.3.4 (*)
└── zbus v4.4.0 (*)

That said, by activating the log feature for dependents, the log will be part of keyring-rs. But we can easily assume that both log and tracing are already part of keyring-rs. So it's not a valid reason.

2. Tracing is designed to be introduced at the application (not the library) level, introducing it at the library level can lead to configuration confusion. So we would need a feature gate, which I don't want to introduce.

Do you have documentation about this statement? AFAIK tracing is for any level. They have both examples for apps and for libs.

3. Using logs provides freedom for application developers to use one of the many other competing libraries that are compatible with logs (rather than tracing).

Isn't it the same for tracing? People can create their own subscriber as far as it is compatible with tracing, see the giant list of existing libs developed by the community.

---

Sorry to insist, but it is not part of my principles to apply rules without fully understanding them. I do not try to agree at all cost, I would switch to log with great pleasure if we have solid arguments to go into this direction. Either I or you misunderstand sth about tracing: I was convinced that the tracing crate is the simple interface (like log), and then you have tracing-subscriber, sentry-tracing, tracing-logfmt etc to consume them (like env_logger, simplelog, stdlogger etc). Do you have concrete examples where log does sth more or better than tracing?

blessed.rs states the following (which I agree):

tracing: Tracing is now the go-to crate for logging.
log: An older and simpler crate if your needs are simple and you are not using any async code.

[brotskydotcom]

Happy to have you insist - isn't that the point of open source? To discuss until we find a position we can agree on? So to your points:

1. You're right, only our dev dependencies depend on log, and only on Windows. I was under the impression that the linux-keyutils crate used log, but in fact it doesn't.
2. I see I wasn't clear enough. What I meant was that the decision about what log infrastructure to use is made at the application level, not the library level. This leads to configuration issues if we use tracing at the library level and the developer doesn't choose a tracing-compatible infrastructure at the app level (see point 3).
3. The advantage of log is that, as the first and oldest simple logging library, it's compatible with all the Rust "better" loggers, such as Slog. If we use tracing at the library level, it's not clear that an app that uses Slog will be able to grab those records easily. (For example, tracing can not grab slog records without adapters.)

To your postscript about blessed.rs: I would argue that the entries there actually point in the direction of log: we are not using any async code, and our needs are simple. Keep in mind that the tracing superstructure is part of the tokio stack, and while tokio is certainly the most popular async rust ecosystem right now, it's still basically unknown to synchronous programers. Also keep in mind that slog is mentioned for structured logging in blessed.rs, not just tracing. In order to be compatible with older and synchronous applications, I would like to stay with log.

Let me know if you find this convincing.

[soywod]

Happy to have you insist - isn't that the point of open source? To discuss until we find a position we can agree on?

Sure, you're right!

3. The advantage of log is that, as the first and oldest simple logging library, it's compatible with all the Rust "better" loggers, such as Slog. If we use tracing at the library level, it's not clear that an app that uses Slog will be able to grab those records easily. (For example, tracing can not grab slog records without adapters.)

To your postscript about blessed.rs: I would argue that the entries there actually point in the direction of log: we are not using any async code, and our needs are simple. Keep in mind that the tracing superstructure is part of the tokio stack, and while tokio is certainly the most popular async rust ecosystem right now, it's still basically unknown to synchronous programers. Also keep in mind that slog is mentioned for structured logging in blessed.rs, not just tracing. In order to be compatible with older and synchronous applications, I would like to stay with log.

By using log you increase the chances that high-level app will be able to grab keyring-rs logs, right? If so then indeed log makes more sense. And I have no argument against the fact that log is for synchronous simple needs, and that it fits keyring-rs for now.

Also, since tracing and log API are compatible, we can easily switch from one to the other (in case keyring-rs becomes more complex or even async). Doors are not closed.

I'm convinced enough, I will refactor the PR as soon as we merge #222. I will take care of that tomorrow.

Thank you for you patience.

[brotskydotcom]

Thank YOU for being such a thoughtful and persistent contributor!!