[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: https://github.com/golang/protobuf/issues/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: https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/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 https://github.com/checkpoint-restore/go-criu/issues/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.