Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Rename opentelemetry-proto/src/proto/tonic module #2508

Open
psandana opened this issue Jan 14, 2025 · 4 comments
Open

Proposal: Rename opentelemetry-proto/src/proto/tonic module #2508

psandana opened this issue Jan 14, 2025 · 4 comments

Comments

@psandana
Copy link
Contributor

As mentioned in open-telemetry/opentelemetry-rust-contrib#134 (comment), 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.

@lalitb
Copy link
Member

lalitb commented Jan 17, 2025

@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?

@psandana
Copy link
Contributor Author

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.

@lalitb
Copy link
Member

lalitb commented Jan 17, 2025

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.

@psandana
Copy link
Contributor Author

FYI, there is a feature gen-tonic-messages. Shall this be renamed also?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants