Implement logr.LogSink for GingkoWriter

[Nuckal777]

Hey 👋,

I use logr in some projects and use Ginkgo for testing. GingkoWriter offers the option to record some output of failing test. Implementing LogSink allows failing Ginkgo tests to outright record what is to be logged anyway, which helps investigating the cause. Adding the glue code would be convenient.

Instead of implementing LogSink directly, the implementation could be wrapped in an other struct. As I have a basic implementation around, I may work on this.

[onsi]

I think I'm following. Would the idea be to provide a struct that wraps GinkgoWriter but implements LogSink so that code under test that expects a LogSink will have one?

[Nuckal777]

Yes, some GinkgoLogr struct, which implements LogSink and writes to the global GinkgoWriter, would be great. So we can do things in tests like:

// next, err := op.Transition(plugin.Parameters{Log: logr.Discard()}, &Data{})
next, err := op.Transition(plugin.Parameters{Log: GinkgoLogr{}}, &Data{})

[onsi]

ah ok, got it - and you'd want it in Ginkgo proper so you could avoid having to pass in GinkgoWriter every time on construction? Do you, generally, always pass in a new struct/instance when adopting logr? Or can you have one shared logs that you pass in everywhere?

[Nuckal777]

I would like to have a logr.LogSink implementation, which writes to GinkgoWriter, in Ginkgo, because logr is quite widely used. GitHub lists 32k dependent packages. At least kubebuilder and kubernetes-sigs/controller-runtime bootstrap logr and Ginkgo together. Certainly, the glue code itself could also live fine in a separate repo.

When I set logr.Loggers in tests I give each struct/function requiring it a new instance. I think using a global shared instance to pass to structs/functions could also work out.

[onsi]

👍 if you're up for working on a PR I'd be happy to pull something in.