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

Select Logging Framework #76

Closed
danehans opened this issue May 26, 2022 · 9 comments
Closed

Select Logging Framework #76

danehans opened this issue May 26, 2022 · 9 comments
Labels
kind/enhancement New feature or request
Milestone

Comments

@danehans
Copy link
Contributor

A logging framework should be utilized to decouple EG from any specific logging implementation. The logging framework should:

  • Support a wide range of logging library implementations, including implementations supported by k8s and k8s-related pkgs. Note that controller-runtime/Istio logging pkgs are built on top of Zap and k8s uses klog.
  • Provide a lightweight interface that can be used to emit logs.
  • Be well documented.
  • Be widely implemented and well maintained by a large, diverse open source community.
  • The API should restrict logging to just 2-3 types of logs, e.g. info and error.
  • Provide structured logging.
  • Provide verbosity levels on info logs as a numerical value instead of semantic meaning such as "warning", "debug", etc. This gives operators an easy way to control the chattiness of log operations.
  • Integrate with the Cobra, making it easy for EG to use a consistent interface for logging.
@danehans
Copy link
Contributor Author

xref: #60

@danehans danehans added the kind/enhancement New feature or request label May 26, 2022
@danehans danehans added this to the 0.2.0 milestone May 26, 2022
@danehans
Copy link
Contributor Author

EG implementation plan xref: #60

@basvanbeek
Copy link
Member

With the same ideas in mind, we created:
https://github.com/tetratelabs/telemetry

The key point is that it is just an interface with some wrapping logic to support dynamic log levels and Istio style scoped loggers, but allows one to plug their own logging implementations.

This makes it easy for people embedding / building on top of EG libraries to select a different logging library while EG maintainers can write EG packages with logging directives in a consistent manner without worrying about the underlying logging implementation being used by an application binary using EG packages.

A logging Implementation supporting this interface can be found here:
https://github.com/tetratelabs/log/tree/v2

Another interface solution is:
https://github.com/go-logr/logr

However I'm personally not in favor of it due to its V-level interface. It makes it hard in a community project to properly identify verbosity of logs. Making it numeric and open ended allows for dis-synchronization around what a specific log levels verbosity should look like. It puts an extra burden on library authors to think about which level would fit and different authors might have a different sense of what a particular level entails. This is the main reason tetratelabs/telemetry only supports the following output levels to keep it simple:

  • error (actionable errors / needing alerts)
  • info (informational / helps provide context)
  • debug (elevated level for deep debugging / development)

Logr also doesn't take care of allowing scoped loggers with dynamic log level setting.

I have no strong opinion on which logging implementation or output format to use for standard EG, I do have one on which logging interface is used.

@LukeShu
Copy link
Contributor

LukeShu commented Jun 7, 2022

dlog is an interface solution that I'd advocate for (disclosure: I'm the original author). It is awfully similar to logr, but

  1. Flattens logr.FromContext(ctx).Info(msg) to dlog.Info(ctx, msg). It was our experience in Emissary/Edge Stack that while ctx was the "main" way to pass the logger around, because you had to extract the logger out of it there was a chance that they would drift, and that this turned out to be an extremely easy mistake for contributors to make.
  2. No V integer; just a few standard log levels: error/warn/info/debug/trace.

I have no strong opinion on which logging implementation or output format to use for standard EG, I do have one on which logging interface is used.

same

@basvanbeek
Copy link
Member

I have taken a look at dlog and I would prefer https://pkg.go.dev/github.com/tetratelabs/telemetry#Logger.

Couple of reasons (also see my explanation on what telemetry/logger provides over logr as the same applies to dlog):

  • A logger itself should not be passed along through context. Typically a log is bound to a library / package / section of your code and holds code point specific context. Letting the request path direct context on its own is tricky.
  • Request context can be added on top of a code section specific logger while the library author can focus specifically on the context build up from within the package logger (request context will then be augmented to the resulting lines if required)
  • Package specific loggers allow for zooming in on specific sections of your code to elevate or squelch log lines with runtime level adjustments
  • typically for Error and Warning logs you'd like to add metrics for aggregated stats on these events. This is made very easy by the telemetry package as it has a metrics interface similar to Istio to automatically drive a count. Driving implementations for the metrics interface could be for instance OpenCensus, OpenTelemetry, or Prometheus.

Note that metrics is an opt in feature. we can choose to do direct OpenTelemetry in our code for metrics purposes without using the telemetry/Metric interface. Using the interface would however allow vendors to choose their implementations just like with logging while we as EG library authors do not need to care about the underlying implementations used in a binary.

@Jayden-onl
Copy link

Jayden-onl commented Jul 1, 2022

Hi, guys.
I wrote a logging library: https://github.com/forgoes/logging
Quicker than Zap, ZeroLog. Benchmark here: https://github.com/forgoes/benchmark-logging
Actually, a logging system is a stream handling system.
My logging library's feature:

  1. quick quick quick;
  2. stream-oriented processing;
  3. parent-child relations between logger;
  4. ....

Sorry that I have not finished the relevant documentation.
Later, I would add more features like:

  1. block logging(for strong consistency scenario);
  2. SafeLogger(you may change the logger's level, handler and so on during runtime, but obviously, SafeLogger will reduce the performance)
  3. ...

Just an introduction, I'm really interested in envoy-gateway project.

@youngnick
Copy link
Contributor

After discussing in the community meeting yesterday, I'd like to propose that we move forward with using the logr interface for now, with some definition around the usable log levels to address @basvanbeek's concerns.

Can maintainers please cast binding votes here with either reactions or comments, and as always non-binding votes are welcomed.

@youngnick
Copy link
Contributor

We discussed this in the community meeting today and agreed that we'll proceed with logr interface for now.

@arkodg
Copy link
Contributor

arkodg commented Jul 8, 2022

Looking at #146 I see the usage of https://github.com/go-logr/logr . Major downsides I notice are

  • the inability to do component level scoping (envoy supports this)
  • passing the log object around/saving it in some context adding more burden on app devs
    it does make sense to continue with logr for now to build out an mvp, hoping we can revisit this post 0.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants