This is an automated email from the ASF dual-hosted git repository.
csringhofer 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 2ac5a24dc IMPALA-14455: Cleanup OpenTelemetry Tracing Startup Flags
2ac5a24dc is described below
commit 2ac5a24dc0cfc9c9e7a1fc86cccf94cd1a2900af
Author: jasonmfehr <[email protected]>
AuthorDate: Thu Sep 18 15:40:38 2025 -0700
IMPALA-14455: Cleanup OpenTelemetry Tracing Startup Flags
Fixes several issues with the OpenTelemetry tracing startup flags:
1. otel_trace_beeswax -- Removes this hidden flag which enabled
tracing of queries submitted over Beeswax. Since this protocol is
deprecated and no tests assert the traces generated by Beeswax
queries, this flag was removed to eliminate an extra check when
determining if OpenTelemetry tracing should be enabled.
2. otel_trace_tls_minimum_version -- Fixes parsing of this flag's
value. This flag is in the format "tlsv1.2" or "tlsv1.3", but the
OpenTelemetry C++ SDK expects the minimum TLS version to be in the
format "1.2" or "1.3". The code now removes the "tlsv" prefix before
passing the value to the OpenTelemetry C++ SDK.
3. otel_trace_tls_insecure_skip_verify -- Fixes the guidance to only
set this flag to true in dev/testing.
Adds ctest tests for the functions that configure the TraceProvider
singleton to ensure startup flags are correctly parsed and applied.
Modifies the http_exporter_config and init_otel_tracer function
signatures in otel.cc to return the actual object they create instead
of a Status since these functions only ever returned OK.
Updates the OpenTelemetry collector docker-compose file to support
the collector receiving traces over both HTTP and HTTPS. This setup
is used to manually smoke test the integration from Impala to an
OpenTelemetry collector.
Change-Id: Ie321fa37c0fd260f783dc6cf47924d53a06d82ea
Reviewed-on: http://gerrit.cloudera.org:8080/23440
Tested-by: Impala Public Jenkins <[email protected]>
Reviewed-by: Joe McDonnell <[email protected]>
---
be/src/observe/otel-flags-trace.cc | 14 +-
be/src/observe/otel-test.cc | 287 +++++++++++++++++++--
be/src/observe/otel.cc | 142 ++++++----
be/src/observe/otel.h | 22 +-
be/src/runtime/exec-env.cc | 2 +-
be/src/testutil/localhost.key | 27 ++
be/src/testutil/localhost.pem | 20 ++
testdata/bin/otel-collector/README.md | 25 +-
testdata/bin/otel-collector/docker-compose.yml | 4 +-
.../{otel-config.yml => otel-config-http.yml} | 0
.../{otel-config.yml => otel-config-https.yml} | 5 +-
11 files changed, 462 insertions(+), 86 deletions(-)
diff --git a/be/src/observe/otel-flags-trace.cc
b/be/src/observe/otel-flags-trace.cc
index 001adb401..e116548d1 100644
--- a/be/src/observe/otel-flags-trace.cc
+++ b/be/src/observe/otel-flags-trace.cc
@@ -61,10 +61,6 @@ DEFINE_validator(otel_trace_exporter, [](const char*
flagname, const string& val
return false;
}); // flag otel_trace_exporter
-DEFINE_bool_hidden(otel_trace_beeswax, false, "Specifies whether or not to
trace queries "
- "submitted via the Beeswax protocol. This flag is hidden because tracing
Beeswax "
- "queries is not supported.");
-
//
// Start of HTTP related flags.
//
@@ -160,7 +156,8 @@ DEFINE_validator(otel_trace_ca_cert_string, [](const char*
flagname,
DEFINE_string(otel_trace_tls_minimum_version, "", "String containing the
minimum allowed "
- "TLS version, if not specified, defaults to the overall minimum TLS
version.");
+ "TLS version the OpenTelemetry SDK will use when communicating with the
collector. "
+ "If empty, will use the value of the 'ssl_minimum_version' flag.");
DEFINE_validator(otel_trace_tls_minimum_version, [](const char* flagname,
const string& value) {
if (value.empty() || value == impala::TLSVersions::TLSV1_2 || value ==
"tlsv1.3") {
@@ -172,13 +169,14 @@ DEFINE_validator(otel_trace_tls_minimum_version, [](const
char* flagname,
}); // flag otel_trace_tls_minimum_version
DEFINE_string(otel_trace_ssl_ciphers, "", "List of allowed TLS cipher suites
when using "
- "TLS 1.2, default to the value of Impala’s ssl_cipher_list startup flag.");
+ "TLS 1.2. If empty, defaults to the value of the 'ssl_cipher_list' startup
flag.");
DEFINE_string(otel_trace_tls_cipher_suites, "", "List of allowed TLS cipher
suites when "
- "using TLS 1.3, default to the value of Impala’s tls_ciphersuites startup
flag.");
+ "using TLS 1.3. If empty, defaults to the value of the 'tls_ciphersuites'
startup "
+ "flag.");
DEFINE_bool(otel_trace_tls_insecure_skip_verify, false, "If set to true, skips
"
- "verification of collector’s TLS certificate. This should only be set to
false for "
+ "verification of collector’s TLS certificate. This should only be set to
true for "
"development / testing");
//
// End of TLS related flags.
diff --git a/be/src/observe/otel-test.cc b/be/src/observe/otel-test.cc
index 0b5f04cb3..72af04394 100644
--- a/be/src/observe/otel-test.cc
+++ b/be/src/observe/otel-test.cc
@@ -17,21 +17,54 @@
#include "observe/otel.h"
+#include <chrono>
#include <string>
#include <string_view>
#include <boost/algorithm/string/replace.hpp>
#include <gtest/gtest.h>
#include "gutil/strings/substitute.h"
-
+#include <opentelemetry/exporters/otlp/otlp_file_exporter.h>
+#include <opentelemetry/exporters/otlp/otlp_http.h>
+#include <opentelemetry/exporters/otlp/otlp_http_exporter.h>
+#include <opentelemetry/exporters/otlp/otlp_http_exporter_options.h>
+#include <opentelemetry/sdk/common/global_log_handler.h>
+#include <opentelemetry/sdk/trace/batch_span_processor.h>
+#include <opentelemetry/sdk/trace/batch_span_processor_options.h>
+#include <opentelemetry/sdk/trace/simple_processor.h>
#include "gen-cpp/Query_types.h"
+#include "observe/otel-log-handler.h"
#include "testutil/scoped-flag-setter.h"
using namespace std;
using namespace impala;
+using namespace opentelemetry::sdk::common::internal_log;
+using namespace opentelemetry::sdk::trace;
+using namespace opentelemetry::exporter::otlp;
-DECLARE_bool(otel_trace_beeswax);
+DECLARE_bool(otel_debug);
+DECLARE_string(otel_trace_additional_headers);
+DECLARE_int32(otel_trace_batch_queue_size);
+DECLARE_int32(otel_trace_batch_max_batch_size);
+DECLARE_int32(otel_trace_batch_schedule_delay_ms);
+DECLARE_string(otel_trace_ca_cert_path);
+DECLARE_string(otel_trace_ca_cert_string);
DECLARE_string(otel_trace_collector_url);
+DECLARE_bool(otel_trace_compression);
+DECLARE_string(otel_trace_exporter);
+DECLARE_double(otel_trace_retry_policy_backoff_multiplier);
+DECLARE_double(otel_trace_retry_policy_initial_backoff_s);
+DECLARE_int32(otel_trace_retry_policy_max_attempts);
+DECLARE_int32(otel_trace_retry_policy_max_backoff_s);
+DECLARE_string(otel_trace_span_processor);
+DECLARE_string(otel_trace_ssl_ciphers);
+DECLARE_int32(otel_trace_timeout_s);
+DECLARE_string(otel_trace_tls_cipher_suites);
+DECLARE_bool(otel_trace_tls_insecure_skip_verify);
+DECLARE_string(otel_trace_tls_minimum_version);
+DECLARE_string(ssl_cipher_list);
+DECLARE_string(ssl_minimum_version);
+DECLARE_string(tls_ciphersuites);
TEST(OtelTest, QueriesTraced) {
const auto runtest = [](const string_view sql_str) -> void {
@@ -113,14 +146,6 @@ TEST(OtelTest, QueriesTraced) {
run_newline_test("INSERT", "INTO TABLE FOO");
run_newline_test("INVALIDATE", "METADATA FOO");
run_newline_test("WITH", "T1 AS SELECT * FROM FOO");
-
- // Beeswax queries are traced when the otel_trace_beeswax flag is set.
- {
- auto trace_beeswax_setter =
- ScopedFlagSetter<bool>::Make(&FLAGS_otel_trace_beeswax, true);
-
- EXPECT_TRUE(should_otel_trace_query("SELECT * FROM foo",
TSessionType::BEESWAX));
- }
}
TEST(OtelTest, QueriesNotTraced) {
@@ -170,16 +195,11 @@ TEST(OtelTest, QueriesNotTraced) {
runtest("--comment only\n");
runtest("--comment only\n--comment only 2");
runtest("--comment only\n--comment only 2\n");
- // TODO: Move to the QueriesTraced test case one IMPALA-14370 is fixed.
+ // TODO: Move to the QueriesTraced test case once IMPALA-14370 is fixed.
runtest(strings::Substitute("/*/ comment */select * from tbl"));
- // Beeswax queries are not traced unless the otel_trace_beeswax flag is set.
- {
- auto trace_beeswax_setter =
- ScopedFlagSetter<bool>::Make(&FLAGS_otel_trace_beeswax, false);
-
- EXPECT_FALSE(should_otel_trace_query("SELECT * FROM foo",
TSessionType::BEESWAX));
- }
+ // Beeswax queries are not traced.
+ EXPECT_FALSE(should_otel_trace_query("SELECT * FROM foo",
TSessionType::BEESWAX));
}
TEST(OtelTest, TLSEnabled) {
@@ -220,3 +240,234 @@ TEST(OtelTest, TLSNotEnabled) {
EXPECT_FALSE(test::otel_tls_enabled_for_testing());
}
}
+
+// Assert the default values of the OtlpHttpExporterOptions struct used to
configure the
+// OtlpHttpExporter.
+TEST(OtelTest, InitHttpDefaults) {
+ FLAGS_otel_trace_collector_url = "https://foo.com";;
+ FLAGS_ssl_minimum_version = "tlsv1.0";
+ FLAGS_ssl_cipher_list = "ssl_ciphers";
+ FLAGS_tls_ciphersuites = "tls_ciphers";
+
+ OtlpHttpExporterOptions actual = test::get_http_exporter_config();
+
+ EXPECT_EQ("https://foo.com";, actual.url);
+ EXPECT_EQ(HttpRequestContentType::kJson, actual.content_type);
+ EXPECT_EQ(false, actual.console_debug);
+ EXPECT_EQ(chrono::seconds(10), actual.timeout);
+ EXPECT_EQ(5, actual.retry_policy_max_attempts);
+ EXPECT_EQ(chrono::seconds(1), actual.retry_policy_initial_backoff);
+ EXPECT_EQ(chrono::duration<float>(5.0), actual.retry_policy_max_backoff);
+ EXPECT_EQ(2.0, actual.retry_policy_backoff_multiplier);
+ EXPECT_EQ("zlib", actual.compression);
+ EXPECT_EQ("1.0", actual.ssl_min_tls);
+ EXPECT_EQ("1.3", actual.ssl_max_tls);
+ EXPECT_EQ("ssl_ciphers", actual.ssl_cipher);
+ EXPECT_EQ("tls_ciphers", actual.ssl_cipher_suite);
+ EXPECT_EQ(false, actual.ssl_insecure_skip_verify);
+ EXPECT_EQ("", actual.ssl_ca_cert_path);
+ EXPECT_EQ("", actual.ssl_ca_cert_string);
+ EXPECT_EQ(0, actual.http_headers.size());
+ EXPECT_TRUE(actual.http_headers.empty());
+}
+
+// Assert the flags that customize the values of the OtlpHttpExporterOptions
struct used
+// to configure the OtlpHttpExporter.
+TEST(OtelTest, InitHttpOverrides) {
+ FLAGS_otel_trace_collector_url = "https://foo.com";;
+ FLAGS_otel_trace_tls_minimum_version = "tlsv1.3";
+ FLAGS_otel_trace_timeout_s = 9;
+ FLAGS_otel_debug = true;
+ FLAGS_otel_trace_retry_policy_max_attempts = 8;
+ FLAGS_otel_trace_retry_policy_initial_backoff_s = 7.0;
+ FLAGS_otel_trace_retry_policy_max_backoff_s = 6;
+ FLAGS_otel_trace_retry_policy_backoff_multiplier = 42.0;
+ FLAGS_otel_trace_ssl_ciphers = "override_ssl_ciphers";
+ FLAGS_otel_trace_tls_cipher_suites = "override_tls_ciphers";
+ FLAGS_otel_trace_tls_insecure_skip_verify = true;
+ FLAGS_otel_trace_ca_cert_path = "ca_cert_path";
+ FLAGS_otel_trace_ca_cert_string = "ca_cert_string";
+ FLAGS_otel_trace_compression = false;
+
+ OtlpHttpExporterOptions actual = test::get_http_exporter_config();
+
+ EXPECT_EQ("https://foo.com";, actual.url);
+ EXPECT_EQ(true, actual.console_debug);
+ EXPECT_EQ(chrono::seconds(9), actual.timeout);
+ EXPECT_EQ(8, actual.retry_policy_max_attempts);
+ EXPECT_EQ(chrono::seconds(7), actual.retry_policy_initial_backoff);
+ EXPECT_EQ(chrono::seconds(6), actual.retry_policy_max_backoff);
+ EXPECT_EQ(42.0, actual.retry_policy_backoff_multiplier);
+ EXPECT_EQ("1.3", actual.ssl_min_tls);
+ EXPECT_EQ("override_ssl_ciphers", actual.ssl_cipher);
+ EXPECT_EQ("override_tls_ciphers", actual.ssl_cipher_suite);
+ EXPECT_EQ(true, actual.ssl_insecure_skip_verify);
+ EXPECT_EQ("ca_cert_path", actual.ssl_ca_cert_path);
+ EXPECT_EQ("ca_cert_string", actual.ssl_ca_cert_string);
+ EXPECT_EQ("none", actual.compression);
+ EXPECT_TRUE(actual.http_headers.empty());
+}
+
+// The otel_trace_additional_headers flag allows for specifying arbitrary HTTP
headers
+// that are added to each HTTP request to the OTel collector. Assert one
additional header
+// is correctly parsed.
+TEST(OtelTest, InitOneHttpHeader) {
+ FLAGS_otel_trace_additional_headers = "foo=bar";
+ OtlpHttpExporterOptions actual = test::get_http_exporter_config();
+
+ EXPECT_EQ(1, actual.http_headers.size());
+ const auto val = actual.http_headers.find("foo");
+ ASSERT_NE(actual.http_headers.cend(), val) << "Could not find header with
key 'foo'";
+ EXPECT_EQ("bar", val->second);
+}
+
+// The otel_trace_additional_headers flag allows for specifying arbitrary HTTP
headers
+// that are added to each HTTP request to the OTel collector. Assert multiple
additional
+// headers (including the same header specified twice) are correctly parsed.
+TEST(OtelTest, InitMultipleHttpHeaders) {
+ FLAGS_otel_trace_additional_headers =
"foo=bar1:::foo2=bar3:::foo=bar2:::foo3=bar4";
+ OtlpHttpExporterOptions actual = test::get_http_exporter_config();
+
+ EXPECT_EQ(4, actual.http_headers.size());
+
+ const auto val2 = actual.http_headers.find("foo2");
+ ASSERT_NE(actual.http_headers.cend(), val2) << "Could not find header with
key 'foo2'";
+ EXPECT_EQ("bar3", val2->second);
+
+ const auto val3 = actual.http_headers.find("foo3");
+ ASSERT_NE(actual.http_headers.cend(), val3) << "Could not find header with
key 'foo3'";
+ EXPECT_EQ("bar4", val3->second);
+
+ bool val1_found = false;
+ bool val2_found = false;
+
+ for (auto iter : actual.http_headers) {
+ if (iter.first == "foo") {
+ if (iter.second == "bar1") {
+ val1_found = true;
+ } else if (iter.second == "bar2") {
+ val2_found = true;
+ }
+ }
+ }
+
+ EXPECT_TRUE(val1_found) << "Did not find header with key 'foo' and value
'bar1'";
+ EXPECT_TRUE(val2_found) << "Did not find header with key 'foo' and value
'bar2'";
+}
+
+// Assert the default values of the BatchSpanProcessorOptions struct used to
configure
+// the BatchSpanProcessor.
+TEST(OtelTest, BatchSpanProcessorDefaults) {
+ const BatchSpanProcessorOptions actual = test::get_batch_processor_config();
+
+ // Defaults come from the BatchSpanProcessorOptions struct definition.
+ EXPECT_EQ(512, actual.max_export_batch_size);
+ EXPECT_EQ(2048, actual.max_queue_size);
+ EXPECT_EQ(chrono::milliseconds(5000), actual.schedule_delay_millis);
+}
+
+// Assert the flags that customize the values of the BatchSpanProcessorOptions
struct
+// used to configure the BatchSpanProcessor.
+TEST(OtelTest, BatchSpanProcessorOverrides) {
+ FLAGS_otel_trace_batch_max_batch_size = 1;
+ FLAGS_otel_trace_batch_queue_size = 2;
+ FLAGS_otel_trace_batch_schedule_delay_ms = 3;
+
+ const BatchSpanProcessorOptions actual = test::get_batch_processor_config();
+
+ // Defaults come from the BatchSpanProcessorOptions struct definition.
+ EXPECT_EQ(1, actual.max_export_batch_size);
+ EXPECT_EQ(2, actual.max_queue_size);
+ EXPECT_EQ(chrono::milliseconds(3), actual.schedule_delay_millis);
+}
+
+// Assert an OtlpHttpExporter is created based on the default value of the
+// otel_trace_exporter flag and assert the default value of that flag.
+TEST(OtelTest, InitExporterHttp) {
+ EXPECT_EQ("otlp_http", FLAGS_otel_trace_exporter);
+ unique_ptr<SpanExporter> exporter = test::get_exporter();
+
+ ASSERT_NE(nullptr, exporter);
+ EXPECT_NE(nullptr, dynamic_cast<OtlpHttpExporter*>(exporter.get()));
+}
+
+// Assert an OtlpFileExporter is created when the otel_trace_exporter flag is
set to
+// "file".
+TEST(OtelTest, InitExporterFile) {
+ FLAGS_otel_trace_exporter = "file";
+ unique_ptr<SpanExporter> exporter = test::get_exporter();
+
+ ASSERT_NE(nullptr, exporter);
+ EXPECT_NE(nullptr, dynamic_cast<OtlpFileExporter*>(exporter.get()));
+}
+
+// Assert a BatchSpanProcessor is created based on the default value of the
+// otel_trace_span_processor flag and assert the default value of that flag.
+TEST(OtelTest, InitSpanProcessorBatch) {
+ EXPECT_EQ("batch", FLAGS_otel_trace_span_processor);
+ unique_ptr<SpanProcessor> processor = test::get_span_processor();
+
+ ASSERT_NE(nullptr, processor);
+ EXPECT_NE(nullptr, dynamic_cast<BatchSpanProcessor*>(processor.get()));
+}
+
+// Assert a SimpleSpanProcessor is created when the otel_trace_span_processor
flag is set
+// to "simple".
+TEST(OtelTest, InitSpanProcessorSimple) {
+ FLAGS_otel_trace_span_processor = "simple";
+ unique_ptr<SpanProcessor> processor = test::get_span_processor();
+
+ ASSERT_NE(nullptr, processor);
+ EXPECT_NE(nullptr, dynamic_cast<SimpleSpanProcessor*>(processor.get()));
+}
+
+// Assert that when otel_debug is true, the GlobalLogHandler is set to an
OtelLogHandler
+// instance and the OTel global log level is set to Debug.
+// Asserts the OTel global log handler is correctly initialized.
+TEST(OtelTest, InitLogHandlerDebug) {
+ FLAGS_otel_debug = true;
+ init_otel_tracer();
+
+ ASSERT_NE(nullptr, GlobalLogHandler::GetLogHandler().get());
+ EXPECT_NE(nullptr, dynamic_cast<OtelLogHandler*>(
+ GlobalLogHandler::GetLogHandler().get()));
+ EXPECT_EQ(LogLevel::Debug, GlobalLogHandler::GetLogLevel());
+
+ shutdown_otel_tracer();
+}
+
+// Assert that when otel_debug is false but VLOG(1) is enabled, the OTel
global log level
+// is set to Info.
+TEST(OtelTest, InitLogHandlerInfoV1) {
+ FLAGS_otel_debug = false;
+ auto vlog_setter = ScopedFlagSetter<int32_t>::Make(&FLAGS_v, 1);
+ init_otel_tracer();
+
+ EXPECT_EQ(LogLevel::Info, GlobalLogHandler::GetLogLevel());
+
+ shutdown_otel_tracer();
+}
+
+// Assert that when otel_debug is false but VLOG(2) is enabled, the OTel
global log level
+// is set to Info.
+TEST(OtelTest, InitLogHandlerInfoV2) {
+ FLAGS_otel_debug = false;
+ auto vlog_setter = ScopedFlagSetter<int32_t>::Make(&FLAGS_v, 2);
+ init_otel_tracer();
+
+ EXPECT_EQ(LogLevel::Info, GlobalLogHandler::GetLogLevel());
+
+ shutdown_otel_tracer();
+}
+
+// Assert that when otel_debug is false and VLOG(1) is disabled, the OTel
global log level
+// is set to None.
+TEST(OtelTest, InitLogHandlerNone) {
+ FLAGS_otel_debug = false;
+ auto vlog_setter = ScopedFlagSetter<int32_t>::Make(&FLAGS_v, 0);
+ init_otel_tracer();
+
+ EXPECT_EQ(LogLevel::None, GlobalLogHandler::GetLogLevel());
+
+ shutdown_otel_tracer();
+}
diff --git a/be/src/observe/otel.cc b/be/src/observe/otel.cc
index b46224110..e10d4afae 100644
--- a/be/src/observe/otel.cc
+++ b/be/src/observe/otel.cc
@@ -56,7 +56,6 @@
#include <opentelemetry/version.h>
#include "common/compiler-util.h"
-#include "common/status.h"
#include "common/version.h"
#include "gen-cpp/Query_types.h"
#include "observe/otel-log-handler.h"
@@ -75,7 +74,6 @@ DECLARE_string(otel_trace_additional_headers);
DECLARE_int32(otel_trace_batch_queue_size);
DECLARE_int32(otel_trace_batch_max_batch_size);
DECLARE_int32(otel_trace_batch_schedule_delay_ms);
-DECLARE_bool(otel_trace_beeswax);
DECLARE_string(otel_trace_ca_cert_path);
DECLARE_string(otel_trace_ca_cert_string);
DECLARE_string(otel_trace_collector_url);
@@ -144,7 +142,7 @@ static inline bool otel_tls_enabled() {
bool should_otel_trace_query(std::string_view sql,
const TSessionType::type& session_type) {
- if (LIKELY(!FLAGS_otel_trace_beeswax) && session_type ==
TSessionType::BEESWAX) {
+ if (UNLIKELY(session_type == TSessionType::BEESWAX)) {
return false;
}
@@ -198,12 +196,9 @@ bool should_otel_trace_query(std::string_view sql,
return false;
} // function should_otel_trace_query
-// Initializes an OtlpHttpExporter instance with configuration from global
flags. The
-// OtlpHttpExporter instance implements the SpanExporter interface. The
function parameter
-// `exporter` is an in-out parameter that will be populated with the created
-// OtlpHttpExporter instance. Returns Status::OK() on success, or an error
Status if
-// configuration fails.
-static Status init_exporter_http(unique_ptr<SpanExporter>& exporter) {
+// Creates an OtlpHttpExporterOptions struct instance with configuration from
global
+// startup flags.
+static OtlpHttpExporterOptions http_exporter_config() {
// Configure OTLP HTTP exporter
OtlpHttpExporterOptions opts;
opts.url = FLAGS_otel_trace_collector_url;
@@ -228,15 +223,20 @@ static Status
init_exporter_http(unique_ptr<SpanExporter>& exporter) {
// TLS Configurations
if (otel_tls_enabled()) {
+ // Set minimum TLS version to the value of the global ssl_minimum_version
flag.
+ // Since this flag is in the format "tlv1.2" or "tlsv1.3", we need to
+ // convert it to the format expected by OtlpHttpExporterOptions by
removing the
+ // "tlsv" prefix.
if (FLAGS_otel_trace_tls_minimum_version.empty()) {
- // Set minimum TLS version to the value of the global
ssl_minimum_version flag.
- // Since this flag is in the format "tlv1.2" or "tlsv1.3", we need to
- // convert it to the format expected by OtlpHttpExporterOptions.
if (!FLAGS_ssl_minimum_version.empty()) {
- opts.ssl_min_tls = FLAGS_ssl_minimum_version.substr(4); // Remove
"tlsv" prefix
+ opts.ssl_min_tls = FLAGS_ssl_minimum_version.substr(4);
+ } else {
+ LOG(WARNING) << "TLS is enabled for the OTel exporter, but neither the
"
+ "'ssl_minimum_version' nor the 'otel_trace_tls_minimum_version'
flags are "
+ "set.";
}
} else {
- opts.ssl_min_tls = FLAGS_otel_trace_tls_minimum_version;
+ opts.ssl_min_tls = FLAGS_otel_trace_tls_minimum_version.substr(4);
}
opts.ssl_insecure_skip_verify = FLAGS_otel_trace_tls_insecure_skip_verify;
@@ -247,6 +247,10 @@ static Status init_exporter_http(unique_ptr<SpanExporter>&
exporter) {
FLAGS_otel_trace_ssl_ciphers;
opts.ssl_cipher_suite = FLAGS_otel_trace_tls_cipher_suites.empty() ?
FLAGS_tls_ciphersuites : FLAGS_otel_trace_tls_cipher_suites;
+
+ VLOG(2) << "OTel minimum TLS version set to '" << opts.ssl_min_tls << "'";
+ VLOG(2) << "OTel TLS 1.2 allowed ciphers set to '" << opts.ssl_cipher <<
"'";
+ VLOG(2) << "OTel TLS 1.3 allowed ciphers set to '" <<
opts.ssl_cipher_suite << "'";
}
// Additional HTTP headers
@@ -261,10 +265,21 @@ static Status
init_exporter_http(unique_ptr<SpanExporter>& exporter) {
}
}
- exporter = OtlpHttpExporterFactory::Create(opts);
+ return opts;
+} // function http_exporter_config
+
+// Creates a BatchSpanProcessorOptions struct instance with configuration from
global
+// startup flags.
+static BatchSpanProcessorOptions batch_processor_config() {
+ BatchSpanProcessorOptions batch_opts;
- return Status::OK();
-} // function init_exporter_http
+ batch_opts.max_queue_size = FLAGS_otel_trace_batch_queue_size;
+ batch_opts.max_export_batch_size = FLAGS_otel_trace_batch_max_batch_size;
+ batch_opts.schedule_delay_millis =
+ chrono::milliseconds(FLAGS_otel_trace_batch_schedule_delay_ms);
+
+ return batch_opts;
+} // function batch_processor_config
// Initializes an OtlpFileExporter instance with configuration from global
flags. The
// OtlpFileExporter instance implements the SpanExporter interface. Returns a
unique_ptr
@@ -289,51 +304,31 @@ static unique_ptr<SpanExporter> init_exporter_file() {
return OtlpFileExporterFactory::Create(exporter_opts);
} // function init_exporter_file
-// Initializes the OpenTelemetry Tracer singleton with the configuration
defined in the
-// coordinator startup flags. Returns Status::OK() on success, or an error
Status if
-// configuration fails.
-Status init_otel_tracer() {
- LOG(INFO) << "Initializing OpenTelemetry tracing.";
- VLOG(2) << "OpenTelemetry version: " << OPENTELEMETRY_VERSION;
- VLOG(2) << "OpenTelemetry ABI version: " << OPENTELEMETRY_ABI_VERSION;
- VLOG(2) << "OpenTelemetry namespace: "
- << OPENTELEMETRY_STRINGIFY(OPENTELEMETRY_NAMESPACE);
-
- otel_log_handler_ = nostd::shared_ptr<LogHandler>(new OtelLogHandler());
- GlobalLogHandler::SetLogHandler(otel_log_handler_);
-
- // Set the OpenTelemetry SDK internal log level based on the current glog
level. The SDK
- // does not support changing the log level once a Provider has been created.
- if (FLAGS_otel_debug) {
- GlobalLogHandler::SetLogLevel(LogLevel::Debug);
- } else if (VLOG_IS_ON(1)) {
- GlobalLogHandler::SetLogLevel(LogLevel::Info);
- } else {
- GlobalLogHandler::SetLogLevel(LogLevel::None);
- }
-
+// Initializes a SpanExporter instance based on the FLAGS_otel_trace_exporter
flag.
+// Returns a unique_ptr which will always be initialized with the created
exporter.
+static unique_ptr<SpanExporter> init_exporter() {
unique_ptr<SpanExporter> exporter;
if(FLAGS_otel_trace_exporter == OTEL_EXPORTER_OTLP_HTTP) {
- RETURN_IF_ERROR(init_exporter_http(exporter));
+ exporter = OtlpHttpExporterFactory::Create(http_exporter_config());
} else {
exporter = init_exporter_file();
}
VLOG(2) << "OpenTelemetry exporter: " << FLAGS_otel_trace_exporter;
- // Set up tracer provider
+ return exporter;
+} // function init_exporter
+
+// Initializes a SpanProcessor instance based on the
FLAGS_otel_trace_span_processor flag.
+// Returns a unique_ptr which will always be initialized with the created
processor.
+static unique_ptr<SpanProcessor> init_span_processor() {
+ unique_ptr<SpanExporter> exporter = init_exporter();
unique_ptr<SpanProcessor> processor;
if (boost::iequals(trim_copy(FLAGS_otel_trace_span_processor),
SPAN_PROCESSOR_BATCH)) {
VLOG(2) << "Using BatchSpanProcessor for OpenTelemetry spans";
- BatchSpanProcessorOptions batch_opts;
-
- batch_opts.max_queue_size = FLAGS_otel_trace_batch_queue_size;
- batch_opts.max_export_batch_size = FLAGS_otel_trace_batch_max_batch_size;
- batch_opts.schedule_delay_millis =
- chrono::milliseconds(FLAGS_otel_trace_batch_schedule_delay_ms);
-
- processor = BatchSpanProcessorFactory::Create(move(exporter), batch_opts);
+ processor = BatchSpanProcessorFactory::Create(move(exporter),
+ batch_processor_config());
} else {
VLOG(2) << "Using SimpleSpanProcessor for OTel spans";
LOG(WARNING) << "Setting --otel_trace_span_processor=simple blocks the
query "
@@ -342,13 +337,37 @@ Status init_otel_tracer() {
processor = make_unique<SimpleSpanProcessor>(move(exporter));
}
- provider_ = TracerProviderFactory::Create(move(processor),
+ return processor;
+} // function init_span_processor
+
+// Initializes the OpenTelemetry Tracer singleton with the configuration
defined in the
+// coordinator startup flags. This tracer is stored in a static variabled
defined in this
+// file.
+void init_otel_tracer() {
+ LOG(INFO) << "Initializing OpenTelemetry tracing.";
+ VLOG(2) << "OpenTelemetry version: " << OPENTELEMETRY_VERSION;
+ VLOG(2) << "OpenTelemetry ABI version: " << OPENTELEMETRY_ABI_VERSION;
+ VLOG(2) << "OpenTelemetry namespace: "
+ << OPENTELEMETRY_STRINGIFY(OPENTELEMETRY_NAMESPACE);
+
+ otel_log_handler_ = nostd::shared_ptr<LogHandler>(new OtelLogHandler());
+ GlobalLogHandler::SetLogHandler(otel_log_handler_);
+
+ // Set the OpenTelemetry SDK internal log level based on the current glog
level. The SDK
+ // does not support changing the log level once a Provider has been created.
+ if (FLAGS_otel_debug) {
+ GlobalLogHandler::SetLogLevel(LogLevel::Debug);
+ } else if (VLOG_IS_ON(1)) {
+ GlobalLogHandler::SetLogLevel(LogLevel::Info);
+ } else {
+ GlobalLogHandler::SetLogLevel(LogLevel::None);
+ }
+
+ provider_ = TracerProviderFactory::Create(init_span_processor(),
sdk::resource::Resource::Create({
{"service.name", "Impala"},
{"service.version", GetDaemonBuildVersion()}
}));
-
- return Status::OK();
} // function init_otel_tracer
void shutdown_otel_tracer() {
@@ -371,6 +390,23 @@ namespace test {
bool otel_tls_enabled_for_testing() {
return otel_tls_enabled();
}
+
+OtlpHttpExporterOptions get_http_exporter_config() {
+ return http_exporter_config();
+}
+
+BatchSpanProcessorOptions get_batch_processor_config() {
+ return batch_processor_config();
+}
+
+unique_ptr<SpanExporter> get_exporter() {
+ return init_exporter();
+}
+
+unique_ptr<SpanProcessor> get_span_processor() {
+ return init_span_processor();
+}
+
} // 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 b11ea7569..c87a8d6a7 100644
--- a/be/src/observe/otel.h
+++ b/be/src/observe/otel.h
@@ -21,7 +21,11 @@
#include <string>
#include <string_view>
-#include "common/status.h"
+#include <opentelemetry/exporters/otlp/otlp_http_exporter_options.h>
+#include <opentelemetry/sdk/trace/batch_span_processor_options.h>
+#include <opentelemetry/sdk/trace/exporter.h>
+#include <opentelemetry/sdk/trace/processor.h>
+
#include "gen-cpp/Query_types.h"
#include "observe/span-manager.h"
#include "service/client-request-state.h"
@@ -47,7 +51,7 @@ bool should_otel_trace_query(std::string_view sql,
// Initializes the OpenTelemetry tracer with the configuration defined in the
coordinator
// startup flags (see otel-flags.cc and otel-flags-trace.cc for the list).
Does not verify
// that OpenTelemetry tracing is enabled (otel_trace_enabled flag).
-Status init_otel_tracer();
+void init_otel_tracer();
// Force flushes any buffered spans and shuts down the OpenTelemetry tracer.
void shutdown_otel_tracer();
@@ -58,6 +62,20 @@ 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();
+
+// Testing helper function to provide access to the static
http_exporter_config()
+// function.
+opentelemetry::exporter::otlp::OtlpHttpExporterOptions
get_http_exporter_config();
+
+// Testing helper function to provide access to the static
batch_processor_config()
+// function.
+opentelemetry::sdk::trace::BatchSpanProcessorOptions
get_batch_processor_config();
+
+// Testing helper function to provide access to the static init_exporter()
function.
+std::unique_ptr<opentelemetry::sdk::trace::SpanExporter> get_exporter();
+
+// Testing helper function to provide access to the static
init_span_processor() function.
+std::unique_ptr<opentelemetry::sdk::trace::SpanProcessor> get_span_processor();
} // namespace test
} // namespace impala
diff --git a/be/src/runtime/exec-env.cc b/be/src/runtime/exec-env.cc
index 8370db0f3..6dc9d1bd6 100644
--- a/be/src/runtime/exec-env.cc
+++ b/be/src/runtime/exec-env.cc
@@ -349,7 +349,7 @@ Status ExecEnv::Init() {
// Initialize OTel
if (FLAGS_is_coordinator && FLAGS_otel_trace_enabled) {
- RETURN_IF_ERROR(init_otel_tracer());
+ init_otel_tracer();
}
// Initialize thread pools
diff --git a/be/src/testutil/localhost.key b/be/src/testutil/localhost.key
new file mode 100644
index 000000000..287885f53
--- /dev/null
+++ b/be/src/testutil/localhost.key
@@ -0,0 +1,27 @@
+-----BEGIN RSA PRIVATE KEY-----
+MIIEowIBAAKCAQEAsdJ1cqffI9xssQysLHF4IiAzE9YSY1Ug3soiPPsJ6q32oWCS
+gHcV+u5/aV9v0PD2I4LGlm8rOtSiObCO6CNWrExB2Pznd02RwvJi90BCo8EcTH44
+676+69WiFCtrdQbwDJP8XRZLWNrkOPUTAuMOdGuQkKtjGoOTssJmVZzxv+UcJW2B
+Och0hCbivajB+e0qoTrTx1hPRmW/9sTVJNDIVPt0gMoc6EYndOz+4BNqhySBu2dG
+fBcaQ2PwCPh8Zgnztubw8iwBPIs1oNY3n3djWPnxo8ZG82nFqu9L435iy7/hGBZG
+DRUUgUizNdS8xCt0uL7TXOH/gllyF2hxVH+jxQIDAQABAoIBAHMwJp53+gbL4aiq
+0dkUQ8KvYwbldBHeciV/gMBJyfm4aPvOh/gprBZZajWC+fa6MGd+bk/CgZlhZhjC
+sz/SrHF+EGWUJghVOiezRcJuYPycuk0espabgCdawtwX6ErtjJBJSH+wUmyjlpUC
+xCbpmFJ33zSnoNHrC8EPRqUMvlkT5xaH3wWY6MmqKvurhmeVM6csYJy2LFnLuAcv
+Kyt0yNVsj4TYeABYfQRycLwiHE5oGhKgMva8M4qN98xtkZ9JnpLVlbC4Xy5xX4Hf
+2m4b0DR0QlmxxaahdxDvVyorSPE3oI9bZHot5Xjglq2hgg3pMMc5mxY5kFFpB6+9
+7AXP4kECgYEA3TfH+sSCLAocsKWioB0j6mG5kEB6oUW1pTC16LezEgM9O5H7Agqe
+SKQHHRgkyjkz7wiN5ByEP4srPDcEgEjSKeh01DyFCK1IrQA+mxLN3m//vDWCLHrb
+UiZSEXiw79jv8xqoCxMKj25UV/NTYp7LuNZOhsP10AOMmY3aGNBfKJECgYEAzcf1
+RXGXK3xwq0DpYshsrhLksm8GGYiKtNPle1y+jc0YK07k7d0UdcIfCPrXChhaWhUu
+lNWdIM9hYtN+nNyrUluG/dbS8A1Xiw/gmvCLf7wLJ52mjOKuY5V1yLiEWzd04Ysx
+3nz14Wv+/YsMJSNVky2dZa2PXxkM8LOjGmaRQfUCgYBXRc1oWiQ8uZSOABqDbluf
++QPbLAT1IOpDjE8Hy4ki3xJGMRZEvOmrIMMJsF+7RAwADnDkAHgQFZht/gqRjakU
+DXghzupw/OQCFGmehjGfwrGyj62WXLWv2BxidinfxccMMoT/MXjmExHFTOKlsp4O
+gsWiFycf9HaAkdzsEzCncQKBgQCjByTA5JIgKJFWi7GayjCX5G39E0pg1jUVt75Z
+8osg6niYbwOdkwYPmUBfK+NLoymJrrhdv7KheMqtseLgQU8Vi1+yIQyyk89kY6rM
+9X9/LiokM5jsivYf/Rv4bn6liZT2zwEuRA/EjHvSwONZVNoKJRxKnqs0azM+SwMP
++mxgNQKBgFpvqbAQyQFOWqngu2gMRV+D/K4VkY5mUGliwy9TOtUUAZ8YrmxI0vN8
+CP4WaV6iDvrXvomFRN94ycOxiSNRanQ5ItBava6PHo6YnFCdREaUDnTx3Jp1IGnk
+SZtV1P2LHzHlCtH0pZwhhOauveS7GPTBQDXZMJ3ufKfdBViNy738
+-----END RSA PRIVATE KEY-----
diff --git a/be/src/testutil/localhost.pem b/be/src/testutil/localhost.pem
new file mode 100644
index 000000000..223a994c1
--- /dev/null
+++ b/be/src/testutil/localhost.pem
@@ -0,0 +1,20 @@
+-----BEGIN CERTIFICATE-----
+MIIDTjCCAjagAwIBAgIUB8U8sXAUFbgKk2FmztEoZkC6JFswDQYJKoZIhvcNAQEL
+BQAwgYsxCzAJBgNVBAYTAlVTMQswCQYDVQQIDAJDQTELMAkGA1UEBwwCU0YxETAP
+BgNVBAoMCENsb3VkZXJhMQ8wDQYDVQQLDAZJbXBhbGExGTAXBgNVBAMMEFdpbGRj
+YXJkIFJvb3QgQ0ExIzAhBgkqhkiG9w0BCQEWFHNhaWxlc2hAY2xvdWRlcmEuY29t
+MCAXDTI1MTAxMzE0MTQ0MloYDzIxMjUwOTE5MTQxNDQyWjAUMRIwEAYDVQQDDAls
+b2NhbGhvc3QwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCx0nVyp98j
+3GyxDKwscXgiIDMT1hJjVSDeyiI8+wnqrfahYJKAdxX67n9pX2/Q8PYjgsaWbys6
+1KI5sI7oI1asTEHY/Od3TZHC8mL3QEKjwRxMfjjrvr7r1aIUK2t1BvAMk/xdFktY
+2uQ49RMC4w50a5CQq2Mag5OywmZVnPG/5RwlbYE5yHSEJuK9qMH57SqhOtPHWE9G
+Zb/2xNUk0MhU+3SAyhzoRid07P7gE2qHJIG7Z0Z8FxpDY/AI+HxmCfO25vDyLAE8
+izWg1jefd2NY+fGjxkbzacWq70vjfmLLv+EYFkYNFRSBSLM11LzEK3S4vtNc4f+C
+WXIXaHFUf6PFAgMBAAGjHjAcMBoGA1UdEQQTMBGCCWxvY2FsaG9zdIcEfwAAATAN
+BgkqhkiG9w0BAQsFAAOCAQEAmW0tQjNXzysc0iViaRpjapNfgDLIVSgw18dzmBYK
+mMK1kFFxgdXcr2PmzXs7dnuUnO5UbvqXdIsAa0UqSZwQvSjFbwpMLdEJ9fj4H9aH
+N42BkL/oDomI5CS2hKCcY7yFtLZaNQHfJRDbzd9EzmmVURtGDyGwqKfT6ThbT9r+
+ec+jVtR1Lj5tnCh0++7eJfUJaMZ6FXp6vluGvkoSZPywvXp7H8FXqRvcXrBRrvrb
+CoOVqrahmifyBRRL/uOBkakLxd/dbcArlo1I3P40LG7SHJ/QUwiflPVz2pApdMt+
+9HTvP6uRU0PdqxzoEvJ+mZA+ygePUdUF7xu9oM8OfxJXIQ==
+-----END CERTIFICATE-----
diff --git a/testdata/bin/otel-collector/README.md
b/testdata/bin/otel-collector/README.md
index b52c818c8..1733fbfa7 100644
--- a/testdata/bin/otel-collector/README.md
+++ b/testdata/bin/otel-collector/README.md
@@ -6,7 +6,8 @@ This directory contains configuration to run an [OpenTelemetry
Collector](https:
## Contents
-- [`otel-config.yml`](./otel-config.yml): OpenTelemetry Collector
configuration file.
+- [`otel-config-http.yml`](./otel-config-http.yml): OpenTelemetry Collector
configuration file with the OTLP receiver supporting http.
+- [`otel-config-https.yml`](./otel-config-https.yml): OpenTelemetry Collector
configuration file with the OTLP receiver supporting https.
- [`docker-compose.yml`](./docker-compose.yml): Alternative Docker Compose
setup for both services.
---
@@ -16,7 +17,11 @@ This directory contains configuration to run an
[OpenTelemetry Collector](https:
### Option 1: Run Interactively
```bash
+ # HTTP (default)
docker-compose -f testdata/bin/otel-collector/docker-compose.yml up
+
+ # HTTPS
+ PROTOCOL=https docker-compose -f
testdata/bin/otel-collector/docker-compose.yml up
```
- This command:
@@ -28,7 +33,11 @@ This directory contains configuration to run an
[OpenTelemetry Collector](https:
### Option 2: Run Detached
```bash
+ # HTTP (default)
docker-compose -f testdata/bin/otel-collector/docker-compose.yml up -d
+
+ # HTTP (default)
+ PROTOCOL=https docker-compose -f
testdata/bin/otel-collector/docker-compose.yml up -d
```
- This command:
@@ -61,7 +70,8 @@ The collector forwards all received traces to Jaeger using
OTLP/gRPC.
To send traces from an Impala cluster to this collector, start Impala with the
following arguments:
```bash
-./bin/start-impala-cluster.py \
+# HTTP (default)
+start-impala-cluster.py \
--cluster_size=2 \
--num_coordinators=1 \
--use_exclusive_coordinators \
@@ -69,6 +79,17 @@ To send traces from an Impala cluster to this collector,
start Impala with the f
--otel_trace_collector_url=http://localhost:55888/v1/traces
--otel_trace_span_processor=simple \
--cluster_id=local_cluster"
+
+# HTTPS
+start-impala-cluster.py \
+ --cluster_size=2 \
+ --num_coordinators=1 \
+ --use_exclusive_coordinators \
+ --impalad_args="-v=2 --otel_trace_enabled=true \
+ --otel_trace_collector_url=https://localhost:55888/v1/traces \
+ --otel_trace_ca_cert_path=${IMPALA_HOME}/be/src/testutil/wildcardCA.pem \
+ --otel_trace_span_processor=simple \
+ --cluster_id=local_cluster"
```
- Ensure the collector is running before starting Impala.
diff --git a/testdata/bin/otel-collector/docker-compose.yml
b/testdata/bin/otel-collector/docker-compose.yml
index d2ab69961..7ebd4cc66 100644
--- a/testdata/bin/otel-collector/docker-compose.yml
+++ b/testdata/bin/otel-collector/docker-compose.yml
@@ -37,7 +37,9 @@ services:
depends_on:
- jaeger
volumes:
- - ./otel-config.yml:/etc/otel/config.yml
+ - ./otel-config-${PROTOCOL:-http}.yml:/etc/otel/config.yml:ro
+ - ../../../be/src/testutil/localhost.pem:/etc/otel/localhost.pem:ro
+ - ../../../be/src/testutil/localhost.key:/etc/otel/localhost.key:ro
command: ["--config", "/etc/otel/config.yml"]
ports:
- "55888:55888" # Expose OTLP HTTP externally
diff --git a/testdata/bin/otel-collector/otel-config.yml
b/testdata/bin/otel-collector/otel-config-http.yml
similarity index 100%
copy from testdata/bin/otel-collector/otel-config.yml
copy to testdata/bin/otel-collector/otel-config-http.yml
diff --git a/testdata/bin/otel-collector/otel-config.yml
b/testdata/bin/otel-collector/otel-config-https.yml
similarity index 90%
rename from testdata/bin/otel-collector/otel-config.yml
rename to testdata/bin/otel-collector/otel-config-https.yml
index f15c35172..c4fb22c77 100644
--- a/testdata/bin/otel-collector/otel-config.yml
+++ b/testdata/bin/otel-collector/otel-config-https.yml
@@ -22,6 +22,9 @@ receivers:
protocols:
http:
endpoint: 0.0.0.0:55888
+ tls:
+ cert_file: /etc/otel/localhost.pem
+ key_file: /etc/otel/localhost.key
exporters:
otlp:
@@ -33,4 +36,4 @@ service:
pipelines:
traces:
receivers: [otlp]
- exporters: [otlp]
+ exporters: [otlp]
\ No newline at end of file
