[6.14.0] Proto registration conflict caused by kv.proto

[berndverst]

Can you please prefix your proto names with something like aerospike_ ?

Proto names are global, and kv.proto is very generic and can easily clash with other tools. See: golang/protobuf#1122

I'm one of the maintainers of github.com/dapr (dapr.io) which includes an aerospike integration. I cannot upgrade to 6.0.14 due to this issue.

[khaf]

Argh. Sorry I did not know about this limitation in go-grpc. I'll take care of it in the next release. It may take a bit since I need to test it against a few other tools we have.

[codeboten]

@khaf will an update on this be part of the next patch release?

[khaf]

Sorry I missed this issue for the last release. I need to look into how this can be addressed. Is it only a client issue or does the shared manifest on both Client and Server need to change?

[TylerHelmuth]

It looks like this issue has been resolved in v7, is that correct?

[khaf]

I backported the fix from the v7 to the v6. Hopefully this will address the issue.

[conradsmi]

The fix partially addressed the issue, but two files were not regenerated after the name change was applied; see auth.pb.go and kv.pb.go. Regenerating these caused the name conflict issue to disappear on my end.

[khaf]

This issue should have been fixed in the v7.2.0 release. I'm closing the ticket, feel free to reopen if the issue still persists.

[TylerHelmuth]

@khaf we are still seeing a conflict: open-telemetry/opentelemetry-collector-contrib#32108

[conradsmi]

I did not see any conflicts after upgrading to v7.2.1 (I assume 7.2.0 would work as well though, just did the bump to latest)

[khaf]

@TylerHelmuth How does the conflict manifest itself? Can you help me reproduce it so that I can verify the fix for good?

[djaglowski]

@khaf, we're still unable to upgrade the library in the OpenTelemetry Collector and ultimately are having to evaluate whether to remove the associated component.

According to the comment here, there is a namespace conflict with https://github.com/checkpoint-restore/go-criu. @antonblock, do you recall how you determined this?

I looked into the other library and found this issue checkpoint-restore/go-criu#94, where they apparently had a similar problem with yet another library. That issue links to a series of PRs which appear to have resolved the problem, so perhaps those would be helpful.

@khaf, would you be willing to reopen the issue?

[khaf]

I am absolutely willing to reopen it. The issue is that I do not know how to reproduce the issue on my end. Every solution and fix that I have tried has been reported by others. Is there a way I can build and encounter the issue on my end so that I can assure that I have the fix.

PS. The file rename solution seems to not have resolved the issue, since that has already been applied.

[djaglowski]

I was able to hack together a unit test which produces the error by creating an aerospike client and then calling criu.MakeCriu().

import (
	as "github.com/aerospike/aerospike-client-go/v7"
	criu "github.com/checkpoint-restore/go-criu/v5"
	"github.com/stretchr/testify/require"
	"github.com/testcontainers/testcontainers-go"
)

func TestProtoCollision(t *testing.T) {
	c, err := testcontainers.GenericContainer(
		context.Background(),
		testcontainers.GenericContainerRequest{
			ContainerRequest: testcontainers.ContainerRequest{
				Image: "aerospike:ce-6.2.0.7_1",
			},
			Started: true,
		})
	require.NoError(t, err)
	defer c.Terminate(context.Background())
	
	asClient, err := as.NewClient("0.0.0.0", 3000)
	require.NoError(t, err)
	defer asClient.Close()
	
	cr := criu.MakeCriu()
	defer cr.Cleanup()
}

Output includes:

panic: proto: file "aerospike_proxy_kv.proto" has a name conflict over DEFAULT
	previously from: "github.com/checkpoint-restore/go-criu/v5/rpc"
	currently from:  "github.com/aerospike/aerospike-client-go/v7/proto/kvs"

[khaf]

@djaglowski As I started working on this, it quickly became obvious that this is not a simple file name conflict anymore, but a namespace one. Unfortunately, this is not something that is easy to resolve without defining a namespace in proto files, consequently breaking the production programs for customers. That would require a long discussion internally between multiple teams which may take months.

The easiest way I could resolve this issue for now is to provide a build flag to disable the gRPC completely, circumventing the problem. Is that something you would be interested in?

[djaglowski]

@khaf, thanks for looking into it. What effect will disabling gRPC completely have on the client?

[khaf]

@djaglowski gRPC is only used in a specific client mode used for our Database As A Service. In normal operation that mode can be excluded. I can provide a build tag to exclude that mode on build and work around this issue until we find a possible permanent resolution.

[djaglowski]

Thanks @khaf, that would be helpful.

[khaf]

@djaglowski The latest Go client release (v7.6.0) should have addressed the issue you are facing. You don't need any changes on your end. Just a recompile should suffice.

[djaglowski]

@khaf, thank you! I can confirm it works on our end. Really appreciate it.