chore: introduce libp2p-logging

[kamuik16]

Description

Fixes #4992

Based on the conversation in #4992.

Change checklist

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• A changelog entry has been made in the appropriate crates

[jxs]

Thanks for this.
Overall looks good to me, left some minor comments. We also need CHANGELOG.md entries.
I think when Thomas mentioned the publish = false wasn't accounting what he said later on #4992 (comment)

[kamuik16]

Thanks for this. Overall looks good to me, left some minor comments. We also need CHANGELOG.md entries. I think when Thomas mentioned the publish = false wasn't accounting what he said later on #4992 (comment)

We also need CHANGELOG.md entries.

Which ones do you suggest? Since the logging is only used in testing part and is not the part of public API, I don't think it needs CHANGELOG.md entries.

[jxs]

Thanks for this. Overall looks good to me, left some minor comments. We also need CHANGELOG.md entries. I think when Thomas mentioned the publish = false wasn't accounting what he said later on #4992 (comment)

We also need CHANGELOG.md entries.

Which ones do you suggest? Since the logging is only used in testing part and is not the part of public API, I don't think it needs CHANGELOG.md entries.

the ones we publish, libp2p-perf and libp2p-server

[kamuik16]

Thanks for this. Overall looks good to me, left some minor comments. We also need CHANGELOG.md entries. I think when Thomas mentioned the publish = false wasn't accounting what he said later on #4992 (comment)

We also need CHANGELOG.md entries.

Which ones do you suggest? Since the logging is only used in testing part and is not the part of public API, I don't think it needs CHANGELOG.md entries.

the ones we publish, libp2p-perf and libp2p-server

added CHANGELOG.md entries.

[kamuik16]

@jxs would appreciate your further review here :)

[kamuik16]

semver check is just failing because the libp2p-logging crate has not been published yet.

[jxs]

LGTM, thanks Krishang, thanks for the ping!
Giving more thought about this one taking into account we'll have to release it and be yet another crate to maintain in exchange for duplicating a couple lines across the repo it's not clear its value, but I am willing to move forward, wdyt @elenaf9 @dariusc93 @guillaumemichel @drHuangMHT

[kamuik16]

LGTM, thanks Krishang, thanks for the ping! Giving more thought about this one taking into account we'll have to release it and be yet another crate to maintain in exchange for duplicating a couple lines across the repo it's not clear its value, but I am willing to move forward, wdyt @elenaf9 @dariusc93 @guillaumemichel @drHuangMHT

yeah, that's why I was suggesting to keep the publish = false and remove its usage from the packages that are released.

Majority of the usage of this is in the tests, so no need to publish it.

[jxs]

LGTM, thanks Krishang, thanks for the ping! Giving more thought about this one taking into account we'll have to release it and be yet another crate to maintain in exchange for duplicating a couple lines across the repo it's not clear its value, but I am willing to move forward, wdyt @elenaf9 @dariusc93 @guillaumemichel @drHuangMHT

yeah, that's why I was suggesting to keep the publish = false and remove its usage from the packages that are released.

Majority of the usage of this is in the tests, so no need to publish it.

but then we still need to have duplicated log initialization code on the published crates (libp2p-perf and libp2p-server) right? Or how would you address it? Nonetheless thanks for your patience bearing with the process Krishang.

[elenaf9]

yeah, that's why I was suggesting to keep the publish = false and remove its usage from the packages that are released.

Majority of the usage of this is in the tests, so no need to publish it.

I am also leaning towards not publishing libp2p-logging, and instead duplicating the lines in the two published crates.
It's not ideal, but it still:

• avoids the verbose let _ = tracing_subscriber::fmt()... in all other tests/ examples
• doesn't require us to maintain yet another published crate.

[kamuik16]

yeah, that's why I was suggesting to keep the publish = false and remove its usage from the packages that are released.
Majority of the usage of this is in the tests, so no need to publish it.

I am also leaning towards not publishing libp2p-logging, and instead duplicating the lines in the two published crates. It's not ideal, but it still:

• avoids the verbose let _ = tracing_subscriber::fmt()... in all other tests/ examples
• doesn't require us to maintain yet another published crate.

yep, I do agree on this, it's better to have 2-3 lines of duplicated code rather than maintaining a whole crate.

I have reverted back the changes on the published crates and made libp2p-logging with publish = false.

[elenaf9]

Thanks for the follow-up @kamuik16!

I just noticed that the new libp2p_logging crate is not used by any crates in examples/. Was that on purpose / is there a reason why it can't be used there?

[kamuik16]

Thanks for the follow-up @kamuik16!

I just noticed that the new libp2p_logging crate is not used by any crates in examples/. Was that on purpose / is there a reason why it can't be used there?

yeah, that was on purpose, examples are meant for user to copy paste and run. If we don't publish the crate, examples would compile in our repository but users who are still new and just want to copy paste and run them, won't be able to do so.

does this make sense?