feat: add olog package

[jaredallard]

Adds a new olog package with the intent of replacing the existing log
package. Based off of the Go Observability Tooling design document¹.

For those without access to that document, the main goals are encapsulated
below:

• Uses *slog.Logger as the returned logging library (Highly recommended
  read about the library)
• Loggers must be instantiated with New() before a log line can be emitted,
  there is no singleton provided by the olog package.
• Packages should create their own logger(s) as required, storing it as a
  package-level global singleton for ease of use (however, loggers can also
  be created on the fly).
• Default global logging level of Info, which all New() loggers respect.
  Ability to change log level at a module and package level. Adjustable
  through configuration while the process is still running.
• Creating wrapped loggers using a provided *slog.Logger (ideally from
  New()) to enable Outreach-specific functionality, like audit logs.
• Pretty printing of logs by default when os.Stderr is a TTY (local development).
• Automatically log otel-logging compliant TraceID and SpanID if a context with
  trace information is passed to the logger.
  • This will require Context function calls to work.

Expected usage is calling New to create new logger per package. This should
be done either as a global variable per-package, or once per-package and passed
around.

Provides a test helper (NewTestCapturer) that enables callers to capture all log
output for the purpose of checking the output/result of certain operations via logs.
This is primarily intended to be used only for the development of the logger, since
logs are not a stable way to ensure an operation has occurred.

Footnotes

1. https://outreach-io.atlassian.net/wiki/spaces/DT/pages/2853765388/Go+Observability+Tooling+Design#Logging ↩

[george-e-shaw-iv]

Should we preemptively pass functional options to this? Maybe even a context too? Just so we don't have to potentially change function signatures in the future. I guess functional options wouldn't change the signature anyways since those are usually variadic..

But another thing I wanted to call out was the inability to force format. Right now you can't override text/JSON format, it only decides based on environment. We should probably allow this to be overridable.

[jaredallard]

Overriding the default handler would be good! I'll add that.

[jaredallard]

I think we should hold off on options because New() is meant to not be configured right now. Given we can add it without breaking anything later, I think we should hold off until we have a good reason to do so.

[jaredallard]

As for NewWithHandler, that doesn't play into this much because that's very much so meant to be used only for tests (honestly, we might want to unexport it).

[amhester]

Yeah, given I don't see a particular reason to pass context in this New func and that options can be easily added later without really changing it's interface (at least users won't need any immediate change), I don't think this is something we need to immediately address. Also, a lot of what we might want to do as far as configuration can be handled with wrappers or methods if needed.

[amhester]

After doing a lot more digging into what's here, as well as looking into the goals of this more, it seems to me there are 2 things missing or incomplete as of now (though it's possible I'm missing something):

1. Ability to configure log levels per module/package. The registry in this package seems to allow for the possibility of setting and checking of log levels by module/package. However, as far as I can tell, there is currently no way for a caller to actually configure the registry, not to mention that nothing in this package is really setting the global registry's map by default (again, that I can tell). This shouldn't be difficult to change, but would need to determine the best way to expose that functionality to users.
2. The ability to automatically log otel-logging compliant TraceID and SpanID is also not provided by this package, though I feel it could easily add a convenient way for users to do so. Currently it's still possible by manually providing the extracted attributes on each logger instance, or by putting them on a logger per RPC call and persisting those attributes for every log/logger made during that call (slog.With(...)). But it would be nice to wrap the logging handlers to provide either automatic extraction of those context vars onto the log record (when context is provided), or by allowing hooks into that context/record before it sends and providing an out of the box otel extraction hook. The wrapper for this could be very light:

import (
    "context"
    "log/slog"

    "github.com/getoutreach/gobox/pkg/trace"
)

type ContextHandler struct {
	slog.Handler
}

func newContextHandler(handler slog.Handler) *ContextHandler {
	return &ContextHandler{Handler: handler}
}

func (ch *ContextHandler) Handle(ctx context.Context, r slog.Record) error {
	traceID := trace.ID(ctx)
	r.AddAttrs(slog.String("traceId", traceID))
	return ch.Handler.Handle(ctx, r)
}

Just an example

[malept]

The ability to automatically log otel-logging compliant TraceID and SpanID is also not provided by this package

Since the otrace package is yet to be written, I would generally argue that this should be punted until that package has a design doc.

[jaredallard]

1. Ability to configure log levels per module/package. The registry in this package seems to allow for the possibility of setting and checking of log levels by module/package. However, as far as I can tell, there is currently no way for a caller to actually configure the registry, not to mention that nothing in this package is really setting the global registry's map by default (again, that I can tell). This shouldn't be difficult to change, but would need to determine the best way to expose that functionality to users.

Yes, I never implemented that functionality (oops). It was meant to be hooked into stencil-golang such that configuration files could set it. Since the goal was for that to be done through configuration. Also then code would be added to the managed HTTP handler to support setting it at runtime, eventually either using a CLI or just a grpcurl to change it at runtime.

I can't touch on point 2 because I honestly don't remember the context, but figured I'd chime in on 1 since it's an OSS repo, so hi! 👋

[getoutreach-ci-1]

🎉 This PR is included in version 1.88.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀