staticbackendhq / core

Backend server API handling user mgmt, database, storage and real-time component
https://staticbackend.com
MIT License
682 stars 67 forks source link

feat: add logger to have better experience with work on app #44

Closed VladPetriv closed 1 year ago

VladPetriv commented 1 year ago

Is your feature request related to a problem? Please describe. As a new contributor, I can say that it's really hard to develop the application without logging because you can get some errors, and "fmt.Println" doesn't give all information about an error. And sometimes he can't print it.

Describe the solution you'd like We can use some modern, custom logger ex: "zap", "logrus" or "zerolog" which will write log's into file and console. It will more comfortable then use default "fmt.Println" or "log.Println")

I see it as a structure that will be passed to other functions as an argument. This structure will configurable with "LOG_LEVEL" or we can do it with "APP_ENV". For ex: if "APP_ENV" is equal to dev we will log messages from all levels but if it equal to prod we will log only error or critical levels)

Additional context People from Discord say that we should use "zerolog" and how I seen it looks not bad. How do you think should we implement it or not?

dstpierre commented 1 year ago

Some questions / thoughts to keep in mind:

  1. We need to stay backward compatible, so we'll need to have strong default when this get into production for version that will not change their config.
  2. I can see some devs wanting to have multi-output, we'll need a good way to configure this.
  3. What will be the strategy for replacing the current fmt.Println / log.Println with the new logger, is this part of this change?

I like to hdea of using APP_ENV with nice development defaults. I was looking at zerolog which says their "pretty" output isn't optimal, so this should only be enabled for APP_ENV=dev.

I'd like to discuss a bit regarding how one would configure this. As you explore which library you'd recommend, let us know what kind of options we have for non-console log (files, external services, etc).

Ideally I'd not want to add a hard dependency on a self-hosted log analysis. But we could certainly propose decent options.

VladPetriv commented 1 year ago

What will be the strategy for replacing the current fmt.Println / log.Println with the new logger?

While watching the project I see that "fmt.Println / log.Println/ log.Fatal etc" doesn't happen very often. So I guess that it won't be a problem to replace it manually.

Is this part of this change?

Yes sure.

I'd like to discuss a bit regarding how one would configure this. As you explore which library you'd recommend, let us know what kind of options we have for non-console log (files, external services, etc).

I don't know how to do this with "zerolog" because I have never worked with it.
But I know that "zap" logger has a built-in caller to write logs into the file and console at the same time. Ex: code

dstpierre commented 1 year ago

Multiple log output with zerolog.

So far I'd see the defaults behavior being:

APP_ENV=dev -> Use a "pretty" log output for all levels (debug, info, etc)

LOG_CONSOLE_LEVEL -> could be use to specify the minimum log level is wanted. For instance, LOG_CONSOLE_LEVEL=error would not includes trace,debug,info and warning level: https://github.com/rs/zerolog#setting-global-log-level

LOG_FILENAME -> if set, write logs to console and this file.

Now, should we think about monitoring size of this file? Who's responsible to clean this file?

How one ship this file to a 3rd party log analytics (self-hosted or paid).

Once we figure out the above, I think it'll be ready for implementation.

VladPetriv commented 1 year ago

Now, should we think about monitoring size of this file? Who's responsible to clean this file?

It's not a problem we'll use "log rotation" with "lumberjack" + "zerolog".

How one ship this file to a 3rd party log analytics (self-hosted or paid).

Hmm. I don't expect to do this but probably in this part of the issue, it'll be manually working. Maybe later we can do something better

VladPetriv commented 1 year ago

"lumberjack" + "zerolog".

For example, it'll delete a log file after a certain period or when the file size equal to some value

VladPetriv commented 1 year ago

@dstpierre If you don't have any questions I can start implementing it)

VladPetriv commented 1 year ago

Can you answer these questions?

dstpierre commented 1 year ago

How many days do you want to keep old log files?

I'd say a sane default would be 14 days to 30 days. Since the goal is most certainly to "ship" those log files somewhere else.

How much max file size in MB do you want to see?

I'd say 150-250 MB is probably an acceptable default. Rough questions I'd say, it's very situational.

How many backups of log files do you want to see?

Something like 3-7? I honestly don't know.

Does the library you're planning to use for log rotation suggests "fit-all" defaults?

VladPetriv commented 1 year ago

Does the library you're planning to use for log rotation suggests "fit-all" defaults?

What do you mean while saying "fit-all defaults"?

If you talk about "defaults" from yesterday's message https://github.com/staticbackendhq/core/issues/44#issuecomment-1217704091. The answer will be yes

dstpierre commented 1 year ago

@VladPetriv I'm just saying that I do not have hard specifics personally regarding log rotation, so if lumberjack has sane defaults, we should use that also by defaults.

VladPetriv commented 1 year ago

@dstpierre Sorry for misunderstanding. And yes lumberjack has default value. So we will use that)

VladPetriv commented 1 year ago

Hey @dstpierre can you review my pr?