DAOS-18292 ddb: Debug level of the DDB GO code can not be defined#17232
DAOS-18292 ddb: Debug level of the DDB GO code can not be defined#17232
Conversation
|
Ticket title is 'Debug level of the DDB GO code can not be defined' |
|
Test stage Functional Hardware Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17232/1/execution/node/1276/log |
3df67cc to
85e6a34
Compare
|
Test stage Functional on EL 8.8 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17232/2/execution/node/1073/log |
85e6a34 to
caf8936
Compare
9891709 to
9cbf6fe
Compare
caf8936 to
72a9f0c
Compare
|
Test stage Functional on EL 8.8 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos/job/PR-17232/4/display/redirect |
|
Test stage NLT on EL 8.8 completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-17232/5/testReport/ |
|
Test stage Functional Hardware Medium Verbs Provider MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17232/5/execution/node/1358/log |
|
Test stage Functional Hardware Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17232/5/execution/node/1399/log |
795df75 to
0187ab0
Compare
0187ab0 to
0f34795
Compare
|
Test stage NLT on EL 8.8 completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-17232/8/testReport/ |
|
Test stage Functional Hardware Medium Verbs Provider MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17232/8/execution/node/1398/log |
Properly configure C DAOS debug facilities from Golang DDB code. Features: recovery Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
0f34795 to
981d4b8
Compare
|
Test stage Functional Hardware Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17232/13/execution/node/1189/log |
|
Test stage Functional Hardware Medium MD on SSD completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-17232/12/testReport/ |
|
Test stage Functional Hardware Medium Verbs Provider MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17232/12/execution/node/1392/log |
Integrate reviewers comments: - Fix UX interface Features: recovery Allow-unstable-test: true Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
|
Test stage Functional Hardware Medium Verbs Provider MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17232/14/execution/node/1390/log |
Integrate reviewers comments: - Update documentation. Features: recovery Allow-unstable-test: true Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
|
To properly integrate the documentation on logging facilities, I have added a ddb man file as it is already done for the |
kjacque
left a comment
There was a problem hiding this comment.
Nice job adding documentation. I think this will be helpful to users. Thanks.
Remaining comments aren't blocking and could be addressed in a follow-on.
src/control/cmd/ddb/main.go
Outdated
| SysdbPath string `long:"db_path" short:"p" description:"Path to the sys db."` | ||
| VosPath string `long:"vos_path" short:"s" description:"Path to the VOS file to open."` | ||
| Version bool `short:"v" long:"version" description:"Show version"` | ||
| Debug bool `long:"debug" description:"Without this option, the console log level is set to ERROR. With this option, the console log level is set to INFO. For more detailed logs, provide the --log_dir path to log to a file."` |
There was a problem hiding this comment.
From user's perspective, he/she may not understand what is ERROR and what is INFO. So this option is mainly for DAOS developer. If that is true, then more convenient and flexible solution maybe something like that:
ddb --debug=mmm --logdir=nnn ...
mmm is different debug level, such as ERROR, WARN, INFO or DEBUG, and so on. nnn is the directory for log messages. These two options are independent from each other. Means that even if the debug level is set as ERROR, we should also allow the user to redirect related error message to the specified log directory.
Just suggestion, not sure how difficult to be implemented, depends on you.
Another question: how will these two options be used under interactive mode? They are per sub-command based or the whole mini-shell will share the same configuration when start the interaction?
There was a problem hiding this comment.
From user's perspective, he/she may not understand what is
ERRORand what isINFO. So this option is mainly for DAOS developer. If that is true, then more convenient and flexible solution maybe something like that:ddb --debug=mmm --logdir=nnn ...mmmis different debug level, such asERROR,WARN,INFOorDEBUG, and so on.nnnis the directory for log messages. These two options are independent from each other. Means that even if the debug level is set asERROR, we should also allow the user to redirect related error message to the specified log directory.Just suggestion, not sure how difficult to be implemented, depends on you.
At this time, it is not really feasible as the INFO logging facilities is also used to print the result of ddb commands such as the version or help information. They maybe other commands using the logging facilities for such usage. The issue is fixed in a follow-up PR needed for properly testing the ddb go code. As soon as, this issue is fixed it should be not so difficult (at first glance) to implement to change the behaviour as you asked. However, I will change a little bit: ERROR messages and above should always printed on stderr: If lthe -log-dir= is defined then the messages will be printed on stderr and in the log file.
If this last proposal makes sense to you, I can back port the part of the PR fixing the invalid logging usage to this PR. Then, I will fix this one as you suggested. Doest it makes sense to you ?
Another question: how will these two options be used under interactive mode? They are per sub-command based or the whole mini-shell will share the same configuration when start the interaction?
Under interactive mode, the whole mini-shell and sub-commands share the same configuration.
There was a problem hiding this comment.
- Use consistent logging façilities
There was a problem hiding this comment.
- Use consistent logging façilities
Fixed with commit 021ee3b
Fix building issue: add missing LD_LIBRARY path. Features: recovery Allow-unstable-test: true Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
Fix reviewers remarks: - Add short description of VOS path in live help output. Features: recovery Allow-unstable-test: true Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
Fix reviewers remarks: - Use consistent logging facilities Features: recovery Allow-unstable-test: true Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
|
Test stage Functional Hardware Large MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos/job/PR-17232/17/display/redirect |
|
Test stage Functional Hardware Medium Verbs Provider MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17232/18/execution/node/1347/log |
kjacque
left a comment
There was a problem hiding this comment.
Comments are minor, not something I would block on. Nice improvements! I like the change to pass the log level--this really makes more sense from the perspective of a developer and/or a user collecting logs to troubleshoot.
| } | ||
| } | ||
|
|
||
| func strToLogLevels(level string) (logging.LogLevel, engine.LogLevel, error) { |
There was a problem hiding this comment.
This would be a good function to write a simple Go unit test for.
| SysdbPath string `long:"db_path" short:"p" description:"Path to the sys db."` | ||
| VosPath string `long:"vos_path" short:"s" description:"Path to the VOS file to open."` | ||
| Version bool `short:"v" long:"version" description:"Show version"` | ||
| Debug string `long:"debug" description:"Logging log level (default to ERROR)"` |
There was a problem hiding this comment.
Would be good to provide the list of options for log level here directly (or at least direct the user to the manpage).
Description
Properly configure C DAOS debug facilities from Golang DDB code.
Steps for the author:
After all prior steps are complete: