add gRPC support #434

akoserwal · 2025-08-14T14:03:09Z

closes #433

Add gRPC support

akoserwal · 2025-08-20T16:21:56Z

One issue with the plugin is that it requires the gRPC server to implement pb.UnimplementedBentoIngressServer to publish and subscribe, which is not feasible in most cases, as it necessitates additional code changes in the application server code. I will replace the generic implementation, which will work without any code changes to the server or receiver client.

jem-davies · 2025-08-20T16:52:12Z

One issue with the plugin is that it requires the gRPC server to implement pb.UnimplementedBentoIngressServer to publish and subscribe, which is not feasible in most cases, as it necessitates additional code changes in the application server code. I will replace the generic implementation, which will work without any code changes to the server or receiver client.

This has been on my want-to-have list for a while! Thanks so much for the effort with the PR. I am currently reading up on gRPC - then I will give the PR a good review.

From my initial thoughts - and what you pertain to in your comment - would it make sense to follow how Bento works with http, we have a http_client and http_server inputs & outputs - so like grpc_client & grpc_server. What we have on this PR would be the grpc_client I am thinking.

When you say you "will replace the generic implementation" - is that a change you wish to make in a follow-up or change on this PR?

akoserwal · 2025-08-20T17:03:04Z

One issue with the plugin is that it requires the gRPC server to implement pb.UnimplementedBentoIngressServer to publish and subscribe, which is not feasible in most cases, as it necessitates additional code changes in the application server code. I will replace the generic implementation, which will work without any code changes to the server or receiver client.

This has been on my want-to-have list for a while! Thanks so much for the effort with the PR. I am currently reading up on gRPC - then I will give the PR a good review.

From my initial thoughts - and what you pertain to in your comment - would it make sense to follow how Bento works with http, we have a http_client and http_server inputs & outputs - so like grpc_client & grpc_server. What we have on this PR would be the grpc_client I am thinking.

When you say you "will replace the generic implementation" - is that a change you wish to make in a follow-up or change on this PR?

yes

jem-davies · 2025-08-20T17:12:54Z

yes

Which?

akoserwal · 2025-08-20T17:24:49Z

yes

Which? sorry, I can create a follow up pr, that will be better as it will new changes.

jem-davies

I think that I would want to resolve some of the obvious issues - then spend more time looking at the implementation.

It has been on my radar to do some gRPC components for Bento - I think that we might want to think about if this approach is the right one - and if we could do something more dynamic so we don't have to use protoc to generate go source at compile time, then have the user stuck with a vague protobuf schema. If we could take the approach like https://github.com/fullstorydev/grpcurl so it's more dynamic.

I am thinking that I will put effort into writing a suite of gRPC components. That will maybe will work with the 4 types of request/response setups: unary RPCs, Server streaming RPCs, Client streaming RPCs and Bidirectional streaming RPCs.

jem-davies · 2025-08-19T14:24:37Z

internal/impl/grpc/register.go

+
+func registerGRPCInput() error {
+	spec := service.NewConfigSpec().
+		Version("1.6.0").


Suggested change

Version("1.6.0").

Version("1.11.0").

jem-davies · 2025-08-19T14:24:55Z

internal/impl/grpc/register.go

+
+func registerGRPCOutput() error {
+	spec := service.NewConfigSpec().
+		Version("1.6.0").


Suggested change

Version("1.6.0").

Version("1.11.0").

jem-davies · 2025-08-19T14:47:01Z

Makefile

+	@GOBIN=$(GOPATH_BIN) $(GO) install google.golang.org/protobuf/cmd/protoc-gen-go@latest
+	@GOBIN=$(GOPATH_BIN) $(GO) install google.golang.org/grpc/cmd/protoc-gen-go-grpc@latest


Think we should pin these deps rather than using the latest tag

jem-davies · 2025-08-20T17:36:54Z

internal/impl/grpc/grpc_dialopts.go

+		tlsCfg := &tls.Config{MinVersion: tls.VersionTLS12}
+		b.opts = append(b.opts, grpc.WithTransportCredentials(credentials.NewTLS(tlsCfg)))


This whole tls setup is really confusing - seems like we use tlsConf, tlsEnabled, _ := conf.FieldTLSToggled("tls") in the constructor but then here we are just replacing the tlsConf with &tls.Config{MinVersion: tls.VersionTLS12}.

jem-davies · 2025-08-26T13:18:40Z

internal/impl/grpc/init.go

I think that it would be better if we altered the PR such that we have input_grpc_client.go & output_grpc_client.go files that:

register the component in init()

define the component spec & constructor

So effectively removing the internal/impl/grpc/init.go & internal/impl/grpc/register.go. It's just the way the vast majority of plugins are implemented - not that this way is flawed.

jem-davies · 2025-08-26T13:51:00Z

internal/impl/grpc/register.go

+`,
+	)
+
+	return service.RegisterOutput("grpc", spec, func(conf *service.ParsedConfig, res *service.Resources) (service.Output, int, error) {


Suggested change

return service.RegisterOutput("grpc", spec, func(conf *service.ParsedConfig, res *service.Resources) (service.Output, int, error) {

return service.RegisterOutput("grpc_client", spec, func(conf *service.ParsedConfig, res *service.Resources) (service.Output, int, error) {

jem-davies · 2025-08-26T17:00:40Z

internal/impl/grpc/grpc_dialopts.go

+	auth                authConfig
+}
+
+func buildDialOptions(_ context.Context, useTLS bool, cfg clientOpts) ([]grpc.DialOption, error) {


How come we have a ctx in the call signature if we aren't using it?

jem-davies · 2025-08-26T17:27:59Z

internal/impl/grpc/grpc_dialopts.go

Not sure if we need this builder pattern - can't we just create a []grpc.DialOption in the constructor?

jem-davies · 2025-08-26T17:29:42Z

internal/impl/grpc/grpc_input.go

+	_ = ctx
+	time.Sleep(5 * time.Millisecond)


Would need to properly handle context termination, and tidy up any created resources properly.

jem-davies · 2025-08-26T17:34:09Z

internal/impl/grpc/register.go

+		Categories("Services").
+		Summary("Receive messages from a gRPC server-side stream using a simple proto contract.").
+		Description("Subscribes to a server-side stream and emits each received frame as a message. Message body is taken from Frame.data and metadata from Frame.metadata. Supports TLS, bearer token auth, custom per-RPC headers, keepalive parameters, and client options such as authority, user agent, and load balancing policy.").
+		Field(service.NewStringField("address").Description("Server address to connect to, e.g. host:port").Default("127.0.0.1:50051")).


Would need to add descriptions to the fields, and more description for the components themselves ideally. Would want to talk about the Frame schema etc...

akoserwal · 2025-08-26T18:11:45Z

@jem-davies I have a separate implementation on the way, I will have a pr open for it. I would suggest wait until I have new Pr open with changes.

jem-davies · 2025-08-27T12:26:04Z

@jem-davies I have a separate implementation on the way, I will have a pr open for it. I would suggest wait until I have new Pr open with changes.

Ok - going to mark this PR as draft

jem-davies · 2025-09-03T12:30:47Z

Closed for #458

add gRPC support

b9cd9bd

akoserwal requested review from gregfurman and jem-davies as code owners August 14, 2025 14:03

akoserwal added 2 commits August 14, 2025 20:40

fixing test

8e4e937

add proto config and installation commands

3dac69f

jem-davies self-assigned this Aug 19, 2025

jem-davies requested changes Aug 26, 2025

View reviewed changes

jem-davies marked this pull request as draft August 27, 2025 12:26

jem-davies removed their assignment Aug 27, 2025

jem-davies closed this Sep 3, 2025

		@GOBIN=$(GOPATH_BIN) $(GO) install google.golang.org/protobuf/cmd/protoc-gen-go@latest
		@GOBIN=$(GOPATH_BIN) $(GO) install google.golang.org/grpc/cmd/protoc-gen-go-grpc@latest

		tlsCfg := &tls.Config{MinVersion: tls.VersionTLS12}
		b.opts = append(b.opts, grpc.WithTransportCredentials(credentials.NewTLS(tlsCfg)))

	return service.RegisterOutput("grpc", spec, func(conf service.ParsedConfig, res service.Resources) (service.Output, int, error) {
	return service.RegisterOutput("grpc_client", spec, func(conf service.ParsedConfig, res service.Resources) (service.Output, int, error) {

Uh oh!

add gRPC support #434

add gRPC support #434

Uh oh!

Conversation

akoserwal commented Aug 14, 2025

Uh oh!

akoserwal commented Aug 20, 2025

Uh oh!

jem-davies commented Aug 20, 2025

Uh oh!

akoserwal commented Aug 20, 2025

Uh oh!

jem-davies commented Aug 20, 2025

Uh oh!

akoserwal commented Aug 20, 2025

Uh oh!

jem-davies left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akoserwal commented Aug 26, 2025

Uh oh!

jem-davies commented Aug 27, 2025

Uh oh!

jem-davies commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jem-davies left a comment •

edited

Loading

jem-davies commented Sep 3, 2025 •

edited

Loading