Suggestions from code review

tarantool · Jul 30, 2021 · 40009c5 · 40009c5
1 parent 0af589d
commit 40009c5
Show file tree

Hide file tree

Showing 3 changed files with 4 additions and 76 deletions.
diff --git a/cartridge/roles/metrics.lua b/cartridge/roles/metrics.lua
@@ -149,6 +149,7 @@ end
 local function apply_config(conf)
     local metrics_conf = conf.metrics or {}
     metrics_conf.export = metrics_conf.export or {}
+    set_labels(metrics_conf['global-labels'])
     local paths = format_paths(metrics_conf.export)
     metrics_vars.config = table.copy(paths)
     for path, format in pairs(metrics_vars.default) do
@@ -158,7 +159,6 @@ local function apply_config(conf)
     end
     apply_routes(paths)
     metrics.enable_default_metrics(metrics_conf.include, metrics_conf.exclude)
-    set_labels(metrics_conf['global-labels'])
 end
 
 local function set_export(export)

diff --git a/metrics/registry.lua b/metrics/registry.lua
@@ -43,7 +43,7 @@ function Registry:unregister(collector)
 end
 
 function Registry:invoke_callbacks()
-    for _, registered_callback in ipairs(self.callbacks) do
+    for registered_callback, _ in pairs(self.callbacks) do
         registered_callback()
     end
 end
@@ -59,23 +59,11 @@ function Registry:collect()
 end
 
 function Registry:register_callback(callback)
-    local found = false
-    for _, registered_callback in ipairs(self.callbacks) do
-        if registered_callback == callback then
-            found = true
-        end
-    end
-    if not found then
-        table.insert(self.callbacks, callback)
-    end
+    self.callbacks[callback] = true
 end
 
 function Registry:unregister_callback(callback)
-    for i, registered_callback in ipairs(self.callbacks) do
-        if registered_callback == callback then
-            table.remove(self.callbacks, i)
-        end
-    end
+    self.callbacks[callback] = nil
 end
 
 function Registry:set_labels(label_pairs)

diff --git a/test/integration/cartridge_role_test.lua b/test/integration/cartridge_role_test.lua
@@ -671,66 +671,6 @@ g.test_invalig_global_labels_names = function()
     }, 'label name is not allowed to be "zone" or "alias"')
 end
 
-g.test_include_metrics = function()
-    local server = g.cluster.main_server
-    server:upload_config({
-        metrics = {
-            export = {
-                {
-                    path = '/metrics',
-                    format = 'json'
-                },
-            },
-            include = {
-                'vinyl', 'luajit',
-            }
-        }
-    })
-
-    local resp = server:http_request('get', '/metrics', {raise = false})
-    t.assert_equals(resp.status, 200)
-
-    local metrics_cnt = #resp.json
-    local vinyl_metrics = fun.iter(resp.json):filter(function(x)
-        return x.metric_name:find('tnt_vinyl')
-    end):length()
-    local lj_metrics = fun.iter(resp.json):filter(function(x)
-        return x.metric_name:find('lj_')
-    end):length()
-    t.assert_equals(metrics_cnt, vinyl_metrics + lj_metrics)
-end
-
-g.test_exclude_metrics = function()
-    local server = g.cluster.main_server
-    server:upload_config({
-        metrics = {
-            export = {
-                {
-                    path = '/metrics',
-                    format = 'json'
-                },
-            },
-            exclude = {
-                'vinyl', 'luajit',
-            }
-        }
-    })
-
-    local resp = server:http_request('get', '/metrics', {raise = false})
-    t.assert_equals(resp.status, 200)
-
-    local metrics_cnt = #resp.json
-    t.assert(metrics_cnt >= 0)
-    local vinyl_metrics = fun.iter(resp.json):filter(function(x)
-        return x.metric_name:find('tnt_vinyl')
-    end):length()
-    t.assert_equals(vinyl_metrics, 0)
-    local lj_metrics = fun.iter(resp.json):filter(function(x)
-        return x.metric_name:find('lj_')
-    end):length()
-    t.assert_equals(lj_metrics, 0)
-end
-
 g.test_exclude_after_include = function()
     local server = g.cluster.main_server
     server:upload_config({