From 1f810f8928624dc8bce04922d2fb309add13bcba Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Vladim=C3=ADr=20=C4=8Cun=C3=A1t?= <vladimir.cunat@nic.cz>
Date: Tue, 8 Feb 2022 12:59:31 +0100
Subject: [PATCH] modules/dnstap: improve UX for common errors

The main thing is the "failed to open socket" message.
But let's also elevate other fatal one-off logs to ERROR level.
---
 modules/dnstap/README.rst |  2 ++
 modules/dnstap/dnstap.c   | 13 ++++++++-----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/modules/dnstap/README.rst b/modules/dnstap/README.rst
index 8a98d07eb..456d2188c 100644
--- a/modules/dnstap/README.rst
+++ b/modules/dnstap/README.rst
@@ -10,6 +10,8 @@ socket in `dnstap format <https://dnstap.info>`_ using fstrm framing library.
 This logging is useful if you need effectively log all DNS traffic.
 
 The unix socket and the socket reader must be present before starting resolver instances.
+Also it needs appropriate filesystem permissions;
+the typical user and group of the daemon are called ``knot-resolver``.
 
 Tunables:
 
diff --git a/modules/dnstap/dnstap.c b/modules/dnstap/dnstap.c
index a137dadb7..a41138de7 100644
--- a/modules/dnstap/dnstap.c
+++ b/modules/dnstap/dnstap.c
@@ -22,6 +22,7 @@
 #include <uv.h>
 
 #define DEBUG_MSG(fmt, ...) kr_log_debug(DNSTAP, fmt, ##__VA_ARGS__);
+#define ERROR_MSG(fmt, ...) kr_log_error(DNSTAP, fmt, ##__VA_ARGS__);
 #define CFG_SOCK_PATH "socket_path"
 #define CFG_IDENTITY_STRING "identity"
 #define CFG_VERSION_STRING "version"
@@ -415,7 +416,7 @@ int dnstap_config(struct kr_module *module, const char *conf) {
 
 		JsonNode *root_node = json_decode(conf);
 		if (!root_node) {
-			DEBUG_MSG("error parsing json\n");
+			ERROR_MSG("error parsing json\n");
 			return kr_error(EINVAL);
 		}
 
@@ -484,13 +485,15 @@ int dnstap_config(struct kr_module *module, const char *conf) {
 	DEBUG_MSG("opening sock file %s\n",sock_path);
 	struct fstrm_writer *writer = dnstap_unix_writer(sock_path);
 	if (!writer) {
-		DEBUG_MSG("can't create unix writer\n");
+		ERROR_MSG("failed to open socket %s\n"
+			"Please ensure that it exists beforehand and has appropriate access permissions.\n",
+			sock_path);
 		return kr_error(EINVAL);
 	}
 
 	struct fstrm_iothr_options *opt = fstrm_iothr_options_init();
 	if (!opt) {
-		DEBUG_MSG("can't init fstrm options\n");
+		ERROR_MSG("can't init fstrm options\n");
 		fstrm_writer_destroy(&writer);
 		return kr_error(EINVAL);
 	}
@@ -499,7 +502,7 @@ int dnstap_config(struct kr_module *module, const char *conf) {
 	data->iothread = fstrm_iothr_init(opt, &writer);
 	fstrm_iothr_options_destroy(&opt);
 	if (!data->iothread) {
-		DEBUG_MSG("can't init fstrm_iothr\n");
+		ERROR_MSG("can't init fstrm_iothr\n");
 		fstrm_writer_destroy(&writer);
 		return kr_error(ENOMEM);
 	}
@@ -510,7 +513,7 @@ int dnstap_config(struct kr_module *module, const char *conf) {
 	data->ioq = fstrm_iothr_get_input_queue_idx(data->iothread, 0);
 	if (!data->ioq) {
 		fstrm_iothr_destroy(&data->iothread);
-		DEBUG_MSG("can't get fstrm queue\n");
+		ERROR_MSG("can't get fstrm queue\n");
 		return kr_error(EBUSY);
 	}
 
-- 
GitLab