remove all init_logger part to use a common way for all crates.

[Tocard]

logging factorisation

To add json logging and avoid to add each logger into each crate, I'm wondering if we can use logger as libs instead.

json logging

The flag from the cli will provide json log if set or regular log to true.

That's the first ever contribution on rust, sorry in advance 👍

Code contributor checklist:

• I have read, understood and followed contributing guide

[teor2345]

Thanks for this PR! Overall, I like the idea of refactoring our logging into a single crate, so it is consistent between binaries.

The logging crate refactor could go in a separate small PR, then adding JSON logging on top of that will be easier to review.

Adding a new crate also makes us think about what else we want to put in there. In this case, we might want a crate for all the things we do when we start a binary. But there’s no need to rename it yet, I want to check with the rest of the team first.

I don’t have a strong opinion about JSON logging, what are the use cases you’re trying to support?

[teor2345]

Seems like our Windows CI has a bug which is impacting all our PRs. Sorry about that!

We use rustfmt and clippy to check our code style, do you need help setting them up?
Running cargo fmt --all should automatically fix formatting, and cargo clippy will give you a list of code style warnings to fix.

[Tocard]

Thanks for this PR! Overall, I like the idea of refactoring our logging into a single crate, so it is consistent between binaries.

The logging crate refactor could go in a separate small PR, then adding JSON logging on top of that will be easier to review.

Adding a new crate also makes us think about what else we want to put in there. In this case, we might want a crate for all the things we do when we start a binary. But there’s no need to rename it yet, I want to check with the rest of the team first.

I don’t have a strong opinion about JSON logging, what are the use cases you’re trying to support?

I can split it into both Pr for sure.

As said into the description of the pull request, that's my first contribution into a rust project. I have no idea about architecture and layout suitable for the language haha.

We are many user into the project who do use logs to gather information and data for a lot of purpose. Json formatted logs are always better for this.

On my case

Logs <= Filebeat => Elasticsearch <=> Kibana / grafana to display stuff about my cluster health,

[Tocard]

Seems like our Windows CI has a bug which is impacting all our PRs. Sorry about that!

We use rustfmt and clippy to check our code style, do you need help setting them up? Running cargo fmt --all should automatically fix formatting, and cargo clippy will give you a list of code style warnings to fix.
I tried to run it and it sayed all goods, Pretty obvious that was not the case haha.

I endup this pull-request later in night, will retry this evening

[Tocard]

Seems like our Windows CI has a bug which is impacting all our PRs. Sorry about that!

We use rustfmt and clippy to check our code style, do you need help setting them up? Running cargo fmt --all should automatically fix formatting, and cargo clippy will give you a list of code style warnings to fix.

seems ok :) thanks

[Tocard]

removed all json part for now

[nazar-pc]

Thanks for the contribution!

Conceptually this makes a lot of sense to me and I think it is a good idea to do the refactoring first to extract common logic, then adding CLI option will be much easier.

The dependency only needs to be added to crates where it is used and remove tracing-subscriber from direct dependencies where you use subspace-logging since it will no longer be necessary.

[teor2345]

After removing the unused dependencies, we should be good to merge this

[Tocard]

Do there is a way to bild the whole project & binary to ensure all deps are fine ?

[Tocard]

what abot this one ? Shall we add dep::subspace-logging ?

if so why this one is needed only here and not to binary node ??

[teor2345]

Yes, this is the feature for using the farmer crate as a binary, rather than a library. We only need to init logging when running the farmer binary, because libraries leave logging to the host application.

if so why this one is needed only here and not to binary node ??

The farmer crate contains a binary and library. The binary for the node is in subspace-node, but the subspace-service crate (and some other crates) are the libraries.

[teor2345]

Do there is a way to bild the whole project & binary to ensure all deps are fine ?

cargo clippy will find missing dependencies and unused imports.

Finding unused dependencies is harder. You could try cargo-udeps, but sometimes it will tell you that a dependency is missing, when it is actually used in an unusual way:
http://stackoverflow.com/questions/72082550/ddg#72082679

[teor2345]

Yes, this is the feature for using the farmer crate as a binary, rather than a library. We only need to init logging when running the farmer binary, because libraries leave logging to the host application.

if so why this one is needed only here and not to binary node ??

The farmer crate contains a binary and library. The binary for the node is in subspace-node, but the subspace-service crate (and some other crates) are the libraries.

[teor2345]

Everywhere you have removed tracing-subscriber, please also remove supports-color.
(Sorry I didn’t catch this earlier!)

[teor2345]

Nitpick: we usually put crates in alphabetical order

[teor2345]

The “deps” line changes this from an optional dependency into a required one for the binary only.

Suggested change

	subspace-logging = { version = "0.0.1", path = "../../shared/subspace-logging" }
	subspace-logging = { version = "0.0.1", path = "../../shared/subspace-logging", optional = true }