This is an automated email from the ASF dual-hosted git repository.
boroknagyz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
The following commit(s) were added to refs/heads/master by this push:
new 2efd22bd7 IMPALA-14403: Fix OpenTelemetry TLS Detection
2efd22bd7 is described below
commit 2efd22bd73f50fe77249dabb4f1ae551e5fe7bbe
Author: jasonmfehr <[email protected]>
AuthorDate: Mon Sep 8 16:30:27 2025 -0700
IMPALA-14403: Fix OpenTelemetry TLS Detection
Fixes the code that detects if the OpenTelemetry collector is using
TLS. Previously, the code only worked properly when the collector URL
was all lowercase. Also removes unnecessary checks that could cause
TLS to be enabled even when the collector URL scheme was not https.
Testing accomplished by adding new ctest tests.
Change-Id: I3bf74f1353545d280575cdb94cf135e55c580ec7
Reviewed-on: http://gerrit.cloudera.org:8080/23397
Reviewed-by: Impala Public Jenkins <[email protected]>
Tested-by: Impala Public Jenkins <[email protected]>
---
be/src/observe/otel-test.cc | 42 +++++++++++++++++++++++++++++++++++++++++-
be/src/observe/otel.cc | 13 +++++++------
be/src/observe/otel.h | 5 +++++
3 files changed, 53 insertions(+), 7 deletions(-)
diff --git a/be/src/observe/otel-test.cc b/be/src/observe/otel-test.cc
index 2a46a22a6..0b5f04cb3 100644
--- a/be/src/observe/otel-test.cc
+++ b/be/src/observe/otel-test.cc
@@ -31,6 +31,7 @@ using namespace std;
using namespace impala;
DECLARE_bool(otel_trace_beeswax);
+DECLARE_string(otel_trace_collector_url);
TEST(OtelTest, QueriesTraced) {
const auto runtest = [](const string_view sql_str) -> void {
@@ -116,7 +117,7 @@ TEST(OtelTest, QueriesTraced) {
// Beeswax queries are traced when the otel_trace_beeswax flag is set.
{
auto trace_beeswax_setter =
- ScopedFlagSetter<bool>::Make(&FLAGS_otel_trace_beeswax,true);
+ ScopedFlagSetter<bool>::Make(&FLAGS_otel_trace_beeswax, true);
EXPECT_TRUE(should_otel_trace_query("SELECT * FROM foo",
TSessionType::BEESWAX));
}
@@ -180,3 +181,42 @@ TEST(OtelTest, QueriesNotTraced) {
EXPECT_FALSE(should_otel_trace_query("SELECT * FROM foo",
TSessionType::BEESWAX));
}
}
+
+TEST(OtelTest, TLSEnabled) {
+ {
+ auto ca_cert_path_setter =
+ ScopedFlagSetter<string>::Make(&FLAGS_otel_trace_collector_url,
"https://foo.com");
+ // NOLINTNEXTLINE(clang-diagnostic-error-undeclared-identifier)
+ EXPECT_TRUE(test::otel_tls_enabled_for_testing());
+ }
+
+ {
+ auto ca_cert_path_setter =
+ ScopedFlagSetter<string>::Make(&FLAGS_otel_trace_collector_url,
"HTTPS://foo.com");
+ // NOLINTNEXTLINE(clang-diagnostic-error-undeclared-identifier)
+ EXPECT_TRUE(test::otel_tls_enabled_for_testing());
+ }
+}
+
+TEST(OtelTest, TLSNotEnabled) {
+ {
+ auto ca_cert_path_setter =
+ ScopedFlagSetter<string>::Make(&FLAGS_otel_trace_collector_url, "");
+ // NOLINTNEXTLINE(clang-diagnostic-error-undeclared-identifier)
+ EXPECT_FALSE(test::otel_tls_enabled_for_testing());
+ }
+
+ {
+ auto ca_cert_path_setter =
+ ScopedFlagSetter<string>::Make(&FLAGS_otel_trace_collector_url,
"http://foo.com");
+ // NOLINTNEXTLINE(clang-diagnostic-error-undeclared-identifier)
+ EXPECT_FALSE(test::otel_tls_enabled_for_testing());
+ }
+
+ {
+ auto ca_cert_path_setter =
+ ScopedFlagSetter<string>::Make(&FLAGS_otel_trace_collector_url,
"HTTP://foo.com");
+ // NOLINTNEXTLINE(clang-diagnostic-error-undeclared-identifier)
+ EXPECT_FALSE(test::otel_tls_enabled_for_testing());
+ }
+}
diff --git a/be/src/observe/otel.cc b/be/src/observe/otel.cc
index 812ce1d8f..dbd71434b 100644
--- a/be/src/observe/otel.cc
+++ b/be/src/observe/otel.cc
@@ -133,12 +133,7 @@ static unique_ptr<trace::TracerProvider> provider_;
// Returns true if any TLS configuration flags are set for the OTel exporter.
static inline bool otel_tls_enabled() {
- return !FLAGS_otel_trace_ca_cert_path.empty()
- || !FLAGS_otel_trace_ca_cert_string.empty()
- || !FLAGS_otel_trace_tls_minimum_version.empty()
- || !FLAGS_otel_trace_ssl_ciphers.empty()
- || !FLAGS_otel_trace_tls_cipher_suites.empty()
- || FLAGS_otel_trace_collector_url.find("https://") == 0;
+ return boost::algorithm::istarts_with(FLAGS_otel_trace_collector_url,
"https://");
} // function otel_tls_enabled
bool should_otel_trace_query(std::string_view sql,
@@ -368,4 +363,10 @@ shared_ptr<SpanManager>
build_span_manager(ClientRequestState* crs) {
provider_->GetTracer(SCOPE_SPAN_NAME, SCOPE_SPAN_SPEC_VERSION), crs);
} // function build_span_manager
+namespace test {
+bool otel_tls_enabled_for_testing() {
+ return otel_tls_enabled();
+}
+} // namespace test
+
} // namespace impala
\ No newline at end of file
diff --git a/be/src/observe/otel.h b/be/src/observe/otel.h
index 025715eb6..b11ea7569 100644
--- a/be/src/observe/otel.h
+++ b/be/src/observe/otel.h
@@ -55,4 +55,9 @@ void shutdown_otel_tracer();
// Builds a SpanManager instance for the given query.
std::shared_ptr<SpanManager> build_span_manager(ClientRequestState*);
+namespace test {
+// Testing helper function to provide access to the static otel_tls_enabled()
function.
+bool otel_tls_enabled_for_testing();
+} // namespace test
+
} // namespace impala