feat: Add method to get event fields inside a Hook

[erezrokah]

Hi 👋 Thank you for this great module 🚀

Opening for feedback as an attempt to fix #493, #587, also referenced from https://github.com/open-telemetry/opentelemetry-go-contrib/pull/5918/files#diff-a9b82897838da6b8eed398e03c9590621e6c2336ff576b3a49fb684ff8bba2a1R110

This is not the cleanest approach, but could be better from using reflection. It doesn't expose the internal buffer representation format so if it ever changes GetFields can be modified.

Feedback very much welcomed

[erezrokah]

Thanks for the quick review @ccoVeille, followed up with the suggestions and added more tests. Also please see the comment about the tradeoff of using JSON Decode

[ccoVeille]

Thanks for applying the changes, here are a few more feedbacks

[ccoVeille]

LGTM 🤞

[chkp-omris]

@ccoVeille @erezrokah I'd also be very happy to have this feature merged, I happened to find this PR looking for a solution.
May I suggest that the implementation simply returns the internal buffer buf to avoid forcing JSON decoding on the user?

[erezrokah]

@ccoVeille @erezrokah I'd also be very happy to have this feature merged, I happened to find this PR looking for a solution.
May I suggest that the implementation simply returns the internal buffer buf to avoid forcing JSON decoding on the user?

I following #637 (comment) as not to expose the internal buf. If we expose buf and then later there's a need to change the internal representation of the event to something else it will be harder to do

[mark-ten9]

It would be great to merge this if possible so that we don't have to use reflection to work around this.

[nduong-ol]

Hello, it has been a while. Would it be possible to merge this PR?

[christopher-taylor]

Chiming in to say I would love this feature to be merged in!

[nduong-ol]

In the end I find that to implement io.Writer interface receive the log output from zerolog and export it with OTLP is quite effective also. We can get the intended formatted log from zerolog without the need to process the internal data ourself

[christopher-taylor]

In the end I find that to implement io.Writer interface receive the log output from zerolog and export it with OTLP is quite effective also. We can get the intended formatted log from zerolog without the need to process the internal data ourself

Do you have a minimal example implementation?

[nduong-ol]

In the end I find that to implement io.Writer interface receive the log output from zerolog and export it with OTLP is quite effective also. We can get the intended formatted log from zerolog without the need to process the internal data ourself

Do you have a minimal example implementation?

Sure. Here the code of the logger with the same approach of the Hook but instead of receiving zerolog.Event, it takes the byte slice from logger output and export it. The approach is similar on how we would write zerolog to a file. In this approach I didn't unmarshal the json log to get the log level because I'm lazy. If you want to can unmarshal the zerolog json to extract information before sending it over OTLP

package` logging

import (
	"context"
	"io"

	"go.opentelemetry.io/otel/log"
	"go.opentelemetry.io/otel/log/global"
)

type config struct {
	provider  log.LoggerProvider
	version   string
	schemaURL string
}

func newConfig(options []Option) config {
	var c config
	for _, opt := range options {
		c = opt.apply(c)
	}

	if c.provider == nil {
		c.provider = global.GetLoggerProvider()
	}
	return c
}

func (c config) logger(name string) log.Logger {
	var opts []log.LoggerOption
	if c.version != "" {
		opts = append(opts, log.WithInstrumentationVersion(c.version))
	}
	if c.schemaURL != "" {
		opts = append(opts, log.WithSchemaURL(c.schemaURL))
	}
	return c.provider.Logger(name, opts...)
}

// Option configures a Hook.
type Option interface {
	apply(config) config
}
type optFunc func(config) config

func (f optFunc) apply(c config) config { return f(c) }

// WithVersion returns an [Option] that configures the version of the
// [log.Logger] used by a [OtelLogger]. The version should be the version of the
// package that is being logged.
func WithVersion(version string) Option {
	return optFunc(func(c config) config {
		c.version = version
		return c
	})
}

// WithSchemaURL returns an [Option] that configures the semantic convention
// schema URL of the [log.Logger] used by a [OtelLogger]. The schemaURL should be
// the schema URL for the semantic conventions used in log records.
func WithSchemaURL(schemaURL string) Option {
	return optFunc(func(c config) config {
		c.schemaURL = schemaURL
		return c
	})
}

// WithLoggerProvider returns an [Option] that configures [log.LoggerProvider]
//
// By default if this Option is not provided, the Logger will use the global LoggerProvider.
func WithLoggerProvider(provider log.LoggerProvider) Option {
	return optFunc(func(c config) config {
		c.provider = provider
		return c
	})
}

var _ io.WriteCloser = (*OtelLogger)(nil)

type OtelLogger struct {
	logger log.Logger
}

func NewOtelLogger(name string, options ...Option) *OtelLogger {
	cfg := newConfig(options)
	return &OtelLogger{
		logger: cfg.logger(name),
	}
}

func (l *OtelLogger) Write(p []byte) (n int, err error) {
	r := log.Record{}
	r.SetSeverity(log.SeverityInfo)
	r.SetBody(log.StringValue(string(p)))
	r.SetSeverityText("INFO")

	l.logger.Emit(context.Background(), r)
	return len(p), nil
}

// Close implements io.Closer, and closes the current logfile.
func (l *OtelLogger) Close() error {
	return nil
}

And the integration with zerolog

       var writers []io.Writer {
		zerolog.ConsoleWriter{Out: os.Stdout, TimeFormat: time.RFC3339},
                NewOtelLogger("app-name"),
	}

	logLevel := zerolog.InfoLevel
	logContext := zerolog.New(io.MultiWriter(writers...)).Level(logLevel).With().Timestamp()

[PeterBocan]

@erezrokah @ccoVeille what's the status of this PR?

[ccoVeille]

@erezrokah @ccoVeille what's the status of this PR?

I simply reviewed the PR. I'm not a maintainer. I wish this PR was merged

[ekristen]

hey @rs apologies for the direct ping however I didn't see a better choice. It's been well over a year since this was opened, and not a lot of activity on the repo in the last 5 months.

Love this library, it's definitely my preferred, would like to see this get merged, so I wanted to call this to your attention and also ask, is there a better way or a preferred way to get maintainer involvement on PRs like this?

Thanks again for the library, much appreciated.

[technicallyty]

Would love if this was merged! I also would benefit from a zerolog otel bridge, which is blocked w/o this PR

[lucasingels]

I would also be very keen to see this merged so we can have a proper and performant open telemetry bridge

[SAdams601]

I would also like to see this feature merged. I do see that the tests no longer pass due to the call to Event::Msg in the tests and the change to clear the buffer made here: https://github.com/rs/zerolog/pull/747/changes#diff-ebc5808085c4784a2adab4890ba806d0c695f71e0c5507f7c267659ee6c5f2e0

Not sure what the intention of having:

			if tc.message != "" {
				tc.e.Msg(tc.message)
			}

Was in the tests originally, but this now clears the buffer.