search

open-telemetry / opentelemetry-go-contrib

Collection of extensions for OpenTelemetry-Go.
https://opentelemetry.io/
Apache License 2.0
1.13k stars 536 forks source link

Log Bridge returning io.Writer #5425

Closed pellared closed 1 month ago

pellared commented 4 months ago

A lot of logging libraries simply accept an io.Writer for configuration. E.g.

We could provide a generic log bridge which returns an io.Writer. That would allow using such libraries with OTel Go Logs.

The biggest challenge would be to determine when a new record starts and ends. By default we could be say that log records are split by EOL.

pellared commented 4 months ago

CC @mx-psi

shivanshuraj1333 commented 4 months ago

The biggest challenge would be to determine when a new record starts and ends. By default we could be say that log records are split by EOL

Would this also work for multiline log messages?

pellared commented 4 months ago

The biggest challenge would be to determine when a new record starts and ends. By default we could be say that log records are split by EOL

Would this also work for multiline log messages?

Nope. Do you have any proposal how to handle it?

My only idea it to add an option would would accept a handler with signature more of less like: func([]byte) []log.Record to make the parsing of the data passed to io.Writer configurable. Then the user could even extract the logging levels, timestamp, etc. from the passed data.

pohly commented 4 months ago

Forget klog.SetOutput. In order to integrate with Kubernetes, you want to write an implementation of a logr.Logger and configure OTel output in https://github.com/kubernetes/kubernetes/tree/master/staging/src/k8s.io/component-base/logs/api/v1.

For example, JSON output is a backend which plugs into that package.

pellared commented 4 months ago

@pohly, so https://github.com/open-telemetry/opentelemetry-go-contrib/issues/5192 is what you need?

There is already a PR open (https://github.com/open-telemetry/opentelemetry-go-contrib/pull/5357), but I had no time to look at it.

pohly commented 4 months ago

Something like that would be a good first step. But for integration into Kubernetes someone needs to think through all of the consequences (how to activate and configure the feature, how it affects other log output) - probably worth a Kubernetes Enhancement Proposal (KEP).

pellared commented 1 month ago

I made a quick analysis. There need to be different log bridges for klog and log because they are using the passed io.Writer differently. For log it should be easy as it looks like each log emition causes a single Write call on the passed io.Writer: https://cs.opensource.google/go/go/+/refs/tags/go1.22.5:src/log/log.go;l=245

pellared commented 1 month ago

Closing this one. I created https://github.com/open-telemetry/opentelemetry-go-contrib/issues/5963

pohly commented 1 month ago

How did you configure klog? When using the "modern" https://github.com/kubernetes/klog/tree/main/textlogger, there should be a single write per log entry: https://github.com/kubernetes/klog/blob/75663bb798999a49e3e4c0f2375ed5cca8164194/textlogger/textlogger.go#L138

pellared commented 1 month ago

How did you configure klog?

My bad, I looked at v1 🤦 Still there would need to be a bridge for each logger...

However, as you mentioned earlier an logr.Logger would be enough for Kubernetes. Referecen:

pohly commented 1 month ago

That textlogger is a logr.Logger implementation. I was just trying to figure out what you meant with "klog".