[Tarantool-patches] [PATCH 4/4] feedback: add cmake option to disable the daemon

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

There is a complaint that the feedback daemon is a 'spying' tool
and because of that can't be used on Gentoo. Its default disabled
option also is not acceptable, the daemon should be eliminated
completely.

The patch introduces cmake option ENABLE_FEEDBACK_DAEMON. It is
ON by default. When set to OFF, all feedback daemon's code is not
included into the binary, its configuration options disappear.

Closes #3308
---
 src/box/CMakeLists.txt                | 10 ++++++++-
 src/box/lua/init.c                    |  9 +++++++++
 src/box/lua/load_cfg.lua              | 29 +++++++++++++++++++++------
 src/trivia/config.h.cmake             |  2 ++
 test/box-tap/feedback_daemon.test.lua | 23 +++++++++++++++++----
 test/box/misc.result                  | 19 ++++++++++++++++--
 test/box/misc.test.lua                | 15 +++++++++++++-
 7 files changed, 93 insertions(+), 14 deletions(-)

diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
index 56758bd2f..4827e786e 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -1,5 +1,11 @@
 file(MAKE_DIRECTORY ${CMAKE_BINARY_DIR}/src/box/lua)
 
+# Sometimes 'spying' code is not acceptable even if it would be
+# disabled by default. That option allows to remove the feedback
+# daemon from the build completely.
+option(ENABLE_FEEDBACK_DAEMON "Feedback daemon which reports debug data to \
+       the Tarantool team" ON)
+
 add_subdirectory(sql)
 
 set(lua_sources)
@@ -7,7 +13,9 @@ lua_source(lua_sources lua/load_cfg.lua)
 lua_source(lua_sources lua/schema.lua)
 lua_source(lua_sources lua/tuple.lua)
 lua_source(lua_sources lua/session.lua)
-lua_source(lua_sources lua/feedback_daemon.lua)
+if (ENABLE_FEEDBACK_DAEMON)
+    lua_source(lua_sources lua/feedback_daemon.lua)
+endif()
 lua_source(lua_sources lua/net_box.lua)
 lua_source(lua_sources lua/upgrade.lua)
 lua_source(lua_sources lua/console.lua)
diff --git a/src/box/lua/init.c b/src/box/lua/init.c
index 9db740de6..29217c21e 100644
--- a/src/box/lua/init.c
+++ b/src/box/lua/init.c
@@ -71,7 +71,9 @@ extern char session_lua[],
 	schema_lua[],
 	load_cfg_lua[],
 	xlog_lua[],
+#if ENABLE_FEEDBACK_DAEMON
 	feedback_daemon_lua[],
+#endif
 	net_box_lua[],
 	upgrade_lua[],
 	console_lua[],
@@ -81,7 +83,14 @@ static const char *lua_sources[] = {
 	"box/session", session_lua,
 	"box/tuple", tuple_lua,
 	"box/schema", schema_lua,
+#if ENABLE_FEEDBACK_DAEMON
+	/*
+	 * It is important to initialize the daemon before
+	 * load_cfg, because the latter peeks up some values
+	 * from the feedback daemon.
+	 */
 	"box/feedback_daemon", feedback_daemon_lua,
+#endif
 	"box/upgrade", upgrade_lua,
 	"box/net_box", net_box_lua,
 	"box/console", console_lua,
diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index b671eb7a2..5d818addf 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -21,6 +21,20 @@ local function locked(f)
     end
 end
 
+--
+-- When feedback is disabled, every single mentioning of it should
+-- be eliminated. Even box.cfg{} should not accept any
+-- 'feedback_*' parameters as valid. This is why they are set to
+-- nil, when the daemon does not exist.
+--
+local function ifdef_feedback(value)
+    return private.feedback_daemon ~= nil and value or nil
+end
+
+local ifdef_feedback_set_params =
+    private.feedback_daemon ~= nil and
+    private.feedback_daemon.set_feedback_params or nil
+
 -- all available options
 local default_cfg = {
     listen              = nil,
@@ -143,9 +157,9 @@ local template_cfg = {
     replication_connect_quorum = 'number',
     replication_skip_conflict = 'boolean',
     replication_anon      = 'boolean',
-    feedback_enabled      = 'boolean',
-    feedback_host         = 'string',
-    feedback_interval     = 'number',
+    feedback_enabled      = ifdef_feedback('boolean'),
+    feedback_host         = ifdef_feedback('string'),
+    feedback_interval     = ifdef_feedback('number'),
     net_msg_max           = 'number',
     sql_cache_size        = 'number',
 }
@@ -236,9 +250,9 @@ local dynamic_cfg = {
     checkpoint_interval     = private.cfg_set_checkpoint_interval,
     checkpoint_wal_threshold = private.cfg_set_checkpoint_wal_threshold,
     worker_pool_threads     = private.cfg_set_worker_pool_threads,
-    feedback_enabled        = private.feedback_daemon.set_feedback_params,
-    feedback_host           = private.feedback_daemon.set_feedback_params,
-    feedback_interval       = private.feedback_daemon.set_feedback_params,
+    feedback_enabled        = ifdef_feedback_set_params,
+    feedback_host           = ifdef_feedback_set_params,
+    feedback_interval       = ifdef_feedback_set_params,
     -- do nothing, affects new replicas, which query this value on start
     wal_dir_rescan_delay    = function() end,
     custom_proc_title       = function()
@@ -258,6 +272,9 @@ local dynamic_cfg = {
     sql_cache_size          = private.cfg_set_sql_cache_size,
 }
 
+ifdef_feedback = nil
+ifdef_feedback_set_params = nil
+
 --
 -- For some options it is important in which order they are set.
 -- For example, setting 'replication', including self, before
diff --git a/src/trivia/config.h.cmake b/src/trivia/config.h.cmake
index 226f83128..89e0d39c6 100644
--- a/src/trivia/config.h.cmake
+++ b/src/trivia/config.h.cmake
@@ -251,6 +251,8 @@
 /* Cacheline size to calculate alignments */
 #define CACHELINE_SIZE 64
 
+#cmakedefine ENABLE_FEEDBACK_DAEMON 1
+
 /*
  * vim: syntax=c
  */
diff --git a/test/box-tap/feedback_daemon.test.lua b/test/box-tap/feedback_daemon.test.lua
index 9e6c59efc..d4adb71f1 100755
--- a/test/box-tap/feedback_daemon.test.lua
+++ b/test/box-tap/feedback_daemon.test.lua
@@ -8,8 +8,6 @@ local fiber = require('fiber')
 local test = tap.test('feedback_daemon')
 local socket = require('socket')
 
-test:plan(11)
-
 box.cfg{log = 'report.log', log_level = 6}
 
 local feedback_save = nil
@@ -49,10 +47,27 @@ local server = socket.tcp_server("127.0.0.1", 0, http_handle)
 local port = server:name().port
 
 local interval = 0.01
-box.cfg{
+local ok, err = pcall(box.cfg, {
     feedback_host = string.format("127.0.0.1:%i", port),
     feedback_interval = interval,
-}
+})
+
+if not ok then
+    --
+    -- gh-3308: feedback daemon is an optional pre-compile time
+    -- defined feature, depending on CMake flags. It is not
+    -- present always. When it is disabled, is should not be
+    -- visible anywhere.
+    --
+    test:plan(2)
+    test:diag('Feedback daemon is supposed to be disabled')
+    test:like(err, 'unexpected option', 'Feedback options are undefined')
+    test:isnil(box.feedback, 'No feedback daemon - no its API')
+    test:check()
+    os.exit(0)
+end
+
+test:plan(11)
 
 local function check(message)
     while feedback_count < 1 do
diff --git a/test/box/misc.result b/test/box/misc.result
index 9b76d7665..10699adc2 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -49,8 +49,24 @@ index = space:create_index('primary', { type = 'hash' })
 ---
 - '  lua says: hello'
 ...
+--
+-- gh-3308: feedback daemon is an optional pre-compile time
+-- defined feature, depending on CMake flags. It is not present
+-- always.
+--
+optional = {feedback = true}
+---
+...
 -- # What's in the box?
-t = {} for n in pairs(box) do table.insert(t, tostring(n)) end table.sort(t)
+t = {}
+---
+...
+for n in pairs(box) do                                                          \
+    if not optional[n] then                                                     \
+        table.insert(t, tostring(n))                                            \
+    end                                                                         \
+end                                                                             \
+table.sort(t)
 ---
 ...
 t
@@ -64,7 +80,6 @@ t
   - ctl
   - error
   - execute
-  - feedback
   - func
   - index
   - info
diff --git a/test/box/misc.test.lua b/test/box/misc.test.lua
index 350f9f75d..4d7fb0a16 100644
--- a/test/box/misc.test.lua
+++ b/test/box/misc.test.lua
@@ -17,8 +17,21 @@ index = space:create_index('primary', { type = 'hash' })
 -- Test Lua from admin console. Whenever producing output,
 -- make sure it's a valid YAML.
 '  lua says: hello'
+
+--
+-- gh-3308: feedback daemon is an optional pre-compile time
+-- defined feature, depending on CMake flags. It is not present
+-- always.
+--
+optional = {feedback = true}
 -- # What's in the box?
-t = {} for n in pairs(box) do table.insert(t, tostring(n)) end table.sort(t)
+t = {}
+for n in pairs(box) do                                                          \
+    if not optional[n] then                                                     \
+        table.insert(t, tostring(n))                                            \
+    end                                                                         \
+end                                                                             \
+table.sort(t)
 t
 t = nil
 
-- 
2.21.1 (Apple Git-122.3)