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

consider providing a method to compute the log level based on a String #18

Closed
carocad opened this issue Feb 22, 2022 · 5 comments · Fixed by #22
Closed

consider providing a method to compute the log level based on a String #18

carocad opened this issue Feb 22, 2022 · 5 comments · Fixed by #22

Comments

@carocad
Copy link

carocad commented Feb 22, 2022

For most of our apps we setup the log level based on environment variables. However go-kit/log currently has hardcoded internal levels which makes mapping from the env var setting to the real level more cumbersome.

I checked the code since you already have a Value interface for the log levels which should be usable through this function, unfortunately it seems unused as here it is casted to levelValue instead of Value.

Hope it helps

@ChrisHines
Copy link
Member

However go-kit/log currently has hardcoded internal levels which makes mapping from the env var setting to the real level more cumbersome.

Would you care to elaborate what you are finding cumbersome?


What type of method are you looking for?

Something like:

func LevelFromString(lvlName string) (Value, error)

We left that type of function out of the package because we considered it part of how applications are configured and we considered configuration out of scope of go-kit (See: https://github.com/go-kit/kit#non-goals). If we provided such a function I worry it would become a point of regular requests to make it accept various different spellings of the levels.

@alexandru-eftimie
Copy link

alexandru-eftimie commented Feb 28, 2022

Right now there doesn't seem to be any other way to set the log level except for using the pattern
func Allow...
If one were to set the level through any sort of application code is there any way currently to avoid a verbose switch with all the levels?

@ChrisHines
Copy link
Member

... is there any way currently to avoid a verbose switch with all the levels?

Not with the code in this package, no. But I think the details of that switch statement depend on details of the application that are out of scope for this package.

If you have many applications within a project that you want to use that switch statement in, that's a good candidate for a project library to encode that behavior in a single place.

I don't think it's a hard switch statement to write. You could probably also define a map[string]level.Option to lookup which option to use.

@alexandru-eftimie
Copy link

The project library is essentially what I ended up doing

@peterbourgon
Copy link
Member

For the record, I also end up doing something like this, once a project reaches a certain size. But few of my projects get there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants