Clarify log level dependency between the SDK and Comet

[adizere]

Summary of problem

From my current understanding, briefly:

• configuring Comet log_level in config.toml can break the SDK-level logging
• configuring --log_level at the CLI overrides the Comet-level logging directive

Put differently, the SDK enforces log_level to be the same across the application and Comet. I'm not sure this is ideal, it might be good to break this dependency.

More context on the problem

CometBFT allows log level to be configured on a per-module basis, for example: log_level = "p2p:info,state:info,statesync:info,*:error". The SDK depends on the log_level string (ref), but the SDK has no knowledge of Comet-level modules (p2p, statesync). For this reason, if we parametrize config.toml with the log_level above, the SDK will thrown an error

Error: failed to parse log level (main:info,state:info,statesync:info,:error): Unknown Level String: 'main:info,state:info,statesync:info,:error', defaulting to NoLevel

cosmos-sdk/server/util.go

Lines 148 to 152 in 2582f0a

	logLvlStr := serverCtx.Viper.GetString(flags.FlagLogLevel)
	logLvl, err := zerolog.ParseLevel(logLvlStr)
	if err != nil {
	return fmt.Errorf("failed to parse log level (%s): %w", logLvlStr, err)
	}

So if we configure log_level, an operator can't start the node due to the error above. This can be fixed by providing a log level in the CLI, for example

gaiad start --home cosmoshub-4/ --log_level error

The problem now is that this disables the Comet log_level directive specified in config.toml.

Requirements and question

The requirements from Comet side is we'd like to allow node operators to configure log_level to be able to debug.

Is it possible or desirable to have independent log levels? One from Comet's config.toml, and another one from the CLI --log_level?

Version

• v0.45*
• v0.46*

I'm aware of recent work here #14967, but doesn't seem to address the problem here. Let me know if I'm missing anything in the above, thanks!

[tac0turtle]

this is known, we dont have filter logging on previous versions only log levels, but an app can override using interceptconfig and place their own logger with custom loggging features. They can use the tendermint logger as well.

Is it possible or desirable to have independent log levels? One from Comet's config.toml, and another one from the CLI --log_level?

the logger is passed to comet in start node, if comet would like a separate logger then they would have to do it. We are setting the logger for apps and in turn passing it to comet as its required. what is the issue with setting it to debug, this is the first time i have heard of this issue

[robert-zaremba]

so, the idea will be to implement the log-level syntax in SDK to make it compatible with CometBFT, this way it won't fail and will be future compatible if we will have more loggers (per module).

[julienrbrt]

so, the idea will be to implement the log-level syntax in SDK to make it compatible with CometBFT, this way it won't fail and will be future compatible if we will have more loggers (per module).

We have a function compatible with both now for filtering: https://pkg.go.dev/cosmossdk.io/log#FilterKeys.
Such function can be replicated at logger instantiation for any logger.

[adizere]

Thanks all for the quick feedback.

what is the issue with setting it to debug, this is the first time i have heard of this issue

Not sure I understand what you mean here. It's ok to set things to debug. But it's not ok to break the app by configuring Comet log_level with specific modules in config.toml.

I don't have specific solutions yet. But to make sure the problem is understood: the per-module log_level from config.toml is not usable at the moment.

• a. one fix is SDK avoids passing log_level from config.toml into the app; instead, the SDK can provide to the app a default log (info) and the app will parametrize it with CLI flag --log-level
  • since the logger is global, this seems impossible, unless we rewire Comet to have its own independent logger, which seems too much
• b. potential fix: Comet removes the support for per-module log_level from config.toml in Comet, therefore log-level in Comet will be compatible with the simpler app/SDK-level syntax
• c. potential fix: SDK logger allows per-module log-level syntax, aligning with Comet's
  • I think this is Robert's point

---

so, the idea will be to implement the log-level syntax in SDK to make it compatible with CometBFT, this way it won't fail and will be future compatible if we will have more loggers (per module).

We have a function compatible with both now for filtering: https://pkg.go.dev/cosmossdk.io/log#FilterKeys. Such function can be replicated at logger instantiation for any logger.

Neat! A couple of questions for me to understand better:

• Is this the same as point c. I wrote above? In other words: can it support a syntax such as "p2p:info,state:info,statesync:info,*:error" ?
• From which SDK version is this available?

[julienrbrt]

So v0.47 reverted to use CometBFT logger directly, which made the filtering working again in that version.
The way it was in v0.45 and v0.46 is that indeed we were parsing the level as a zerolog level, which was making the cometbft logging filtering not working adequately.

• Is this the same as point c. I wrote above? In other words: can it support a syntax such as "p2p:info,state:info,statesync:info,*:error" ?
• From which SDK version is this available?

In the next version (v0.48), we've refactored away the usage of CometBFT logger but kept the filtering behavior.
With minimum manual wrapping and function overwriting (not provided by the SDK), that logger can be used in all versions, but the best experience will be in v0.48.