Skip to content

Commit 35b9a89

Browse files
authored
GH-46508: [C++] Upgrade OpenTelemetry cpp to avoid build error on recent Clang (#46509)
### Rationale for this change A warning is introduced in Clang 19.1.0 and AppleClang 17.0.0 that won't be demoted by `-Wno-error`, causing opentelemetry-cpp build failure. ### What changes are included in this PR? Upgrade opentelemetry-cpp (and opentelemetry-proto correspondingly) to the most recent version, which has addressed this issue in open-telemetry/opentelemetry-cpp#3133. With this upgrade, we found several false-positives in sanitizers. The reason seems to be that we build bundled third-party dependencies as static libraries and don't instrument their code. This is known to cause false-positives as per https://github.com/google/sanitizers/wiki/threadsanitizercppmanual#non-instrumented-code . So some suppressions and disablements are also made in this PR. ### Are these changes tested? Manually build pass. ### Are there any user-facing changes? None. * GitHub Issue: #46508 Authored-by: Rossi Sun <[email protected]> Signed-off-by: Rossi Sun <[email protected]>
1 parent d86682c commit 35b9a89

File tree

6 files changed

+42
-12
lines changed

6 files changed

+42
-12
lines changed
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing,
12+
# software distributed under the License is distributed on an
13+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
# KIND, either express or implied. See the License for the
15+
# specific language governing permissions and limitations
16+
# under the License.
17+
18+
# Note this file is merely a placeholder that contains no suppressions for now.
19+
# But it may become useful in the future.

cpp/build-support/lsan-suppressions.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,4 @@ leak:CRYPTO_zalloc
2626
# without LSAN_OPTIONS=fast_unwind_on_malloc=0:malloc_context_size=100
2727
leak:opentelemetry::v1::context::ThreadLocalContextStorage::GetStack
2828
leak:opentelemetry::v1::context::ThreadLocalContextStorage::Stack::Resize
29+
leak:std::make_shared<opentelemetry::v1::trace::NoopTracer>

cpp/build-support/run-test.sh

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,9 @@ function setup_sanitizers() {
7575
UBSAN_OPTIONS="$UBSAN_OPTIONS suppressions=$ROOT/build-support/ubsan-suppressions.txt"
7676
export UBSAN_OPTIONS
7777

78-
# Enable leak detection even under LLVM 3.4, where it was disabled by default.
79-
# This flag only takes effect when running an ASAN build.
80-
# ASAN_OPTIONS="$ASAN_OPTIONS detect_leaks=1"
81-
# export ASAN_OPTIONS
78+
# Set up suppressions for AddressSanitizer
79+
ASAN_OPTIONS="$ASAN_OPTIONS suppressions=$ROOT/build-support/asan-suppressions.txt"
80+
export ASAN_OPTIONS
8281

8382
# Set up suppressions for LeakSanitizer
8483
LSAN_OPTIONS="$LSAN_OPTIONS suppressions=$ROOT/build-support/lsan-suppressions.txt"

cpp/build-support/tsan-suppressions.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,6 @@
1717

1818
# Thread leak in CUDA
1919
thread:libcuda.so
20+
21+
# False-positives in OpenTelemetry because of non-instrumented code.
22+
race:^opentelemetry

cpp/src/arrow/flight/flight_test.cc

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,9 @@
7070
// > other API headers. This approach efficiently avoids the conflict
7171
// > between the two different versions of Abseil.
7272
#include "arrow/util/tracing_internal.h"
73-
#ifdef ARROW_WITH_OPENTELEMETRY
73+
// When running with OTel, ASAN reports false-positives that can't be easily suppressed.
74+
// Disable OTel for ASAN. See GH-46509.
75+
#if defined(ARROW_WITH_OPENTELEMETRY) && !defined(ADDRESS_SANITIZER)
7476
# include <opentelemetry/context/propagation/global_propagator.h>
7577
# include <opentelemetry/context/propagation/text_map_propagator.h>
7678
# include <opentelemetry/sdk/trace/processor.h>
@@ -95,7 +97,9 @@ const char kAuthHeader[] = "authorization";
9597
class OtelEnvironment : public ::testing::Environment {
9698
public:
9799
void SetUp() override {
98-
#ifdef ARROW_WITH_OPENTELEMETRY
100+
// When running with OTel, ASAN reports false-positives that can't be easily suppressed.
101+
// Disable OTel for ASAN. See GH-46509.
102+
#if defined(ARROW_WITH_OPENTELEMETRY) && !defined(ADDRESS_SANITIZER)
99103
// The default tracer always generates no-op spans which have no
100104
// span/trace ID. Set up a different tracer. Note, this needs to be run
101105
// before Arrow uses OTel as GetTracer() gets a tracer once and keeps it
@@ -1682,7 +1686,9 @@ class TracingTestServer : public FlightServerBase {
16821686
auto* middleware =
16831687
reinterpret_cast<TracingServerMiddleware*>(call_context.GetMiddleware("tracing"));
16841688
if (!middleware) return Status::Invalid("Could not find middleware");
1685-
#ifdef ARROW_WITH_OPENTELEMETRY
1689+
// When running with OTel, ASAN reports false-positives that can't be easily suppressed.
1690+
// Disable OTel for ASAN. See GH-46509.
1691+
#if defined(ARROW_WITH_OPENTELEMETRY) && !defined(ADDRESS_SANITIZER)
16861692
// Ensure the trace context is present (but the value is random so
16871693
// we cannot assert any particular value)
16881694
EXPECT_FALSE(middleware->GetTraceContext().empty());
@@ -1731,7 +1737,9 @@ class TestTracing : public ::testing::Test {
17311737
std::unique_ptr<FlightServerBase> server_;
17321738
};
17331739

1734-
#ifdef ARROW_WITH_OPENTELEMETRY
1740+
// When running with OTel, ASAN reports false-positives that can't be easily suppressed.
1741+
// Disable OTel for ASAN. See GH-46509.
1742+
#if defined(ARROW_WITH_OPENTELEMETRY) && !defined(ADDRESS_SANITIZER)
17351743
// Must define it ourselves to avoid a linker error
17361744
constexpr size_t kSpanIdSize = opentelemetry::trace::SpanId::kSize;
17371745
constexpr size_t kTraceIdSize = opentelemetry::trace::TraceId::kSize;

cpp/thirdparty/versions.txt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,10 @@ ARROW_MIMALLOC_BUILD_VERSION=v2.0.6
8686
ARROW_MIMALLOC_BUILD_SHA256_CHECKSUM=9f05c94cc2b017ed13698834ac2a3567b6339a8bde27640df5a1581d49d05ce5
8787
ARROW_NLOHMANN_JSON_BUILD_VERSION=v3.12.0
8888
ARROW_NLOHMANN_JSON_BUILD_SHA256_CHECKSUM=4b92eb0c06d10683f7447ce9406cb97cd4b453be18d7279320f7b2f025c10187
89-
ARROW_OPENTELEMETRY_BUILD_VERSION=v1.13.0
90-
ARROW_OPENTELEMETRY_BUILD_SHA256_CHECKSUM=7735cc56507149686e6019e06f588317099d4522480be5f38a2a09ec69af1706
91-
ARROW_OPENTELEMETRY_PROTO_BUILD_VERSION=v0.17.0
92-
ARROW_OPENTELEMETRY_PROTO_BUILD_SHA256_CHECKSUM=f269fbcb30e17b03caa1decd231ce826e59d7651c0f71c3b28eb5140b4bb5412
89+
ARROW_OPENTELEMETRY_BUILD_VERSION=v1.21.0
90+
ARROW_OPENTELEMETRY_BUILD_SHA256_CHECKSUM=98e5546f577a11b52a57faed1f4cc60d8c1daa44760eba393f43eab5a8ec46a2
91+
ARROW_OPENTELEMETRY_PROTO_BUILD_VERSION=v1.7.0
92+
ARROW_OPENTELEMETRY_PROTO_BUILD_SHA256_CHECKSUM=11330d850f5e24d34c4246bc8cb21fcd311e7565d219195713455a576bb11bed
9393
ARROW_ORC_BUILD_VERSION=2.1.2
9494
ARROW_ORC_BUILD_SHA256_CHECKSUM=55451e65dea6ed42afb39fe33a88f9dcea8928dca0a0c9c23ef5545587810b4c
9595
ARROW_PROTOBUF_BUILD_VERSION=v21.3

0 commit comments

Comments
 (0)