Isolation of setup-protoc jobs from RUST-CI by luckymachi · Pull Request #2772 · open-telemetry/otel-arrow

luckymachi · 2026-04-28T15:34:46Z

Change Summary

As mentioned in issue 2768, setup-protoc jobs are dropped in favour of a targeted compile_proto job.

What issue does this PR close?

This issue closes issue 2768

linux-foundation-easycla · 2026-04-28T15:34:52Z

The committers listed above are authorized under a signed CLA.

✅ login: drewrelmas / name: Drew Relmas (0459f98, 50925be)
✅ login: luckymachi / name: luckymachi (5b93645, 66e5e74, f2b8ca6)
✅ login: luckymachi / name: Max Jacinto (0459f98)

codecov · 2026-04-28T18:15:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.09%. Comparing base (77f6f4b) to head (5b93645).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2772      +/-   ##
==========================================
- Coverage   86.09%   86.09%   -0.01%     
==========================================
  Files         694      694              
  Lines      263421   263421              
==========================================
- Hits       226789   226786       -3     
- Misses      36108    36111       +3     
  Partials      524      524

Components	Coverage Δ
otap-dataflow	`87.05% <ø> (-0.01%)`	⬇️
query_abstraction	`80.61% <ø> (ø)`
query_engine	`90.76% <ø> (ø)`
otel-arrow-go	`52.45% <ø> (ø)`
quiver	`92.25% <ø> (ø)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

drewrelmas · 2026-04-28T19:20:46Z

 #[derive(Clone, PartialEq, ::prost::Message)]
 pub struct ExportLogsServiceRequest {
-    #[prost(message, repeated, tag="1")]
+    #[prost(message, repeated, tag = "1")]


Can I assume that these changes to the .rs files are actually formatting changes missing in the main branch?

I assume it has been a while since someone ran and committed compile-proto and some tooling has updated resulting in these changes?

Yes, all changes related to src/proto/ are formatting ones; they all tackle specifically changing x=y to x = y, none of the content has been altered

Interestingly I tried the compile-proto on my machine from main branch and it didn't produce any diff. Perhaps the CI machine uses a different version/tool than I have... Did you get this diff when running locally?

Locally compile-proto did produce a diff for me (the exact ones in this commit) because my /proto dir got updated accordingly. In theory it should give the diff right now because main has the old x=y format; perhaps it could be good to add an extra cargo fmt within compile-proto?

I'd be careful about adding cargo fmt here. compile-proto already turns off formatting on purpose - xtask/src/genproto.rs sets cfg.format(false) . The intent seems to be that the committed .rs files should match the raw generator output exactly.

One small suggestion: it might be worth pinning the protoc version in arduino/setup-protoc so CI uses a stable compiler and a future upstream default change can't cause unrelated failures here.

Side note - since Cargo.lock isn't checked in (which is fine), prost-build and tonic-prost-build can also pick different versions on different machines. That's probably why compile-proto gives a diff for some people but not others. Pinning protoc won't fully fix that, but it's one less thing that can drift.

Sounds reasonable! Is there an specific protoc version that would be preferred for CI? https://github.com/protocolbuffers/protobuf/releases

I'd suggest using 34.1, that's the current stable protobuf release - https://github.com/protocolbuffers/protobuf/releases/tag/v34.1

Awesome, should be good to go now. Also pinned the version in rust-validation-tests.yml and rust-bench.yml to keep it consistent 👍

Co-authored-by: Drew Relmas <drewrelmas@gmail.com>

drewrelmas

LGTM, provided protoc action is removed in additional places it is not needed

drewrelmas · 2026-04-29T19:53:32Z

@@ -49,6 +50,7 @@ jobs:
      - uses: arduino/setup-protoc@c65c819552d16ad3c9b72d9dfd5ba5237b9c906b # v3.0.0
        with:
          repo-token: ${{ secrets.GITHUB_TOKEN }}
+          version: "34.1"


I don't think we need protoc in the rust-bench workflow for same reason, can you please remove?

drewrelmas · 2026-04-29T19:53:54Z

      - uses: arduino/setup-protoc@c65c819552d16ad3c9b72d9dfd5ba5237b9c906b # v3.0.0
        with:
          repo-token: ${{ secrets.GITHUB_TOKEN }}
+          version: "34.1"


Same here, potentially don't need it

Great points; just committed a new version removing protoc from both files 👍

setup-protoc reorg in rust-ci

66e5e74

luckymachi requested a review from a team as a code owner April 28, 2026 15:34

github-actions Bot added rust Pull requests that update Rust code ci-repo Repository maintenance, build, GH workflows, repo cleanup, or other chores labels Apr 28, 2026

github-project-automation Bot added this to OTel-Arrow Apr 28, 2026

drewrelmas reviewed Apr 28, 2026

View reviewed changes

Removed extra comment in new compile-proto job

0459f98

Co-authored-by: Drew Relmas <drewrelmas@gmail.com>

drewrelmas linked an issue Apr 29, 2026 that may be closed by this pull request

Isolate protoc install into a targeted Rust-CI job #2768

Closed

1 task

luckymachi force-pushed the rust-ci_setup-protoc_cleanup branch 3 times, most recently from 6ead2ef to 4147797 Compare April 29, 2026 18:40

drewrelmas approved these changes Apr 29, 2026

View reviewed changes

setup-protoc reorg in rust-ci

f2b8ca6

luckymachi force-pushed the rust-ci_setup-protoc_cleanup branch from 4147797 to f2b8ca6 Compare April 29, 2026 19:59

drewrelmas and others added 2 commits April 29, 2026 13:02

Merge branch 'main' into rust-ci_setup-protoc_cleanup

50925be

setup-protoc reorg in rust-ci

5b93645

lalitb approved these changes Apr 29, 2026

View reviewed changes

drewrelmas enabled auto-merge April 29, 2026 20:37

drewrelmas added this pull request to the merge queue Apr 29, 2026

Merged via the queue into open-telemetry:main with commit 7757946 Apr 29, 2026
85 checks passed

github-project-automation Bot moved this to Done in OTel-Arrow Apr 29, 2026

Conversation

luckymachi commented Apr 28, 2026

Change Summary

What issue does this PR close?

Uh oh!

linux-foundation-easycla Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luckymachi Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luckymachi Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lalitb Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

drewrelmas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

linux-foundation-easycla Bot commented Apr 28, 2026 •

edited

Loading

codecov Bot commented Apr 28, 2026 •

edited

Loading

luckymachi Apr 28, 2026 •

edited

Loading

luckymachi Apr 28, 2026 •

edited

Loading

lalitb Apr 29, 2026 •

edited

Loading