Split go.opentelemetry.io/proto module to exclude grpc dependencies

[utezduyar]

The generated files in the collector/ have 3 parts. The data structure, grpc client and grpc server. They are all packed in the same module file. grpc dependencies are brought in and compiled in the output even if the client is only using http/otlp. The go compiler cannot decide if the grpc code is in use or not (my guess is due to some interfaces) therefore my suggestion is to solve it in the go package layer.

The relevant thread is open-telemetry/opentelemetry-go#2579

I suggest to split the data structure files in one module, grpc files in another module. This would be of course backwards compatible change.

[pellared]

Creating a go.opentelemetry.io/proto/otlp/collector module would be a breaking change as the new go.opentelemetry.io/proto/otlp will lack the gRPC services code.

This would be problematic for someone who would try to only bump go.opentelemetry.io/proto/otlp. The user would need to additionally add go.opentelemetry.io/proto/otlp/collector to their go.mod. Otherwise e.g. otlptracegrpc will not build.

The alternatives I see are:

• making this change as a v2 release (my preference)
• creating a new module e.g. go.opentelemetry.io/proto/light/otlp that will contain only protobuf types without gRPC services (I am opposed to it as we would need to maintain more code and it may be confusing for the readers why they are different possibilities)

CC @open-telemetry/go-approvers @open-telemetry/collector-approvers

[MrAlias]

Would the idea be that we keep up to date the v1, add a v2 without the gRPC dependence and also keep that up to date, and add a collector package to hold the server definitions?

How long will the v1 be supported?

[pellared]

The idea is to:

• create go.opentelemetry.io/proto/otlp/v2 and go.opentelemetry.io/proto/otlp/collector/v2 modules (v2)
• deprecate and stop maitaining go.opentelemetry.io/proto/otlp module (v1)

[MrAlias]

The idea is to:

• create go.opentelemetry.io/proto/otlp/v2 and go.opentelemetry.io/proto/otlp/collector/v2 modules (v2)
• deprecate and stop maitaining go.opentelemetry.io/proto/otlp module (v1)

Where will the server definitions be defined?

[pellared]

Where will the server definitions be defined?

go.opentelemetry.io/proto/otlp/collector/v2

[MrAlias]

Aren't they already defined in go.opentelemetry.io/proto/otlp/collector? Why does that package need a v2?

[pellared]

I think it would be inconvenient to support v1 and v2 at the same time.

I would rather prefer separate go module like go.opentelemetry.io/proto/otlp/light.

However, there is also an issue from maitainance perspective that we would need to set the go_package proto options using CLI args rather than in proto definition (which we could simply update if we go the v2 route).

I can try experimenting and showcase it on a fork.

[pellared]

The alternatives I see are:

• making this change as a v2 release (my preference)

I created #154

• creating a new module e.g. go.opentelemetry.io/proto/light/otlp that will contain only protobuf types without gRPC services (I am opposed to it as we would need to maintain more code and it may be confusing for the readers why they are different possibilities)

I created #155

[tigrannajaryan]

I think it would be inconvenient to support v1 and v2 at the same time.

@pellared How do you plan to support both at the same time after this PR is merged? Any subsequent proto changes will only apply to v2, no? You won't be able to generate v1 code anymore. So v1 becomes obsolete as soon as any other change to the proto is made.

Am I missing something?

[tigrannajaryan]

An alternate way that does not require any changes to the proto repo: patch the proto files locally in this repo. That would allow you to continue supporting both v1 and v2 at the same time from the same proto file even if the proto files evolve. You can generate twice, once per version.

We have a precedent for patching protos before generation: the Collector does it.

[pellared]

@tigrannajaryan

@pellared How do you plan to support both at the same time open-telemetry/opentelemetry-proto#532 is merged? Any subsequent proto changes will only apply to v2, no? You won't be able to generate v1 code anymore. So v1 becomes obsolete as soon as any other change to the proto is made.

Correct. I described in #154 that the v1 would be deprecated.

An alternate way that does not require any changes to the proto repo: patch the proto files locally in this repo. That would allow you to continue supporting both v1 and v2 at the same time from the same proto file even if the proto files evolve. You can generate twice, once per version.

If I understand correctly this is exactly what I propose here #155.

I have no opinion which approach is better. Each has its pros and cons.

EDIT: This #155 is probably better as:

1. It does not require any changes from consumers using OTLP over gRPC.
2. For these consumers, the v2 approach would lead to depend on 2 modules instead of 1.
3. There is a precedent in Collector.

[pellared]

Go SIG meeting:
Postponing the proposal (focusing on Go Logs SDK).