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

GH-36013: [C++] Disabling bundled OpenTelemetry with Protobuf 3.22+ #36016

Merged
merged 5 commits into from
Jun 16, 2023

Conversation

bkietz
Copy link
Member

@bkietz bkietz commented Jun 9, 2023

OpenTelemetry and Protobuf 3.22+ have conflicting versions of Abseil, which is causing build errors. This patch disables OpenTelemetry on macOS because Homebrew provides Protobuf 3.22+.

@bkietz bkietz requested a review from lidavidm June 9, 2023 19:25
@github-actions
Copy link

github-actions bot commented Jun 9, 2023

⚠️ GitHub issue #36013 has been automatically assigned in GitHub to PR creator.

@lidavidm
Copy link
Member

lidavidm commented Jun 9, 2023

LGTM, I think the macOS issue is #35987

@bkietz bkietz marked this pull request as ready for review June 9, 2023 21:04
ci/docker/ubuntu-22.04-cpp.dockerfile Outdated Show resolved Hide resolved
cpp/cmake_modules/ThirdpartyToolchain.cmake Outdated Show resolved Hide resolved
@kou kou changed the title GH-36013: [C++] Disabling bundled opentelemetry GH-36013: [C++] Disabling bundled OpenTelemetry Jun 9, 2023
@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Jun 9, 2023
@kou
Copy link
Member

kou commented Jun 9, 2023

We may not need to disable OpenTelemetry for Feodra 35, Ubuntu 20.04 and Ubuntu 22.04 because they use Protobuf < 3.22.

They weren't failed in our nightly CI: https://lists.apache.org/thread/yoq1zstmq5fpcg2kxw811pmlq0f8qnvn

@bkietz
Copy link
Member Author

bkietz commented Jun 9, 2023

Hmm, and there's still a link failure in macOS https://github.com/apache/arrow/actions/runs/5225374020/jobs/9434755803?pr=36016#step:9:1546

@kou
Copy link
Member

kou commented Jun 9, 2023

The failure is #35987 as David said. It's not related to this.

@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jun 9, 2023
@kou
Copy link
Member

kou commented Jun 10, 2023

@github-actions crossbow submit test-fedora-35-cpp

@github-actions
Copy link

Revision: 58c073a

Submitted crossbow builds: ursacomputing/crossbow @ actions-df4ae3fe29

Task Status
test-fedora-35-cpp Github Actions

@kou
Copy link
Member

kou commented Jun 10, 2023

The Fedora 35 job passed.
How about reverting Ubuntu 20.04/22.04 changes too?

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Jun 10, 2023
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jun 15, 2023
@kou
Copy link
Member

kou commented Jun 15, 2023

@github-actions crossbow submit test-ubuntu-*-cpp

@github-actions
Copy link

Revision: bdfba60

Submitted crossbow builds: ursacomputing/crossbow @ actions-3f91702e2f

Task Status
test-ubuntu-20.04-cpp Github Actions
test-ubuntu-22.04-cpp Github Actions

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@kou kou changed the title GH-36013: [C++] Disabling bundled OpenTelemetry GH-36013: [C++] Disabling bundled OpenTelemetry with Protobuf 3.22+ Jun 16, 2023
@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting change review Awaiting change review labels Jun 16, 2023
@kou kou merged commit 41309de into apache:main Jun 16, 2023
@bkietz bkietz deleted the 36013-disable-bundled-otel branch June 16, 2023 12:31
@conbench-apache-arrow
Copy link

Conbench analyzed the 6 benchmark runs on commit 41309de8.

There were 3 benchmark results indicating a performance regression:

The full Conbench report has more details.

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

Successfully merging this pull request may close these issues.

[C++] Bundled OpenTelemetry build is broken
3 participants