go-log icon indicating copy to clipboard operation
go-log copied to clipboard

Expose `configFromEnv` as a public method

Open ribasushi opened this issue 4 years ago • 8 comments

There is a legitimate use-case to be able to examine the result of configFromEnv from an external caller. See discussion here for details: https://github.com/filecoin-project/lotus/pull/6743#discussion_r668728046

ribasushi avatar Dec 02 '21 14:12 ribasushi

i second this. i'd like to be able to look at which, if any, subsystems have logging enabled, and their levels.

my use case wants a few things -

  1. i'd like to conditionally enable the subsystem information on log lines only if there are multiple subsystems enabled aside from my app's main logger.
  2. i want to set my app's main logger to INFO log level by default through code, but still allow the user to configure it through env (currently it does a hard-overwrite of the env config for the subsystem, so it becomes unconfigurable for the user)
  3. i want to implement subsystem groups, which would control a predefined set of subsystems without forcing the user to configure each one manually. this doesn't need special support from go-log, it could be accomplished by creating a dummy subsystem, whose log level would be inspected, and applied to a set of other dependent subsystems. it would need this feature, though

i would be willing to contribute a pr for this if i can get a green light^^

elijaharita avatar Dec 15 '21 20:12 elijaharita

i would be willing to contribute a pr for this if i can get a green light^^

@elijaharita just checked with maintainers: you have a 🍏🚦to PR, let's do it!

ribasushi avatar Dec 15 '21 21:12 ribasushi

@ribasushi okay, proposal, here we go

we can leave configFromEnv inside of init() to avoid breaking existing code. instead, we would save the config to a private global variable upon running SetupLogging(). we would have a public function GetConfig() return a copy of the config. the library user would then simply read the copy for their purposes, and/or modify that copy and subsequently call SetupLogging() again to apply their config changes.

pretty simple but it gives the library user a ton more control without breaking anything. what u think?

elijaharita avatar Dec 16 '21 20:12 elijaharita

modify that copy and subsequently call SetupLogging() again to apply their config changes.

This is dicey... do you really need this? The rest sounds good!

ribasushi avatar Dec 16 '21 20:12 ribasushi

great! as for dicey, how so? do you have an alternative idea for allowing the user to update config with code?

elijaharita avatar Dec 16 '21 20:12 elijaharita

actually, calling SetupLogging again could be done in the library's current state too. is there even a way to configure the logger if not like that? @ribasushi

elijaharita avatar Dec 16 '21 20:12 elijaharita

I re-read your requirements, I missed the requirement to write at runtime ( 2 ) So yeah, ignore my concern, all good!

ribasushi avatar Dec 16 '21 20:12 ribasushi

@elijaharita is this closed by #129?

aschmahmann avatar Feb 25 '22 16:02 aschmahmann