go-grpc-middleware.v2: Action Plan.

[bwplotka]

Hi 👋

We have tons of stuff to improve in this library and there wasn't really time for this. Let's do it finally! Let's maintain this issue as a single place to track our work. 💪 cc @domgreen @johanbrandhorst @mwitkow

The plan is to start from scratch in the v2 branch. We will work in the background until it's done.

Work Done

• Started v2 branch, implemented all while leaving same API. Assignee: @bwplotka

• Never again use those weird module_names 🙃 Done Assignee: @bwplotka

• Multi-module architecture, allowing us to avoid dependency hell, but also retaining mono repo. Module structure: Done Assignee: @bwplotka
  The key part is to make sure the core is never importing the provider's code. There has to be indirection. This way the lib has minimum dependencies possible, with core literally having core only.

  If maintaining multi modules will be problematic, we can look into modularise cc @Helcaraxan

TODO (help wanted!) for 2.0:

Blockers:

• Do one more review of all interfaces we expose to check if we can simplify something. Assignee @johanbrandhorst
• Upgrade gRPC and use https://godoc.org/google.golang.org/grpc#ChainUnaryInterceptor Assignee: @bwplotka
• Upgrade golang/protobuf to v2. Same with gogo? Assignee: @bwplotka
• Move to GitHub actions Assignee: @bwplotka
• MIgration docs!
• Start using gorelease (https://godoc.org/golang.org/x/exp/cmd/gorelease)
• Consider adding whitelist behaviour to auth interceptor: How to enable whitelist auth?
• Add tracing interface and move opentracing to provides. Add opentelemetry providers.
• Use the v2 in Thanos for first hand expierience in upgrading. Assignee: @bwplotka

Can be done in 2.x

• Move https://github.com/grpc-ecosystem/go-grpc-prometheus as another provider. Deprecate repo.
• Add failint to avoid not needed imports (e.g v1 packages)
• Move to more maintained validators
• Review if we can simplify retry internally
• Stale bot
• Dependabot

Any other ideas?

[brancz]

I have no strong opinion about essentially having a monorepo, but I do wonder what the benefit is. I think they were separate because they had nothing in common not even shared unit test framework or anything. So I wonder if it’s worth it considering that given that things become harder with modules...

More generally though I do think these things need a pretty major overhaul and would love to see that happen! :)

[bwplotka]

Thanks for the feedback!

I have no strong opinion about essentially having a monorepo, but I do wonder what the benefit is. I think they were separate because they had nothing in common not even shared unit test framework or anything.

Now (as you will see in #276) all middlewares with reuse the majority of its code. Same for test frameworks (: It will be then easier for all maintainers (including Dom, Johan) to understand Prometheus one.

So I wonder if it’s worth it considering that given that things become harder with modules...

Yes, but in the same time we have to solve it for logrus, zap, go-kit and potentially other implementations (: So far it's quite easy to work with multiple modules, but let's see the pain points (:

[brancz]

Yeah loggers would all be in the same repo, I'm more thinking things that are more distinct like logging and metrics middleware. But as I said, I'm not against trying this at all.

[bwplotka]

Not against you mean (:

…

On Mon, 2 Mar 2020 at 09:24, Frederic Branczyk ***@***.***> wrote: Yeah loggers would all be in the same repo, I'm more thinking things that are more distinct like logging and metrics middleware. But as I said, I'm against trying this at all. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#275?email_source=notifications&email_token=ABVA3O7SELUGU3RKVE4CNCDRFN3NZA5CNFSM4K6STMT2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENORY3Y#issuecomment-593304687>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABVA3O344RE4XJQAMC7SRLLRFN3NZANCNFSM4K6STMTQ> .

[bwplotka]

Essentially this is only what all implementations (including Prometheus) will need to implement and test (!):

type ClientReportable interface {
	ClientReporter(ctx context.Context, typ GRPCType, serviceName string, methodName string) (Reporter, context.Context)
}

type ServerReportable interface {
	ServerReporter(ctx context.Context, typ GRPCType, serviceName string, methodName string) (Reporter, context.Context)
}

type Reporter interface {
	PostCall(error, time.Duration)

	PostMsgSend(error, time.Duration)
	PostMsgReceive(error, time.Duration)
}

[brancz]

Not against you mean (:

whops, what a great typo.. :D fixed my comment