feat!: introduce logger choice via slog

[blgm]

Breaking Changes

This package now accepts a *slog.Logger from the Go standard library, rather than a Lager logger. This allows the use of alternative loggers.

• This package no longer requires you to import code.cloudfoundry.org/lager/v3.
• The constructors New(), NewWithCustomAuth(), NewWithOptions(), and also AttachRoutes() all take a *slog.Logger
• apiresponses.FailureResponse errors with a ValidatedStatusCode() method also take a *slog.Logger rather than a Lager logger
• The middleware middlewares.APIVersionMiddleware has had the LoggerFactory field removed, and a new field Logger added with type *slog.Logger.

If you want to continue to use Lager, you can just convert it to a *slog.Logger, for which you will need Lager v3.0.3 for example:

logger := lager.NewLogger("a-lager-logger")
router := brokerapi.New(serviceBroker, slog.New(lager.NewHandler(logger)), credentials)

Reasons

Historically brokerapi has required use of the lager logger. In Go 1.21, structured logging was introduced into the Go standard library via the log/slog package, and slog compatability was added to lager.

brokerapi has been modified to require a slog logger to be passed rather than a lager logger. This allows users a choice of logger. Users who still want to use lager can easily do that using the lager/slog compatability.

And users who want to use slog or an slog-compatible logger can do that instead.

A key advantage is that lager is no longer a dependency of this package, which simplifies package management for apps that use brokerapi and other libraries which use lager.

Resolves #267

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/186934394

The labels on this github issue will be updated when the story is started.

[elgohr]

Wouldn’t it be better to use an interface that matches the slog/Logger, instead of the concrete type?

[blgm]

Wouldn’t it be better to use an interface that matches the slog/Logger, instead of the concrete type?

Good point. That's a good pattern in general for Go. I think I was influenced by an example (that I can no longer find) that I interpreted as it being "normal" to pass a logger concrete type. But the more I think about it, the more it makes sense to take an interface that's defined in brokerapi. (One of the big issues with lager.Loger is that despite being an interface, it's defined in the lager package, so you end up being tightly coupled to lager even if you write another logger that implements the interface.)

I'll have a go at re-working this to make that change.

[blgm]

There is a snag in using the interface approach. We use the With() method which returns a *slog.Logger, so if you define the interface to be:

type Logger interface {
	With(args ...any) Logger
}

Then *slog.Logger does implement this interface because With() has the wrong return type. And the alternative:

type Logger interface {
	With(args ...any) *slog.Logger
}

Is an improvement on a concrete type, but still ties us to the concrete type. But I had been thinking about usages of With() because in this PR they are quite boilerplate-heavy. If we could refactor them away, then we might get something much nicer.

[elgohr]

I'm struggeling at the same spot for the last 30 minutes 😂
Sadly it's go, not swift (where we could just have had an interface extension as a default implementation...)

[elgohr]

I tried to turn https://github.com/pivotal-cf/brokerapi/blob/main/utils/context.go#L46 around, into a context-aware handler, so that we could have all values within the context. But in the end we would need to set the handler, which is also not going to work without slog...

[elgohr]

In the end people have a dependency to the standard library (anyways) and could provide a custom slog.Handler. So let’s keep the slog.Logger for simplicity?

[blgm]

Thanks for trying @elgohr. I also considered not using the With() method which means that you can write an interface that doesn't mention *slog.Logger. But I agree that just taking a concrete *slog.Logger is simpler for now, and if we discover a better fix later, it would not be a breaking change.

[FelisiaM]

I know this didn't exist before but I think we should log the bindingID here as well since it is the binding last operation