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

log: introduce per-module log_level #7672

Merged
merged 4 commits into from
Dec 9, 2022

Conversation

Gumix
Copy link
Contributor

@Gumix Gumix commented Sep 12, 2022

log: add log.new() function that creates a new logger
It allows to create a new instance of a log module, with a custom name, e.g.:

local my_log = require('log').new('my_module')

The name is added to the log message after fiber name:

YYYY-MM-DD hh:mm:ss.ms [PID]: CORD/FID/FIBERNAME/MODULENAME LEVEL> MSG

log: add per-module log level setting via log.cfg() or box.cfg()
Now it is possible to specify the log level for each module separately, e.g.:

box.cfg {
    log_level = 5,
    log_modules = {
        ['foo.bar'] = 1,
        expirationd = 'debug'
    }
}

The name of a module is determined automatically, or it is possible to create a logger with a custom name by using log.new().

log: implement automatic module name deduction
The name of a module is determined automatically during the execution of require('log') in the module's source code. The name is derived from its filename, including a part of the path. This is implemented by overriding the built-in require function.

Closes #3211

@Gumix Gumix force-pushed the iverbin/gh-3211-per-module-logging branch 2 times, most recently from c95d748 to 1d2d19b Compare September 13, 2022 14:00
@kyukhin kyukhin marked this pull request as draft September 13, 2022 14:00
@Gumix Gumix force-pushed the iverbin/gh-3211-per-module-logging branch from 1d2d19b to fc96a76 Compare September 13, 2022 14:46
@Gumix Gumix marked this pull request as ready for review September 13, 2022 14:54
@Gumix Gumix requested a review from locker September 13, 2022 14:54
Copy link
Member

@locker locker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it difficult to review this PR because of how it's split. I think it should be re-split as follows:

  1. Add log.new() function that creates a new log module. The function should take the module name and the log level.
  2. Add a utility module/function that can be usd to extract the module name from the file name or stack (the code that's currently located in src/lua/init.lua).
  3. Implement automatic module name deduction in log.new() (something like log.new({module = 'auto'})).
  4. Add new global log options.
    • Allow the user to enable automatic module name deduction via log.cfg().
    • Allow the user to set per-module log level via log.cfg() by passing a map{name->level} for opts.level.
  5. Implement the box API mapping for the new log options.

Each part should be covered with tests.

We also need to agree on the names (the current API looks a bit awkward to me).

@locker locker assigned Gumix and unassigned locker Sep 14, 2022
@Gumix Gumix marked this pull request as draft September 15, 2022 15:01
@Gumix Gumix force-pushed the iverbin/gh-3211-per-module-logging branch from fc96a76 to 36b8c48 Compare November 7, 2022 19:54
@coveralls
Copy link

coveralls commented Nov 7, 2022

Coverage Status

Coverage increased (+0.02%) to 84.829% when pulling 9ba8167 on Gumix:iverbin/gh-3211-per-module-logging into 05002ed
on tarantool:master
.

@Gumix Gumix force-pushed the iverbin/gh-3211-per-module-logging branch 2 times, most recently from 3381a77 to e5f2255 Compare November 7, 2022 23:21
@Gumix Gumix marked this pull request as ready for review November 7, 2022 23:31
@Gumix Gumix requested a review from nshy November 7, 2022 23:31
@Gumix Gumix assigned nshy and unassigned Gumix Nov 7, 2022
src/lua/log.lua Show resolved Hide resolved
src/lua/log.lua Outdated Show resolved Hide resolved
src/lua/log.lua Outdated Show resolved Hide resolved
test/box-luatest/gh_3211_per_module_log_level_test.lua Outdated Show resolved Hide resolved
test/box-luatest/gh_3211_per_module_log_level_test.lua Outdated Show resolved Hide resolved
test/box-luatest/gh_3211_per_module_log_level_test.lua Outdated Show resolved Hide resolved
src/lua/log.lua Outdated Show resolved Hide resolved
src/lua/log.lua Outdated Show resolved Hide resolved
src/lua/log.lua Outdated Show resolved Hide resolved
changelogs/unreleased/gh-3211-per-module-log_level.md Outdated Show resolved Hide resolved
@nshy nshy assigned Gumix and unassigned nshy Nov 8, 2022
@Gumix Gumix force-pushed the iverbin/gh-3211-per-module-logging branch 2 times, most recently from f6282fa to ab4bb03 Compare November 15, 2022 18:55
@Gumix Gumix requested a review from nshy November 15, 2022 19:12
@Gumix Gumix assigned nshy and unassigned Gumix Nov 15, 2022
src/lib/core/say.c Outdated Show resolved Hide resolved
src/lib/core/say.h Outdated Show resolved Hide resolved
src/lua/log.lua Outdated Show resolved Hide resolved
src/lua/log.lua Outdated Show resolved Hide resolved
src/box/lua/load_cfg.lua Outdated Show resolved Hide resolved
src/lua/log.lua Outdated Show resolved Hide resolved
src/lua/log.lua Show resolved Hide resolved
src/lua/log.lua Show resolved Hide resolved
changelogs/unreleased/gh-3211-per-module-log_level.md Outdated Show resolved Hide resolved
@nshy nshy removed their assignment Nov 18, 2022
@locker locker assigned Gumix and unassigned locker Dec 2, 2022
@Gumix Gumix force-pushed the iverbin/gh-3211-per-module-logging branch 2 times, most recently from 45558d3 to d18da53 Compare December 2, 2022 14:03
@Gumix Gumix assigned locker and unassigned Gumix Dec 2, 2022
@locker locker added the full-ci Enables all tests for a pull request label Dec 2, 2022
@Gumix Gumix added do not merge Not ready to be merged and removed full-ci Enables all tests for a pull request labels Dec 2, 2022
@Gumix Gumix assigned Gumix and unassigned locker Dec 2, 2022
@Gumix Gumix added full-ci Enables all tests for a pull request and removed do not merge Not ready to be merged labels Dec 2, 2022
@Gumix Gumix assigned locker and unassigned Gumix Dec 2, 2022
@Gumix
Copy link
Contributor Author

Gumix commented Dec 2, 2022

Cartridge tests use some dirty magic, which doesn't work with require overloading:
https://github.com/tarantool/cartridge/blob/f241bf46f07b6bf2d5df38fba705720545a8218e/test/unit/upload_test.lua#L16-L18

Filed an issue: tarantool/cartridge#1998

@Gumix Gumix assigned Gumix and unassigned locker Dec 2, 2022
@Gumix Gumix added full-ci Enables all tests for a pull request and removed full-ci Enables all tests for a pull request labels Dec 5, 2022
Gumix and others added 4 commits December 9, 2022 16:43
It allows to create a new instance of a log module, with a custom name:
local my_log = require('log').new('my_module')

The name is added to the log message after fiber name:
YYYY-MM-DD hh:mm:ss.ms [PID]: CORD/FID/FIBERNAME/MODULENAME LEVEL> MSG

Part of tarantool#3211

NO_DOC=See next commit
NO_CHANGELOG=See next commit

Co-authored-by: AnastasMIPT <beliaev.ab@tarantool.org>
Now it is possible to specify the log level for each module separately,
e.g.:

box.cfg {
    log_level = 5,
    log_modules = {
        ['foo.bar'] = 1,
        expirationd = 'debug'
    }
}

Part of tarantool#3211

NO_DOC=See next commit
NO_CHANGELOG=See next commit

Co-authored-by: AnastasMIPT <beliaev.ab@tarantool.org>
Now the name of a module, from which the logging function was called,
is determined automatically during the execution of `require('log')`
in the module's source code. This is implemented by overriding the
built-in `require` function.

Part of tarantool#3211

NO_DOC=See next commit
NO_CHANGELOG=See next commit

Co-authored-by: AnastasMIPT <beliaev.ab@tarantool.org>
Closes tarantool#3211

NO_TEST=Documentation

@TarantoolBot document
Title: Per-module log level
Root document: https://www.tarantool.io/en/doc/latest/reference/configuration/#logging

Since version 2.11 it is possible to specify the log level for each
module separately, e.g.:

box.cfg {
    log_level = 5,
    log_modules = {
        ['foo.bar'] = 1,
        expirationd = 'debug'
    }
}

The name of a module is determined automatically during the execution
of `require('log')` in the module's source code. The name is derived
from its filename, including a part of the path. Also it is possible
to create a logger with a custom name by using `log.new()`.

---

Root document: https://www.tarantool.io/en/doc/latest/reference/reference_lua/log/
New function: log.new(name)
Creates a new logger with a custom name.

Parameter `name`:
Type: string
Optional: false

Example:
box.cfg{log_level='error', log_modules={my_module='info'}}
log = require('log')
my_log = log.new('my_module')
my_log.info('Info')
@Gumix Gumix force-pushed the iverbin/gh-3211-per-module-logging branch from d18da53 to 9ba8167 Compare December 9, 2022 13:44
@Gumix Gumix added the full-ci Enables all tests for a pull request label Dec 9, 2022
@Gumix Gumix assigned locker and unassigned Gumix Dec 9, 2022
@locker locker merged commit 1622097 into tarantool:master Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full-ci Enables all tests for a pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Per-module logging
4 participants