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

Improved log format in blockchain_explorer/helpers.rs #86

Merged
merged 5 commits into from
May 12, 2017
Merged

Improved log format in blockchain_explorer/helpers.rs #86

merged 5 commits into from
May 12, 2017

Conversation

dj8yfo
Copy link
Contributor

@dj8yfo dj8yfo commented May 3, 2017

No description provided.

@dj8yfo
Copy link
Contributor Author

dj8yfo commented May 3, 2017

If EXONUM_SRC_PATH env variable is false or not set:

[1493816662 : 482] - [ TRACE ] - exonum::events::network - Try to reconnect with delay 1000
[1493816662 : 482] - [ TRACE ] - exonum::events::network - 127.0.0.1:5400: Establish connection with 127.0.0.1:5402, id: 130
[1493816662 : 482] - [ TRACE ] - exonum::events::network - 127.0.0.1:5400: Add reconnect timeout to=127.0.0.1:5402, delay=2000
[1493816662 : 482] - [ TRACE ] - exonum::events::network - Try to reconnect with delay 1000
[1493816662 : 482] - [ TRACE ] - exonum::events::network - 127.0.0.1:5400: Establish connection with 127.0.0.1:5403, id: 131
[1493816662 : 482] - [ TRACE ] - exonum::events::network - 127.0.0.1:5400: Add reconnect timeout to=127.0.0.1:5403, delay=2000
[1493816662 : 482] - [ TRACE ] - exonum::events::network - 127.0.0.1:5400: outgoing connection with addr 127.0.0.1:5401 closed

else:

[1493816722 : 726] - [ TRACE ] - exonum::events::network:/Users/sysmanj/Exonum/exonum-core/exonum/src/events/network.rs:266 - Try to reconnect with delay 1000
[1493816722 : 726] - [ TRACE ] - exonum::events::network:/Users/sysmanj/Exonum/exonum-core/exonum/src/events/network.rs:248 - 127.0.0.1:5400: Establish connec
tion with 127.0.0.1:5403, id: 130
[1493816722 : 726] - [ TRACE ] - exonum::events::network:/Users/sysmanj/Exonum/exonum-core/exonum/src/events/network.rs:400 - 127.0.0.1:5400: Add reconnect ti
meout to=127.0.0.1:5403, delay=2000
[1493816722 : 727] - [ TRACE ] - exonum::events::network:/Users/sysmanj/Exonum/exonum-core/exonum/src/events/network.rs:170 - 127.0.0.1:5400: outgoing connect
ion with addr 127.0.0.1:5401 closed
[1493816722 : 727] - [ TRACE ] - exonum::events::network:/Users/sysmanj/Exonum/exonum-core/exonum/src/events/network.rs:170 - 127.0.0.1:5400: outgoing connect
ion with addr 127.0.0.1:5402 closed

Copy link
Contributor

@alekseysidorov alekseysidorov left a comment

Choose a reason for hiding this comment

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

EXONUM_SRC_PATH is not intuitive, i think the better name is EXONUM_LOG_VERBOSE.

@dj8yfo
Copy link
Contributor Author

dj8yfo commented May 3, 2017

@alekseysidorov maybe not EXONUM_LOG_VERBOSE, but LOG_VERBOSE_SRC_PATH?
because technically the modules aren't restricted to exonum ones only.

@dj8yfo
Copy link
Contributor Author

dj8yfo commented May 3, 2017

LOG_VERBOSE_SRC_FILE_LINE

@alekseysidorov
Copy link
Contributor

In my opinion variables with SRC are associated with the directory with source codes.

@dj8yfo
Copy link
Contributor Author

dj8yfo commented May 3, 2017

@alekseysidorov yep. And those are referenced in each log, if you set the variable to true.

stanislav-tkach
stanislav-tkach previously approved these changes May 3, 2017
.unwrap()
.parse::<bool>();
if let Ok(flag) = param_parse {
verbose_src_path = flag;
Copy link
Contributor

Choose a reason for hiding this comment

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

let verbose_src_path = match env::var("LOG_VERBOSE_SRC_FILE_LINE") {
    Ok(val) => val.parse::<bool>().unwrap_or(false),
    Err(_) => false,
};

Probably would be better.

let now = (ts.as_secs() * 1000 + ts.subsec_nanos() as u64 / 1000000).to_string();
let secs = ts.as_secs().to_string();
let millis = (ts.subsec_nanos() as u64 / 1000000).to_string();

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps, we will need some "time utils" for such things...

Copy link
Contributor

Choose a reason for hiding this comment

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

May be just use chrono crate and format time with rfc2822?

stanislav-tkach
stanislav-tkach previously approved these changes May 4, 2017
@dj8yfo dj8yfo requested a review from vldm May 4, 2017 12:29
vldm
vldm previously approved these changes May 4, 2017
Copy link
Contributor

@vldm vldm left a comment

Choose a reason for hiding this comment

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

LGTM, but maybe move format closure into static function? And I better prefer RUST_VERBOSE_PATH name.

alekseysidorov
alekseysidorov previously approved these changes May 4, 2017
Copy link
Contributor

@alekseysidorov alekseysidorov left a comment

Choose a reason for hiding this comment

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

Oleg explained me some details, I was not right.
But can we use the slog crate instead of this helper in our applications?

@dj8yfo dj8yfo dismissed stale reviews from alekseysidorov, vldm, and stanislav-tkach via ccddf5c May 4, 2017 16:23
Copy link
Contributor

@vldm vldm left a comment

Choose a reason for hiding this comment

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

LGTM

@defuz defuz merged commit a1f3a91 into exonum:master May 12, 2017
vldm pushed a commit that referenced this pull request Jul 13, 2017
Improved log format in blockchain_explorer/helpers.rs

Former-commit-id: f39abde61d31c3375b61df67cf13daceba4fe75a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants