feat: optional console log message filter

[rvanasa]

Resolves #201 by adding an optional consoleFilter install arg.

Show all log messages (default):

dfx deploy evm_rpc --argument "(record { consoleFilter = opt variant { ShowAll } })"

Hide all log messages:

dfx deploy evm_rpc --argument "(record { consoleFilter = opt variant { HideAll } })"

Only show INFO ... messages:

dfx deploy evm_rpc --argument "(record { consoleFilter = opt variant { ShowPattern = "^INFO" } })"

Hide TRACE_HTTP ... messages:

dfx deploy evm_rpc --argument "(record { consoleFilter = opt variant { HidePattern = "^TRACE_HTTP" } })"

Only show messages related to eth_getLogs:

dfx deploy evm_rpc --argument "(record { consoleFilter = opt variant { ShowPattern = "eth_getLogs" } })"

Note that ShowPattern and HidePattern accept regular expressions and are evaluated using the regex crate.

[gregorydemay]

Thanks a lot @rvanasa for this PR and the changes regarding to logging. I have one more comment regarding the mix of test logic with the productive code, but otherwise looks good to me!

[gregorydemay]

mixing test code with productive code is usually not a good idea and I think here you actually don't need it, so I would remove this and the declaration L24-27.

Here is an alternative of how it could look like in the test file src/logs/tests.rs:

...

declare_log_buffer!(name = TEST_BUF, capacity = 1000);
const TEST: PrintProxySink = PrintProxySink(&Priority::Debug, &TEST_BUF);

...

#[test]
fn should_show_all() {
    set_console_filter(ConsoleFilter::ShowAll);
    log!(TEST, "ABC");
    log!(TEST, "123");
    log!(TEST, "!@#");
    assert_eq!(get_messages(), vec!["ABC", "123", "!@#"]);
}

...

fn get_messages() -> Vec<String> {
    export(&TEST_BUF)
        .into_iter()
        .map(|entry| entry.message)
        .collect()
}

Does-it make sense?

[rvanasa]

The problem I'm running into with this approach is that the messages are always added to the buffer regardless of the filter, which only affects whether the messages are printed to the console.

The simplest solution would be to change the logic so that the console messages are the same as the logs. I also looked into intercepting the println!() output, but compared to this, it seemed cleaner to add the DISPLAYED_LOG_ENTRIES data structure to simulate this in a more controlled way. A third option could be to call ConsoleFilter::is_valid() (now renamed try_is_valid()) directly instead of going through a PrintProxySink. Do you have a preference for any of these solutions or something else?

[gregorydemay]

the messages are always added to the buffer regardless of the filter, which only affects whether the messages are printed to the console.

good point, I missed this. I should have tried with more tests :)

The simplest solution would be to change the logic so that the console messages are the same as the logs.

I have the impression this would indeed the simplest. I guess for this it would then maybe make sense to rename ConsoleFilter to something like LogFilter?

[rvanasa]

Done!

[gregorydemay]

nit: I think we should panic in case the regex is not valid instead of just silently returning false

[rvanasa]

I suppose we can do this because it's only used in the local replica. Updated.

[gregorydemay]

I suppose we can do this because it's only used in the local replica.

By local replica you mean running locally in dfx? If yes, I think we can and should also do this even if we were to use that feature productively. This could only be set during an upgrade and so if the regex is wrong then the panic will revert the upgrade which is IMO the correct thing to do.

[rvanasa]

Yes, that makes sense. This is now implemented.

[letmejustputthishere]

We should make sure this is properly documented with a new release. Do we expect projects that deploy their own evm rpc canister to only deploy the latest release as well?

[rvanasa]

We should make sure this is properly documented with a new release.

For sure. I'll work with @gregorydemay and @jessiemongeon1 on making sure the documentation is updated for the new changes.

Do we expect projects that deploy their own evm rpc canister to only deploy the latest release as well?

We can recommend this, although the previous version should continue working as well.

[letmejustputthishere]

We can recommend this, although the previous version should continue working as well.

what about deploying main?

[rvanasa]

If I understand the question correctly, this is a work in progress since it requires an NNS proposal. We also want to give a window of time for developers to migrate to the new version before redeploying.

[rvanasa]

Documentation PR: dfinity/portal#3575

[letmejustputthishere]

what i meant was should we advise projects not to deploy from main, but only from releases instead :)

[rvanasa]

Haha, okay, I see what you're saying. Yes, that is what we should recommend.