Log Levels and Method Filtering

[mckamyk]

This PR allows the user to mute method-only calls (like eth_chainId, eth_getFilterChanges), while maintaining logs for newly mined blocks, as well as readonly calls to contracts, and any corresponding console.log(...) calls from inside contracts.

to use, implement the following code.

extendEnvironment((hre) => {
  hre.network.provider.request({
    method: 'hardhat_setLogMethods',
    params: [false], // or true to re-enable, which is default, ofc
  });
});

This PR adds a request method hardhat_setLogMethods with params [boolean] that toggles the function of printMethod inside the ModulesLogger class.

It also comes with connective tissue from HardhatNetworkProvider -> HardhatModule -> ModuleLogger.

[alcuadrado]

This really interesting. Thanks for submitting it.

I have two questions for you:

1. Wouldn't it be better to add a config values instead of am rpc method for this?
2. How about creating a list of methods that won't be displayed instead of just a boolean?

[mckamyk]

1. Wouldn't it be better to add a config values instead of am rpc method for this?

Probably. I was following the methods for how hardhat_setLoggingEnabled was implemented, and while this was a quick low-effort fix to solve annoying log messages on my part, I can work on extending a config option, I'll just need to investigate more on how y'all implement config variables.

2. How about creating a list of methods that won't be displayed instead of just a boolean?

Definitely, i was thinking assigning logging levels to each rpc method, then assigning the logging level in config, but i think this solution is more elegant.

The answer to both questions is yes, those are great ideas. I wanted to gauge y'all's interest on this to see if it was something y'all found valuable.

I can work on implementing both of these next week, i might have questions for some developers to make sure I go the right direction, @fvictorio has been great on discord with that.

[fvictorio]

I agree with making it part of the config (and changeable with an RPC method). I've been thinking a bit about this, and I would start with a simple string option like this:

hardhat: {
  logging: "disabled" | "default" | "minimal" | "json"
}

("disabled" could be false instead, or we can accept both, I don't know).

default is what we have now, minimal could be an opinionated list of disabled methods. I wouldn't implement json now, but it's something we've been asked to support, so I'm adding it as an example of another possible option.

Then later, in a different PR, we can add support for a more advanced config, for example:

logging: {
  methods: ["eth_sendTransaction", ...],
  expandTransactions: false,
  console: true
}

I just made this up, we should actually think more carefully about how that "advanced logging config" would look like.

[mckamyk]

I think we can agree default and disabled are easy wins, they already exist. I'll implement those for sure. minimal seems like it's close to what it's already implemented in this PR, unless you had different behavior for that in mind. It's simply skipping the printMethod() function in the ModulesLogger.

I personally like the idea of supplying a list of methods to omit, rather than list of methods to include. Knowing me, i would forget to include a method and fight hardhat for days wondering why is not serving a particular method. An omitMethods option in the logging config seems like the most efficient solution for now, (in addition to default, disabled, minimal) especially with my limited knowledge of how all this is passed.

So i purpose:

hardhat: {
  logging: {
    mode: "disabled" | "default" | "minimal" | "json",
    omitMethods: ['eth_getFilterChanges']
  }
}

Let me know what y'all want and I'll get started on Monday.

[alcuadrado]

I like how this is looking so far.

A few points:

• I'm not sure if we need a "disabled" mode, as this will coexist with loggingEnabled.
• What would minimal and json do?

I guess by json you mean to log every JSON-RPC request/reposnse, don't you, @fvictorio ? Those only exist when using the node task, so IMO it should belong to it.

Maybe we can have something like:

logging: {
   level: "default" | "minimal" | "verbose"
   omitMethods: string[]
}

• "default": what we do today
• "minimal": Only print the method names
• "verbose": print params and response of ever methods
• omitMethods should work independently of the level

[fvictorio]

Hey @mckamyk, are you still interested in working on this?

[mckamyk]

Yes, apologies, Miami conference and interviews got me a little busy. I've been able to get the method call into a config option.

Still working.

[mckamyk]

Update, I've just gotten past a really long continuous sprint and should have more free time to keep going. I was able to get a config option for disabling method calls, working on testing it and implementing the other logging modes.

[mckamyk]

I've tied the config file into the ModuleLogger. Its loading the object @alcuadrado specified in his last post. OmitMethods currently works as expected, as it was the easier one to achieve (along with default of course). I'm assume with the minimal you want to drop logs that are 'details' of a transaction, such as contract deployment, calls and transactions, value, gas, etc. to only showing the method name, which I should be able to do without much help. Would this include blocking of console.log methods in contracts?

[mckamyk]

minimal option has been implemented with a simple skip of printing. Any call to the this._log(...) function in ModulesLogger will check if minimal is set. If so, the log message will be discarded. The only things that skip this._log(...) are methods and console logs. I changed the behavior of calling this._replaceLastLine(...) a bit, as it was deleting the most recent console log most of the time. Now it will write to a new line if the previous message was a console.log from solidity. This behavior only applies while in minimal level.

I also fixed the config resolution to load defaults properly.

@fvictorio @alcuadrado Next would be the verbose mode. Would this make all methods print the indented text similar to contract creation/state changes? My concern here is that it looks like the current way the logger is logging methods (without any details) is via the public printMethod(method: string) function. I may be wrong, but consumers of the ModulesLogger are not passing in tx details for all methods, so the modules logger doesn't have any information for these transactions.

Do you see it as within the scope of this PR to adjust the consumers to pass in tx data for all methods?

[fvictorio]

Hey @mckamyk, I just wanted to say that we haven't forgotten about this PR. The problem is that we'll need some time to think about this from a product perspective (there are a lot of little details that we need to consider), and we have our hands full right now. I hope we can do it soon, but I can't promise anything :sweat_smile:

[mckamyk]

You're fine to let it sit. This is very non-important. I've compiled my version and am using it in my projects.

Feel free to throw me an invite to a meeting if you want to discuss more.

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: 2a5201357c6ff0fa0ae1772aab0d372dd83b863c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[drortirosh]

Re: configuration Instead of methods list, why not use a regex, E.g. includeMethodsRegex: "send|log" Or: excludeMethodRegex: "get|block"