Extract github.com/go-kit/kit/log to github.com/go-kit/log

[peterbourgon]

I have several use cases, now, where it would be beneficial if projects could use Go kit's package log without pulling in anything else. I would like to extract it to it's own module and package, tentatively named kitlog to avoid the name collision. And then I would like to point the existing package at it, maybe with type aliases if they are up for the task.

@ChrisHines — Do you think this is a reasonable idea, and is feasible? Can you identify any blockers?

[ChrisHines]

I support the idea in principle and would be happy to help make it happen. I have not yet thought it through enough to identify any blockers, but I have a hard time imagining any from a code or Go module perspective.

If we move it to a separate repo what happens to the issue history? Clearly it can stay in the go-kit/kit repo in the short term. It would be nice to eventually have the new repo be self contained though. I have migrated git commit histories while splitting a repo in the past, but I haven't migrated issues, PRs, etc. I don't know what tooling might be available or how well it works. I have lots of questions about that.

[basvanbeek]

I really like this idea and it has been a blocker for one of my teams in the past to use Go kit's log.

This can also provide us some experience if we want to refactor certain pieces to also live in their own repo's. Observability and Tracing instrumentation bridges come to mind.

[peterbourgon]

I'm sure there are tools that would let us extract and transplant the commit history from the kit repo, I'll look into that.

On further thought, it might not be necessary to alias kit/log to kitlog, we could just freeze the kit/log package and point folks to the new one in the documentation. Thoughts?

@basvanbeek Yep, I think the ultimate goal would be to extract all of the major units of functionality into their own repos/modules, and I think go-kit/tracing would be a good candidate for the next one.

How do people feel about the name kitlog? How annoying is it to deal with the current name conflict?

[ChrisHines]

On further thought, it might not be necessary to alias kit/log to kitlog, we could just freeze the kit/log package and point folks to the new one in the documentation. Thoughts?

That is an option. It might be OK given the stability of the log/** packages, but it feels like a fall back position if other approaches become too cumbersome.

How do people feel about the name kitlog? How annoying is it to deal with the current name conflict?

My first reaction is negative to kitlog due to the increased length, but maybe it's OK. Existing code adopting the new package could use an import alias to minimize code churn during migration. I think I need to try some examples to get a feel for it.

The existing name conflict with the stdlib log package only seems to impact me when first adding logging to some code and goimports type tools tend add the stdlib package by default. Once the import statement is corrected I don't find it a problem since all the functions names are different. The number of files that import both packages tends to be very small in my experience, hopefully only one place in a project if it needs to set up a StdlibAdapter.

[peterbourgon]

After some thought I think I'm actually -1 on kitlog — I think the benefits of using log. namespace in code significantly outweigh the cost of conflicting with the stdlib package.

[ChrisHines]

We should also consider giving the log subtree into its own go.mod as an alternative to moving it into a separate repo.

Pros:

• No change of import paths, code, or issue tracking.
• We can tag it v1.0.0 separate from the rest of Go kit.
• Least work required.
• All the benefits of it being a separate module.

Cons:

• Requires discipline in git tag naming (e.g. log/v1.0.0).

Personally I think this maybe the best approach unless there is a downside I haven't thought of yet.

[peterbourgon]

There are enough caveats and complexities involved with multi-module repos that I would not willingly opt in to that unless I had no other choice.

[ChrisHines]

Thanks for the link. I hadn't read through that page in a while. I agree that in the current context there are some big downsides to making log a submodule. The most scary part is described in the section "Is it possible to add a module to a multi-module repository?". The money quote is:

The second class: the path at which the module is being added is in version control and contains one or more existing packages. This case requires a considerable amount of care.

IMO, the main show stopper is that we would have to create a fake dependency from log to the main go-kit/kit module to avoid triggering an “ambiguous import” error at compile time.

So yes, after further consideration I agree a submodule is not such a great idea given that the log package is already published as part of the main module.

[sagikazarmark]

I'm sure there are tools that would let us extract and transplant the commit history from the kit repo, I'll look into that.

That can easily be done with just git. I had a similar task once and I blogged about it here. If you need assistance with the split, I can help.

[andig]

That can easily be done with just git. I had a similar task once and I blogged about it here. If you need assistance with the split, I can help.

With what @sagikazarmark mentioned it should be possible to transfer github.com/go-kit/kit/log to github.com/go-kit/log given that

There are enough caveats and complexities involved with multi-module repos that I would not willingly opt in to that unless I had no other choice.

rules out the mono-repo solution. In addition, github.com/go-kit/kit/log should be deprecated and no longer referenced from go-kit.

What would it take to move this forward? Anything external contributors could do?

[peterbourgon]

go-kit/log#2

[sagikazarmark]

What's the next step? Do you plan to introduce larger changes to the extracted log library? What about backwards compatibility? Do you plan to keep the log library in the main module as well?

[andig]

Imho the following is needed for a viable migration path:

• un-prerelease go-kit/log
• make go-kit/kit depend on go-kit/log
• deprecate go-kit/kit/log
• in a later release remove go-kit/kit/log

Imho deprecation and possibly removal are needed to ensure we'll get the required transition to go-kit/log

[peterbourgon]

I intend to do that first step in the next few days, and I'll mark go-kit/log as deprecated in some way, but I won't be doing anything stronger than that. People who want to keep using go-kit/log in its current form may continue doing so indefinitely.

[sagikazarmark]

Adding Deprecated: use X instead to a comment above the logger interface will trigger certain linters.

[peterbourgon]

Yeah, and I don't think I want to do that. I'll probably just add a paragraph or something to the docs.

[ChrisHines]

If we are going to maintain both packages in perpetuity, what is the plan in terms of responding to bug reports or feature requests (which, thankfully are not high volume)? I would like to have a clearly documented plan for both packages that I understand before we pull the trigger on a full release of the new package.

[peterbourgon]

I don't intend to maintain kit/log after this. Maybe the official deprecation flag is the right thing to do.

[ChrisHines]

OK. What about the other parts of kit that import kit/log? Should we update them to import this package? Based on what's been said so far that would seem like the best plan. That would give us a sequence like:

1. Release go-kit/log
2. Update go-kit/kit import paths to go-kit/log and tag a release
3. Deprecate go-kit/kit/log
   • ? Sometime later remove go-kit/kit/log from the kit repo ?, or
   • ? Update go-kit/kit/log to use go-kit/log (maybe with type aliases too?) ?

I'm not sure which end game for go-kit/kit/log provides the best user experience. Personally I want to make sure that we don't cause a lot of problems for existing users with whatever we do there. Is it OK to ask people to change the import path at some point in the future when they upgrade to a new version of go-kit/kit? If so, when would we do that, and how would we communicate it?

[peterbourgon]

Ooh, good questions. Let me ponder.

[andig]

Note: README still contains some references to kitlog.

[ChrisHines]

Note: README still contains some references to kitlog.

The only remaining references to go-kit/kit/log I could find in the README are at the very bottom in the "Benchmarks & comparisons" section. Those references are correct since the two repositories mentioned still import kit/log.

It looks like github.com/imkira/go-loggers-bench is still accepting PRs as recently as a few months ago, so maybe we could submit a PR to update the import path in that repo.

It looks like github.com/uber-common/zap is now go.uber.org/zap (hosted at https://github.com/uber-go/zap), so our README has the wrong URL (although Github still redirects to the new location). I don't know if they would take a PR to update the benchmark code or not.

Personally I feel like maybe we should just remove that section of the README. My sense is that the Go community has moved on from caring too much about the performance of logging packages.

[peterbourgon]

I'd like to release go-kit/log as step one, and then do any migrations in go-kit/kit afterwards. Beyond tagging a non-alpha v0.0.1 and updating the README, is there anything else that should happen? /cc @ChrisHines @sagikazarmark

[ChrisHines]

I agree with releasing the new repo before changing anything in go-kit/kit is the right approach. We'll need a proper release of the new module before anything else can depend on it anyway. I think updating the README is the only task left before making a release.

What version will we call the new release?

[sagikazarmark]

Quickly went through the docs and the readme, everything looks fine (apart from the notice in the readme).

Any reason not to tag a minor version, eg. v0.1.0?

[andig]

I would still suggest to make the log README consistent in their usage of log and kitlog package names.
UPDATE I withdraw this comment. It's necessary in the context of replacing the stdlib logger.

[peterbourgon]

@ChrisHines @sagikazarmark I would just do v0.0.1 by default, given the current thing is v0.0.1-alpha. But if there's a rationale for something different I'm all ears — would v0.1.0 make more sense by some rule of semver? (It will stay at major version 0 forever, if that affects the decision.)

[sagikazarmark]

I don't think it matters that much. IMO v0.0.x usually means that it's a very early project tagged only to put something out there whereas a minor version sends the message that it's somewhat ready to use.

In terms of semver, the last segment should only ever be used for patch releases that fixes something in a previous minor release which is not the case.

tl;dr v0.1.0 sends a better message about the reliability of the package in my opinion, that's all.

[ChrisHines]

@sagikazarmark said:

v0.1.0 sends a better message about the reliability of the package in my opinion, that's all.

I tend to agree, but I don't care that much for the initial tagging.

@peterbourgon said:

It will stay at major version 0 forever, if that affects the decision.

👎

I understand the reluctance to tag v1.x.x as it relates to the frustration many have with forcing backwards compatibility concerns and not wanting to deal with v2+ modules. But this module seems like a good candidate for v1 since the API has been stable for years and I didn't think we had any plans to change it.

I guess moving the packages to their own repo with a new module path creates an opportunity to make some changes without breaking the numerous dependents that use kit/log, but I did not think that was the point of the new repo. If that's why this is getting extracted then we have to be more circumspect about deprecating kit/log or changing it to depend on the new module.

Please explain why you would like to keep this v0 forever?

[peterbourgon]

Don't fret — I have no plans to make any breaking changes, and would object to any breaking changes being made by others.

In short, Go modules, and specifically Semantic Import Versioning, makes incorrect and invalid assumptions about what a major semantic version means. In my opinion, the consequences are severe enough that it is better, on balance, to opt out entirely.

But I actually mis-spoke in my previous comment. I'll be happy to jump to a stable major version if and when SIV is made optional.

[peterbourgon]

tl;dr v0.1.0 sends a better message about the reliability of the package in my opinion, that's all.

Absent any intervention, I'll update the kit/log README to remove the giant disclaimer, and tag v0.1.0, tomorrow.

[peterbourgon]

Done. I initially intended to keep both packages around indefinitely; that may be a bad idea? Separate discussion, in any case.