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

Consider removing tonic-build from opentelemetry-proto build dependencies #873

Closed
matklad opened this issue Sep 6, 2022 · 1 comment · Fixed by #881
Closed

Consider removing tonic-build from opentelemetry-proto build dependencies #873

matklad opened this issue Sep 6, 2022 · 1 comment · Fixed by #881
Labels
A-trace Area: issues related to tracing enhancement New feature or request

Comments

@matklad
Copy link
Contributor

matklad commented Sep 6, 2022

At the moment, opentelemetry-proto has tonic-build in its build dependencies:

[build-dependencies]
tonic-build = { version = "0.8.0", optional = true }
prost-build = { version = "0.11.1", optional = true }

This means .proto -> .rs compilation happens in the downstream projects which use otl-rust. That is, downstream project:

  • should have access to protoc during build
  • spend time compiling tonic-build itself, and running it on fixed otl protos
  • have more deps in Cargo.lock

An alternative would be to publish otl-proto crate with all Rust code pre-generated. A couple of ways to go about that:

  • commit generated code to the repo, with a #[test] which checks that the generated code is fresh (so, move tonic-build from a build-dep to a dev-deps. dev deps, unilke builddeps, don't "infect" downstream consumers)
  • run code generation specifically before publishing, such that the generated code gets into the .crate archive sumbitted to crates.io, but not into the git repo.
@djc
Copy link
Contributor

djc commented Sep 6, 2022

We already do something similar for opentelemetry-stackdriver, I guess we could easily copy that code over.

@TommyCpp TommyCpp added enhancement New feature or request A-trace Area: issues related to tracing labels Sep 16, 2022
readysetbot pushed a commit to readysettech/readyset that referenced this issue Feb 1, 2023
Depend on the latest git version of opentelemetry-otlp, to get the fix
for open-telemetry/opentelemetry-rust#873,
avoiding depending on the `protoc` binary at build-time.

Change-Id: I81950ee9aa29cd4945e028bc2484e39c87c2e331
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/4222
Tested-by: Buildkite CI
Reviewed-by: Ian Meyer <ian@readyset.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-trace Area: issues related to tracing enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants