-
Notifications
You must be signed in to change notification settings - Fork 68
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
Logging guidelines draft #512
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nicely summarized, nice 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good document.
docs/logging-guidelines.md
Outdated
``` | ||
do this: | ||
``` | ||
[2020-08-27T07:56:22Z DEBUG yagna] Initializing GNT payment driver... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[2020-08-27T07:56:22Z DEBUG yagna] Initializing GNT payment driver... | |
[2020-08-27T07:56:22Z DEBUG yagna] Initializing GNT payment driver |
|
||
**Audience:** | ||
- Users | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is also very important for node owners/admins.
log this: | ||
|
||
``` | ||
[2020-08-27T07:56:22Z ERROR yagna] E00342 - WASM ExeUnit DEPLOY: Error downloading remote file to temp folder '/tmp/yagna_data': File IO error: Path not found |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my experience of debugging yagna: the file name and line number where the error occurred is very important as it's ofter easier to just read the source code to find the problem. (I know you mentioned it in "Log entry content" section.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, albeit the error code indicated in this example serves this purpose: with a specific error code it should be fairly instantaneous to identify the location responsible for a particular issue (I'm conscious that providing file/line number may not be feasible for many dev platforms).
docs/logging-guidelines.md
Outdated
- **Grouping or correlating** attribute (attribute which allows to filter events related to a single command, activity, API request, etc.) | ||
- Thread id | ||
- Bespoke correlation id | ||
- Log entry **description** (human readable, no linebreaks) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sometimes "no linebreaks" is opposite of "human readable"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, rewritten
66fc3a6
Merged latest master state and added reference to logging guidelines from the SDP doc. |
@@ -106,6 +106,10 @@ https://doc.rust-lang.org/1.0.0/style/README.html | |||
To enforce formatting, code should be formatted using rustfmt tool (https://github.com/rust-lang/rustfmt). | |||
To install it, run `rustup component add rustfmt` command. To format files in the working dictory, please run `cargo fmt` command. | |||
|
|||
### Logging Guidelines | |||
|
|||
The logging guidelines for Yagna component development can be found here: logging-guidelines.md. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this does not render as link
👍 good job guys! Thx! |
No description provided.