otellogr emits nil context causing panic in exporters

[gregoryfranklin]

Description

By default, otellogr emits logs with a nil context - this causes exporters that do not support nil contexts to panic.

This happens because it uses a context that is never initialised

func NewLogSink(name string, options ...Option) *LogSink {
	return &LogSink{
		// does not initialise ctx
	}
}

type LogSink struct {
	ctx           context.Context
}

func (l *LogSink) Info(level int, msg string, keysAndValues ...any) {
	ctx, attr := convertKVs(l.ctx, keysAndValues...)

	l.logger.Emit(ctx, record)  // emits nil context
}

Environment

• go.opentelemetry.io/contrib/bridges/otellogr v0.8.0

Steps To Reproduce

Run the following code:

package main

import (
	"github.com/go-logr/logr"
	"go.opentelemetry.io/contrib/bridges/otellogr"
	"go.opentelemetry.io/otel/exporters/stdout/stdoutlog"
	"go.opentelemetry.io/otel/sdk/log"
)

func main() {
	exporter, _ := stdoutlog.New()
	processor := log.NewSimpleProcessor(exporter)
	provider := log.NewLoggerProvider(log.WithProcessor(processor))
	sink := otellogr.NewLogSink("example", otellogr.WithLoggerProvider(provider))
	logger := logr.New(sink)
	logger.Info("hello")
}

Results in

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x570653]

goroutine 1 [running]:
go.opentelemetry.io/otel/exporters/stdout/stdoutlog.(*Exporter).Export(0xc000016250, {0x0, 0x0}, {0xc0001284e0?, 0x0?, 0x0?})
	/tmp/.cache/go/pkg/mod/go.opentelemetry.io/otel/exporters/stdout/[email protected]/exporter.go:49 +0xf3
go.opentelemetry.io/otel/sdk/log.(*SimpleProcessor).OnEmit(0xc000070180, {0x0, 0x0}, 0xc000128340)
	/tmp/.cache/go/pkg/mod/go.opentelemetry.io/otel/sdk/[email protected]/simple.go:58 +0x1ad
go.opentelemetry.io/otel/sdk/log.(*logger).Emit(_, {_, _}, {{}, {0x0, 0x0, 0x0}, {0x0, 0x0, 0x0}, ...})
	/tmp/.cache/go/pkg/mod/go.opentelemetry.io/otel/sdk/[email protected]/logger.go:40 +0x162
go.opentelemetry.io/contrib/bridges/otellogr.(*LogSink).Info(0xc00013e100, 0x5aae00?, {0x5cb744?, 0x623c60?}, {0x0, 0x0, 0x0})
	/tmp/.cache/go/pkg/mod/go.opentelemetry.io/contrib/bridges/[email protected]/logsink.go:238 +0x173
github.com/go-logr/logr.Logger.Info({{0x625428?, 0xc00013e100?}, 0xc000145f20?}, {0x5cb744, 0x5}, {0x0, 0x0, 0x0})
	/tmp/.cache/go/pkg/mod/github.com/go-logr/[email protected]/logr.go:280 +0xf3
main.main()
	/tmp/otellogr/main.go:16 +0x1ad

Expected behavior

Using otellogr should not cause panics.

Workaround

Its possible to work around this problem by passing a context via WithValues

sink := otellogr.NewLogSink(
	"example",
	otellogr.WithLoggerProvider(provider),
).WithValues("ctx", context.Background()),

[dmathieu]

cc @scorpionknifes @pellared as code owners.

[flc1125]

Because there is no default ctx set here.

opentelemetry-go-contrib/bridges/otellogr/logsink.go

Lines 173 to 179 in 13b1cc5

	return &LogSink{
	name: name,
	provider: c.provider,
	logger: c.provider.Logger(name, opts...),
	levelSeverity: c.levelSeverity,
	opts: opts,
	}

---

In addition, the ctx here can be directly used as l.ctx without having to define otherwise.

opentelemetry-go-contrib/bridges/otellogr/logsink.go

Lines 204 to 206 in 13b1cc5

	ctx := context.Background()
	param := log.EnabledParameters{Severity: l.levelSeverity(level)}
	return l.logger.Enabled(ctx, param)

[dmathieu]

If the context is mandatory, should we require it as a mandatory parameter?
Something like:

otellogr.NewLogSink(ctx, "example", otellogr.WithLoggerProvider(provider))

[flc1125]

There is another way to allow ctx to be carried at any time (this situation may require more in practice,).

Because most of the time we log, the context we need is more appropriate based on where the code is currently being executed.

ctx, span := otel.Tracer("example").Start(context.Background(), "example")
defer span.End()

sink := otellogr.NewLogSink("example", otellogr.WithLoggerProvider(provider))
sink.WithContext(ctx).Info(".....")

[flc1125]

I found some information, but I'm not sure if this discussion applies.

• contextual logging for slog go-logr/logr#234 (comment)
• https://github.com/veqryn/slog-context

[flc1125]

If there is no one to deal with this problem, I can help deal with it.

[scorpionknifes]

I'm not certain what's the best way forward here.

I would prefer returning a context.Background() instead of init it with a default context. As defaulting it with a context may incentivise users to pass in a context from a trace that won't be related to the log line that does not have ctx passed in.

[flc1125]

I will handle it as follows. Do you have any good suggestions?

• I will set the default ctx value to context.Background();
• To facilitate the use of a custom ctx, I will add a WithContext(ctx) method to implement it;
• To facilitate the context association for tracing, I will add a WithContext(ctx) method in *LogSink to pass in and return a new *LogSink instance.