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

Expose configFromEnv as a public method #122

Open
ribasushi opened this issue Dec 2, 2021 · 8 comments
Open

Expose configFromEnv as a public method #122

ribasushi opened this issue Dec 2, 2021 · 8 comments
Labels
need/triage Needs initial labeling and prioritization

Comments

@ribasushi
Copy link
Contributor

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

@ribasushi ribasushi added the need/triage Needs initial labeling and prioritization label Dec 2, 2021
@ipfs ipfs deleted a comment from welcome bot Dec 2, 2021
@elijaharita
Copy link
Contributor

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^^

@ribasushi
Copy link
Contributor Author

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!

@elijaharita
Copy link
Contributor

@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?

@ribasushi
Copy link
Contributor Author

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!

@elijaharita
Copy link
Contributor

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

@elijaharita
Copy link
Contributor

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

@ribasushi
Copy link
Contributor Author

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

@aschmahmann
Copy link
Contributor

@elijaharita is this closed by #129?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/triage Needs initial labeling and prioritization
Projects
None yet
Development

No branches or pull requests

3 participants