From 725b56c093295dd00eeed33e166991356f19bb5c Mon Sep 17 00:00:00 2001 From: Fedor Telnov Date: Fri, 3 Nov 2023 09:59:25 +0300 Subject: [PATCH 1/8] add performant labels serializer `make_key` is not very efficient itself, so special `labels_serializer` is added with optimized scheme. --- metrics/api.lua | 63 +++++++++++++++++++++++++++++++++++ metrics/collectors/shared.lua | 4 +++ metrics/init.lua | 1 + test/metrics_test.lua | 27 +++++++++++++++ 4 files changed, 95 insertions(+) diff --git a/metrics/api.lua b/metrics/api.lua index d36f1c9a..21ebdcc6 100644 --- a/metrics/api.lua +++ b/metrics/api.lua @@ -125,6 +125,68 @@ local function set_global_labels(label_pairs) registry:set_labels(label_pairs) end +--- Prepares a serializer for label pairs with given keys. +--- +--- `make_key`, which is used during every metric-related operation, is not very efficient itself. +--- To mitigate it, one could add his own serialization implementation. +--- It is done via passing `__metrics_make_key` callback to the label pairs table. +--- +--- This function gives you ready-to-use serializer, so you don't have to create one yourself. +--- +--- BEWARE! If keys of the `label_pairs` somehow change between serialization turns, it would raise error mostlikely. +--- Therefore, it's important to understand full scope of needed fields. For instance, for histogram:observe, +--- an additional label 'le' is always needed. +--- +--- @class LabelsSerializer +--- @field wrap function(label_pairs: table): table Wraps given `label_pairs` with an efficient serialization. +--- @field serialize function(label_pairs: table): string Serialize given `label_pairs` into the key. +--- Exposed so you can write your own serializers on top of it. +--- +--- @param labels_keys string[] Label keys for the further use. +--- @return LabelsSerializer +local function labels_serializer(labels_keys) + table.sort(labels_keys) + + -- used to protect label_pairs from altering. + local keys_index = {} + for _, key in ipairs(labels_keys) do + keys_index[key] = true + end + + local function serialize(label_pairs) + local result = "" + for idx, label in ipairs(labels_keys) do + if idx ~= 1 then + result = result .. '\t' + end + result = result .. label .. '\t' .. label_pairs[label] + end + return result + end + + local pairs_metatable = { + __index = { + __metrics_make_key = function(self) + return serialize(self) + end + }, + -- It protects pairs from being altered with unexpected labels. + __newindex = function(table, key, value) + if not keys_index[key] then + error(('Label "%s" is unexpected'):format(key), 2) + end + table[key] = value + end + } + + return { + wrap = function(label_pairs) + return setmetatable(label_pairs, pairs_metatable) + end, + serialize = serialize + } +end + return { registry = registry, collectors = collectors, @@ -140,4 +202,5 @@ return { unregister_callback = unregister_callback, invoke_callbacks = invoke_callbacks, set_global_labels = set_global_labels, + labels_serializer = labels_serializer } diff --git a/metrics/collectors/shared.lua b/metrics/collectors/shared.lua index 2cb195b4..0234c928 100644 --- a/metrics/collectors/shared.lua +++ b/metrics/collectors/shared.lua @@ -47,6 +47,10 @@ function Shared.make_key(label_pairs) if type(label_pairs) ~= 'table' then return "" end + -- `label_pairs` provides its own serialization scheme, it must be used instead of default one. + if label_pairs.__metrics_make_key then + return label_pairs:__metrics_make_key() + end local parts = {} for k, v in pairs(label_pairs) do table.insert(parts, k .. '\t' .. v) diff --git a/metrics/init.lua b/metrics/init.lua index 9adb6662..a511e8cf 100644 --- a/metrics/init.lua +++ b/metrics/init.lua @@ -27,6 +27,7 @@ return setmetatable({ unregister_callback = api.unregister_callback, invoke_callbacks = api.invoke_callbacks, set_global_labels = api.set_global_labels, + labels_serializer = api.labels_serializer, enable_default_metrics = tarantool.enable, cfg = cfg.cfg, http_middleware = http_middleware, diff --git a/test/metrics_test.lua b/test/metrics_test.lua index 1b373305..9bbd30c2 100755 --- a/test/metrics_test.lua +++ b/test/metrics_test.lua @@ -269,3 +269,30 @@ g.test_deprecated_version = function(cg) "use require%('metrics'%)._VERSION instead." t.assert_not_equals(cg.server:grep_log(warn), nil) end + +g.test_labels_serializer_consistent = function() + local shared = require("metrics.collectors.shared") + + local label_pairs = { + abc = 123, + cba = "123", + cda = 0, + eda = -1, + acb = 456 + } + local label_keys = {} + for key, _ in pairs(label_pairs) do + table.insert(label_keys, key) + end + + local serializer = metrics.labels_serializer(label_keys) + local actual = serializer.serialize(label_pairs) + local wrapped = serializer.wrap(table.copy(label_pairs)) + + t.assert_equals(actual, shared.make_key(label_pairs)) + t.assert_equals(actual, shared.make_key(wrapped)) + t.assert_not_equals(wrapped.__metrics_make_key, nil) + + -- trying to set unexpected label. + t.assert_error_msg_contains('Label "new_label" is unexpected', function() wrapped.new_label = "123456" end) +end From 7765182ae76f146d3c6b33ca2551638ff5612921 Mon Sep 17 00:00:00 2001 From: Fedor Telnov Date: Fri, 3 Nov 2023 10:31:53 +0300 Subject: [PATCH 2/8] fix stackoverflow issue, add hist tests --- metrics/api.lua | 15 +++++++++------ test/metrics_test.lua | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/metrics/api.lua b/metrics/api.lua index 21ebdcc6..631d5bc1 100644 --- a/metrics/api.lua +++ b/metrics/api.lua @@ -147,7 +147,7 @@ end local function labels_serializer(labels_keys) table.sort(labels_keys) - -- used to protect label_pairs from altering. + -- used to protect label_pairs from altering with unexpected keys. local keys_index = {} for _, key in ipairs(labels_keys) do keys_index[key] = true @@ -155,11 +155,14 @@ local function labels_serializer(labels_keys) local function serialize(label_pairs) local result = "" - for idx, label in ipairs(labels_keys) do - if idx ~= 1 then - result = result .. '\t' + for _, label in ipairs(labels_keys) do + local value = label_pairs[label] + if value ~= nil then + if result ~= "" then + result = result .. '\t' + end + result = result .. label .. '\t' .. value end - result = result .. label .. '\t' .. label_pairs[label] end return result end @@ -175,7 +178,7 @@ local function labels_serializer(labels_keys) if not keys_index[key] then error(('Label "%s" is unexpected'):format(key), 2) end - table[key] = value + rawset(table, key, value) end } diff --git a/test/metrics_test.lua b/test/metrics_test.lua index 9bbd30c2..0544085a 100755 --- a/test/metrics_test.lua +++ b/test/metrics_test.lua @@ -5,6 +5,7 @@ local g = t.group('collectors') local metrics = require('metrics') local utils = require('test.utils') +local json = require("json") g.before_all(utils.create_server) g.after_all(utils.drop_server) @@ -295,4 +296,20 @@ g.test_labels_serializer_consistent = function() -- trying to set unexpected label. t.assert_error_msg_contains('Label "new_label" is unexpected', function() wrapped.new_label = "123456" end) + + -- must result in error, as "le" label is not precached, but is added during hist:observe. + local hist = metrics.histogram('hist', 'test histogram', {2}) + t.assert_error_msg_contains('Label "le" is unexpected', function() hist:observe(3, wrapped) end) + + -- after we add the needed key, hist:observe should work. + table.insert(label_keys, "le") + local serializer = metrics.labels_serializer(label_keys) + local hist = metrics.histogram('hist2', 'test histogram 2', {2}) + + hist:observe(3, serializer.wrap(table.copy(label_pairs))) + local state = table.deepcopy(hist) + hist:observe(3, label_pairs) + + t.assert_equals(hist.observations, state.observations) + t.assert_equals(hist.label_pairs, state.label_pairs) end From 0aeaf76bbee80d6dbfe2bc1fb4961b7d77f12f36 Mon Sep 17 00:00:00 2001 From: Fedor Telnov Date: Fri, 3 Nov 2023 10:34:27 +0300 Subject: [PATCH 3/8] satisfy linter --- metrics/api.lua | 2 +- test/metrics_test.lua | 15 +++++++-------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/metrics/api.lua b/metrics/api.lua index 631d5bc1..2e7f5bc8 100644 --- a/metrics/api.lua +++ b/metrics/api.lua @@ -143,7 +143,7 @@ end --- Exposed so you can write your own serializers on top of it. --- --- @param labels_keys string[] Label keys for the further use. ---- @return LabelsSerializer +--- @return LabelsSerializer local function labels_serializer(labels_keys) table.sort(labels_keys) diff --git a/test/metrics_test.lua b/test/metrics_test.lua index 0544085a..57e439ea 100755 --- a/test/metrics_test.lua +++ b/test/metrics_test.lua @@ -5,7 +5,6 @@ local g = t.group('collectors') local metrics = require('metrics') local utils = require('test.utils') -local json = require("json") g.before_all(utils.create_server) g.after_all(utils.drop_server) @@ -303,13 +302,13 @@ g.test_labels_serializer_consistent = function() -- after we add the needed key, hist:observe should work. table.insert(label_keys, "le") - local serializer = metrics.labels_serializer(label_keys) - local hist = metrics.histogram('hist2', 'test histogram 2', {2}) + local hist_serializer = metrics.labels_serializer(label_keys) + local hist2 = metrics.histogram('hist2', 'test histogram 2', {2}) - hist:observe(3, serializer.wrap(table.copy(label_pairs))) - local state = table.deepcopy(hist) - hist:observe(3, label_pairs) + hist2:observe(3, hist_serializer.wrap(table.copy(label_pairs))) + local state = table.deepcopy(hist2) + hist2:observe(3, label_pairs) - t.assert_equals(hist.observations, state.observations) - t.assert_equals(hist.label_pairs, state.label_pairs) + t.assert_equals(hist2.observations, state.observations) + t.assert_equals(hist2.label_pairs, state.label_pairs) end From e0fd16729057461b9f87aa3fb59647f65e240b38 Mon Sep 17 00:00:00 2001 From: Fedor Telnov Date: Fri, 3 Nov 2023 10:50:39 +0300 Subject: [PATCH 4/8] add changelog note --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 94ee47e7..d239fa67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `metrics.cfg{}` `"all"` metasection for array `include` and `exclude` (`metrics.cfg{include={'all'}}` can be used instead of `metrics.cfg{include='all'}`, `metrics.cfg{exclude={'all'}}` can be used instead of `metrics.cfg{include='none'}`) +- Allow users to decide on labels serialization scheme themselves. Add `labels_serializer`, which provides alternative efficient labels serialization scheme. ## [1.0.0] - 2023-05-22 ### Changed From 2a1901ce949802516865a74be803336c0b772f9b Mon Sep 17 00:00:00 2001 From: Fedor Telnov Date: Fri, 3 Nov 2023 14:05:06 +0300 Subject: [PATCH 5/8] automatically add needed labels --- metrics/api.lua | 5 +++-- test/metrics_test.lua | 19 +++++++------------ 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/metrics/api.lua b/metrics/api.lua index 2e7f5bc8..6474445f 100644 --- a/metrics/api.lua +++ b/metrics/api.lua @@ -134,8 +134,7 @@ end --- This function gives you ready-to-use serializer, so you don't have to create one yourself. --- --- BEWARE! If keys of the `label_pairs` somehow change between serialization turns, it would raise error mostlikely. ---- Therefore, it's important to understand full scope of needed fields. For instance, for histogram:observe, ---- an additional label 'le' is always needed. +--- We cover internal cases already, for example, "le" key is always added for the histograms. --- --- @class LabelsSerializer --- @field wrap function(label_pairs: table): table Wraps given `label_pairs` with an efficient serialization. @@ -145,6 +144,8 @@ end --- @param labels_keys string[] Label keys for the further use. --- @return LabelsSerializer local function labels_serializer(labels_keys) + -- we always add keys that are needed for metrics' internals. + table.insert(labels_keys, "le") table.sort(labels_keys) -- used to protect label_pairs from altering with unexpected keys. diff --git a/test/metrics_test.lua b/test/metrics_test.lua index 57e439ea..82659721 100755 --- a/test/metrics_test.lua +++ b/test/metrics_test.lua @@ -296,19 +296,14 @@ g.test_labels_serializer_consistent = function() -- trying to set unexpected label. t.assert_error_msg_contains('Label "new_label" is unexpected', function() wrapped.new_label = "123456" end) - -- must result in error, as "le" label is not precached, but is added during hist:observe. - local hist = metrics.histogram('hist', 'test histogram', {2}) - t.assert_error_msg_contains('Label "le" is unexpected', function() hist:observe(3, wrapped) end) - - -- after we add the needed key, hist:observe should work. - table.insert(label_keys, "le") + -- check that builtin metrics work. local hist_serializer = metrics.labels_serializer(label_keys) - local hist2 = metrics.histogram('hist2', 'test histogram 2', {2}) + local hist = metrics.histogram('hist', 'test histogram', {2}) - hist2:observe(3, hist_serializer.wrap(table.copy(label_pairs))) - local state = table.deepcopy(hist2) - hist2:observe(3, label_pairs) + hist:observe(3, hist_serializer.wrap(table.copy(label_pairs))) + local state = table.deepcopy(hist) + hist:observe(3, label_pairs) - t.assert_equals(hist2.observations, state.observations) - t.assert_equals(hist2.label_pairs, state.label_pairs) + t.assert_equals(hist.observations, state.observations) + t.assert_equals(hist.label_pairs, state.label_pairs) end From 270cbe50481715491c3d6b508d7a439396c40695 Mon Sep 17 00:00:00 2001 From: Fedor Telnov Date: Fri, 3 Nov 2023 14:15:47 +0300 Subject: [PATCH 6/8] enhance algo by protecting against repetive labels --- metrics/api.lua | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/metrics/api.lua b/metrics/api.lua index 6474445f..0d59b626 100644 --- a/metrics/api.lua +++ b/metrics/api.lua @@ -145,18 +145,22 @@ end --- @return LabelsSerializer local function labels_serializer(labels_keys) -- we always add keys that are needed for metrics' internals. - table.insert(labels_keys, "le") - table.sort(labels_keys) - + local __labels_keys = { "le" } -- used to protect label_pairs from altering with unexpected keys. - local keys_index = {} + local keys_index = { le = true } + + -- keep only unique labels for _, key in ipairs(labels_keys) do - keys_index[key] = true + if not keys_index[key] then + table.insert(__labels_keys, key) + keys_index[key] = true + end end + table.sort(__labels_keys) local function serialize(label_pairs) local result = "" - for _, label in ipairs(labels_keys) do + for _, label in ipairs(__labels_keys) do local value = label_pairs[label] if value ~= nil then if result ~= "" then From 1f6d58f7a6913eed9a5f2c269df308db834ca594 Mon Sep 17 00:00:00 2001 From: Fedor Telnov Date: Fri, 3 Nov 2023 14:49:28 +0300 Subject: [PATCH 7/8] add wrapping to http_middleware --- metrics/http_middleware.lua | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/metrics/http_middleware.lua b/metrics/http_middleware.lua index cd8655fc..b873e83a 100644 --- a/metrics/http_middleware.lua +++ b/metrics/http_middleware.lua @@ -69,6 +69,8 @@ function export.configure_default_collector(...) export.set_default_collector(export.build_default_collector(...)) end +local labels_serializer = metrics_api.labels_serializer({ "path", "method", "status" }) + --- Measure latency and invoke collector with labels from given route -- -- @tab collector @@ -86,11 +88,11 @@ function export.observe(collector, route, handler, ...) error(('incorrect http handler for %s %s: expecting return response object'): format(route.method, route.path), 0) end - return { + return labels_serializer.wrap({ path = route.path, method = route.method, status = (not ok and 500) or result.status or 200, - } + }) end, handler, ...) end From 961ec61d854152986826bf6d0dcc8cef6eb49213 Mon Sep 17 00:00:00 2001 From: Fedor Telnov Date: Fri, 3 Nov 2023 15:50:11 +0300 Subject: [PATCH 8/8] move to the separate module, rename metamethod --- metrics/api.lua | 74 ++---------------------------- metrics/collectors/shared.lua | 12 ++--- metrics/serializers.lua | 86 +++++++++++++++++++++++++++++++++++ test/metrics_test.lua | 2 +- 4 files changed, 94 insertions(+), 80 deletions(-) create mode 100644 metrics/serializers.lua diff --git a/metrics/api.lua b/metrics/api.lua index 0d59b626..b9407fd3 100644 --- a/metrics/api.lua +++ b/metrics/api.lua @@ -9,6 +9,8 @@ local Gauge = require('metrics.collectors.gauge') local Histogram = require('metrics.collectors.histogram') local Summary = require('metrics.collectors.summary') +local serializers = require("metrics.serializers") + local registry = rawget(_G, '__metrics_registry') if not registry then registry = Registry.new() @@ -125,76 +127,6 @@ local function set_global_labels(label_pairs) registry:set_labels(label_pairs) end ---- Prepares a serializer for label pairs with given keys. ---- ---- `make_key`, which is used during every metric-related operation, is not very efficient itself. ---- To mitigate it, one could add his own serialization implementation. ---- It is done via passing `__metrics_make_key` callback to the label pairs table. ---- ---- This function gives you ready-to-use serializer, so you don't have to create one yourself. ---- ---- BEWARE! If keys of the `label_pairs` somehow change between serialization turns, it would raise error mostlikely. ---- We cover internal cases already, for example, "le" key is always added for the histograms. ---- ---- @class LabelsSerializer ---- @field wrap function(label_pairs: table): table Wraps given `label_pairs` with an efficient serialization. ---- @field serialize function(label_pairs: table): string Serialize given `label_pairs` into the key. ---- Exposed so you can write your own serializers on top of it. ---- ---- @param labels_keys string[] Label keys for the further use. ---- @return LabelsSerializer -local function labels_serializer(labels_keys) - -- we always add keys that are needed for metrics' internals. - local __labels_keys = { "le" } - -- used to protect label_pairs from altering with unexpected keys. - local keys_index = { le = true } - - -- keep only unique labels - for _, key in ipairs(labels_keys) do - if not keys_index[key] then - table.insert(__labels_keys, key) - keys_index[key] = true - end - end - table.sort(__labels_keys) - - local function serialize(label_pairs) - local result = "" - for _, label in ipairs(__labels_keys) do - local value = label_pairs[label] - if value ~= nil then - if result ~= "" then - result = result .. '\t' - end - result = result .. label .. '\t' .. value - end - end - return result - end - - local pairs_metatable = { - __index = { - __metrics_make_key = function(self) - return serialize(self) - end - }, - -- It protects pairs from being altered with unexpected labels. - __newindex = function(table, key, value) - if not keys_index[key] then - error(('Label "%s" is unexpected'):format(key), 2) - end - rawset(table, key, value) - end - } - - return { - wrap = function(label_pairs) - return setmetatable(label_pairs, pairs_metatable) - end, - serialize = serialize - } -end - return { registry = registry, collectors = collectors, @@ -210,5 +142,5 @@ return { unregister_callback = unregister_callback, invoke_callbacks = invoke_callbacks, set_global_labels = set_global_labels, - labels_serializer = labels_serializer + labels_serializer = serializers.basic_labels_serializer } diff --git a/metrics/collectors/shared.lua b/metrics/collectors/shared.lua index 0234c928..1abe5600 100644 --- a/metrics/collectors/shared.lua +++ b/metrics/collectors/shared.lua @@ -1,6 +1,7 @@ local clock = require('clock') local fiber = require('fiber') local log = require('log') +local serializers = require("metrics.serializers") local Shared = {} @@ -48,15 +49,10 @@ function Shared.make_key(label_pairs) return "" end -- `label_pairs` provides its own serialization scheme, it must be used instead of default one. - if label_pairs.__metrics_make_key then - return label_pairs:__metrics_make_key() + if label_pairs.__metrics_serialize then + return label_pairs:__metrics_serialize() end - local parts = {} - for k, v in pairs(label_pairs) do - table.insert(parts, k .. '\t' .. v) - end - table.sort(parts) - return table.concat(parts, '\t') + return serializers.default_labels_serializer(label_pairs) end function Shared:remove(label_pairs) diff --git a/metrics/serializers.lua b/metrics/serializers.lua new file mode 100644 index 00000000..10505da5 --- /dev/null +++ b/metrics/serializers.lua @@ -0,0 +1,86 @@ +--- Default slow algorithm, polluted with sorting. +--- It is used when nothing is known about `label_pairs`. +local function default_labels_serializer(label_pairs) + local parts = {} + for k, v in pairs(label_pairs) do + table.insert(parts, k .. '\t' .. v) + end + table.sort(parts) + return table.concat(parts, '\t') +end + + +--- Prepares a serializer for label pairs with given keys. +--- +--- `make_key`, which is used during every metric-related operation, is not very efficient itself. +--- To mitigate it, one could add his own serialization implementation. +--- It is done via passing `__metrics_serialize` callback to the label pairs table. +--- +--- This function gives you ready-to-use serializer, so you don't have to create one yourself. +--- +--- BEWARE! If keys of the `label_pairs` somehow change between serialization turns, it would raise error mostlikely. +--- We cover internal cases already, for example, "le" key is always added for the histograms. +--- +--- @class LabelsSerializer +--- @field wrap function(label_pairs: table): table Wraps given `label_pairs` with an efficient serialization. +--- @field serialize function(label_pairs: table): string Serialize given `label_pairs` into the key. +--- Exposed so you can write your own serializers on top of it. +--- +--- @param labels_keys string[] Label keys for the further use. +--- @return LabelsSerializer +local function basic_labels_serializer(labels_keys) + -- we always add keys that are needed for metrics' internals. + local __labels_keys = { "le" } + -- used to protect label_pairs from altering with unexpected keys. + local keys_index = { le = true } + + -- keep only unique labels + for _, key in ipairs(labels_keys) do + if not keys_index[key] then + table.insert(__labels_keys, key) + keys_index[key] = true + end + end + table.sort(__labels_keys) + + local function serialize(label_pairs) + local result = "" + for _, label in ipairs(__labels_keys) do + local value = label_pairs[label] + if value ~= nil then + if result ~= "" then + result = result .. '\t' + end + result = result .. label .. '\t' .. value + end + end + return result + end + + local pairs_metatable = { + __index = { + __metrics_serialize = function(self) + return serialize(self) + end + }, + -- It protects pairs from being altered with unexpected labels. + __newindex = function(table, key, value) + if not keys_index[key] then + error(('Label "%s" is unexpected'):format(key), 2) + end + rawset(table, key, value) + end + } + + return { + wrap = function(label_pairs) + return setmetatable(label_pairs, pairs_metatable) + end, + serialize = serialize + } +end + +return { + default_labels_serializer = default_labels_serializer, + basic_labels_serializer = basic_labels_serializer +} diff --git a/test/metrics_test.lua b/test/metrics_test.lua index 82659721..2029c99b 100755 --- a/test/metrics_test.lua +++ b/test/metrics_test.lua @@ -291,7 +291,7 @@ g.test_labels_serializer_consistent = function() t.assert_equals(actual, shared.make_key(label_pairs)) t.assert_equals(actual, shared.make_key(wrapped)) - t.assert_not_equals(wrapped.__metrics_make_key, nil) + t.assert_not_equals(wrapped.__metrics_serialize, nil) -- trying to set unexpected label. t.assert_error_msg_contains('Label "new_label" is unexpected', function() wrapped.new_label = "123456" end)