opentelemetry-rust
opentelemetry-rust copied to clipboard
Proposal: Rename opentelemetry-proto/src/proto/tonic module
As mentioned in https://github.com/open-telemetry/opentelemetry-rust-contrib/pull/134#issuecomment-2587606751, OpenTelemetry use proto defined structs from this tonic module. Despite the name references Tonic, they are pretty generic, not restricted by Tonic scenarios.
Thus, I propose to rename the module to better describe the contents.
https://github.com/open-telemetry/opentelemetry-rust/tree/main/opentelemetry-proto/src/proto/tonic
The first obvious option to me, is just to place the proto/tonic/* files directly in the proto/ folder. This more-or-less follows the name they got from the upstream opentelemetry-proto repo.
@psandana - Agree with the change. Moving directly to proto/ dir, or else to src/generated dir ( so src contains proto, generated and transform dirs). And I think it would make sense to keep it non-breaking, by still keeping the tonic as an alias?
If you prefer to keep tonic as an alias, I think it is ok. Why is the reason to keep it as is? If you still plan to remove, just not now, I would suggest adding a deprecation notice.
Why is the reason to keep it as is? If you still plan to remove, just not now, I would suggest adding a deprecation notice.
Yes, want to deprecate and then remove.
FYI, there is a feature gen-tonic-messages. Shall this be renamed also?
@psandana Sorry for late reply.
FYI, there is a feature gen-tonic-messages. Shall this be renamed also?
Yes, that can be renamed. However, the gRPC code for services within tonic directory will depend on tonic, and we would still need a feature flag containing "tonic" for that.
Also, to add the historical reason behind the existence of tonic directory - earlier both tonic + prost and grpcio + protobuf were supported for generating code, and so there was specific directories for tonic and grpcio. The grpcio code generation was later removed. And then we were left with tonic directory :)
So the change is more complex than expected :D granted. Let see if we can do something meaningful about this during the next weeks.
My proposal to have cleaner design:
-
All Protobuf messages are currently generated under
src/proto/tonic. These should be restructured (logically, not physically) intoproto::opentelemetry::proto::* -
All gRPC service code (ie generated files with naming
*collector*) can remain in the same directory (src/proto/tonic) but be exposed viaproto::tonic::opentelemetry::proto::collector::*, behind thegen-tonicfeature. -
(optional) To reduce confusion about the "tonic" directory containing non-tonic code, we could consider renaming
src/proto/tonicto something more neutral likesrc/proto/generated, while still organizing tonic-specific exports via a mod tonic gate. This keeps file moves minimal while improving clarity. -
gen-tonic-messagesfeature flag to be removed completely. This was only required when bothtonic + prostandgrpcio + protobufwere used, which is no longer a case.
This allows users who only need OTLP types for serialization/deserialization (e.g., over HTTP or file) to avoid unnecessary dependencies like tonic, keeping builds lean and dependency trees minimal.
Also, as a separate improvement: opentelemetry-proto should not depend on opentelemetry-sdk. Instead, the conversion logic (from SDK data types to OTLP wire format) could live in opentelemetry-otlp — keeping opentelemetry-proto focused purely on data definitions.
I have some idea on how to do this cleanup - can pick this sometime next week, if no one has concern, or looking into this.
closing this, as this will be covered through #3045.