diff --git a/CHANGELOG.md b/CHANGELOG.md index 01f754f2..c3a28d34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed +- metrics registry refactoring to search with `O(1)` [#188](https://github.com/tarantool/metrics/issues/188) + ### Fixed - be gentle to http routes, don't leave gaps in the array [#246](https://github.com/tarantool/metrics/issues/246) diff --git a/metrics/registry.lua b/metrics/registry.lua index 1e995163..8dc4bda2 100644 --- a/metrics/registry.lua +++ b/metrics/registry.lua @@ -15,11 +15,7 @@ function Registry:clear() end function Registry:find(kind, name) - for _, c in ipairs(self.collectors) do - if c.name == name and c.kind == kind then - return c - end - end + return self.collectors[name .. kind] end function Registry:find_or_create(class, name, ...) @@ -32,22 +28,18 @@ end function Registry:register(collector) assert(collector ~= nil, 'Collector is empty') - assert(not is_empty(collector.name), "Collector''s name is empty") - assert(not is_empty(collector.kind), "Collector''s kind is empty") + assert(not is_empty(collector.name), "Collector's name is empty") + assert(not is_empty(collector.kind), "Collector's kind is empty") if self:find(collector.kind, collector.name) then error('Already registered') end collector:set_registry(self) - table.insert(self.collectors, collector) + self.collectors[collector.name .. collector.kind] = collector return collector end function Registry:unregister(collector) - for i, c in ipairs(self.collectors) do - if c.name == collector.name and c.kind == collector.kind then - table.remove(self.collectors, i) - end - end + self.collectors[collector.name .. collector.kind] = nil end function Registry:invoke_callbacks() diff --git a/test/collectors_test.lua b/test/collectors_test.lua index 16eadf62..62c683ba 100755 --- a/test/collectors_test.lua +++ b/test/collectors_test.lua @@ -21,6 +21,14 @@ g.after_each(function() metrics.clear() end) +local function len(tbl) + local l = 0 + for _ in pairs(tbl) do + l = l + 1 + end + return l +end + g.test_counter = function() t.assert_error_msg_contains("bad argument #1 to counter (string expected, got nil)", function() metrics.counter() @@ -38,7 +46,7 @@ g.test_counter = function() local collectors = metrics.collectors() local observations = metrics.collect() local obs = utils.find_obs('cnt', {}, observations) - t.assert_equals(#collectors, 1, 'counter seen as only collector') + t.assert_equals(len(collectors), 1, 'counter seen as only collector') t.assert_equals(obs.value, 8, '3 + 5 = 8 (via metrics.collectors())') t.assert_equals(c:collect()[1].value, 8, '3 + 5 = 8') @@ -70,7 +78,7 @@ g.test_gauge = function() local collectors = metrics.collectors() local observations = metrics.collect() local obs = utils.find_obs('gauge', {}, observations) - t.assert_equals(#collectors, 1, 'gauge seen as only collector') + t.assert_equals(len(collectors), 1, 'gauge seen as only collector') t.assert_equals(obs.value, -2, '3 - 5 = -2 (via metrics.collectors())') t.assert_equals(gauge:collect()[1].value, -2, '3 - 5 = -2') @@ -100,7 +108,7 @@ g.test_histogram = function() h:observe(5) local collectors = metrics.collectors() - t.assert_equals(#collectors, 1, 'histogram seen as only 1 collector') + t.assert_equals(len(collectors), 1, 'histogram seen as only 1 collector') local observations = metrics.collect() local obs_sum = utils.find_obs('hist_sum', {}, observations) local obs_count = utils.find_obs('hist_count', {}, observations) @@ -117,7 +125,7 @@ g.test_histogram = function() h:observe(3, { foo = 'bar' }) collectors = metrics.collectors() - t.assert_equals(#collectors, 1, 'still histogram seen as only 1 collector') + t.assert_equals(len(collectors), 1, 'still histogram seen as only 1 collector') observations = metrics.collect() obs_sum = utils.find_obs('hist_sum', { foo = 'bar' }, observations) obs_count = utils.find_obs('hist_count', { foo = 'bar' }, observations) @@ -145,7 +153,7 @@ g.test_counter_cache = function() local collectors = metrics.collectors() local observations = metrics.collect() local obs = utils.find_obs('cnt', {}, observations) - t.assert_equals(#collectors, 2, 'counter_1 and counter_2 refer to the same object') + t.assert_equals(len(collectors), 2, 'counter_1 and counter_2 refer to the same object') t.assert_equals(obs.value, 8, '3 + 5 = 8') obs = utils.find_obs('cnt2', {}, observations) t.assert_equals(obs.value, 7, 'counter_3 is the only reference to cnt2') diff --git a/test/lj_metrics_test.lua b/test/lj_metrics_test.lua index 9582bbbd..c9f1d727 100755 --- a/test/lj_metrics_test.lua +++ b/test/lj_metrics_test.lua @@ -50,5 +50,5 @@ g.test_lj_metrics = function() "lj_gc_steps_pause", } - t.assert_covers(lj_metrics, expected_lj_metrics) + t.assert_items_equals(lj_metrics, expected_lj_metrics) end diff --git a/test/plugins/graphite_test.lua b/test/plugins/graphite_test.lua index 21242f17..0b152517 100755 --- a/test/plugins/graphite_test.lua +++ b/test/plugins/graphite_test.lua @@ -33,6 +33,10 @@ g.before_all(function() metrics.enable_default_metrics(); end) +g.before_each(function() + metrics.clear() +end) + g.after_each(function() -- Delete all collectors and global labels metrics.clear() @@ -96,6 +100,7 @@ g.test_graphite_sends_data_to_socket = function() local obs_table = graphite_obs:split(' ') t.assert_equals(obs_table[1], 'tarantool.test_cnt') t.assert_equals(obs_table[2], '1') + sock:close() end local function mock_graphite_worker()