From 23cf4649e0ad55a24d5fbcf21da767fd06c4d0f5 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Thu, 13 Jan 2022 10:27:28 +0300 Subject: [PATCH] Fix statistics module ldoc descriptions Follows PR review. Use ldoc aliases for types and [opt] for optional parameters. Fix typos and outdated info in descriptions. Fix incorrect description of argument table fields. Use @local for local functions. Wrap language constructions in backticks. It is not clear how to describe modules with constants for ldoc. ldoc-styled description for `crud.stats.operation` is available at `crud.stats.module`. See [1] for possible updates. 1. https://github.com/lunarmodules/LDoc/issues/369 Follows up #224 --- crud/stats/local_registry.lua | 80 ++++++++++--------- crud/stats/metrics_registry.lua | 106 ++++++++++++++------------ crud/stats/module.lua | 131 ++++++++++++++++++++------------ crud/stats/operation.lua | 5 ++ crud/stats/registry_common.lua | 15 ++-- crud/stats/stash.lua | 8 +- 6 files changed, 202 insertions(+), 143 deletions(-) diff --git a/crud/stats/local_registry.lua b/crud/stats/local_registry.lua index 1a5de6288..eff24abba 100644 --- a/crud/stats/local_registry.lua +++ b/crud/stats/local_registry.lua @@ -1,3 +1,6 @@ +---- Module +-- @module crud.stats.local_registry +-- local errors = require('errors') local dev_checks = require('crud.common.dev_checks') @@ -9,18 +12,19 @@ local registry = {} local internal = stash.get('local_registry') local StatsLocalError = errors.new_class('StatsLocalError', {capture_stack = false}) ---- Initialize local metrics registry +--- Initialize local metrics registry. -- -- Registries are not meant to used explicitly -- by users, init is not guaranteed to be idempotent. -- -- @function init --- @tparam table opts -- --- @tfield boolean quantiles --- Quantiles is not supported for local, only `false` is valid. +-- @tab opts +-- +-- @bool opts.quantiles +-- Quantiles is not supported for local, only `false` is valid. -- --- @treturn boolean Returns true. +-- @treturn boolean Returns `true`. -- function registry.init(opts) dev_checks({ quantiles = 'boolean' }) @@ -36,14 +40,14 @@ function registry.init(opts) return true end ---- Destroy local metrics registry +--- Destroy local metrics registry. -- -- Registries are not meant to used explicitly -- by users, destroy is not guaranteed to be idempotent. -- -- @function destroy -- --- @treturn boolean Returns true. +-- @treturn boolean Returns `true`. -- function registry.destroy() internal.registry = nil @@ -51,20 +55,20 @@ function registry.destroy() return true end ---- Get copy of local metrics registry +--- Get copy of local metrics registry. -- -- Registries are not meant to used explicitly -- by users, get is not guaranteed to work without init. -- -- @function get -- --- @tparam string space_name --- (Optional) If specified, returns table with statistics +-- @string[opt] space_name +-- If specified, returns table with statistics -- of operations on table, separated by operation type and -- execution status. If there wasn't any requests for table, --- returns {}. In not specified, returns table with statistics --- about all existing spaces and count of calls to spaces --- that wasn't found. +-- returns `{}`. If not specified, returns table with statistics +-- about all existing spaces, count of calls to spaces +-- that wasn't found and count of schema reloads. -- -- @treturn table Returns copy of metrics registry (or registry section). -- @@ -78,14 +82,14 @@ function registry.get(space_name) return table.deepcopy(internal.registry) end ---- Check if space statistics are present in registry +--- Check if space statistics are present in registry. -- -- @function is_unknown_space -- --- @tparam string space_name +-- @string space_name -- Name of space. -- --- @treturn boolean True, if space stats found. False otherwise. +-- @treturn boolean `true`, if space stats found. `false` otherwise. -- function registry.is_unknown_space(space_name) dev_checks('string') @@ -93,24 +97,24 @@ function registry.is_unknown_space(space_name) return internal.registry.spaces[space_name] == nil end ---- Increase requests count and update latency info +--- Increase requests count and update latency info. -- -- @function observe -- --- @tparam string space_name +-- @string space_name -- Name of space. -- --- @tparam number latency +-- @number latency -- Time of call execution. -- --- @tparam string op +-- @string op -- Label of registry collectors. --- Use `require('crud.common.const').OP` to pick one. +-- Use `require('crud.stats.module').op` to pick one. -- --- @tparam string success --- 'ok' if no errors on execution, 'error' otherwise. +-- @string success +-- `'ok'` if no errors on execution, `'error'` otherwise. -- --- @treturn boolean Returns true. +-- @treturn boolean Returns `true`. -- function registry.observe(latency, space_name, op, status) dev_checks('number', 'string', 'string', 'string') @@ -125,11 +129,11 @@ function registry.observe(latency, space_name, op, status) return true end ---- Increase count of "space not found" collector by one +--- Increase count of "space not found" collector by one. -- -- @function observe_space_not_found -- --- @treturn boolean Returns true. +-- @treturn boolean Returns `true`. -- function registry.observe_space_not_found() internal.registry.space_not_found = internal.registry.space_not_found + 1 @@ -137,20 +141,20 @@ function registry.observe_space_not_found() return true end ---- Increase statistics of storage select/pairs calls +--- Increase statistics of storage select/pairs calls. -- -- @function observe_fetch -- --- @tparam string space_name +-- @string space_name -- Name of space. -- --- @tparam number tuples_fetched +-- @number tuples_fetched -- Count of tuples fetched during storage call. -- --- @tparam number tuples_lookup +-- @number tuples_lookup -- Count of tuples looked up on storages while collecting response. -- --- @treturn boolean Returns true. +-- @treturn boolean Returns `true`. -- function registry.observe_fetch(tuples_fetched, tuples_lookup, space_name) dev_checks('number', 'number', 'string') @@ -165,17 +169,17 @@ function registry.observe_fetch(tuples_fetched, tuples_lookup, space_name) return true end ---- Increase statistics of planned map reduces during select/pairs +--- Increase statistics of planned map reduces during select/pairs. -- -- @function observe_map_reduces -- --- @tparam number count +-- @number count -- Count of map reduces planned. -- --- @tparam string space_name +-- @string space_name -- Name of space. -- --- @treturn boolean Returns true. +-- @treturn boolean Returns `true`. -- function registry.observe_map_reduces(count, space_name) dev_checks('number', 'string') @@ -189,14 +193,14 @@ function registry.observe_map_reduces(count, space_name) return true end ---- Increase statistics of schema reloads +--- Increase statistics of schema reloads. -- -- @function observe_schema_reloads -- --- @tparam number count +-- @number count -- Schema reloads performed. -- --- @treturn boolean Returns true. +-- @treturn boolean Returns `true`. -- function registry.observe_schema_reloads(count) dev_checks('number') diff --git a/crud/stats/metrics_registry.lua b/crud/stats/metrics_registry.lua index bf66f89c7..ceccc67e9 100644 --- a/crud/stats/metrics_registry.lua +++ b/crud/stats/metrics_registry.lua @@ -1,3 +1,6 @@ +---- Module +-- @module crud.stats.metrics_registry +-- local is_package, metrics = pcall(require, 'metrics') local dev_checks = require('crud.common.dev_checks') @@ -56,11 +59,11 @@ local DEFAULT_AGE_PARAMS = { -- In fact, user may use `metrics >= 0.5.0`, `metrics != 0.9.0` -- if he wants to use metrics without quantiles, and `metrics >= 0.9.0` -- if he wants to use metrics with quantiles. But this is confusing, --- so let's use a single restriction for both cases. +-- so we use a single restriction solving both cases. -- -- @function is_supported -- --- @treturn boolean Returns true if `metrics >= 0.10.0` found, false otherwise. +-- @treturn boolean Returns `true` if `metrics >= 0.10.0` found, `false` otherwise. -- function registry.is_supported() if is_package == false then @@ -83,12 +86,12 @@ end -- -- @function init -- --- @tparam table opts +-- @tab opts -- --- @tfield boolean quantiles --- If true, computes latency as 0.99 quantile with aging. +-- @bool opts.quantiles +-- If `true`, computes latency as 0.99 quantile with aging. -- --- @treturn boolean Returns true. +-- @treturn boolean Returns `true`. -- function registry.init(opts) dev_checks({ quantiles = 'boolean' }) @@ -132,7 +135,7 @@ function registry.init(opts) return true end ---- Unregister collectors in global metrics registry +--- Unregister collectors in global metrics registry. -- -- Registries are not meant to used explicitly -- by users, destroy is not guaranteed to be idempotent. @@ -140,7 +143,7 @@ end -- -- @function destroy -- --- @treturn boolean Returns true. +-- @treturn boolean Returns `true`. -- function registry.destroy() for _, c in pairs(internal.registry) do @@ -153,19 +156,21 @@ function registry.destroy() return true end ---- Compute `latency` field of an observation --- If it is a { time = ..., count = ... } observation, +--- Compute `latency` field of an observation. +-- +-- If it is a `{ time = ..., count = ... }` observation, -- compute latency as overall average and store it -- inside observation object. -- --- @tparam table obs --- Objects from registry_common --- stats.spaces[name][op][status]. +-- @function compute_obs_latency +-- @local +-- +-- @tab obs +-- Objects from `registry_common` +-- `stats.spaces[name][op][status]`. -- If something like `details` collector -- passed, do nothing. -- --- @function compute_obs_latency --- local function compute_obs_latency(obs) if obs.count == nil or obs.time == nil then return @@ -178,12 +183,17 @@ local function compute_obs_latency(obs) end end ---- Compute `latency` field of each observation +--- Compute `latency` field of each observation. +-- -- If quantiles disabled, we need to compute -- latency as overall average from `time` and -- `count` values. -- --- @function compute_obs_latency +-- @function compute_latencies +-- @local +-- +-- @tab stats +-- Object from registry_common stats. -- local function compute_latencies(stats) for _, space_stats in pairs(stats.spaces) do @@ -195,20 +205,20 @@ local function compute_latencies(stats) end end ---- Get copy of global metrics registry +--- Get copy of global metrics registry. -- -- Registries are not meant to used explicitly -- by users, get is not guaranteed to work without init. -- -- @function get -- --- @tparam string space_name --- (Optional) If specified, returns table with statistics +-- @string[opt] space_name +-- If specified, returns table with statistics -- of operations on table, separated by operation type and -- execution status. If there wasn't any requests for table, --- returns {}. In not specified, returns table with statistics --- about all existing spaces and count of calls to spaces --- that wasn't found. +-- returns `{}`. If not specified, returns table with statistics +-- about all existing spaces, count of calls to spaces +-- that wasn't found and count of schema reloads. -- -- @treturn table Returns copy of metrics registry. function registry.get(space_name) @@ -285,14 +295,14 @@ function registry.get(space_name) return stats end ---- Check if space statistics are present in registry +--- Check if space statistics are present in registry. -- -- @function is_unknown_space -- --- @tparam string space_name +-- @string space_name -- Name of space. -- --- @treturn boolean True, if space stats found. False otherwise. +-- @treturn boolean `true`, if space stats found. `false` otherwise. -- function registry.is_unknown_space(space_name) dev_checks('string') @@ -318,24 +328,24 @@ function registry.is_unknown_space(space_name) return true end ---- Increase requests count and update latency info +--- Increase requests count and update latency info. -- -- @function observe -- --- @tparam string space_name +-- @string space_name -- Name of space. -- --- @tparam number latency +-- @number latency -- Time of call execution. -- --- @tparam string op +-- @string op -- Label of registry collectors. --- Use `require('crud.common.const').OP` to pick one. +-- Use `require('crud.stats.module').op` to pick one. -- --- @tparam string success --- 'ok' if no errors on execution, 'error' otherwise. +-- @string success +-- `'ok'` if no errors on execution, `'error'` otherwise. -- --- @treturn boolean Returns true. +-- @treturn boolean Returns `true`. -- function registry.observe(latency, space_name, op, status) dev_checks('number', 'string', 'string', 'string') @@ -350,11 +360,11 @@ function registry.observe(latency, space_name, op, status) return true end ---- Increase count of "space not found" collector by one +--- Increase count of "space not found" collector by one. -- -- @function observe_space_not_found -- --- @treturn boolean Returns true. +-- @treturn boolean Returns `true`. -- function registry.observe_space_not_found() internal.registry[metric_name.space_not_found]:inc(1) @@ -362,20 +372,20 @@ function registry.observe_space_not_found() return true end ---- Increase statistics of storage select/pairs calls +--- Increase statistics of storage select/pairs calls. -- -- @function observe_fetch -- --- @tparam string space_name +-- @string space_name -- Name of space. -- --- @tparam number tuples_fetched +-- @number tuples_fetched -- Count of tuples fetched during storage call. -- --- @tparam number tuples_lookup +-- @number tuples_lookup -- Count of tuples looked up on storages while collecting response. -- --- @treturn boolean Returns true. +-- @treturn boolean Returns `true`. -- function registry.observe_fetch(tuples_fetched, tuples_lookup, space_name) dev_checks('number', 'number', 'string') @@ -388,17 +398,17 @@ function registry.observe_fetch(tuples_fetched, tuples_lookup, space_name) return true end ---- Increase statistics of planned map reduces during select/pairs +--- Increase statistics of planned map reduces during select/pairs. -- -- @function observe_map_reduces -- --- @tparam number count +-- @number count -- Count of map reduces planned. -- --- @tparam string space_name +-- @string space_name -- Name of space. -- --- @treturn boolean Returns true. +-- @treturn boolean Returns `true`. -- function registry.observe_map_reduces(count, space_name) dev_checks('number', 'string') @@ -409,14 +419,14 @@ function registry.observe_map_reduces(count, space_name) return true end ---- Increase statistics of schema reloads +--- Increase statistics of schema reloads. -- -- @function observe_schema_reloads -- --- @tparam number count +-- @number count -- Schema reloads performed. -- --- @treturn boolean Returns true. +-- @treturn boolean Returns `true`. -- function registry.observe_schema_reloads(count) dev_checks('number') diff --git a/crud/stats/module.lua b/crud/stats/module.lua index e4465d604..1432983e0 100644 --- a/crud/stats/module.lua +++ b/crud/stats/module.lua @@ -1,3 +1,6 @@ +---- Module +-- @module crud.stats.module +-- local clock = require('clock') local checks = require('checks') local errors = require('errors') @@ -6,7 +9,6 @@ local vshard = require('vshard') local dev_checks = require('crud.common.dev_checks') local fiber_context = require('crud.common.fiber_context') local utils = require('crud.common.utils') -local op_module = require('crud.stats.operation') local stash = require('crud.stats.stash') local StatsError = errors.new_class('StatsError', {capture_stack = false}) @@ -24,35 +26,40 @@ if metrics_registry.is_supported() then drivers['metrics'] = metrics_registry end ---- Check if statistics module if enabled +--- Check if statistics module if enabled. -- -- @function is_enabled -- --- @treturn[1] boolean Returns true or false. +-- @treturn[1] boolean Returns `true` or `false`. -- function stats.is_enabled() return internal.registry ~= nil end ---- Initializes statistics registry, enables callbacks and wrappers +--- Initializes statistics registry, enables callbacks and wrappers. -- -- If already enabled, do nothing. -- -- @function enable -- --- @tparam table opts +-- @tab[opt] opts -- --- @tfield string driver --- 'local' or 'metrics'. --- If 'local', stores statistics in local registry (some Lua tables) --- and computes latency as overall average. 'metrics' requires --- `metrics >= 0.9.0` installed and stores statistics in --- global metrics registry (integrated with exporters) --- and computes latency as 0.99 quantile with aging. --- If 'metrics' driver is available, it is used by default, --- otherwise 'local' is used. +-- @string[opt] opts.driver +-- `'local'` or `'metrics'`. +-- If `'local'`, stores statistics in local registry (some Lua tables) +-- and computes latency as overall average. `'metrics'` requires +-- `metrics >= 0.9.0` installed and stores statistics in +-- global metrics registry (integrated with exporters). +-- `'metrics'` driver supports computing latency as 0.99 quantile with aging. +-- If `'metrics'` driver is available, it is used by default, +-- otherwise `'local'` is used. -- --- @treturn boolean Returns true. +-- @bool[opt=false] opts.quantiles +-- If `'metrics'` driver used, you can enable +-- computing requests latency as 0.99 quantile with aging. +-- Performance overhead for enabling is near 10%. +-- +-- @treturn boolean Returns `true`. -- function stats.enable(opts) checks({ driver = '?string', quantiles = '?boolean' }) @@ -97,14 +104,14 @@ function stats.enable(opts) return true end ---- Resets statistics registry +--- Resets statistics registry. -- --- After reset collectors set is the same as right --- after first stats.enable(). +-- After reset collectors are the same as right +-- after initial stats.enable(). -- -- @function reset -- --- @treturn boolean Returns true. +-- @treturn boolean Returns `true`. -- function stats.reset() if not stats.is_enabled() then @@ -117,7 +124,7 @@ function stats.reset() return true end ---- Destroys statistics registry and disable callbacks +--- Destroys statistics registry and disable callbacks. -- -- If already disabled, do nothing. -- @@ -137,12 +144,12 @@ function stats.disable() return true end ---- Get statistics on CRUD operations +--- Get statistics on CRUD operations. -- -- @function get -- --- @tparam string space_name --- (Optional) If specified, returns table with statistics +-- @string[opt] space_name +-- If specified, returns table with statistics -- of operations on space, separated by operation type and -- execution status. If there wasn't any requests of "op" type -- for space, there won't be corresponding collectors. @@ -151,7 +158,7 @@ end -- that wasn't found. -- -- @treturn table Statistics on CRUD operations. --- If statistics disabled, returns {}. +-- If statistics disabled, returns `{}`. -- function stats.get(space_name) checks('?string') @@ -245,32 +252,32 @@ local function wrap_tail(space_name, op, opts, start_time, call_status, ...) return ... end ---- Wrap CRUD operation call to collect statistics +--- Wrap CRUD operation call to collect statistics. -- -- Approach based on `box.atomic()`: -- https://github.com/tarantool/tarantool/blob/b9f7204b5e0d10b443c6f198e9f7f04e0d16a867/src/box/lua/schema.lua#L369 -- -- @function wrap -- --- @tparam function func +-- @func func -- Function to wrap. First argument is expected to -- be a space name string. If statistics enabled, -- errors are caught and thrown again. -- --- @tparam string op +-- @string op -- Label of registry collectors. -- Use `require('crud.stats.module').op` to pick one. -- --- @tparam table opts +-- @tab[opt] opts -- --- @tfield boolean pairs --- (Optional, default: false) If false, second return value --- of wrapped function is treated as error (`nil, err` case). --- Since pairs calls return three arguments as generator --- and throw errors if needed, use { pairs = true } to --- wrap them. +-- @bool[opt=false] opts.pairs +-- If false, second return value +-- of wrapped function is treated as error (`nil, err` case). +-- Since pairs calls return three arguments as generator +-- and throw errors if needed, use `{ pairs = true }` to +-- wrap them. -- --- @return First two arguments of wrapped function output. +-- @return Wrapped function output. -- function stats.wrap(func, op, opts) dev_checks('function', 'string', { pairs = '?boolean' }) @@ -293,23 +300,24 @@ function stats.wrap(func, op, opts) end local storage_stats_schema = { tuples_fetched = 'number', tuples_lookup = 'number' } ---- Callback to collect storage tuples stats (select/pairs) +--- Callback to collect storage tuples stats (select/pairs). -- -- @function update_fetch_stats +-- @local -- --- @tparam table storage_stats +-- @tab storage_stats -- Statistics from select storage call. -- --- @tfield number tuples_fetched --- Count of tuples fetched during storage call. +-- @number storage_stats.tuples_fetched +-- Count of tuples fetched during storage call. -- --- @tfield number tuples_lookup --- Count of tuples looked up on storages while collecting response. +-- @number storage_stats.tuples_lookup +-- Count of tuples looked up on storages while collecting response. -- --- @tparam string space_name +-- @string space_name -- Name of space. -- --- @treturn boolean Returns true. +-- @treturn boolean Returns `true`. -- local function update_fetch_stats(storage_stats, space_name) dev_checks(storage_stats_schema, 'string') @@ -327,7 +335,7 @@ local function update_fetch_stats(storage_stats, space_name) return true end ---- Returns callback to collect storage tuples stats (select/pairs) +--- Returns callback to collect storage tuples stats (select/pairs). -- -- @function get_fetch_callback -- @@ -342,15 +350,40 @@ function stats.get_fetch_callback() return update_fetch_stats end ---- Table with CRUD operation lables +--- Table with CRUD operation lables. +-- +-- @tfield string INSERT +-- Identifies both `insert` and `insert_object`. +-- +-- @tfield string GET +-- +-- @tfield string REPLACE +-- Identifies both `replace` and `replace_object`. -- --- @table label +-- @tfield string UPDATE -- -stats.op = op_module +-- @tfield string UPSERT +-- Identifies both `upsert` and `upsert_object`. +-- +-- @tfield string DELETE +-- +-- @tfield string SELECT +-- Identifies both `pairs` and `select`. +-- +-- @tfield string TRUNCATE +-- +-- @tfield string LEN +-- +-- @tfield string BORDERS +-- Identifies both `min` and `max`. +-- +stats.op = require('crud.stats.operation') ---- Stats module internal state (for debug/test) +--- Stats module internal state (for debug/test). +-- +-- @tfield[opt] table opts Current module options. -- --- @table internal +-- @tfield[opt] table driver Current module driver package. -- stats.internal = internal diff --git a/crud/stats/operation.lua b/crud/stats/operation.lua index ee1f98266..607ea1aaf 100644 --- a/crud/stats/operation.lua +++ b/crud/stats/operation.lua @@ -1,3 +1,8 @@ +-- It is not clear how to describe modules +-- with constants for ldoc. ldoc-styled description +-- for this mpdule is available at `crud.stats.module`. +-- See https://github.com/lunarmodules/LDoc/issues/369 +-- for possible updates. return { -- INSERT identifies both `insert` and `insert_object`. INSERT = 'insert', diff --git a/crud/stats/registry_common.lua b/crud/stats/registry_common.lua index 48bd30f42..7a29845a0 100644 --- a/crud/stats/registry_common.lua +++ b/crud/stats/registry_common.lua @@ -1,13 +1,16 @@ +---- Module +-- @module crud.stats.registry_common +-- local dev_checks = require('crud.common.dev_checks') local op_module = require('crud.stats.operation') local registry_common = {} ---- Build collectors for local registry +--- Build collectors for local registry. -- -- @function build_collectors -- --- @tparam string op +-- @string op -- Label of registry collectors. -- Use `require('crud.stats.module').op` to pick one. -- @@ -42,17 +45,17 @@ function registry_common.build_collectors(op) return collectors end ---- Initialize all statistic collectors for a space operation +--- Initialize all statistic collectors for a space operation. -- -- @function init_collectors_if_required -- --- @tparam table spaces +-- @tab spaces -- `spaces` section of registry. -- --- @tparam string space_name +-- @string space_name -- Name of space. -- --- @tparam string op +-- @string op -- Label of registry collectors. -- Use `require('crud.stats.module').op` to pick one. -- diff --git a/crud/stats/stash.lua b/crud/stats/stash.lua index bd3c182ca..d0d84febc 100644 --- a/crud/stats/stash.lua +++ b/crud/stats/stash.lua @@ -1,13 +1,17 @@ +---- Module +-- @module crud.stats.stash +-- local dev_checks = require('crud.common.dev_checks') local stash = {} ---- Get a stash instance, initialize if needed +--- Get a stash instance, initialize if needed. +-- -- Stashes are persistent to package reload and cartridge roles reload. -- -- @function get -- --- @tparam string name +-- @string name -- Stash identifier. -- -- @treturn table A stash instance.