You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "bkietz (via GitHub)" <gi...@apache.org> on 2023/06/09 19:25:10 UTC
[GitHub] [arrow] bkietz opened a new pull request, #36016: GH-36013: [C++] Disabling bundled opentelemetry
bkietz opened a new pull request, #36016:
URL: https://github.com/apache/arrow/pull/36016
OpenTelemetry and Protobuf have conflicting versions of Abseil, which is causing build errors. This patch disables opentelemetry everywhere we can't rely on conda-forge's patched opentelemetry... which seems to be all the CI jobs?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on pull request #36016: GH-36013: [C++] Disabling bundled OpenTelemetry
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on PR #36016:
URL: https://github.com/apache/arrow/pull/36016#issuecomment-1585148345
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #36016: GH-36013: [C++] Disabling bundled opentelemetry
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #36016:
URL: https://github.com/apache/arrow/pull/36016#issuecomment-1585117627
LGTM, I think the macOS issue is https://github.com/apache/arrow/issues/35987
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #36016: GH-36013: [C++] Disabling bundled OpenTelemetry
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36016:
URL: https://github.com/apache/arrow/pull/36016#issuecomment-1585403131
Revision: 58c073a10c41c3906a1e7a3bb4cccf6c846edaa3
Submitted crossbow builds: [ursacomputing/crossbow @ actions-df4ae3fe29](https://github.com/ursacomputing/crossbow/branches/all?query=actions-df4ae3fe29)
|Task|Status|
|----|------|
|test-fedora-35-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/5227386181/jobs/9438961904)|
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #36016: GH-36013: [C++] Disabling bundled OpenTelemetry
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #36016:
URL: https://github.com/apache/arrow/pull/36016#issuecomment-1593667880
@github-actions crossbow submit test-ubuntu-*-cpp
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #36016: GH-36013: [C++] Disabling bundled OpenTelemetry
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #36016:
URL: https://github.com/apache/arrow/pull/36016#issuecomment-1585394789
@github-actions crossbow submit test-fedora-35-cpp
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #36016: GH-36013: [C++] Disabling bundled OpenTelemetry
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36016:
URL: https://github.com/apache/arrow/pull/36016#issuecomment-1593670376
Revision: bdfba604b5cfeafd0d78392618d40063b04813d2
Submitted crossbow builds: [ursacomputing/crossbow @ actions-3f91702e2f](https://github.com/ursacomputing/crossbow/branches/all?query=actions-3f91702e2f)
|Task|Status|
|----|------|
|test-ubuntu-20.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/5283192494/jobs/9559114022)|
|test-ubuntu-22.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/5283192711/jobs/9559114412)|
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #36016: GH-36013: [C++] Disabling bundled OpenTelemetry
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #36016:
URL: https://github.com/apache/arrow/pull/36016#discussion_r1224821404
##########
ci/docker/ubuntu-22.04-cpp.dockerfile:
##########
@@ -197,3 +196,7 @@ ENV absl_SOURCE=BUNDLED \
PATH=/usr/lib/ccache/:$PATH \
PYTHON=python3 \
xsimd_SOURCE=BUNDLED
+
+# GH-36013 disabling opentelemetry here because we can't
+# get the patched version from conda
+# ARROW_WITH_OPENTELEMETRY=ON \
Review Comment:
Ah, thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #36016: GH-36013: [C++] Disabling bundled opentelemetry
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36016:
URL: https://github.com/apache/arrow/pull/36016#issuecomment-1585037510
:warning: GitHub issue #36013 **has been automatically assigned in GitHub** to PR creator.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #36016: GH-36013: [C++] Disabling bundled OpenTelemetry
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #36016:
URL: https://github.com/apache/arrow/pull/36016#discussion_r1231130689
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -4511,6 +4511,7 @@ endif()
macro(build_opentelemetry)
message(STATUS "Building OpenTelemetry from source")
+ message(WARNING "This can lead to build errors due to conflicting abseil (GH-36013).")
Review Comment:
That looks good
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #36016: GH-36013: [C++] Disabling bundled OpenTelemetry
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #36016:
URL: https://github.com/apache/arrow/pull/36016#issuecomment-1585138346
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #36016: GH-36013: [C++] Disabling bundled OpenTelemetry
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #36016:
URL: https://github.com/apache/arrow/pull/36016#issuecomment-1585419265
The Fedora 35 job passed.
How about reverting Ubuntu 20.04/22.04 changes too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on a diff in pull request #36016: GH-36013: [C++] Disabling bundled opentelemetry
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #36016:
URL: https://github.com/apache/arrow/pull/36016#discussion_r1224777213
##########
ci/docker/ubuntu-22.04-cpp.dockerfile:
##########
@@ -197,3 +196,7 @@ ENV absl_SOURCE=BUNDLED \
PATH=/usr/lib/ccache/:$PATH \
PYTHON=python3 \
xsimd_SOURCE=BUNDLED
+
+# GH-36013 disabling opentelemetry here because we can't
+# get the patched version from conda
+# ARROW_WITH_OPENTELEMETRY=ON \
Review Comment:
We can use comment in `ENV`:
```diff
diff --git a/ci/docker/ubuntu-22.04-cpp.dockerfile b/ci/docker/ubuntu-22.04-cpp.dockerfile
index e6fd44ff2..6dc5ccbcb 100644
--- a/ci/docker/ubuntu-22.04-cpp.dockerfile
+++ b/ci/docker/ubuntu-22.04-cpp.dockerfile
@@ -184,7 +184,9 @@ ENV absl_SOURCE=BUNDLED \
ARROW_WITH_BROTLI=ON \
ARROW_WITH_BZ2=ON \
ARROW_WITH_LZ4=ON \
- ARROW_WITH_OPENTELEMETRY=ON \
+ # GH-36013 disabling opentelemetry here because we can't
+ # get the patched version from conda
+ # ARROW_WITH_OPENTELEMETRY=ON \
ARROW_WITH_SNAPPY=ON \
ARROW_WITH_ZLIB=ON \
ARROW_WITH_ZSTD=ON \
```
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -109,6 +109,12 @@ if("${jemalloc_SOURCE}" STREQUAL "")
set(jemalloc_SOURCE "BUNDLED")
endif()
+# GH-36013 bundled opentelemetry-cpp build is broken, change the default
+# to CONDA until a patched bundled version is available.
+if("${opentelemetry-cpp_SOURCE}" STREQUAL "")
+ set(opentelemetry-cpp_SOURCE "CONDA")
+endif()
+
Review Comment:
Do we need this?
I think that this is needless. I think that we just need to disable OpenTelemetry without conda.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou merged pull request #36016: GH-36013: [C++] Disabling bundled OpenTelemetry with Protobuf 3.22+
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou merged PR #36016:
URL: https://github.com/apache/arrow/pull/36016
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] conbench-apache-arrow[bot] commented on pull request #36016: GH-36013: [C++] Disabling bundled OpenTelemetry with Protobuf 3.22+
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #36016:
URL: https://github.com/apache/arrow/pull/36016#issuecomment-1595998944
Conbench analyzed the 6 benchmark runs on commit `41309de8`.
There were 3 benchmark results indicating a performance regression:
- Commit Run on `arm64-t4g-linux-compute` at [2023-06-16 05:46:09Z](http://conbench.ursa.dev/compare/runs/0e658fde2dea47aab7a97225764ae50a...8670f0e6b8fc4f3b95fbaa3d96f63a13/)
- [params=<STATIC_VECTOR(std::shared_ptr<int>)>, source=cpp-micro, suite=arrow-small-vector-benchmark](http://conbench.ursa.dev/compare/benchmarks/0648bdb0df847f4f8000fc263ba49ebf...0648bf766a28715d8000c8d58a611e20)
- Commit Run on `ursa-thinkcentre-m75q` at [2023-06-17 11:04:36Z](http://conbench.ursa.dev/compare/runs/1ad1a6969b5145a687d024863cb9e5b8...cc8581caa90941e791b48e9861f4677a/)
- [source=cpp-micro, suite=arrow-utf8-util-benchmark](http://conbench.ursa.dev/compare/benchmarks/0648d6c23531736f8000d67d0db65edb...0648d935cecd7df1800028542b065e26)
- and 1 more (see the report linked below)
The [full Conbench report](https://github.com/apache/arrow/runs/14350779741) has more details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #36016: GH-36013: [C++] Disabling bundled OpenTelemetry
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #36016:
URL: https://github.com/apache/arrow/pull/36016#issuecomment-1585152786
The failure is #35987 as David said. It's not related to this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on a diff in pull request #36016: GH-36013: [C++] Disabling bundled OpenTelemetry
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #36016:
URL: https://github.com/apache/arrow/pull/36016#discussion_r1224998638
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -4511,6 +4511,7 @@ endif()
macro(build_opentelemetry)
message(STATUS "Building OpenTelemetry from source")
+ message(WARNING "This can lead to build errors due to conflicting abseil (GH-36013).")
Review Comment:
How about reporting an error if Protobuf is 3.22 or later?
```suggestion
if(Protobuf_VERSION VERSION_GREATER_EQUAL 3.22)
message(FATAL_ERROR "GH-36013: Can't use bundled OpenTelemetry with Protobuf 3.22 or later: ${Protobuf_VERSION}")
endif()
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org