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

[refactor] #4323: telemetry features, tokio console, logs, and configuration #4377

Merged
merged 19 commits into from Mar 26, 2024

Conversation

0x009922
Copy link
Contributor

@0x009922 0x009922 commented Mar 21, 2024

Description

This is an improved iteration of #4369

  • Remove logger.tokio_console_address configuration parameter entirely: this could be configured in a more refined way using default ENVs (see console_subscriber docs)
  • Rename [telemetry.dev] config section to [dev_telemetry]
  • Update telemetry and dev-telemetry features of iroha
  • Update tokio-console feature of iroha_logger: now console seamlessly starts whenever this feature is enabled
  • Improve logs & warnings
  • Fix dev-telemetry file logs: separate each item with \n, and recursively create parent directories
  • Improve config validation: handle file paths

Linked issue

Closes #4323

Benefits

  • Less config parameters
  • More flexible way to configure tokio console
  • Less feature mess (?)
  • Better warnings
  • Refactoring

Checklist

  • Document how to use tokio console
  • Figure out what happened to Cargo.tomls
  • Check manually several cases, how Iroha behaves with telemetry/dev-telemetry features enabled depending on telemetry configuration (warnings, logs, file output)
  • Check manually Tokio Console (works, but requires TRACE log level)

@0x009922 0x009922 added Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST config-changes Changes in configuration and start up of the Iroha Refactor Improvement to overall code quality labels Mar 21, 2024
@0x009922 0x009922 self-assigned this Mar 21, 2024
Copy link

@BAStos525

Cargo.toml Outdated Show resolved Hide resolved
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
- remove `tokio_console_address` configuration
- leverage default ENVs for tokio console config
- `iroha_logger/tokio-console` feature fully controls
  whether it is enabled
- refactoring chores

Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
@github-actions github-actions bot added the api-changes Changes in the API for client libraries label Mar 22, 2024
@0x009922 0x009922 changed the title [refactor] #4323: re-arrange telemetry & tokio console config [refactor] #4323: telemetry features, tokio console, logs, and configuration Mar 22, 2024
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
@0x009922 0x009922 marked this pull request as ready for review March 24, 2024 23:50
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
cli/src/lib.rs Outdated Show resolved Hide resolved
@Erigara
Copy link
Contributor

Erigara commented Mar 25, 2024

Is telemetry used at all? I'm aware that QA and DevOps team using prometheus metrics.
But i'm not aware of using WS for telemetry, if it's not required we can make code simpler.

@0x009922
Copy link
Contributor Author

@Erigara, created a separate discussion for it: #4385

Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 8428396018

Details

  • 133 of 194 (68.56%) changed or added relevant lines in 7 files are covered.
  • 4990 unchanged lines in 91 files lost coverage.
  • Overall coverage increased (+0.8%) to 57.623%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cli/src/main.rs 0 2 0.0%
torii/src/lib.rs 0 2 0.0%
logger/src/lib.rs 15 18 83.33%
config/src/parameters/user.rs 33 40 82.5%
cli/src/lib.rs 80 92 86.96%
telemetry/src/dev.rs 0 35 0.0%
Files with Coverage Reduction New Missed Lines %
primitives/src/conststr.rs 1 91.14%
ffi/derive/src/convert.rs 1 84.45%
primitives/src/lib.rs 1 0.0%
core/src/sumeragi/network_topology.rs 1 98.78%
telemetry/src/dev.rs 2 0.0%
crypto/src/signature/bls/mod.rs 2 0.0%
config/src/snapshot.rs 3 78.57%
data_model/derive/src/has_origin.rs 3 95.16%
config/src/kura.rs 3 80.0%
logger/src/lib.rs 3 92.5%
Totals Coverage Status
Change from base Build 7884695009: 0.8%
Covered Lines: 23107
Relevant Lines: 40100

💛 - Coveralls

@0x009922 0x009922 merged commit 0c49284 into hyperledger:iroha2-dev Mar 26, 2024
12 checks passed
@0x009922 0x009922 deleted the config-tokio-console branch March 26, 2024 05:43
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.

None yet

4 participants