unleash icon indicating copy to clipboard operation
unleash copied to clipboard

feat: Add functionality to customize log layout and introduce JSON logger

Open budjb opened this issue 1 year ago • 5 comments

About the changes

This MR introduces the ability to customize the log layout of the default logger configuration without having to provide an entirely custom logger configuration. It also adds JSON structured logs (under the json name) using the log4js-json-layout library.

Important files

This is a relatively small MR but most of the changes are related to the logger.ts and create-config.ts files.

Discussion points

N/A

budjb avatar Dec 27 '23 19:12 budjb

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
unleash-monorepo-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 27, 2023 7:10pm

vercel[bot] avatar Dec 27 '23 19:12 vercel[bot]

Someone is attempting to deploy a commit to the unleash-team Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] avatar Dec 27 '23 19:12 vercel[bot]

Thanks for taking the time to pull this together.

I think it makes sense for Unleash to support json logging natively, as it is becoming in standard these days.

I do share @Tymek concern on taking on a niche dependency for this, and want to take the opportunity to assess our options.

ivarconr avatar Dec 29 '23 21:12 ivarconr

@ivarconr I can definitely see the point about the dependency. It has not been updated in a while, however, it's a very lightweight library.

The work to convert a log object to a JSON string is fairly trivial and that dependency can certainly be removed.

I agree that it makes sense to include easily-configured JSON logging support, as structured logs are a fairly well-established best practice with logging aggregation services. The only tricky thing, I think, is providing the flexibility to configure the names of certain important keys in the resulting object (e.g. the key that contains the log message, the key for the timestamp, etc.). DataDog, for example, wants the log body to be contained in the message key for it to show correctly in its log view. I'm not sure what other services might require. A simple env var with key/val pairs might do the trick, perhaps.

The underlying goal of this MR is to allow the ability to configure a JSON logger through environment variables and without having to maintain a custom container image with that support added in. Though this was admittedly a while ago, there seemed to be support for the idea on the Slack channels. I've finally gotten around to sending in the contribution :D

If the idea of adding first-class JSON logging support is agreeable, I can move forward with an implementation of it without adding the external dependency. Let me know your thoughts.

budjb avatar Jan 03 '24 03:01 budjb

I am in favor of supporting json logs.

But I am not fully convinced we want to commit ourself to use log4js in the future. In fact, if you do not provide a custom logger, I think the logger utilized by Unleash should be an internal concern of Unleash.

For Unleash Cloud we actually use Winston, which have served our json needs well for a few years.

I am considering that we might want to base a json logger on Winston instead and consider migrating over to Winston for the plain logger in the next major of Unleash (I suspect it would be to hard to keep the logs exactly the same between to two different providers, and any change to the log output should be considered a breaking change).

One need we have for Unleash Cloud is to enrich the log with static fields (clientId, plan, region etc), named defaultMeta in Winston. This might be an internal need of our Cloud offering.

Another thing we do is to enrich the Prometheus metrics with counters from log levels. I believe this can be useful for everyone hosting Unleash.

I would love more pls opinions here. I still believe the logger used by Unleash should be an implementation detail, but we should consider how we can allow a few more options in a structured way to provide new formats and some more customization of the Unleash logs. Those options could be encapsulated in a log container in the option object.

ivarconr avatar Jan 04 '24 22:01 ivarconr