AztecProtocol / aztec-packages

Apache License 2.0
192 stars 194 forks source link

Refactor logging to use a single logger that alpha team decides #9234

Open ludamad opened 1 week ago

ludamad commented 1 week ago

Winston is very featureful, we should combine the LOG_JSON mode wth our stdout mode which it both supports and get rid of our custom logger, doing any sort of transforms we currently do before hitting winston

ludamad commented 4 days ago

Note @alexghr had thoughts, can you write them here?

alexghr commented 3 days ago

I quite like pino for logging. It logs JSON all the time and is a drop-in replacement for debug (see pino-debug) so we wouldn't have to change any of our logging code (at least initially).

For local logs pino has pino-pretty to pretty print the JSON logs (we'd just have to redefine our start scripts to pipe stdout through pino-pretty).

Pino is supposedly faster than both winston and debug but I haven't tested this myself (pino benchmarks);

Both pino and winston have official OpenTelemetry instrumentation plugins (to capture context and push logs to the collector):

CC @spalladino as he had some thoughts on logging too!

ludamad commented 3 days ago

Ah gotcha, well it's pretty trivial to move to Winston too, curious on @spalladino's thoughts too

ludamad commented 3 days ago

Hmm what was the rationale to keep node debugger module?

ludamad commented 1 day ago

@spypsy I'm probably leaning towards redoing this issue as just attaching an opentelemetry plugin for winston so we can point it to the k8s collector URL from the local network tests