-
Notifications
You must be signed in to change notification settings - Fork 69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace lager.Logger with log/slog.Logger #267
Comments
We have created an issue in Pivotal Tracker to manage this: https://www.pivotaltracker.com/story/show/185850722 The labels on this github issue will be updated when the story is started. |
I think this is the right strategic direction. The earliest I would want to do this is when Go 1.20 goes out of support, as until that point users would expect I recently created a PR against func foo(slogLogger *slog.Logger) {
lagerLogger := lager.NewLogger("my-lager-logger")
lagerLogger.RegisterSink(lager.NewSlogSink(slogLogger))
brokerapi.New(Broker{}, lagerLogger, Creds{})
... It doesn't get rid of the dependency on |
Regarding the timeline I would agree. Regarding the logger and in context of SBOMs I would see it as a problem if every dependency would add their favorite logging framework (version) to the game. Today I could write a wrapper on my own - but I can’t remove the dependency. |
## Breaking Changes This package now accepts a `*slog.Logger` from the Go standard library, rather than a [Lager logger](https://github.com/cloudfoundry/lager). 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](https://github.com/cloudfoundry/lager/releases/tag/v3.0.3) for example: ```go 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`](https://github.com/cloudfoundry/lager) logger. In Go 1.21, structured logging was introduced into the Go standard library via the [`log/slog`](https://pkg.go.dev/log/slog) package, and `slog` [compatability was added](cloudfoundry/lager@4bf4955) 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
Is your feature request related to a problem? Please describe.
At the moment the brokerapi requires an import of
lager.Logger
as another third party dependency.As the
lager.Logger
interface containsLogFormat
inSink
, it can't be implemented without the dependency to lager.Logger.I would like to avoid the dependency to another third party dependency.
Describe the solution you'd like
Replace
lager.Logger
with an own interface or uselog/slog.Logger
as a solution from the standard library.Describe alternatives you've considered
No response
Additional context
No response
The text was updated successfully, but these errors were encountered: