log: break out the backends?

[peterbourgon]

The question was raised whether we ought to break out the various logging backends to their own packages underneath package log, similar to package metrics. I was initially for the idea, but in mocking out the UX, realized it would have the effect of changing the relatively-nice

import (
    "github.com/go-kit/kit/log"
)

var logger log.Logger
logger = log.NewJSONLogger(os.Stdout)
logger = log.NewContext(logger).With("foo", 123)

to the slightly more awkward

import (
    "github.com/go-kit/kit/log"
    jsonlog "github.com/go-kit/kit/log/json"
)

var logger log.Logger
logger = jsonlog.New(os.Stdout)
logger = log.NewContext(logger).With("foo", 123)

The same critique applies to package metrics, but in that case we have considerably different construction semantics for each of the backends. In this case, packages seem to act purely as namespaces, which I guess is an antipattern.

Thoughts?

[basvanbeek]

I'm not sure either.

I see the ease of use benefit of keeping everything in a single package but it discourages regarding kit/log as an open standard logging interface to which various logging backends can be implemented (some of them hosted within our repository, others maybe outside of it).

As previously mentioned it would not be a first (kit/metrics) and now with our support for kit/tracing/opentracing we have a similar approach there.

[ChrisHines]

I think this current situation is acceptable for now. I suggested the idea in the context of adding additional Logger implementations that could add more external dependencies to kit/log. My thinking is that organizations or applications will most likely standardize on a single log output format. It would be unfortunate if vendoring kit/log meant vendoring many unneeded dependencies for unused backends.

[ChrisHines]

The change in the UX doesn't bother me much. It's not that bad, and it likely will only appear once per application.

[peterbourgon]

OK. Perhaps we can try making this change as part of #189?

[ChrisHines]

@peterbourgon Should we do this for v1.0.0 since this is a breaking change?

[peterbourgon]

If we decided it was a good idea, yes. But the couple of times I prototyped it, I didn't end up convinced it was a good idea, given each backend just exposes 1 constructor. I think I'd rather close this ticket. WDYT?

[seh]

Another cost to consider: The unit tests for kit/log make use of various backends. Now, those tests sit in the dedicated and separate package "log_test," so they could pull in a backend or two, but it's annoying—and suspicious—to have to test the "log" package by pulling in implementations of the package's exported interfaces.

I suppose that database/sql has a similar challenge with its drivers.

[willfaught]

Go Team guidance is if there are only few things in a separate package, it isn't worth it, so I agree.

…

On Mon, Feb 20, 2017 at 11:43 PM Peter Bourgon ***@***.***> wrote: If we decided it was a good idea, yes. But the couple of times I prototyped it, I didn't end up convinced it was a good idea, given each backend just exposes 1 constructor. I think I'd rather close this ticket. WDYT? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#274 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAD5Vguchnbz-OdhsvuP-kQCEfGpwsxWks5repWXgaJpZM4IjvHA> .