-
Notifications
You must be signed in to change notification settings - Fork 26
Optimize label handling with predefined label keys in counter/gauge #508
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
Conversation
variable labels: 85.6 s Bench scriptlocal metrics = require('metrics')
local clock = require('clock')
local log = require('log')
local GAUGES_COUNT = 75
local KEYS_COUNT = 10000
local OPS_COUNT = 50000000
local label_keys = {
"service", "version", "env", "region", "status", "deployment"
}
local label_values = {
service = { "auth", "cart", "payment", "storage", "analytics", "notify", "gateway" },
version = { "v1.0", "v1.1", "v1.2", "v2.0", "v2.1", "v2.2", "v3.0-beta" },
env = { "prod", "stage", "dev", "test", "canary" },
region = { "eu-west", "us-east", "us-west", "asia-south", "asia-east" },
status = { "active", "inactive", "draining", "booting" },
deployment = { "blue", "green", "red", "canary" }
}
local function generate_random_labels()
local labels = {}
for _, key in ipairs(label_keys) do
local values = label_values[key]
labels[key] = values[math.random(1, #values)]
end
return labels
end
local label_combinations = {}
for _ = 1, KEYS_COUNT do
table.insert(label_combinations, generate_random_labels())
end
local var_gauges = {}
local fixed_gauges = {}
for i = 1, GAUGES_COUNT do
var_gauges[i] = metrics.gauge('var_gauge_' .. i, 'Description for gauge ' .. i)
fixed_gauges[i] = metrics.gauge('fixed_gauge_' .. i, 'Description for gauge ' .. i, {}, label_keys)
end
local function run_bench(gauges)
local start_time = clock.monotonic()
for _ = 1, OPS_COUNT do
local gauge_idx = math.random(1, #gauges)
local label_idx = math.random(1, #label_combinations)
local value = math.random(1, 1000)
gauges[gauge_idx]:set(value, label_combinations[label_idx])
end
return clock.monotonic() - start_time
end
local t1 = run_bench(var_gauges)
local t2 = run_bench(fixed_gauges)
local t3 = run_bench(var_gauges)
local t4 = run_bench(fixed_gauges)
log.info("variable labels: %.10f s", (t1 + t3) / 2)
log.info("fixed labels: %.10f s", (t2 + t4) / 2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the motivation? Is it the optimization or more assertive API for metrics
users? If it's the optimization, I think the solution can be expanded to remove redundant table allocation (parts = {}
) or even table.concat
. As far as I remember, they also take a lot of work.
We shouldn't forget to
- add a CHANGELOG entry,
- update the API documentation (like here).
if type(label_pairs) ~= 'table' then | ||
return "" | ||
end | ||
|
||
if label_keys ~= nil then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be
if type(label_pairs) ~= 'table' then | |
return "" | |
end | |
if label_keys ~= nil then | |
if (label_keys == nil) and (type(label_pairs) ~= 'table') then | |
return "" | |
end | |
if label_keys ~= nil then |
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If label_keys
is not nil and label_pairs
is invalid, we won't be able to make the key
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you're fine with ignoring invalid label_pairs
, including nil
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see what you mean now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have any more question to this implementation. There is also a field for improvement. For example, one can create parts
table and compute keys length only once per collector. Maybe table.concat
can be removed at all in spite of some pre-built format string. Is there actually a performance increase? And how much is it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, squash commits into a single one well-described commit.
Bump metric package submodule to 1.6.1. Commits from PR[1-8] add and remove metrics to Tarantool. 1. tarantool/metrics#508 2. tarantool/metrics#516 3. tarantool/metrics#519 4. tarantool/metrics#521 5. tarantool/metrics#524 6. tarantool/metrics#525 7. tarantool/metrics#527 8. tarantool/metrics#529 Closes #11950 @TarantoolBot document Title: Bump metrics module to 1.6.1 `'tnt_memory'`, `'tnt_memory_virt'`, `'schema_needs_upgrade'` are new metrics, introduced in metrics 1.6.0, and enabled by default. `'tnt_cartridge_config_checksum'` is new metric, introduced in metrics 1.5.0. New optional ``label_keys`` parameter for ``counter()`` and ``gauge()`` metrics are presented in metrics 1.4.0.
Bump metric package submodule to 1.6.1. Commits from PR[1-7] add and remove metrics to Tarantool. 1. tarantool/metrics#508 2. tarantool/metrics#516 3. tarantool/metrics#519 4. tarantool/metrics#521 5. tarantool/metrics#524 6. tarantool/metrics#527 7. tarantool/metrics#529 Closes #11950 @TarantoolBot document Title: Bump metrics module to 1.6.1 `'tnt_memory'`, `'tnt_memory_virt'`, `'schema_needs_upgrade'` are new metrics, introduced in metrics 1.6.0, and enabled by default. `'tnt_cartridge_config_checksum'` is new metric, introduced in metrics 1.5.0. New optional ``label_keys`` parameter for ``counter()`` and ``gauge()`` metrics are presented in metrics 1.4.0.
Bump metric package submodule to 1.6.1. Commits from PR[1-7] add new metrics to Tarantool. 1. tarantool/metrics#508 2. tarantool/metrics#516 3. tarantool/metrics#519 4. tarantool/metrics#521 5. tarantool/metrics#524 6. tarantool/metrics#527 7. tarantool/metrics#529 Closes #11950 @TarantoolBot document Title: Bump metrics module to 1.6.1 `'tnt_memory'`, `'tnt_memory_virt'`, `'schema_needs_upgrade'` are new metrics, introduced in metrics 1.6.0, and enabled by default. `'tnt_cartridge_config_checksum'` is new metric, introduced in metrics 1.5.0. New optional ``label_keys`` parameter for ``counter()`` and ``gauge()`` metrics are presented in metrics 1.4.0.
Bump metric package submodule to 1.6.1. Commits from PR[1-7] add new metrics to Tarantool. 1. tarantool/metrics#508 2. tarantool/metrics#516 3. tarantool/metrics#519 4. tarantool/metrics#521 5. tarantool/metrics#524 6. tarantool/metrics#527 7. tarantool/metrics#529 Closes #11950 @TarantoolBot document Title: Bump metrics module to 1.6.1 `'tnt_memory'`, `'tnt_memory_virt'`, `'schema_needs_upgrade'` are new metrics, introduced in metrics 1.6.0, and enabled by default. `'tnt_cartridge_config_checksum'` is new metric, introduced in metrics 1.5.0. New optional ``label_keys`` parameter for ``counter()`` and ``gauge()`` metrics are presented in metrics 1.4.0.
This change adds support for predefined label keys in
counter
andgauge
metric constructors, optimizing label processing.Changes:
label_keys
table tocounter()
andgauge()
initializationlabel_keys
are provided, eliminating unnecessary processing overheadDynamic labels still supported when
label_keys
is omitted.Closes TNTP-3453