tap: allow for embedded execution of tap from other go programs

[iameli]

Hi @dholms! Love tap and wanna use it but as you may be aware we at Streamplace are well-known microservice haterz. I wanna embed the thing. Which I think is totally doable, except currently all the code is in the main package which can't be embedded. This commit moves all the code into a top level libtap directory, leaving only the actual CLI-handling urfave/cli stuff in the main package.

Why libtap? Because the binary itself is named tap and so make all would conflict otherwise. Other repos solve that conflict with a pkg subdirectory but I thought maybe moving every file in the repo would be a little excessive. So libtap.

Tested make all and podman build -f ./cmd/tap/Dockerfile -t tap ., looks fine.

[bnewbold]

I want to point out that this would be a pretty big shift in implied maintenance surface for the codebase. We intend to maintain the service and minimize breaking changes to the wire protocol, but exposing all the internal APIs as a supported package is a much bigger lift. I'll let dholms correct me if i'm wrong, but I think we'd be a little hesitant to take that on until we get a bit more experience with this service.

We are also part way through a slow refactor of the SDKs/packages in this repo (indigo). I'd definitely like to have a simple firehose consumer as part of the maintained SDK, and probably some sync1.1 code which maintains account state and handles backfill/resync.

[iameli]

I'm happy to do a version of this that exposes exactly one function. Just let me invoke the thing programmatically rather than needing to run it as a separate binary.

[dholms]

Glad your getting good use out of it! And sympathize with the microservice hate.

I do think that offering this is as a package shifts expectations on API/maintenance even if we're only exposing one function. And as Bryan points out, we're slowly refactoring this repo and I'd like to maintain a similar set of practices for the various services in here. I'm not opposed to offering this (& other services) as packages in principle, but I don't think this is the right time to do so.

Does maintaining a lightweight fork handle your needs for now?

[iameli]

I do think that offering this is as a package shifts expectations on API/maintenance even if we're only exposing one function. And as Bryan points out, we're slowly refactoring this repo and I'd like to maintain a similar set of practices for the various services in here. I'm not opposed to offering this (& other services) as packages in principle, but I don't think this is the right time to do so.

I've taken a pass at a version that only exposes the following interface:

runtap.Run(ctx context.Context, argv []string) error

Everything else is in an internal subdirectory and can't be used as a library. I contend this doesn't do much to increase the API surface area; it's just an alternate entrypoint to the same CLI arguments. This allows embedding use cases like we see in k3s, where you can run k3s kubectl or k3s crictl as subcommands without bundling lots of binaries. This is a strength of the Go ecosystem, IMO.

Does maintaining a lightweight fork handle your needs for now?

No shot, I'm renaming 14 files and changing their import paths. There are merge conflicts just from the three days since I opened this; that'd happen every time we needed to rebase. We just got back to mainline indigo as an upstream dependency, I don't wanna fork again. We'd just keep using our less-good sync implementation until it's available as a package.

[iameli]

Mmyup.

I'll rebase today but continuing to keep this fork maintained is infeasible.

I'm not opposed to offering this (& other services) as packages in principle, but I don't think this is the right time to do so.

Happy to take on the kind of work to make this happen in The Right Way. But would need some guidance on what that is.