Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set tracing target for networking messages. #12582

Merged
merged 1 commit into from
Aug 29, 2023

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Aug 28, 2023

This changes the log messages for messages related to network traffic to use the "network" tracing target. This makes it easier to do CARGO_LOG=network=trace CARGO_HTTP_DEBUG=true instead of trying to figure out which modules to include (and to avoid CARGO_LOG=trace which can be too noisy). For example, #12290 moved the location of some log messages to a different module, which broke the documented workflow of using CARGO_LOG=cargo::ops::registry=debug to get networking information.

@rustbot
Copy link
Collaborator

rustbot commented Aug 28, 2023

r? @weihanglo

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-dependency-resolution Area: dependency resolution and the resolver A-documenting-cargo-itself Area: Cargo's documentation A-networking Area: networking issues, curl, etc. A-registries Area: registries A-sparse-registry Area: http sparse registries S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 28, 2023
@ehuss
Copy link
Contributor Author

ehuss commented Aug 28, 2023

I'm not sure if we want to start using tracing targets, but I'm intrigued by the idea and I think it would be something interesting to explore. For example, to get fingerprint logging, the logs from cargo_util::paths is extremely useful, but unfortunately people normally don't include it. I'd like to explore adding additional targets like "fingerprint" for things like that.

@epage
Copy link
Contributor

epage commented Aug 28, 2023

@bors r+

This seems like a good place for us to experiment with this feature.

@bors
Copy link
Contributor

bors commented Aug 28, 2023

📌 Commit f0176a1 has been approved by epage

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 28, 2023
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a dependency is set with the same tracing target, will CARGO_LOG=network enable it also?

If this is the case, should we have a less generic target name?

@bors
Copy link
Contributor

bors commented Aug 28, 2023

⌛ Testing commit f0176a1 with merge 5522091...

@bors
Copy link
Contributor

bors commented Aug 29, 2023

☀️ Test successful - checks-actions
Approved by: epage
Pushing 5522091 to master...

@bors bors merged commit 5522091 into rust-lang:master Aug 29, 2023
@ehuss
Copy link
Contributor Author

ehuss commented Aug 29, 2023

If a dependency is set with the same tracing target, will CARGO_LOG=network enable it also?

If this is the case, should we have a less generic target name?

I believe targets are a global namespace, so I think so. But if a dependency does that, presumably the message would be related to networking and we would want it included? The idea is to show everything networking-related.

Also, it seems to be pretty rare that I've seen anyone use the target feature. I would be surprised if any low-level library is doing that.

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 29, 2023
Update cargo

18 commits in 925280f028db3a322935e040719a0754703947cf..96fe1c9e1aecd8f57063e3753969bb6418fd2fd5
2023-08-25 21:16:44 +0000 to 2023-08-29 20:10:34 +0000
- fix(lints): Fail when overriding inherited lints (rust-lang/cargo#12584)
- cargo install: suggest --git when package name is url (rust-lang/cargo#12575)
- chore: remove unstable-options for logout (rust-lang/cargo#12588)
- Improve logout message for asymmetric tokens (rust-lang/cargo#12587)
- fix(update): Remove references to -p in help (rust-lang/cargo#12586)
- fix(update): Make `-p` more convenient by being positional (rust-lang/cargo#12545)
- Set tracing target for networking messages. (rust-lang/cargo#12582)
- Retry docs (rust-lang/cargo#12583)
- feat(resolver): **Very** preliminary MSRV resolver support (rust-lang/cargo#12560)
- Update git2 (rust-lang/cargo#12580)
- Explain how `version` works for `git` dependencies (rust-lang/cargo#12270)
- Improve deserialization errors of untagged enums (rust-lang/cargo#12574)
- Add support for `target.'cfg(..)'.linker` (rust-lang/cargo#12535)
- Improve resolver version mismatch warning (rust-lang/cargo#12573)
- Stabilize `--keep-going` (rust-lang/cargo#12568)
- Define {{command}} for use in src/doc/man/includes (rust-lang/cargo#12570)
- Update serde (rust-lang/cargo#12569)
- chore: add missing `windows-sys` features back (rust-lang/cargo#12563)

r? ghost
@ehuss ehuss added this to the 1.74.0 milestone Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependency-resolution Area: dependency resolution and the resolver A-documenting-cargo-itself Area: Cargo's documentation A-networking Area: networking issues, curl, etc. A-registries Area: registries A-sparse-registry Area: http sparse registries S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants