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

Add debuglevel JSON-RPC #1184

Closed
jrick opened this issue Jun 19, 2018 · 8 comments · Fixed by #2449
Closed

Add debuglevel JSON-RPC #1184

jrick opened this issue Jun 19, 2018 · 8 comments · Fixed by #2449

Comments

@jrick
Copy link
Member

jrick commented Jun 19, 2018

The debuglevel method allows the debug level of all loggers, or only a particular subsystem's logger, to be modified to a specified level. It is currently unimplemented by dcrwallet, so dcrwallet instead tries to perform RPC passthrough to dcrd.

The implementation can't be simply a copy of dcrd's due to dcrwallet's more modular structure. In dcrd, the JSON-RPC server is implemented as part of the main package, while in dcrwallet it is implemented in the legacyrpc package. Therefore, in order to provide access to the logger variables (in the main package), a mechanism similar to the RequestProcessShutdown server method should be used to indicate the request to the main package. In this case, I feel it makes more sense to invoke a callback set by main rather than using a channel.

@ggoranov
Copy link

I'm busy with this issue.

@ggoranov
Copy link

The proposal with the RequestProcessShutdown mechanism works pretty well. I'm also considering to split parseAndSetDebugLevels into parseDebugLevels and setDebugLevels so that parseDebugLevels can be reused on returning meaningful error when sending invalid pair subsystem_id - debuglevel.

E.g

dcrctl --wallet debuglevel "DCRW1=debug"
The specified subsystem [DCRW1] is invalid -- supported subsytems [CMGR DCRW GRPC LODR RPCS SYNC TKBY WLLT]

@jrick
Copy link
Member Author

jrick commented Oct 11, 2018

sounds reasonable

@ggoranov
Copy link

As parseAndSetDebugLevels and subsystemLoggers are part of main package it would have needed too many code changes to make it possible for dcrctl to return full error description.
With the last commit, we show error in Stderr.

@itswisdomagain
Copy link
Member

Is this done? I'd like to take it up.

@jrick
Copy link
Member Author

jrick commented Apr 15, 2020

there is very likely little to be gained by being able to configure the verbosity levels anymore. All important messages are (or should be) always logged, and the Debug and Trace levels are very infrequently used, and where they are, only in very old code. We have also begun using runtime/trace for trace messages, which provides context-scoped logs. These have proved to be far more valuable than subsystem-level control, as they are easier to search through and grok compared to the unreasonable verbosity that you'd otherwise get with our standard logger.

We can probably skip supporting this but I'm open to feedback on that decision.

@jrick
Copy link
Member Author

jrick commented Apr 28, 2020

Closing per previous comment.

@jrick jrick closed this as completed Apr 28, 2020
@jrick
Copy link
Member Author

jrick commented Dec 16, 2024

Reopening with an implementation here #2449.

There's lots of useful information currently hidden behind MIXP and MIXC debug levels, and it restarting to turn these on or off is less than ideal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants