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

Better logging/telemetry hooks #1602

Open
hmh opened this issue Sep 10, 2024 · 0 comments
Open

Better logging/telemetry hooks #1602

hmh opened this issue Sep 10, 2024 · 0 comments

Comments

@hmh
Copy link

hmh commented Sep 10, 2024

While trying to get more telemetry and logging of requests/replies/errors into one of our DNS-based applications which uses miekg/dns, I came across several pain points, which I'd like to address (yes, I'd do the work) if we can agree with the best way to do that.

  • hooks like InvalidMsgHook know nothing about the connection, so it is impossible to log useful data such as the DNS client's IP and port that is sending malformed packets. You can have blind telemetry (counters, etc), but logging capabilities get very limited.

  • DecorateWriter/Reader are wire-level, and thus unsuitable for logging or telemetry of DNS data in the request/reply.

  • private implementation of type "response" in serve.go gets in the way of trying to leverage a modified ResponseWriter that does logging in WriteMsg().

So, I'd like to propose a DecorateResponseWriter, and that low-level serveDNS in server.go stop issuing wire replies without going through a configurable ResponseWriter. It could be done through an indirection like the one that is currently handling DecorateWriter.

For the hooks, to avoid API break I guess the easiest way would be to provide InvalidMsgHook2 and MsgAcceptFunc2 that also take a ResponseWriter as a second parameter, and which by default just pass through to the older InvalidMsgHook and MsgAcceptFunc hooks. I am sure this is not very idiomatic, but it should be extremely cheap runtime-wise.

Note that this new MsgAcceptFunc2 could be used to refuse access (with a DNS REFUSED reply if so desired) based on client connection data, which is awkward to do on DecorateReader.

Does this look sane? Or have I misunderstood something (everything) and there is no need for those changes so as to be able to log and count NOTIMP replies, FORMERR replies, and discarded packets (with their origin) ?

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

No branches or pull requests

1 participant