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

[fix] #4030, #4063, #4064, #4079: re-architect logger and dynamic configuration #4100

Merged

Conversation

0x009922
Copy link
Contributor

@0x009922 0x009922 commented Nov 30, 2023

Description

Started by updating configuration endpoints according to #4063, this PR expanded to major update of dynamic configuration mechanism, Logger configuration (#4030, #4064) and initialisation.

Logger initialisation is a global one-time thing, which was done with iroha_logger::init(Configuration) function, which might accept the config and initialise logger, or might not, without an error. While initialising, it injected a ReloadHandle into the configuration instance, so that all updates to config.logger.max_log_level would trigger updates set up within iroha_logger::init. Than that config was copied many times in Torii and log level was actually reloaded on POST /configuration updates.

However, the configuration value update was not reflected in subsequent GET /configuration requests (#4079), because each time config is copied (it was copied for each request), it was editing a copy of config without affecting an original one. Since a copy of config was still containing ReloadHandle from Logger, the log level was updated.

Another problem appeared because of combination of configuration & logger architecture, in tests. It appeared that some tests ran concurrently, each calling iroha_logger::init with independent Configurations. However, only one iroha_logger::init might succeed, and another silently pretends that everything is ok. It worked fine, until one of Iroha instances fails to update the log level, because its Configuration passed into iroha_logger::init was not injected with a ReloadHandle.

Shortly, the system was unsound, i.e. it was easy to misuse.

Changes in this PR:

  • Remove ReloadHandle and SyncValue from configuration
  • Introduce Kiso (jp 基礎 - foundation; basis): an actor responsible for holding the configuration state and broadcasting its updates across the system in a fire-and-forget way
  • Update Logger configuration according to [suggestion] Move logger.terminal_colors setting to the CLI layer #4030 and [suggestion] Simplify logger's behaviour and configuration #4064
  • Introduce Logger actor, which might be initialised only once per process, and which is responsible for log level reloading and telemetry subscriptions
  • Update Logger initialisation: split init(Configuration) -> Result<Option<Telemetries>> into
    • init_global(Configuration) -> Result<LoggerHandle>, initialising the Logger actor once per process, and failing if it was done already
    • test_logger() -> LoggerHandle, lazily initialising the Logger actor with pre-defined configuration, suitable for tests

Chores:

  • Remove redundant fields from Kura configuration
  • Refactor telemetry broadcasting mechanisms

Linked issue

Closes #4030 #4063 #4064 #4079

Benefits

  • Improves configuration UX
  • Improves system soundness

Checklist

  • Refactor Kiso in a fire-and-forget manner
  • Play with logger manually
    • Check if bunyan can parse current JSON output
  • Update logger config in py test env
  • Update README logger documentation
  • Use 202 Accepted for POST /configuration

Migration guide

Update Logger configuration

In the configuration file, under LOGGER.* namespace:

  • Rename MAX_LOG_LEVEL to LEVEL
  • Remove COMPACT_MODE; use FORMAT: "full" (if false) and FORMAT: "compact" (if true) instead
  • Remove LOG_FILE_PATH; use FORMAT: "json" instead and redirect stdout to a file, e.g. iroha > log.json
  • Remove TERMINAL_COLORS; use --terminal-colors/--no-terminal-colors CLI arguments instead
  • Remove TELEMETRY_CAPACITY

Env variables:

  • LOG_LEVEL: instead of MAX_LOG_LEVEL
  • LOG_FORMAT: new parameter
  • LOG_TOKIO_CONSOLE_ADDR: instead of TOKIO_CONSOLE_ADDR

Example file before:

{
  "LOGGER": {
    "MAX_LOG_LEVEL": "INFO",
    "TELEMETRY_CAPACITY": 1000,
    "COMPACT_MODE": false,
    "LOG_FILE_PATH": null,
    "TERMINAL_COLORS": true
  }
}

Example file after:

{
  "LOGGER": {
    "LEVEL": "INFO",
    "FORMAT": "full"
  }
}

Update Kura configuration

In the configuration file, under KURA.* namespace, remove ACTOR_CHANNEL_CAPACITY and BLOCKS_PER_STORAGE_FILE fields.

Update configuration endpoints usage

  • GET /configuration now returns JSON with only { logger: { level } }.
  • POST /configuration now accepts JSON in the same shape

POST /configuration request before:

{"LogLevel":"DEBUG"}

POST /configuration request after:

{"logger":{"level":"DEBUG"}}

@0x009922 0x009922 added Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST api-changes Changes in the API for client libraries config-changes Changes in configuration and start up of the Iroha Refactor Improvement to overall code quality labels Nov 30, 2023
@0x009922 0x009922 self-assigned this Nov 30, 2023
@0x009922 0x009922 changed the title [fix] #4030 #4063 #4064: re-architect logger and dynamic configuration [fix] #4030 #4063 #4064 #4079: re-architect logger and dynamic configuration Nov 30, 2023
@0x009922 0x009922 changed the title [fix] #4030 #4063 #4064 #4079: re-architect logger and dynamic configuration [fix] #4030, #4063, #4064, #4079: re-architect logger and dynamic configuration Nov 30, 2023
@Erigara Erigara self-assigned this Nov 30, 2023
.gitignore Outdated Show resolved Hide resolved
cli/src/main.rs Show resolved Hide resolved
core/src/kiso.rs Show resolved Hide resolved
scripts/test_env.py Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
cli/src/main.rs Show resolved Hide resolved
cli/src/main.rs Outdated Show resolved Hide resolved
cli/src/lib.rs Outdated Show resolved Hide resolved
logger/src/lib.rs Outdated Show resolved Hide resolved
logger/src/lib.rs Show resolved Hide resolved
logger/src/telemetry.rs Outdated Show resolved Hide resolved
core/src/kura.rs Outdated Show resolved Hide resolved
core/src/kiso.rs Outdated Show resolved Hide resolved
futures/tests/basic.rs Outdated Show resolved Hide resolved
core/src/kiso.rs Outdated Show resolved Hide resolved
logger/tests/setting_logger.rs Outdated Show resolved Hide resolved
scripts/test_env.py Outdated Show resolved Hide resolved
core/src/kiso.rs Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Dec 6, 2023

Pull Request Test Coverage Report for Build 7175878272

  • 273 of 483 (56.52%) changed or added relevant lines in 21 files are covered.
  • 6921 unchanged lines in 128 files lost coverage.
  • Overall coverage decreased (-3.1%) to 56.349%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cli/src/torii/mod.rs 0 2 0.0%
logger/src/telemetry.rs 16 18 88.89%
telemetry/src/futures.rs 0 2 0.0%
cli/src/style.rs 0 3 0.0%
core/test_network/src/lib.rs 0 4 0.0%
logger/src/lib.rs 43 47 91.49%
telemetry/src/dev.rs 0 5 0.0%
client/src/client.rs 0 6 0.0%
config/src/client_api.rs 23 31 74.19%
core/src/kiso.rs 87 102 85.29%
Files with Coverage Reduction New Missed Lines %
cli/src/style.rs 1 0.0%
config/base/derive/src/view.rs 1 99.37%
config/src/block_sync.rs 1 95.0%
config/src/network.rs 1 93.75%
config/src/torii.rs 1 95.45%
config/src/wasm.rs 1 87.5%
core/src/smartcontracts/isi/block.rs 1 87.5%
logger/src/lib.rs 1 93.98%
ffi/src/option.rs 2 71.43%
config/src/genesis.rs 3 72.92%
Totals Coverage Status
Change from base Build 5423219773: -3.1%
Covered Lines: 23057
Relevant Lines: 40918

💛 - Coveralls

@0x009922 0x009922 requested review from Erigara and DCNick3 December 6, 2023 07:06
@0x009922 0x009922 marked this pull request as ready for review December 6, 2023 07:06
@mversic mversic force-pushed the logger-and-config-actors branch from 4ac7515 to 23bd7e6 Compare December 11, 2023 09:43
@0x009922 0x009922 requested review from DCNick3 and Erigara December 12, 2023 01:57
@0x009922 0x009922 merged commit 23e7214 into hyperledger-iroha:iroha2-dev Dec 12, 2023
10 checks passed
@0x009922 0x009922 deleted the logger-and-config-actors branch December 12, 2023 07:49
Asem-Abdelhady pushed a commit to Asem-Abdelhady/iroha that referenced this pull request Jan 9, 2024
…ha#4064, hyperledger-iroha#4079: re-architect logger and dynamic configuration (hyperledger-iroha#4100)

Co-authored-by: ⭐️NINIKA⭐️ <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Asem-Abdelhady pushed a commit to Asem-Abdelhady/iroha that referenced this pull request Jan 22, 2024
…ha#4064, hyperledger-iroha#4079: re-architect logger and dynamic configuration (hyperledger-iroha#4100)

Co-authored-by: ⭐️NINIKA⭐️ <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Asem-Abdelhady <[email protected]>
Asem-Abdelhady pushed a commit to Asem-Abdelhady/iroha that referenced this pull request Feb 9, 2024
…ha#4064, hyperledger-iroha#4079: re-architect logger and dynamic configuration (hyperledger-iroha#4100)

Co-authored-by: ⭐️NINIKA⭐️ <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-changes Changes in the API for client libraries config-changes Changes in configuration and start up of the Iroha Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST Refactor Improvement to overall code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants