Add external logger support

[shell-skrimp]

Hello,

This PR adds support for external loggers via io.Writer. Sample code of it working is below:

package main

import (
	"fmt"
	"log"
	"time"

	"github.com/robfig/cron/v3"
	"go.uber.org/zap"
)

func main() {
	zapLogger, _ := zap.NewDevelopment()

	stdOutLogger := zap.NewStdLog(zapLogger)

	zW := stdOutLogger.Writer()

	el := cron.NewExternalLogger(zW)
	c := cron.New(
		cron.WithChain(cron.SkipIfStillRunning(el)),
		cron.WithSeconds(),
	)

	if _, err := c.AddFunc("* * * * * *", testJob); err != nil {
		log.Fatal(err)
	}

	c.Start()

	time.Sleep(1 * time.Minute)
}

func testJob() {
	time.Sleep(3 * time.Second)

	fmt.Println("job")
}

Should output something like:

2023-10-26T14:46:27.000-0500	INFO	cron/logger.go:76	cron: /home/.../cron/logger.go:76: skip
2023-10-26T14:46:28.000-0500	INFO	cron/logger.go:76	cron: /home/.../cron/logger.go:76: skip
2023-10-26T14:46:29.000-0500	INFO	cron/logger.go:76	cron: /home/.../cron/logger.go:76: skip
job
2023-10-26T14:46:31.000-0500	INFO	cron/logger.go:76	cron: /home/.../cron/logger.go:76: skip
2023-10-26T14:46:32.000-0500	INFO	cron/logger.go:76	cron: /home/.../cron/logger.go:76: skip
job
2023-10-26T14:46:33.000-0500	INFO	cron/logger.go:76	cron: /home/.../cron/logger.go:76: skip

[janrnc]

Hello,
this seems to me an adapter for the zap logger into the required cron.Logger interface. Even though the new method you're proposing takes a generic io.Writer as a parameter, I see this as very use-case-specific. An adapter is defined depending on the specific adaptee structure and IMHO it should not be provided by the package itself. I don't see the need for additional external logger support since (external?) logging is already supported, it's more about aligning APIs.

[shell-skrimp]

The example uses zap, but any other logger that supports io.Writer should also work. While it is true that cron supports its own logger, it doesnt support the native standard library logging interface (which this adds support for). The problem with the current implementation is that a project may incorporate in robfig/cron and have their own logging library (logrus/zap), but because robfig/cron doesnt support unifying the logs the output is drastically different than what would be expected if the end user is logging centrally. For example, the level is different, the timestamps could be different, the output could be in a different format (json); all these are things that outside loggers handle.

Logging libraries provide a way to get an io.Writer out of them so that third party modules (robfig/cron) write to the io.Writer are unified around whatever logger is being used. For example, a list of go loggers that support this:

• logrus: https://pkg.go.dev/github.com/sirupsen/Logrus#readme-logger-as-an-io-writer
• zap: https://pkg.go.dev/go.uber.org/zap#NewStdLog
• slog: https://pkg.go.dev/golang.org/x/exp/slog#NewJSONHandler, https://pkg.go.dev/golang.org/x/exp/slog#NewTextHandler
• zerolog: https://pkg.go.dev/github.com/rs/zerolog#New

The problem here is that the robfig/cron logger is opinionated instead of extensible. This PR makes it extensible so that the end user can use any logger they want.

I also want to point out that other projects do this to allow for unification of logs, for example go-fiber: https://docs.gofiber.io/api/middleware/logger/

It's interesting that you say it's more about aligning APIs; but you don't realize that supporting io.Writer does just that.

[janrnc]

Thank you for your quick reply!

Do you agree that a logger created with

zapLogger, _ := zap.NewDevelopment()
stdOutLogger := zap.NewStdLog(zapLogger)
zW := stdOutLogger.Writer()

logger := cron.VerbosePrintfLogger(log.New(zW, "cron: ", log.Llongfile))

would behave almost the same way as the external logger you are adding? "Almost" because of the extra call to formatTimes().

[shell-skrimp]

You can definitely do it that way to the same affect. If that is preferred method in robfin/cron I would suggest updating docs to reflect that. Especially since that doesnt follow the same pattern as various other projects do (passing an io.Writer instead of wrapping an internal logger interface).

[janrnc]

I'm not a maintainer and hence I cannot make any decision. Just wanted to share my thoughts.

However, I consider very interesting what you pointed out and maybe this could lead to some discussion for a potential logging API refactor proposal.

[shell-skrimp]

Refactor is probably overkill for something like this, especially with projects that use this library. That's why this PR adds additional functionality instead of removing/changing the old.