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

Make tracing optional #2156

Closed
wants to merge 1 commit into from
Closed

Make tracing optional #2156

wants to merge 1 commit into from

Conversation

ssrlive
Copy link

@ssrlive ssrlive commented Feb 28, 2024

Log feature is added to support users to customize whether to output logs or not.

Copy link
Collaborator

@djc djc left a comment

Choose a reason for hiding this comment

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

What's your use case? What are you trying to achieve?

If we're going to do something like this, I think we should do it more like rustls, avoiding putting guards on every tracing call:

https://github.com/rustls/rustls/blob/main/rustls/src/lib.rs#L370

@@ -36,7 +36,7 @@ hickory-proto = { version = "0.24.0", path = "crates/proto", default-features =


# logging
tracing = "0.1.30"
tracing = { version = "0.1.30", features = ["log"] }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why/how is this related?

Copy link
Author

@ssrlive ssrlive Feb 28, 2024

Choose a reason for hiding this comment

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

Some documentation says that this feature allows tracing to work with logs.

https://docs.rs/tracing/0.1.40/tracing/#log-compatibility

@djc djc changed the title feat: log added Make tracing optional Feb 28, 2024
@ssrlive
Copy link
Author

ssrlive commented Feb 28, 2024

If you have a better solution, I'd love to use yours.

@marcus0x62
Copy link
Contributor

I might be confused about what you are trying to accomplish, but you can already log (and control the logging level) in both the server binary and, if you register a logger, when using the library crates. There's a simple example in the included utility programs, such as resolve (see the logger function in util/src/lib.rs.)

No logging:

marcusb@dom hickory-dns/target/debug % ./resolve foo.com

Querying for foo.com A from udp:8.8.8.8:53, tcp:8.8.8.8:53, udp:8.8.4.4:53, tcp:8.8.4.4:53, udp:[2001:4860:4860::8888]:53, tcp:[2001:4860:4860::8888]:53, udp:[2001:4860:4860::8844]:53, tcp:[2001:4860:4860::8844]:53
Success for query foo.com IN A
foo.com. 468 IN A 34.206.39.153

Debug-level logging:

marcusb@dom hickory-dns/target/debug % RUST_LOG=debug ./resolve foo.com

Querying for foo.com A from udp:8.8.8.8:53, tcp:8.8.8.8:53, udp:8.8.4.4:53, tcp:8.8.4.4:53, udp:[2001:4860:4860::8888]:53, tcp:[2001:4860:4860::8888]:53, udp:[2001:4860:4860::8844]:53, tcp:[2001:4860:4860::8844]:53
2024-02-28T15:13:17.808989Z DEBUG hickory_proto::xfer::dns_handle: querying: foo.com A
2024-02-28T15:13:17.809008Z DEBUG hickory_resolver::name_server::name_server_pool: sending request: [Query { name: Name("foo.com"), query_type: A, query_class: IN }]
2024-02-28T15:13:17.809027Z DEBUG hickory_resolver::name_server::name_server: reconnecting: NameServerConfig { socket_addr: [2001:4860:4860::8888]:53, protocol: Udp, tls_dns_name: None, trust_negative_responses: true, bind_addr: None }
2024-02-28T15:13:17.809051Z DEBUG hickory_proto::xfer: enqueueing message:QUERY:[Query { name: Name("foo.com"), query_type: A, query_class: IN }]
2024-02-28T15:13:17.809059Z DEBUG hickory_resolver::name_server::name_server: reconnecting: NameServerConfig { socket_addr: 8.8.4.4:53, protocol: Udp, tls_dns_name: None, trust_negative_responses: true, bind_addr: None }
2024-02-28T15:13:17.809065Z DEBUG hickory_proto::xfer: enqueueing message:QUERY:[Query { name: Name("foo.com"), query_type: A, query_class: IN }]
2024-02-28T15:13:17.809189Z DEBUG hickory_proto::udp::udp_client_stream: final message: ; header 26473:QUERY:RD:NoError:QUERY:0/0/0
; query
;; foo.com. IN A

2024-02-28T15:13:17.809193Z DEBUG hickory_proto::udp::udp_client_stream: final message: ; header 62830:QUERY:RD:NoError:QUERY:0/0/0
; query
;; foo.com. IN A

2024-02-28T15:13:17.809237Z DEBUG hickory_proto::udp::udp_stream: created socket successfully
2024-02-28T15:13:17.809272Z DEBUG hickory_proto::udp::udp_stream: created socket successfully
2024-02-28T15:13:17.809289Z DEBUG hickory_resolver::name_server::name_server: name_server connection failure: io error: Network is unreachable (os error 101)
2024-02-28T15:13:17.830275Z DEBUG hickory_proto::udp::udp_client_stream: received message id: 26473
2024-02-28T15:13:17.830296Z DEBUG hickory_proto::error: Response:; header 26473:RESPONSE:RD,RA:NoError:QUERY:1/0/0
; query
;; foo.com. IN A
; answers 1
foo.com. 131 IN A 34.206.39.153
; nameservers 0
; additionals 0

2024-02-28T15:13:17.830311Z DEBUG hickory_proto::error: Response:; header 26473:RESPONSE:RD,RA:NoError:QUERY:1/0/0
; query
;; foo.com. IN A
; answers 1
foo.com. 131 IN A 34.206.39.153
; nameservers 0
; additionals 0

Success for query foo.com IN A
foo.com. 131 IN A 34.206.39.153

@ssrlive
Copy link
Author

ssrlive commented Feb 28, 2024

I wish I had a choice when using this library and could turn off the log output of this library. The current situation is to either turn off all log output or output the logs of all libraries. There is little room for choice.

@ssrlive
Copy link
Author

ssrlive commented Feb 28, 2024

I know about the RUST_LOG=debug magic, but not every user is proficient in this magic.

@djc
Copy link
Collaborator

djc commented Feb 28, 2024

I know about the RUST_LOG=debug magic, but not every user is proficient in this magic.

You can also wire up that stuff in your downstream binary.

But -- you still haven't really explained your situation/goals in much detail.

@ssrlive
Copy link
Author

ssrlive commented Feb 28, 2024

my use case is, when i use hickory-dns with feature log, I can print logs. when i remove log from hickory-dns, do not output anything.

@ssrlive
Copy link
Author

ssrlive commented Feb 28, 2024

I don't know why the test failed. my change did not break any logic.

@marcus0x62
Copy link
Contributor

I don't think controlling logging with a build-time flag is a good idea. Someone might be in a situation with a compiled binary where they initially don't want logging, but need to enable it on an ad-hoc basis. Additionally, developers that use the library crates can filter out the hickory_* crates from their logger definition, if they feel the need.

@ssrlive
Copy link
Author

ssrlive commented Feb 28, 2024

I want some libraries to output logs but tracing library to output no logs.
I hope my message is clear.

@djc
Copy link
Collaborator

djc commented Feb 29, 2024

my use case is, when i use hickory-dns with feature log, I can print logs. when i remove log from hickory-dns, do not output anything.

By hickory-dns, do you mean the server binary? Because that's very different from what this PR does.

@djc
Copy link
Collaborator

djc commented Feb 29, 2024

I want some libraries to output logs but tracing library to output no logs. I hope my message is clear.

As @marcus0x62 already mentioned, the Rust log/tracing ecosystem also already offers ways to filter messages from which crates you see logs.

@ssrlive ssrlive closed this by deleting the head repository Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants