This is needed for log sinks where the underlying stack unwinding doesn't support an explicit call depth, for example testing.T.Log.

One such log sink is currently pending in https://github.com/kubernetes/klog/pull/240. It currently incorrectly logs logr.go as call site.

Thanks for logr.

I've been using logr across the components of an app, deployed to different different Google Cloud Platform (GCP) services (GKE, Cloud Functions, Cloud Run) and all use Cloud Logging.

To benefit most from Cloud Logging, I want to use structured logging but:

1. Cloud Logging requires that log entries be JSON formatted
2. Cloud Logging requires that logs use GCP-specific keys (e.g. logging.googleapis.com/labels, logging.googleapis.com/sourceLocation)

For (1), I've switched from stdr to zerologr to emit JSON-formatted logs.

This requires taking a dependency on rs/zerolog when all I really need is the standard library's encoding/json.

Unfortunately, zerologr entries omit the caller (file, line) struct that's almost a match for Cloud Logging's SourceLocation.

Is there a way to retain this?

For (2), I'm using the GCP-specific keys when writing logs but I'd prefer to not be so tightly bound.

Should I consider writing a JSON LogSink?

Can I chain LogSinks or would I then also need to write a LogSink that combines to JSON and GCP-specific keys?

Thanks!

A related but different stupid question, why do LogSinks bind Info and Err to a single writer? For container logging, I'd prefer to write Info to os.Stdout but Err to os.Stderr. As it is, I can either have one or the other, or I need to have duplicate Loggers so that I can stdout.Info and stderr.Err (and never stdout.Err or stderr.Info) which seems redundant.

Is it possible to extend the API to accept context when logging? The reason is that we want to extract information from the context like opentelemetry trace ids (trace_id/span_id) or other information that we propagate in the app using the context.

One common pattern for logging is to pass the current logging context to called functions and types using a Go context. This decouples much of the program from a specific implementation of the logr interface. This is similar to adding a logger argument to every function but is more general since it allows non logr-aware packages to pass contexts through to logr-aware packages.

A suggested API (these are top level functions):

// FromContext returns a logr.Logger constructed from the context or returns a null logger if no
// logger details are found in the context.
func FromContext(ctx context.Context) logr.Logger

// NewContext returns a new context that embeds the logger.
func NewContext(ctx context.Context, l logr.Logger) context.Context

This can already be implemented in a separate package (I created logctxr as an example) but I thought it would be worth considering as a core addition to the logr package.

Updated 2020-10-20 to remove NewLogger function based on discussion

Switch the interfaces over to support/favor structured logging. Format-string style logging is no longer supported by the interface. Instead, callers should use tag-value pairs to introduce variable information in a structured way.

The general pattern is based of go.uber.org/zap (specifically, the SugaredLogger), but we keep some ofour own opinions (log levels go up, not down, numeric levels, etc), and keep a limited interface.

In Kubernetes, sometimes klog.ErrorF is called without having an error. During the structured logging migration, that then gets translated to klog.ErrorS(nil, ...). I wonder whether the same nil error would or should be allowed for a logr.Logger. If yes, then we should document it.

Currently it is undefined behavior, so one could argue that explicitly allowing it is not an API change and just documents how loggers should have been implemented all along anyway (tolerate user errors if it was considered as one, don't crash, etc.).

The intention is to use this when the output is structured (like JSON) when the original type would be logged as string. It also has some other use cases.

This approach was chosen instead of a full marshaler API as in zapcore.ObjectMarshaler because the implementation is simpler. The overhead for large types is expected to be higher, but it is not certain yet whether this is relevant in practice. If it is, then a marshaler API can still be added later.

For alternatives, see the discussion in https://github.com/kubernetes/klog/issues/262

@wojas: @thockin and I were already discussing this a bit. Here's a formal PR, perhaps you can also have a look?

thanks to nice work, because of the funcr package use a api strconv.FormatComplex that come from go1.16, if some one want to use this log interface, he must update his golang version, this seems no need for a minimal log interface

Two aspects were not spelled out explicitly, which sometimes led to misunderstandings:

• error messages are always printed
• the error instance is optional

Fixes: https://github.com/go-logr/logr/issues/118

Add new Options fields which are function pointers to optional hooks to be called.

This allow users to manipulate the way that kv-pairs are logged. For example:

• prefix or rename builtin keys
• log WithValues() kv-pairs as a single pseudo-struct
• collect telemetry around how often each log is triggered

This adds a new type PseudoStruct which can be used to trigger special logging - a kvList is treated as a struct.

This also exposes the Caller type, to enable hooks to recognize and use caller information.

Fixes #97

(builds on #110)

When WithCallDepth stored the new sink, it didn't update the cached withCallDepth implementation. When that then was called again by another WithCallDepth, the previously incremented call depth was overwritten.

This makes it possible to reference it in other LogSinks which want to provide the same functionality without pulling in the funcr source code.

Doing the same with current master results in "go mod vendor" including "funcr.go" under "vendor".

This is like the existing funcr.PseudoStruct except that it uses structs as members of the slice. The reasons for that difference are making the API type-safe at compile time and (subjectively) nicer rendering when a LogSink does not support the type.

Here's how zapr and klogr handle funcr.PseudoStruct:

{"caller":"test/output.go:435","msg":"funcr","v":0,"parent":["a",1,"b",2]}
I output.go:486] "funcr" parent=[a 1 b 2]

Here's the same for logr.KeysAndValues:

{"caller":"test/output.go:435","msg":"keys and values","v":0,"parent":[{"Key":"a","Value":1},{"Key":"b","Value":2}]}
I output.go:486] "keys and values" parent=[{Key:a Value:1} {Key:b Value:2}]

The tentative support for logr.KeysAndValues would lead to output like this:

{"caller":"test/output.go:<LINE>","msg":"keys and values","v":0,"parent":{"boolsub":true,"intsub":1,"recursive":[{"Key":"sub","Value":"level2"}]}}
I output.go:<LINE>] "keys and values" parent={ boolsub=true intsub=1 recursive={ sub="level2" } }

The implementation in klogr is recursive and handles KeysAndValues nested inside KeysAndValues, the one in zapr is simpler and only handles it at the top level.

Having to add a modified logger to a context when adding some values to the context which are meant to be logged (like trace ID) has two drawbacks:

• the instrumentation code which adds the values to the context must be aware of logging
• modifying the logger incurs a cost, whether some actual log entry then gets emitted or not.

A better approach is to add the values only to the context, then during logging extract them. This is the same approach that contextual logging in Go is going to use.

I have experimented with a few different approaches for this, including doing it entirely in klog by wrapping the LogSink in a logger with one that injects the extra values. That is complicated and not worth discussing further.

In logr itself there are different approaches - see individual commits in https://github.com/pohly/logr/commits/with-context-all

The one proposed here implements logging of the additional values in Logger (no need to modify LogSinks) and passes them to the LogSink via WithValues.

Hey there 👋 PR #143 was merged a few months ago, however, it is not in any release (last release 1.2.3 does not include this feature). Any chance you could produce a new release with this feature?

I'd like to avoid pinning my dependency to @master or to a specific commit, if possible. Thank you!