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

Registry refactored to search with O(1) #244

Merged
merged 5 commits into from
Jul 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 5 additions & 13 deletions metrics/registry.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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, ...)
Expand All @@ -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()
Expand Down
18 changes: 13 additions & 5 deletions test/collectors_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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')
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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')
Expand Down
2 changes: 1 addition & 1 deletion test/lj_metrics_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 5 additions & 0 deletions test/plugins/graphite_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down