[PR #1554] [MERGED] Simplify server logging to single logline per request #2413

New issue

Closed

opened 2026-03-16 08:52:28 +03:00 by kerem · 0 comments

kerem commented

2026-03-16 08:52:28 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/hickory-dns/hickory-dns/pull/1554
Author: @bluejekyll
Created: 9/20/2021
Status: ✅ Merged
Merged: 10/7/2021
Merged by: @bluejekyll

Base: main ← Head: simplified-logging

📝 Commits (10+)

d2b6649 Simplify server logging to single logline per request
7de488c update cangelog for server request logging
32c25ea cleanup use statements
80dc425 Change response code to Debug format in log-line
eeacdb6 simplify request handling by verifying query form
b2d4359 cleanup flag printing
74cc323 hoist return out of match statement
b649ea9 cleanup request handling by enforcing single query during parsing
ec04265 refactor write! of strings to Formatter::write_str
8451f74 fix SyncClient::send and add FormErr test

📊 Changes

27 files changed (+937 additions, -417 deletions)

View changed files

📝 CHANGELOG.md (+4 -0)
📝 crates/proto/src/error.rs (+26 -3)
📝 crates/proto/src/https/error.rs (+1 -1)
📝 crates/proto/src/https/https_client_stream.rs (+1 -1)
📝 crates/proto/src/op/header.rs (+80 -20)
📝 crates/proto/src/op/message.rs (+25 -28)
📝 crates/proto/src/op/op_code.rs (+20 -16)
📝 crates/proto/src/rr/rdata/svcb.rs (+8 -10)
📝 crates/proto/src/xfer/dns_response.rs (+45 -12)
📝 crates/resolver/src/config.rs (+1 -1)
📝 crates/server/src/authority/catalog.rs (+138 -130)
📝 crates/server/src/authority/message_request.rs (+97 -49)
📝 crates/server/src/authority/message_response.rs (+37 -20)
📝 crates/server/src/server/https_handler.rs (+15 -19)
📝 crates/server/src/server/mod.rs (+3 -1)
➕ crates/server/src/server/protocol.rs (+37 -0)
📝 crates/server/src/server/request_handler.rs (+96 -3)
📝 crates/server/src/server/response_handler.rs (+22 -13)
📝 crates/server/src/server/server_future.rs (+170 -44)
📝 crates/server/src/store/forwarder/authority.rs (+3 -3)

...and 7 more files

📄 Description

Fixes: #1525

@williamdes, This PR should help with the above issue. There was a big refactor I needed to do for this, but we should see a single logline on requests example:

1632165134.315584:INFO:trust_dns_server::server::server_future:651:request:1837 src:UDP://127.0.0.1#52175 QUERY:www.exmple.com.:A:IN qflags:RD,AD; response:No Error rr:1/0/1 rflags: RD,RA;
1632165167.438796:INFO:trust_dns_server::server::server_future:651:request:13648 src:UDP://127.0.0.1#53266 QUERY:www.google.com.:A:IN qflags:RD,AD; response:No Error rr:1/0/1 rflags: RD,RA;
1632165210.496483:INFO:trust_dns_server::server::server_future:651:request:43077 src:UDP://127.0.0.1#57542 QUERY:www.example.com.:A:IN qflags:RD,AD; response:No Error rr:1/0/1 rflags: RD,AA;
1632165243.481158:INFO:trust_dns_server::server::server_future:651:request:24552 src:UDP://127.0.0.1#55267 QUERY:xxx.example.com.:A:IN qflags:RD,AD; response:Non-Existent Domain rr:0/1/1 rflags: RD,AA;
1632165267.610928:INFO:trust_dns_server::server::server_future:651:request:47603 src:UDP://127.0.0.1#57723 QUERY:example.com.:A:IN qflags:RD,AD; response:No Error rr:1/0/3 rflags: RD,AA;

I think we can use this opportunity to cleanup some of the Display implementations for these various sections. let me know what you think. As an example, the Display for the response code is multi-word right now, that might not be desirable? Also, do we want to the code+description instead of just the description there?

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/hickory-dns/hickory-dns/pull/1554 **Author:** [@bluejekyll](https://github.com/bluejekyll) **Created:** 9/20/2021 **Status:** ✅ Merged **Merged:** 10/7/2021 **Merged by:** [@bluejekyll](https://github.com/bluejekyll) **Base:** `main` ← **Head:** `simplified-logging` --- ### 📝 Commits (10+) - [`d2b6649`](https://github.com/hickory-dns/hickory-dns/commit/d2b6649290801ea836278a91d6e57a8d8a7db234) Simplify server logging to single logline per request - [`7de488c`](https://github.com/hickory-dns/hickory-dns/commit/7de488c3bf7b2779fa908637a235fe2ce9fe1712) update cangelog for server request logging - [`32c25ea`](https://github.com/hickory-dns/hickory-dns/commit/32c25ea3f0d35730ee88b91e49b57bd1e0eabd3c) cleanup use statements - [`80dc425`](https://github.com/hickory-dns/hickory-dns/commit/80dc4256600626e54dda17f9d705e11bf8bb6b55) Change response code to Debug format in log-line - [`eeacdb6`](https://github.com/hickory-dns/hickory-dns/commit/eeacdb62d1420b81fc0f7805dd6ed657ea56c65e) simplify request handling by verifying query form - [`b2d4359`](https://github.com/hickory-dns/hickory-dns/commit/b2d4359bc509f4104966b72f75fc951b2c8213bf) cleanup flag printing - [`74cc323`](https://github.com/hickory-dns/hickory-dns/commit/74cc323b74517d98ab36c56b48f72835741ca567) hoist return out of match statement - [`b649ea9`](https://github.com/hickory-dns/hickory-dns/commit/b649ea92e80a2fffaddcb9f4d663ff68a42ddcac) cleanup request handling by enforcing single query during parsing - [`ec04265`](https://github.com/hickory-dns/hickory-dns/commit/ec04265726d1c088a10288e1860edefd4ca6dca4) refactor write! of strings to Formatter::write_str - [`8451f74`](https://github.com/hickory-dns/hickory-dns/commit/8451f74b38dcad95c825d3507193f9f7764f2c8c) fix SyncClient::send and add FormErr test ### 📊 Changes **27 files changed** (+937 additions, -417 deletions) <details> <summary>View changed files</summary> 📝 `CHANGELOG.md` (+4 -0) 📝 `crates/proto/src/error.rs` (+26 -3) 📝 `crates/proto/src/https/error.rs` (+1 -1) 📝 `crates/proto/src/https/https_client_stream.rs` (+1 -1) 📝 `crates/proto/src/op/header.rs` (+80 -20) 📝 `crates/proto/src/op/message.rs` (+25 -28) 📝 `crates/proto/src/op/op_code.rs` (+20 -16) 📝 `crates/proto/src/rr/rdata/svcb.rs` (+8 -10) 📝 `crates/proto/src/xfer/dns_response.rs` (+45 -12) 📝 `crates/resolver/src/config.rs` (+1 -1) 📝 `crates/server/src/authority/catalog.rs` (+138 -130) 📝 `crates/server/src/authority/message_request.rs` (+97 -49) 📝 `crates/server/src/authority/message_response.rs` (+37 -20) 📝 `crates/server/src/server/https_handler.rs` (+15 -19) 📝 `crates/server/src/server/mod.rs` (+3 -1) ➕ `crates/server/src/server/protocol.rs` (+37 -0) 📝 `crates/server/src/server/request_handler.rs` (+96 -3) 📝 `crates/server/src/server/response_handler.rs` (+22 -13) 📝 `crates/server/src/server/server_future.rs` (+170 -44) 📝 `crates/server/src/store/forwarder/authority.rs` (+3 -3) _...and 7 more files_ </details> ### 📄 Description Fixes: #1525 @williamdes, This PR should help with the above issue. There was a big refactor I needed to do for this, but we should see a single logline on requests example: ```console 1632165134.315584:INFO:trust_dns_server::server::server_future:651:request:1837 src:UDP://127.0.0.1#52175 QUERY:www.exmple.com.:A:IN qflags:RD,AD; response:No Error rr:1/0/1 rflags: RD,RA; 1632165167.438796:INFO:trust_dns_server::server::server_future:651:request:13648 src:UDP://127.0.0.1#53266 QUERY:www.google.com.:A:IN qflags:RD,AD; response:No Error rr:1/0/1 rflags: RD,RA; 1632165210.496483:INFO:trust_dns_server::server::server_future:651:request:43077 src:UDP://127.0.0.1#57542 QUERY:www.example.com.:A:IN qflags:RD,AD; response:No Error rr:1/0/1 rflags: RD,AA; 1632165243.481158:INFO:trust_dns_server::server::server_future:651:request:24552 src:UDP://127.0.0.1#55267 QUERY:xxx.example.com.:A:IN qflags:RD,AD; response:Non-Existent Domain rr:0/1/1 rflags: RD,AA; 1632165267.610928:INFO:trust_dns_server::server::server_future:651:request:47603 src:UDP://127.0.0.1#57723 QUERY:example.com.:A:IN qflags:RD,AD; response:No Error rr:1/0/3 rflags: RD,AA; ``` I think we can use this opportunity to cleanup some of the Display implementations for these various sections. let me know what you think. As an example, the Display for the response code is multi-word right now, that might not be desirable? Also, do we want to the code+description instead of just the description there? --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>